Re: [PATCH 2/4] i2c: at91: implement i2c bus recovery

[<Codrin.Ciubotariu@microchip.com>]

On 22.10.2019 10:59, Kamel Bouhara wrote:
> On 21/10/2019 22:20, Wolfram Sang wrote:
>> On Wed, Oct 02, 2019 at 04:46:56PM +0200, Kamel Bouhara wrote:
>>> Implement i2c bus recovery when slaves devices might hold SDA low.
>>> In this case re-assign SCL/SDA to gpios and issue 9 dummy clock pulses
>>> until the slave release SDA.
>>>
>>> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
>>
>> Setting up the bus_recovery looks OK. However, I don't see any call to
>> i2c_recover_bus(), so the bus_recovery is never used. Did you test this
>> and see an effect?
>>
> Indeed, I guess I mess it up while doing some git stuff, it should be 
> called from at91_do_twi_transfer() when the transfer times out...
> I actually tested it and verified the recovery is triggered by pulling 
> the SCL to the ground ...
> 
>> Also, I think we should merge this patch "[PATCH v3] i2c: at91: Send bus
>> clear command if SCL or SDA is down" into this series. The crucial thing
>> for both is when to apply the recovery (at the beginning of a
>> transfer!). The rest is "just" that some HW needs a bus_recovery_info
>> for pinctrl/GPIO handling (from this patch), while other HW needs a
>> bus_recovery_info with a custom recover_bus callback.
>>
>> Opinions?
>>
> I'm OK to merge the two series.

So at the beginning of a new transfer, we should check if SDA (or SCL?) 
is low and, if it's true, only then we should try recover the bus.

Kamel, let me know if I can help with anything.

Best regards,
Codrin