Re: [PATCH] i2c: i801: Use I2C_CLASS_HWMON for i2c mux children

Heiner Kallweit
2023-12-20T21:07:52Z

On 10.11.2023 12:08, Jean Delvare wrote:
> Hi Heiner,
>
> On Thu, 9 Nov 2023 17:34:15 +0100, Heiner Kallweit wrote:
>> On 09.11.2023 14:52, Jean Delvare wrote:
>>> For example, if DMI data says the system is using DDR4 and we were able
>>> to detect and instantiate an ee1004 SPD device at I2C address 0x50 then
>>> we should probe for a JC42.4-compliant temperature sensor device at I2C
>>> address 0x18. An SPD EEPROM at 0x51 would correspond to a possible
>>> JC42.4-compliant device at 0x19, and so on.
>>
>> Nice idea. How about our ASUS use case in i801? i2c_register_spd() isn't
>> called in case of muxing, neither for the parent nor for the mux children.
>
> When I introduced i2c_register_spd(), I wanted to go one step at a
> time, as I didn't know what to expect due to the wide variety of
> systems affected. For this reason, systems where the SMBus is
> multiplexed were originally excluded. There's also a limitation in the
> function itself, on the number of memory slots, which is 4 for now, but
> could be lifted to 8 (maximum number of memory slots connected to a
> single SMBus segment).
>
> The Asus boards we are talking about may have more than 8 memory slots,
> so they would fail the test. If we want to be able to use
> i2c_register_spd() on these boards, some adjustments will be needed.
> For example, we could add a parameter to bypass the memory slot
> counting, so that the function could be called on children segments
> (only a subset of the memory slots will be connected to that segment,
> so the total slot count it irrelevant). We would ignore the DMI data
> and hard-code dimm_count to 8 in that case, to probe all possible
> addresses.
>
> Another approach would be to add a parameter (probably a bit field)
> describing which addresses should be probed, instead of guessing that
> from the dimm_count collected from DMI data. For the Asus boards in
> question, the information is known, and this would speed things up a
> bit by not probing addresses which can't possibly correspond to a
> memory module on a given board. That's only a minor optimization
> though, so not necessarily worth it. OTOH having that possibility would
> give more control on the probing, which may be useful for specific
> boards.
>

Apart from the described extension of i2c_register_spd() I wonder where
to best call it for the muxed smbus segments.

At first I think we have to make the assumption that in case of muxing
memory slots are on the muxed child segments only.
At least I don't see an easy way to deal with the potential scenario that
some memory slots are on the parent segment, and some on muxed segments.

i801_add_mux() instantiates the i2c-mux-gpio platform device, and I think
loading and probing the i2c-mux-gpio driver can happen asynchronously.
This would mean we can't call i2c_register_spd() for the child segments
from i801. So we may have to call it from the i2c-mux-gpio driver.