On 24 January 2013 16:36, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > 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 ... :-) Haha.. >> +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? Good point actually. It may happen we are asked to recover already working system :) >> + /* >> + * 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. Sure. >> + 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; > } So, the whole loop is for 9 pulses at max and it runs 18 times. Twice per clock. With my code, i only check for sda after supplying full clock pulse, and you are asking me to check that in middle of clock. Isn't it wrong? >> +int i2c_generic_scl_recovery(struct i2c_adapter *adap) >> +{ >> + adap->bus_recovery_info->set_scl(adap, 1); > Why this? This came out of some earlier discussion we had. We should start with high and then do low-high 9 times. -- 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