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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <516f5dfd-3e84-4938-bfea-5c52e884f1af@roeck-us.net>
Date: Fri, 30 Jan 2026 18:06:59 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Kurt Borja <kuurtb@...il.com>,
 TINSAE TADESSE <tinsaetadesse2015@...il.com>
Cc: Armin Wolf <W_Armin@....de>,
 "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
 linux-kernel@...r.kernel.org, Guenter Roeck <groeck7@...il.com>
Subject: Re: [PATCH 1/3] hwmon: spd5118: Do not fail resume on temporary I2C
 errors

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@...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);
>>>
>>> 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
>