[PATCH v4 1/2] hwmon: (spd5118) Select page 0 unconditionally during probe

[PATCH v4 1/2] hwmon: (spd5118) Select page 0 unconditionally during probe

[Armin Wolf]

Some Intel i2c controllers can be configured by the BIOS to reject
writes to the SPD device. This often causes problems when the register
page needs to be changed, usually during resume.

Avoid probing on affected devices by unconditionally selecting page 0
by writing the SPD5118_REG_I2C_LEGACY_MODE register during probe.
This will fail on affected controllers and thus prevent the driver
from probing.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
Changes since v3:
- return early on obviously unsupported devices

Changes since v2:
- restore original register content if i2c init fails

Changes since v1:
- Avoid zeroing reserved bits inside SPD5118_REG_I2C_LEGACY_MODE
---
 drivers/hwmon/spd5118.c | 65 +++++++++++++++++++++--------------------
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 9724cf70b61d..f79b46085ecc 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -637,44 +637,47 @@ static int spd5118_i2c_init(struct i2c_client *client)
 				     I2C_FUNC_SMBUS_WORD_DATA))
 		return -ENODEV;
 
+	/* Early check to avoid obviously unsupported I2C devices */
 	regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE);
-	if (regval < 0 || (regval && regval != 0x5118))
+	if (regval < 0)
+		return regval;
+
+	/*
+	 * Some SPD5118 devices report 0x0 when page 0 is not selected,
+	 * so we only fail here if the register value is not 0x0 or 0x5118.
+	 */
+	if (regval && regval != 0x5118)
 		return -ENODEV;
 
 	/*
-	 * If the device type registers return 0, it is possible that the chip
-	 * has a non-zero page selected and takes the specification literally,
+	 * We must select page 0 to ensure that we can reliably read the volatile
+	 * registers on chips that take the specification literally,
 	 * i.e. disables access to volatile registers besides the page register
 	 * if the page is not 0. The Renesas/ITD SPD5118 Hub Controller is known
-	 * to show this behavior. Try to identify such chips.
+	 * to show this behavior.
+	 *
+	 * We must also perform an unconditional register write to detect if
+	 * the i2c controller blocks write accesses to the SPD device. Some Intel
+	 * controllers might be configured by the BIOS to do this.
 	 */
-	if (!regval) {
-		/* Vendor ID registers must also be 0 */
-		regval = i2c_smbus_read_word_data(client, SPD5118_REG_VENDOR);
-		if (regval)
-			return -ENODEV;
-
-		/* The selected page in MR11 must not be 0 */
-		mode = i2c_smbus_read_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE);
-		if (mode < 0 || (mode & ~SPD5118_LEGACY_MODE_MASK) ||
-		    !(mode & SPD5118_LEGACY_PAGE_MASK))
-			return -ENODEV;
-
-		err = i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE,
-						mode & SPD5118_LEGACY_MODE_ADDR);
-		if (err)
-			return -ENODEV;
-
-		/*
-		 * If the device type registers are still bad after selecting
-		 * page 0, this is not a SPD5118 device. Restore original
-		 * legacy mode register value and abort.
-		 */
-		regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE);
-		if (regval != 0x5118) {
-			i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE, mode);
-			return -ENODEV;
-		}
+	mode = i2c_smbus_read_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE);
+	if (mode < 0)
+		return mode;
+
+	err = i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE,
+					mode & ~SPD5118_LEGACY_PAGE_MASK);
+	if (err < 0)
+		return err;
+
+	/* We only need to access SPD5118_REG_TYPE again if regval was 0x0 */
+	if (regval == 0x5118)
+		return 0;
+
+	regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE);
+	if (regval != 0x5118) {
+		/* Restore original register content */
+		i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE, mode);
+		return -ENODEV;
 	}
 
 	/* We are reasonably sure that this is really a SPD5118 hub controller */
-- 
2.39.5

[PATCH v4 2/2] hwmon: (spd5118) Avoid probing when 16-bit addressing is enabled

[Armin Wolf]

Support for 16-bit addressing was removed when support for i3c
was added to the driver. Switching between 8-bit and 16-bit addressing
might confuse the system firmware, so we are forced to bail out if
16-bit addressing was configured during boot.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
Changes since v3:
- new patch
---
 drivers/hwmon/spd5118.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index f79b46085ecc..52f354489341 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -16,6 +16,7 @@
 
 #include <linux/bitops.h>
 #include <linux/bits.h>
+#include <linux/dev_printk.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/i3c/device.h>
@@ -664,6 +665,13 @@ static int spd5118_i2c_init(struct i2c_client *client)
 	if (mode < 0)
 		return mode;
 
+	/* 16-bit addressing is not supported */
+	if (mode & SPD5118_LEGACY_MODE_ADDR) {
+		dev_notice(&client->dev,
+			   "Unable to access device due to 16-bit addressing being enabled\n");
+		return -ENODEV;
+	}
+
 	err = i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE,
 					mode & ~SPD5118_LEGACY_PAGE_MASK);
 	if (err < 0)
-- 
2.39.5

Re: [PATCH v4 1/2] hwmon: (spd5118) Select page 0 unconditionally during probe

[sashiko-bot]

> Some Intel i2c controllers can be configured by the BIOS to reject
> writes to the SPD device. This often causes problems when the register
> page needs to be changed, usually during resume.
> 
> Avoid probing on affected devices by unconditionally selecting page 0
> by writing the SPD5118_REG_I2C_LEGACY_MODE register during probe.
> This will fail on affected controllers and thus prevent the driver
> from probing.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901200142.495319-1-W_Armin@gmx.de?part=1

Re: [PATCH v4 2/2] hwmon: (spd5118) Avoid probing when 16-bit addressing is enabled

[sashiko-bot]

> Support for 16-bit addressing was removed when support for i3c
> was added to the driver. Switching between 8-bit and 16-bit addressing
> might confuse the system firmware, so we are forced to bail out if
> 16-bit addressing was configured during boot.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901200142.495319-1-W_Armin@gmx.de?part=2

Re: [PATCH v4 1/2] hwmon: (spd5118) Select page 0 unconditionally during probe

[Guenter Roeck]

On Tue, Sep 01, 2026 at 10:01:41PM +0200, Armin Wolf wrote:
> Some Intel i2c controllers can be configured by the BIOS to reject
> writes to the SPD device. This often causes problems when the register
> page needs to be changed, usually during resume.
> 
> Avoid probing on affected devices by unconditionally selecting page 0
> by writing the SPD5118_REG_I2C_LEGACY_MODE register during probe.
> This will fail on affected controllers and thus prevent the driver
> from probing.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Applied.

Thanks,
Guenter

Re: [PATCH v4 2/2] hwmon: (spd5118) Avoid probing when 16-bit addressing is enabled

[Guenter Roeck]

On Tue, Sep 01, 2026 at 10:01:42PM +0200, Armin Wolf wrote:
> Support for 16-bit addressing was removed when support for i3c
> was added to the driver. Switching between 8-bit and 16-bit addressing
> might confuse the system firmware, so we are forced to bail out if
> 16-bit addressing was configured during boot.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Applied.

Thanks,
Guenter

Re: [PATCH v4 2/2] hwmon: (spd5118) Avoid probing when 16-bit addressing is enabled

[Armin Wolf]

Am 02.09.26 um 01:04 schrieb Guenter Roeck:

> On Tue, Sep 01, 2026 at 10:01:42PM +0200, Armin Wolf wrote:
>> Support for 16-bit addressing was removed when support for i3c
>> was added to the driver. Switching between 8-bit and 16-bit addressing
>> might confuse the system firmware, so we are forced to bail out if
>> 16-bit addressing was configured during boot.
>>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> Applied.
>
> Thanks,
> Guenter

Thank you :)

Hopefully the suspend issues on DDR5-based Intel systems are finally solved.

Thanks,
Armin Wolf

Re: [PATCH v4 2/2] hwmon: (spd5118) Avoid probing when 16-bit addressing is enabled

[Guenter Roeck]

On 9/2/26 09:56, Armin Wolf wrote:
> Am 02.09.26 um 01:04 schrieb Guenter Roeck:
> 
>> On Tue, Sep 01, 2026 at 10:01:42PM +0200, Armin Wolf wrote:
>>> Support for 16-bit addressing was removed when support for i3c
>>> was added to the driver. Switching between 8-bit and 16-bit addressing
>>> might confuse the system firmware, so we are forced to bail out if
>>> 16-bit addressing was configured during boot.
>>>
>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> Applied.
>>
>> Thanks,
>> Guenter
> 
> Thank you :)
> 
> Hopefully the suspend issues on DDR5-based Intel systems are finally solved.
> 
I hope so too. I'll let the patches rest in -next for a couple of weeks and
then flag them for backport into stable.

Of course now we have the AMD suspend problem. Never ending story :-(.

Guenter