Hello, On Thu, Jan 24, 2013 at 04:06:37PM +0530, Viresh Kumar wrote: > Most number of versions for any patchset i submitted :) So let's see if you do a v10 ... :-) > +static int i2c_generic_recovery(struct i2c_adapter *adap) > +{ > + struct i2c_bus_recovery_info *bri = adap->bus_recovery_info; > + int i = 0, val = 1, ret = 0; > + > + if (bri->prepare_recovery) > + bri->prepare_recovery(bri); > + Do we want to break out here if sda is high? > + /* > + * By this time SCL is high, as we need to give 9 falling-rising edges > + */ > + while (i++ < RECOVERY_CLK_CNT * 2) { > + /* SCL shouldn't be low here */ > + if (val && !bri->get_scl(adap)) { > + dev_err(&adap->dev, "SCL is stuck Low exit recovery\n"); > + ret = -EBUSY; > + goto unprepare; > + } > + > + val = !val; > + bri->set_scl(adap, val); > + ndelay(clk_delay); > + > + /* break if sda got high, check only when scl line is high */ Above you wrote "SCL", here "scl". I suggest to use one of them consistently and use the same capitalisation for sda. > + if (bri->get_sda && val) > + if (bri->get_sda(adap)) > + break; Maybe better: /* break if sda got high */ if (bri->get_sda && bri->get_sda(adap)) { /* don't leave with scl low */ if (!val) bri->set_scl(adap, 1); break; } > + } > + > +unprepare: > + if (bri->unprepare_recovery) > + bri->unprepare_recovery(bri); > + > + return ret; > +} > + > +int i2c_generic_scl_recovery(struct i2c_adapter *adap) > +{ > + adap->bus_recovery_info->set_scl(adap, 1); Why this? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html