[PATCH v2 0/2] i2c: jz4780: Program the spike filter, add the X1600

[PATCH v2 0/2] i2c: jz4780: Program the spike filter, add the X1600

[Olivier Louvignes]

v1 said the X1600 moved the SDA set-up register from 0x94 to 0xa0. It
didn't: as Nikolaus pointed out from the programming manuals, SDASU is
at 0x94 on the X1000, X1830 and X1600 alike, and 0xa0 is the spike
suppression length. v1 worked by accident, by writing the set-up value
into the spike filter.

What was wrong on the HiBy R1: started from the vendor kernel, the
filter held 116 APB cycles, longer than SCL's high period at 400 kHz
(102 cycles at 140 MHz), and every transfer lost arbitration. Setting
the filter by hand before single reads, transfers work from 1 to 100
cycles and fail from 110 up, whatever SDASU holds (1 to 100 tried). A
cold boot leaves it at its reset value of 1 and works.

So patch 1 programs the filter for every X1000-class SoC, to the 50 ns
of the I2C specification, and the X1600 needs no driver change: patch 2
adds its compatible with a fallback to ingenic,x1000-i2c.

Changes since v1:
- Replace "Add support for the X1600" (wrong register) with a fix to
  the spike filter for all X1000-class SoCs.
- The binding now falls back to ingenic,x1000-i2c.
- Rebased on i2c/i2c-next.

v1: https://lore.kernel.org/all/20261023081452.44118-1-olivier@mgcrea.io/

Olivier Louvignes (2):
  i2c: jz4780: Program the spike suppression length
  dt-bindings: i2c: ingenic,i2c: Add the X1600

 .../devicetree/bindings/i2c/ingenic,i2c.yaml         |  3 +++
 drivers/i2c/busses/i2c-jz4780.c                      | 12 ++++++++++++
 2 files changed, 15 insertions(+)


base-commit: 03a5cffc818695157414456dec695f7c30b9ffff
--
2.39.5

[PATCH v2 1/2] i2c: jz4780: Program the spike suppression length

[Olivier Louvignes]

The X1000 and the SoCs after it (X1830, X1600) have a spike suppression
register at 0xa0: I2C_FLT in the X1000 programming manual, SMB_FLT in
the X1830's, FS_SPKLEN in the X1600's. It holds the longest pulse on SCL
or SDA, in APB clock cycles, that the controller filters out. It resets
to 1, and the driver never writes it, so it keeps whatever ran before.

On a HiBy R1 (X1600) started from the vendor kernel, the register held
116. At 400 kHz with a 140 MHz APB clock, SCL is high for 102 cycles,
so the filter hides the controller's own clock and every transfer ends
in a lost arbitration (TXABRT 0x1000). Driving reads by hand on the R1
with the filter set to each value: they work from 1 to 100 cycles and
fail at 110 and above.

Program the 50 ns the I2C specification sets as the longest spike to
suppress in Fast-mode, rounded up to whole APB cycles (7 at 140 MHz).
With it, the R1 reads back 7 on both controllers; after setting the
filter to 116 by hand, the next transfer puts 7 back and 50 reads out of
50 succeed.

Suggested-by: H. Nikolaus Schaller <hns@goldelico.com>
Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
---
 drivers/i2c/busses/i2c-jz4780.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c
index 695be3b21460..d1bb75d9a102 100644
--- a/drivers/i2c/busses/i2c-jz4780.c
+++ b/drivers/i2c/busses/i2c-jz4780.c
@@ -58,6 +58,7 @@
 #define JZ4780_I2C_ENSTA	0x9C
 #define JZ4780_I2C_SDAHD	0xD0
 #define X1000_I2C_SDAHD		0x7C
+#define X1000_I2C_FLT		0xA0
 
 #define JZ4780_I2C_CTRL_STPHLD		BIT(7)
 #define JZ4780_I2C_CTRL_SLVDIS		BIT(6)
@@ -253,6 +254,7 @@ static int jz4780_i2c_set_speed(struct jz4780_i2c *i2c)
 	int cnt_period = 0;	/* period count of the SCL clock */
 	int setup_time = 0;
 	int hold_time = 0;
+	int spike_len;
 	unsigned short tmp = 0;
 	int i2c_clk = i2c->speed;
 
@@ -356,6 +358,16 @@ static int jz4780_i2c_set_speed(struct jz4780_i2c *i2c)
 			jz4780_i2c_writew(i2c, JZ4780_I2C_SDAHD, 0);
 	}
 
+	/*
+	 * The spike filter keeps whatever ran before left in it, and one longer
+	 * than the SCL high period makes every transfer lose arbitration. Use
+	 * the 50 ns the I2C specification sets for Fast-mode spikes.
+	 */
+	if (i2c->cdata->version >= ID_X1000) {
+		spike_len = DIV_ROUND_UP(50 * dev_clk_khz, 1000000);
+		jz4780_i2c_writew(i2c, X1000_I2C_FLT, clamp(spike_len, 1, 255));
+	}
+
 	return 0;
 }
 

base-commit: 03a5cffc818695157414456dec695f7c30b9ffff
-- 
2.39.5

[PATCH v2 2/2] dt-bindings: i2c: ingenic,i2c: Add the X1600

[Olivier Louvignes]

The X1600's I2C controller has the X1000's registers, so it falls back
to ingenic,x1000-i2c.

Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
---
 Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml b/Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml
index b61fdc9548d8..9004f9799fd0 100644
--- a/Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml
@@ -24,6 +24,9 @@ properties:
       - items:
           - const: ingenic,jz4780-i2c
           - const: ingenic,jz4770-i2c
+      - items:
+          - const: ingenic,x1600-i2c
+          - const: ingenic,x1000-i2c
 
   reg:
     maxItems: 1
-- 
2.39.5

Re: [PATCH v2 2/2] dt-bindings: i2c: ingenic,i2c: Add the X1600

[Krzysztof Kozlowski]

On 24/10/2026 23:25, Olivier Louvignes wrote:
> The X1600's I2C controller has the X1000's registers, so it falls back
> to ingenic,x1000-i2c.
> 
> Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>

Why changing order of patches? Previous was correct one.


Please organize the patch documenting the compatible (DT bindings)
before the patch using that compatible.
See also:
https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bindings/submitting-patches.rst#L46

With this fixed:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Re: [PATCH v2 2/2] dt-bindings: i2c: ingenic,i2c: Add the X1600

[Andi Shyti]

Hi,

On Mon, Oct 26, 2026 at 09:06:43AM +0100, Krzysztof Kozlowski wrote:
> On 24/10/2026 23:25, Olivier Louvignes wrote:
> > The X1600's I2C controller has the X1000's registers, so it falls back
> > to ingenic,x1000-i2c.
> > 
> > Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
> 
> Why changing order of patches? Previous was correct one.
> 
> 
> Please organize the patch documenting the compatible (DT bindings)
> before the patch using that compatible.
> See also:
> https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bindings/submitting-patches.rst#L46

I can swap the patches before applying if they are OK. Next time,
please, follow Krzysztof's instructions.

I'll wait for Nikolaus ack/r-b for applying the patches.

Thanks,
Andi

> With this fixed:
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> 
> Best regards,
> Krzysztof