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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <51E57211.2040001@yahoo.es>
Date:	Wed, 17 Jul 2013 00:17:21 +0800
From:	Hein Tibosch <hein_tibosch@...oo.es>
To:	balbi@...com, Grygorii Strashko <grygorii.strashko@...com>
CC:	Tony Lindgren <tony@...mide.com>,
	linux-i2c <linux-i2c@...r.kernel.org>,
	linux-omap <linux-omap@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] i2c-omap: always send stop after nack

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@...oo.es>

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/