Hello Romain, Tomi, On Tue, 03 Dec 2024 09:42:07 +0100 Romain Gantois <romain.gantois@xxxxxxxxxxx> wrote: > Hi Tomi, > > On vendredi 29 novembre 2024 13:01:58 heure normale d’Europe centrale Tomi Valkeinen wrote: > > 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. > > > > Indeed, the need for dynamic address translation isn't solely determined by > the ATR model. It's also determined by the number of logical I2C devices > connected to the downstream ports. In that sense, hardcoding the flag in the > ATR driver doesn't seem completely appropriate. > > However, you could reasonably imagine that some future ATR models won't > support hot-swapping aliases at runtime. In this case, this flag will be > necessary at the very least as a capability flag i.e. "this ATR model can do > dynamic translation but it's not necessarily activated by default". > > > 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. > > > > Indeed, if an ATR component doesn't support hot-swapping of aliases, then > it will be broken anyway if a device attaches after the ATR's been initialized. > Maybe we should just assume that all supported ATR's should be capable of > modifying their translation table after initialization then. I think this is a reasonable assumption, and so we should not implement support for "non-dynamic ATRs" unless (until?) there is a valid use case. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
