[PATCH] i2c: qcom-cci: enable SCL clock stretching in standard mode on CCI v2

[PATCH] i2c: qcom-cci: enable SCL clock stretching in standard mode on CCI v2

[Hitesh Patel]

The CCI v2 standard-mode (100 kHz) timing table leaves SCL clock
stretching disabled, unlike the fast-plus table for the same block.
A slave that holds SCL low is then not waited for: the master keeps
its own clock timing and the transfer fails with a NACK or returns
corrupt data.

This is hit with a camera reached through a GMSL serializer/
deserializer I2C tunnel (MAX9296A/MAX96717 on the RB3 Gen2 vision
mezzanine). The deserializer acknowledges the address locally, but
forwards the transaction over the coax link and stretches SCL until
the remote side has completed it, which takes well over one clock
period at 100 kHz. Without stretching the register reads of the
sensor behind the link intermittently return garbage and writes are
dropped, which shows up as random sensor init failures.

Clock stretching is part of the I2C specification for every speed
mode and a device that does not stretch is unaffected by enabling it,
so enable it for standard mode as it already is for fast-plus.

Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
---
 drivers/i2c/busses/i2c-qcom-cci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
index 25b6e4e9e..327efa86f 100644
--- a/drivers/i2c/busses/i2c-qcom-cci.c
+++ b/drivers/i2c/busses/i2c-qcom-cci.c
@@ -723,7 +723,7 @@ static const struct cci_data cci_v2_data = {
 		.thd_dat = 22,
 		.thd_sta = 162,
 		.tbuf = 227,
-		.scl_stretch_en = 0,
+		.scl_stretch_en = 1,
 		.trdhld = 6,
 		.tsp = 3
 	},

base-commit: 68142f986ff04b2b70b31db00f719bf690f64a9a
-- 
2.43.0

Re: [PATCH] i2c: qcom-cci: enable SCL clock stretching in standard mode on CCI v2

[Konrad Dybcio]

On 9/14/26 3:23 PM, Hitesh Patel wrote:
> The CCI v2 standard-mode (100 kHz) timing table leaves SCL clock
> stretching disabled, unlike the fast-plus table for the same block.
> A slave that holds SCL low is then not waited for: the master keeps
> its own clock timing and the transfer fails with a NACK or returns
> corrupt data.
> 
> This is hit with a camera reached through a GMSL serializer/
> deserializer I2C tunnel (MAX9296A/MAX96717 on the RB3 Gen2 vision
> mezzanine). The deserializer acknowledges the address locally, but
> forwards the transaction over the coax link and stretches SCL until
> the remote side has completed it, which takes well over one clock
> period at 100 kHz. Without stretching the register reads of the
> sensor behind the link intermittently return garbage and writes are
> dropped, which shows up as random sensor init failures.
> 
> Clock stretching is part of the I2C specification for every speed
> mode and a device that does not stretch is unaffected by enabling it,
> so enable it for standard mode as it already is for fast-plus.
> 
> Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
> ---
>  drivers/i2c/busses/i2c-qcom-cci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
> index 25b6e4e9e..327efa86f 100644
> --- a/drivers/i2c/busses/i2c-qcom-cci.c
> +++ b/drivers/i2c/busses/i2c-qcom-cci.c
> @@ -723,7 +723,7 @@ static const struct cci_data cci_v2_data = {
>  		.thd_dat = 22,
>  		.thd_sta = 162,
>  		.tbuf = 227,
> -		.scl_stretch_en = 0,
> +		.scl_stretch_en = 1,

Would it make sense to keep it enabled unconditionally then?

Konrad

Re: [PATCH] i2c: qcom-cci: enable SCL clock stretching in standard mode on CCI v2

[Hitesh Patel]

On 21/09/26 17:04, Konrad Dybcio wrote:
> Would it make sense to keep it enabled unconditionally then?

Yes, I think so. Clock stretching is mandatory for every speed mode
in the I2C spec, and a slave that does not stretch never sees a
difference. The per-table scl_stretch_en field only exists because
the values were copied from the downstream tables, where all but the
msm8953 fast-plus one happen to be 0.

I'll send a v2 that sets the bit unconditionally in cci_init() and
drops the field. I can only test it on SC7280 (CCI v2, both 100 kHz
through the GMSL tunnel and 1 MHz for the on-board devices); the
other tables are untested but the change is the same for all of them.

Thanks,
Hitesh

[PATCH v2] i2c: qcom-cci: always enable SCL clock stretching

[Hitesh Patel]

Every CCI timing table except the msm8953 fast-plus one leaves SCL
clock stretching disabled in MISC_CTL. A slave that holds SCL low is
then not waited for: the master keeps its own clock timing and the
transfer fails with a NACK or returns corrupt data.

This is hit with a camera reached through a GMSL serializer/
deserializer I2C tunnel (MAX9296A/MAX96717 on the RB3 Gen2 vision
mezzanine, SC7280 CCI v2 at 100 kHz). The deserializer acknowledges
the address locally, forwards the transaction over the coax link and
stretches SCL until the remote side has completed it, which takes well
over one clock period. Without stretching the register reads of the
sensor behind the link intermittently return garbage and writes are
dropped, which shows up as random sensor init failures.

Clock stretching is part of the I2C specification for every speed
mode and a device that does not stretch is unaffected by it, so there
is no reason to make it a per-table parameter. Set the bit
unconditionally and drop the scl_stretch_en field.

Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>
---
 drivers/i2c/busses/i2c-qcom-cci.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
index 873e901..fbd5507 100644
--- a/drivers/i2c/busses/i2c-qcom-cci.c
+++ b/drivers/i2c/busses/i2c-qcom-cci.c
@@ -27,6 +27,7 @@
 #define CCI_I2C_Mm_SDA_CTL_1(m)			(0x108 + 0x100 * (m))
 #define CCI_I2C_Mm_SDA_CTL_2(m)			(0x10c + 0x100 * (m))
 #define CCI_I2C_Mm_MISC_CTL(m)			(0x110 + 0x100 * (m))
+#define CCI_I2C_MISC_CTL_SCL_STRETCH_EN	BIT(8)
 
 #define CCI_I2C_Mm_READ_DATA(m)			(0x118 + 0x100 * (m))
 #define CCI_I2C_Mm_READ_BUF_LEVEL(m)		(0x11c + 0x100 * (m))
@@ -97,7 +98,6 @@ struct hw_params {
 	u16 thd_dat; /* data hold time */
 	u16 thd_sta; /* hold time (repeated) START condition */
 	u16 tbuf; /* bus free time between a STOP and START condition */
-	u8 scl_stretch_en;
 	u16 trdhld;
 	u16 tsp; /* pulse width of spikes suppressed by the input filter */
 };
@@ -263,7 +263,7 @@ static void cci_init(struct cci *cci)
 		val = hw->tbuf;
 		writel(val, cci->base + CCI_I2C_Mm_SDA_CTL_2(i));
 
-		val = hw->scl_stretch_en << 8 | hw->trdhld << 4 | hw->tsp;
+		val = CCI_I2C_MISC_CTL_SCL_STRETCH_EN | hw->trdhld << 4 | hw->tsp;
 		writel(val, cci->base + CCI_I2C_Mm_MISC_CTL(i));
 	}
 }
@@ -659,7 +659,6 @@ static const struct cci_data cci_v1_data = {
 		.thd_dat = 10,
 		.thd_sta = 77,
 		.tbuf = 118,
-		.scl_stretch_en = 0,
 		.trdhld = 6,
 		.tsp = 1
 	},
@@ -671,7 +670,6 @@ static const struct cci_data cci_v1_data = {
 		.thd_dat = 13,
 		.thd_sta = 18,
 		.tbuf = 32,
-		.scl_stretch_en = 0,
 		.trdhld = 6,
 		.tsp = 3
 	},
@@ -692,7 +690,6 @@ static const struct cci_data cci_v1_5_data = {
 		.thd_dat = 10,
 		.thd_sta = 77,
 		.tbuf = 118,
-		.scl_stretch_en = 0,
 		.trdhld = 6,
 		.tsp = 1
 	},
@@ -704,7 +701,6 @@ static const struct cci_data cci_v1_5_data = {
 		.thd_dat = 13,
 		.thd_sta = 18,
 		.tbuf = 32,
-		.scl_stretch_en = 0,
 		.trdhld = 6,
 		.tsp = 3
 	},
@@ -725,7 +721,6 @@ static const struct cci_data cci_v2_data = {
 		.thd_dat = 22,
 		.thd_sta = 162,
 		.tbuf = 227,
-		.scl_stretch_en = 0,
 		.trdhld = 6,
 		.tsp = 3
 	},
@@ -737,7 +732,6 @@ static const struct cci_data cci_v2_data = {
 		.thd_dat = 22,
 		.thd_sta = 35,
 		.tbuf = 62,
-		.scl_stretch_en = 0,
 		.trdhld = 6,
 		.tsp = 3
 	},
@@ -749,7 +743,6 @@ static const struct cci_data cci_v2_data = {
 		.thd_dat = 16,
 		.thd_sta = 15,
 		.tbuf = 24,
-		.scl_stretch_en = 0,
 		.trdhld = 3,
 		.tsp = 3
 	},
@@ -770,7 +763,6 @@ static const struct cci_data cci_msm8953_data = {
 		.thd_dat = 10,
 		.thd_sta = 77,
 		.tbuf = 118,
-		.scl_stretch_en = 0,
 		.trdhld = 6,
 		.tsp = 1
 	},
@@ -782,7 +774,6 @@ static const struct cci_data cci_msm8953_data = {
 		.thd_dat = 13,
 		.thd_sta = 18,
 		.tbuf = 32,
-		.scl_stretch_en = 0,
 		.trdhld = 6,
 		.tsp = 3
 	},
@@ -794,7 +785,6 @@ static const struct cci_data cci_msm8953_data = {
 		.thd_dat = 16,
 		.thd_sta = 15,
 		.tbuf = 19,
-		.scl_stretch_en = 1,
 		.trdhld = 3,
 		.tsp = 3
 	},
-- 
2.43.0

base-commit: 68142f986ff04b2b70b31db00f719bf690f64a9a

Re: [PATCH v2] i2c: qcom-cci: always enable SCL clock stretching

[Andi Shyti]

Hi Hitesh,

Please don't send next versions as --in-reply-to, it makes life
difficult.

On Mon, Sep 21, 2026 at 06:49:54PM +0530, Hitesh Patel wrote:
> Every CCI timing table except the msm8953 fast-plus one leaves SCL
> clock stretching disabled in MISC_CTL. A slave that holds SCL low is
> then not waited for: the master keeps its own clock timing and the
> transfer fails with a NACK or returns corrupt data.
> 
> This is hit with a camera reached through a GMSL serializer/
> deserializer I2C tunnel (MAX9296A/MAX96717 on the RB3 Gen2 vision
> mezzanine, SC7280 CCI v2 at 100 kHz). The deserializer acknowledges
> the address locally, forwards the transaction over the coax link and
> stretches SCL until the remote side has completed it, which takes well
> over one clock period. Without stretching the register reads of the
> sensor behind the link intermittently return garbage and writes are
> dropped, which shows up as random sensor init failures.
> 
> Clock stretching is part of the I2C specification for every speed
> mode and a device that does not stretch is unaffected by it, so there
> is no reason to make it a per-table parameter. Set the bit
> unconditionally and drop the scl_stretch_en field.
> 
> Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>

I'll wait for an ack from Konrad here.

Thanks,
Andi

Re: [PATCH v2] i2c: qcom-cci: always enable SCL clock stretching

[Hitesh Patel]

Hi Konrad,

A gentle ping on this one - Andi is holding it for your ack.

The v2 is what you suggested: it drops the per-table scl_stretch_en
field and sets CCI_I2C_MISC_CTL_SCL_STRETCH_EN unconditionally in
cci_init(), so every speed mode on every CCI version gets it.

Tested on SC7280 (CCI v2) at 100 kHz through a GMSL I2C tunnel and at
1 MHz for the on-board devices. The other tables are untested, but the
change is identical for all of them.

  https://lore.kernel.org/linux-i2c/20260921131954.1690578-1-hitesh@ebytelogic.com/

Thanks,
Hitesh

Re: [PATCH v2] i2c: qcom-cci: always enable SCL clock stretching

[Loic Poulain]

On Mon, Sep 21, 2026 at 3:20 PM Hitesh Patel <hitesh@ebytelogic.com> wrote:
>
> Every CCI timing table except the msm8953 fast-plus one leaves SCL
> clock stretching disabled in MISC_CTL. A slave that holds SCL low is
> then not waited for: the master keeps its own clock timing and the
> transfer fails with a NACK or returns corrupt data.
>
> This is hit with a camera reached through a GMSL serializer/
> deserializer I2C tunnel (MAX9296A/MAX96717 on the RB3 Gen2 vision
> mezzanine, SC7280 CCI v2 at 100 kHz). The deserializer acknowledges
> the address locally, forwards the transaction over the coax link and
> stretches SCL until the remote side has completed it, which takes well
> over one clock period. Without stretching the register reads of the
> sensor behind the link intermittently return garbage and writes are
> dropped, which shows up as random sensor init failures.
>
> Clock stretching is part of the I2C specification for every speed
> mode and a device that does not stretch is unaffected by it, so there
> is no reason to make it a per-table parameter. Set the bit
> unconditionally and drop the scl_stretch_en field.
>
> Signed-off-by: Hitesh Patel <hitesh@ebytelogic.com>

Looks good to me. In addition to Andi's point about using
--in-reply-to, don't forget to include a short changelog when
submitting a new series/patch revision.

Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>


> ---
>  drivers/i2c/busses/i2c-qcom-cci.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
> index 873e901..fbd5507 100644
> --- a/drivers/i2c/busses/i2c-qcom-cci.c
> +++ b/drivers/i2c/busses/i2c-qcom-cci.c
> @@ -27,6 +27,7 @@
>  #define CCI_I2C_Mm_SDA_CTL_1(m)                        (0x108 + 0x100 * (m))
>  #define CCI_I2C_Mm_SDA_CTL_2(m)                        (0x10c + 0x100 * (m))
>  #define CCI_I2C_Mm_MISC_CTL(m)                 (0x110 + 0x100 * (m))
> +#define CCI_I2C_MISC_CTL_SCL_STRETCH_EN        BIT(8)
>
>  #define CCI_I2C_Mm_READ_DATA(m)                        (0x118 + 0x100 * (m))
>  #define CCI_I2C_Mm_READ_BUF_LEVEL(m)           (0x11c + 0x100 * (m))
> @@ -97,7 +98,6 @@ struct hw_params {
>         u16 thd_dat; /* data hold time */
>         u16 thd_sta; /* hold time (repeated) START condition */
>         u16 tbuf; /* bus free time between a STOP and START condition */
> -       u8 scl_stretch_en;
>         u16 trdhld;
>         u16 tsp; /* pulse width of spikes suppressed by the input filter */
>  };
> @@ -263,7 +263,7 @@ static void cci_init(struct cci *cci)
>                 val = hw->tbuf;
>                 writel(val, cci->base + CCI_I2C_Mm_SDA_CTL_2(i));
>
> -               val = hw->scl_stretch_en << 8 | hw->trdhld << 4 | hw->tsp;
> +               val = CCI_I2C_MISC_CTL_SCL_STRETCH_EN | hw->trdhld << 4 | hw->tsp;
>                 writel(val, cci->base + CCI_I2C_Mm_MISC_CTL(i));
>         }
>  }
> @@ -659,7 +659,6 @@ static const struct cci_data cci_v1_data = {
>                 .thd_dat = 10,
>                 .thd_sta = 77,
>                 .tbuf = 118,
> -               .scl_stretch_en = 0,
>                 .trdhld = 6,
>                 .tsp = 1
>         },
> @@ -671,7 +670,6 @@ static const struct cci_data cci_v1_data = {
>                 .thd_dat = 13,
>                 .thd_sta = 18,
>                 .tbuf = 32,
> -               .scl_stretch_en = 0,
>                 .trdhld = 6,
>                 .tsp = 3
>         },
> @@ -692,7 +690,6 @@ static const struct cci_data cci_v1_5_data = {
>                 .thd_dat = 10,
>                 .thd_sta = 77,
>                 .tbuf = 118,
> -               .scl_stretch_en = 0,
>                 .trdhld = 6,
>                 .tsp = 1
>         },
> @@ -704,7 +701,6 @@ static const struct cci_data cci_v1_5_data = {
>                 .thd_dat = 13,
>                 .thd_sta = 18,
>                 .tbuf = 32,
> -               .scl_stretch_en = 0,
>                 .trdhld = 6,
>                 .tsp = 3
>         },
> @@ -725,7 +721,6 @@ static const struct cci_data cci_v2_data = {
>                 .thd_dat = 22,
>                 .thd_sta = 162,
>                 .tbuf = 227,
> -               .scl_stretch_en = 0,
>                 .trdhld = 6,
>                 .tsp = 3
>         },
> @@ -737,7 +732,6 @@ static const struct cci_data cci_v2_data = {
>                 .thd_dat = 22,
>                 .thd_sta = 35,
>                 .tbuf = 62,
> -               .scl_stretch_en = 0,
>                 .trdhld = 6,
>                 .tsp = 3
>         },
> @@ -749,7 +743,6 @@ static const struct cci_data cci_v2_data = {
>                 .thd_dat = 16,
>                 .thd_sta = 15,
>                 .tbuf = 24,
> -               .scl_stretch_en = 0,
>                 .trdhld = 3,
>                 .tsp = 3
>         },
> @@ -770,7 +763,6 @@ static const struct cci_data cci_msm8953_data = {
>                 .thd_dat = 10,
>                 .thd_sta = 77,
>                 .tbuf = 118,
> -               .scl_stretch_en = 0,
>                 .trdhld = 6,
>                 .tsp = 1
>         },
> @@ -782,7 +774,6 @@ static const struct cci_data cci_msm8953_data = {
>                 .thd_dat = 13,
>                 .thd_sta = 18,
>                 .tbuf = 32,
> -               .scl_stretch_en = 0,
>                 .trdhld = 6,
>                 .tsp = 3
>         },
> @@ -794,7 +785,6 @@ static const struct cci_data cci_msm8953_data = {
>                 .thd_dat = 16,
>                 .thd_sta = 15,
>                 .tbuf = 19,
> -               .scl_stretch_en = 1,
>                 .trdhld = 3,
>                 .tsp = 3
>         },
> --
> 2.43.0
>
> base-commit: 68142f986ff04b2b70b31db00f719bf690f64a9a

Re: [PATCH v2] i2c: qcom-cci: always enable SCL clock stretching

[Andi Shyti]

Hi Hitesh,

>  drivers/i2c/busses/i2c-qcom-cci.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)

I'm trying to apply your patch but it has some conflicts, do you
mind sending a v3 rebased on top of -next or i2c/i2c-next?

Thanks,
Andi