Re: [BUG] hwmon: (spd5118) resume fails -ENXIO on write protected hubs
GG <[email protected]>
| Newsgroups | org.kernel.vger.linux-hwmon,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <vPlYOTmuxKCPw1cRvxIWJfqFl7p8endHIMWS9DZwINEUCPql8VLF7GBkmEBMokDKtHop6K5TAPUAlk-17CFWlA8extBUV7XGnuVx1KJK3iU=@bungeetaco.com> |
Sorry - I should have searched the archives before sending my last email. I've since found this from February, which is the same problem: https://lore.kernel.org/linux-hwmon/[email protected]/ And I missed the obvious clue sitting in my own boot log: i801_smbus 0000:00:1f.4: SPD Write Disable is set So my kernel already knew. Apologies for the noise. One thing I'll mention in case it's useful: that series above has spd5118 refuse to probe when writes are disabled. On my machine that would lose me two working temperature sensors. Reads are completely fine here, I get correct temps, and my temperature limits read back properly. It's only writing that fails. Happy to test anything on my hardware if it would help. Gabriel G On Monday, August 17th, 2026 at 12:29 PM, GG <[email protected]> wrote: > Hi there, > > I apologize in advance if this isn't the right place to send this. I ran into > a quirk on my own machine, went digging, and ended up working out what causes > it, so I figured it was probably worth sharing. Please feel free to ignore all > of this if it's not useful or doesn't apply. > > Before you read any further, three things you should probably know: > > - I'm not a programmer. I can tell you what my hardware does, and I ran > every test below myself on it, but if you ask me to defend the actual > implementation I'll be out of my depth pretty quickly. > > - I worked this out with an AI coding assistant (Claude, claude-opus-5) over > a long back-and-forth debugging session. It suggested things to try and > wrote the instrumented test modules, and I built and ran each one on this > machine and fed the results back. It drafted this write-up too, and I've > edited it. > > - So this isn't a formal patch submission and there's deliberately no > Signed-off-by on it. Treat it as a bug report that happens to come with a > suggestion attached, and please feel free to bin the patch entirely and do > something completely different. > > The one thing I can offer that might actually be worth something is the > hardware. This machine reproduces the problem every single time, so I'm happy > to test whatever you'd like, in whatever form. > > Anyway, here's what I found. > > spd5118_resume() fails with -ENXIO on every single suspend/resume cycle here, > and I think it's because the SPD hubs on this board are write protected by the > platform firmware. > > Hardware: > Gigabyte Z790 AORUS PRO X, BIOS F11a > Intel i9-14900KS > 2x 32GB DDR5, hubs at i2c 12-0051 and 12-0053 > Both report: DDR5 temperature sensor: vendor 0x06:0x32 revision 1.6 > Kernel 7.1.8 (CachyOS) > > This shows up on every resume: > > spd5118 12-0051: Failed to write b = 0: -6 > spd5118 12-0051: PM: dpm_run_callback(): spd5118_resume returns -6 > spd5118 12-0051: PM: spd5118_resume returned -6 after 15085 usecs > spd5118 12-0051: PM: failed to resume async: error -6 > > The sensor itself works fine either side of a suspend and the configured > limits survive, so nothing is actually broken for me as a user. It's just the > error, plus 5-18ms of pointless failing SMBus traffic per DIMM every time. > > The thing that finally made it click is that this has nothing to do with > suspend or resume at all. These hubs refuse writes all the time. Writing > temp1_max back to the exact value it already holds, on a fully booted and idle > system, fails the same way: > > $ cat /sys/class/hwmon/hwmon3/temp1_max > 55000 > $ sudo sh -c 'echo 55000 > /sys/class/hwmon/hwmon3/temp1_max' > sh: line 1: echo: write error: No such device or address > > Reads are completely unaffected. During resume, raw i2c_smbus_read_byte_data() > on MR11 and on every other register succeeds on the first try and returns > sensible values. Every single i2c_smbus_write_byte_data() comes back -ENXIO. > > Which means this, in spd5118_suspend(): > > regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG, SPD5118_TS_DISABLE, > SPD5118_TS_DISABLE); > > is failing silently, because its return value isn't checked. The sensor never > actually gets disabled. But the cache still gets marked dirty, so on resume > regcache_sync() goes off to restore state that was never changed in the first > place. > > The error message itself sent me down the wrong path for quite a while. It > doesn't come from the sync, it comes from regcache_sync()'s epilogue, which > force-writes every regmap range selector and then hands that write's error > back as its own return value. For spd5118 in I2C legacy mode that selector is > MR11, which is also a cached writeable register, and with window_start 0 and > window_len 0x100 it sits inside its own paging window. That's why the message > singles out register 0xb, which had me convinced for ages that there was > something specifically wrong with MR11. > > Dead ends, in case it saves anyone else the time: > > - Dropping MR11 from the regcache before the sync does silence the "Failed > to write b = 0" message, but resume still fails, because the next register > isn't writeable either. > - MR11 already reads back as 0 on resume, so there's no stale page. > - Reads succeed on the first attempt, so the hub isn't slow to wake up. > - Polling for a write to be accepted for up to 2 seconds never got one > through, and added 3 seconds to resume. > - Pinning i2c-i801 to power/control=on for the whole cycle changes nothing. > The adapter isn't marked suspended at that point either: > __i2c_check_suspended() (drivers/i2c/i2c-core.h) returns -ESHUTDOWN and > emits a "Transfer while suspended" dev_WARN, and neither of those shows up > anywhere in my logs. > > The patch below is what ended up working for me here: check whether the device > actually accepted that suspend-time write, and if it didn't, don't bother > marking the cache dirty or syncing on resume. With it applied, resume returns > 0 in 0us and the error is gone. I ran 5 suspend/resume cycles on a fresh boot > with no debug kernel options, and /sys/kernel/debug/suspend_stats went from > success=0 failed_resume=0 to success=5 failed_resume=0, with temperatures and > limits unaffected throughout. > > One thing I can't check myself: I don't have any hardware with a hub that does > accept writes, so I can't confirm the normal path still behaves properly on a > machine like that. > > Candidate change against drivers/hwmon/spd5118.c: > > --- a/drivers/hwmon/spd5118.c > +++ b/drivers/hwmon/spd5118.c > @@ -79,6 +79,7 @@ struct spd5118_data { > struct regmap *regmap; > struct mutex nvmem_lock; > bool is_16bit; > + bool suspend_wp; /* device NAKed the suspend write: nothing to restore */ > }; > > @@ -498,10 +499,27 @@ static int spd5118_suspend(struct device *dev) > return err; > > regcache_cache_bypass(regmap, true); > - regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG, SPD5118_TS_DISABLE, > - SPD5118_TS_DISABLE); > + err = regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG, > + SPD5118_TS_DISABLE, SPD5118_TS_DISABLE); > regcache_cache_bypass(regmap, false); > > + /* > + * Platform firmware may write protect the SPD hub after memory > + * training, in which case the device serves reads but NAKs every write > + * with -ENXIO. The temperature sensor was then never disabled, so > + * there is no device state to restore on resume. > + */ > + data->suspend_wp = err < 0; > + if (data->suspend_wp) { > + dev_dbg(dev, "write protected, skipping state save (%d)\n", err); > + /* Still avoid bus traffic while suspended. */ > + regcache_cache_only(regmap, true); > + return 0; > + } > + > regcache_cache_only(regmap, true); > regcache_mark_dirty(regmap); > > @@ -514,6 +532,10 @@ static int spd5118_resume(struct device *dev) > struct regmap *regmap = data->regmap; > > regcache_cache_only(regmap, false); > + > + if (data->suspend_wp) > + return 0; > + > return regcache_sync(regmap); > } > > As far as I can tell this goes back to d1b4c755081a ("hwmon: (spd5118) Add > suspend/resume support"), which first shipped in v6.11. > > Thanks for taking a look, and no worries at all if this isn't useful. > > Gabriel G