Re: [PATCH] i2c-omap: always send stop after nack

From: Hein Tibosch
Date: Tue Jul 16 2013 - 12:19:29 EST


Hi Grygorii, Filipe,

On 7/16/2013 9:00 PM, Felipe Balbi wrote:
> On Tue, Jul 16, 2013 at 03:08:04PM +0300, Grygorii Strashko wrote:
>> Hi Felipe,
>> On 07/16/2013 02:27 PM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Tue, Jul 16, 2013 at 02:01:11PM +0300, Grygorii Strashko wrote:
>>>>>>>> On a OMAP4460, i2c-bus-3:
>>>>>>>>
>>>>>>>> A driver (lm75) is causing many 'timeout waiting for bus ready' errors.
>>>>>>>> SDA remains high (as it should), but SCL remains low after a NACK.
>>>>>>>> The bus becomes _unusable for other clients_.
>>>>>>>>
>>>>>>>> While probing, "lm75" writes a command, followed by a read + stop,
>>>>>>>> but the write command is NACK'd. The chip does accept other writes/reads,
>>>>>>>> it just refuses to ack invalid commands.

In case the NACK may not be ignored, I believe it is correct
to break out of the loop and send a stop for 2 reasons:

1. all chips, including the target chip, will know that the
current transaction is finished

2. to set CLK high again, which prevents numerous timeouts
on subsequent transactions

As a test I've set 'I2C_M_IGNORE_NAK' for all .detect messages
sent by the lm75 driver.
Now the chip (tmp105) will provide read data as expected
after the _repeated start_, even though it NACK'd the preceding
WR command.
And also the detection trick does work now, addresses 4,5,6,7 return
the same read data as was returned from the last valid address 2.

Felipe:
> Which means your original patch starts to make a lot more sense. I
> wonder is this is really what we should be doing though - breaking out
> of the loop, I mean.

So yes, we have to break the loop as the caller is not interested
in processing any further commands.

In i2c/algos/i2c-algo-bit.c, bit_xfer() works exactly the same way:
break the loop (unless IGNORE_NAK) and call unconditionally i2c_stop().

It looks like TI's i2c-davinci will have the same problem as i2c-omap,
and will need the same change.

Grygorii, if you submit the patch, please add my

Signed-off-by: Hein Tibosch <hein_tibosch@xxxxxxxx>

cause you were earlier to notice and fix this problem.

Hein
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/