Add a config for the v1.2.5 CCI found on msm8953 which has different
values in .params compared to others already supported in the driver.
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
drivers/i2c/busses/i2c-qcom-cci.c | 46 +++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
index a3afa11a71a10dbb720ee9acb566991fe55b98a0..2e3ceafec6cc755ea6083f2e6b85240d1a6d9187 100644
--- a/drivers/i2c/busses/i2c-qcom-cci.c
+++ b/drivers/i2c/busses/i2c-qcom-cci.c
@@ -785,8 +785,54 @@ static const struct cci_data cci_v2_data = {
},
};
+static const struct cci_data cci_msm8953_data = {
+ .num_masters = 2,
+ .queue_size = { 64, 16 },
+ .quirks = {
+ .max_write_len = 11,
+ .max_read_len = 12,
+ },
+ .params[I2C_MODE_STANDARD] = {
+ .thigh = 78,
+ .tlow = 114,
+ .tsu_sto = 28,
+ .tsu_sta = 28,
+ .thd_dat = 10,
+ .thd_sta = 77,
+ .tbuf = 118,
+ .scl_stretch_en = 0,
+ .trdhld = 6,
+ .tsp = 1
+ },
+ .params[I2C_MODE_FAST] = {
+ .thigh = 20,
+ .tlow = 28,
+ .tsu_sto = 21,
+ .tsu_sta = 21,
+ .thd_dat = 13,
+ .thd_sta = 18,
+ .tbuf = 32,
+ .scl_stretch_en = 0,
+ .trdhld = 6,
+ .tsp = 3
+ },
+ .params[I2C_MODE_FAST_PLUS] = {
+ .thigh = 16,
+ .tlow = 22,
+ .tsu_sto = 17,
+ .tsu_sta = 18,
+ .thd_dat = 16,
+ .thd_sta = 15,
+ .tbuf = 19,
+ .scl_stretch_en = 1,
+ .trdhld = 3,
+ .tsp = 3
+ },
+};
+
static const struct of_device_id cci_dt_match[] = {
{ .compatible = "qcom,msm8226-cci", .data = &cci_v1_data},
+ { .compatible = "qcom,msm8953-cci", .data = &cci_msm8953_data},
{ .compatible = "qcom,msm8974-cci", .data = &cci_v1_5_data},
{ .compatible = "qcom,msm8996-cci", .data = &cci_v2_data},
--
2.50.1On Sun, Aug 10, 2025 at 05:37:53PM +0200, Luca Weiss wrote:
> Add a config for the v1.2.5 CCI found on msm8953 which has different
Given the above version number...
> static const struct of_device_id cci_dt_match[] = {
> { .compatible = "qcom,msm8226-cci", .data = &cci_v1_data},
> + { .compatible = "qcom,msm8953-cci", .data = &cci_msm8953_data},
... why don't we use it here to stay consistent? cci_v1_2_5_data?
> { .compatible = "qcom,msm8974-cci", .data = &cci_v1_5_data},
> { .compatible = "qcom,msm8996-cci", .data = &cci_v2_data},
Hi Wolfram,
On 2025-08-11 14:13, Wolfram Sang wrote:
> On Sun, Aug 10, 2025 at 05:37:53PM +0200, Luca Weiss wrote:
>> Add a config for the v1.2.5 CCI found on msm8953 which has different
>
> Given the above version number...
>
>> static const struct of_device_id cci_dt_match[] = {
>> { .compatible = "qcom,msm8226-cci", .data = &cci_v1_data},
>> + { .compatible = "qcom,msm8953-cci", .data = &cci_msm8953_data},
>
> ... why don't we use it here to stay consistent? cci_v1_2_5_data?
I don't think the existing 'v2' or 'v1' configs have much to do with the
actual
HW_VERSION of the IP block. For example on of the newer Qualcomm SoCs
has HW
version 1.7.0 and is many years newer than the msm8996 which was called
'v2'.
I'm also not sure what these parameters depend on, if it's CCI HW
version, or
something else. So naming it after the SoC should be a safer bet. Also
the
msm8974-cci was only named 'v1.5' because it's an inbetween mix of the
v1 and
v2 that were already upstream so arguably that one shouldn't have been
called
v1.5 in the first place either.
Let me know what you think. Maybe also someone from Qualcomm/Linaro can
jump
in and share their thoughts, if someone knows more what these params
depend on.
Regards
Luca
>
>> { .compatible = "qcom,msm8974-cci", .data = &cci_v1_5_data},
>> { .compatible = "qcom,msm8996-cci", .data = &cci_v2_data},
On 8/15/25 9:12 AM, Luca Weiss wrote:
> Hi Wolfram,
>
> On 2025-08-11 14:13, Wolfram Sang wrote:
>> On Sun, Aug 10, 2025 at 05:37:53PM +0200, Luca Weiss wrote:
>>> Add a config for the v1.2.5 CCI found on msm8953 which has different
>>
>> Given the above version number...
>>
>>> static const struct of_device_id cci_dt_match[] = {
>>> { .compatible = "qcom,msm8226-cci", .data = &cci_v1_data},
>>> + { .compatible = "qcom,msm8953-cci", .data = &cci_msm8953_data},
>>
>> ... why don't we use it here to stay consistent? cci_v1_2_5_data?
>
> I don't think the existing 'v2' or 'v1' configs have much to do with the actual
> HW_VERSION of the IP block. For example on of the newer Qualcomm SoCs has HW
> version 1.7.0 and is many years newer than the msm8996 which was called 'v2'.
Most of the i2c-speed-dependent configs are electrical and therefore may
be configured as you wish.. I recall Sony Xperia kernels made changes to
that rather often, whether really necessary - I don't know.
The programming guide suggests a couple sets of values, picked in order
to meet the I2C spec.
The queue depths and max r/w lengths are per-instance (SoC specific) and
change very rarely.
>
> I'm also not sure what these parameters depend on, if it's CCI HW version, or
> something else. So naming it after the SoC should be a safer bet. Also the
> msm8974-cci was only named 'v1.5' because it's an inbetween mix of the v1 and
> v2 that were already upstream so arguably that one shouldn't have been called
> v1.5 in the first place either.
AFAICS there is no "v2" CCI in existence, msm8996 is v1.4.0 and newer
platforms are v1.x.y.
JFYI there's a revision ID register at (base + 0x0) which the driver's probe
function conveniently reads and prints with dev_dbg.
Konrad
Hi Luca, > I'm also not sure what these parameters depend on, if it's CCI HW version, > or > something else. So naming it after the SoC should be a safer bet. Also the > msm8974-cci was only named 'v1.5' because it's an inbetween mix of the v1 > and > v2 that were already upstream so arguably that one shouldn't have been > called > v1.5 in the first place either. > > Let me know what you think. Maybe also someone from Qualcomm/Linaro can jump > in and share their thoughts, if someone knows more what these params depend > on. Thanks for the heads up. I agree that it needs someone from Qualcomm for definite answers. But if nobody chimes in, your patch is good as is from my side. Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Thanks, Wolfram
On Fri, Aug 15, 2025 at 10:31 AM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > Hi Luca, > > > I'm also not sure what these parameters depend on, if it's CCI HW version, > > or > > something else. So naming it after the SoC should be a safer bet. Also the > > msm8974-cci was only named 'v1.5' because it's an inbetween mix of the v1 > > and > > v2 that were already upstream so arguably that one shouldn't have been > > called > > v1.5 in the first place either. That's correct, this is a local version, not matching HW IP version. The config depends both on the HW version and the CCI core clock. As our timings are statically configured we should also ensure that the CCI clock is correct... > > > > Let me know what you think. Maybe also someone from Qualcomm/Linaro can jump > > in and share their thoughts, if someone knows more what these params depend > > on. That's fair enough. Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
© 2016 - 2025 Red Hat, Inc.