On 26.02.2024 11:32, Wolfram Sang wrote:
> Hi Heiner,
>
>> + /* Check whether we're a child adapter on a muxed segment */
>
> The comment describes the 'if' but not 'then'. How about sth like "If we
> are a child on a muxed segment then limit slots to..."?
>
OK, this would be better.
>> + if (i2c_parent_is_i2c_adapter(adap)) {
>> + slot_count = 8;
>
> I don't know much about DMI. I just noticed that there are no printouts
> in this code path. Will there be one for the parent?
>
With the patch as-is there's we omit printout for systems with > 8 memory
slots. I'm not aware of any way to find out how many memory slots belong to
a specific child bus segment. So all we could do is print per child segment
how many slots are populated. But we have a printout per populated slot
already: "Successfully instantiated SPD at 0x%hx\n"
So IMO we don't loose any relevant info.
>> + } else {
>> + if (slot_count > 8) {
>> + dev_err(&adap->dev,
>> + "More than 8 memory slots on a single bus, mux config missing?\n");
>
> With this error message, I as a user would think I need to setup a mux
> config somewhere. But it is missing from DMI, or? Then, we should
> probably use even FW_BUG in the message?
>
Actually a developer has to add the config to i801's mux_dmi_table[].
So yes, we should change the message to something like:
"More than 8 memory slots on a single bus, contact i801 maintainer to
add the missing mux configuration"
> Happy hacking,
>
> Wolfram
>
Heiner
