Re: [PATCH] i2c: i801: Simplify class-based client device instantiation

[Jean Delvare <jdelvare@suse.de>]

Hi Heiner, Andi,

On Tue, 10 Oct 2023 21:27:44 +0200, Heiner Kallweit wrote:
> Now that the legacy eeprom driver was removed, the only remaining i2c
> client driver with class SPD autodetection is jc42, and this driver
> supports also class HWMON. Therefore we can remove class SPD from the

I did not notice this change when it happened back in 2016. This broke
the i2c-i801 driver as a side effect, because the Asus-specific mux
code assumes that no I2C device driver can probe both the SMBus trunk
and the SMBus segments behind the muxes. This is done (as you must know
as this patch touches that part of the code) by ensuring that the trunk
and the muxed segments do not share any class flag. This was sufficient
as long as all device driver themselves only registered for one class,
but this is no longer the case of the jc42 driver.

So loading the jc42 driver on one of these Asus server board systems
would possibly result in multiple jc42 devices being instantiated for
the same underlying hardware device. The one instantiated on the trunk
would return incorrect values or errors depending on the mux setting.

Probably this went unnoticed because nobody was running such old server
boards when the change happened, or they would stick to older kernel
versions.

> supported classes of the i801 adapter driver.
> Legacy class-based instantiation shouldn't be used in new code, so I
> think we can remove also the generic logic that ensures that supported
> classes of parent and muxed adapters don't overlap.

Agreed. If we were to add support for a new server board with muxed
SMBus, we would disable class-based probing and instead explicitly
instantiate devices. To be honest, I don't know why we didn't do that
for the Asus Z8 series already, as I think it was already available,
and it would have made the code a lot more simple.

If anyone ever complains about the bug mentioned above, then we'll have
to do it anyway.

> Note: i801 parent supports just class HWMON now, and muxed childs

"children" ^^

> class SPD, so the supported classes don't overlap.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/i2c/busses/i2c-i801.c | 38 +++++++----------------------------
>  1 file changed, 7 insertions(+), 31 deletions(-)
> (...)

Fine with me.

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support