linux-kernel - Re: [PATCH v4 05/14] i2c-octeon: Enable high-level controller and improve on bus contention

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: <20160323203215.GF19849@katana>
Date:	Wed, 23 Mar 2016 21:32:15 +0100
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Jan Glauber <jglauber@...ium.com>
Cc:	linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
	David Daney <ddaney@...iumnetworks.com>,
	David Daney <david.daney@...ium.com>
Subject: Re: [PATCH v4 05/14] i2c-octeon: Enable high-level controller and
 improve on bus contention

On Fri, Mar 18, 2016 at 09:46:30AM +0100, Jan Glauber wrote:
> From: David Daney <david.daney@...ium.com>
> 
> Use High Level Controller when possible.

Can you give me a one line description what this Controller is? I'd
assume it can do simple write-then-read messages with less setup?

> i2c-octeon was reacting badly to bus contention: when in
> direct-access mode (for transfers > 8 bytes, which cannot use the
> high-level controller) some !ACK or arbitration-loss states were
> not causing the current transfer to be aborted, and the bus released.

So, what does this patch do? Enable HLC for transfers < 8 byte? And for
all other transfers we still suffer from the same problem?

Such information should be here, too. It helps reviewing when I already
have the big picture.

> There's one place in i2c protocol that !ACK is an acceptable
> response: in the final byte of a read cycle.  In this case the
> destination is not saying that the transfer failed, just that it
> doesn't want more data.

Ehrm, no? For reads, the MASTER is saying it doesn't need any more data.
And an I2C eeprom can legally NACK a write, e.g. when it is still
processing the previous write. Also, NACK is a valid response after the
address phase, meaning there is no device listening.

Does the implementation cover the above cases?

> This enables correct behavior of ACK on final byte of non-final read
> msgs too.

The patch is huge and very hard to review. Maybe it needs to be split
up. Brainstorming example: a) move functions like octeon_i2c_set_clock()
upwards, b) change them if needed, c) implement HLC functions, d) add
switching logic to use HLC or non-HLC functions...

But first we need to be clear on the big picture view.

Thanks,

   Wolfram

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)