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@xxxxxxxxx>
>> ---
>> 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
