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

[TINSAE TADESSE]

On Sun, Feb 1, 2026 at 4:25 AM Guenter Roeck <linux@xxxxxxxxxxxx> 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.
From e4fa15d9a667b46057f98092a46dc60a99f547fb Mon Sep 17 00:00:00 2001
From: Tinsae Tadesse <tinsaetadesse2015@xxxxxxxxx>
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@xxxxxxxxx>
---
 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

From f232d9710bd0d1cf290a28234bc98e4224101916 Mon Sep 17 00:00:00 2001
From: Tinsae Tadesse <tinsaetadesse2015@xxxxxxxxx>
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@xxxxxxxxx>
---
 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