| Message ID | fc057deb-49f9-49cf-9549-13b2538ed92b@gmail.com |
|---|---|
| Headers | show |
| Series | i2c: Support i2c_register_spd() on multiplexed bus segments | expand |
Hi! 2024-01-10 at 21:13, Heiner Kallweit wrote: > If this is an adapter on a muxed bus segment, assume that each segment > is connected to a subset of the (> 8) overall memory slots. In this > case let's probe the maximum of 8 slots, however stop if the number > of overall populated slots is reached. > > If we're not on a muxed segment and the total number of slots is > 8, > report an error, because then not all SPD eeproms can be addressed. > Presumably the bus is muxed, but the mux config is missing. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/i2c/i2c-smbus.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c > index 74807c6db..e94714d5a 100644 > --- a/drivers/i2c/i2c-smbus.c > +++ b/drivers/i2c/i2c-smbus.c > @@ -351,13 +351,18 @@ void i2c_register_spd(struct i2c_adapter *adap) > if (!dimm_count) > return; > > - dev_info(&adap->dev, "%d/%d memory slots populated (from DMI)\n", > - dimm_count, slot_count); > - > - if (slot_count > 8) { > - dev_warn(&adap->dev, > - "Systems with more than 8 memory slots not supported yet, not instantiating SPD\n"); > - return; > + /* Check whether we're a child adapter on a muxed segment */ > + if (i2c_parent_is_i2c_adapter(adap)) { > + if (slot_count > 8) > + slot_count = 8;' The comment "Only works on systems with 1 to 8 memory slots" above i2c_register_spd() is now incorrect and needs adjusting. > + } else { > + dev_info(&adap->dev, "%d/%d memory slots populated (from DMI)\n", > + dimm_count, slot_count); > + if (slot_count > 8) { > + dev_err(&adap->dev, > + "More than 8 memory slots on a single bus, mux config missing?\n"); > + return; > + } > } > > /* The loop below this hunk is limited by dimm_count, but it is checked separately for each muxed segment. It is therefore now possible to instantiate a total number of slots larger than the dimm_count. That was not possible before. I don't know if that's a problem, but the check have been (silently) relaxed. Cheers, Peter
On 11.01.2024 00:00, Peter Rosin wrote: > Hi! > > 2024-01-10 at 21:13, Heiner Kallweit wrote: >> If this is an adapter on a muxed bus segment, assume that each segment >> is connected to a subset of the (> 8) overall memory slots. In this >> case let's probe the maximum of 8 slots, however stop if the number >> of overall populated slots is reached. >> >> If we're not on a muxed segment and the total number of slots is > 8, >> report an error, because then not all SPD eeproms can be addressed. >> Presumably the bus is muxed, but the mux config is missing. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> drivers/i2c/i2c-smbus.c | 19 ++++++++++++------- >> 1 file changed, 12 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c >> index 74807c6db..e94714d5a 100644 >> --- a/drivers/i2c/i2c-smbus.c >> +++ b/drivers/i2c/i2c-smbus.c >> @@ -351,13 +351,18 @@ void i2c_register_spd(struct i2c_adapter *adap) >> if (!dimm_count) >> return; >> >> - dev_info(&adap->dev, "%d/%d memory slots populated (from DMI)\n", >> - dimm_count, slot_count); >> - >> - if (slot_count > 8) { >> - dev_warn(&adap->dev, >> - "Systems with more than 8 memory slots not supported yet, not instantiating SPD\n"); >> - return; >> + /* Check whether we're a child adapter on a muxed segment */ >> + if (i2c_parent_is_i2c_adapter(adap)) { >> + if (slot_count > 8) >> + slot_count = 8;' > > The comment "Only works on systems with 1 to 8 memory slots" above > i2c_register_spd() is now incorrect and needs adjusting. > Right, this comment can be removed. I'll wait for more feedback on the series before submitting a v2. >> + } else { >> + dev_info(&adap->dev, "%d/%d memory slots populated (from DMI)\n", >> + dimm_count, slot_count); >> + if (slot_count > 8) { >> + dev_err(&adap->dev, >> + "More than 8 memory slots on a single bus, mux config missing?\n"); >> + return; >> + } >> } >> >> /* > > The loop below this hunk is limited by dimm_count, but it is checked > separately for each muxed segment. It is therefore now possible to > instantiate a total number of slots larger than the dimm_count. > That was not possible before. I don't know if that's a problem, but > the check have been (silently) relaxed. > That's intentional. Keeping the current logic would have required to add some kind of counter at the parent level, keeping track of how many memory modules were instantiated per muxed segment. For the sake of simplicity this was omitted. Instead we may probe more slots than needed, however only impact should be that i2c_register_spd() may take a little bit longer on affected systems. > Cheers, > Peter Heiner
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. 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/busses/i2c-i801.c | 1 + drivers/i2c/i2c-mux.c | 4 ++++ drivers/i2c/i2c-smbus.c | 19 ++++++++++++------- drivers/i2c/muxes/i2c-mux-gpio.c | 1 + include/linux/i2c-mux.h | 1 + include/linux/platform_data/i2c-mux-gpio.h | 2 ++ 6 files changed, 21 insertions(+), 7 deletions(-)