[PATCH] [RFC] i2c-imx: send a soft bus reset during probe

* [PATCH] [RFC] i2c-imx: send a soft bus reset during probe
@ 2012-09-26 22:56 Uwe Kleine-König
  2012-09-27  9:53 ` Sascha Hauer
  0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2012-09-26 22:56 UTC (permalink / raw)
  To: barebox; +Cc: kernel

If an i2c slave pulls SDA low when the SoC wants to start a transfer, it
cannot get the bus. This can happen for example if the SoC is reset when
it just successfully addressed a slave but before it rises SCL for the
slave to ack. Currently this makes the driver refuse to do anything. You
can force this situation with the following code assuming you have a
device on address 0x50:

	/* generate a start condition */
	gpio_direction_input(I2CSCL);
	gpio_direction_input(I2CSDA);
	mdelay(1);
	gpio_direction_output(I2CSDA, 0);

	mdelay(1);

	/* address device 0x50 */
	for (i = 0; i < 8; ++i) {
		gpio_direction_output(I2CSCL, 0);
		mdelay(1);
		if ((0x50 << 1) >> (7 - i))
			gpio_direction_input(I2CSDA);
		else
			gpio_direction_output(I2CSDA, 0);
		mdelay(1);
		gpio_direction_input(I2CSCL);
		mdelay(1);
	}
	gpio_direction_output(I2CSCL, 0);

	reset_cpu(0);

To circumvent this situation just clock the bus 9 times without pulling
on SDA following a start condition and finalizing with a repeated start and
stop. This is suggested in an Atmel at24 datasheet (5226G-SEEPR-11/09):

        2-WIRE SOFTWARE RESET: After an interruption in protocol, power loss or
        system reset, any 2-wire part can be reset by following these steps:
        (a) Create a start bit condition,
        (b) Clock 9 cycles,
        (c) Create another start bit followed by a stop bit condition as
            shown below. The device is ready for the next communication
            after the above steps have been completed.

        SCL /¯\_/¯\_/¯\_/¯\_/¯\_/¯\_/¯\_/¯\_/¯\_/¯\_/¯\_/¯\
        SDA ¯\_/¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯\___/¯
             S   1   2   3   4   5   6   7   8   9   Sr  P

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

I developped this patch on an older barebox version (2012.04.0) and it
doesn't fit on current master (or next). But that's not that bad because
it's an RFC patch and I will happily resend if the idea is accepted.

This patch solves the bus stall described in the commit message above
and I think it cannot do any harm:
 - if no slave occupies the bus, something strange might happen if there
   is a slave with address 0x7f. This address is reserved though in
   the I²C-Bus Specification Version 2.1.
 - if a slave is out of sync and pulls SDA low during the (intended) S
   at the beginning, no slave will detect a start condition. So the
   clocks only affect the (single?) slave that is currently active. This
   one completes its cycle during the 9 clocks, ignores potential clock
   pulses at the end of its cycle and the following Sr resets the state
   machine.

This sounds reasonable to me (and obviously some engineers at Atmel),
but I'd still welcome some more eyes and thoughts on this.

Also I intend do verify the intended behaviour with an oscilloscope.

Best regards
Uwe

 drivers/i2c/busses/i2c-imx.c |   39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index da6218f..ba774b9 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -207,12 +207,9 @@ static int i2c_imx_acked(struct i2c_adapter *adapter)
 	return 0;
 }
 
-static int i2c_imx_start(struct i2c_adapter *adapter)
+static void __i2c_imx_start(struct imx_i2c_struct *i2c_imx, unsigned int *i2cr)
 {
-	struct imx_i2c_struct *i2c_imx = to_imx_i2c_struct(adapter);
 	void __iomem *base = i2c_imx->base;
-	unsigned int temp = 0;
-	int result;
 
 	writeb(i2c_imx->ifdr, base + IMX_I2C_IFDR);
 	/* Enable I2C controller */
@@ -223,9 +220,19 @@ static int i2c_imx_start(struct i2c_adapter *adapter)
 	udelay(100);
 
 	/* Start I2C transaction */
-	temp = readb(base + IMX_I2C_I2CR);
-	temp |= I2CR_MSTA;
-	writeb(temp, base + IMX_I2C_I2CR);
+	*i2cr = readb(base + IMX_I2C_I2CR);
+	*i2cr |= I2CR_MSTA;
+	writeb(*i2cr, base + IMX_I2C_I2CR);
+}
+
+static int i2c_imx_start(struct i2c_adapter *adapter)
+{
+	struct imx_i2c_struct *i2c_imx = to_imx_i2c_struct(adapter);
+	int result;
+	unsigned int temp;
+	void __iomem *base = i2c_imx->base;
+
+	__i2c_imx_start(i2c_imx, &temp);
 
 	result = i2c_imx_bus_busy(adapter, 1);
 	if (result)
@@ -239,6 +246,22 @@ static int i2c_imx_start(struct i2c_adapter *adapter)
 	return result;
 }
 
+static void i2c_imx_busreset(struct imx_i2c_struct *i2c_imx)
+{
+	void __iomem *base = i2c_imx->base;
+	unsigned int temp;
+
+	__i2c_imx_start(i2c_imx, &temp);
+
+	/* send 9 clocks without anything on SDA followed by Sr */
+	writeb(temp | I2CR_RSTA, base + IMX_I2C_I2CR);
+
+	writeb(0xff, base + IMX_I2C_I2DR);
+
+	/* send a P */
+	writeb(0, i2c_imx->base + IMX_I2C_I2CR);
+}
+
 static void i2c_imx_stop(struct i2c_adapter *adapter)
 {
 	struct imx_i2c_struct *i2c_imx = to_imx_i2c_struct(adapter);
@@ -485,6 +508,8 @@ static int __init i2c_imx_probe(struct device_d *pdev)
 	writeb(0, i2c_imx->base + IMX_I2C_I2CR);
 	writeb(0, i2c_imx->base + IMX_I2C_I2SR);
 
+	i2c_imx_busreset(i2c_imx);
+
 	/* Add I2C adapter */
 	ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
 	if (ret < 0) {
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread