[PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Tinsae Tadesse]

SPD5118 DDR5 temperature sensors may be temporarily unavailable
during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
register synchronization to be retried later.

Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
---
 drivers/hwmon/spd5118.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 5da44571b6a0..ec9f14f6e0df 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
 {
 	struct spd5118_data *data = dev_get_drvdata(dev);
 	struct regmap *regmap = data->regmap;
+	int ret;
 
 	regcache_cache_only(regmap, false);
-	return regcache_sync(regmap);
+	ret = regcache_sync(regmap);
+	if(ret == -ENXIO || ret == -EIO) {
+		dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
+		return 0;
+	}
+	return ret;
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
-- 
2.47.3

[PATCH 2/3] hwmon: spd5118: Retry temperature reads after temporary I2C errors

[Tinsae Tadesse]

After suspend/resume, the SPD5118 hub or the underlying I2C adapter
may not be immediately available, causing register reads to fail.

Attempt a single regcache re-synchronization and retry the read to
allow recovery.

Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
---
 drivers/hwmon/spd5118.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index ec9f14f6e0df..63f798991363 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -101,6 +101,7 @@ static int spd5118_read_temp(struct regmap *regmap, u32 attr, long *val)
 	int reg, err;
 	u8 regval[2];
 	u16 temp;
+	bool retried = false;
 
 	switch (attr) {
 	case hwmon_temp_input:
@@ -122,9 +123,17 @@ static int spd5118_read_temp(struct regmap *regmap, u32 attr, long *val)
 		return -EOPNOTSUPP;
 	}
 
+retry:
 	err = regmap_bulk_read(regmap, reg, regval, 2);
-	if (err)
+	if (err) {
+		if (!retried && (err == -ENXIO || err == -EIO)) {
+			retried = true;
+			regcache_mark_dirty(regmap);
+			if (!regcache_sync(regmap))
+				goto retry;
+		}
 		return err;
+	}
 
 	temp = (regval[1] << 8) | regval[0];
 
-- 
2.47.3

Re: [PATCH 2/3] hwmon: spd5118: Retry temperature reads after temporary I2C errors

[Guenter Roeck]

On 1/10/26 09:19, Tinsae Tadesse wrote:
> After suspend/resume, the SPD5118 hub or the underlying I2C adapter
> may not be immediately available, causing register reads to fail.
> 
> Attempt a single regcache re-synchronization and retry the read to
> allow recovery.
> 
> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
> ---
>   drivers/hwmon/spd5118.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index ec9f14f6e0df..63f798991363 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -101,6 +101,7 @@ static int spd5118_read_temp(struct regmap *regmap, u32 attr, long *val)
>   	int reg, err;
>   	u8 regval[2];
>   	u16 temp;
> +	bool retried = false;
>   
>   	switch (attr) {
>   	case hwmon_temp_input:
> @@ -122,9 +123,17 @@ static int spd5118_read_temp(struct regmap *regmap, u32 attr, long *val)
>   		return -EOPNOTSUPP;
>   	}
>   
> +retry:
>   	err = regmap_bulk_read(regmap, reg, regval, 2);
> -	if (err)
> +	if (err) {
> +		if (!retried && (err == -ENXIO || err == -EIO)) {
> +			retried = true;
> +			regcache_mark_dirty(regmap);
> +			if (!regcache_sync(regmap))
> +				goto retry;
> +		}
>   		return err;
> +	}
>   
>   	temp = (regval[1] << 8) | regval[0];
>   


Wrong solution. This affects all accesses, not just the first access after
resume. regmap_bulk_read() can fail for any reason, not just because the
system just returned from resume. The above code just assumes that the failure
is due to a bad resume based on the returned error code. That may be the case
for the i801 controller driver, but even that would be just by chance.

Guenter

[PATCH 3/3] hwmon: spd5118: Avoid hardware access during suspend and resume

[Tinsae Tadesse]

The SPD5118 hub may be inaccessible during suspend/resume.
Avoid updating/writing hardware in PM callbacks by switching the regmap
to cache-only mode during suspend and deferring synchronization
until first access.

This prevents the below I2C errors and async resume failures.

spd5118 ...: Failed to write b = 0: -6
spd5118 ...: PM: dpm_run_callback(): spd5118_resume returns -6
spd5118 ...: PM: failed to resume async: error -6

Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
---
 drivers/hwmon/spd5118.c | 36 ++++++++++--------------------------
 1 file changed, 10 insertions(+), 26 deletions(-)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 63f798991363..a8afde7f47b2 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -494,25 +494,12 @@ static const struct regmap_config spd5118_regmap16_config = {
 static int spd5118_suspend(struct device *dev)
 {
 	struct spd5118_data *data = dev_get_drvdata(dev);
-	struct regmap *regmap = data->regmap;
-	u32 regval;
-	int err;
 
 	/*
-	 * Make sure the configuration register in the regmap cache is current
-	 * before bypassing it.
+	 * The SPD5118 hub may be inaccessible; avoid hardware access.
 	 */
-	err = regmap_read(regmap, SPD5118_REG_TEMP_CONFIG, &regval);
-	if (err < 0)
-		return err;
-
-	regcache_cache_bypass(regmap, true);
-	regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG, SPD5118_TS_DISABLE,
-			   SPD5118_TS_DISABLE);
-	regcache_cache_bypass(regmap, false);
-
-	regcache_cache_only(regmap, true);
-	regcache_mark_dirty(regmap);
+	regcache_cache_only(data->regmap, true);
+	regcache_mark_dirty(data->regmap);
 
 	return 0;
 }
@@ -520,16 +507,13 @@ static int spd5118_suspend(struct device *dev)
 static int spd5118_resume(struct device *dev)
 {
 	struct spd5118_data *data = dev_get_drvdata(dev);
-	struct regmap *regmap = data->regmap;
-	int ret;
-
-	regcache_cache_only(regmap, false);
-	ret = regcache_sync(regmap);
-	if(ret == -ENXIO || ret == -EIO) {
-		dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
-		return 0;
-	}
-	return ret;
+	
+	/* 
+	 * Re-enable hardware access; sync is deferred until first read.
+	 */
+	regcache_cache_only(data->regmap, false);
+	
+	return 0;
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
-- 
2.47.3

Re: [PATCH 3/3] hwmon: spd5118: Avoid hardware access during suspend and resume

[Guenter Roeck]

On 1/10/26 09:19, Tinsae Tadesse wrote:
> The SPD5118 hub may be inaccessible during suspend/resume.
> Avoid updating/writing hardware in PM callbacks by switching the regmap
> to cache-only mode during suspend and deferring synchronization
> until first access.
> 
> This prevents the below I2C errors and async resume failures.
> 
> spd5118 ...: Failed to write b = 0: -6
> spd5118 ...: PM: dpm_run_callback(): spd5118_resume returns -6
> spd5118 ...: PM: failed to resume async: error -6
> 
> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
> ---
>   drivers/hwmon/spd5118.c | 36 ++++++++++--------------------------
>   1 file changed, 10 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index 63f798991363..a8afde7f47b2 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -494,25 +494,12 @@ static const struct regmap_config spd5118_regmap16_config = {
>   static int spd5118_suspend(struct device *dev)
>   {
>   	struct spd5118_data *data = dev_get_drvdata(dev);
> -	struct regmap *regmap = data->regmap;
> -	u32 regval;
> -	int err;
>   
>   	/*
> -	 * Make sure the configuration register in the regmap cache is current
> -	 * before bypassing it.
> +	 * The SPD5118 hub may be inaccessible; avoid hardware access.
>   	 */
> -	err = regmap_read(regmap, SPD5118_REG_TEMP_CONFIG, &regval);
> -	if (err < 0)
> -		return err;
> -

There was a reason for this code. If the register was never read,
the regmap cache will not contain the correct and expected values.

> -	regcache_cache_bypass(regmap, true);
> -	regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG, SPD5118_TS_DISABLE,
> -			   SPD5118_TS_DISABLE);

The point of this code was to disable the sensors during suspend.
Leaving the sensors enabled defeats the purpose of the suspend function.

Something is seriously broken in the system configuration if the i2c
controller suspends early, before the devices connected to it had a chance
to suspend. Maybe that should be fixed instead ?

> -	regcache_cache_bypass(regmap, false);
> -
> -	regcache_cache_only(regmap, true);
> -	regcache_mark_dirty(regmap);
> +	regcache_cache_only(data->regmap, true);
> +	regcache_mark_dirty(data->regmap);

It seems you dislike local variables. That is not a reason for removing them.

Guenter

>   
>   	return 0;
>   }
> @@ -520,16 +507,13 @@ static int spd5118_suspend(struct device *dev)
>   static int spd5118_resume(struct device *dev)
>   {
>   	struct spd5118_data *data = dev_get_drvdata(dev);
> -	struct regmap *regmap = data->regmap;
> -	int ret;
> -
> -	regcache_cache_only(regmap, false);
> -	ret = regcache_sync(regmap);
> -	if(ret == -ENXIO || ret == -EIO) {
> -		dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
> -		return 0;
> -	}
> -	return ret;
> +	
> +	/*
> +	 * Re-enable hardware access; sync is deferred until first read.
> +	 */
> +	regcache_cache_only(data->regmap, false);
> +	
> +	return 0;
>   }
>   
>   static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Armin Wolf]

Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:

> SPD5118 DDR5 temperature sensors may be temporarily unavailable
> during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
> register synchronization to be retried later.

Hi,

do you know if the error is caused by the SPD5118 device itself or by the underlying
i2c controller? Please also share the output of "acpidump" and the name of the i2c
controller used to communicate with the SPD5118.

> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
> ---
>   drivers/hwmon/spd5118.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index 5da44571b6a0..ec9f14f6e0df 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
>   {
>   	struct spd5118_data *data = dev_get_drvdata(dev);
>   	struct regmap *regmap = data->regmap;
> +	int ret;
>   
>   	regcache_cache_only(regmap, false);
> -	return regcache_sync(regmap);
> +	ret = regcache_sync(regmap);
> +	if(ret == -ENXIO || ret == -EIO) {
> +		dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
> +		return 0;
> +	}

The specification says that the SPD5118 might take up to 10ms to initialize its i2c interface
after power on. Can you test if simply waiting for 10ms before syncing the regcache solves this
issue?

Thanks,
Armin Wolf

> +	return ret;
>   }
>   
>   static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

[-- Attachment #1: Type: text/plain, Size: 3928 bytes --]

On Sun, Jan 11, 2026 at 1:27 AM Armin Wolf <W_Armin@gmx.de> wrote:
>
> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
>
> > SPD5118 DDR5 temperature sensors may be temporarily unavailable
> > during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
> > register synchronization to be retried later.
>
> Hi,
>
> do you know if the error is caused by the SPD5118 device itself or by the underlying
> i2c controller? Please also share the output of "acpidump" and the name of the i2c
> controller used to communicate with the SPD5118.
>
> > Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
> > ---
> >   drivers/hwmon/spd5118.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> > index 5da44571b6a0..ec9f14f6e0df 100644
> > --- a/drivers/hwmon/spd5118.c
> > +++ b/drivers/hwmon/spd5118.c
> > @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
> >   {
> >       struct spd5118_data *data = dev_get_drvdata(dev);
> >       struct regmap *regmap = data->regmap;
> > +     int ret;
> >
> >       regcache_cache_only(regmap, false);
> > -     return regcache_sync(regmap);
> > +     ret = regcache_sync(regmap);
> > +     if(ret == -ENXIO || ret == -EIO) {
> > +             dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
> > +             return 0;
> > +     }
>
> The specification says that the SPD5118 might take up to 10ms to initialize its i2c interface
> after power on. Can you test if simply waiting for 10ms before syncing the regcache solves this
> issue?
>
> Thanks,
> Armin Wolf
>
> > +     return ret;
> >   }
> >
> >   static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);

Hi Armin,

> Do you know if the error is caused by the SPD5118 device itself or by the underlying i2c controller?

The error appears to be caused by the underlying I2C controller and platform
power sequencing rather than by the SPD5118 device itself.

The failure manifests as a temporary -ENXIO occurring only during s2idle
resume. The SPD5118 temperature sensor works correctly before suspend and
after resume once the bus becomes available again. This indicates that the
driver’s resume callback may be invoked before the I2C controller or firmware
has fully re-enabled access to the SPD hub.

> Please also share the output of "acpidump" and the name of the i2c
controller used to communicate with the SPD5118.

I have attached the output of acpidump as requested.
The SPD5118 is connected via I2C bus 14 and accessed through the Intel
I801 SMBus controller (0000:00:1f.4), which is ACPI-managed.

> Can you test if simply waiting for 10ms before syncing the regcache solves this
issue?

I tested adding an explicit msleep(10) in spd5118_resume() before calling
regcache_sync(), for the I2C interface to become ready after power-on.
With this delay in place, the resume failures (-ENXIO during regcache sync)
no longer occur, and repeated suspend/resume cycles are completed successfully.

However, relying on a fixed delay in the resume path is not robust and would
not be suitable across platforms with different firmware and power sequencing.
It also still performs hardware I/O during PM resume.

Additional evidence comes from running sensors, where all the temperature
limit and alarm attributes fail with “Can’t read” and temp1 reports N/A,
after adding msleep(10). All hwmon attributes (temperature input,
limits, and alarms) fail uniformly, which points to a bus-level access
failure rather than an issue with specific SPD5118 registers.
This supports deferring regcache synchronization and avoiding I2C transactions
in the resume callback, since userspace may attempt to access hwmon
attributes before the
bus or device is ready.

[-- Attachment #2: acpidump.txt.gz --]
[-- Type: application/gzip, Size: 697411 bytes --]

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Armin Wolf]

Am 12.01.26 um 12:48 schrieb TINSAE TADESSE:

> On Sun, Jan 11, 2026 at 1:27 AM Armin Wolf <W_Armin@gmx.de> wrote:
>> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
>>
>>> SPD5118 DDR5 temperature sensors may be temporarily unavailable
>>> during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
>>> register synchronization to be retried later.
>> Hi,
>>
>> do you know if the error is caused by the SPD5118 device itself or by the underlying
>> i2c controller? Please also share the output of "acpidump" and the name of the i2c
>> controller used to communicate with the SPD5118.
>>
>>> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
>>> ---
>>>    drivers/hwmon/spd5118.c | 8 +++++++-
>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>> index 5da44571b6a0..ec9f14f6e0df 100644
>>> --- a/drivers/hwmon/spd5118.c
>>> +++ b/drivers/hwmon/spd5118.c
>>> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
>>>    {
>>>        struct spd5118_data *data = dev_get_drvdata(dev);
>>>        struct regmap *regmap = data->regmap;
>>> +     int ret;
>>>
>>>        regcache_cache_only(regmap, false);
>>> -     return regcache_sync(regmap);
>>> +     ret = regcache_sync(regmap);
>>> +     if(ret == -ENXIO || ret == -EIO) {
>>> +             dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
>>> +             return 0;
>>> +     }
>> The specification says that the SPD5118 might take up to 10ms to initialize its i2c interface
>> after power on. Can you test if simply waiting for 10ms before syncing the regcache solves this
>> issue?
>>
>> Thanks,
>> Armin Wolf
>>
>>> +     return ret;
>>>    }
>>>
>>>    static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
> Hi Armin,
>
>> Do you know if the error is caused by the SPD5118 device itself or by the underlying i2c controller?
> The error appears to be caused by the underlying I2C controller and platform
> power sequencing rather than by the SPD5118 device itself.
>
> The failure manifests as a temporary -ENXIO occurring only during s2idle
> resume. The SPD5118 temperature sensor works correctly before suspend and
> after resume once the bus becomes available again. This indicates that the
> driver’s resume callback may be invoked before the I2C controller or firmware
> has fully re-enabled access to the SPD hub.
>
>> Please also share the output of "acpidump" and the name of the i2c
> controller used to communicate with the SPD5118.
>
> I have attached the output of acpidump as requested.
> The SPD5118 is connected via I2C bus 14 and accessed through the Intel
> I801 SMBus controller (0000:00:1f.4), which is ACPI-managed.

Interesting, the ACPI code seems to do two things when the i2c controller suspends (aka is put into D3):

1. A unknown register 0x84 ("PMEC") is modified
2. The PCI BAR of the i2c controller is disabled

Since the PCI bar is not re-enabled during resume, i suspect that either the firmware
is buggy or that the firmware relies on the operating system to restore any BAR settings
when resuming.

I do not know how the PCI core handles suspend, so i CCed the associated maintainers.

>> Can you test if simply waiting for 10ms before syncing the regcache solves this
> issue?
>
> I tested adding an explicit msleep(10) in spd5118_resume() before calling
> regcache_sync(), for the I2C interface to become ready after power-on.
> With this delay in place, the resume failures (-ENXIO during regcache sync)
> no longer occur, and repeated suspend/resume cycles are completed successfully.
>
> However, relying on a fixed delay in the resume path is not robust and would
> not be suitable across platforms with different firmware and power sequencing.
> It also still performs hardware I/O during PM resume.

In this case the 10 ms delay is OK since the specification of the SPD5118 device explicitly
states that the device needs those 10ms to become operational after loosing power.

> Additional evidence comes from running sensors, where all the temperature
> limit and alarm attributes fail with “Can’t read” and temp1 reports N/A,
> after adding msleep(10). All hwmon attributes (temperature input,
> limits, and alarms) fail uniformly, which points to a bus-level access
> failure rather than an issue with specific SPD5118 registers.

Strange, what kind of error is reported then accessing those sysfs attributes? Can you still
access the nvmem part of the SPD5118 device?

Can you also check if accessing tempX_enable works? If yes then please try to set this
attribute to "1" if it is still set to "0".

Additionally, please use "i2cdump" or "i2cdetect" to check if other i2c devices on the same
bus are also affected by this.

> This supports deferring regcache synchronization and avoiding I2C transactions
> in the resume callback, since userspace may attempt to access hwmon
> attributes before the
> bus or device is ready.

As already stated by Guenter, the root cause might be the i2c controller itself. Having
this deferred regcache sync only acts as a workaround, but we strongly prefer having a
real solution.

Thanks,
Armin Wolf

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/12/26 09:41, Armin Wolf wrote:
> Am 12.01.26 um 12:48 schrieb TINSAE TADESSE:
> 
>> On Sun, Jan 11, 2026 at 1:27 AM Armin Wolf <W_Armin@gmx.de> wrote:
>>> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
>>>
>>>> SPD5118 DDR5 temperature sensors may be temporarily unavailable
>>>> during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
>>>> register synchronization to be retried later.
>>> Hi,
>>>
>>> do you know if the error is caused by the SPD5118 device itself or by the underlying
>>> i2c controller? Please also share the output of "acpidump" and the name of the i2c
>>> controller used to communicate with the SPD5118.
>>>
>>>> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
>>>> ---
>>>>    drivers/hwmon/spd5118.c | 8 +++++++-
>>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>>> index 5da44571b6a0..ec9f14f6e0df 100644
>>>> --- a/drivers/hwmon/spd5118.c
>>>> +++ b/drivers/hwmon/spd5118.c
>>>> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
>>>>    {
>>>>        struct spd5118_data *data = dev_get_drvdata(dev);
>>>>        struct regmap *regmap = data->regmap;
>>>> +     int ret;
>>>>
>>>>        regcache_cache_only(regmap, false);
>>>> -     return regcache_sync(regmap);
>>>> +     ret = regcache_sync(regmap);
>>>> +     if(ret == -ENXIO || ret == -EIO) {
>>>> +             dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
>>>> +             return 0;
>>>> +     }
>>> The specification says that the SPD5118 might take up to 10ms to initialize its i2c interface
>>> after power on. Can you test if simply waiting for 10ms before syncing the regcache solves this
>>> issue?
>>>
>>> Thanks,
>>> Armin Wolf
>>>
>>>> +     return ret;
>>>>    }
>>>>
>>>>    static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
>> Hi Armin,
>>
>>> Do you know if the error is caused by the SPD5118 device itself or by the underlying i2c controller?
>> The error appears to be caused by the underlying I2C controller and platform
>> power sequencing rather than by the SPD5118 device itself.
>>
>> The failure manifests as a temporary -ENXIO occurring only during s2idle
>> resume. The SPD5118 temperature sensor works correctly before suspend and
>> after resume once the bus becomes available again. This indicates that the
>> driver’s resume callback may be invoked before the I2C controller or firmware
>> has fully re-enabled access to the SPD hub.
>>
>>> Please also share the output of "acpidump" and the name of the i2c
>> controller used to communicate with the SPD5118.
>>
>> I have attached the output of acpidump as requested.
>> The SPD5118 is connected via I2C bus 14 and accessed through the Intel
>> I801 SMBus controller (0000:00:1f.4), which is ACPI-managed.
> 
> Interesting, the ACPI code seems to do two things when the i2c controller suspends (aka is put into D3):
> 
> 1. A unknown register 0x84 ("PMEC") is modified
> 2. The PCI BAR of the i2c controller is disabled
> 
> Since the PCI bar is not re-enabled during resume, i suspect that either the firmware
> is buggy or that the firmware relies on the operating system to restore any BAR settings
> when resuming.
> 
> I do not know how the PCI core handles suspend, so i CCed the associated maintainers.
> 
>>> Can you test if simply waiting for 10ms before syncing the regcache solves this
>> issue?
>>
>> I tested adding an explicit msleep(10) in spd5118_resume() before calling
>> regcache_sync(), for the I2C interface to become ready after power-on.
>> With this delay in place, the resume failures (-ENXIO during regcache sync)
>> no longer occur, and repeated suspend/resume cycles are completed successfully.
>>
>> However, relying on a fixed delay in the resume path is not robust and would
>> not be suitable across platforms with different firmware and power sequencing.
>> It also still performs hardware I/O during PM resume.
> 
> In this case the 10 ms delay is OK since the specification of the SPD5118 device explicitly
> states that the device needs those 10ms to become operational after loosing power.
> 

I would agree, but it seems to be quite unlikely that this is the actual problem
since that would mean that the SPD1118 is actually accessed less than 10ms after
it was powered on. That is unlikely even if the power rail to the SPD chip is separate
from the power rail to the DDR itself. On top of that, pretty much every chip has
similar restrictions. I really don't want to have to add similar code to every driver
because a power sequencer somewhere has a problem and doesn't add the required delay
after enabling power to a chip (or disables power to a chip before it is properly
suspended). We have to make _some_ assumptions in the kernel, and one of those
assumptions is that the chip is still accessible on suspend, and that it is accessible
on resume.

Worse - if the problem is the i2c controller, there is no guarantee that the 10ms
delay is sufficient. It may be sufficient in this case, but if the resume order changes
and the i2c controller is re-enabled even later than it is today, we'd end up in the
same situation. Either case, if the i2c controller is indeed the problem, trying to
work around that problem in the driver of a device attached to it is just wrong.

Guenter

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

[-- Attachment #1: Type: text/plain, Size: 3930 bytes --]

Hi Armin,

> Do you know if the error is caused by the SPD5118 device itself or by the underlying i2c controller?

The error appears to be caused by the underlying I2C controller and platform
power sequencing rather than by the SPD5118 device itself.

The failure manifests as a temporary -ENXIO occurring only during s2idle
resume. The SPD5118 temperature sensor works correctly before suspend and
after resume once the bus becomes available again. This indicates that the
driver’s resume callback may be invoked before the I2C controller or firmware
has fully re-enabled access to the SPD hub.

> Please also share the output of "acpidump" and the name of the i2c
controller used to communicate with the SPD5118.

I have attached the output of acpidump as requested.
The SPD5118 is connected via I2C bus 14 and accessed through the Intel
I801 SMBus controller (0000:00:1f.4), which is ACPI-managed.

> Can you test if simply waiting for 10ms before syncing the regcache solves this
issue?

I tested adding an explicit msleep(10) in spd5118_resume() before calling
regcache_sync(), for the I2C interface to become ready after power-on.
With this delay in place, the resume failures (-ENXIO during regcache sync)
no longer occur, and repeated suspend/resume cycles are completed successfully.

However, relying on a fixed delay in the resume path is not robust and would
not be suitable across platforms with different firmware and power sequencing.
It also still performs hardware I/O during PM resume.

Additional evidence comes from running sensors, where all the temperature
limit and alarm attributes fail with “Can’t read” and temp1 reports N/A,
after adding msleep(10). All hwmon attributes (temperature input,
limits, and alarms) fail uniformly, which points to a bus-level access
failure rather than an issue with specific SPD5118 registers.
This supports deferring regcache synchronization and avoiding I2C transactions
in the resume callback, since userspace may attempt to access hwmon
attributes before the
bus or device is ready.


On Sun, Jan 11, 2026 at 1:27 AM Armin Wolf <W_Armin@gmx.de> wrote:
>
> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
>
> > SPD5118 DDR5 temperature sensors may be temporarily unavailable
> > during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
> > register synchronization to be retried later.
>
> Hi,
>
> do you know if the error is caused by the SPD5118 device itself or by the underlying
> i2c controller? Please also share the output of "acpidump" and the name of the i2c
> controller used to communicate with the SPD5118.
>
> > Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
> > ---
> >   drivers/hwmon/spd5118.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> > index 5da44571b6a0..ec9f14f6e0df 100644
> > --- a/drivers/hwmon/spd5118.c
> > +++ b/drivers/hwmon/spd5118.c
> > @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
> >   {
> >       struct spd5118_data *data = dev_get_drvdata(dev);
> >       struct regmap *regmap = data->regmap;
> > +     int ret;
> >
> >       regcache_cache_only(regmap, false);
> > -     return regcache_sync(regmap);
> > +     ret = regcache_sync(regmap);
> > +     if(ret == -ENXIO || ret == -EIO) {
> > +             dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
> > +             return 0;
> > +     }
>
> The specification says that the SPD5118 might take up to 10ms to initialize its i2c interface
> after power on. Can you test if simply waiting for 10ms before syncing the regcache solves this
> issue?
>
> Thanks,
> Armin Wolf
>
> > +     return ret;
> >   }
> >
> >   static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);

[-- Attachment #2: acpidump.txt.gz --]
[-- Type: application/gzip, Size: 697411 bytes --]

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/10/26 14:27, Armin Wolf wrote:
> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
> 
>> SPD5118 DDR5 temperature sensors may be temporarily unavailable
>> during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
>> register synchronization to be retried later.
> 
> Hi,
> 
> do you know if the error is caused by the SPD5118 device itself or by the underlying
> i2c controller? Please also share the output of "acpidump" and the name of the i2c
> controller used to communicate with the SPD5118.
> 
>> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
>> ---
>>   drivers/hwmon/spd5118.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>> index 5da44571b6a0..ec9f14f6e0df 100644
>> --- a/drivers/hwmon/spd5118.c
>> +++ b/drivers/hwmon/spd5118.c
>> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
>>   {
>>       struct spd5118_data *data = dev_get_drvdata(dev);
>>       struct regmap *regmap = data->regmap;
>> +    int ret;
>>       regcache_cache_only(regmap, false);
>> -    return regcache_sync(regmap);
>> +    ret = regcache_sync(regmap);
>> +    if(ret == -ENXIO || ret == -EIO) {
>> +        dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
>> +        return 0;
>> +    }
> 
> The specification says that the SPD5118 might take up to 10ms to initialize its i2c interface
> after power on. Can you test if simply waiting for 10ms before syncing the regcache solves this
> issue?
> 

It seems to be highly unlikely that this code executes within 10ms of powering on the memory.

Guenter

> Thanks,
> Armin Wolf
> 
>> +    return ret;
>>   }
>>   static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
> 

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Armin Wolf]

Am 12.01.26 um 17:36 schrieb Guenter Roeck:

> On 1/10/26 14:27, Armin Wolf wrote:
>> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
>>
>>> SPD5118 DDR5 temperature sensors may be temporarily unavailable
>>> during s2idle resume. Ignore temporary -ENXIO and -EIO errors during 
>>> resume and allow
>>> register synchronization to be retried later.
>>
>> Hi,
>>
>> do you know if the error is caused by the SPD5118 device itself or by 
>> the underlying
>> i2c controller? Please also share the output of "acpidump" and the 
>> name of the i2c
>> controller used to communicate with the SPD5118.
>>
>>> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
>>> ---
>>>   drivers/hwmon/spd5118.c | 8 +++++++-
>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>> index 5da44571b6a0..ec9f14f6e0df 100644
>>> --- a/drivers/hwmon/spd5118.c
>>> +++ b/drivers/hwmon/spd5118.c
>>> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
>>>   {
>>>       struct spd5118_data *data = dev_get_drvdata(dev);
>>>       struct regmap *regmap = data->regmap;
>>> +    int ret;
>>>       regcache_cache_only(regmap, false);
>>> -    return regcache_sync(regmap);
>>> +    ret = regcache_sync(regmap);
>>> +    if(ret == -ENXIO || ret == -EIO) {
>>> +        dev_warn(dev, "SPD hub not responding on resume (%d), 
>>> deferring init\n", ret);
>>> +        return 0;
>>> +    }
>>
>> The specification says that the SPD5118 might take up to 10ms to 
>> initialize its i2c interface
>> after power on. Can you test if simply waiting for 10ms before 
>> syncing the regcache solves this
>> issue?
>>
>
> It seems to be highly unlikely that this code executes within 10ms of 
> powering on the memory.
>
> Guenter
>
AFAIK the 10ms are associated with the VDDIO supply, the VDDSPD main supply is different from that.
I just want to test if this device disables VDDIO during suspend-to-idle.

I have another theory: if the SPD5118 somehow looses power, we might still need to manually put the
device into 16-bit address mode using standard 8-bit i2c commands.

Thanks,
Armin Wolf

>> Thanks,
>> Armin Wolf
>>
>>> +    return ret;
>>>   }
>>>   static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, 
>>> spd5118_resume);
>>
>

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/12/26 09:46, Armin Wolf wrote:
> Am 12.01.26 um 17:36 schrieb Guenter Roeck:
> 
>> On 1/10/26 14:27, Armin Wolf wrote:
>>> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
>>>
>>>> SPD5118 DDR5 temperature sensors may be temporarily unavailable
>>>> during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
>>>> register synchronization to be retried later.
>>>
>>> Hi,
>>>
>>> do you know if the error is caused by the SPD5118 device itself or by the underlying
>>> i2c controller? Please also share the output of "acpidump" and the name of the i2c
>>> controller used to communicate with the SPD5118.
>>>
>>>> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
>>>> ---
>>>>   drivers/hwmon/spd5118.c | 8 +++++++-
>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>>> index 5da44571b6a0..ec9f14f6e0df 100644
>>>> --- a/drivers/hwmon/spd5118.c
>>>> +++ b/drivers/hwmon/spd5118.c
>>>> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
>>>>   {
>>>>       struct spd5118_data *data = dev_get_drvdata(dev);
>>>>       struct regmap *regmap = data->regmap;
>>>> +    int ret;
>>>>       regcache_cache_only(regmap, false);
>>>> -    return regcache_sync(regmap);
>>>> +    ret = regcache_sync(regmap);
>>>> +    if(ret == -ENXIO || ret == -EIO) {
>>>> +        dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
>>>> +        return 0;
>>>> +    }
>>>
>>> The specification says that the SPD5118 might take up to 10ms to initialize its i2c interface
>>> after power on. Can you test if simply waiting for 10ms before syncing the regcache solves this
>>> issue?
>>>
>>
>> It seems to be highly unlikely that this code executes within 10ms of powering on the memory.
>>
>> Guenter
>>
> AFAIK the 10ms are associated with the VDDIO supply, the VDDSPD main supply is different from that.
> I just want to test if this device disables VDDIO during suspend-to-idle.
> 
> I have another theory: if the SPD5118 somehow looses power, we might still need to manually put the
> device into 16-bit address mode using standard 8-bit i2c commands.
> 

Uh, no, we can not do that. I tried. Changing the access mode causes bad hiccups at least
with some boards. They interpret that as a memory configuration change, and the next warm
reboot will end up in the BIOS. Then, after the RAM configuration is updated, a cold reboot
will again detect a configuration change and BIOS will be entered again.

That does make me wonder how the problem shows up in the first place, since the BIOS
usually does access the SPD5118 during resume, at least on my systems with DDR5. Granted,
those are with AMD CPUs, but I would assume that Intel BIOS versions are not different.

Guenter

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Armin Wolf]

Am 12.01.26 um 19:17 schrieb Guenter Roeck:

> On 1/12/26 09:46, Armin Wolf wrote:
>> Am 12.01.26 um 17:36 schrieb Guenter Roeck:
>>
>>> On 1/10/26 14:27, Armin Wolf wrote:
>>>> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
>>>>
>>>>> SPD5118 DDR5 temperature sensors may be temporarily unavailable
>>>>> during s2idle resume. Ignore temporary -ENXIO and -EIO errors 
>>>>> during resume and allow
>>>>> register synchronization to be retried later.
>>>>
>>>> Hi,
>>>>
>>>> do you know if the error is caused by the SPD5118 device itself or 
>>>> by the underlying
>>>> i2c controller? Please also share the output of "acpidump" and the 
>>>> name of the i2c
>>>> controller used to communicate with the SPD5118.
>>>>
>>>>> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
>>>>> ---
>>>>>   drivers/hwmon/spd5118.c | 8 +++++++-
>>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>>>> index 5da44571b6a0..ec9f14f6e0df 100644
>>>>> --- a/drivers/hwmon/spd5118.c
>>>>> +++ b/drivers/hwmon/spd5118.c
>>>>> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
>>>>>   {
>>>>>       struct spd5118_data *data = dev_get_drvdata(dev);
>>>>>       struct regmap *regmap = data->regmap;
>>>>> +    int ret;
>>>>>       regcache_cache_only(regmap, false);
>>>>> -    return regcache_sync(regmap);
>>>>> +    ret = regcache_sync(regmap);
>>>>> +    if(ret == -ENXIO || ret == -EIO) {
>>>>> +        dev_warn(dev, "SPD hub not responding on resume (%d), 
>>>>> deferring init\n", ret);
>>>>> +        return 0;
>>>>> +    }
>>>>
>>>> The specification says that the SPD5118 might take up to 10ms to 
>>>> initialize its i2c interface
>>>> after power on. Can you test if simply waiting for 10ms before 
>>>> syncing the regcache solves this
>>>> issue?
>>>>
>>>
>>> It seems to be highly unlikely that this code executes within 10ms 
>>> of powering on the memory.
>>>
>>> Guenter
>>>
>> AFAIK the 10ms are associated with the VDDIO supply, the VDDSPD main 
>> supply is different from that.
>> I just want to test if this device disables VDDIO during 
>> suspend-to-idle.
>>
>> I have another theory: if the SPD5118 somehow looses power, we might 
>> still need to manually put the
>> device into 16-bit address mode using standard 8-bit i2c commands.
>>
>
> Uh, no, we can not do that. I tried. Changing the access mode causes 
> bad hiccups at least
> with some boards. They interpret that as a memory configuration 
> change, and the next warm
> reboot will end up in the BIOS. Then, after the RAM configuration is 
> updated, a cold reboot
> will again detect a configuration change and BIOS will be entered again.
>
> That does make me wonder how the problem shows up in the first place, 
> since the BIOS
> usually does access the SPD5118 during resume, at least on my systems 
> with DDR5. Granted,
> those are with AMD CPUs, but I would assume that Intel BIOS versions 
> are not different.
>
> Guenter
>
During suspend-to-idle the RAM stays active, so the firmware does not really need to access the SPD device.
I meant that if the SPD device is configured during boot to operate in 16-bit mode and looses power during
suspend-to-idle, the firmware might not reconfigure the SPD to continue operate in 16-bit mode after resume.

Thanks,
Armin Wolf

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/12/26 10:22, Armin Wolf wrote:
> Am 12.01.26 um 19:17 schrieb Guenter Roeck:
> 
>> On 1/12/26 09:46, Armin Wolf wrote:
>>> Am 12.01.26 um 17:36 schrieb Guenter Roeck:
>>>
>>>> On 1/10/26 14:27, Armin Wolf wrote:
>>>>> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
>>>>>
>>>>>> SPD5118 DDR5 temperature sensors may be temporarily unavailable
>>>>>> during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
>>>>>> register synchronization to be retried later.
>>>>>
>>>>> Hi,
>>>>>
>>>>> do you know if the error is caused by the SPD5118 device itself or by the underlying
>>>>> i2c controller? Please also share the output of "acpidump" and the name of the i2c
>>>>> controller used to communicate with the SPD5118.
>>>>>
>>>>>> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
>>>>>> ---
>>>>>>   drivers/hwmon/spd5118.c | 8 +++++++-
>>>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>>>>> index 5da44571b6a0..ec9f14f6e0df 100644
>>>>>> --- a/drivers/hwmon/spd5118.c
>>>>>> +++ b/drivers/hwmon/spd5118.c
>>>>>> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
>>>>>>   {
>>>>>>       struct spd5118_data *data = dev_get_drvdata(dev);
>>>>>>       struct regmap *regmap = data->regmap;
>>>>>> +    int ret;
>>>>>>       regcache_cache_only(regmap, false);
>>>>>> -    return regcache_sync(regmap);
>>>>>> +    ret = regcache_sync(regmap);
>>>>>> +    if(ret == -ENXIO || ret == -EIO) {
>>>>>> +        dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
>>>>>> +        return 0;
>>>>>> +    }
>>>>>
>>>>> The specification says that the SPD5118 might take up to 10ms to initialize its i2c interface
>>>>> after power on. Can you test if simply waiting for 10ms before syncing the regcache solves this
>>>>> issue?
>>>>>
>>>>
>>>> It seems to be highly unlikely that this code executes within 10ms of powering on the memory.
>>>>
>>>> Guenter
>>>>
>>> AFAIK the 10ms are associated with the VDDIO supply, the VDDSPD main supply is different from that.
>>> I just want to test if this device disables VDDIO during suspend-to-idle.
>>>
>>> I have another theory: if the SPD5118 somehow looses power, we might still need to manually put the
>>> device into 16-bit address mode using standard 8-bit i2c commands.
>>>
>>
>> Uh, no, we can not do that. I tried. Changing the access mode causes bad hiccups at least
>> with some boards. They interpret that as a memory configuration change, and the next warm
>> reboot will end up in the BIOS. Then, after the RAM configuration is updated, a cold reboot
>> will again detect a configuration change and BIOS will be entered again.
>>
>> That does make me wonder how the problem shows up in the first place, since the BIOS
>> usually does access the SPD5118 during resume, at least on my systems with DDR5. Granted,
>> those are with AMD CPUs, but I would assume that Intel BIOS versions are not different.
>>
>> Guenter
>>
> During suspend-to-idle the RAM stays active, so the firmware does not really need to access the SPD device.
> I meant that if the SPD device is configured during boot to operate in 16-bit mode and looses power during
> suspend-to-idle, the firmware might not reconfigure the SPD to continue operate in 16-bit mode after resume.
> 

This would be a severe BIOS/Firmware problem. I'd really want to see evidence that
restoring SPD5118_REG_I2C_LEGACY_MODE is necessary on resume before touching that
register. Even then I'd want to see evidence that touching it doesn't cause
problems on a variety of boards before actually doing it. After all, we would not
know if the register was reconfigured by the firmware for some reason, or if the
chip lost power and the firmware didn't handle it.

Having said that, even if there is evidence that the chip can lose power and end up
in the wrong mode on resume, I'd rather check for that condition, issue a WARN_ONCE(),
and disable the driver instead of touching its configuration.

Sorry, but from my experience SPD5118 access in general is quite fragile, and not
touching its configuration was the only way I could find to make it work reliably.

Guenter

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

On Mon, Jan 12, 2026 at 10:11 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 1/12/26 10:22, Armin Wolf wrote:
> > Am 12.01.26 um 19:17 schrieb Guenter Roeck:
> >
> >> On 1/12/26 09:46, Armin Wolf wrote:
> >>> Am 12.01.26 um 17:36 schrieb Guenter Roeck:
> >>>
> >>>> On 1/10/26 14:27, Armin Wolf wrote:
> >>>>> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
> >>>>>
> >>>>>> SPD5118 DDR5 temperature sensors may be temporarily unavailable
> >>>>>> during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
> >>>>>> register synchronization to be retried later.
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> do you know if the error is caused by the SPD5118 device itself or by the underlying
> >>>>> i2c controller? Please also share the output of "acpidump" and the name of the i2c
> >>>>> controller used to communicate with the SPD5118.
> >>>>>
> >>>>>> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
> >>>>>> ---
> >>>>>>   drivers/hwmon/spd5118.c | 8 +++++++-
> >>>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> >>>>>> index 5da44571b6a0..ec9f14f6e0df 100644
> >>>>>> --- a/drivers/hwmon/spd5118.c
> >>>>>> +++ b/drivers/hwmon/spd5118.c
> >>>>>> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
> >>>>>>   {
> >>>>>>       struct spd5118_data *data = dev_get_drvdata(dev);
> >>>>>>       struct regmap *regmap = data->regmap;
> >>>>>> +    int ret;
> >>>>>>       regcache_cache_only(regmap, false);
> >>>>>> -    return regcache_sync(regmap);
> >>>>>> +    ret = regcache_sync(regmap);
> >>>>>> +    if(ret == -ENXIO || ret == -EIO) {
> >>>>>> +        dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
> >>>>>> +        return 0;
> >>>>>> +    }
> >>>>>
> >>>>> The specification says that the SPD5118 might take up to 10ms to initialize its i2c interface
> >>>>> after power on. Can you test if simply waiting for 10ms before syncing the regcache solves this
> >>>>> issue?
> >>>>>
> >>>>
> >>>> It seems to be highly unlikely that this code executes within 10ms of powering on the memory.
> >>>>
> >>>> Guenter
> >>>>
> >>> AFAIK the 10ms are associated with the VDDIO supply, the VDDSPD main supply is different from that.
> >>> I just want to test if this device disables VDDIO during suspend-to-idle.
> >>>
> >>> I have another theory: if the SPD5118 somehow looses power, we might still need to manually put the
> >>> device into 16-bit address mode using standard 8-bit i2c commands.
> >>>
> >>
> >> Uh, no, we can not do that. I tried. Changing the access mode causes bad hiccups at least
> >> with some boards. They interpret that as a memory configuration change, and the next warm
> >> reboot will end up in the BIOS. Then, after the RAM configuration is updated, a cold reboot
> >> will again detect a configuration change and BIOS will be entered again.
> >>
> >> That does make me wonder how the problem shows up in the first place, since the BIOS
> >> usually does access the SPD5118 during resume, at least on my systems with DDR5. Granted,
> >> those are with AMD CPUs, but I would assume that Intel BIOS versions are not different.
> >>
> >> Guenter
> >>
> > During suspend-to-idle the RAM stays active, so the firmware does not really need to access the SPD device.
> > I meant that if the SPD device is configured during boot to operate in 16-bit mode and looses power during
> > suspend-to-idle, the firmware might not reconfigure the SPD to continue operate in 16-bit mode after resume.
> >
>
> This would be a severe BIOS/Firmware problem. I'd really want to see evidence that
> restoring SPD5118_REG_I2C_LEGACY_MODE is necessary on resume before touching that
> register. Even then I'd want to see evidence that touching it doesn't cause
> problems on a variety of boards before actually doing it. After all, we would not
> know if the register was reconfigured by the firmware for some reason, or if the
> chip lost power and the firmware didn't handle it.
>
> Having said that, even if there is evidence that the chip can lose power and end up
> in the wrong mode on resume, I'd rather check for that condition, issue a WARN_ONCE(),
> and disable the driver instead of touching its configuration.
>
> Sorry, but from my experience SPD5118 access in general is quite fragile, and not
> touching its configuration was the only way I could find to make it work reliably.
>
> Guenter
>

I experimented by configuring the driver to observe
SPD5118_REG_I2C_LEGACY_MODE and SPD5118_REG_TEMP_CONFIG registers
before suspend and immediately after resume, prior to any
driver-initiated writes.
Across multiple suspend-to-idle cycles, register values remained unchanged,
indicating that the SPD5118 does not lose configuration and
does not require reprogramming of legacy/16-bit mode on resume.
Resume failures correlate with temporary I2C unavailability rather
than loss of device state.

Since the device configuration remains intact across resume and the
failure correlates with I2C availability,
restoring SPD5118_REG_I2C_LEGACY_MODE is neither required nor justified.
For this reason, I agree that restoring this register would be unsafe
without broader platform testing.

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

On Mon, Jan 12, 2026 at 9:22 PM Armin Wolf <W_Armin@gmx.de> wrote:
>
> Am 12.01.26 um 19:17 schrieb Guenter Roeck:
>
> > On 1/12/26 09:46, Armin Wolf wrote:
> >> Am 12.01.26 um 17:36 schrieb Guenter Roeck:
> >>
> >>> On 1/10/26 14:27, Armin Wolf wrote:
> >>>> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
> >>>>
> >>>>> SPD5118 DDR5 temperature sensors may be temporarily unavailable
> >>>>> during s2idle resume. Ignore temporary -ENXIO and -EIO errors
> >>>>> during resume and allow
> >>>>> register synchronization to be retried later.
> >>>>
> >>>> Hi,
> >>>>
> >>>> do you know if the error is caused by the SPD5118 device itself or
> >>>> by the underlying
> >>>> i2c controller? Please also share the output of "acpidump" and the
> >>>> name of the i2c
> >>>> controller used to communicate with the SPD5118.
> >>>>
> >>>>> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
> >>>>> ---
> >>>>>   drivers/hwmon/spd5118.c | 8 +++++++-
> >>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> >>>>> index 5da44571b6a0..ec9f14f6e0df 100644
> >>>>> --- a/drivers/hwmon/spd5118.c
> >>>>> +++ b/drivers/hwmon/spd5118.c
> >>>>> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
> >>>>>   {
> >>>>>       struct spd5118_data *data = dev_get_drvdata(dev);
> >>>>>       struct regmap *regmap = data->regmap;
> >>>>> +    int ret;
> >>>>>       regcache_cache_only(regmap, false);
> >>>>> -    return regcache_sync(regmap);
> >>>>> +    ret = regcache_sync(regmap);
> >>>>> +    if(ret == -ENXIO || ret == -EIO) {
> >>>>> +        dev_warn(dev, "SPD hub not responding on resume (%d),
> >>>>> deferring init\n", ret);
> >>>>> +        return 0;
> >>>>> +    }
> >>>>
> >>>> The specification says that the SPD5118 might take up to 10ms to
> >>>> initialize its i2c interface
> >>>> after power on. Can you test if simply waiting for 10ms before
> >>>> syncing the regcache solves this
> >>>> issue?
> >>>>
> >>>
> >>> It seems to be highly unlikely that this code executes within 10ms
> >>> of powering on the memory.
> >>>
> >>> Guenter
> >>>
> >> AFAIK the 10ms are associated with the VDDIO supply, the VDDSPD main
> >> supply is different from that.
> >> I just want to test if this device disables VDDIO during
> >> suspend-to-idle.
> >>
> >> I have another theory: if the SPD5118 somehow looses power, we might
> >> still need to manually put the
> >> device into 16-bit address mode using standard 8-bit i2c commands.
> >>
> >
> > Uh, no, we can not do that. I tried. Changing the access mode causes
> > bad hiccups at least
> > with some boards. They interpret that as a memory configuration
> > change, and the next warm
> > reboot will end up in the BIOS. Then, after the RAM configuration is
> > updated, a cold reboot
> > will again detect a configuration change and BIOS will be entered again.
> >
> > That does make me wonder how the problem shows up in the first place,
> > since the BIOS
> > usually does access the SPD5118 during resume, at least on my systems
> > with DDR5. Granted,
> > those are with AMD CPUs, but I would assume that Intel BIOS versions
> > are not different.
> >
> > Guenter
> >
> During suspend-to-idle the RAM stays active, so the firmware does not really need to access the SPD device.
> I meant that if the SPD device is configured during boot to operate in 16-bit mode and looses power during
> suspend-to-idle, the firmware might not reconfigure the SPD to continue operate in 16-bit mode after resume.
>
> Thanks,
> Armin Wolf
>

Hi Armin,

I tested whether firmware reinitializes the SPD5118 by comparing key
registers across cold boot and s2idle resume.
Register values remained unchanged across resume cycles, suggesting
firmware does not reconfigure the device during resume.
To avoid introducing platform-specific regressions, no attempt was
made to restore SPD5118_REG_I2C_LEGACY_MODE.

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Armin Wolf]

Am 13.01.26 um 20:16 schrieb TINSAE TADESSE:

> On Mon, Jan 12, 2026 at 9:22 PM Armin Wolf <W_Armin@gmx.de> wrote:
>> Am 12.01.26 um 19:17 schrieb Guenter Roeck:
>>
>>> On 1/12/26 09:46, Armin Wolf wrote:
>>>> Am 12.01.26 um 17:36 schrieb Guenter Roeck:
>>>>
>>>>> On 1/10/26 14:27, Armin Wolf wrote:
>>>>>> Am 10.01.26 um 18:19 schrieb Tinsae Tadesse:
>>>>>>
>>>>>>> SPD5118 DDR5 temperature sensors may be temporarily unavailable
>>>>>>> during s2idle resume. Ignore temporary -ENXIO and -EIO errors
>>>>>>> during resume and allow
>>>>>>> register synchronization to be retried later.
>>>>>> Hi,
>>>>>>
>>>>>> do you know if the error is caused by the SPD5118 device itself or
>>>>>> by the underlying
>>>>>> i2c controller? Please also share the output of "acpidump" and the
>>>>>> name of the i2c
>>>>>> controller used to communicate with the SPD5118.
>>>>>>
>>>>>>> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
>>>>>>> ---
>>>>>>>    drivers/hwmon/spd5118.c | 8 +++++++-
>>>>>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>>>>>> index 5da44571b6a0..ec9f14f6e0df 100644
>>>>>>> --- a/drivers/hwmon/spd5118.c
>>>>>>> +++ b/drivers/hwmon/spd5118.c
>>>>>>> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
>>>>>>>    {
>>>>>>>        struct spd5118_data *data = dev_get_drvdata(dev);
>>>>>>>        struct regmap *regmap = data->regmap;
>>>>>>> +    int ret;
>>>>>>>        regcache_cache_only(regmap, false);
>>>>>>> -    return regcache_sync(regmap);
>>>>>>> +    ret = regcache_sync(regmap);
>>>>>>> +    if(ret == -ENXIO || ret == -EIO) {
>>>>>>> +        dev_warn(dev, "SPD hub not responding on resume (%d),
>>>>>>> deferring init\n", ret);
>>>>>>> +        return 0;
>>>>>>> +    }
>>>>>> The specification says that the SPD5118 might take up to 10ms to
>>>>>> initialize its i2c interface
>>>>>> after power on. Can you test if simply waiting for 10ms before
>>>>>> syncing the regcache solves this
>>>>>> issue?
>>>>>>
>>>>> It seems to be highly unlikely that this code executes within 10ms
>>>>> of powering on the memory.
>>>>>
>>>>> Guenter
>>>>>
>>>> AFAIK the 10ms are associated with the VDDIO supply, the VDDSPD main
>>>> supply is different from that.
>>>> I just want to test if this device disables VDDIO during
>>>> suspend-to-idle.
>>>>
>>>> I have another theory: if the SPD5118 somehow looses power, we might
>>>> still need to manually put the
>>>> device into 16-bit address mode using standard 8-bit i2c commands.
>>>>
>>> Uh, no, we can not do that. I tried. Changing the access mode causes
>>> bad hiccups at least
>>> with some boards. They interpret that as a memory configuration
>>> change, and the next warm
>>> reboot will end up in the BIOS. Then, after the RAM configuration is
>>> updated, a cold reboot
>>> will again detect a configuration change and BIOS will be entered again.
>>>
>>> That does make me wonder how the problem shows up in the first place,
>>> since the BIOS
>>> usually does access the SPD5118 during resume, at least on my systems
>>> with DDR5. Granted,
>>> those are with AMD CPUs, but I would assume that Intel BIOS versions
>>> are not different.
>>>
>>> Guenter
>>>
>> During suspend-to-idle the RAM stays active, so the firmware does not really need to access the SPD device.
>> I meant that if the SPD device is configured during boot to operate in 16-bit mode and looses power during
>> suspend-to-idle, the firmware might not reconfigure the SPD to continue operate in 16-bit mode after resume.
>>
>> Thanks,
>> Armin Wolf
>>
> Hi Armin,
>
> I tested whether firmware reinitializes the SPD5118 by comparing key
> registers across cold boot and s2idle resume.
> Register values remained unchanged across resume cycles, suggesting
> firmware does not reconfigure the device during resume.
> To avoid introducing platform-specific regressions, no attempt was
> made to restore SPD5118_REG_I2C_LEGACY_MODE.
>
Alright, thanks for checking. In this case the error indeed seems to come from the i2c controller

Thanks,
Armin Wolf

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/10/26 09:19, Tinsae Tadesse wrote:
> SPD5118 DDR5 temperature sensors may be temporarily unavailable
> during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
> register synchronization to be retried later.
> 
> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
> ---
>   drivers/hwmon/spd5118.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index 5da44571b6a0..ec9f14f6e0df 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
>   {
>   	struct spd5118_data *data = dev_get_drvdata(dev);
>   	struct regmap *regmap = data->regmap;
> +	int ret;
>   
>   	regcache_cache_only(regmap, false);
> -	return regcache_sync(regmap);
> +	ret = regcache_sync(regmap);
> +	if(ret == -ENXIO || ret == -EIO) {
> +		dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
> +		return 0;
> +	}
> +	return ret;
>   }
>   
>   static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);


Based on the subsequent exchange, this is not the appropriate solution.
The problem is that the i2c controller suspends too early. It should suspend
later instead. Alternatively, this driver should suspend early, but it does
not look like respective PM operations are available.

I would suggest to try replacing DEFINE_SIMPLE_DEV_PM_OPS() in drivers/i2c/busses/i2c-i801.c
with SET_LATE_SYSTEM_SLEEP_PM_OPS() or similar.

Guenter

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

On Mon, Jan 12, 2026 at 7:30 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 1/10/26 09:19, Tinsae Tadesse wrote:
> > SPD5118 DDR5 temperature sensors may be temporarily unavailable
> > during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
> > register synchronization to be retried later.
> >
> > Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
> > ---
> >   drivers/hwmon/spd5118.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> > index 5da44571b6a0..ec9f14f6e0df 100644
> > --- a/drivers/hwmon/spd5118.c
> > +++ b/drivers/hwmon/spd5118.c
> > @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
> >   {
> >       struct spd5118_data *data = dev_get_drvdata(dev);
> >       struct regmap *regmap = data->regmap;
> > +     int ret;
> >
> >       regcache_cache_only(regmap, false);
> > -     return regcache_sync(regmap);
> > +     ret = regcache_sync(regmap);
> > +     if(ret == -ENXIO || ret == -EIO) {
> > +             dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
> > +             return 0;
> > +     }
> > +     return ret;
> >   }
> >
> >   static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
>
>
> Based on the subsequent exchange, this is not the appropriate solution.
> The problem is that the i2c controller suspends too early. It should suspend
> later instead. Alternatively, this driver should suspend early, but it does
> not look like respective PM operations are available.
>
> I would suggest to try replacing DEFINE_SIMPLE_DEV_PM_OPS() in drivers/i2c/busses/i2c-i801.c
> with SET_LATE_SYSTEM_SLEEP_PM_OPS() or similar.
>
> Guenter
>

Hi Guenter,

I tested changing the i801 SMBus controller to use
SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
change, spd5118 resume failures (-ENXIO)
still persist, suggesting PM ordering alone is insufficient and other
firmware interactions are involved.

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/13/26 11:15, TINSAE TADESSE wrote:
> On Mon, Jan 12, 2026 at 7:30 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 1/10/26 09:19, Tinsae Tadesse wrote:
>>> SPD5118 DDR5 temperature sensors may be temporarily unavailable
>>> during s2idle resume. Ignore temporary -ENXIO and -EIO errors during resume and allow
>>> register synchronization to be retried later.
>>>
>>> Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
>>> ---
>>>    drivers/hwmon/spd5118.c | 8 +++++++-
>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>> index 5da44571b6a0..ec9f14f6e0df 100644
>>> --- a/drivers/hwmon/spd5118.c
>>> +++ b/drivers/hwmon/spd5118.c
>>> @@ -512,9 +512,15 @@ static int spd5118_resume(struct device *dev)
>>>    {
>>>        struct spd5118_data *data = dev_get_drvdata(dev);
>>>        struct regmap *regmap = data->regmap;
>>> +     int ret;
>>>
>>>        regcache_cache_only(regmap, false);
>>> -     return regcache_sync(regmap);
>>> +     ret = regcache_sync(regmap);
>>> +     if(ret == -ENXIO || ret == -EIO) {
>>> +             dev_warn(dev, "SPD hub not responding on resume (%d), deferring init\n", ret);
>>> +             return 0;
>>> +     }
>>> +     return ret;
>>>    }
>>>
>>>    static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
>>
>>
>> Based on the subsequent exchange, this is not the appropriate solution.
>> The problem is that the i2c controller suspends too early. It should suspend
>> later instead. Alternatively, this driver should suspend early, but it does
>> not look like respective PM operations are available.
>>
>> I would suggest to try replacing DEFINE_SIMPLE_DEV_PM_OPS() in drivers/i2c/busses/i2c-i801.c
>> with SET_LATE_SYSTEM_SLEEP_PM_OPS() or similar.
>>
>> Guenter
>>
> 
> Hi Guenter,
> 
> I tested changing the i801 SMBus controller to use
> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
> change, spd5118 resume failures (-ENXIO)
> still persist, suggesting PM ordering alone is insufficient and other
> firmware interactions are involved.

How about the problem in the suspend function ? Is that also still seen ?

Also, the subject talks about -EIO. Is that still seen ?

Either case, can you enable debug logs for the i801 driver ?
It should generate log entries when it reports errors.

Thanks,
Guenter

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/14/26 05:07, TINSAE TADESSE wrote:
...
>>> Hi Guenter,
>>>
>>> I tested changing the i801 SMBus controller to use
>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
>>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
>>> change, spd5118 resume failures (-ENXIO)
>>> still persist, suggesting PM ordering alone is insufficient and other
>>> firmware interactions are involved.
>>
>> How about the problem in the suspend function ? Is that also still seen ?
>>
>> Also, the subject talks about -EIO. Is that still seen ?
>>
>> Either case, can you enable debug logs for the i801 driver ?
>> It should generate log entries when it reports errors.
>>
>> Thanks,
>> Guenter
>>
> 
> Hi Guenter,
> 
> Thank you for the questions. To clarify:
> 
Please do not drop mailing lists from replies.

> 1) I have not observed any failures in the suspend path. The suspend
> callback completes successfully, and
> I have not seen I2C errors or warnings during suspend at any point.

Sorry, I seem to be missing something.

In that case, what is the point of patch 3/3 of your series which
removes hardware accesses from the suspend function ?

> 2) I have also not observed -EIO in my testing. The error consistently
> reported on resume and subsequent hwmon access is -ENXIO.
> Earlier references to -EIO were based on assumptions rather than
> observed logs, and I should have been clearer about that.
> 

Thanks for the clarification.

Guenter

> I am enabling debug logging for the i801 driver to collect more
> concrete evidence of controller state during resume.

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

On Wed, Jan 14, 2026 at 5:23 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 1/14/26 05:07, TINSAE TADESSE wrote:
> ...
> >>> Hi Guenter,
> >>>
> >>> I tested changing the i801 SMBus controller to use
> >>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
> >>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
> >>> change, spd5118 resume failures (-ENXIO)
> >>> still persist, suggesting PM ordering alone is insufficient and other
> >>> firmware interactions are involved.
> >>
> >> How about the problem in the suspend function ? Is that also still seen ?
> >>
> >> Also, the subject talks about -EIO. Is that still seen ?
> >>
> >> Either case, can you enable debug logs for the i801 driver ?
> >> It should generate log entries when it reports errors.
> >>
> >> Thanks,
> >> Guenter
> >>
> >
> > Hi Guenter,
> >
> > Thank you for the questions. To clarify:
> >
> Please do not drop mailing lists from replies.
>
> > 1) I have not observed any failures in the suspend path. The suspend
> > callback completes successfully, and
> > I have not seen I2C errors or warnings during suspend at any point.
>
> Sorry, I seem to be missing something.
>
> In that case, what is the point of patch 3/3 of your series which
> removes hardware accesses from the suspend function ?
>
> > 2) I have also not observed -EIO in my testing. The error consistently
> > reported on resume and subsequent hwmon access is -ENXIO.
> > Earlier references to -EIO were based on assumptions rather than
> > observed logs, and I should have been clearer about that.
> >
>
> Thanks for the clarification.
>
> Guenter
>
> > I am enabling debug logging for the i801 driver to collect more
> > concrete evidence of controller state during resume.
>

Hi Guenter,

> Sorry, I seem to be missing something.
>
> In that case, what is the point of patch 3/3 of your series which
> removes hardware accesses from the suspend function ?

You are right to question this, and I agree that it needs clarification.

Patch 3/3 was originally proposed under the assumption that the resume failures
were caused by spd5118 performing I2C transactions while the
controller was not yet available,
and that removing hardware accesses from the suspend path might
mitigate the issue.
At that point, I assumed the problem was limited to the resume callback.

After enabling detailed i801 debug logging and testing with
SET_LATE_SYSTEM_SLEEP_PM_OPS() in the i801 driver,
it became clear that this assumption was incorrect. The controller
itself reports "i801_smbus: No response"
both during suspend and immediately after resume, and spd5118 merely
propagates the resulting -ENXIO.
This indicates that the issue is not caused by spd5118 suspend/resume
behavior, but by the unavailability of the
SMBus controller due to platform or firmware interactions during
s2idle transitions.

Given this, I agree that patch 3/3 does not address the root cause and
does not provide a justified improvement.
I am therefore fine with dropping it.

Thank you for pointing this out.

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/15/26 05:50, TINSAE TADESSE wrote:
> On Wed, Jan 14, 2026 at 5:23 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 1/14/26 05:07, TINSAE TADESSE wrote:
>> ...
>>>>> Hi Guenter,
>>>>>
>>>>> I tested changing the i801 SMBus controller to use
>>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
>>>>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
>>>>> change, spd5118 resume failures (-ENXIO)
>>>>> still persist, suggesting PM ordering alone is insufficient and other
>>>>> firmware interactions are involved.
>>>>
>>>> How about the problem in the suspend function ? Is that also still seen ?
>>>>
>>>> Also, the subject talks about -EIO. Is that still seen ?
>>>>
>>>> Either case, can you enable debug logs for the i801 driver ?
>>>> It should generate log entries when it reports errors.
>>>>
>>>> Thanks,
>>>> Guenter
>>>>
>>>
>>> Hi Guenter,
>>>
>>> Thank you for the questions. To clarify:
>>>
>> Please do not drop mailing lists from replies.
>>
>>> 1) I have not observed any failures in the suspend path. The suspend
>>> callback completes successfully, and
>>> I have not seen I2C errors or warnings during suspend at any point.
>>
>> Sorry, I seem to be missing something.
>>
>> In that case, what is the point of patch 3/3 of your series which
>> removes hardware accesses from the suspend function ?
>>
>>> 2) I have also not observed -EIO in my testing. The error consistently
>>> reported on resume and subsequent hwmon access is -ENXIO.
>>> Earlier references to -EIO were based on assumptions rather than
>>> observed logs, and I should have been clearer about that.
>>>
>>
>> Thanks for the clarification.
>>
>> Guenter
>>
>>> I am enabling debug logging for the i801 driver to collect more
>>> concrete evidence of controller state during resume.
>>
> 
> Hi Guenter,
> 
>> Sorry, I seem to be missing something.
>>
>> In that case, what is the point of patch 3/3 of your series which
>> removes hardware accesses from the suspend function ?
> 
> You are right to question this, and I agree that it needs clarification.
> 
> Patch 3/3 was originally proposed under the assumption that the resume failures
> were caused by spd5118 performing I2C transactions while the
> controller was not yet available,
> and that removing hardware accesses from the suspend path might
> mitigate the issue.
> At that point, I assumed the problem was limited to the resume callback.
> 
> After enabling detailed i801 debug logging and testing with
> SET_LATE_SYSTEM_SLEEP_PM_OPS() in the i801 driver,
> it became clear that this assumption was incorrect. The controller
> itself reports "i801_smbus: No response"
> both during suspend and immediately after resume, and spd5118 merely
> propagates the resulting -ENXIO.

Outch, that really hurts, because it means that something is seriously
broken in both the suspend and resume path. The device _must_ be accessible
in the suspend path. Otherwise there is no guarantee that the device is
accessible for normal (pre-suspend) operation. After all, someone could
run a script reading sysfs attributes in a tight loop continuously,
or the thermal subsystem could try to access the chip. That would suddenly
start to fail if something in the device access path starts to be suspended
while the underlying hardware is still believed to be operational.

I could imagine some hack/quirk for the resume path, such as delaying resume
for some period of time for affected hardware, but I have no idea what to
do on the suspend side. We can not just drop device writes during suspend
because some broken hardware/firmware does not let us actually access
(and thus suspend) the hardware anymore by the time the suspend function
is called.

Guenter

> This indicates that the issue is not caused by spd5118 suspend/resume
> behavior, but by the unavailability of the
> SMBus controller due to platform or firmware interactions during
> s2idle transitions.
> 
> Given this, I agree that patch 3/3 does not address the root cause and
> does not provide a justified improvement.
> I am therefore fine with dropping it.
> 
> Thank you for pointing this out.
> 

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

On Fri, Jan 16, 2026 at 9:24 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 1/15/26 05:50, TINSAE TADESSE wrote:
> > On Wed, Jan 14, 2026 at 5:23 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> On 1/14/26 05:07, TINSAE TADESSE wrote:
> >> ...
> >>>>> Hi Guenter,
> >>>>>
> >>>>> I tested changing the i801 SMBus controller to use
> >>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
> >>>>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
> >>>>> change, spd5118 resume failures (-ENXIO)
> >>>>> still persist, suggesting PM ordering alone is insufficient and other
> >>>>> firmware interactions are involved.
> >>>>
> >>>> How about the problem in the suspend function ? Is that also still seen ?
> >>>>
> >>>> Also, the subject talks about -EIO. Is that still seen ?
> >>>>
> >>>> Either case, can you enable debug logs for the i801 driver ?
> >>>> It should generate log entries when it reports errors.
> >>>>
> >>>> Thanks,
> >>>> Guenter
> >>>>
> >>>
> >>> Hi Guenter,
> >>>
> >>> Thank you for the questions. To clarify:
> >>>
> >> Please do not drop mailing lists from replies.
> >>
> >>> 1) I have not observed any failures in the suspend path. The suspend
> >>> callback completes successfully, and
> >>> I have not seen I2C errors or warnings during suspend at any point.
> >>
> >> Sorry, I seem to be missing something.
> >>
> >> In that case, what is the point of patch 3/3 of your series which
> >> removes hardware accesses from the suspend function ?
> >>
> >>> 2) I have also not observed -EIO in my testing. The error consistently
> >>> reported on resume and subsequent hwmon access is -ENXIO.
> >>> Earlier references to -EIO were based on assumptions rather than
> >>> observed logs, and I should have been clearer about that.
> >>>
> >>
> >> Thanks for the clarification.
> >>
> >> Guenter
> >>
> >>> I am enabling debug logging for the i801 driver to collect more
> >>> concrete evidence of controller state during resume.
> >>
> >
> > Hi Guenter,
> >
> >> Sorry, I seem to be missing something.
> >>
> >> In that case, what is the point of patch 3/3 of your series which
> >> removes hardware accesses from the suspend function ?
> >
> > You are right to question this, and I agree that it needs clarification.
> >
> > Patch 3/3 was originally proposed under the assumption that the resume failures
> > were caused by spd5118 performing I2C transactions while the
> > controller was not yet available,
> > and that removing hardware accesses from the suspend path might
> > mitigate the issue.
> > At that point, I assumed the problem was limited to the resume callback.
> >
> > After enabling detailed i801 debug logging and testing with
> > SET_LATE_SYSTEM_SLEEP_PM_OPS() in the i801 driver,
> > it became clear that this assumption was incorrect. The controller
> > itself reports "i801_smbus: No response"
> > both during suspend and immediately after resume, and spd5118 merely
> > propagates the resulting -ENXIO.
>
> Outch, that really hurts, because it means that something is seriously
> broken in both the suspend and resume path. The device _must_ be accessible
> in the suspend path. Otherwise there is no guarantee that the device is
> accessible for normal (pre-suspend) operation. After all, someone could
> run a script reading sysfs attributes in a tight loop continuously,
> or the thermal subsystem could try to access the chip. That would suddenly
> start to fail if something in the device access path starts to be suspended
> while the underlying hardware is still believed to be operational.
>
> I could imagine some hack/quirk for the resume path, such as delaying resume
> for some period of time for affected hardware, but I have no idea what to
> do on the suspend side. We can not just drop device writes during suspend
> because some broken hardware/firmware does not let us actually access
> (and thus suspend) the hardware anymore by the time the suspend function
> is called.
>
> Guenter
>
> > This indicates that the issue is not caused by spd5118 suspend/resume
> > behavior, but by the unavailability of the
> > SMBus controller due to platform or firmware interactions during
> > s2idle transitions.
> >
> > Given this, I agree that patch 3/3 does not address the root cause and
> > does not provide a justified improvement.
> > I am therefore fine with dropping it.
> >
> > Thank you for pointing this out.
> >
>

Hi Guenter,

Thanks for the continued review and for questioning the earlier
direction — that helped narrow this down properly.

After enabling full i801 debug logging (included below in this email)
and inspecting both drivers, it became clear that the resume
failures are not caused by spd5118 accessing the hardware too
early, nor by PM ordering issues. Instead, the SMBus controller
explicitly reports “SPD Write Disable is set”, and any
block write transactions to the SPD device consistently fail with
DEV_ERR. spd5118 merely propagates the resulting -ENXIO.

With that in mind, I have dropped the earlier patch that attempted
to remove hardware access from the suspend path
unconditionally.
That patch does not address the root cause and is no longer
part of the series.

I am instead proposing a minimal 2-patch series:

1/2 records whether the platform enforces SPD write disable at probe
time (no behavior change).
2/2 avoids regcache writeback during suspend/resume when the device
operates in read-only mode, while still allowing read access to
temperature inputs.

This avoids issuing SMBus transactions that are architecturally
blocked on these systems, and does not rely on
delays or PM ordering assumptions, and leaves behavior unchanged on
platforms where SPD writes are permitted.

If this direction looks acceptable, I’m happy to re-spin and post the
series formally.

Thanks again for the guidance.


[   13.530830] i2c-core: driver [spd5118] registered
...
[   29.555298] i801_smbus 0000:00:1f.4: SPD Write Disable is set
...
[   29.582447] i2c i2c-14: Creating spd5118 at 0x52
[   29.590744] spd5118 14-0052: probe
...
[   29.618983] spd5118 14-0052: DDR5 temperature sensor: vendor
0x00:0xb3 revision 2.2
[   29.619662] i2c i2c-14: client [spd5118] registered with bus id 14-0052
...
[ 1057.504362] PM: suspend entry (s2idle)
[ 1057.944405] spd5118 14-0052: Entering suspend path...
[ 1057.945387] i801_smbus 0000:00:1f.4: i801 access: command=1a,
size=8, addr=0x52, read_write=1
[ 1057.946251] i801_smbus 0000:00:1f.4: i801 access: command=b,
size=8, addr=0x52, read_write=1
[ 1057.946866] i801_smbus 0000:00:1f.4: i801 access: command=1a,
size=8, addr=0x52, read_write=1
[ 1057.948324] i801_smbus 0000:00:1f.4: i801 access: command=b,
size=8, addr=0x52, read_write=1
[ 1057.949817] i801_smbus 0000:00:1f.4: i801 access: command=1a,
size=8, addr=0x52, read_write=0
[ 1057.949904] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4,
SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
[ 1057.949952] i801_smbus 0000:00:1f.4: No response
[ 1057.950215] i801_smbus 0000:00:1f.4: Entering suspend path...
[ 1059.338647] ACPI: EC: interrupt blocked
[ 1060.756385] ACPI: EC: interrupt unblocked
[ 1060.926423] i801_smbus 0000:00:1f.4: Entering resume path...
[ 1060.926623] spd5118 14-0052: Entering resume path...
[ 1060.927930] i801_smbus 0000:00:1f.4: i801 access: command=b,
size=8, addr=0x52, read_write=0
[ 1060.927969] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4,
SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
[ 1060.927995] i801_smbus 0000:00:1f.4: No response
[ 1060.928477] i801_smbus 0000:00:1f.4: i801 access: command=b,
size=8, addr=0x52, read_write=0
[ 1060.928517] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4,
SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
[ 1060.928543] i801_smbus 0000:00:1f.4: No response
[ 1060.928582] spd5118 14-0052: Failed to write b = 0: -6
[ 1060.928707] spd5118 14-0052: PM: dpm_run_callback(): spd5118_resume
returns -6
[ 1060.928981] spd5118 14-0052: PM: failed to resume async: error -6
[ 1062.414560] PM: suspend exit

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Armin Wolf]

Am 24.01.26 um 15:45 schrieb TINSAE TADESSE:

> On Fri, Jan 16, 2026 at 9:24 AM Guenter Roeck <linux@roeck-us.net> wrote:
>> On 1/15/26 05:50, TINSAE TADESSE wrote:
>>> On Wed, Jan 14, 2026 at 5:23 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>>> On 1/14/26 05:07, TINSAE TADESSE wrote:
>>>> ...
>>>>>>> Hi Guenter,
>>>>>>>
>>>>>>> I tested changing the i801 SMBus controller to use
>>>>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
>>>>>>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
>>>>>>> change, spd5118 resume failures (-ENXIO)
>>>>>>> still persist, suggesting PM ordering alone is insufficient and other
>>>>>>> firmware interactions are involved.
>>>>>> How about the problem in the suspend function ? Is that also still seen ?
>>>>>>
>>>>>> Also, the subject talks about -EIO. Is that still seen ?
>>>>>>
>>>>>> Either case, can you enable debug logs for the i801 driver ?
>>>>>> It should generate log entries when it reports errors.
>>>>>>
>>>>>> Thanks,
>>>>>> Guenter
>>>>>>
>>>>> Hi Guenter,
>>>>>
>>>>> Thank you for the questions. To clarify:
>>>>>
>>>> Please do not drop mailing lists from replies.
>>>>
>>>>> 1) I have not observed any failures in the suspend path. The suspend
>>>>> callback completes successfully, and
>>>>> I have not seen I2C errors or warnings during suspend at any point.
>>>> Sorry, I seem to be missing something.
>>>>
>>>> In that case, what is the point of patch 3/3 of your series which
>>>> removes hardware accesses from the suspend function ?
>>>>
>>>>> 2) I have also not observed -EIO in my testing. The error consistently
>>>>> reported on resume and subsequent hwmon access is -ENXIO.
>>>>> Earlier references to -EIO were based on assumptions rather than
>>>>> observed logs, and I should have been clearer about that.
>>>>>
>>>> Thanks for the clarification.
>>>>
>>>> Guenter
>>>>
>>>>> I am enabling debug logging for the i801 driver to collect more
>>>>> concrete evidence of controller state during resume.
>>> Hi Guenter,
>>>
>>>> Sorry, I seem to be missing something.
>>>>
>>>> In that case, what is the point of patch 3/3 of your series which
>>>> removes hardware accesses from the suspend function ?
>>> You are right to question this, and I agree that it needs clarification.
>>>
>>> Patch 3/3 was originally proposed under the assumption that the resume failures
>>> were caused by spd5118 performing I2C transactions while the
>>> controller was not yet available,
>>> and that removing hardware accesses from the suspend path might
>>> mitigate the issue.
>>> At that point, I assumed the problem was limited to the resume callback.
>>>
>>> After enabling detailed i801 debug logging and testing with
>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() in the i801 driver,
>>> it became clear that this assumption was incorrect. The controller
>>> itself reports "i801_smbus: No response"
>>> both during suspend and immediately after resume, and spd5118 merely
>>> propagates the resulting -ENXIO.
>> Outch, that really hurts, because it means that something is seriously
>> broken in both the suspend and resume path. The device _must_ be accessible
>> in the suspend path. Otherwise there is no guarantee that the device is
>> accessible for normal (pre-suspend) operation. After all, someone could
>> run a script reading sysfs attributes in a tight loop continuously,
>> or the thermal subsystem could try to access the chip. That would suddenly
>> start to fail if something in the device access path starts to be suspended
>> while the underlying hardware is still believed to be operational.
>>
>> I could imagine some hack/quirk for the resume path, such as delaying resume
>> for some period of time for affected hardware, but I have no idea what to
>> do on the suspend side. We can not just drop device writes during suspend
>> because some broken hardware/firmware does not let us actually access
>> (and thus suspend) the hardware anymore by the time the suspend function
>> is called.
>>
>> Guenter
>>
>>> This indicates that the issue is not caused by spd5118 suspend/resume
>>> behavior, but by the unavailability of the
>>> SMBus controller due to platform or firmware interactions during
>>> s2idle transitions.
>>>
>>> Given this, I agree that patch 3/3 does not address the root cause and
>>> does not provide a justified improvement.
>>> I am therefore fine with dropping it.
>>>
>>> Thank you for pointing this out.
>>>
> Hi Guenter,
>
> Thanks for the continued review and for questioning the earlier
> direction — that helped narrow this down properly.
>
> After enabling full i801 debug logging (included below in this email)
> and inspecting both drivers, it became clear that the resume
> failures are not caused by spd5118 accessing the hardware too
> early, nor by PM ordering issues. Instead, the SMBus controller
> explicitly reports “SPD Write Disable is set”, and any
> block write transactions to the SPD device consistently fail with
> DEV_ERR. spd5118 merely propagates the resulting -ENXIO.

Oh no, this likely happens even when merely reading values, as the spd5118
uses a page register to switch between different register pages. In order
to access temperature data (page 0), you might already have to issue a
write access to the page register. The only reason why it works for you
is that the spd5118 likely already has page 0 selected by the system firmware
during boot.

> With that in mind, I have dropped the earlier patch that attempted
> to remove hardware access from the suspend path
> unconditionally.
> That patch does not address the root cause and is no longer
> part of the series.
>
> I am instead proposing a minimal 2-patch series:
>
> 1/2 records whether the platform enforces SPD write disable at probe
> time (no behavior change).
> 2/2 avoids regcache writeback during suspend/resume when the device
> operates in read-only mode, while still allowing read access to
> temperature inputs.
>
> This avoids issuing SMBus transactions that are architecturally
> blocked on these systems, and does not rely on
> delays or PM ordering assumptions, and leaves behavior unchanged on
> platforms where SPD writes are permitted.
>
> If this direction looks acceptable, I’m happy to re-spin and post the
> series formally.
>
> Thanks again for the guidance.

I do not know if this is a reliable solution, as the system firmware might
select a different register page during resume. This will then prevent the
driver from functioning.

I would love to see the spd5118 driver working on such systems with reduced
functionality, but i will leave it to Guenter to decide if this approach is
maintainable.

Besides that: did the spd5118 driver load automatically on your device?

Thanks,
Armin Wolf

>
>
> [   13.530830] i2c-core: driver [spd5118] registered
> ...
> [   29.555298] i801_smbus 0000:00:1f.4: SPD Write Disable is set
> ...
> [   29.582447] i2c i2c-14: Creating spd5118 at 0x52
> [   29.590744] spd5118 14-0052: probe
> ...
> [   29.618983] spd5118 14-0052: DDR5 temperature sensor: vendor
> 0x00:0xb3 revision 2.2
> [   29.619662] i2c i2c-14: client [spd5118] registered with bus id 14-0052
> ...
> [ 1057.504362] PM: suspend entry (s2idle)
> [ 1057.944405] spd5118 14-0052: Entering suspend path...
> [ 1057.945387] i801_smbus 0000:00:1f.4: i801 access: command=1a,
> size=8, addr=0x52, read_write=1
> [ 1057.946251] i801_smbus 0000:00:1f.4: i801 access: command=b,
> size=8, addr=0x52, read_write=1
> [ 1057.946866] i801_smbus 0000:00:1f.4: i801 access: command=1a,
> size=8, addr=0x52, read_write=1
> [ 1057.948324] i801_smbus 0000:00:1f.4: i801 access: command=b,
> size=8, addr=0x52, read_write=1
> [ 1057.949817] i801_smbus 0000:00:1f.4: i801 access: command=1a,
> size=8, addr=0x52, read_write=0
> [ 1057.949904] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4,
> SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
> [ 1057.949952] i801_smbus 0000:00:1f.4: No response
> [ 1057.950215] i801_smbus 0000:00:1f.4: Entering suspend path...
> [ 1059.338647] ACPI: EC: interrupt blocked
> [ 1060.756385] ACPI: EC: interrupt unblocked
> [ 1060.926423] i801_smbus 0000:00:1f.4: Entering resume path...
> [ 1060.926623] spd5118 14-0052: Entering resume path...
> [ 1060.927930] i801_smbus 0000:00:1f.4: i801 access: command=b,
> size=8, addr=0x52, read_write=0
> [ 1060.927969] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4,
> SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
> [ 1060.927995] i801_smbus 0000:00:1f.4: No response
> [ 1060.928477] i801_smbus 0000:00:1f.4: i801 access: command=b,
> size=8, addr=0x52, read_write=0
> [ 1060.928517] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4,
> SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
> [ 1060.928543] i801_smbus 0000:00:1f.4: No response
> [ 1060.928582] spd5118 14-0052: Failed to write b = 0: -6
> [ 1060.928707] spd5118 14-0052: PM: dpm_run_callback(): spd5118_resume
> returns -6
> [ 1060.928981] spd5118 14-0052: PM: failed to resume async: error -6
> [ 1062.414560] PM: suspend exit
>

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/24/26 11:11, Armin Wolf wrote:
> Am 24.01.26 um 15:45 schrieb TINSAE TADESSE:
> 
>> On Fri, Jan 16, 2026 at 9:24 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>> On 1/15/26 05:50, TINSAE TADESSE wrote:
>>>> On Wed, Jan 14, 2026 at 5:23 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>>>> On 1/14/26 05:07, TINSAE TADESSE wrote:
>>>>> ...
>>>>>>>> Hi Guenter,
>>>>>>>>
>>>>>>>> I tested changing the i801 SMBus controller to use
>>>>>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
>>>>>>>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
>>>>>>>> change, spd5118 resume failures (-ENXIO)
>>>>>>>> still persist, suggesting PM ordering alone is insufficient and other
>>>>>>>> firmware interactions are involved.
>>>>>>> How about the problem in the suspend function ? Is that also still seen ?
>>>>>>>
>>>>>>> Also, the subject talks about -EIO. Is that still seen ?
>>>>>>>
>>>>>>> Either case, can you enable debug logs for the i801 driver ?
>>>>>>> It should generate log entries when it reports errors.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Guenter
>>>>>>>
>>>>>> Hi Guenter,
>>>>>>
>>>>>> Thank you for the questions. To clarify:
>>>>>>
>>>>> Please do not drop mailing lists from replies.
>>>>>
>>>>>> 1) I have not observed any failures in the suspend path. The suspend
>>>>>> callback completes successfully, and
>>>>>> I have not seen I2C errors or warnings during suspend at any point.
>>>>> Sorry, I seem to be missing something.
>>>>>
>>>>> In that case, what is the point of patch 3/3 of your series which
>>>>> removes hardware accesses from the suspend function ?
>>>>>
>>>>>> 2) I have also not observed -EIO in my testing. The error consistently
>>>>>> reported on resume and subsequent hwmon access is -ENXIO.
>>>>>> Earlier references to -EIO were based on assumptions rather than
>>>>>> observed logs, and I should have been clearer about that.
>>>>>>
>>>>> Thanks for the clarification.
>>>>>
>>>>> Guenter
>>>>>
>>>>>> I am enabling debug logging for the i801 driver to collect more
>>>>>> concrete evidence of controller state during resume.
>>>> Hi Guenter,
>>>>
>>>>> Sorry, I seem to be missing something.
>>>>>
>>>>> In that case, what is the point of patch 3/3 of your series which
>>>>> removes hardware accesses from the suspend function ?
>>>> You are right to question this, and I agree that it needs clarification.
>>>>
>>>> Patch 3/3 was originally proposed under the assumption that the resume failures
>>>> were caused by spd5118 performing I2C transactions while the
>>>> controller was not yet available,
>>>> and that removing hardware accesses from the suspend path might
>>>> mitigate the issue.
>>>> At that point, I assumed the problem was limited to the resume callback.
>>>>
>>>> After enabling detailed i801 debug logging and testing with
>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() in the i801 driver,
>>>> it became clear that this assumption was incorrect. The controller
>>>> itself reports "i801_smbus: No response"
>>>> both during suspend and immediately after resume, and spd5118 merely
>>>> propagates the resulting -ENXIO.
>>> Outch, that really hurts, because it means that something is seriously
>>> broken in both the suspend and resume path. The device _must_ be accessible
>>> in the suspend path. Otherwise there is no guarantee that the device is
>>> accessible for normal (pre-suspend) operation. After all, someone could
>>> run a script reading sysfs attributes in a tight loop continuously,
>>> or the thermal subsystem could try to access the chip. That would suddenly
>>> start to fail if something in the device access path starts to be suspended
>>> while the underlying hardware is still believed to be operational.
>>>
>>> I could imagine some hack/quirk for the resume path, such as delaying resume
>>> for some period of time for affected hardware, but I have no idea what to
>>> do on the suspend side. We can not just drop device writes during suspend
>>> because some broken hardware/firmware does not let us actually access
>>> (and thus suspend) the hardware anymore by the time the suspend function
>>> is called.
>>>
>>> Guenter
>>>
>>>> This indicates that the issue is not caused by spd5118 suspend/resume
>>>> behavior, but by the unavailability of the
>>>> SMBus controller due to platform or firmware interactions during
>>>> s2idle transitions.
>>>>
>>>> Given this, I agree that patch 3/3 does not address the root cause and
>>>> does not provide a justified improvement.
>>>> I am therefore fine with dropping it.
>>>>
>>>> Thank you for pointing this out.
>>>>
>> Hi Guenter,
>>
>> Thanks for the continued review and for questioning the earlier
>> direction — that helped narrow this down properly.
>>
>> After enabling full i801 debug logging (included below in this email)
>> and inspecting both drivers, it became clear that the resume
>> failures are not caused by spd5118 accessing the hardware too
>> early, nor by PM ordering issues. Instead, the SMBus controller
>> explicitly reports “SPD Write Disable is set”, and any
>> block write transactions to the SPD device consistently fail with
>> DEV_ERR. spd5118 merely propagates the resulting -ENXIO.
> 
> Oh no, this likely happens even when merely reading values, as the spd5118
> uses a page register to switch between different register pages. In order
> to access temperature data (page 0), you might already have to issue a
> write access to the page register. The only reason why it works for you
> is that the spd5118 likely already has page 0 selected by the system firmware
> during boot.
> 

Exactly. There is no guarantee that page 0 is selected.

>> With that in mind, I have dropped the earlier patch that attempted
>> to remove hardware access from the suspend path
>> unconditionally.
>> That patch does not address the root cause and is no longer
>> part of the series.
>>
>> I am instead proposing a minimal 2-patch series:
>>
>> 1/2 records whether the platform enforces SPD write disable at probe
>> time (no behavior change).
>> 2/2 avoids regcache writeback during suspend/resume when the device
>> operates in read-only mode, while still allowing read access to
>> temperature inputs.
>>
>> This avoids issuing SMBus transactions that are architecturally
>> blocked on these systems, and does not rely on
>> delays or PM ordering assumptions, and leaves behavior unchanged on
>> platforms where SPD writes are permitted.
>>
>> If this direction looks acceptable, I’m happy to re-spin and post the
>> series formally.
>>
>> Thanks again for the guidance.
> 
> I do not know if this is a reliable solution, as the system firmware might
> select a different register page during resume. This will then prevent the
> driver from functioning.
> 

No, it is not reliable. The driver is simply not usable in this scenario.
This isn't just the temperature sensor code - the eeprom code is affected
as well.

> I would love to see the spd5118 driver working on such systems with reduced
> functionality, but i will leave it to Guenter to decide if this approach is
> maintainable.
> 
> Besides that: did the spd5118 driver load automatically on your device?
> 

I thought that was disabled. The i801 driver is supposed to detect if write
protect is enabled and, if so, it is supposed to not instantiate the spd5118
driver for DDR3. Support for this was added with commit 4d6d35d3417d ("i2c:
smbus: introduce Write Disable-aware SPD instantiating functions"). Apparently
the code to do this never made it into the i801 driver.

The i801 driver needs to be fixed to inform the spd initialization code
that the spd5118 address range is write protected. The patch to do this was
"i2c: i801: Do not instantiate spd5118 under SPD Write Disable". I have no idea
why that patch didn't make it upstream.

Guenter

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Armin Wolf]

Am 25.01.26 um 23:36 schrieb Guenter Roeck:

> On 1/24/26 11:11, Armin Wolf wrote:
>> Am 24.01.26 um 15:45 schrieb TINSAE TADESSE:
>>
>>> On Fri, Jan 16, 2026 at 9:24 AM Guenter Roeck <linux@roeck-us.net> 
>>> wrote:
>>>> On 1/15/26 05:50, TINSAE TADESSE wrote:
>>>>> On Wed, Jan 14, 2026 at 5:23 PM Guenter Roeck <linux@roeck-us.net> 
>>>>> wrote:
>>>>>> On 1/14/26 05:07, TINSAE TADESSE wrote:
>>>>>> ...
>>>>>>>>> Hi Guenter,
>>>>>>>>>
>>>>>>>>> I tested changing the i801 SMBus controller to use
>>>>>>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
>>>>>>>>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
>>>>>>>>> change, spd5118 resume failures (-ENXIO)
>>>>>>>>> still persist, suggesting PM ordering alone is insufficient 
>>>>>>>>> and other
>>>>>>>>> firmware interactions are involved.
>>>>>>>> How about the problem in the suspend function ? Is that also 
>>>>>>>> still seen ?
>>>>>>>>
>>>>>>>> Also, the subject talks about -EIO. Is that still seen ?
>>>>>>>>
>>>>>>>> Either case, can you enable debug logs for the i801 driver ?
>>>>>>>> It should generate log entries when it reports errors.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Guenter
>>>>>>>>
>>>>>>> Hi Guenter,
>>>>>>>
>>>>>>> Thank you for the questions. To clarify:
>>>>>>>
>>>>>> Please do not drop mailing lists from replies.
>>>>>>
>>>>>>> 1) I have not observed any failures in the suspend path. The 
>>>>>>> suspend
>>>>>>> callback completes successfully, and
>>>>>>> I have not seen I2C errors or warnings during suspend at any point.
>>>>>> Sorry, I seem to be missing something.
>>>>>>
>>>>>> In that case, what is the point of patch 3/3 of your series which
>>>>>> removes hardware accesses from the suspend function ?
>>>>>>
>>>>>>> 2) I have also not observed -EIO in my testing. The error 
>>>>>>> consistently
>>>>>>> reported on resume and subsequent hwmon access is -ENXIO.
>>>>>>> Earlier references to -EIO were based on assumptions rather than
>>>>>>> observed logs, and I should have been clearer about that.
>>>>>>>
>>>>>> Thanks for the clarification.
>>>>>>
>>>>>> Guenter
>>>>>>
>>>>>>> I am enabling debug logging for the i801 driver to collect more
>>>>>>> concrete evidence of controller state during resume.
>>>>> Hi Guenter,
>>>>>
>>>>>> Sorry, I seem to be missing something.
>>>>>>
>>>>>> In that case, what is the point of patch 3/3 of your series which
>>>>>> removes hardware accesses from the suspend function ?
>>>>> You are right to question this, and I agree that it needs 
>>>>> clarification.
>>>>>
>>>>> Patch 3/3 was originally proposed under the assumption that the 
>>>>> resume failures
>>>>> were caused by spd5118 performing I2C transactions while the
>>>>> controller was not yet available,
>>>>> and that removing hardware accesses from the suspend path might
>>>>> mitigate the issue.
>>>>> At that point, I assumed the problem was limited to the resume 
>>>>> callback.
>>>>>
>>>>> After enabling detailed i801 debug logging and testing with
>>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() in the i801 driver,
>>>>> it became clear that this assumption was incorrect. The controller
>>>>> itself reports "i801_smbus: No response"
>>>>> both during suspend and immediately after resume, and spd5118 merely
>>>>> propagates the resulting -ENXIO.
>>>> Outch, that really hurts, because it means that something is seriously
>>>> broken in both the suspend and resume path. The device _must_ be 
>>>> accessible
>>>> in the suspend path. Otherwise there is no guarantee that the 
>>>> device is
>>>> accessible for normal (pre-suspend) operation. After all, someone 
>>>> could
>>>> run a script reading sysfs attributes in a tight loop continuously,
>>>> or the thermal subsystem could try to access the chip. That would 
>>>> suddenly
>>>> start to fail if something in the device access path starts to be 
>>>> suspended
>>>> while the underlying hardware is still believed to be operational.
>>>>
>>>> I could imagine some hack/quirk for the resume path, such as 
>>>> delaying resume
>>>> for some period of time for affected hardware, but I have no idea 
>>>> what to
>>>> do on the suspend side. We can not just drop device writes during 
>>>> suspend
>>>> because some broken hardware/firmware does not let us actually access
>>>> (and thus suspend) the hardware anymore by the time the suspend 
>>>> function
>>>> is called.
>>>>
>>>> Guenter
>>>>
>>>>> This indicates that the issue is not caused by spd5118 suspend/resume
>>>>> behavior, but by the unavailability of the
>>>>> SMBus controller due to platform or firmware interactions during
>>>>> s2idle transitions.
>>>>>
>>>>> Given this, I agree that patch 3/3 does not address the root cause 
>>>>> and
>>>>> does not provide a justified improvement.
>>>>> I am therefore fine with dropping it.
>>>>>
>>>>> Thank you for pointing this out.
>>>>>
>>> Hi Guenter,
>>>
>>> Thanks for the continued review and for questioning the earlier
>>> direction — that helped narrow this down properly.
>>>
>>> After enabling full i801 debug logging (included below in this email)
>>> and inspecting both drivers, it became clear that the resume
>>> failures are not caused by spd5118 accessing the hardware too
>>> early, nor by PM ordering issues. Instead, the SMBus controller
>>> explicitly reports “SPD Write Disable is set”, and any
>>> block write transactions to the SPD device consistently fail with
>>> DEV_ERR. spd5118 merely propagates the resulting -ENXIO.
>>
>> Oh no, this likely happens even when merely reading values, as the 
>> spd5118
>> uses a page register to switch between different register pages. In 
>> order
>> to access temperature data (page 0), you might already have to issue a
>> write access to the page register. The only reason why it works for you
>> is that the spd5118 likely already has page 0 selected by the system 
>> firmware
>> during boot.
>>
>
> Exactly. There is no guarantee that page 0 is selected.
>
>>> With that in mind, I have dropped the earlier patch that attempted
>>> to remove hardware access from the suspend path
>>> unconditionally.
>>> That patch does not address the root cause and is no longer
>>> part of the series.
>>>
>>> I am instead proposing a minimal 2-patch series:
>>>
>>> 1/2 records whether the platform enforces SPD write disable at probe
>>> time (no behavior change).
>>> 2/2 avoids regcache writeback during suspend/resume when the device
>>> operates in read-only mode, while still allowing read access to
>>> temperature inputs.
>>>
>>> This avoids issuing SMBus transactions that are architecturally
>>> blocked on these systems, and does not rely on
>>> delays or PM ordering assumptions, and leaves behavior unchanged on
>>> platforms where SPD writes are permitted.
>>>
>>> If this direction looks acceptable, I’m happy to re-spin and post the
>>> series formally.
>>>
>>> Thanks again for the guidance.
>>
>> I do not know if this is a reliable solution, as the system firmware 
>> might
>> select a different register page during resume. This will then 
>> prevent the
>> driver from functioning.
>>
>
> No, it is not reliable. The driver is simply not usable in this scenario.
> This isn't just the temperature sensor code - the eeprom code is affected
> as well.
>
Ok.

>> I would love to see the spd5118 driver working on such systems with 
>> reduced
>> functionality, but i will leave it to Guenter to decide if this 
>> approach is
>> maintainable.
>>
>> Besides that: did the spd5118 driver load automatically on your device?
>>
>
> I thought that was disabled. The i801 driver is supposed to detect if 
> write
> protect is enabled and, if so, it is supposed to not instantiate the 
> spd5118
> driver for DDR3. Support for this was added with commit 4d6d35d3417d 
> ("i2c:
> smbus: introduce Write Disable-aware SPD instantiating functions"). 
> Apparently
> the code to do this never made it into the i801 driver.
>
> The i801 driver needs to be fixed to inform the spd initialization code
> that the spd5118 address range is write protected. The patch to do 
> this was
> "i2c: i801: Do not instantiate spd5118 under SPD Write Disable". I 
> have no idea
> why that patch didn't make it upstream.
>
> Guenter
>
Good question, do you want to send the message to the i2c maintainers about this
or should i do it?

Thanks,
Armin Wolf

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/26/26 01:40, Armin Wolf wrote:
...
>>> Besides that: did the spd5118 driver load automatically on your device?
>>>
>>
>> I thought that was disabled. The i801 driver is supposed to detect if write
>> protect is enabled and, if so, it is supposed to not instantiate the spd5118
>> driver for DDR3. Support for this was added with commit 4d6d35d3417d ("i2c:
>> smbus: introduce Write Disable-aware SPD instantiating functions"). Apparently
>> the code to do this never made it into the i801 driver.
>>
>> The i801 driver needs to be fixed to inform the spd initialization code
>> that the spd5118 address range is write protected. The patch to do this was
>> "i2c: i801: Do not instantiate spd5118 under SPD Write Disable". I have no idea
>> why that patch didn't make it upstream.
>>
>> Guenter
>>
> Good question, do you want to send the message to the i2c maintainers about this
> or should i do it?
> 

Please go ahead. I am so far behind with everything :-(.

Thanks,
Guenter

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

[-- Attachment #1.1: Type: text/plain, Size: 10038 bytes --]

On Mon, Jan 26, 2026 at 1:36 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 1/24/26 11:11, Armin Wolf wrote:
> > Am 24.01.26 um 15:45 schrieb TINSAE TADESSE:
> >
> >> On Fri, Jan 16, 2026 at 9:24 AM Guenter Roeck <linux@roeck-us.net>
wrote:
> >>> On 1/15/26 05:50, TINSAE TADESSE wrote:
> >>>> On Wed, Jan 14, 2026 at 5:23 PM Guenter Roeck <linux@roeck-us.net>
wrote:
> >>>>> On 1/14/26 05:07, TINSAE TADESSE wrote:
> >>>>> ...
> >>>>>>>> Hi Guenter,
> >>>>>>>>
> >>>>>>>> I tested changing the i801 SMBus controller to use
> >>>>>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
> >>>>>>>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
> >>>>>>>> change, spd5118 resume failures (-ENXIO)
> >>>>>>>> still persist, suggesting PM ordering alone is insufficient and
other
> >>>>>>>> firmware interactions are involved.
> >>>>>>> How about the problem in the suspend function ? Is that also
still seen ?
> >>>>>>>
> >>>>>>> Also, the subject talks about -EIO. Is that still seen ?
> >>>>>>>
> >>>>>>> Either case, can you enable debug logs for the i801 driver ?
> >>>>>>> It should generate log entries when it reports errors.
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> Guenter
> >>>>>>>
> >>>>>> Hi Guenter,
> >>>>>>
> >>>>>> Thank you for the questions. To clarify:
> >>>>>>
> >>>>> Please do not drop mailing lists from replies.
> >>>>>
> >>>>>> 1) I have not observed any failures in the suspend path. The
suspend
> >>>>>> callback completes successfully, and
> >>>>>> I have not seen I2C errors or warnings during suspend at any point.
> >>>>> Sorry, I seem to be missing something.
> >>>>>
> >>>>> In that case, what is the point of patch 3/3 of your series which
> >>>>> removes hardware accesses from the suspend function ?
> >>>>>
> >>>>>> 2) I have also not observed -EIO in my testing. The error
consistently
> >>>>>> reported on resume and subsequent hwmon access is -ENXIO.
> >>>>>> Earlier references to -EIO were based on assumptions rather than
> >>>>>> observed logs, and I should have been clearer about that.
> >>>>>>
> >>>>> Thanks for the clarification.
> >>>>>
> >>>>> Guenter
> >>>>>
> >>>>>> I am enabling debug logging for the i801 driver to collect more
> >>>>>> concrete evidence of controller state during resume.
> >>>> Hi Guenter,
> >>>>
> >>>>> Sorry, I seem to be missing something.
> >>>>>
> >>>>> In that case, what is the point of patch 3/3 of your series which
> >>>>> removes hardware accesses from the suspend function ?
> >>>> You are right to question this, and I agree that it needs
clarification.
> >>>>
> >>>> Patch 3/3 was originally proposed under the assumption that the
resume failures
> >>>> were caused by spd5118 performing I2C transactions while the
> >>>> controller was not yet available,
> >>>> and that removing hardware accesses from the suspend path might
> >>>> mitigate the issue.
> >>>> At that point, I assumed the problem was limited to the resume
callback.
> >>>>
> >>>> After enabling detailed i801 debug logging and testing with
> >>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() in the i801 driver,
> >>>> it became clear that this assumption was incorrect. The controller
> >>>> itself reports "i801_smbus: No response"
> >>>> both during suspend and immediately after resume, and spd5118 merely
> >>>> propagates the resulting -ENXIO.
> >>> Outch, that really hurts, because it means that something is seriously
> >>> broken in both the suspend and resume path. The device _must_ be
accessible
> >>> in the suspend path. Otherwise there is no guarantee that the device
is
> >>> accessible for normal (pre-suspend) operation. After all, someone
could
> >>> run a script reading sysfs attributes in a tight loop continuously,
> >>> or the thermal subsystem could try to access the chip. That would
suddenly
> >>> start to fail if something in the device access path starts to be
suspended
> >>> while the underlying hardware is still believed to be operational.
> >>>
> >>> I could imagine some hack/quirk for the resume path, such as delaying
resume
> >>> for some period of time for affected hardware, but I have no idea
what to
> >>> do on the suspend side. We can not just drop device writes during
suspend
> >>> because some broken hardware/firmware does not let us actually access
> >>> (and thus suspend) the hardware anymore by the time the suspend
function
> >>> is called.
> >>>
> >>> Guenter
> >>>
> >>>> This indicates that the issue is not caused by spd5118 suspend/resume
> >>>> behavior, but by the unavailability of the
> >>>> SMBus controller due to platform or firmware interactions during
> >>>> s2idle transitions.
> >>>>
> >>>> Given this, I agree that patch 3/3 does not address the root cause
and
> >>>> does not provide a justified improvement.
> >>>> I am therefore fine with dropping it.
> >>>>
> >>>> Thank you for pointing this out.
> >>>>
> >> Hi Guenter,
> >>
> >> Thanks for the continued review and for questioning the earlier
> >> direction — that helped narrow this down properly.
> >>
> >> After enabling full i801 debug logging (included below in this email)
> >> and inspecting both drivers, it became clear that the resume
> >> failures are not caused by spd5118 accessing the hardware too
> >> early, nor by PM ordering issues. Instead, the SMBus controller
> >> explicitly reports “SPD Write Disable is set”, and any
> >> block write transactions to the SPD device consistently fail with
> >> DEV_ERR. spd5118 merely propagates the resulting -ENXIO.
> >
> > Oh no, this likely happens even when merely reading values, as the
spd5118
> > uses a page register to switch between different register pages. In
order
> > to access temperature data (page 0), you might already have to issue a
> > write access to the page register. The only reason why it works for you
> > is that the spd5118 likely already has page 0 selected by the system
firmware
> > during boot.
> >
>
> Exactly. There is no guarantee that page 0 is selected.
>
> >> With that in mind, I have dropped the earlier patch that attempted
> >> to remove hardware access from the suspend path
> >> unconditionally.
> >> That patch does not address the root cause and is no longer
> >> part of the series.
> >>
> >> I am instead proposing a minimal 2-patch series:
> >>
> >> 1/2 records whether the platform enforces SPD write disable at probe
> >> time (no behavior change).
> >> 2/2 avoids regcache writeback during suspend/resume when the device
> >> operates in read-only mode, while still allowing read access to
> >> temperature inputs.
> >>
> >> This avoids issuing SMBus transactions that are architecturally
> >> blocked on these systems, and does not rely on
> >> delays or PM ordering assumptions, and leaves behavior unchanged on
> >> platforms where SPD writes are permitted.
> >>
> >> If this direction looks acceptable, I’m happy to re-spin and post the
> >> series formally.
> >>
> >> Thanks again for the guidance.
> >
> > I do not know if this is a reliable solution, as the system firmware
might
> > select a different register page during resume. This will then prevent
the
> > driver from functioning.
> >
>
> No, it is not reliable. The driver is simply not usable in this scenario.
> This isn't just the temperature sensor code - the eeprom code is affected
> as well.
>
> > I would love to see the spd5118 driver working on such systems with
reduced
> > functionality, but i will leave it to Guenter to decide if this
approach is
> > maintainable.
> >
> > Besides that: did the spd5118 driver load automatically on your device?
> >
>
> I thought that was disabled. The i801 driver is supposed to detect if
write
> protect is enabled and, if so, it is supposed to not instantiate the
spd5118
> driver for DDR3. Support for this was added with commit 4d6d35d3417d
("i2c:
> smbus: introduce Write Disable-aware SPD instantiating functions").
Apparently
> the code to do this never made it into the i801 driver.
>
> The i801 driver needs to be fixed to inform the spd initialization code
> that the spd5118 address range is write protected. The patch to do this
was
> "i2c: i801: Do not instantiate spd5118 under SPD Write Disable". I have
no idea
> why that patch didn't make it upstream.
>
> Guenter
>

Hi Guenter,

> > The i801 driver needs to be fixed to inform the spd initialization code
> > that the spd5118 address range is write protected. The patch to do
> > this was
> > "i2c: i801: Do not instantiate spd5118 under SPD Write Disable". I
> > have no idea
> > why that patch didn't make it upstream.

I initially considered exposing SPD write-protection as a capability to be
consumed by spd5118.
However, spd5118 already depends on firmware-initialized state (page
selection) and cannot reliably
determine safe operation at probe time without issuing writes.
Given that, suppressing SPD instantiation in the i801 driver when SPD Write
Disable is
the only solution here.

Tested the remaining patch [1], and it does not seem to fix the issue, as
it is added at the wrong stage
in the initialization of the i801 driver. The attached log shows that
spd5118 is instantiated and fully probed
before the "do not instantiate spd5118 under SPD Write Disable" is acted
upon.
This confirms that suppressing instantiation must occur before adapter
registration in i801.
Once the adapter is registered, the SPD scan has already happened, and
spd5118 has bound successfully
based on the firmware-initialized state, even though correct operation is
impossible.
Therefore, the fix must be run prior to adapter registration.

[1]
https://lore.kernel.org/all/20250430-for-upstream-i801-spd5118-no-instantiate-v2-2-2f54d91ae2c7@canonical.com/


The fix is to register the SPD write-disable policy before
i2c_add_adapter(), so the I2C core never
probes SPD addresses on write-protected platforms.

[-- Attachment #1.2: Type: text/html, Size: 13170 bytes --]

[-- Attachment #2: dmesg_i2c_i801_logs_3-partial --]
[-- Type: application/octet-stream, Size: 6692 bytes --]

...
[    0.216526] ACPI: Core revision 20250807
[    0.253548] ACPI: PM: Registering ACPI NVS region [mem 0x61d05000-0x61f04fff] (2097152 bytes)
[    0.271138] acpiphp: ACPI Hot Plug PCI Controller Driver version: 0.5
...
[    0.840692] ACPI: EC: EC started
...
[    1.319750] PCI: Using ACPI for IRQ routing
...
[   11.968950] i2c-core: driver [spd5118] registered
...
[   28.144882] i801_smbus 0000:00:1f.4: SPD Write Disable is set
[   28.145042] i801_smbus 0000:00:1f.4: SMBus using PCI interrupt
[   28.145068] i801_smbus 0000:00:1f.4: Create iTCO device...
[   28.148928] i801_smbus 0000:00:1f.4: Registering adapter SMBus I801 adapter at 0000:00:1f.4
[   28.169362] i801_smbus 0000:00:1f.4: i801 access: command=0, size=1, addr=0x50, read_write=1
[   28.169395] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x5, SMBHSTSTS_DEV_ERR=4
[   28.169411] i801_smbus 0000:00:1f.4: No response
[   28.169640] i801_smbus 0000:00:1f.4: i801 access: command=0, size=1, addr=0x51, read_write=1
[   28.169671] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x5, SMBHSTSTS_DEV_ERR=4
[   28.169694] i801_smbus 0000:00:1f.4: No response
[   28.170015] i801_smbus 0000:00:1f.4: i801 access: command=0, size=1, addr=0x52, read_write=1
[   28.170790] i801_smbus 0000:00:1f.4: i801 access: command=0, size=3, addr=0x52, read_write=1
[   28.171430] i801_smbus 0000:00:1f.4: i801 access: command=3, size=3, addr=0x52, read_write=1
[   28.171953] i801_smbus 0000:00:1f.4: i801 access: command=5, size=2, addr=0x52, read_write=1
[   28.172475] i801_smbus 0000:00:1f.4: i801 access: command=13, size=2, addr=0x52, read_write=1
[   28.173000] i801_smbus 0000:00:1f.4: i801 access: command=14, size=2, addr=0x52, read_write=1
[   28.173524] i801_smbus 0000:00:1f.4: i801 access: command=2, size=2, addr=0x52, read_write=1
[   28.174165] i801_smbus 0000:00:1f.4: i801 access: command=1a, size=2, addr=0x52, read_write=1
[   28.174189] i2c i2c-14: Creating spd5118 at 0x52
[   28.178923] spd5118 14-0052: probe
[   28.179882] i801_smbus 0000:00:1f.4: i801 access: command=0, size=3, addr=0x52, read_write=1
[   28.180482] i801_smbus 0000:00:1f.4: i801 access: command=b, size=2, addr=0x52, read_write=1
[   28.181527] i801_smbus 0000:00:1f.4: i801 access: command=b, size=8, addr=0x52, read_write=1
[   28.182100] i801_smbus 0000:00:1f.4: i801 access: command=5, size=8, addr=0x52, read_write=1
[   28.182652] i801_smbus 0000:00:1f.4: i801 access: command=2, size=8, addr=0x52, read_write=1
[   28.183174] i801_smbus 0000:00:1f.4: i801 access: command=3, size=8, addr=0x52, read_write=1
[   28.183702] i801_smbus 0000:00:1f.4: i801 access: command=4, size=8, addr=0x52, read_write=1
[   28.195085] spd5118 14-0052: DDR5 temperature sensor: vendor 0x00:0xb3 revision 2.2
[   28.195710] i2c i2c-14: client [spd5118] registered with bus id 14-0052
[   28.195906] i801_smbus 0000:00:1f.4: i801 access: command=0, size=1, addr=0x53, read_write=1
[   28.195923] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x5, SMBHSTSTS_DEV_ERR=4
[   28.195933] i801_smbus 0000:00:1f.4: No response
[   28.196122] i801_smbus 0000:00:1f.4: i801 access: command=0, size=1, addr=0x54, read_write=1
[   28.196159] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x5, SMBHSTSTS_DEV_ERR=4
[   28.196175] i801_smbus 0000:00:1f.4: No response
[   28.197320] i801_smbus 0000:00:1f.4: i801 access: command=0, size=1, addr=0x55, read_write=1
[   28.197340] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x5, SMBHSTSTS_DEV_ERR=4
[   28.197352] i801_smbus 0000:00:1f.4: No response
[   28.197539] i801_smbus 0000:00:1f.4: i801 access: command=0, size=1, addr=0x56, read_write=1
[   28.197557] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x5, SMBHSTSTS_DEV_ERR=4
[   28.197569] i801_smbus 0000:00:1f.4: No response
[   28.197754] i801_smbus 0000:00:1f.4: i801 access: command=0, size=1, addr=0x57, read_write=1
[   28.197775] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x5, SMBHSTSTS_DEV_ERR=4
[   28.197790] i801_smbus 0000:00:1f.4: No response
[   28.197983] i801_smbus 0000:00:1f.4: i801 access: command=0, size=0, addr=0x48, read_write=0
[   28.198004] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x1, SMBHSTSTS_DEV_ERR=4
[   28.198020] i801_smbus 0000:00:1f.4: No response
[   28.198210] i801_smbus 0000:00:1f.4: i801 access: command=0, size=0, addr=0x49, read_write=0
[   28.198231] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x1, SMBHSTSTS_DEV_ERR=4
[   28.198247] i801_smbus 0000:00:1f.4: No response
[   28.198448] i801_smbus 0000:00:1f.4: i801 access: command=0, size=0, addr=0x4a, read_write=0
[   28.199053] i801_smbus 0000:00:1f.4: i801 access: command=fe, size=3, addr=0x4a, read_write=1
[   28.199249] i801_smbus 0000:00:1f.4: i801 access: command=0, size=0, addr=0x4b, read_write=0
[   28.199272] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x1, SMBHSTSTS_DEV_ERR=4
[   28.199287] i801_smbus 0000:00:1f.4: No response
[   28.199306] i801_smbus 0000:00:1f.4: Enable host notify interrupt...
[   28.199318] i801_smbus 0000:00:1f.4: Registering optional targets...
...
[  156.718777] PM: suspend entry (s2idle)
[  157.175065] i801_smbus 0000:00:1f.4: i801 access: command=1a, size=8, addr=0x52, read_write=1
[  157.177574] i801_smbus 0000:00:1f.4: i801 access: command=b, size=8, addr=0x52, read_write=1
[  157.178272] i801_smbus 0000:00:1f.4: i801 access: command=1a, size=8, addr=0x52, read_write=1
[  157.179003] i801_smbus 0000:00:1f.4: i801 access: command=b, size=8, addr=0x52, read_write=1
[  157.179255] i801_smbus 0000:00:1f.4: i801 access: command=1a, size=8, addr=0x52, read_write=0
[  157.179300] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
[  157.179333] i801_smbus 0000:00:1f.4: No response
[  158.563252] ACPI: EC: interrupt blocked
[  176.447363] ACPI: EC: interrupt unblocked
[  176.616035] i801_smbus 0000:00:1f.4: i801 access: command=b, size=8, addr=0x52, read_write=0
[  176.616082] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
[  176.616107] i801_smbus 0000:00:1f.4: No response
[  176.616214] i801_smbus 0000:00:1f.4: i801 access: command=b, size=8, addr=0x52, read_write=0
[  176.616249] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
[  176.616273] i801_smbus 0000:00:1f.4: No response
[  176.616309] spd5118 14-0052: Failed to write b = 0: -6
[  176.616358] spd5118 14-0052: PM: dpm_run_callback(): spd5118_resume returns -6
[  176.616638] spd5118 14-0052: PM: failed to resume async: error -6
[  178.061460] PM: suspend exit

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/27/26 02:35, TINSAE TADESSE wrote:
> 
> 
> On Mon, Jan 26, 2026 at 1:36 AM Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>> wrote:
>  >
>  > On 1/24/26 11:11, Armin Wolf wrote:
>  > > Am 24.01.26 um 15:45 schrieb TINSAE TADESSE:
>  > >
>  > >> On Fri, Jan 16, 2026 at 9:24 AM Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>> wrote:
>  > >>> On 1/15/26 05:50, TINSAE TADESSE wrote:
>  > >>>> On Wed, Jan 14, 2026 at 5:23 PM Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>> wrote:
>  > >>>>> On 1/14/26 05:07, TINSAE TADESSE wrote:
>  > >>>>> ...
>  > >>>>>>>> Hi Guenter,
>  > >>>>>>>>
>  > >>>>>>>> I tested changing the i801 SMBus controller to use
>  > >>>>>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
>  > >>>>>>>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
>  > >>>>>>>> change, spd5118 resume failures (-ENXIO)
>  > >>>>>>>> still persist, suggesting PM ordering alone is insufficient and other
>  > >>>>>>>> firmware interactions are involved.
>  > >>>>>>> How about the problem in the suspend function ? Is that also still seen ?
>  > >>>>>>>
>  > >>>>>>> Also, the subject talks about -EIO. Is that still seen ?
>  > >>>>>>>
>  > >>>>>>> Either case, can you enable debug logs for the i801 driver ?
>  > >>>>>>> It should generate log entries when it reports errors.
>  > >>>>>>>
>  > >>>>>>> Thanks,
>  > >>>>>>> Guenter
>  > >>>>>>>
>  > >>>>>> Hi Guenter,
>  > >>>>>>
>  > >>>>>> Thank you for the questions. To clarify:
>  > >>>>>>
>  > >>>>> Please do not drop mailing lists from replies.
>  > >>>>>
>  > >>>>>> 1) I have not observed any failures in the suspend path. The suspend
>  > >>>>>> callback completes successfully, and
>  > >>>>>> I have not seen I2C errors or warnings during suspend at any point.
>  > >>>>> Sorry, I seem to be missing something.
>  > >>>>>
>  > >>>>> In that case, what is the point of patch 3/3 of your series which
>  > >>>>> removes hardware accesses from the suspend function ?
>  > >>>>>
>  > >>>>>> 2) I have also not observed -EIO in my testing. The error consistently
>  > >>>>>> reported on resume and subsequent hwmon access is -ENXIO.
>  > >>>>>> Earlier references to -EIO were based on assumptions rather than
>  > >>>>>> observed logs, and I should have been clearer about that.
>  > >>>>>>
>  > >>>>> Thanks for the clarification.
>  > >>>>>
>  > >>>>> Guenter
>  > >>>>>
>  > >>>>>> I am enabling debug logging for the i801 driver to collect more
>  > >>>>>> concrete evidence of controller state during resume.
>  > >>>> Hi Guenter,
>  > >>>>
>  > >>>>> Sorry, I seem to be missing something.
>  > >>>>>
>  > >>>>> In that case, what is the point of patch 3/3 of your series which
>  > >>>>> removes hardware accesses from the suspend function ?
>  > >>>> You are right to question this, and I agree that it needs clarification.
>  > >>>>
>  > >>>> Patch 3/3 was originally proposed under the assumption that the resume failures
>  > >>>> were caused by spd5118 performing I2C transactions while the
>  > >>>> controller was not yet available,
>  > >>>> and that removing hardware accesses from the suspend path might
>  > >>>> mitigate the issue.
>  > >>>> At that point, I assumed the problem was limited to the resume callback.
>  > >>>>
>  > >>>> After enabling detailed i801 debug logging and testing with
>  > >>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() in the i801 driver,
>  > >>>> it became clear that this assumption was incorrect. The controller
>  > >>>> itself reports "i801_smbus: No response"
>  > >>>> both during suspend and immediately after resume, and spd5118 merely
>  > >>>> propagates the resulting -ENXIO.
>  > >>> Outch, that really hurts, because it means that something is seriously
>  > >>> broken in both the suspend and resume path. The device _must_ be accessible
>  > >>> in the suspend path. Otherwise there is no guarantee that the device is
>  > >>> accessible for normal (pre-suspend) operation. After all, someone could
>  > >>> run a script reading sysfs attributes in a tight loop continuously,
>  > >>> or the thermal subsystem could try to access the chip. That would suddenly
>  > >>> start to fail if something in the device access path starts to be suspended
>  > >>> while the underlying hardware is still believed to be operational.
>  > >>>
>  > >>> I could imagine some hack/quirk for the resume path, such as delaying resume
>  > >>> for some period of time for affected hardware, but I have no idea what to
>  > >>> do on the suspend side. We can not just drop device writes during suspend
>  > >>> because some broken hardware/firmware does not let us actually access
>  > >>> (and thus suspend) the hardware anymore by the time the suspend function
>  > >>> is called.
>  > >>>
>  > >>> Guenter
>  > >>>
>  > >>>> This indicates that the issue is not caused by spd5118 suspend/resume
>  > >>>> behavior, but by the unavailability of the
>  > >>>> SMBus controller due to platform or firmware interactions during
>  > >>>> s2idle transitions.
>  > >>>>
>  > >>>> Given this, I agree that patch 3/3 does not address the root cause and
>  > >>>> does not provide a justified improvement.
>  > >>>> I am therefore fine with dropping it.
>  > >>>>
>  > >>>> Thank you for pointing this out.
>  > >>>>
>  > >> Hi Guenter,
>  > >>
>  > >> Thanks for the continued review and for questioning the earlier
>  > >> direction — that helped narrow this down properly.
>  > >>
>  > >> After enabling full i801 debug logging (included below in this email)
>  > >> and inspecting both drivers, it became clear that the resume
>  > >> failures are not caused by spd5118 accessing the hardware too
>  > >> early, nor by PM ordering issues. Instead, the SMBus controller
>  > >> explicitly reports “SPD Write Disable is set”, and any
>  > >> block write transactions to the SPD device consistently fail with
>  > >> DEV_ERR. spd5118 merely propagates the resulting -ENXIO.
>  > >
>  > > Oh no, this likely happens even when merely reading values, as the spd5118
>  > > uses a page register to switch between different register pages. In order
>  > > to access temperature data (page 0), you might already have to issue a
>  > > write access to the page register. The only reason why it works for you
>  > > is that the spd5118 likely already has page 0 selected by the system firmware
>  > > during boot.
>  > >
>  >
>  > Exactly. There is no guarantee that page 0 is selected.
>  >
>  > >> With that in mind, I have dropped the earlier patch that attempted
>  > >> to remove hardware access from the suspend path
>  > >> unconditionally.
>  > >> That patch does not address the root cause and is no longer
>  > >> part of the series.
>  > >>
>  > >> I am instead proposing a minimal 2-patch series:
>  > >>
>  > >> 1/2 records whether the platform enforces SPD write disable at probe
>  > >> time (no behavior change).
>  > >> 2/2 avoids regcache writeback during suspend/resume when the device
>  > >> operates in read-only mode, while still allowing read access to
>  > >> temperature inputs.
>  > >>
>  > >> This avoids issuing SMBus transactions that are architecturally
>  > >> blocked on these systems, and does not rely on
>  > >> delays or PM ordering assumptions, and leaves behavior unchanged on
>  > >> platforms where SPD writes are permitted.
>  > >>
>  > >> If this direction looks acceptable, I’m happy to re-spin and post the
>  > >> series formally.
>  > >>
>  > >> Thanks again for the guidance.
>  > >
>  > > I do not know if this is a reliable solution, as the system firmware might
>  > > select a different register page during resume. This will then prevent the
>  > > driver from functioning.
>  > >
>  >
>  > No, it is not reliable. The driver is simply not usable in this scenario.
>  > This isn't just the temperature sensor code - the eeprom code is affected
>  > as well.
>  >
>  > > I would love to see the spd5118 driver working on such systems with reduced
>  > > functionality, but i will leave it to Guenter to decide if this approach is
>  > > maintainable.
>  > >
>  > > Besides that: did the spd5118 driver load automatically on your device?
>  > >
>  >
>  > I thought that was disabled. The i801 driver is supposed to detect if write
>  > protect is enabled and, if so, it is supposed to not instantiate the spd5118
>  > driver for DDR3. Support for this was added with commit 4d6d35d3417d ("i2c:
>  > smbus: introduce Write Disable-aware SPD instantiating functions"). Apparently
>  > the code to do this never made it into the i801 driver.
>  >
>  > The i801 driver needs to be fixed to inform the spd initialization code
>  > that the spd5118 address range is write protected. The patch to do this was
>  > "i2c: i801: Do not instantiate spd5118 under SPD Write Disable". I have no idea
>  > why that patch didn't make it upstream.
>  >
>  > Guenter
>  >
> 
> Hi Guenter,
> 
>  > > The i801 driver needs to be fixed to inform the spd initialization code
>  > > that the spd5118 address range is write protected. The patch to do
>  > > this was
>  > > "i2c: i801: Do not instantiate spd5118 under SPD Write Disable". I
>  > > have no idea
>  > > why that patch didn't make it upstream.
> 
> I initially considered exposing SPD write-protection as a capability to be consumed by spd5118.
> However, spd5118 already depends on firmware-initialized state (page selection) and cannot reliably
> determine safe operation at probe time without issuing writes.
> Given that, suppressing SPD instantiation in the i801 driver when SPD Write Disable is
> the only solution here.
> 
> Tested the remaining patch [1], and it does not seem to fix the issue, as it is added at the wrong stage
> in the initialization of the i801 driver. The attached log shows that spd5118 is instantiated and fully probed
> before the "do not instantiate spd5118 under SPD Write Disable" is acted upon.
> This confirms that suppressing instantiation must occur before adapter registration in i801.
> Once the adapter is registered, the SPD scan has already happened, and spd5118 has bound successfully
> based on the firmware-initialized state, even though correct operation is impossible.
> Therefore, the fix must be run prior to adapter registration.
> 
> [1] https://lore.kernel.org/all/20250430-for-upstream-i801-spd5118-no-instantiate-v2-2-2f54d91ae2c7@canonical.com/ <https://lore.kernel.org/all/20250430-for-upstream-i801-spd5118-no-instantiate-v2-2-2f54d91ae2c7@canonical.com/>
> 
> 
> The fix is to register the SPD write-disable policy before i2c_add_adapter(), so the I2C core never
> probes SPD addresses on write-protected platforms.
> 

Do you have SENSORS_SPD5118_DETECT enabled in your configuration ? It should be disabled
on systems with DMI enabled.

Guenter

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

[-- Attachment #1: Type: text/plain, Size: 12336 bytes --]

On Tue, Jan 27, 2026 at 7:39 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 1/27/26 02:35, TINSAE TADESSE wrote:
> >
> >
> > On Mon, Jan 26, 2026 at 1:36 AM Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>> wrote:
> >  >
> >  > On 1/24/26 11:11, Armin Wolf wrote:
> >  > > Am 24.01.26 um 15:45 schrieb TINSAE TADESSE:
> >  > >
> >  > >> On Fri, Jan 16, 2026 at 9:24 AM Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>> wrote:
> >  > >>> On 1/15/26 05:50, TINSAE TADESSE wrote:
> >  > >>>> On Wed, Jan 14, 2026 at 5:23 PM Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>> wrote:
> >  > >>>>> On 1/14/26 05:07, TINSAE TADESSE wrote:
> >  > >>>>> ...
> >  > >>>>>>>> Hi Guenter,
> >  > >>>>>>>>
> >  > >>>>>>>> I tested changing the i801 SMBus controller to use
> >  > >>>>>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
> >  > >>>>>>>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
> >  > >>>>>>>> change, spd5118 resume failures (-ENXIO)
> >  > >>>>>>>> still persist, suggesting PM ordering alone is insufficient and other
> >  > >>>>>>>> firmware interactions are involved.
> >  > >>>>>>> How about the problem in the suspend function ? Is that also still seen ?
> >  > >>>>>>>
> >  > >>>>>>> Also, the subject talks about -EIO. Is that still seen ?
> >  > >>>>>>>
> >  > >>>>>>> Either case, can you enable debug logs for the i801 driver ?
> >  > >>>>>>> It should generate log entries when it reports errors.
> >  > >>>>>>>
> >  > >>>>>>> Thanks,
> >  > >>>>>>> Guenter
> >  > >>>>>>>
> >  > >>>>>> Hi Guenter,
> >  > >>>>>>
> >  > >>>>>> Thank you for the questions. To clarify:
> >  > >>>>>>
> >  > >>>>> Please do not drop mailing lists from replies.
> >  > >>>>>
> >  > >>>>>> 1) I have not observed any failures in the suspend path. The suspend
> >  > >>>>>> callback completes successfully, and
> >  > >>>>>> I have not seen I2C errors or warnings during suspend at any point.
> >  > >>>>> Sorry, I seem to be missing something.
> >  > >>>>>
> >  > >>>>> In that case, what is the point of patch 3/3 of your series which
> >  > >>>>> removes hardware accesses from the suspend function ?
> >  > >>>>>
> >  > >>>>>> 2) I have also not observed -EIO in my testing. The error consistently
> >  > >>>>>> reported on resume and subsequent hwmon access is -ENXIO.
> >  > >>>>>> Earlier references to -EIO were based on assumptions rather than
> >  > >>>>>> observed logs, and I should have been clearer about that.
> >  > >>>>>>
> >  > >>>>> Thanks for the clarification.
> >  > >>>>>
> >  > >>>>> Guenter
> >  > >>>>>
> >  > >>>>>> I am enabling debug logging for the i801 driver to collect more
> >  > >>>>>> concrete evidence of controller state during resume.
> >  > >>>> Hi Guenter,
> >  > >>>>
> >  > >>>>> Sorry, I seem to be missing something.
> >  > >>>>>
> >  > >>>>> In that case, what is the point of patch 3/3 of your series which
> >  > >>>>> removes hardware accesses from the suspend function ?
> >  > >>>> You are right to question this, and I agree that it needs clarification.
> >  > >>>>
> >  > >>>> Patch 3/3 was originally proposed under the assumption that the resume failures
> >  > >>>> were caused by spd5118 performing I2C transactions while the
> >  > >>>> controller was not yet available,
> >  > >>>> and that removing hardware accesses from the suspend path might
> >  > >>>> mitigate the issue.
> >  > >>>> At that point, I assumed the problem was limited to the resume callback.
> >  > >>>>
> >  > >>>> After enabling detailed i801 debug logging and testing with
> >  > >>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() in the i801 driver,
> >  > >>>> it became clear that this assumption was incorrect. The controller
> >  > >>>> itself reports "i801_smbus: No response"
> >  > >>>> both during suspend and immediately after resume, and spd5118 merely
> >  > >>>> propagates the resulting -ENXIO.
> >  > >>> Outch, that really hurts, because it means that something is seriously
> >  > >>> broken in both the suspend and resume path. The device _must_ be accessible
> >  > >>> in the suspend path. Otherwise there is no guarantee that the device is
> >  > >>> accessible for normal (pre-suspend) operation. After all, someone could
> >  > >>> run a script reading sysfs attributes in a tight loop continuously,
> >  > >>> or the thermal subsystem could try to access the chip. That would suddenly
> >  > >>> start to fail if something in the device access path starts to be suspended
> >  > >>> while the underlying hardware is still believed to be operational.
> >  > >>>
> >  > >>> I could imagine some hack/quirk for the resume path, such as delaying resume
> >  > >>> for some period of time for affected hardware, but I have no idea what to
> >  > >>> do on the suspend side. We can not just drop device writes during suspend
> >  > >>> because some broken hardware/firmware does not let us actually access
> >  > >>> (and thus suspend) the hardware anymore by the time the suspend function
> >  > >>> is called.
> >  > >>>
> >  > >>> Guenter
> >  > >>>
> >  > >>>> This indicates that the issue is not caused by spd5118 suspend/resume
> >  > >>>> behavior, but by the unavailability of the
> >  > >>>> SMBus controller due to platform or firmware interactions during
> >  > >>>> s2idle transitions.
> >  > >>>>
> >  > >>>> Given this, I agree that patch 3/3 does not address the root cause and
> >  > >>>> does not provide a justified improvement.
> >  > >>>> I am therefore fine with dropping it.
> >  > >>>>
> >  > >>>> Thank you for pointing this out.
> >  > >>>>
> >  > >> Hi Guenter,
> >  > >>
> >  > >> Thanks for the continued review and for questioning the earlier
> >  > >> direction — that helped narrow this down properly.
> >  > >>
> >  > >> After enabling full i801 debug logging (included below in this email)
> >  > >> and inspecting both drivers, it became clear that the resume
> >  > >> failures are not caused by spd5118 accessing the hardware too
> >  > >> early, nor by PM ordering issues. Instead, the SMBus controller
> >  > >> explicitly reports “SPD Write Disable is set”, and any
> >  > >> block write transactions to the SPD device consistently fail with
> >  > >> DEV_ERR. spd5118 merely propagates the resulting -ENXIO.
> >  > >
> >  > > Oh no, this likely happens even when merely reading values, as the spd5118
> >  > > uses a page register to switch between different register pages. In order
> >  > > to access temperature data (page 0), you might already have to issue a
> >  > > write access to the page register. The only reason why it works for you
> >  > > is that the spd5118 likely already has page 0 selected by the system firmware
> >  > > during boot.
> >  > >
> >  >
> >  > Exactly. There is no guarantee that page 0 is selected.
> >  >
> >  > >> With that in mind, I have dropped the earlier patch that attempted
> >  > >> to remove hardware access from the suspend path
> >  > >> unconditionally.
> >  > >> That patch does not address the root cause and is no longer
> >  > >> part of the series.
> >  > >>
> >  > >> I am instead proposing a minimal 2-patch series:
> >  > >>
> >  > >> 1/2 records whether the platform enforces SPD write disable at probe
> >  > >> time (no behavior change).
> >  > >> 2/2 avoids regcache writeback during suspend/resume when the device
> >  > >> operates in read-only mode, while still allowing read access to
> >  > >> temperature inputs.
> >  > >>
> >  > >> This avoids issuing SMBus transactions that are architecturally
> >  > >> blocked on these systems, and does not rely on
> >  > >> delays or PM ordering assumptions, and leaves behavior unchanged on
> >  > >> platforms where SPD writes are permitted.
> >  > >>
> >  > >> If this direction looks acceptable, I’m happy to re-spin and post the
> >  > >> series formally.
> >  > >>
> >  > >> Thanks again for the guidance.
> >  > >
> >  > > I do not know if this is a reliable solution, as the system firmware might
> >  > > select a different register page during resume. This will then prevent the
> >  > > driver from functioning.
> >  > >
> >  >
> >  > No, it is not reliable. The driver is simply not usable in this scenario.
> >  > This isn't just the temperature sensor code - the eeprom code is affected
> >  > as well.
> >  >
> >  > > I would love to see the spd5118 driver working on such systems with reduced
> >  > > functionality, but i will leave it to Guenter to decide if this approach is
> >  > > maintainable.
> >  > >
> >  > > Besides that: did the spd5118 driver load automatically on your device?
> >  > >
> >  >
> >  > I thought that was disabled. The i801 driver is supposed to detect if write
> >  > protect is enabled and, if so, it is supposed to not instantiate the spd5118
> >  > driver for DDR3. Support for this was added with commit 4d6d35d3417d ("i2c:
> >  > smbus: introduce Write Disable-aware SPD instantiating functions"). Apparently
> >  > the code to do this never made it into the i801 driver.
> >  >
> >  > The i801 driver needs to be fixed to inform the spd initialization code
> >  > that the spd5118 address range is write protected. The patch to do this was
> >  > "i2c: i801: Do not instantiate spd5118 under SPD Write Disable". I have no idea
> >  > why that patch didn't make it upstream.
> >  >
> >  > Guenter
> >  >
> >
> > Hi Guenter,
> >
> >  > > The i801 driver needs to be fixed to inform the spd initialization code
> >  > > that the spd5118 address range is write protected. The patch to do
> >  > > this was
> >  > > "i2c: i801: Do not instantiate spd5118 under SPD Write Disable". I
> >  > > have no idea
> >  > > why that patch didn't make it upstream.
> >
> > I initially considered exposing SPD write-protection as a capability to be consumed by spd5118.
> > However, spd5118 already depends on firmware-initialized state (page selection) and cannot reliably
> > determine safe operation at probe time without issuing writes.
> > Given that, suppressing SPD instantiation in the i801 driver when SPD Write Disable is
> > the only solution here.
> >
> > Tested the remaining patch [1], and it does not seem to fix the issue, as it is added at the wrong stage
> > in the initialization of the i801 driver. The attached log shows that spd5118 is instantiated and fully probed
> > before the "do not instantiate spd5118 under SPD Write Disable" is acted upon.
> > This confirms that suppressing instantiation must occur before adapter registration in i801.
> > Once the adapter is registered, the SPD scan has already happened, and spd5118 has bound successfully
> > based on the firmware-initialized state, even though correct operation is impossible.
> > Therefore, the fix must be run prior to adapter registration.
> >
> > [1] https://lore.kernel.org/all/20250430-for-upstream-i801-spd5118-no-instantiate-v2-2-2f54d91ae2c7@canonical.com/ <https://lore.kernel.org/all/20250430-for-upstream-i801-spd5118-no-instantiate-v2-2-2f54d91ae2c7@canonical.com/>
> >
> >
> > The fix is to register the SPD write-disable policy before i2c_add_adapter(), so the I2C core never
> > probes SPD addresses on write-protected platforms.
> >
>
> Do you have SENSORS_SPD5118_DETECT enabled in your configuration ? It should be disabled
> on systems with DMI enabled.
>
> Guenter
>

Hello Guenter,

Disabling CONFIG_SENSORS_SPD5118_DETECT completely avoids the issue on
affected platforms,
even without any code changes. This confirms that the failures are
triggered specifically by automatic
SPD5118 instantiation on systems where the i801 controller enforces
SPD Write Disable.
The existing Kconfig already documents that auto-detection is optional
and that, on x86, instantiation
is expected to be coordinated by the SMBus subsystem.

The fact that disabling auto-detection avoids the issue is evidence
that instantiation itself is
unsafe under SPD Write Disable, not that spd5118 is malfunctioning.

The existing Kconfig help already says "Disabling the detect function
will speed up boot time and reduce
the risk of mis-detecting…", and the attached patch documents the
reason why mis-detection can happen.

[-- Attachment #2: 0001-hwmon-spd5118-Document-detect-limitations-under-SPD-.patch --]
[-- Type: text/x-patch, Size: 1544 bytes --]

From 058ae6336392f26bcfd825fa9c01914c52f47c4d Mon Sep 17 00:00:00 2001
From: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
Date: Tue, 27 Jan 2026 22:11:25 +0300
Subject: [PATCH] hwmon: spd5118: Document detect limitations under SPD Write
 Disable

Document that on some x86 platforms the SMBus controller may enforce
SPD Write Disable, which can prevent reliable operation of SPD5118
devices. In such configurations, automatic detection may result in
probe failures unless instantiation is coordinated by the SMBus
controller driver.

No functional change.

Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
---
 drivers/hwmon/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 157678b821fc..309fa639804d 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2342,6 +2342,12 @@ config SENSORS_SPD5118_DETECT
 	  I2C SMBus subsystem. Devicetree based systems will instantiate
 	  attached devices if the DIMMs are listed in the devicetree file.
 
+	  Note that on some x86 platforms the SMBus controller may enforce
+	  SPD Write Disable, which can prevent reliable operation of SPD5118
+	  devices. In such configurations, automatic detection may lead to
+	  probe failures unless instantiation is coordinated by the SMBus
+	  controller driver.
+
 	  Disabling the detect function will speed up boot time and reduce
 	  the risk of mis-detecting SPD5118 compliant devices. However, it
 	  may result in missed DIMMs under some circumstances.
-- 
2.47.3

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

Hi,

On Tue, Jan 27, 2026 at 10:23:24PM +0300, TINSAE TADESSE wrote:
> 
> Disabling CONFIG_SENSORS_SPD5118_DETECT completely avoids the issue on
> affected platforms,
> even without any code changes. This confirms that the failures are
> triggered specifically by automatic
> SPD5118 instantiation on systems where the i801 controller enforces
> SPD Write Disable.

Thanks for confirming. Can you try if the patch below fixes the problem ?
It is a wild shot, but it might be worth a try.

Thanks,
Guenter

---
From b44c31c2c779a67827e3144b818cf21f5efacea1 Mon Sep 17 00:00:00 2001
From: Guenter Roeck <linux@roeck-us.net>
Date: Tue, 27 Jan 2026 15:32:32 -0800
Subject: [PATCH] hwmon: (spd5118) Explicitly enable temperature sensor in
 probe function

Instantiating the driver does not make sense if the temperature sensor
is disabled, so enable it unconditionally in the probe function.

If that fails, write operations to the chip are likely disabled
by the I2C controller. Bail out with an eror message if that happens.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/spd5118.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 5da44571b6a0..3e0e780014f9 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -552,6 +552,11 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
 	if (!spd5118_vendor_valid(bank, vendor))
 		return -ENODEV;
 
+	if (regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG,
+			       SPD5118_TS_DISABLE, 0) < 0)
+		return dev_err_probe(dev, -ENODEV,
+				     "Failed to enable temperature sensor\n");
+
 	data->regmap = regmap;
 	mutex_init(&data->nvmem_lock);
 	dev_set_drvdata(dev, data);
-- 
2.45.2

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Kurt Borja]

On Tue Jan 27, 2026 at 6:41 PM -05, Guenter Roeck wrote:
> Hi,
>
> On Tue, Jan 27, 2026 at 10:23:24PM +0300, TINSAE TADESSE wrote:
>> 
>> Disabling CONFIG_SENSORS_SPD5118_DETECT completely avoids the issue on
>> affected platforms,
>> even without any code changes. This confirms that the failures are
>> triggered specifically by automatic
>> SPD5118 instantiation on systems where the i801 controller enforces
>> SPD Write Disable.
>
> Thanks for confirming. Can you try if the patch below fixes the problem ?
> It is a wild shot, but it might be worth a try.
>
> Thanks,
> Guenter
>
> ---
> From b44c31c2c779a67827e3144b818cf21f5efacea1 Mon Sep 17 00:00:00 2001
> From: Guenter Roeck <linux@roeck-us.net>
> Date: Tue, 27 Jan 2026 15:32:32 -0800
> Subject: [PATCH] hwmon: (spd5118) Explicitly enable temperature sensor in
>  probe function
>
> Instantiating the driver does not make sense if the temperature sensor
> is disabled, so enable it unconditionally in the probe function.
>
> If that fails, write operations to the chip are likely disabled
> by the I2C controller. Bail out with an eror message if that happens.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/hwmon/spd5118.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index 5da44571b6a0..3e0e780014f9 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -552,6 +552,11 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
>  	if (!spd5118_vendor_valid(bank, vendor))
>  		return -ENODEV;
>  
> +	if (regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG,
> +			       SPD5118_TS_DISABLE, 0) < 0)
> +		return dev_err_probe(dev, -ENODEV,
> +				     "Failed to enable temperature sensor\n");
> +
>  	data->regmap = regmap;
>  	mutex_init(&data->nvmem_lock);
>  	dev_set_drvdata(dev, data);

Hi Guenter,

I'm experiencing the same issue reported in this thread. This patch does
not fix it for me :(.

-- 
Thanks,
 ~ Kurt

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

Hi Kurt,

On 1/30/26 16:55, Kurt Borja wrote:
> On Tue Jan 27, 2026 at 6:41 PM -05, Guenter Roeck wrote:
>> Hi,
>>
>> On Tue, Jan 27, 2026 at 10:23:24PM +0300, TINSAE TADESSE wrote:
>>>
>>> Disabling CONFIG_SENSORS_SPD5118_DETECT completely avoids the issue on
>>> affected platforms,
>>> even without any code changes. This confirms that the failures are
>>> triggered specifically by automatic
>>> SPD5118 instantiation on systems where the i801 controller enforces
>>> SPD Write Disable.
>>
>> Thanks for confirming. Can you try if the patch below fixes the problem ?
>> It is a wild shot, but it might be worth a try.
>>
>> Thanks,
>> Guenter
>>
>> ---
>>  From b44c31c2c779a67827e3144b818cf21f5efacea1 Mon Sep 17 00:00:00 2001
>> From: Guenter Roeck <linux@roeck-us.net>
>> Date: Tue, 27 Jan 2026 15:32:32 -0800
>> Subject: [PATCH] hwmon: (spd5118) Explicitly enable temperature sensor in
>>   probe function
>>
>> Instantiating the driver does not make sense if the temperature sensor
>> is disabled, so enable it unconditionally in the probe function.
>>
>> If that fails, write operations to the chip are likely disabled
>> by the I2C controller. Bail out with an eror message if that happens.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   drivers/hwmon/spd5118.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>> index 5da44571b6a0..3e0e780014f9 100644
>> --- a/drivers/hwmon/spd5118.c
>> +++ b/drivers/hwmon/spd5118.c
>> @@ -552,6 +552,11 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
>>   	if (!spd5118_vendor_valid(bank, vendor))
>>   		return -ENODEV;
>>   
>> +	if (regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG,
>> +			       SPD5118_TS_DISABLE, 0) < 0)
>> +		return dev_err_probe(dev, -ENODEV,
>> +				     "Failed to enable temperature sensor\n");
>> +
>>   	data->regmap = regmap;
>>   	mutex_init(&data->nvmem_lock);
>>   	dev_set_drvdata(dev, data);
> 
> Hi Guenter,
> 
> I'm experiencing the same issue reported in this thread. This patch does
> not fix it for me :(.
> 
Thanks for a note. Well, it was a wild shot, so it is not entirely surprising
that it didn't work. I suspect regmap doesn't actually write the register
if the value is unchanged. Another option might be to try writing a value
(e.g., 0x01) into register 0x13 or 0x14. Those are "clear status" registers.
If that fails, we would know that the chip is write protected.

Thanks,
Guenter

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Kurt Borja]

On Fri Jan 30, 2026 at 8:21 PM -05, Guenter Roeck wrote:
> Hi Kurt,
>
> On 1/30/26 16:55, Kurt Borja wrote:
>> On Tue Jan 27, 2026 at 6:41 PM -05, Guenter Roeck wrote:
>>> Hi,
>>>
>>> On Tue, Jan 27, 2026 at 10:23:24PM +0300, TINSAE TADESSE wrote:
>>>>
>>>> Disabling CONFIG_SENSORS_SPD5118_DETECT completely avoids the issue on
>>>> affected platforms,
>>>> even without any code changes. This confirms that the failures are
>>>> triggered specifically by automatic
>>>> SPD5118 instantiation on systems where the i801 controller enforces
>>>> SPD Write Disable.
>>>
>>> Thanks for confirming. Can you try if the patch below fixes the problem ?
>>> It is a wild shot, but it might be worth a try.
>>>
>>> Thanks,
>>> Guenter
>>>
>>> ---
>>>  From b44c31c2c779a67827e3144b818cf21f5efacea1 Mon Sep 17 00:00:00 2001
>>> From: Guenter Roeck <linux@roeck-us.net>
>>> Date: Tue, 27 Jan 2026 15:32:32 -0800
>>> Subject: [PATCH] hwmon: (spd5118) Explicitly enable temperature sensor in
>>>   probe function
>>>
>>> Instantiating the driver does not make sense if the temperature sensor
>>> is disabled, so enable it unconditionally in the probe function.
>>>
>>> If that fails, write operations to the chip are likely disabled
>>> by the I2C controller. Bail out with an eror message if that happens.
>>>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>> ---
>>>   drivers/hwmon/spd5118.c | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>> index 5da44571b6a0..3e0e780014f9 100644
>>> --- a/drivers/hwmon/spd5118.c
>>> +++ b/drivers/hwmon/spd5118.c
>>> @@ -552,6 +552,11 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
>>>   	if (!spd5118_vendor_valid(bank, vendor))
>>>   		return -ENODEV;
>>>   
>>> +	if (regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG,
>>> +			       SPD5118_TS_DISABLE, 0) < 0)
>>> +		return dev_err_probe(dev, -ENODEV,
>>> +				     "Failed to enable temperature sensor\n");
>>> +
>>>   	data->regmap = regmap;
>>>   	mutex_init(&data->nvmem_lock);
>>>   	dev_set_drvdata(dev, data);
>> 
>> Hi Guenter,
>> 
>> I'm experiencing the same issue reported in this thread. This patch does
>> not fix it for me :(.
>> 
> Thanks for a note. Well, it was a wild shot, so it is not entirely surprising
> that it didn't work. I suspect regmap doesn't actually write the register
> if the value is unchanged. Another option might be to try writing a value

I tried forcing the write with

	bool change;
	err = regmap_update_bits_base(regmap, SPD5118_REG_TEMP_CONFIG,
				      SPD5118_TS_DISABLE, 0, &change, false, true);
	if (err)
		return dev_err_probe(dev, err,
				     "Failed to enable temperature sensor\n");

and it fails to probe

	spd5118 17-0051: error -ENXIO: Failed to enable temperature sensor
	spd5118 17-0053: error -ENXIO: Failed to enable temperature sensor


> (e.g., 0x01) into register 0x13 or 0x14. Those are "clear status" registers.
> If that fails, we would know that the chip is write protected.

I'll try this later too.

>
> Thanks,
> Guenter

-- 
Thanks,
 ~ Kurt

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/30/26 17:54, Kurt Borja wrote:
> On Fri Jan 30, 2026 at 8:21 PM -05, Guenter Roeck wrote:
>> Hi Kurt,
>>
>> On 1/30/26 16:55, Kurt Borja wrote:
>>> On Tue Jan 27, 2026 at 6:41 PM -05, Guenter Roeck wrote:
>>>> Hi,
>>>>
>>>> On Tue, Jan 27, 2026 at 10:23:24PM +0300, TINSAE TADESSE wrote:
>>>>>
>>>>> Disabling CONFIG_SENSORS_SPD5118_DETECT completely avoids the issue on
>>>>> affected platforms,
>>>>> even without any code changes. This confirms that the failures are
>>>>> triggered specifically by automatic
>>>>> SPD5118 instantiation on systems where the i801 controller enforces
>>>>> SPD Write Disable.
>>>>
>>>> Thanks for confirming. Can you try if the patch below fixes the problem ?
>>>> It is a wild shot, but it might be worth a try.
>>>>
>>>> Thanks,
>>>> Guenter
>>>>
>>>> ---
>>>>   From b44c31c2c779a67827e3144b818cf21f5efacea1 Mon Sep 17 00:00:00 2001
>>>> From: Guenter Roeck <linux@roeck-us.net>
>>>> Date: Tue, 27 Jan 2026 15:32:32 -0800
>>>> Subject: [PATCH] hwmon: (spd5118) Explicitly enable temperature sensor in
>>>>    probe function
>>>>
>>>> Instantiating the driver does not make sense if the temperature sensor
>>>> is disabled, so enable it unconditionally in the probe function.
>>>>
>>>> If that fails, write operations to the chip are likely disabled
>>>> by the I2C controller. Bail out with an eror message if that happens.
>>>>
>>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>> ---
>>>>    drivers/hwmon/spd5118.c | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>>> index 5da44571b6a0..3e0e780014f9 100644
>>>> --- a/drivers/hwmon/spd5118.c
>>>> +++ b/drivers/hwmon/spd5118.c
>>>> @@ -552,6 +552,11 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
>>>>    	if (!spd5118_vendor_valid(bank, vendor))
>>>>    		return -ENODEV;
>>>>    
>>>> +	if (regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG,
>>>> +			       SPD5118_TS_DISABLE, 0) < 0)
>>>> +		return dev_err_probe(dev, -ENODEV,
>>>> +				     "Failed to enable temperature sensor\n");
>>>> +
>>>>    	data->regmap = regmap;
>>>>    	mutex_init(&data->nvmem_lock);
>>>>    	dev_set_drvdata(dev, data);
>>>
>>> Hi Guenter,
>>>
>>> I'm experiencing the same issue reported in this thread. This patch does
>>> not fix it for me :(.
>>>
>> Thanks for a note. Well, it was a wild shot, so it is not entirely surprising
>> that it didn't work. I suspect regmap doesn't actually write the register
>> if the value is unchanged. Another option might be to try writing a value
> 
> I tried forcing the write with
> 
> 	bool change;
> 	err = regmap_update_bits_base(regmap, SPD5118_REG_TEMP_CONFIG,
> 				      SPD5118_TS_DISABLE, 0, &change, false, true);
> 	if (err)
> 		return dev_err_probe(dev, err,
> 				     "Failed to enable temperature sensor\n");
> 
> and it fails to probe
> 
> 	spd5118 17-0051: error -ENXIO: Failed to enable temperature sensor
> 	spd5118 17-0053: error -ENXIO: Failed to enable temperature sensor
> 

Nice, I didn't know about that API function. And, even better, 'change' can be NULL.

> 
>> (e.g., 0x01) into register 0x13 or 0x14. Those are "clear status" registers.
>> If that fails, we would know that the chip is write protected.
> 
> I'll try this later too.
> 

Don't bother. I'll write up a patch using regmap_update_bits_base().

Thanks!
Guenter

>>
>> Thanks,
>> Guenter
> 

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Armin Wolf]

Am 31.01.26 um 03:06 schrieb Guenter Roeck:

> On 1/30/26 17:54, Kurt Borja wrote:
>> On Fri Jan 30, 2026 at 8:21 PM -05, Guenter Roeck wrote:
>>> Hi Kurt,
>>>
>>> On 1/30/26 16:55, Kurt Borja wrote:
>>>> On Tue Jan 27, 2026 at 6:41 PM -05, Guenter Roeck wrote:
>>>>> Hi,
>>>>>
>>>>> On Tue, Jan 27, 2026 at 10:23:24PM +0300, TINSAE TADESSE wrote:
>>>>>>
>>>>>> Disabling CONFIG_SENSORS_SPD5118_DETECT completely avoids the 
>>>>>> issue on
>>>>>> affected platforms,
>>>>>> even without any code changes. This confirms that the failures are
>>>>>> triggered specifically by automatic
>>>>>> SPD5118 instantiation on systems where the i801 controller enforces
>>>>>> SPD Write Disable.
>>>>>
>>>>> Thanks for confirming. Can you try if the patch below fixes the 
>>>>> problem ?
>>>>> It is a wild shot, but it might be worth a try.
>>>>>
>>>>> Thanks,
>>>>> Guenter
>>>>>
>>>>> ---
>>>>>   From b44c31c2c779a67827e3144b818cf21f5efacea1 Mon Sep 17 
>>>>> 00:00:00 2001
>>>>> From: Guenter Roeck <linux@roeck-us.net>
>>>>> Date: Tue, 27 Jan 2026 15:32:32 -0800
>>>>> Subject: [PATCH] hwmon: (spd5118) Explicitly enable temperature 
>>>>> sensor in
>>>>>    probe function
>>>>>
>>>>> Instantiating the driver does not make sense if the temperature 
>>>>> sensor
>>>>> is disabled, so enable it unconditionally in the probe function.
>>>>>
>>>>> If that fails, write operations to the chip are likely disabled
>>>>> by the I2C controller. Bail out with an eror message if that happens.
>>>>>
>>>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>>> ---
>>>>>    drivers/hwmon/spd5118.c | 5 +++++
>>>>>    1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>>>> index 5da44571b6a0..3e0e780014f9 100644
>>>>> --- a/drivers/hwmon/spd5118.c
>>>>> +++ b/drivers/hwmon/spd5118.c
>>>>> @@ -552,6 +552,11 @@ static int spd5118_common_probe(struct device 
>>>>> *dev, struct regmap *regmap,
>>>>>        if (!spd5118_vendor_valid(bank, vendor))
>>>>>            return -ENODEV;
>>>>>    +    if (regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG,
>>>>> +                   SPD5118_TS_DISABLE, 0) < 0)
>>>>> +        return dev_err_probe(dev, -ENODEV,
>>>>> +                     "Failed to enable temperature sensor\n");
>>>>> +
>>>>>        data->regmap = regmap;
>>>>>        mutex_init(&data->nvmem_lock);
>>>>>        dev_set_drvdata(dev, data);
>>>>
>>>> Hi Guenter,
>>>>
>>>> I'm experiencing the same issue reported in this thread. This patch 
>>>> does
>>>> not fix it for me :(.
>>>>
>>> Thanks for a note. Well, it was a wild shot, so it is not entirely 
>>> surprising
>>> that it didn't work. I suspect regmap doesn't actually write the 
>>> register
>>> if the value is unchanged. Another option might be to try writing a 
>>> value
>>
>> I tried forcing the write with
>>
>>     bool change;
>>     err = regmap_update_bits_base(regmap, SPD5118_REG_TEMP_CONFIG,
>>                       SPD5118_TS_DISABLE, 0, &change, false, true);
>>     if (err)
>>         return dev_err_probe(dev, err,
>>                      "Failed to enable temperature sensor\n");
>>
>> and it fails to probe
>>
>>     spd5118 17-0051: error -ENXIO: Failed to enable temperature sensor
>>     spd5118 17-0053: error -ENXIO: Failed to enable temperature sensor
>>
>
> Nice, I didn't know about that API function. And, even better, 
> 'change' can be NULL.
>
>>
>>> (e.g., 0x01) into register 0x13 or 0x14. Those are "clear status" 
>>> registers.
>>> If that fails, we would know that the chip is write protected.
>>
>> I'll try this later too.
>>
>
> Don't bother. I'll write up a patch using regmap_update_bits_base().
>
> Thanks!
> Guenter
>
Good idea, maybe regmap_write_bits() is exactly what we need for this.

Thanks,
Armin Wolf

>>>
>>> Thanks,
>>> Guenter
>>
>
>

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

On Wed, Jan 28, 2026 at 2:41 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> Hi,
>
> On Tue, Jan 27, 2026 at 10:23:24PM +0300, TINSAE TADESSE wrote:
> >
> > Disabling CONFIG_SENSORS_SPD5118_DETECT completely avoids the issue on
> > affected platforms,
> > even without any code changes. This confirms that the failures are
> > triggered specifically by automatic
> > SPD5118 instantiation on systems where the i801 controller enforces
> > SPD Write Disable.
>
> Thanks for confirming. Can you try if the patch below fixes the problem ?
> It is a wild shot, but it might be worth a try.
>
> Thanks,
> Guenter
>
> ---
> From b44c31c2c779a67827e3144b818cf21f5efacea1 Mon Sep 17 00:00:00 2001
> From: Guenter Roeck <linux@roeck-us.net>
> Date: Tue, 27 Jan 2026 15:32:32 -0800
> Subject: [PATCH] hwmon: (spd5118) Explicitly enable temperature sensor in
>  probe function
>
> Instantiating the driver does not make sense if the temperature sensor
> is disabled, so enable it unconditionally in the probe function.
>
> If that fails, write operations to the chip are likely disabled
> by the I2C controller. Bail out with an eror message if that happens.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/hwmon/spd5118.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index 5da44571b6a0..3e0e780014f9 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -552,6 +552,11 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
>         if (!spd5118_vendor_valid(bank, vendor))
>                 return -ENODEV;
>
> +       if (regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG,
> +                              SPD5118_TS_DISABLE, 0) < 0)
> +               return dev_err_probe(dev, -ENODEV,
> +                                    "Failed to enable temperature sensor\n");
> +
>         data->regmap = regmap;
>         mutex_init(&data->nvmem_lock);
>         dev_set_drvdata(dev, data);
> --
> 2.45.2
>

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 5da44571b6a0..ec77930bbc7a 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -552,6 +552,14 @@ static int spd5118_common_probe(struct device
*dev, struct regmap *regmap,
        if (!spd5118_vendor_valid(bank, vendor))
                return -ENODEV;

+       err = regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG,
SPD5118_TS_DISABLE, 0);
+       if (err < 0) {
+               dev_err(dev, "Failed to enable temperature sensor: %d\n", err);
+               return -ENODEV;
+       } else {
+               dev_info(dev, "Successfully enabled temperature
sensor. Return value is: %d\n", err);
+       }
+
        data->regmap = regmap;
        mutex_init(&data->nvmem_lock);
        dev_set_drvdata(dev, data);

Hi Guenter,

Thanks for the patch suggestion — I tested it on the affected system.

The probe-time write to SPD5118_REG_TEMP_CONFIG returns 0, and the
temperature sensor is enabled as expected. However, this does not
prevent the suspend/resume failures.

This is due to the missing 'force_write' flag passed from
regmap_update_bits() to the regmap_update_bits_base() function.
If this flag is missing or has a value of false, the corresponding call
to _regmap_update_bits() will just return the result of the preceding
register read operation (which will always work).


[   27.018685] i801_smbus 0000:00:1f.4: i801 access: command=1a,
size=8, addr=0x52, read_write=1
[   27.018709] spd5118 14-0052: Successfully enabled temperature
sensor. Return value is: 0
[   27.031170] spd5118 14-0052: DDR5 temperature sensor: vendor
0x00:0xb3 revision 2.2

Sorry for my late response!

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

On Tue, Jan 27, 2026 at 1:35 PM TINSAE TADESSE
<tinsaetadesse2015@gmail.com> wrote:
>
>
>
> On Mon, Jan 26, 2026 at 1:36 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > On 1/24/26 11:11, Armin Wolf wrote:
> > > Am 24.01.26 um 15:45 schrieb TINSAE TADESSE:
> > >
> > >> On Fri, Jan 16, 2026 at 9:24 AM Guenter Roeck <linux@roeck-us.net> wrote:
> > >>> On 1/15/26 05:50, TINSAE TADESSE wrote:
> > >>>> On Wed, Jan 14, 2026 at 5:23 PM Guenter Roeck <linux@roeck-us.net> wrote:
> > >>>>> On 1/14/26 05:07, TINSAE TADESSE wrote:
> > >>>>> ...
> > >>>>>>>> Hi Guenter,
> > >>>>>>>>
> > >>>>>>>> I tested changing the i801 SMBus controller to use
> > >>>>>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
> > >>>>>>>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
> > >>>>>>>> change, spd5118 resume failures (-ENXIO)
> > >>>>>>>> still persist, suggesting PM ordering alone is insufficient and other
> > >>>>>>>> firmware interactions are involved.
> > >>>>>>> How about the problem in the suspend function ? Is that also still seen ?
> > >>>>>>>
> > >>>>>>> Also, the subject talks about -EIO. Is that still seen ?
> > >>>>>>>
> > >>>>>>> Either case, can you enable debug logs for the i801 driver ?
> > >>>>>>> It should generate log entries when it reports errors.
> > >>>>>>>
> > >>>>>>> Thanks,
> > >>>>>>> Guenter
> > >>>>>>>
> > >>>>>> Hi Guenter,
> > >>>>>>
> > >>>>>> Thank you for the questions. To clarify:
> > >>>>>>
> > >>>>> Please do not drop mailing lists from replies.
> > >>>>>
> > >>>>>> 1) I have not observed any failures in the suspend path. The suspend
> > >>>>>> callback completes successfully, and
> > >>>>>> I have not seen I2C errors or warnings during suspend at any point.
> > >>>>> Sorry, I seem to be missing something.
> > >>>>>
> > >>>>> In that case, what is the point of patch 3/3 of your series which
> > >>>>> removes hardware accesses from the suspend function ?
> > >>>>>
> > >>>>>> 2) I have also not observed -EIO in my testing. The error consistently
> > >>>>>> reported on resume and subsequent hwmon access is -ENXIO.
> > >>>>>> Earlier references to -EIO were based on assumptions rather than
> > >>>>>> observed logs, and I should have been clearer about that.
> > >>>>>>
> > >>>>> Thanks for the clarification.
> > >>>>>
> > >>>>> Guenter
> > >>>>>
> > >>>>>> I am enabling debug logging for the i801 driver to collect more
> > >>>>>> concrete evidence of controller state during resume.
> > >>>> Hi Guenter,
> > >>>>
> > >>>>> Sorry, I seem to be missing something.
> > >>>>>
> > >>>>> In that case, what is the point of patch 3/3 of your series which
> > >>>>> removes hardware accesses from the suspend function ?
> > >>>> You are right to question this, and I agree that it needs clarification.
> > >>>>
> > >>>> Patch 3/3 was originally proposed under the assumption that the resume failures
> > >>>> were caused by spd5118 performing I2C transactions while the
> > >>>> controller was not yet available,
> > >>>> and that removing hardware accesses from the suspend path might
> > >>>> mitigate the issue.
> > >>>> At that point, I assumed the problem was limited to the resume callback.
> > >>>>
> > >>>> After enabling detailed i801 debug logging and testing with
> > >>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() in the i801 driver,
> > >>>> it became clear that this assumption was incorrect. The controller
> > >>>> itself reports "i801_smbus: No response"
> > >>>> both during suspend and immediately after resume, and spd5118 merely
> > >>>> propagates the resulting -ENXIO.
> > >>> Outch, that really hurts, because it means that something is seriously
> > >>> broken in both the suspend and resume path. The device _must_ be accessible
> > >>> in the suspend path. Otherwise there is no guarantee that the device is
> > >>> accessible for normal (pre-suspend) operation. After all, someone could
> > >>> run a script reading sysfs attributes in a tight loop continuously,
> > >>> or the thermal subsystem could try to access the chip. That would suddenly
> > >>> start to fail if something in the device access path starts to be suspended
> > >>> while the underlying hardware is still believed to be operational.
> > >>>
> > >>> I could imagine some hack/quirk for the resume path, such as delaying resume
> > >>> for some period of time for affected hardware, but I have no idea what to
> > >>> do on the suspend side. We can not just drop device writes during suspend
> > >>> because some broken hardware/firmware does not let us actually access
> > >>> (and thus suspend) the hardware anymore by the time the suspend function
> > >>> is called.
> > >>>
> > >>> Guenter
> > >>>
> > >>>> This indicates that the issue is not caused by spd5118 suspend/resume
> > >>>> behavior, but by the unavailability of the
> > >>>> SMBus controller due to platform or firmware interactions during
> > >>>> s2idle transitions.
> > >>>>
> > >>>> Given this, I agree that patch 3/3 does not address the root cause and
> > >>>> does not provide a justified improvement.
> > >>>> I am therefore fine with dropping it.
> > >>>>
> > >>>> Thank you for pointing this out.
> > >>>>
> > >> Hi Guenter,
> > >>
> > >> Thanks for the continued review and for questioning the earlier
> > >> direction — that helped narrow this down properly.
> > >>
> > >> After enabling full i801 debug logging (included below in this email)
> > >> and inspecting both drivers, it became clear that the resume
> > >> failures are not caused by spd5118 accessing the hardware too
> > >> early, nor by PM ordering issues. Instead, the SMBus controller
> > >> explicitly reports “SPD Write Disable is set”, and any
> > >> block write transactions to the SPD device consistently fail with
> > >> DEV_ERR. spd5118 merely propagates the resulting -ENXIO.
> > >
> > > Oh no, this likely happens even when merely reading values, as the spd5118
> > > uses a page register to switch between different register pages. In order
> > > to access temperature data (page 0), you might already have to issue a
> > > write access to the page register. The only reason why it works for you
> > > is that the spd5118 likely already has page 0 selected by the system firmware
> > > during boot.
> > >
> >
> > Exactly. There is no guarantee that page 0 is selected.
> >
> > >> With that in mind, I have dropped the earlier patch that attempted
> > >> to remove hardware access from the suspend path
> > >> unconditionally.
> > >> That patch does not address the root cause and is no longer
> > >> part of the series.
> > >>
> > >> I am instead proposing a minimal 2-patch series:
> > >>
> > >> 1/2 records whether the platform enforces SPD write disable at probe
> > >> time (no behavior change).
> > >> 2/2 avoids regcache writeback during suspend/resume when the device
> > >> operates in read-only mode, while still allowing read access to
> > >> temperature inputs.
> > >>
> > >> This avoids issuing SMBus transactions that are architecturally
> > >> blocked on these systems, and does not rely on
> > >> delays or PM ordering assumptions, and leaves behavior unchanged on
> > >> platforms where SPD writes are permitted.
> > >>
> > >> If this direction looks acceptable, I’m happy to re-spin and post the
> > >> series formally.
> > >>
> > >> Thanks again for the guidance.
> > >
> > > I do not know if this is a reliable solution, as the system firmware might
> > > select a different register page during resume. This will then prevent the
> > > driver from functioning.
> > >
> >
> > No, it is not reliable. The driver is simply not usable in this scenario.
> > This isn't just the temperature sensor code - the eeprom code is affected
> > as well.
> >
> > > I would love to see the spd5118 driver working on such systems with reduced
> > > functionality, but i will leave it to Guenter to decide if this approach is
> > > maintainable.
> > >
> > > Besides that: did the spd5118 driver load automatically on your device?
> > >
> >
> > I thought that was disabled. The i801 driver is supposed to detect if write
> > protect is enabled and, if so, it is supposed to not instantiate the spd5118
> > driver for DDR3. Support for this was added with commit 4d6d35d3417d ("i2c:
> > smbus: introduce Write Disable-aware SPD instantiating functions"). Apparently
> > the code to do this never made it into the i801 driver.
> >
> > The i801 driver needs to be fixed to inform the spd initialization code
> > that the spd5118 address range is write protected. The patch to do this was
> > "i2c: i801: Do not instantiate spd5118 under SPD Write Disable". I have no idea
> > why that patch didn't make it upstream.
> >
> > Guenter
> >
>
> Hi Guenter,
>
> > > The i801 driver needs to be fixed to inform the spd initialization code
> > > that the spd5118 address range is write protected. The patch to do
> > > this was
> > > "i2c: i801: Do not instantiate spd5118 under SPD Write Disable". I
> > > have no idea
> > > why that patch didn't make it upstream.
>
> I initially considered exposing SPD write-protection as a capability to be consumed by spd5118.
> However, spd5118 already depends on firmware-initialized state (page selection) and cannot reliably
> determine safe operation at probe time without issuing writes.
> Given that, suppressing SPD instantiation in the i801 driver when SPD Write Disable is
> the only solution here.
>
> Tested the remaining patch [1], and it does not seem to fix the issue, as it is added at the wrong stage
> in the initialization of the i801 driver. The attached log shows that spd5118 is instantiated and fully probed
> before the "do not instantiate spd5118 under SPD Write Disable" is acted upon.
> This confirms that suppressing instantiation must occur before adapter registration in i801.
> Once the adapter is registered, the SPD scan has already happened, and spd5118 has bound successfully
> based on the firmware-initialized state, even though correct operation is impossible.
> Therefore, the fix must be run prior to adapter registration.
>
> [1] https://lore.kernel.org/all/20250430-for-upstream-i801-spd5118-no-instantiate-v2-2-2f54d91ae2c7@canonical.com/
>
>
> The fix is to register the SPD write-disable policy before i2c_add_adapter(), so the I2C core never
> probes SPD addresses on write-protected platforms.
>


Hi Guenter,

While investigating this issue, I previously mentioned
about a flow where SPD write disabled state can be
exported as a capability from the i801 controller, so
that the SPD5118 hwmon driver consumes it.

The SPD write disabled state is known to the controller
driver (i2c-i801), but this information is not
propagated to client drivers. As a result, auto-detected
devices may be instantiated and probed even though the
controller cannot support the required access model.

This raises a major architectural question:

Should SMBus / I2C controller drivers be able to
advertise bus-level capability constraints (such as SPD
write disabled state) to client drivers, so that clients can
make an informed decision during probe?

A capability-based approach would allow:
* controller drivers to expose what is possible on a given bus
* client drivers to decide whether they can operate correctly
* avoidance of device-specific policy in controller drivers
* consistent handling across different SPD-capable devices

I actually tested the possibility of detecting, propagating,
and consuming the SPD write disabled state using an I2C
adapter capability flag. Using this approach, I was able to
fix the issue even with the CONFIG_SENSORS_SPD5118_DETECT
kernel configuration enabled.

At this stage, I am not proposing a specific implementation.
The goal of this RFC is to get agreement on whether this type
of problem should be solved through capability propagation,
and if so, what mechanism would be preferred.

Any feedback on design direction, or existing infrastructure
that could be reused would be very welcome.

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/31/26 11:50, TINSAE TADESSE wrote:
...
> 
> Hi Guenter,
> 
> While investigating this issue, I previously mentioned
> about a flow where SPD write disabled state can be
> exported as a capability from the i801 controller, so
> that the SPD5118 hwmon driver consumes it.
> 
> The SPD write disabled state is known to the controller
> driver (i2c-i801), but this information is not
> propagated to client drivers. As a result, auto-detected
> devices may be instantiated and probed even though the
> controller cannot support the required access model.
> 
> This raises a major architectural question:
> 
> Should SMBus / I2C controller drivers be able to
> advertise bus-level capability constraints (such as SPD
> write disabled state) to client drivers, so that clients can
> make an informed decision during probe?
> 
> A capability-based approach would allow:
> * controller drivers to expose what is possible on a given bus
> * client drivers to decide whether they can operate correctly
> * avoidance of device-specific policy in controller drivers
> * consistent handling across different SPD-capable devices
> 
> I actually tested the possibility of detecting, propagating,
> and consuming the SPD write disabled state using an I2C
> adapter capability flag. Using this approach, I was able to
> fix the issue even with the CONFIG_SENSORS_SPD5118_DETECT
> kernel configuration enabled.
> 
> At this stage, I am not proposing a specific implementation.
> The goal of this RFC is to get agreement on whether this type
> of problem should be solved through capability propagation,
> and if so, what mechanism would be preferred.
> 
> Any feedback on design direction, or existing infrastructure
> that could be reused would be very welcome.
> 

I think it is a good idea, but how would the flag look like ?
The i801 controller only write protects a range of addresses;
I think it is 0x50..0x57. So any I2C_FUNC flag would presumably
have to be address range specific. You could try adding
something like I2C_FUNC_SPD_WRITE_PROTECTED. Either case,
you'll have to ask the I2C subsystem maintainers for advice.
I would suggest to give it a try.

Thanks,
Guenter

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

[-- Attachment #1: Type: text/plain, Size: 2677 bytes --]

On Sun, Feb 1, 2026 at 4:25 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 1/31/26 11:50, TINSAE TADESSE wrote:
> ...
> >
> > Hi Guenter,
> >
> > While investigating this issue, I previously mentioned
> > about a flow where SPD write disabled state can be
> > exported as a capability from the i801 controller, so
> > that the SPD5118 hwmon driver consumes it.
> >
> > The SPD write disabled state is known to the controller
> > driver (i2c-i801), but this information is not
> > propagated to client drivers. As a result, auto-detected
> > devices may be instantiated and probed even though the
> > controller cannot support the required access model.
> >
> > This raises a major architectural question:
> >
> > Should SMBus / I2C controller drivers be able to
> > advertise bus-level capability constraints (such as SPD
> > write disabled state) to client drivers, so that clients can
> > make an informed decision during probe?
> >
> > A capability-based approach would allow:
> > * controller drivers to expose what is possible on a given bus
> > * client drivers to decide whether they can operate correctly
> > * avoidance of device-specific policy in controller drivers
> > * consistent handling across different SPD-capable devices
> >
> > I actually tested the possibility of detecting, propagating,
> > and consuming the SPD write disabled state using an I2C
> > adapter capability flag. Using this approach, I was able to
> > fix the issue even with the CONFIG_SENSORS_SPD5118_DETECT
> > kernel configuration enabled.
> >
> > At this stage, I am not proposing a specific implementation.
> > The goal of this RFC is to get agreement on whether this type
> > of problem should be solved through capability propagation,
> > and if so, what mechanism would be preferred.
> >
> > Any feedback on design direction, or existing infrastructure
> > that could be reused would be very welcome.
> >
>
> I think it is a good idea, but how would the flag look like ?
> The i801 controller only write protects a range of addresses;
> I think it is 0x50..0x57. So any I2C_FUNC flag would presumably
> have to be address range specific. You could try adding
> something like I2C_FUNC_SPD_WRITE_PROTECTED. Either case,
> you'll have to ask the I2C subsystem maintainers for advice.
> I would suggest to give it a try.
>
> Thanks,
> Guenter
>

Hello Guenter,

Thanks for the feedback.
I have attached one implementation that I tested and confirmed to
resolve the issue.
I will forward an RFC to the i2c maintainers, and if you don't mind,
I will also add you to the email list.

Thanks again for the guidance.

[-- Attachment #2: 0002-PATCH-2-2-hwmon-spd5118-Fail-probe-if-SPD-writes-are.patch --]
[-- Type: text/x-patch, Size: 1908 bytes --]

From e4fa15d9a667b46057f98092a46dc60a99f547fb Mon Sep 17 00:00:00 2001
From: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
Date: Sun, 1 Feb 2026 09:40:24 +0300
Subject: [PATCH 2/2] [PATCH 2/2] hwmon: spd5118: Fail probe if SPD writes are
 disabled

SPD5118 requires write access for page selection, configuration,
and cache synchronization during suspend/resume. If the host
controller does not allow SPD writes, the driver cannot function
properly.

Detect this state using adapter quirks and determine whether to
stop the probe.

Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
---
 drivers/hwmon/spd5118.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 5da44571b6a0..10a6dfb30985 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -525,6 +525,8 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
 	unsigned int capability, revision, vendor, bank;
 	struct spd5118_data *data;
 	struct device *hwmon_dev;
+	struct i2c_client *client;
+	const struct i2c_adapter_quirks *quirks;
 	int err;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
@@ -552,6 +554,20 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
 	if (!spd5118_vendor_valid(bank, vendor))
 		return -ENODEV;
 
+	/*
+	 * SPD5118 requires write access for correct operation
+	 * (page selection, configuration, and suspend/resume cache sync).
+	 * If the SPD writes are blocked by the SMBus controller, the
+	 * probe fails.
+	 */
+	client = to_i2c_client(dev);
+	quirks = client->adapter->quirks;
+	if (quirks && (quirks->flags & I2C_AQ_SPD_WRITE_DISABLED)) {
+		dev_err_probe(dev, -ENODEV, "SPD Write Disable is set on adapter; refusing probe\n");
+		return -ENODEV;
+	}
+
+
 	data->regmap = regmap;
 	mutex_init(&data->nvmem_lock);
 	dev_set_drvdata(dev, data);
-- 
2.47.3


[-- Attachment #3: 0001-PATCH-1-2-i2c-i801-Detect-SPD-Write-Disable-and-expo.patch --]
[-- Type: text/x-patch, Size: 2474 bytes --]

From f232d9710bd0d1cf290a28234bc98e4224101916 Mon Sep 17 00:00:00 2001
From: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
Date: Sun, 1 Feb 2026 08:14:04 +0300
Subject: [PATCH 1/2] [PATCH 1/2] i2c: i801: Detect SPD Write Disable and
 expose as adapter quirk

Detect SPD Write Disable in SMBHSTCFG and expose it through
I2C adapter quirk. The I2C client driver may decide whether
SPD write operations are supported without implementing
device-specific policies in the SMBus controller driver.

Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
---
 drivers/i2c/busses/i2c-i801.c | 16 +++++++++++++++-
 include/linux/i2c.h           |  3 +++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 9e1789725edf..d771e9f5f82f 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1533,6 +1533,11 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	int err, i, bar = SMBBAR;
 	struct i801_priv *priv;
+	struct i2c_adapter_quirks *quirks;
+
+	quirks = devm_kzalloc(&dev->dev, sizeof(*quirks), GFP_KERNEL);
+	if (!quirks)
+		return -ENOMEM;
 
 	priv = devm_kzalloc(&dev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -1600,8 +1605,17 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		/* Disable SMBus interrupt feature if SMBus using SMI# */
 		priv->features &= ~FEATURE_IRQ;
 	}
-	if (priv->original_hstcfg & SMBHSTCFG_SPD_WD)
+
+	/*
+	 * Detect the SPD Write Disabled status. Mark the adapter
+	 * as unable to perform SPD writes, which allows consuming
+	 * drivers to decide on safe operation.
+	 */
+	if (priv->original_hstcfg & SMBHSTCFG_SPD_WD) {
 		pci_info(dev, "SPD Write Disable is set\n");
+		quirks->flags |= I2C_AQ_SPD_WRITE_DISABLED;
+	}
+	priv->adapter.quirks = quirks;
 
 	/* Clear special mode bits */
 	if (priv->features & (FEATURE_SMBUS_PEC | FEATURE_BLOCK_BUFFER))
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 20fd41b51d5c..4b89f0bf62a1 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -726,6 +726,9 @@ struct i2c_adapter_quirks {
 /* adapter cannot do repeated START */
 #define I2C_AQ_NO_REP_START		BIT(7)
 
+/* SPD writes are blocked by host controller */
+#define I2C_AQ_SPD_WRITE_DISABLED	BIT(8)
+
 /*
  * i2c_adapter is the structure used to identify a physical i2c bus along
  * with the access algorithms necessary to access it.
-- 
2.47.3

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[Guenter Roeck]

On 1/31/26 23:30, TINSAE TADESSE wrote:
> On Sun, Feb 1, 2026 at 4:25 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 1/31/26 11:50, TINSAE TADESSE wrote:
>> ...
>>>
>>> Hi Guenter,
>>>
>>> While investigating this issue, I previously mentioned
>>> about a flow where SPD write disabled state can be
>>> exported as a capability from the i801 controller, so
>>> that the SPD5118 hwmon driver consumes it.
>>>
>>> The SPD write disabled state is known to the controller
>>> driver (i2c-i801), but this information is not
>>> propagated to client drivers. As a result, auto-detected
>>> devices may be instantiated and probed even though the
>>> controller cannot support the required access model.
>>>
>>> This raises a major architectural question:
>>>
>>> Should SMBus / I2C controller drivers be able to
>>> advertise bus-level capability constraints (such as SPD
>>> write disabled state) to client drivers, so that clients can
>>> make an informed decision during probe?
>>>
>>> A capability-based approach would allow:
>>> * controller drivers to expose what is possible on a given bus
>>> * client drivers to decide whether they can operate correctly
>>> * avoidance of device-specific policy in controller drivers
>>> * consistent handling across different SPD-capable devices
>>>
>>> I actually tested the possibility of detecting, propagating,
>>> and consuming the SPD write disabled state using an I2C
>>> adapter capability flag. Using this approach, I was able to
>>> fix the issue even with the CONFIG_SENSORS_SPD5118_DETECT
>>> kernel configuration enabled.
>>>
>>> At this stage, I am not proposing a specific implementation.
>>> The goal of this RFC is to get agreement on whether this type
>>> of problem should be solved through capability propagation,
>>> and if so, what mechanism would be preferred.
>>>
>>> Any feedback on design direction, or existing infrastructure
>>> that could be reused would be very welcome.
>>>
>>
>> I think it is a good idea, but how would the flag look like ?
>> The i801 controller only write protects a range of addresses;
>> I think it is 0x50..0x57. So any I2C_FUNC flag would presumably
>> have to be address range specific. You could try adding
>> something like I2C_FUNC_SPD_WRITE_PROTECTED. Either case,
>> you'll have to ask the I2C subsystem maintainers for advice.
>> I would suggest to give it a try.
>>
>> Thanks,
>> Guenter
>>
> 
> Hello Guenter,
> 
> Thanks for the feedback.
> I have attached one implementation that I tested and confirmed to
> resolve the issue.
> I will forward an RFC to the i2c maintainers, and if you don't mind,
> I will also add you to the email list.
> 
> Thanks again for the guidance.

Looks good. I didn't know about the quirks, and that drivers are allowed to use them.
We live and learn. Go ahead, and please copy the hardware monitoring mailing list.
I'll hold off sending my patch upstream until we get a response; your solution is
much cleaner.

Thanks,
Guenter

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

[-- Attachment #1: Type: text/plain, Size: 4873 bytes --]

On Sun, Feb 1, 2026 at 6:21 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 1/31/26 23:30, TINSAE TADESSE wrote:
> > On Sun, Feb 1, 2026 at 4:25 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> On 1/31/26 11:50, TINSAE TADESSE wrote:
> >> ...
> >>>
> >>> Hi Guenter,
> >>>
> >>> While investigating this issue, I previously mentioned
> >>> about a flow where SPD write disabled state can be
> >>> exported as a capability from the i801 controller, so
> >>> that the SPD5118 hwmon driver consumes it.
> >>>
> >>> The SPD write disabled state is known to the controller
> >>> driver (i2c-i801), but this information is not
> >>> propagated to client drivers. As a result, auto-detected
> >>> devices may be instantiated and probed even though the
> >>> controller cannot support the required access model.
> >>>
> >>> This raises a major architectural question:
> >>>
> >>> Should SMBus / I2C controller drivers be able to
> >>> advertise bus-level capability constraints (such as SPD
> >>> write disabled state) to client drivers, so that clients can
> >>> make an informed decision during probe?
> >>>
> >>> A capability-based approach would allow:
> >>> * controller drivers to expose what is possible on a given bus
> >>> * client drivers to decide whether they can operate correctly
> >>> * avoidance of device-specific policy in controller drivers
> >>> * consistent handling across different SPD-capable devices
> >>>
> >>> I actually tested the possibility of detecting, propagating,
> >>> and consuming the SPD write disabled state using an I2C
> >>> adapter capability flag. Using this approach, I was able to
> >>> fix the issue even with the CONFIG_SENSORS_SPD5118_DETECT
> >>> kernel configuration enabled.
> >>>
> >>> At this stage, I am not proposing a specific implementation.
> >>> The goal of this RFC is to get agreement on whether this type
> >>> of problem should be solved through capability propagation,
> >>> and if so, what mechanism would be preferred.
> >>>
> >>> Any feedback on design direction, or existing infrastructure
> >>> that could be reused would be very welcome.
> >>>
> >>
> >> I think it is a good idea, but how would the flag look like ?
> >> The i801 controller only write protects a range of addresses;
> >> I think it is 0x50..0x57. So any I2C_FUNC flag would presumably
> >> have to be address range specific. You could try adding
> >> something like I2C_FUNC_SPD_WRITE_PROTECTED. Either case,
> >> you'll have to ask the I2C subsystem maintainers for advice.
> >> I would suggest to give it a try.
> >>
> >> Thanks,
> >> Guenter
> >>
> >
> > Hello Guenter,
> >
> > Thanks for the feedback.
> > I have attached one implementation that I tested and confirmed to
> > resolve the issue.
> > I will forward an RFC to the i2c maintainers, and if you don't mind,
> > I will also add you to the email list.
> >
> > Thanks again for the guidance.
>
> Looks good. I didn't know about the quirks, and that drivers are allowed to use them.
> We live and learn. Go ahead, and please copy the hardware monitoring mailing list.
> I'll hold off sending my patch upstream until we get a response; your solution is
> much cleaner.
>
> Thanks,
> Guenter
>


Hi I2C maintainers,

I am working on a platform where the Intel i801 SMBus controller
exposes a hardware feature called "SPD Write Disable". When
enabled, the controller blocks all write transactions to the
SPD EEPROM address range (typically 0x50–0x57), while still
allowing reads.

This creates a problem for the spd5118 hwmon driver, as SPD5118
requires write access for correct operation (page selection,
configuration, suspend/resume cache synchronization). If the controller
silently rejects writes, the driver can misbehave and generate
repeated SMBus DEV_ERR storms.

Currently, the I2C core does not provide a mechanism to expose
write restrictions. Rather than embedding SPD5118-specific policy
in the i801 driver, we are exploring a __bus-level capability/quirk__
that allows consumer drivers to decide whether they can operate safely.

Conceptually:
1. The SMBus controller (i801) detects SPD-WD in hardware
2. The controller exposes this as an adapter-level quirk or capability
3. The I2C core propagates this to I2C clients (already handled by i2c-core)
4. Drivers such as `spd5118` can refuse probe if write access is required

We have implemented and tested this approach (open to guidance) using
a newly defined 'I2C_AQ_SPD_WRITE_DISABLED' quirk flag. The goal is
to keep controller drivers free of device-specific policy, while giving client
drivers enough information to fail probe cleanly instead of
misbehaving at runtime.

If this approach sounds reasonable, please review the attached patch series.

Thanks for your time and guidance.

[-- Attachment #2: 0002-hwmon-spd5118-Fail-probe-if-SPD-writes-are-disabled.patch --]
[-- Type: text/x-patch, Size: 1892 bytes --]

From a7a6f6e725b02c2bf99db3c65abb22d390146723 Mon Sep 17 00:00:00 2001
From: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
Date: Mon, 2 Feb 2026 09:41:37 +0300
Subject: [PATCH 2/2] hwmon: spd5118: Fail probe if SPD writes are disabled

SPD5118 requires write access for page selection, configuration,
and cache synchronization during suspend/resume. If the host
controller does not allow SPD writes, the driver cannot function
properly.

Detect this state using adapter quirks and determine whether to
stop the probe.

Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
---
 drivers/hwmon/spd5118.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 5da44571b6a0..094d05472562 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -525,6 +525,8 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
 	unsigned int capability, revision, vendor, bank;
 	struct spd5118_data *data;
 	struct device *hwmon_dev;
+	struct i2c_client *client;
+	const struct i2c_adapter_quirks *quirks;
 	int err;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
@@ -552,6 +554,19 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
 	if (!spd5118_vendor_valid(bank, vendor))
 		return -ENODEV;
 
+	/*
+	 * SPD5118 requires write access for correct operation
+	 * (page selection, configuration, and suspend/resume cache sync).
+	 * If the SPD writes are blocked by the SMBus controller, the
+	 * probe fails.
+	 */
+	client = to_i2c_client(dev);
+	quirks = client->adapter->quirks;
+	if (quirks && (quirks->flags & I2C_AQ_SPD_WRITE_DISABLED)) {
+		dev_err_probe(dev, -ENODEV, "SPD Write Disable is set on adapter; refusing probe\n");
+		return -ENODEV;
+	}
+
 	data->regmap = regmap;
 	mutex_init(&data->nvmem_lock);
 	dev_set_drvdata(dev, data);
-- 
2.47.3


[-- Attachment #3: 0001-i2c-i801-Detect-SPD-Write-Disable-and-expose-as-adap.patch --]
[-- Type: text/x-patch, Size: 2462 bytes --]

From 6d93cc581becf9b0c6ac8c1bd35ad2cd993a82d7 Mon Sep 17 00:00:00 2001
From: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
Date: Mon, 2 Feb 2026 09:40:32 +0300
Subject: [PATCH 1/2] i2c: i801: Detect SPD Write Disable and expose as adapter
 quirk

Detect SPD Write Disable in SMBHSTCFG and expose it through
I2C adapter quirk. The I2C client driver may decide whether
SPD write operations are supported without implementing
device-specific policies in the SMBus controller driver.

Signed-off-by: Tinsae Tadesse <tinsaetadesse2015@gmail.com>
---
 drivers/i2c/busses/i2c-i801.c | 16 +++++++++++++++-
 include/linux/i2c.h           |  3 +++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 9e1789725edf..d771e9f5f82f 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1533,6 +1533,11 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	int err, i, bar = SMBBAR;
 	struct i801_priv *priv;
+	struct i2c_adapter_quirks *quirks;
+
+	quirks = devm_kzalloc(&dev->dev, sizeof(*quirks), GFP_KERNEL);
+	if (!quirks)
+		return -ENOMEM;
 
 	priv = devm_kzalloc(&dev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -1600,8 +1605,17 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		/* Disable SMBus interrupt feature if SMBus using SMI# */
 		priv->features &= ~FEATURE_IRQ;
 	}
-	if (priv->original_hstcfg & SMBHSTCFG_SPD_WD)
+
+	/*
+	 * Detect the SPD Write Disabled status. Mark the adapter
+	 * as unable to perform SPD writes, which allows consuming
+	 * drivers to decide on safe operation.
+	 */
+	if (priv->original_hstcfg & SMBHSTCFG_SPD_WD) {
 		pci_info(dev, "SPD Write Disable is set\n");
+		quirks->flags |= I2C_AQ_SPD_WRITE_DISABLED;
+	}
+	priv->adapter.quirks = quirks;
 
 	/* Clear special mode bits */
 	if (priv->features & (FEATURE_SMBUS_PEC | FEATURE_BLOCK_BUFFER))
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 20fd41b51d5c..4b89f0bf62a1 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -726,6 +726,9 @@ struct i2c_adapter_quirks {
 /* adapter cannot do repeated START */
 #define I2C_AQ_NO_REP_START		BIT(7)
 
+/* SPD writes are blocked by host controller */
+#define I2C_AQ_SPD_WRITE_DISABLED	BIT(8)
+
 /*
  * i2c_adapter is the structure used to identify a physical i2c bus along
  * with the access algorithms necessary to access it.
-- 
2.47.3

Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors

[TINSAE TADESSE]

On Sat, Jan 24, 2026 at 10:11 PM Armin Wolf <W_Armin@gmx.de> wrote:
>
> Am 24.01.26 um 15:45 schrieb TINSAE TADESSE:
>
> > On Fri, Jan 16, 2026 at 9:24 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >> On 1/15/26 05:50, TINSAE TADESSE wrote:
> >>> On Wed, Jan 14, 2026 at 5:23 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>>> On 1/14/26 05:07, TINSAE TADESSE wrote:
> >>>> ...
> >>>>>>> Hi Guenter,
> >>>>>>>
> >>>>>>> I tested changing the i801 SMBus controller to use
> >>>>>>> SET_LATE_SYSTEM_SLEEP_PM_OPS() instead of
> >>>>>>> DEFINE_SIMPLE_DEV_PM_OPS() as a diagnostic experiment. With this
> >>>>>>> change, spd5118 resume failures (-ENXIO)
> >>>>>>> still persist, suggesting PM ordering alone is insufficient and other
> >>>>>>> firmware interactions are involved.
> >>>>>> How about the problem in the suspend function ? Is that also still seen ?
> >>>>>>
> >>>>>> Also, the subject talks about -EIO. Is that still seen ?
> >>>>>>
> >>>>>> Either case, can you enable debug logs for the i801 driver ?
> >>>>>> It should generate log entries when it reports errors.
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Guenter
> >>>>>>
> >>>>> Hi Guenter,
> >>>>>
> >>>>> Thank you for the questions. To clarify:
> >>>>>
> >>>> Please do not drop mailing lists from replies.
> >>>>
> >>>>> 1) I have not observed any failures in the suspend path. The suspend
> >>>>> callback completes successfully, and
> >>>>> I have not seen I2C errors or warnings during suspend at any point.
> >>>> Sorry, I seem to be missing something.
> >>>>
> >>>> In that case, what is the point of patch 3/3 of your series which
> >>>> removes hardware accesses from the suspend function ?
> >>>>
> >>>>> 2) I have also not observed -EIO in my testing. The error consistently
> >>>>> reported on resume and subsequent hwmon access is -ENXIO.
> >>>>> Earlier references to -EIO were based on assumptions rather than
> >>>>> observed logs, and I should have been clearer about that.
> >>>>>
> >>>> Thanks for the clarification.
> >>>>
> >>>> Guenter
> >>>>
> >>>>> I am enabling debug logging for the i801 driver to collect more
> >>>>> concrete evidence of controller state during resume.
> >>> Hi Guenter,
> >>>
> >>>> Sorry, I seem to be missing something.
> >>>>
> >>>> In that case, what is the point of patch 3/3 of your series which
> >>>> removes hardware accesses from the suspend function ?
> >>> You are right to question this, and I agree that it needs clarification.
> >>>
> >>> Patch 3/3 was originally proposed under the assumption that the resume failures
> >>> were caused by spd5118 performing I2C transactions while the
> >>> controller was not yet available,
> >>> and that removing hardware accesses from the suspend path might
> >>> mitigate the issue.
> >>> At that point, I assumed the problem was limited to the resume callback.
> >>>
> >>> After enabling detailed i801 debug logging and testing with
> >>> SET_LATE_SYSTEM_SLEEP_PM_OPS() in the i801 driver,
> >>> it became clear that this assumption was incorrect. The controller
> >>> itself reports "i801_smbus: No response"
> >>> both during suspend and immediately after resume, and spd5118 merely
> >>> propagates the resulting -ENXIO.
> >> Outch, that really hurts, because it means that something is seriously
> >> broken in both the suspend and resume path. The device _must_ be accessible
> >> in the suspend path. Otherwise there is no guarantee that the device is
> >> accessible for normal (pre-suspend) operation. After all, someone could
> >> run a script reading sysfs attributes in a tight loop continuously,
> >> or the thermal subsystem could try to access the chip. That would suddenly
> >> start to fail if something in the device access path starts to be suspended
> >> while the underlying hardware is still believed to be operational.
> >>
> >> I could imagine some hack/quirk for the resume path, such as delaying resume
> >> for some period of time for affected hardware, but I have no idea what to
> >> do on the suspend side. We can not just drop device writes during suspend
> >> because some broken hardware/firmware does not let us actually access
> >> (and thus suspend) the hardware anymore by the time the suspend function
> >> is called.
> >>
> >> Guenter
> >>
> >>> This indicates that the issue is not caused by spd5118 suspend/resume
> >>> behavior, but by the unavailability of the
> >>> SMBus controller due to platform or firmware interactions during
> >>> s2idle transitions.
> >>>
> >>> Given this, I agree that patch 3/3 does not address the root cause and
> >>> does not provide a justified improvement.
> >>> I am therefore fine with dropping it.
> >>>
> >>> Thank you for pointing this out.
> >>>
> > Hi Guenter,
> >
> > Thanks for the continued review and for questioning the earlier
> > direction — that helped narrow this down properly.
> >
> > After enabling full i801 debug logging (included below in this email)
> > and inspecting both drivers, it became clear that the resume
> > failures are not caused by spd5118 accessing the hardware too
> > early, nor by PM ordering issues. Instead, the SMBus controller
> > explicitly reports “SPD Write Disable is set”, and any
> > block write transactions to the SPD device consistently fail with
> > DEV_ERR. spd5118 merely propagates the resulting -ENXIO.
>
> Oh no, this likely happens even when merely reading values, as the spd5118
> uses a page register to switch between different register pages. In order
> to access temperature data (page 0), you might already have to issue a
> write access to the page register. The only reason why it works for you
> is that the spd5118 likely already has page 0 selected by the system firmware
> during boot.
>
> > With that in mind, I have dropped the earlier patch that attempted
> > to remove hardware access from the suspend path
> > unconditionally.
> > That patch does not address the root cause and is no longer
> > part of the series.
> >
> > I am instead proposing a minimal 2-patch series:
> >
> > 1/2 records whether the platform enforces SPD write disable at probe
> > time (no behavior change).
> > 2/2 avoids regcache writeback during suspend/resume when the device
> > operates in read-only mode, while still allowing read access to
> > temperature inputs.
> >
> > This avoids issuing SMBus transactions that are architecturally
> > blocked on these systems, and does not rely on
> > delays or PM ordering assumptions, and leaves behavior unchanged on
> > platforms where SPD writes are permitted.
> >
> > If this direction looks acceptable, I’m happy to re-spin and post the
> > series formally.
> >
> > Thanks again for the guidance.
>
> I do not know if this is a reliable solution, as the system firmware might
> select a different register page during resume. This will then prevent the
> driver from functioning.
>
> I would love to see the spd5118 driver working on such systems with reduced
> functionality, but i will leave it to Guenter to decide if this approach is
> maintainable.
>
> Besides that: did the spd5118 driver load automatically on your device?
>
> Thanks,
> Armin Wolf
>
> >
> >
> > [   13.530830] i2c-core: driver [spd5118] registered
> > ...
> > [   29.555298] i801_smbus 0000:00:1f.4: SPD Write Disable is set
> > ...
> > [   29.582447] i2c i2c-14: Creating spd5118 at 0x52
> > [   29.590744] spd5118 14-0052: probe
> > ...
> > [   29.618983] spd5118 14-0052: DDR5 temperature sensor: vendor
> > 0x00:0xb3 revision 2.2
> > [   29.619662] i2c i2c-14: client [spd5118] registered with bus id 14-0052
> > ...
> > [ 1057.504362] PM: suspend entry (s2idle)
> > [ 1057.944405] spd5118 14-0052: Entering suspend path...
> > [ 1057.945387] i801_smbus 0000:00:1f.4: i801 access: command=1a,
> > size=8, addr=0x52, read_write=1
> > [ 1057.946251] i801_smbus 0000:00:1f.4: i801 access: command=b,
> > size=8, addr=0x52, read_write=1
> > [ 1057.946866] i801_smbus 0000:00:1f.4: i801 access: command=1a,
> > size=8, addr=0x52, read_write=1
> > [ 1057.948324] i801_smbus 0000:00:1f.4: i801 access: command=b,
> > size=8, addr=0x52, read_write=1
> > [ 1057.949817] i801_smbus 0000:00:1f.4: i801 access: command=1a,
> > size=8, addr=0x52, read_write=0
> > [ 1057.949904] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4,
> > SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
> > [ 1057.949952] i801_smbus 0000:00:1f.4: No response
> > [ 1057.950215] i801_smbus 0000:00:1f.4: Entering suspend path...
> > [ 1059.338647] ACPI: EC: interrupt blocked
> > [ 1060.756385] ACPI: EC: interrupt unblocked
> > [ 1060.926423] i801_smbus 0000:00:1f.4: Entering resume path...
> > [ 1060.926623] spd5118 14-0052: Entering resume path...
> > [ 1060.927930] i801_smbus 0000:00:1f.4: i801 access: command=b,
> > size=8, addr=0x52, read_write=0
> > [ 1060.927969] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4,
> > SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
> > [ 1060.927995] i801_smbus 0000:00:1f.4: No response
> > [ 1060.928477] i801_smbus 0000:00:1f.4: i801 access: command=b,
> > size=8, addr=0x52, read_write=0
> > [ 1060.928517] i801_smbus 0000:00:1f.4: i801 timeout: status=0x4,
> > SMBHSTCNT=0x15, SMBHSTSTS_DEV_ERR=4
> > [ 1060.928543] i801_smbus 0000:00:1f.4: No response
> > [ 1060.928582] spd5118 14-0052: Failed to write b = 0: -6
> > [ 1060.928707] spd5118 14-0052: PM: dpm_run_callback(): spd5118_resume
> > returns -6
> > [ 1060.928981] spd5118 14-0052: PM: failed to resume async: error -6
> > [ 1062.414560] PM: suspend exit
> >


Hi Armin,

> >> Besides that: did the spd5118 driver load automatically on your device?
Yes.


> >> Oh no, this likely happens even when merely reading values, as the
> >> spd5118
> >> uses a page register to switch between different register pages. In
> >> order
> >> to access temperature data (page 0), you might already have to issue a
> >> write access to the page register. The only reason why it works for you
> >> is that the spd5118 likely already has page 0 selected by the system
> >> firmware
> >> during boot.

I tested the assumption by explicitly attempting to switch the SPD5118
page in i2c_probe()
using the same mechanism as the existing recovery logic. On my
platform, the write to
SPD5118_REG_I2C_LEGACY_MODE fails with SMBHSTSTS_DEV_ERR, and the page
remains unchanged.
This demonstrates that page selection requires a write and cannot be
performed on platforms
with SPD write disable. Temperature reads therefore only work because
firmware has
pre-selected page 0, and Linux has no way to recover if that state is
lost (e.g. across suspend).
This confirms that a “read-only SPD” mode cannot be made robust.

/* Simulate later temp read attempts depend on startup page writes */
dev_info(dev, "(i2c-probe) Initial legacy_mode = 0x%02x\n", mode);
i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE, mode |
SPD5118_LEGACY_PAGE_MASK);
mode = i2c_smbus_read_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE);
dev_info(dev, "(i2c-probe) Updated legacy_mode = 0x%02x\n", mode);

/* Log output */
i801_smbus 0000:00:1f.4: SPD Write Disable is set
i801_smbus 0000:00:1f.4: SMBus using PCI interrupt
...
spd5118 14-0052: (i2c-probe) Initial legacy_mode = 0x00
i801_smbus 0000:00:1f.4: i801 access: command=b, size=2, addr=0x52, read_write=0
i801_smbus 0000:00:1f.4: i801 timeout: status=0x4, SMBHSTCNT=0x9,
SMBHSTSTS_DEV_ERR=4
i801_smbus 0000:00:1f.4: i801 access: command=b, size=2, addr=0x52, read_write=1
spd5118 14-0052: (i2c-probe) Updated legacy_mode = 0x00