mbox series

[v2,0/4] i2c: Support i2c_register_spd() on multiplexed bus segments

Message ID 9f37f64e-f5b8-4928-8716-6d2846c2688a@gmail.com
Headers show
Series i2c: Support i2c_register_spd() on multiplexed bus segments | expand

Message

Heiner Kallweit Jan. 13, 2024, 11:23 a.m. UTC
i801 is the last bus driver supporting I2C_CLASS_SPD. It's used for
device probing on muxed bus segments. Only known use case so far is
systems with more than 8 memory modules (with thermal sensor) on
muxed smbus segments.
As discussed with Jean, to be able to remove I2C_CLASS_SPD completely
the following has to be done:

1. Extend i2c_register_spd() for use on muxed bus segments
2. Enable explicit instantiation of thermal sensors on memory modules
3. Extend i801 to call i2c_register_spd() on muxed bus segments

Step 2 has been accomplished:
caba40ec3531 ("eeprom: at24: Probe for DDR3 thermal sensor in the SPD case")
393bd1000f81 ("eeprom: ee1004: add support for temperature sensor")

Patch 1 does step 1
Patches 2 and 3 provide the basis for patch 4
Patch 4 does step 3

Note: i801 creates the mux platform device, loading and probing of the
mux driver may be asynchronous. Therefore we can't call i2c_register_spd()
for the muxed segments from i801. Instead we have to add a flag to the
platform data, so that the mux driver knows it's supposed to call
i2c_register_spd().

This series replaces the earlier RFC series.

v2:
- remove now obsolete comment in patch 1
- fix link error in some configs in patch 2

Heiner Kallweit (4):
  i2c: smbus: Prepare i2c_register_spd for usage on muxed segments
  i2c: mux: add basic support for calling i2c_register_spd on muxed bus
    segments
  i2c: mux: gpio: Allow to call i2c_register_spd on a muxed segment
  i2c: i801: Call i2c_register_spd() on muxed bus segments

 drivers/i2c/Kconfig                        |  1 +
 drivers/i2c/busses/i2c-i801.c              |  1 +
 drivers/i2c/i2c-mux.c                      |  4 ++++
 drivers/i2c/i2c-smbus.c                    | 20 ++++++++++++--------
 drivers/i2c/muxes/i2c-mux-gpio.c           |  1 +
 include/linux/i2c-mux.h                    |  1 +
 include/linux/platform_data/i2c-mux-gpio.h |  2 ++
 7 files changed, 22 insertions(+), 8 deletions(-)

Comments

Heiner Kallweit Jan. 26, 2024, 9:15 p.m. UTC | #1
On 13.01.2024 12:23, Heiner Kallweit wrote:
> i801 is the last bus driver supporting I2C_CLASS_SPD. It's used for
> device probing on muxed bus segments. Only known use case so far is
> systems with more than 8 memory modules (with thermal sensor) on
> muxed smbus segments.
> As discussed with Jean, to be able to remove I2C_CLASS_SPD completely
> the following has to be done:
> 
> 1. Extend i2c_register_spd() for use on muxed bus segments
> 2. Enable explicit instantiation of thermal sensors on memory modules
> 3. Extend i801 to call i2c_register_spd() on muxed bus segments
> 
> Step 2 has been accomplished:
> caba40ec3531 ("eeprom: at24: Probe for DDR3 thermal sensor in the SPD case")
> 393bd1000f81 ("eeprom: ee1004: add support for temperature sensor")
> 
> Patch 1 does step 1
> Patches 2 and 3 provide the basis for patch 4
> Patch 4 does step 3
> 
> Note: i801 creates the mux platform device, loading and probing of the
> mux driver may be asynchronous. Therefore we can't call i2c_register_spd()
> for the muxed segments from i801. Instead we have to add a flag to the
> platform data, so that the mux driver knows it's supposed to call
> i2c_register_spd().
> 
> This series replaces the earlier RFC series.
> 
> v2:
> - remove now obsolete comment in patch 1
> - fix link error in some configs in patch 2
> 
> Heiner Kallweit (4):
>   i2c: smbus: Prepare i2c_register_spd for usage on muxed segments
>   i2c: mux: add basic support for calling i2c_register_spd on muxed bus
>     segments
>   i2c: mux: gpio: Allow to call i2c_register_spd on a muxed segment
>   i2c: i801: Call i2c_register_spd() on muxed bus segments
> 
>  drivers/i2c/Kconfig                        |  1 +
>  drivers/i2c/busses/i2c-i801.c              |  1 +
>  drivers/i2c/i2c-mux.c                      |  4 ++++
>  drivers/i2c/i2c-smbus.c                    | 20 ++++++++++++--------
>  drivers/i2c/muxes/i2c-mux-gpio.c           |  1 +
>  include/linux/i2c-mux.h                    |  1 +
>  include/linux/platform_data/i2c-mux-gpio.h |  2 ++
>  7 files changed, 22 insertions(+), 8 deletions(-)
> 
Any further feedback on this series, or is it ready to go?
Wolfram Sang Feb. 21, 2024, 5:19 p.m. UTC | #2
Hi Heiner,

> Note: i801 creates the mux platform device, loading and probing of the
> mux driver may be asynchronous. Therefore we can't call i2c_register_spd()
> for the muxed segments from i801. Instead we have to add a flag to the
> platform data, so that the mux driver knows it's supposed to call
> i2c_register_spd().

Has it been considered to use a bus_notifier and check for
BUS_NOTIFY_BOUND_DRIVER?

I'd really like to keep it inside i801 if possible. First, all these
flags in mux drivers only for this corner case are relatively intrusive.
Second, selecting SMBUS for I2C_MUX is also a tad too much for my taste.

I understand that removing CLASS_SPD is a worthy goal. So, if all fails
we could still try this. But I'd think with bus_notifiers it should be
possible to keep it all in i801.

Do you think this could work?

Happy hacking,

   Wolfram
Heiner Kallweit Feb. 21, 2024, 7:46 p.m. UTC | #3
On 21.02.2024 18:19, Wolfram Sang wrote:
> Hi Heiner,
> 
>> Note: i801 creates the mux platform device, loading and probing of the
>> mux driver may be asynchronous. Therefore we can't call i2c_register_spd()
>> for the muxed segments from i801. Instead we have to add a flag to the
>> platform data, so that the mux driver knows it's supposed to call
>> i2c_register_spd().
> 
> Has it been considered to use a bus_notifier and check for
> BUS_NOTIFY_BOUND_DRIVER?
> 
> I'd really like to keep it inside i801 if possible. First, all these
> flags in mux drivers only for this corner case are relatively intrusive.
> Second, selecting SMBUS for I2C_MUX is also a tad too much for my taste.
> 
Right, it's not as clean as I would like to have it.
So far I didn't see any other/better option.

> I understand that removing CLASS_SPD is a worthy goal. So, if all fails
> we could still try this. But I'd think with bus_notifiers it should be
> possible to keep it all in i801.
> 
> Do you think this could work?
> 
Thanks for the hint. I have to admit I wasn't aware of this option.
I'll check and will come back to you.

> Happy hacking,
> 
>    Wolfram
> 
Heiner
Heiner Kallweit Feb. 22, 2024, 10:05 p.m. UTC | #4
On 21.02.2024 18:19, Wolfram Sang wrote:
> Hi Heiner,
> 
>> Note: i801 creates the mux platform device, loading and probing of the
>> mux driver may be asynchronous. Therefore we can't call i2c_register_spd()
>> for the muxed segments from i801. Instead we have to add a flag to the
>> platform data, so that the mux driver knows it's supposed to call
>> i2c_register_spd().
> 
> Has it been considered to use a bus_notifier and check for
> BUS_NOTIFY_BOUND_DRIVER?
> 
I checked, and it looks like this:
Best would be to check for binding gpio mux driver to platform device
"i2c-mux-gpio" has completed. But the bus notification doesn't work for
platform devices.
Instead we can check for the BUS_NOTIFY_ADD_DEVICE event for the
child i2c adapters. This *should* work.
I tested that the events are properly recognized. However I don't have
hw with a muxed SMBUS, so I can't test the actual functionality.
I'll submit a RFC patch.

> I'd really like to keep it inside i801 if possible. First, all these
> flags in mux drivers only for this corner case are relatively intrusive.
> Second, selecting SMBUS for I2C_MUX is also a tad too much for my taste.
> 
> I understand that removing CLASS_SPD is a worthy goal. So, if all fails
> we could still try this. But I'd think with bus_notifiers it should be
> possible to keep it all in i801.
> 
> Do you think this could work?
> 
> Happy hacking,
> 
>    Wolfram
> 
Heiner