| Message ID | 20241125-fpc202-v3-0-34e86bcb5b56@bootlin.com |
|---|---|
| Headers | show |
| Series | misc: Support TI FPC202 dual-port controller | expand |
Hello Conor, On lundi 25 novembre 2024 19:26:35 heure normale d’Europe centrale Conor Dooley wrote: > On Mon, Nov 25, 2024 at 09:45:15AM +0100, Romain Gantois wrote: > > The FPC202 dual port controller serves as a low speed signal aggregator > > for ... > > + > > +required: > > + - compatible > > + - gpio-controller > > + - "#gpio-cells" > > + - reg > > + - "#address-cells" > > + - "#size-cells" > > + - i2c@0 > > + - i2c@1 > > btw, why are both downstream ports required? It's because both downstream ports are always present in an FPC202 unit so in my opinion, it doesn't make sense to describe an FPC202 with only one downstream port. I suppose you could also consider that ports would only be described in the DT if they were connected to something in the hardware, but I don't think it would make sense to use an FPC202 in this way. After all, the whole point of this component is to act as an I2C ATR and low-speed signal aggregator for downstream devices which would have address collisions if you placed them on the same I2C bus. But then again, you could consider that DT bindings should only describe what is possible, and not only what makes sense as a use case. I don't really know how to answer this question myself, so I'll refer to the maintainers' opinions. Best Regards,
On Tue, Nov 26, 2024 at 09:05:42AM +0100, Romain Gantois wrote: > Hello Conor, > > On lundi 25 novembre 2024 19:26:35 heure normale d’Europe centrale Conor Dooley wrote: > > On Mon, Nov 25, 2024 at 09:45:15AM +0100, Romain Gantois wrote: > > > The FPC202 dual port controller serves as a low speed signal aggregator > > > for > ... > > > + > > > +required: > > > + - compatible > > > + - gpio-controller > > > + - "#gpio-cells" > > > + - reg > > > + - "#address-cells" > > > + - "#size-cells" > > > + - i2c@0 > > > + - i2c@1 > > > > btw, why are both downstream ports required? > > It's because both downstream ports are always present in an FPC202 unit > so in my opinion, it doesn't make sense to describe an FPC202 with only one > downstream port. > > I suppose you could also consider that ports would only be described in the DT > if they were connected to something in the hardware, but I don't think it would > make sense to use an FPC202 in this way. After all, the whole point of this > component is to act as an I2C ATR and low-speed signal aggregator for > downstream devices which would have address collisions if you placed them > on the same I2C bus. > > But then again, you could consider that DT bindings should only describe what is > possible, and not only what makes sense as a use case. I don't really know how to > answer this question myself, so I'll refer to the maintainers' opinions. I don't really know what how this device works, which is why I am asking questions. If there is no use case were someone would only wire up one of the downstream ports then making both required is fine. I was just thinking that someone might only hook devices up to one side of it and leave the other unused entirely. Seemed like it could serve its role without both sides being used based on the diagram in https://docs.kernel.org/i2c/i2c-address-translators.html unless it is not possible for the atr to share the "parent" i2c bus with other devices?
On mardi 26 novembre 2024 19:09:43 heure normale d’Europe centrale Conor Dooley wrote: > On Tue, Nov 26, 2024 at 09:05:42AM +0100, Romain Gantois wrote: > > Hello Conor, ... > > > > But then again, you could consider that DT bindings should only describe > > what is possible, and not only what makes sense as a use case. I don't > > really know how to answer this question myself, so I'll refer to the > > maintainers' opinions. > I don't really know what how this device works, which is why I am asking > questions. If there is no use case were someone would only wire up one > of the downstream ports then making both required is fine. I was just > thinking that someone might only hook devices up to one side of it and > leave the other unused entirely. Seemed like it could serve its role > without both sides being used based on the diagram in > https://docs.kernel.org/i2c/i2c-address-translators.html > unless it is not possible for the atr to share the "parent" i2c bus with > other devices? It is possible for the FPC202 to share it's parent bus with other devices. And I guess you could wire up only one port and use the component as a simple address translator and GPIO aggregator. So indeed, requiring both ports to be described seems unnecessary. Thanks,
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
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. Best Regards, Romain Signed-off-by: Romain Gantois <romain.gantois@bootlin.com> --- Changes in v3: - Described the "reg" property of downstream ports in the FPC202 bindings - Link to v2: https://lore.kernel.org/r/20241118-fpc202-v2-0-744e4f192a2d@bootlin.com Changes in v2: - Renamed port nodes to match i2c adapter bindings. - Declared atr ops struct as static const. - Free downstream ports during FPC202 removal. - Link to v1: https://lore.kernel.org/r/20241108-fpc202-v1-0-fe42c698bc92@bootlin.com --- Romain Gantois (9): dt-bindings: misc: Describe TI FPC202 dual port controller media: i2c: ds90ub960: Replace aliased clients list with bitmap media: i2c: ds90ub960: Protect alias_use_mask with a mutex i2c: use client addresses directly in ATR interface i2c: move ATR alias pool to a separate struct i2c: rename field 'alias_list' of struct i2c_atr_chan to 'alias_pairs' i2c: support per-channel ATR alias pools i2c: Support dynamic address translation misc: add FPC202 dual port controller driver .../devicetree/bindings/misc/ti,fpc202.yaml | 96 +++++ MAINTAINERS | 7 + drivers/i2c/i2c-atr.c | 480 ++++++++++++++------- drivers/media/i2c/ds90ub913.c | 9 +- drivers/media/i2c/ds90ub953.c | 9 +- drivers/media/i2c/ds90ub960.c | 65 +-- drivers/misc/Kconfig | 11 + drivers/misc/Makefile | 1 + drivers/misc/ti_fpc202.c | 440 +++++++++++++++++++ include/linux/i2c-atr.h | 67 ++- 10 files changed, 989 insertions(+), 196 deletions(-) --- base-commit: adc218676eef25575469234709c2d87185ca223a change-id: 20241017-fpc202-6f0b739c2078 Best regards,