[PATCH] i2c: i801: Do not instantiate spd5118 under SPD Write Disable

Yo-Jung (Leo) Lin posted 1 patch 6 months, 3 weeks ago
There is a newer version of this series
drivers/i2c/busses/i2c-i801.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
[PATCH] i2c: i801: Do not instantiate spd5118 under SPD Write Disable
Posted by Yo-Jung (Leo) Lin 6 months, 3 weeks ago
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 Lin (Leo) <leo.lin@canonical.com>
---
This is intended to be the fixed version of the second patch in the
following patch series[1]:

[PATCH v2 0/2] i2c: i801: don't instantiate spd5118 under SPD Write Disable

The original version got reverted due to breaking i386 defconfig build.
However the first patch in that series didn't. To avoid the first patch in
the series being repeatedly sent and applied, this is sent as a separate
patch. 

Also not pulling Reviewed-by from Guenter because the patch differs from
what was originally reviewed.

[1] https://lore.kernel.org/all/20250430-for-upstream-i801-spd5118-no-instantiate-v2-0-2f54d91ae2c7@canonical.com/
---
 drivers/i2c/busses/i2c-i801.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index a7f89946dad4..de17532402b9 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1157,6 +1157,27 @@ static void dmi_check_onboard_devices(const struct dmi_header *dm, void *adap)
 	}
 }
 
+static void __i801_register_spd(struct i801_priv *priv)
+{
+	if (priv->original_hstcfg & SMBHSTCFG_SPD_WD)
+		i2c_register_spd_write_disable(&priv->adapter);
+	else
+		i2c_register_spd_write_enable(&priv->adapter);
+}
+
+#ifdef CONFIG_I2C_I801_MUX
+static void i801_register_spd(struct i801_priv *priv)
+{
+	if (!priv->mux_pdev)
+		__i801_register_spd(priv);
+}
+#else
+static void i801_register_spd(struct i801_priv *priv)
+{
+	__i801_register_spd(priv);
+}
+#endif
+
 /* Register optional targets */
 static void i801_probe_optional_targets(struct i801_priv *priv)
 {
@@ -1177,10 +1198,7 @@ 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);
+	i801_register_spd(priv);
 }
 #else
 static void __init input_apanel_init(void) {}
@@ -1283,7 +1301,7 @@ 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));
+	__i801_register_spd(priv);
 
 	return NOTIFY_OK;
 }

---
base-commit: 176e917e010cb7dcc605f11d2bc33f304292482b
change-id: 20250526-for-upstream-not-instantiate-spd5118-463225b346a0

Best regards,
-- 
Yo-Jung (Leo) Lin <leo.lin@canonical.com>
Re: [PATCH] i2c: i801: Do not instantiate spd5118 under SPD Write Disable
Posted by kernel test robot 6 months, 3 weeks ago
Hi Yo-Jung,

kernel test robot noticed the following build errors:

[auto build test ERROR on 176e917e010cb7dcc605f11d2bc33f304292482b]

url:    https://github.com/intel-lab-lkp/linux/commits/Yo-Jung-Leo-Lin/i2c-i801-Do-not-instantiate-spd5118-under-SPD-Write-Disable/20250528-163253
base:   176e917e010cb7dcc605f11d2bc33f304292482b
patch link:    https://lore.kernel.org/r/20250528-for-upstream-not-instantiate-spd5118-v1-1-8216e2d38918%40canonical.com
patch subject: [PATCH] i2c: i801: Do not instantiate spd5118 under SPD Write Disable
config: loongarch-randconfig-001-20250529 (https://download.01.org/0day-ci/archive/20250529/202505290728.VsNgBfDO-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250529/202505290728.VsNgBfDO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505290728.VsNgBfDO-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-i801.c: In function 'i801_notifier_call':
>> drivers/i2c/busses/i2c-i801.c:1304:9: error: implicit declaration of function '__i801_register_spd' [-Wimplicit-function-declaration]
    1304 |         __i801_register_spd(priv);
         |         ^~~~~~~~~~~~~~~~~~~


vim +/__i801_register_spd +1304 drivers/i2c/busses/i2c-i801.c

  1291	
  1292	static int i801_notifier_call(struct notifier_block *nb, unsigned long action,
  1293				      void *data)
  1294	{
  1295		struct i801_priv *priv = container_of(nb, struct i801_priv, mux_notifier_block);
  1296		struct device *dev = data;
  1297	
  1298		if (action != BUS_NOTIFY_ADD_DEVICE ||
  1299		    dev->type != &i2c_adapter_type ||
  1300		    i2c_root_adapter(dev) != &priv->adapter)
  1301			return NOTIFY_DONE;
  1302	
  1303		/* Call i2c_register_spd for muxed child segments */
> 1304		__i801_register_spd(priv);
  1305	
  1306		return NOTIFY_OK;
  1307	}
  1308	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] i2c: i801: Do not instantiate spd5118 under SPD Write Disable
Posted by Yo-Jung (Leo) Lin 6 months, 3 weeks ago
On Thu, May 29, 2025 at 8:00 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Yo-Jung,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on 176e917e010cb7dcc605f11d2bc33f304292482b]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Yo-Jung-Leo-Lin/i2c-i801-Do-not-instantiate-spd5118-under-SPD-Write-Disable/20250528-163253
> base:   176e917e010cb7dcc605f11d2bc33f304292482b
> patch link:    https://lore.kernel.org/r/20250528-for-upstream-not-instantiate-spd5118-v1-1-8216e2d38918%40canonical.com
> patch subject: [PATCH] i2c: i801: Do not instantiate spd5118 under SPD Write Disable
> config: loongarch-randconfig-001-20250529 (https://download.01.org/0day-ci/archive/20250529/202505290728.VsNgBfDO-lkp@intel.com/config)
> compiler: loongarch64-linux-gcc (GCC) 15.1.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250529/202505290728.VsNgBfDO-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202505290728.VsNgBfDO-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>    drivers/i2c/busses/i2c-i801.c: In function 'i801_notifier_call':
> >> drivers/i2c/busses/i2c-i801.c:1304:9: error: implicit declaration of function '__i801_register_spd' [-Wimplicit-function-declaration]
>     1304 |         __i801_register_spd(priv);
>          |         ^~~~~~~~~~~~~~~~~~~
>
>
> vim +/__i801_register_spd +1304 drivers/i2c/busses/i2c-i801.c
>
>   1291
>   1292  static int i801_notifier_call(struct notifier_block *nb, unsigned long action,
>   1293                                void *data)
>   1294  {
>   1295          struct i801_priv *priv = container_of(nb, struct i801_priv, mux_notifier_block);
>   1296          struct device *dev = data;
>   1297
>   1298          if (action != BUS_NOTIFY_ADD_DEVICE ||
>   1299              dev->type != &i2c_adapter_type ||
>   1300              i2c_root_adapter(dev) != &priv->adapter)
>   1301                  return NOTIFY_DONE;
>   1302
>   1303          /* Call i2c_register_spd for muxed child segments */
> > 1304          __i801_register_spd(priv);
>   1305
>   1306          return NOTIFY_OK;
>   1307  }
>   1308

I think this happens likely because I put the __i801_register_spd() inside the:

#if defined CONFIG_X86 && defined CONFIG_DMI
...
#endif

So the function went missing in loongarch. Will fix this in v2.

>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki