On 18.09.2017 07:24, Vignesh R wrote: > > > On Saturday 16 September 2017 05:01 AM, Strashko, Grygorii wrote: >> >> >> On 09/14/2017 10:39 AM, Claudio Foellmi wrote: >>> A very conservative check for bus activity (to prevent interference >>> in multimaster setups) prevented the bus recovery methods from being >>> triggered in the case that SDA or SCL was stuck low. >>> This defeats the purpose of the recovery mechanism, which was introduced >>> for exactly this situation (a slave device keeping SDA pulled down). >>> >>> Note that bus lockups can persist across reboots. The only other options >>> are to reset or power cycle the offending slave device, and many i2c >>> slaves do not even have a reset pin. >>> >>> If we see that one of the lines is low for the entire timeout duration, >>> we can actually be sure that there is no other master driving the bus. >>> It is therefore save for us to attempt a bus recovery. >>> >> >> Reviewed-by: Grygorii Strashko <grygorii.strashko@xxxxxx> >> >>> Signed-off-by: Claudio Foellmi <claudio.foellmi@xxxxxxxx> >>> --- >>> Caveat: It turns out I don't have the hardware to fully test the >>> recovery mechanism. My faulty i2c slave device actually pulls down SCL, >>> not SDA (so the recovery will not succeed in my case). > > Maybe, you could detect SCL stuck low case by reading status of SCL line > from OMAP_I2C_SYSTEST_REG and then call IP reset (there is nothing much > that can be done) instead of bus recovery. > I plan on posting a related patch soon, that will print better messages if the generic recovery fails. If SCL is stuck low, I think the best we can do is make the problem visible in the kernel log. >>> But by directly connecting SDA to ground, I could at least make sure >>> the recovery function gets called after applying this patch. >>> > > I had seen flood of XRDY & RRDY interrupts as soon as TMODE is set to > 0x3 as part of omap_i2c_prepare_recovery() leading to unusable system. > Did you observe this behavior on your system? Could you mention the > platform on which this experiment done? > So attempting bus recovery is dangerous on some platforms? I did not notice any obvious problems (assuming an 'unusable system' would be hard to miss), but then again I only have one target to test on. I'm working with a TI AM3352, the slave is a NXP NT3H2211 on i2c-1. >>> drivers/i2c/busses/i2c-omap.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c >>> index 1ebb5e9..4b25fd1 100644 >>> --- a/drivers/i2c/busses/i2c-omap.c >>> +++ b/drivers/i2c/busses/i2c-omap.c >>> @@ -563,8 +563,13 @@ static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *omap) >>> } >>> >>> if (time_after(jiffies, timeout)) { >>> + /* >>> + * SDA or SCL were low for the entire timeout without >>> + * any activity detected. Most likely, a slave is >>> + * locking up the bus with no master driving the clock. >>> + */ >>> dev_warn(omap->dev, "timeout waiting for bus ready\n"); >>> - return -ETIMEDOUT; >>> + return i2c_recover_bus(&omap->adapter); >>> } >>> >>> msleep(1); >>> >> > -- Regards Claudio -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html