On 24 January 2013 16:24, Uwe Kleine-König
<[email protected]> wrote:
> On Thu, Jan 24, 2013 at 08:24:45AM +0100, Wolfram Sang wrote:
> Some more comments below.
Always welcomed :)
>> * if (val && bri->get_sda)
>>
>> > + /* Check SCL again to see fault condition */
>> > + if (!bri->get_scl(adap)) {
>> > + dev_err(&adap->dev, "SCL is stuck Low during
>> > recovery, exiting recovery.\n");
>>
>> > + goto unprepare;
>> > + }
>> > +
>> > + if (bri->get_sda(adap))
>> > + break;
> Indention suggests that you want this in the body of the if (val &&
> bri->get_sda). Unfortunately the C-Compiler won't notice without braces.
Wow!! It was a blunder. Don't know how the braces got dropped.
My bad, but the new patchset (already posted), must have fixed it by
mistake :)
>> Checking SDA should be done before setting SCL? That would
>>
>> a) let us escape early if SDA got HIGH magically somehow until we enter
>> the for loop for the first time
>> b) breaking out of the for loop after the last pulse is moot
> You mean:
>
> val = 0;
>
> for (i = 0; i < RECOVERY_CLK_CNT * 2; i++, val = !val) {
> if (!val && !bri->get_scl(adap))
> scl stuck low (or wait a bit to sort out
> clock streching)
> if (bri->get_sda && bri->get_sda(adap))
> break;
>
> bri->set_scl(adap, val);
> ndelay(clk_delay);
> }
>
> Looks ok I think. This would also get rid of the seperate scl check
> before the loop.
I have done something similar in V9..
--
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
