| Message ID | 1424369209-26735-1-git-send-email-balbi@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote: > If either SCL or SDA are stuck low, we need to > recover the bus using the procedure described > on section 3.1.16 of the I2C specification. > > Note that we're trying to implement the procedure > exactly as described by that section. First we > check which line is stuck low, then implement > one or the other procedure. If SDA recovery procedure > fails, we reset our IP in an attempt to make it work. > > Signed-off-by: Felipe Balbi <balbi@ti.com> > --- > > Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with > 1000 iterations of i2cdetect on all available buses. > > That said, I couldn't get any device to hold the bus busy so I could > see this working. If anybody has any good way of forcing a condition > so that we need bus recovery, I'd be glad to look at. > > cheers > > drivers/i2c/busses/i2c-omap.c | 71 +++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 69 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index 0e894193accf..c3e4da751adf 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -472,6 +472,73 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) > return 0; > } > > +static void omap_i2c_clock_pulse(struct omap_i2c_dev *dev) > +{ > + u32 reg; > + int i; > + > + /* Enable testmode */ > + reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG); > + reg |= OMAP_I2C_SYSTEST_ST_EN; > + omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg); > + > + for (i = 0; i < 9; i++) { > + reg |= OMAP_I2C_SYSTEST_SCL_O; > + omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg); > + mdelay(100); > + reg &= ~OMAP_I2C_SYSTEST_SCL_O; > + omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg); > + mdelay(100); > + } > + > + /* Disable testmode */ > + reg &= ~OMAP_I2C_SYSTEST_ST_EN; > + omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg); > +} > + > +static void omap_i2c_bus_recover(struct omap_i2c_dev *dev) > +{ > + u32 reg1; > + u32 reg2; > + > + /* > + * First differentiate SCL stuck low from SDA stuck low using our > + * SYSTEST register. Depending on which line is stuck low, we will > + * either Reset our I2C IP (SCL stuck low) or drive 9 clock pulses on > + * SCL (SDA stuck low) to tell the device to release the bus. > + * > + * If, after 9 clock pulses on SCL device still doesn't release the > + * bus, there's nothing more we can do; we will still try to Reset > + * our I2C IP anyway. > + */ > + > + reg1 = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG); > + msleep(1); Hmm... I wonder if this msleep() should be scaled based on i2c bus frequency.
On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote: > If either SCL or SDA are stuck low, we need to > recover the bus using the procedure described > on section 3.1.16 of the I2C specification. > > Note that we're trying to implement the procedure > exactly as described by that section. First we > check which line is stuck low, then implement > one or the other procedure. If SDA recovery procedure > fails, we reset our IP in an attempt to make it work. > > Signed-off-by: Felipe Balbi <balbi@ti.com> > --- > > Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with > 1000 iterations of i2cdetect on all available buses. > > That said, I couldn't get any device to hold the bus busy so I could > see this working. If anybody has any good way of forcing a condition > so that we need bus recovery, I'd be glad to look at. ping
On Mon, Mar 09, 2015 at 11:39:17AM -0500, Felipe Balbi wrote: > On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote: > > If either SCL or SDA are stuck low, we need to > > recover the bus using the procedure described > > on section 3.1.16 of the I2C specification. > > > > Note that we're trying to implement the procedure > > exactly as described by that section. First we > > check which line is stuck low, then implement > > one or the other procedure. If SDA recovery procedure > > fails, we reset our IP in an attempt to make it work. > > > > Signed-off-by: Felipe Balbi <balbi@ti.com> > > --- > > > > Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with > > 1000 iterations of i2cdetect on all available buses. > > > > That said, I couldn't get any device to hold the bus busy so I could > > see this working. If anybody has any good way of forcing a condition > > so that we need bus recovery, I'd be glad to look at. > > ping any comments here ?? Anybody at all ????
Hi Felipe, On 03/11/2015 03:50 AM, Felipe Balbi wrote: > On Mon, Mar 09, 2015 at 11:39:17AM -0500, Felipe Balbi wrote: >> On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote: >>> If either SCL or SDA are stuck low, we need to >>> recover the bus using the procedure described >>> on section 3.1.16 of the I2C specification. >>> >>> Note that we're trying to implement the procedure >>> exactly as described by that section. First we >>> check which line is stuck low, then implement >>> one or the other procedure. If SDA recovery procedure >>> fails, we reset our IP in an attempt to make it work. >>> >>> Signed-off-by: Felipe Balbi <balbi@ti.com> >>> --- >>> >>> Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with >>> 1000 iterations of i2cdetect on all available buses. >>> >>> That said, I couldn't get any device to hold the bus busy so I could >>> see this working. If anybody has any good way of forcing a condition >>> so that we need bus recovery, I'd be glad to look at. >> >> ping > > any comments here ?? Anybody at all ???? > I think the I2C bus recovery infrastructure should be used here ;) As I did there https://lkml.org/lkml/2014/12/1/397, but there are no comments too :( regards, -grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 11, 2015 at 03:47:53PM +0200, Grygorii Strashko wrote: > Hi Felipe, > > On 03/11/2015 03:50 AM, Felipe Balbi wrote: > >On Mon, Mar 09, 2015 at 11:39:17AM -0500, Felipe Balbi wrote: > >>On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote: > >>>If either SCL or SDA are stuck low, we need to > >>>recover the bus using the procedure described > >>>on section 3.1.16 of the I2C specification. > >>> > >>>Note that we're trying to implement the procedure > >>>exactly as described by that section. First we > >>>check which line is stuck low, then implement > >>>one or the other procedure. If SDA recovery procedure > >>>fails, we reset our IP in an attempt to make it work. > >>> > >>>Signed-off-by: Felipe Balbi <balbi@ti.com> > >>>--- > >>> > >>>Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with > >>>1000 iterations of i2cdetect on all available buses. > >>> > >>>That said, I couldn't get any device to hold the bus busy so I could > >>>see this working. If anybody has any good way of forcing a condition > >>>so that we need bus recovery, I'd be glad to look at. > >> > >>ping > > > >any comments here ?? Anybody at all ???? > > > > I think the I2C bus recovery infrastructure should be used here ;) > As I did there https://lkml.org/lkml/2014/12/1/397, but > there are no comments too :( Sorry, guys, a lot of stuff going on in I2C. Bus recovery needs a more generic look. I'll try, but can't promise for 4.1. If it fails 4.1., it will get priority for 4.2.
On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote: > If either SCL or SDA are stuck low, we need to > recover the bus using the procedure described > on section 3.1.16 of the I2C specification. > > Note that we're trying to implement the procedure > exactly as described by that section. First we > check which line is stuck low, then implement > one or the other procedure. If SDA recovery procedure > fails, we reset our IP in an attempt to make it work. > > Signed-off-by: Felipe Balbi <balbi@ti.com> As Grygorii already mentioned: can you convert it to the standard i2c bus recovery mechanism? And is the timeout you replace with the recovery caused by SDA stuck high (check the thread starting with http://thread.gmane.org/gmane.linux.kernel/1841371/focus=22435) Thanks, Wolfram
On Fri, Apr 10, 2015 at 11:41:22PM +0200, Wolfram Sang wrote: > On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote: > > If either SCL or SDA are stuck low, we need to > > recover the bus using the procedure described > > on section 3.1.16 of the I2C specification. > > > > Note that we're trying to implement the procedure > > exactly as described by that section. First we > > check which line is stuck low, then implement > > one or the other procedure. If SDA recovery procedure > > fails, we reset our IP in an attempt to make it work. > > > > Signed-off-by: Felipe Balbi <balbi@ti.com> > > As Grygorii already mentioned: can you convert it to the standard i2c > bus recovery mechanism? > > And is the timeout you replace with the recovery caused by SDA stuck > high (check the thread starting with > http://thread.gmane.org/gmane.linux.kernel/1841371/focus=22435) SDA stuck low is one reason, yes. There could be other reasons like the far end (i2c client) just crapping out. I know of one touchscreen controller which will just die if you continuously try to read from unexistent registers. It will NAK for a while and later just die, completely. The only way to recover from that today is resetting the board. I have a feeling that resetting the touch controller might be doable if we have a reset pin tied to a GPIO, haven't tried that yet. In any case this is off-topic. I'll rebase the patch and fix what you asked, then retest, and resend.
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 0e894193accf..c3e4da751adf 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -472,6 +472,73 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) return 0; } +static void omap_i2c_clock_pulse(struct omap_i2c_dev *dev) +{ + u32 reg; + int i; + + /* Enable testmode */ + reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG); + reg |= OMAP_I2C_SYSTEST_ST_EN; + omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg); + + for (i = 0; i < 9; i++) { + reg |= OMAP_I2C_SYSTEST_SCL_O; + omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg); + mdelay(100); + reg &= ~OMAP_I2C_SYSTEST_SCL_O; + omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg); + mdelay(100); + } + + /* Disable testmode */ + reg &= ~OMAP_I2C_SYSTEST_ST_EN; + omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg); +} + +static void omap_i2c_bus_recover(struct omap_i2c_dev *dev) +{ + u32 reg1; + u32 reg2; + + /* + * First differentiate SCL stuck low from SDA stuck low using our + * SYSTEST register. Depending on which line is stuck low, we will + * either Reset our I2C IP (SCL stuck low) or drive 9 clock pulses on + * SCL (SDA stuck low) to tell the device to release the bus. + * + * If, after 9 clock pulses on SCL device still doesn't release the + * bus, there's nothing more we can do; we will still try to Reset + * our I2C IP anyway. + */ + + reg1 = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG); + msleep(1); + reg2 = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG); + + if (!(reg1 & OMAP_I2C_SYSTEST_SCL_I_FUNC) && + !(reg2 & OMAP_I2C_SYSTEST_SCL_I_FUNC)) { + dev_err(dev->dev, "SCL is stuck low, resetting\n"); + omap_i2c_reset(dev); + } + + if (!(reg1 & OMAP_I2C_SYSTEST_SDA_I_FUNC) && + !(reg2 & OMAP_I2C_SYSTEST_SDA_I_FUNC)) { + dev_err(dev->dev, "SDA is stuck low, driving 9 pulses on SCL\n"); + omap_i2c_clock_pulse(dev); + + reg1 = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG); + msleep(1); + reg2 = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG); + + if ((reg1 & OMAP_I2C_SYSTEST_SDA_I_FUNC) && + (reg2 & OMAP_I2C_SYSTEST_SDA_I_FUNC)) { + dev_err(dev->dev, "SDA still stuck, resetting\n"); + omap_i2c_reset(dev); + } + } +} + /* * Waiting on Bus Busy */ @@ -482,8 +549,8 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *dev) timeout = jiffies + OMAP_I2C_TIMEOUT; while (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG) & OMAP_I2C_STAT_BB) { if (time_after(jiffies, timeout)) { - dev_warn(dev->dev, "timeout waiting for bus ready\n"); - return -ETIMEDOUT; + omap_i2c_bus_recover(dev); + return 0; } msleep(1); }
If either SCL or SDA are stuck low, we need to recover the bus using the procedure described on section 3.1.16 of the I2C specification. Note that we're trying to implement the procedure exactly as described by that section. First we check which line is stuck low, then implement one or the other procedure. If SDA recovery procedure fails, we reset our IP in an attempt to make it work. Signed-off-by: Felipe Balbi <balbi@ti.com> --- Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with 1000 iterations of i2cdetect on all available buses. That said, I couldn't get any device to hold the bus busy so I could see this working. If anybody has any good way of forcing a condition so that we need bus recovery, I'd be glad to look at. cheers drivers/i2c/busses/i2c-omap.c | 71 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 2 deletions(-)