| Message ID | 1383571752-18546-1-git-send-email-marex@denx.de |
|---|---|
| State | Accepted |
| Delegated to: | Heiko Schocher |
| Headers | show |
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
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;
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