Message ID | 20250430-for-upstream-i801-spd5118-no-instantiate-v2-2-2f54d91ae2c7@canonical.com |
---|---|
State | New |
Headers | show |
Series | i2c: i801: don't instantiate spd5118 under SPD Write Disable | expand |
Hi Yo-Jung, On Wed, May 07, 2025 at 10:50:10AM +0200, Yo-Jung (Leo) Lin wrote: > On Tue, May 6, 2025 at 1:53 PM Andi Shyti <andi.shyti@kernel.org> wrote: > > ... > > > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/ > i2c-i801.c > > index a7f89946dad4..88474409e82d 100644 > > --- a/drivers/i2c/busses/i2c-i801.c > > +++ b/drivers/i2c/busses/i2c-i801.c > > @@ -1177,10 +1177,12 @@ static void i801_probe_optional_targets(struct > i801_priv *priv) > > dmi_walk(dmi_check_onboard_devices, &priv->adapter); > > > > /* Instantiate SPD EEPROMs unless the SMBus is multiplexed */ > > -#ifdef CONFIG_I2C_I801_MUX > > - if (!priv->mux_pdev) > > -#endif > > - i2c_register_spd_write_enable(&priv->adapter); > > + if (!IS_ENABLED(CONFIG_I2C_I801_MUX) || !priv->mux_pdev) { > > this if is not really working and it has been reported the > following compile error: > > drivers/i2c/busses/i2c-i801.c: In function 'i801_probe_optional_targets': > drivers/i2c/busses/i2c-i801.c:1180:54: error: 'struct i801_priv' has no > member named 'mux_pdev' > 1180 | if (!IS_ENABLED(CONFIG_I2C_I801_MUX) || !priv->mux_pdev) { > | > > Therefore I'm going to revert just this patch 2/2. Please do > resubmit it and please test it properly with the > CONFIG_I2C_I801_MUX enabled and disabled. > > > Sorry for all the troubles. Will try to propose another patch that takes this > into consideration. don't worry, it happens, that's why we test at different levels ;-) Andi
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index a7f89946dad4..88474409e82d 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -1177,10 +1177,12 @@ static void i801_probe_optional_targets(struct i801_priv *priv) dmi_walk(dmi_check_onboard_devices, &priv->adapter); /* Instantiate SPD EEPROMs unless the SMBus is multiplexed */ -#ifdef CONFIG_I2C_I801_MUX - if (!priv->mux_pdev) -#endif - i2c_register_spd_write_enable(&priv->adapter); + if (!IS_ENABLED(CONFIG_I2C_I801_MUX) || !priv->mux_pdev) { + if (priv->original_hstcfg & SMBHSTCFG_SPD_WD) + i2c_register_spd_write_disable(&priv->adapter); + else + i2c_register_spd_write_enable(&priv->adapter); + } } #else static void __init input_apanel_init(void) {} @@ -1283,7 +1285,10 @@ static int i801_notifier_call(struct notifier_block *nb, unsigned long action, return NOTIFY_DONE; /* Call i2c_register_spd for muxed child segments */ - i2c_register_spd_write_enable(to_i2c_adapter(dev)); + if (priv->original_hstcfg & SMBHSTCFG_SPD_WD) + i2c_register_spd_write_disable(to_i2c_adapter(dev)); + else + i2c_register_spd_write_enable(to_i2c_adapter(dev)); return NOTIFY_OK; }
If SPD Write Disable bit in the SMBus is enabled, writing data to addresses from 0x50 to 0x57 is forbidden. This may lead to the following issues for spd5118 devices: 1) Writes to the sensor hwmon sysfs attributes will always result in ENXIO. 2) During system-wide resume, errors may occur during regcache sync, resulting in the following error messages: kernel: spd5118 1-0050: failed to write b = 0: -6 kernel: spd5118 1-0050: pm: dpm_run_callback(): spd5118_resume [spd5118] returns -6 kernel: spd5118 1-0050: pm: failed to resume async: error -6 3) nvmem won't be usable, because writing to the page selector becomes impossible. Also, BIOS vendors may choose to set the page to a value != 0 after a board reset. This will make the sensor not functional unless its MR11 register can be changed, which is impossible due to writes being disabled. To address these issues, don't instantiate it at all if the SPD Write Disable bit is set. Signed-off-by: Yo-Jung (Leo) Lin <leo.lin@canonical.com> --- drivers/i2c/busses/i2c-i801.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)