[U-Boot] [PATCH V2] i2c: mxs_i2c: Squash endless loop

[U-Boot] [PATCH V2] i2c: mxs_i2c: Squash endless loop

[Marek Vasut]

The endless waiting for a bit to be set can cause a hang, add a timeout
so we prevent such situation. A testcase for such a hang is below. The
testcase assumes a device to be present at address 0x50 and a device to
NOT be present at address 0x42 . Also note that the "sleep 1" induced
delays are imperative for this bug to manifest .

i2c read 0x42 0x0.2 0x10 0x42000000 ; sleep 1 ; \
i2c read 0x50 0x0.2 0x10 0x42000000 ; sleep 1 ; \
i2c read 0x42 0x0.2 0x10 0x42000000

The expected result of the above command is:

Error reading the chip.
Error reading the chip.

While without this patch, we observe a hang in the last read from 0x42
precisely when waiting for this bit to be set.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Stefano Babic <sbabic@denx.de>

---
 drivers/i2c/mxs_i2c.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

V2: Replace -EINVAL with -ETIMEDOUT

diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c
index 46106b7..a298c95 100644
--- a/drivers/i2c/mxs_i2c.c
+++ b/drivers/i2c/mxs_i2c.c
@@ -150,6 +150,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
 {
 	struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE;
 	uint32_t tmp = 0;
+	int timeout = MXS_I2C_MAX_TIMEOUT;
 	int ret;
 	int i;
 
@@ -169,9 +170,17 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
 
 	for (i = 0; i < len; i++) {
 		if (!(i & 3)) {
-			while (readl(&i2c_regs->hw_i2c_queuestat) &
-				I2C_QUEUESTAT_RD_QUEUE_EMPTY)
-				;
+			while (--timeout) {
+				tmp = readl(&i2c_regs->hw_i2c_queuestat);
+				if (!(tmp & I2C_QUEUESTAT_RD_QUEUE_EMPTY))
+					break;
+			}
+
+			if (!timeout) {
+				debug("MXS I2C: Failed receiving data!\n");
+				return -ETIMEDOUT;
+			}
+
 			tmp = readl(&i2c_regs->hw_i2c_queuedata);
 		}
 		buffer[i] = tmp & 0xff;
-- 
1.8.4.rc3

[U-Boot] [U-Boot,V2] i2c: mxs_i2c: Squash endless loop

[Heiko Schocher]

Hello Marek,

Am 04.11.2013 14:29, schrieb Marek Vasut:
> The endless waiting for a bit to be set can cause a hang, add a timeout
> so we prevent such situation. A testcase for such a hang is below. The
> testcase assumes a device to be present at address 0x50 and a device to
> NOT be present at address 0x42 . Also note that the "sleep 1" induced
> delays are imperative for this bug to manifest .
>
> i2c read 0x42 0x0.2 0x10 0x42000000 ; sleep 1 ; \
> i2c read 0x50 0x0.2 0x10 0x42000000 ; sleep 1 ; \
> i2c read 0x42 0x0.2 0x10 0x42000000
>
> The expected result of the above command is:
>
> Error reading the chip.
> Error reading the chip.
>
> While without this patch, we observe a hang in the last read from 0x42
> precisely when waiting for this bit to be set.
>
> Signed-off-by: Marek Vasut<marex@denx.de>
> Cc: Fabio Estevam<fabio.estevam@freescale.com>
> Cc: Heiko Schocher<hs@denx.de>
> Cc: Stefano Babic<sbabic@denx.de>
>
> ---
> drivers/i2c/mxs_i2c.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)

Thanks!

Applied to u-boot-i2c.git

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany