[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ12PfNFtrKOiQRzh9T+VPa3FDK6VdG8y_xBqM4B6BD50pQzWg@mail.gmail.com>
Date: Sat, 31 Jan 2026 14:26:45 +0300
From: TINSAE TADESSE <tinsaetadesse2015@...il.com>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Armin Wolf <W_Armin@....de>,
"linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C errors
On Wed, Jan 28, 2026 at 2:41 AM Guenter Roeck <linux@...ck-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@...ck-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@...ck-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!
Powered by blists - more mailing lists