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

Jean Delvare
2023-11-06T14:50:42Z

Hi Heiner,

On Tue, 31 Oct 2023 21:30:22 +0100, Heiner Kallweit wrote:
> In case there ever should be a jc42-driven device on a muxed child bus,
> we may end up with multiple logical devices, see linked discussion.
>
> jc42 is the only remaining i2c client device driver supporting
> I2C_CLASS_SPD, however it supports I2C_CLASS_HWMON too. Therefore
> we can safely switch to I2C_CLASS_HWMON on the muxed child busses.
> IMO it's more logical anyway that the muxed child busses support
> the same class that the parent supports if it's not muxed.
>
> This change is one further step towards removing I2C_CLASS_SPD.
>
> Link: https://www.spinics.net/lists/linux-i2c/msg65458.html
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/i2c/busses/i2c-i801.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 070999139..8c9bb2e09 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1298,7 +1298,7 @@ static struct i801_mux_config i801_mux_config_asus_z8_d12 = {
> .gpio_chip = "gpio_ich",
> .values = { 0x02, 0x03 },
> .n_values = 2,
> - .classes = { I2C_CLASS_SPD, I2C_CLASS_SPD },
> + .classes = { I2C_CLASS_HWMON, I2C_CLASS_HWMON },
> .gpios = { 52, 53 },
> .n_gpios = 2,
> };
> @@ -1307,7 +1307,7 @@ static struct i801_mux_config i801_mux_config_asus_z8_d18 = {
> .gpio_chip = "gpio_ich",
> .values = { 0x02, 0x03, 0x01 },
> .n_values = 3,
> - .classes = { I2C_CLASS_SPD, I2C_CLASS_SPD, I2C_CLASS_SPD },
> + .classes = { I2C_CLASS_HWMON, I2C_CLASS_HWMON, I2C_CLASS_HWMON },
> .gpios = { 52, 53 },
> .n_gpios = 2,
> };
> @@ -1395,6 +1395,9 @@ static void i801_add_mux(struct i801_priv *priv)
>
> mux_config = id->driver_data;
>
> + /* Parent and mux children class support must not overlap */
> + priv->adapter.class = 0;

There may also be a hardware monitoring device on the parent segment on
these boards. With this change, that device will no longer be detected.
That would be a regression, so I have to nack this proposed change,
sorry.

The only way forward I can think of (if we want to get rid of
I2C_CLASS_SPD) would be to remove device auto-detection on children
segments completely (.class = 0 for them) and instead actively probe
for SPD EEPROMs and JC42 thermal sensors on these segments. For SPD we
should be able to just reuse i2c_register_spd(). For JC42 we would need
similar code, which doesn't exist yet.

--
Jean Delvare
SUSE L3 Support