linux-kernel - Re: [PATCH v3 0/9] misc: Support TI FPC202 dual-port controller

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: <0a125973-fd33-455d-a3ab-fba3357155ee@ideasonboard.com>
Date: Fri, 29 Nov 2024 14:01:58 +0200
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Romain Gantois <romain.gantois@...tlin.com>,
 Luca Ceresoli <luca.ceresoli@...tlin.com>
Cc: Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 Kory Maincent <kory.maincent@...tlin.com>, linux-i2c@...r.kernel.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-media@...r.kernel.org, linux-gpio@...r.kernel.org,
 Wolfram Sang <wsa+renesas@...g-engineering.com>,
 Andi Shyti <andi.shyti@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Derek Kiernan <derek.kiernan@....com>,
 Dragan Cvetic <dragan.cvetic@....com>, Arnd Bergmann <arnd@...db.de>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Mauro Carvalho Chehab <mchehab@...nel.org>,
 Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
 <brgl@...ev.pl>, Cosmin Tanislav <demonsingur@...il.com>
Subject: Re: [PATCH v3 0/9] misc: Support TI FPC202 dual-port controller

Hi,

On 25/11/2024 10:45, Romain Gantois wrote:
> Hello everyone,
> 
> This is version three of my series which adds support for the TI FPC202
> dual-port controller. This is an unusual kind of device which is used as a
> low-speed signal aggregator for various types of SFP-like hardware ports.
> 
> The FPC202 exposes an I2C, or SPI (not supported in this series) control
> interface, which can be used to access two downstream I2C busses, along
> with a set of low-speed GPIO signals for each port. It also has I2C address
> translation (ATR) features, which allow multiple I2C devices with the same
> address (e.g. SFP EEPROMs at address 0x50) to be accessed from the upstream
> control interface on different addresses.
> 
> I've chosen to add this driver to the misc subsystem, as it doesn't
> strictly belong in either the i2c or gpio sybsystem, and as far as I know
> it is the first device of its kind to be added to the kernel.
> 
> Along with the FPC202 driver itself, this series also adds support for
> dynamic address translation to the i2c-atr module. This allows I2C address
> translators to update their translation table on-the-fly when they receive
> transactions to unmapped clients. This feature is needed by the FPC202
> driver to access up to three logical I2C devices per-port, given that the
> FPC202 address translation table only has two address slots.

While the FPD-Link devices are quite different than the TPC202, I wonder 
what's the difference wrt. the ATR... Afaics, the difference is that the 
FPC202 has 2 slots whereas UB960 has 8. So if you have 3+ remote devices 
on FPC202, you get problems, or if you have 9+ devices on UB960, you get 
problems.

Yet this series adds a I2C_ATR_FLAG_DYNAMIC_C2A flag which the driver 
needs to set, and the i2c-atr has different code paths depending on the 
flag. In other words, either the driver author (if it's a hardcoded 
flag) or the driver (if it's set dynamically) is assumed to know how 
many remote devices there are, and whether that flag is needed.

On the other hand, if I consider I2C_ATR_FLAG_DYNAMIC_C2A meaning that 
the device can support dynamically changing the ATR, then it makes more 
sense, and also UB960 should set the flag.

But then I wonder, do we even have cases with ATRs that need to be 
programmed once at init time, and cannot be changed afterwards? If not, 
then the I2C_ATR_FLAG_DYNAMIC_C2A can be the default, and the 
non-I2C_ATR_FLAG_DYNAMIC_C2A code can be dropped. Actually, even the 
current upstream i2c-atr is dynamic in a sense: the clients are attached 
via the i2c_atr_bus_notifier_call(), one by one.

If we just have .attach_addr()/.detach_addr(), and call those on demand, 
and perhaps use LRU to handle the ATR table, it would maybe work for 
both FPC202 and FPD-Link just fine.

So TLDR: do we even need any kind of special "dynamic atr"-feature for 
FPC202, or is it really just a small improvement to the current i2c-atr 
and applies to all ATR devices?

How this relates to the "Support dynamic address translation" and GMSL, 
I'm not sure yet.

  Tomi