diff mbox series

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

Message ID 20230514103634.235917-2-mail@mariushoch.de
State Under Review
Delegated to: Wolfram Sang
Headers show
Series i2c: i801: Force no IRQ for Dell Latitude E7450 | expand

Commit Message

Marius Hoch May 14, 2023, 10:36 a.m. UTC
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@mariushoch.de>
---
 drivers/i2c/busses/i2c-i801.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

kernel test robot May 14, 2023, 12:06 p.m. UTC | #1
Hi Marius,

kernel test robot noticed the following build warnings:

[auto build test WARNING on cc3c44c9fda264c6d401be04e95449a57c1231c6]

url:    https://github.com/intel-lab-lkp/linux/commits/Marius-Hoch/i2c-i801-Force-no-IRQ-for-Dell-Latitude-E7450/20230514-183912
base:   cc3c44c9fda264c6d401be04e95449a57c1231c6
patch link:    https://lore.kernel.org/r/20230514103634.235917-2-mail%40mariushoch.de
patch subject: [PATCH 1/2] i2c: i801: Force no IRQ for Dell Latitude E7450
config: ia64-buildonly-randconfig-r002-20230514
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/4de98f83205987d00c89fd710f3cc85f7b64fc87
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Marius-Hoch/i2c-i801-Force-no-IRQ-for-Dell-Latitude-E7450/20230514-183912
        git checkout 4de98f83205987d00c89fd710f3cc85f7b64fc87
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/i2c/busses/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305141951.IaiS1vCr-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/i2c/busses/i2c-i801.c:1628: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * These DELL devices claim an IRQ for the SMBus (usually 18), but we can't use


vim +1628 drivers/i2c/busses/i2c-i801.c

  1626	
  1627	/**
> 1628	 * These DELL devices claim an IRQ for the SMBus (usually 18), but we can't use
  1629	 * it, as its actually for the I2C accelerometer.
  1630	 */
  1631	static const struct dmi_system_id dmi_force_no_irq[] = {
  1632		{
  1633			.matches = {
  1634				DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
  1635				DMI_MATCH(DMI_PRODUCT_NAME, "Latitude E7450"),
  1636			},
  1637		},
  1638		{} /* Terminating entry */
  1639	};
  1640
kernel test robot May 14, 2023, 12:06 p.m. UTC | #2
Hi Marius,

kernel test robot noticed the following build warnings:

[auto build test WARNING on cc3c44c9fda264c6d401be04e95449a57c1231c6]

url:    https://github.com/intel-lab-lkp/linux/commits/Marius-Hoch/i2c-i801-Force-no-IRQ-for-Dell-Latitude-E7450/20230514-183912
base:   cc3c44c9fda264c6d401be04e95449a57c1231c6
patch link:    https://lore.kernel.org/r/20230514103634.235917-2-mail%40mariushoch.de
patch subject: [PATCH 1/2] i2c: i801: Force no IRQ for Dell Latitude E7450
config: riscv-randconfig-r042-20230514
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project b0fb98227c90adf2536c9ad644a74d5e92961111)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/4de98f83205987d00c89fd710f3cc85f7b64fc87
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Marius-Hoch/i2c-i801-Force-no-IRQ-for-Dell-Latitude-E7450/20230514-183912
        git checkout 4de98f83205987d00c89fd710f3cc85f7b64fc87
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/i2c/busses/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305141904.2XlN6bop-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/i2c/busses/i2c-i801.c:1628: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * These DELL devices claim an IRQ for the SMBus (usually 18), but we can't use


vim +1628 drivers/i2c/busses/i2c-i801.c

  1626	
  1627	/**
> 1628	 * These DELL devices claim an IRQ for the SMBus (usually 18), but we can't use
  1629	 * it, as its actually for the I2C accelerometer.
  1630	 */
  1631	static const struct dmi_system_id dmi_force_no_irq[] = {
  1632		{
  1633			.matches = {
  1634				DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
  1635				DMI_MATCH(DMI_PRODUCT_NAME, "Latitude E7450"),
  1636			},
  1637		},
  1638		{} /* Terminating entry */
  1639	};
  1640
Jean Delvare June 4, 2023, 2:38 p.m. UTC | #3
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@mariushoch.de>
> ---
>  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?

> +	}
> +
>  	err = pcim_enable_device(dev);
>  	if (err) {
>  		dev_err(&dev->dev, "Failed to enable SMBus PCI device (%d)\n",
Marius Hoch June 18, 2023, 1:08 p.m. UTC | #4
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@mariushoch.de>
>> ---
>>   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",
>
diff mbox series

Patch

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);
 }
 
+/**
+ * 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)) {
+		/* Force no IRQ for these devices, otherwise pcim_enable_device will fail */
+		dev->irq = IRQ_NOTCONNECTED;
+		dev->irq_managed = 1;
+	}
+
 	err = pcim_enable_device(dev);
 	if (err) {
 		dev_err(&dev->dev, "Failed to enable SMBus PCI device (%d)\n",