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

Tomi Valkeinen
2024-11-29T12:02:04Z

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