Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux