Re: [PATCH v4 5/6] i2c: smbus: Support DDR5 SPD EEPROMs

[Heiner Kallweit]

On 12.06.2024 18:19, Wolfram Sang wrote:
> 
> CCing Heiner...
> 
>>>>> Yes, maybe this could be simplified to "(LP)DDR memory types"
>>>>>
>>>>
>>>> I rephrased it to "Only works for (LP)DDR memory types up to DDR5".
>>>
>>> Thanks!
>>>
>>>> How about "Only works on systems with 1 to 8 memory slots" ?
>>>
>>> This is a question for Heiner. I'd think it is is still correct, but I
>>> don't know exactly.
>>>
>>
>> My interpretation was that it should work if the DIMMs are connected to
>> multiplexed I2C busses, but probably not if they are connected to
>> different adapters. The error message in that case is a bit misleading,
>> though, because it claims that "More than 8 memory slots on a single bus",
>> which isn't necessarily the case. For example, it should be perfectly valid
>> to have up to 24 DIMMs in this system.
>>
>> i2c-0/name:SMBus PIIX4 adapter port 0 at 0b00
>> i2c-1/name:SMBus PIIX4 adapter port 2 at 0b00
>> i2c-2/name:SMBus PIIX4 adapter port 1 at 0b20
>>
>> ... but I guess that is a question for someone with such a system to answer.
>>
>> Ultimately the handling of systems with more than 8 memory slots will need
>> to be updated at some point. On my systems, with 'i2c: piix4: Register SPDs'
>> applied, I see
>>
>> i2c i2c-0: 4/4 memory slots populated (from DMI)
>>     [my system is running 6.6.y which still generates that message]
>> i2c i2c-0: Successfully instantiated SPD at 0x50
>> i2c i2c-0: Successfully instantiated SPD at 0x51
>> i2c i2c-0: Successfully instantiated SPD at 0x52
>> i2c i2c-0: Successfully instantiated SPD at 0x53
>> i2c i2c-1: 4/4 memory slots populated (from DMI)
>> i2c i2c-2: 4/4 memory slots populated (from DMI)
>>
>> meaning the function is called for each adapter (which makes sense).
>> However, the code counting the DIMMs doesn't really take the adapter
>> into account, meaning adapters 1 and 2 are still probed even though
>> all DIMMs were already instantiated from adapter 0.
>>
>> On a system with more than 8 DIMMs connected to different piix4 adapters
>> (without mux) we'd probably see something like
>>
>> i2c i2c-0: More than 8 memory slots on a single bus, contact i801 maintainer ...
>> i2c i2c-1: More than 8 memory slots on a single bus, contact i801 maintainer ...
>> i2c i2c-2: More than 8 memory slots on a single bus, contact i801 maintainer ...
>>
>> which wouldn't be very helpful. I think the main problem may be that
>> the i801 driver implements sub-adapters as muxes, but the piix4 driver
>> doesn't do (or need) that. The message is also i801 centric which doesn't
>> apply anymore after 'i2c: piix4: Register SPDs' is applied.
>>
>> However, I would not want to even try changing that code without access
>> to a system using piix4 and supporting more than 8 memory slots.
>>
>> Thanks,
>> Guenter
>>
>>

It seems Intel systems never have more than one i801 SMBUS adapter,
therefore systems with more than 8 memory slots have to use muxing.
The current code was developed for the Intel use case, and therefore
doesn't consider that a system may have dedicated SMBUS controllers
per 8 memory slots. So support for this scenario has to be added.