i2c clock stretching (suspected) problem

bthomas

Hi all,

I connected another microcontroller (LPC824) as slave to the i2c bus of the omega. That microcontroller works on 5V levels for i2c so I added the simple FET trick as documented but I suspect that the LPC824 is stretching the clock but the omega is not seeing that. I thought it was a good idea to replace the FETs with a real i2c bus repeater so I replaced the FETs with a PCA9509 but the exact same problem appears.

Here is a capture image of the logic analyzer:

I had channel 0 and 1 connected to SCL and SDA on the 5V side and channel 2 and 3 to SCL and SDA on the 3.3V side. The problem is that the LPC824 is sending an ack but the omega does not see that. In the picture you can see that SDA on the 5V side goes low with the 9th clock pulse but the rising edge of the 9th clock pulse on the omega side is ahead of that on the 5V side. Also, in the analog part you can see that the 9th clock pulse starts with a bit lower voltage and then increases to its normal level. I think that is caused by the LPC824 doing clock stretching.

If I understood correctly the current system software should support clock stretching on the omega. So why then isn't the clock stretched on the omega side?

The software running on the omega is the one it came with. The first output line of the kernel is:

[    0.000000] Linux version 4.14.81 (root@39bdb52655b3) (gcc version 7.3.0 (OpenWrt GCC 7.3.0 r0+7483-a03ac7fd8e)) #0 Mon Feb 22 22:13:35 2021

It states 2021 while the announcement of support for clock stretching dates back to 2018 so I'd say my firmware should support it, right?

bthomas

I think I have found the solution. Register SM0CTL0 has a bit SM0_ODRAIN that sets the pins to either push/pull (I assume) or open drain output. It was 1.

1: When SIF output is logic 1, the output is pulled high by SIF master 0.

Changing this to 0 solved my problem.

I think this is something that should be changed in the device driver.

luz

@bthomas said in i2c clock stretching (suspected) problem:

I think this is something that should be changed in the device driver.

Yes, this is a bug in the driver (`drivers/i2c/busses/i2c-mt7621.c), at least in Linux 4.14.241 as used in OpenWrt 19.07.8 (not exactly, but near the OnionOS fork I think).

It was fixed later, comparing with current Linux 5.10.146 in OpenWrt 22.03.2 (I'm using that for my Omegas):

OpenWrt Linux 4.14.241:

static void mtk_i2c_reset(struct mtk_i2c_dev *i2c)
{
	device_reset(i2c->adap.dev.parent);
	barrier();
	mtk_i2c_w32(i2c,
		   SM0CTL0_ODRAIN
		   | ((i2c->clk_div << 16) & SM0CTL0_CLK_DIV_MASK)
		   | SM0CTL0_EN
		   | SM0CTL0_SCL_STRETCH, REG_SM0CTL0_REG);
	mtk_i2c_w32(i2c, 0, REG_SM0CFG2_REG);
}

OpenWrt Linux 5.10.146:

static void mtk_i2c_reset(struct mtk_i2c *i2c)
{
	int ret;

	ret = device_reset(i2c->adap.dev.parent);
	if (ret)
		dev_err(i2c->dev, "I2C reset failed!\n");

	/*
	 * Don't set SM0CTL0_ODRAIN as its bit meaning is inverted. To
	 * configure open-drain mode, this bit needs to be cleared.
	 */
	iowrite32(((i2c->clk_div << 16) & SM0CTL0_CLK_DIV_MASK) | SM0CTL0_EN |
		  SM0CTL0_SCL_STRETCH, i2c->base + REG_SM0CTL0_REG);
	iowrite32(0, i2c->base + REG_SM0CFG2_REG);
}

The comment exactly states what you found out - the register bit has a misleading name, one would assume a flag called SM0CTL0_ODRAIN would enable open drain when set, not when cleared. It should better be named SM0CTL0_PUSHPULL…

bthomas

@luz Thanks for the confirmation!