On 24 January 2013 16:36, Uwe Kleine-König
<[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
