[PATCH 0/1] hwmon: (spd5118) support write protected devices

Kean Ren posted 1 patch 4 days, 14 hours ago
Only 0 patches received!
Documentation/hwmon/spd5118.rst |  10 ++++
drivers/hwmon/spd5118.c         | 110 +++++++++++++++++++++++++++++++++++-----
2 files changed, 108 insertions(+), 12 deletions(-)
[PATCH 0/1] hwmon: (spd5118) support write protected devices
Posted by Kean Ren 4 days, 14 hours ago
This patch makes the spd5118 driver usable on systems that do not allow
writes to SPD devices, while keeping the temperature sensor available.

Problem
=======
On many systems the SMBus controller is configured to block writes to SPD
addresses 0x50-0x57. On the i801 family this is SPD Write Disable
(SMBHSTCFG bit 4), set by the BIOS; reads are unaffected. There is no way
for a driver to query this.

The driver assumes that it can write to the device, which does not hold on
such systems:

 - writes to the hwmon attributes always fail with -ENXIO
 - the temperature sensor disable in spd5118_suspend() fails silently
 - spd5118_resume() fails in regcache_sync(), and the kernel reports a
   device resume failure:

     spd5118 0-0050: Failed to write b = 0: -6
     spd5118 0-0050: PM: failed to resume async: error -6

Register 0xb in that message is the page selector register, which regmap
writes back from its cache at the end of regcache_sync(). The write of the
saved configuration register before that fails as well, but the cache code
does not report that error.

What the patch does
===================
Detect write protection once at probe time by writing the page selector
register back with the value just read from it. This is a no-op on a device
which accepts writes, and it fails if writes are blocked. If writes are not
possible, run the device read-only:

 - drop the register cache (REGCACHE_NONE), so that regcache_sync() cannot
   attempt any write; this is what makes resume work
 - skip the writes in the suspend and resume callbacks
 - expose the writable attributes as 0444
 - do not clear a latched alarm status from the alarm read path, as that
   requires a write
 - limit the nvmem 'eeprom' attribute to the currently selected page,
   instead of returning errors for pages that cannot be reached

What is kept
============
The temperature sensor remains usable. For a chip which is already on page
0 - the normal case - reading the temperature and the temperature limits
does not need any write: regmap only writes the page selector when the
selected page actually changes.

What is not available on such systems
=====================================
 - the temperature limits cannot be configured
 - the sensor cannot be disabled during suspend, so it keeps running
 - SPD EEPROM pages 1..7 are not readable, because selecting a page is
   itself a write (the attribute is limited accordingly)
 - a chip which reports a non-zero page and needs the page reset in
   spd5118_i2c_init() is still rejected at probe, since that reset is also
   a write

Relation to earlier proposals
=============================
Two other approaches were posted for the same problem:

1) Make the driver read-only as well, and additionally avoid instantiating
   it on an i801 adapter when SPD writes are disabled:

     hwmon: (spd5118) restrict writes under SPD write protection
     https://lore.kernel.org/all/20250416-for-upstream-spd5118-spd-write-prot-detect-v1-2-8b3bcafe9dad@canonical.com/

     i2c: i801: don't instantiate spd5118 under SPD Write Disable
     https://lore.kernel.org/all/20250430-for-upstream-i801-spd5118-no-instantiate-v2-2f54d91ae2c7@canonical.com/

2) Expose the restriction as an adapter quirk and have spd5118 fail probe,
   on the grounds that write access is mandatory (including a follow-up
   suggestion to skip SPD instantiation on such adapters):

     i2c: i801: Detect SPD Write Disable and expose as adapter quirk
     https://lore.kernel.org/all/20260205102942.28745-1-tinsaetadesse2015@gmail.com/

This patch is deliberately limited to the driver side. It is complementary
to gating instantiation on the adapter side rather than competing with it:
not instantiating avoids probing a driver which cannot work on a given bus,
while tolerating a write protected device keeps the driver correct when it
is instantiated anyway - from devicetree, from userspace via new_device, or
on a controller which blocks SPD writes without the adapter driver knowing
about it.

The trade-off this patch makes is that DDR5 memory temperature monitoring
keeps working on affected systems, instead of not being available at all.

Testing
=======
Tested on a Lenovo ThinkPad P1 Gen 9 (DDR5), with SPD Write Disable set by
the BIOS:

 - before: suspend-to-idle woke immediately; suspend_stats.failed_resume
   incremented on every cycle (14 times) and last_failed_dev was "0-0050"
 - after: 10 suspend/resume cycles with failed_resume and failed_suspend
   remaining 0
 - the chip is still detected and reports its temperature; the writable
   attributes are 0444

Kean Ren (1):
  hwmon: (spd5118) support write protected devices

 Documentation/hwmon/spd5118.rst |  10 ++++
 drivers/hwmon/spd5118.c         | 110 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 108 insertions(+), 12 deletions(-)

-- 
2.53.0