Re: [PATCH 1/2] i2c: i801: Force no IRQ for Dell Latitude E7450

From: Marius Hoch
Date: Sun Jun 18 2023 - 09:08:13 EST


Hi Jean,

On 04/06/2023 16:38, Jean Delvare wrote:
On Sun, 14 May 2023 12:36:33 +0200, Marius Hoch wrote:
The Dell Latitude E7450 uses IRQ 18 for the accelerometer,
but also claims that the SMBus uses IRQ 18. This will
result in:

i801_smbus 0000:00:1f.3: PCI INT C: failed to register GSI
i801_smbus 0000:00:1f.3: Failed to enable SMBus PCI device (-16)
i801_smbus: probe of 0000:00:1f.3 failed with error -16

Force the SMBus IRQ to IRQ_NOTCONNECTED in this case, so that
we fall back to polling, which also seems to be what the (very
dated) Windows 7 drivers on the Dell Latitude E7450 do.

This was tested on Dell Latitude E7450.

Signed-off-by: Marius Hoch <mail@xxxxxxxxxxxxx>
---
drivers/i2c/busses/i2c-i801.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index ac5326747c51..5fd2ac585160 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1624,6 +1624,20 @@ static void i801_setup_hstcfg(struct i801_priv *priv)
pci_write_config_byte(priv->pci_dev, SMBHSTCFG, hstcfg);
}
+/**
As reported by the kernel test robot, please don't start a comment with
/** unless it's a kernel-doc-style comment.

+ * These DELL devices claim an IRQ for the SMBus (usually 18), but we can't use
+ * it, as its actually for the I2C accelerometer.
+ */
+static const struct dmi_system_id dmi_force_no_irq[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Latitude E7450"),
+ },
+ },
+ {} /* Terminating entry */
+};
+
static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
{
int err, i;
@@ -1657,6 +1671,12 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
if (!(priv->features & FEATURE_BLOCK_BUFFER))
priv->features &= ~FEATURE_BLOCK_PROC;
+ if (dmi_check_system(dmi_force_no_irq)) {
If the problem is caused by dev->irq being 255, and now that we know
that this value is special on x86, wouldn't it make more sense to
restrict this quirk to CONFIG_X86 and simply check for dev->irq ==
0xff? This would save us the extra effort of maintaining a list of
machines which need the quirk.

+ /* Force no IRQ for these devices, otherwise pcim_enable_device will fail */
+ dev->irq = IRQ_NOTCONNECTED;
+ dev->irq_managed = 1;
This field is undocumented so I have no idea what it does. Is it not
sufficient to set irq to IRQ_NOTCONNECTED?
If irq_managed is not set our irq value will be entirely ignored it seems (thus leading to the same code path/ failure initially outlined).

+ }
+
err = pcim_enable_device(dev);
if (err) {
dev_err(&dev->dev, "Failed to enable SMBus PCI device (%d)\n",