| Message ID | 1f81a126-11b4-aa22-1e2c-9824e0ad730c@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | i2c: i801: Series with minor improvements | expand |
Hi Heiner, On Fri, 15 Apr 2022 18:56:30 +0200, Heiner Kallweit wrote: > According to the datasheets interrupt mode and i2c block read are > supported on all chip versions. Therefore set both feature flags for > all chip versions. While the datasheets do match your claims (I checked the 82801CAM aka ICH3-M datasheet), I have a hard time believing we would have made the feature device-dependent without a good reason (and I have vague memories that there was a problem, although I can't find any proof of that). So I'll try to resurrect my old ICH3-M-based laptop and test these changes on it. If I manage to get a Linux distribution to install on that 20-year-old system... > Note: Don't remove the two feature flags as such (at least for now), > so that in case of a problem users can use the disable_features > module parameter to disable a problematic feature. Agreed.
Hi Heiner, I was able to resurrect my Sony Vaio GR214EP laptop to test this patch on an Intel 82801CAM (ICH3-M) chipset. And as I suspected, it does not work. I get the following error in the kernel log: [13563.411101] i801_smbus 0000:00:1f.3: SMBus using PCI interrupt [13565.434712] irq 9: nobody cared (try booting with the "irqpoll" option) [13565.434728] Pid: 321, comm: udevd Tainted: G O 3.4.63-2.44-default #1 [13565.434734] Call Trace: [13565.434773] [<c0205349>] try_stack_unwind+0x199/0x1b0 [13565.434793] [<c02041c7>] dump_trace+0x47/0xf0 [13565.434808] [<c02053ab>] show_trace_log_lvl+0x4b/0x60 [13565.434820] [<c02053d8>] show_trace+0x18/0x20 [13565.434837] [<c0682e81>] dump_stack+0x6d/0x72 [13565.434855] [<c02adad1>] __report_bad_irq+0x21/0xc0 [13565.434870] [<c02adef1>] note_interrupt+0x181/0x1d0 [13565.434887] [<c02abe9e>] handle_irq_event_percpu+0x9e/0x1d0 [13565.434901] [<c02abffe>] handle_irq_event+0x2e/0x50 [13565.434915] [<c02ae3e6>] handle_level_irq+0x56/0x90 [13565.434928] [<c0204168>] handle_irq+0x88/0xa0 [13565.434939] handlers: [13565.434949] [<c04bbf6c>] acpi_irq [13565.435027] [<e0baad00>] usb_hcd_irq [13565.435054] [<e0baad00>] usb_hcd_irq [13565.435079] [<e0baad00>] usb_hcd_irq [13565.435193] [<e0d6fb80>] radeon_driver_irq_handler_kms [13565.435206] [<e0af5c60>] yenta_interrupt [13565.435214] [<e0af5c60>] yenta_interrupt [13565.435227] [<e0b514c0>] irq_handler [13565.435238] [<e0aeec90>] snd_intel8x0m_interrupt [13565.435251] [<e0c514a0>] snd_intel8x0_interrupt [13565.435264] [<e0ab4060>] e100_intr [13565.435278] [<e09465d0>] i801_isr [13565.435283] Disabling IRQ #9 [13565.437114] i801_smbus 0000:00:1f.3: Transaction timeout [13565.437125] i801_smbus 0000:00:1f.3: Terminating the current operation [13565.439189] i801_smbus 0000:00:1f.3: Failed terminating the transaction If it matters, and as seen in the list of handlers above, IRQ9 is heavily shared on this laptop (ACPI, USB, graphics, PCMCIA, sound, network and SMBus).
Hi Heiner, On Mon, 13 Jun 2022 19:08:41 +0200, Jean Delvare wrote: > I was able to resurrect my Sony Vaio GR214EP laptop to test this patch > on an Intel 82801CAM (ICH3-M) chipset. And as I suspected, it does not > work. I get the following error in the kernel log: > > [13563.411101] i801_smbus 0000:00:1f.3: SMBus using PCI interrupt > [13565.434712] irq 9: nobody cared (try booting with the "irqpoll" option) And now it's all coming back to me. The reason why we did not enable interrupts on chipsets older than the ICH5 is because they lack bit INTS in PCI register PCISTS. When the IRQ line is shared, we don't know whether the interrupt was caused by our device or by another device. Specifically, the following piece of code fails (it returns IRQ_NONE unconditionally): static irqreturn_t i801_isr(int irq, void *dev_id) { (...) /* Confirm this is our interrupt */ pci_read_config_word(priv->pci_dev, PCI_STATUS, &pcists); if (!(pcists & PCI_STATUS_INTERRUPT)) return IRQ_NONE; I tried replacing the code above by a check on SMBHSTSTS instead: status = inb_p(SMBHSTSTS(priv)); if (!(status & STATUS_FLAGS)) return IRQ_NONE; It seems to work, however my testing is limited and I don't know how reliable that would be if the other devices sharing the interrupt line were used heavily at the same time (the laptop is idling in text mode at the moment so the fact that the interrupt line is heavily shared does not really get exercised). Then I loaded the driver with interrupts disabled (disable_features=0x10) to see if I2C block read was working. It is NOT working, as can be seen from the following dumps from the memory SPD EEPROM: linux-w6i1:/home/jdelvare/src/i2c-i801 # i2cdump -y 4 0x50 b 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef 00: 80 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 ??????@.?uT.??.? 10: 8f 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 ?????.??`..???, 20: 15 08 15 08 00 00 00 00 00 00 00 00 00 00 00 00 ????............ 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 12 b9 ..............?? 40: 7f da 00 00 00 00 00 00 53 53 50 31 33 33 2d 30 ??......SSP133-0 50: 36 34 33 32 33 4a 00 00 00 00 00 00 00 01 02 00 64323J.......??. 60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 64 cd ..............d? 80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ 90: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ a0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ b0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ c0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ linux-w6i1:/home/jdelvare/src/i2c-i801 # i2cdump -y 4 0x50 i 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef 00: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? 10: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? 20: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? 30: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? 40: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? 50: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? 60: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? 70: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? 80: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? 90: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? a0: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? b0: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? c0: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? d0: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? e0: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? f0: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? As you can see, the requested offset of the I2C block read (0x00, then 0x20, then 0x40 etc.) is being ignored, and instead every I2C block read starts at EEPROM offset 0x01. If you compare the datasheets of the ICH5 (Intel doc 252516-001) and ICH3-M (Intel doc 290716-001), section "I2C Read", you will see that the description of the command is different. The format described for the ICH5 (table 114 in the datasheet) does match what the Linux i2c stack calls an I2C block read, while the format described for the ICH3-M (table 5-87 in the datasheet) does NOT. Apparently the original implementation was aimed at specific devices using 10-bit I2C addressing. As no other SMBus host device implemented that, we did not even allocate an SMBus command constant to it (and the fact that Intel changed the format in later hardware iterations proves that this was the right thing to do). Looking at the ICH3-M implementation of the "I2C Block Read" transfer, I feel very lucky that I did not trash my memory SPD EEPROM while running the command. Because the format really starts with a WRITE of 2 bytes to the EEPROM before reading from it. So at least the I2C block read part of the patch is never going to work. The interrupt part could work if we change the test as described above, however I would question the relevance of making that change considering that it is no longer the obvious clean-up you originally proposed, and would then impact recent hardware too. I would hate to introduce a regression for the sole purpose of enabling interrupts on 20-year old hardware which I doubt many people are still using. I am available to perform any test you want me to on this old laptop. As long as it runs...
On 14.06.2022 14:59, Jean Delvare wrote: > Hi Heiner, > > On Mon, 13 Jun 2022 19:08:41 +0200, Jean Delvare wrote: >> I was able to resurrect my Sony Vaio GR214EP laptop to test this patch >> on an Intel 82801CAM (ICH3-M) chipset. And as I suspected, it does not >> work. I get the following error in the kernel log: >> >> [13563.411101] i801_smbus 0000:00:1f.3: SMBus using PCI interrupt >> [13565.434712] irq 9: nobody cared (try booting with the "irqpoll" option) > > And now it's all coming back to me. The reason why we did not enable > interrupts on chipsets older than the ICH5 is because they lack bit > INTS in PCI register PCISTS. When the IRQ line is shared, we don't know > whether the interrupt was caused by our device or by another device. > Specifically, the following piece of code fails (it returns IRQ_NONE > unconditionally): > > static irqreturn_t i801_isr(int irq, void *dev_id) > { > (...) > /* Confirm this is our interrupt */ > pci_read_config_word(priv->pci_dev, PCI_STATUS, &pcists); > if (!(pcists & PCI_STATUS_INTERRUPT)) > return IRQ_NONE; > > I tried replacing the code above by a check on SMBHSTSTS instead: > > status = inb_p(SMBHSTSTS(priv)); > if (!(status & STATUS_FLAGS)) > return IRQ_NONE; > > It seems to work, however my testing is limited and I don't know how > reliable that would be if the other devices sharing the interrupt line > were used heavily at the same time (the laptop is idling in text mode > at the moment so the fact that the interrupt line is heavily shared > does not really get exercised). > > Then I loaded the driver with interrupts disabled > (disable_features=0x10) to see if I2C block read was working. It is NOT > working, as can be seen from the following dumps from the memory SPD > EEPROM: > > linux-w6i1:/home/jdelvare/src/i2c-i801 # i2cdump -y 4 0x50 b > 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef > 00: 80 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 ??????@.?uT.??.? > 10: 8f 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 ?????.??`..???, > 20: 15 08 15 08 00 00 00 00 00 00 00 00 00 00 00 00 ????............ > 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 12 b9 ..............?? > 40: 7f da 00 00 00 00 00 00 53 53 50 31 33 33 2d 30 ??......SSP133-0 > 50: 36 34 33 32 33 4a 00 00 00 00 00 00 00 01 02 00 64323J.......??. > 60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 64 cd ..............d? > 80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > 90: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > a0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > b0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > c0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > > linux-w6i1:/home/jdelvare/src/i2c-i801 # i2cdump -y 4 0x50 i > 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef > 00: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? > 10: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? > 20: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? > 30: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? > 40: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? > 50: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? > 60: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? > 70: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? > 80: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? > 90: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? > a0: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? > b0: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? > c0: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? > d0: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? > e0: 08 04 0d 09 02 40 00 01 75 54 00 82 10 00 01 8f ?????@.?uT.??.?? > f0: 04 06 01 01 00 0e a0 60 00 00 14 0f 14 2c 20 15 ????.??`..???, ? > > As you can see, the requested offset of the I2C block read (0x00, then > 0x20, then 0x40 etc.) is being ignored, and instead every I2C block > read starts at EEPROM offset 0x01. > > If you compare the datasheets of the ICH5 (Intel doc 252516-001) and > ICH3-M (Intel doc 290716-001), section "I2C Read", you will see that > the description of the command is different. The format described for > the ICH5 (table 114 in the datasheet) does match what the Linux i2c > stack calls an I2C block read, while the format described for the > ICH3-M (table 5-87 in the datasheet) does NOT. Apparently the original > implementation was aimed at specific devices using 10-bit I2C > addressing. As no other SMBus host device implemented that, we did not > even allocate an SMBus command constant to it (and the fact that Intel > changed the format in later hardware iterations proves that this was the > right thing to do). > > Looking at the ICH3-M implementation of the "I2C Block Read" transfer, > I feel very lucky that I did not trash my memory SPD EEPROM while > running the command. Because the format really starts with a WRITE of 2 > bytes to the EEPROM before reading from it. > > So at least the I2C block read part of the patch is never going to > work. The interrupt part could work if we change the test as described > above, however I would question the relevance of making that change > considering that it is no longer the obvious clean-up you originally > proposed, and would then impact recent hardware too. I would hate to > introduce a regression for the sole purpose of enabling interrupts on > 20-year old hardware which I doubt many people are still using. > > I am available to perform any test you want me to on this old laptop. > As long as it runs... > Thanks for testing! So I'll drop this patch from the series.
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 1d8182901..a9737f14d 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -12,70 +12,70 @@ /* * Supports the following Intel I/O Controller Hubs (ICH): * - * I/O Block I2C - * region SMBus Block proc. block - * Chip name PCI ID size PEC buffer call read - * --------------------------------------------------------------------------- - * 82801AA (ICH) 0x2413 16 no no no no - * 82801AB (ICH0) 0x2423 16 no no no no - * 82801BA (ICH2) 0x2443 16 no no no no - * 82801CA (ICH3) 0x2483 32 soft no no no - * 82801DB (ICH4) 0x24c3 32 hard yes no no - * 82801E (ICH5) 0x24d3 32 hard yes yes yes - * 6300ESB 0x25a4 32 hard yes yes yes - * 82801F (ICH6) 0x266a 32 hard yes yes yes - * 6310ESB/6320ESB 0x269b 32 hard yes yes yes - * 82801G (ICH7) 0x27da 32 hard yes yes yes - * 82801H (ICH8) 0x283e 32 hard yes yes yes - * 82801I (ICH9) 0x2930 32 hard yes yes yes - * EP80579 (Tolapai) 0x5032 32 hard yes yes yes - * ICH10 0x3a30 32 hard yes yes yes - * ICH10 0x3a60 32 hard yes yes yes - * 5/3400 Series (PCH) 0x3b30 32 hard yes yes yes - * 6 Series (PCH) 0x1c22 32 hard yes yes yes - * Patsburg (PCH) 0x1d22 32 hard yes yes yes - * Patsburg (PCH) IDF 0x1d70 32 hard yes yes yes - * Patsburg (PCH) IDF 0x1d71 32 hard yes yes yes - * Patsburg (PCH) IDF 0x1d72 32 hard yes yes yes - * DH89xxCC (PCH) 0x2330 32 hard yes yes yes - * Panther Point (PCH) 0x1e22 32 hard yes yes yes - * Lynx Point (PCH) 0x8c22 32 hard yes yes yes - * Lynx Point-LP (PCH) 0x9c22 32 hard yes yes yes - * Avoton (SOC) 0x1f3c 32 hard yes yes yes - * Wellsburg (PCH) 0x8d22 32 hard yes yes yes - * Wellsburg (PCH) MS 0x8d7d 32 hard yes yes yes - * Wellsburg (PCH) MS 0x8d7e 32 hard yes yes yes - * Wellsburg (PCH) MS 0x8d7f 32 hard yes yes yes - * Coleto Creek (PCH) 0x23b0 32 hard yes yes yes - * Wildcat Point (PCH) 0x8ca2 32 hard yes yes yes - * Wildcat Point-LP (PCH) 0x9ca2 32 hard yes yes yes - * BayTrail (SOC) 0x0f12 32 hard yes yes yes - * Braswell (SOC) 0x2292 32 hard yes yes yes - * Sunrise Point-H (PCH) 0xa123 32 hard yes yes yes - * Sunrise Point-LP (PCH) 0x9d23 32 hard yes yes yes - * DNV (SOC) 0x19df 32 hard yes yes yes - * Emmitsburg (PCH) 0x1bc9 32 hard yes yes yes - * Broxton (SOC) 0x5ad4 32 hard yes yes yes - * Lewisburg (PCH) 0xa1a3 32 hard yes yes yes - * Lewisburg Supersku (PCH) 0xa223 32 hard yes yes yes - * Kaby Lake PCH-H (PCH) 0xa2a3 32 hard yes yes yes - * Gemini Lake (SOC) 0x31d4 32 hard yes yes yes - * Cannon Lake-H (PCH) 0xa323 32 hard yes yes yes - * Cannon Lake-LP (PCH) 0x9da3 32 hard yes yes yes - * Cedar Fork (PCH) 0x18df 32 hard yes yes yes - * Ice Lake-LP (PCH) 0x34a3 32 hard yes yes yes - * Ice Lake-N (PCH) 0x38a3 32 hard yes yes yes - * Comet Lake (PCH) 0x02a3 32 hard yes yes yes - * Comet Lake-H (PCH) 0x06a3 32 hard yes yes yes - * Elkhart Lake (PCH) 0x4b23 32 hard yes yes yes - * Tiger Lake-LP (PCH) 0xa0a3 32 hard yes yes yes - * Tiger Lake-H (PCH) 0x43a3 32 hard yes yes yes - * Jasper Lake (SOC) 0x4da3 32 hard yes yes yes - * Comet Lake-V (PCH) 0xa3a3 32 hard yes yes yes - * Alder Lake-S (PCH) 0x7aa3 32 hard yes yes yes - * Alder Lake-P (PCH) 0x51a3 32 hard yes yes yes - * Alder Lake-M (PCH) 0x54a3 32 hard yes yes yes - * Raptor Lake-S (PCH) 0x7a23 32 hard yes yes yes + * I/O Block + * region SMBus Block proc. + * Chip name PCI ID size PEC buffer call + * ------------------------------------------------------------------- + * 82801AA (ICH) 0x2413 16 no no no + * 82801AB (ICH0) 0x2423 16 no no no + * 82801BA (ICH2) 0x2443 16 no no no + * 82801CA (ICH3) 0x2483 32 soft no no + * 82801DB (ICH4) 0x24c3 32 hard yes no + * 82801E (ICH5) 0x24d3 32 hard yes yes + * 6300ESB 0x25a4 32 hard yes yes + * 82801F (ICH6) 0x266a 32 hard yes yes + * 6310ESB/6320ESB 0x269b 32 hard yes yes + * 82801G (ICH7) 0x27da 32 hard yes yes + * 82801H (ICH8) 0x283e 32 hard yes yes + * 82801I (ICH9) 0x2930 32 hard yes yes + * EP80579 (Tolapai) 0x5032 32 hard yes yes + * ICH10 0x3a30 32 hard yes yes + * ICH10 0x3a60 32 hard yes yes + * 5/3400 Series (PCH) 0x3b30 32 hard yes yes + * 6 Series (PCH) 0x1c22 32 hard yes yes + * Patsburg (PCH) 0x1d22 32 hard yes yes + * Patsburg (PCH) IDF 0x1d70 32 hard yes yes + * Patsburg (PCH) IDF 0x1d71 32 hard yes yes + * Patsburg (PCH) IDF 0x1d72 32 hard yes yes + * DH89xxCC (PCH) 0x2330 32 hard yes yes + * Panther Point (PCH) 0x1e22 32 hard yes yes + * Lynx Point (PCH) 0x8c22 32 hard yes yes + * Lynx Point-LP (PCH) 0x9c22 32 hard yes yes + * Avoton (SOC) 0x1f3c 32 hard yes yes + * Wellsburg (PCH) 0x8d22 32 hard yes yes + * Wellsburg (PCH) MS 0x8d7d 32 hard yes yes + * Wellsburg (PCH) MS 0x8d7e 32 hard yes yes + * Wellsburg (PCH) MS 0x8d7f 32 hard yes yes + * Coleto Creek (PCH) 0x23b0 32 hard yes yes + * Wildcat Point (PCH) 0x8ca2 32 hard yes yes + * Wildcat Point-LP (PCH) 0x9ca2 32 hard yes yes + * BayTrail (SOC) 0x0f12 32 hard yes yes + * Braswell (SOC) 0x2292 32 hard yes yes + * Sunrise Point-H (PCH) 0xa123 32 hard yes yes + * Sunrise Point-LP (PCH) 0x9d23 32 hard yes yes + * DNV (SOC) 0x19df 32 hard yes yes + * Emmitsburg (PCH) 0x1bc9 32 hard yes yes + * Broxton (SOC) 0x5ad4 32 hard yes yes + * Lewisburg (PCH) 0xa1a3 32 hard yes yes + * Lewisburg Supersku (PCH) 0xa223 32 hard yes yes + * Kaby Lake PCH-H (PCH) 0xa2a3 32 hard yes yes + * Gemini Lake (SOC) 0x31d4 32 hard yes yes + * Cannon Lake-H (PCH) 0xa323 32 hard yes yes + * Cannon Lake-LP (PCH) 0x9da3 32 hard yes yes + * Cedar Fork (PCH) 0x18df 32 hard yes yes + * Ice Lake-LP (PCH) 0x34a3 32 hard yes yes + * Ice Lake-N (PCH) 0x38a3 32 hard yes yes + * Comet Lake (PCH) 0x02a3 32 hard yes yes + * Comet Lake-H (PCH) 0x06a3 32 hard yes yes + * Elkhart Lake (PCH) 0x4b23 32 hard yes yes + * Tiger Lake-LP (PCH) 0xa0a3 32 hard yes yes + * Tiger Lake-H (PCH) 0x43a3 32 hard yes yes + * Jasper Lake (SOC) 0x4da3 32 hard yes yes + * Comet Lake-V (PCH) 0xa3a3 32 hard yes yes + * Alder Lake-S (PCH) 0x7aa3 32 hard yes yes + * Alder Lake-P (PCH) 0x51a3 32 hard yes yes + * Alder Lake-M (PCH) 0x54a3 32 hard yes yes + * Raptor Lake-S (PCH) 0x7a23 32 hard yes yes * * Features supported by this driver: * Software PEC no @@ -168,7 +168,7 @@ #define I801_WORD_DATA 0x0C #define I801_PROC_CALL 0x10 #define I801_BLOCK_DATA 0x14 -#define I801_I2C_BLOCK_DATA 0x18 /* ICH5 and later */ +#define I801_I2C_BLOCK_DATA 0x18 #define I801_BLOCK_PROC_CALL 0x1C /* I801 Host Control register bits */ @@ -973,11 +973,8 @@ static const struct i2c_algorithm smbus_algorithm = { .functionality = i801_func, }; -#define FEATURES_ICH5 (FEATURE_BLOCK_PROC | FEATURE_I2C_BLOCK_READ | \ - FEATURE_IRQ | FEATURE_SMBUS_PEC | \ - FEATURE_BLOCK_BUFFER | FEATURE_HOST_NOTIFY) -#define FEATURES_ICH4 (FEATURE_SMBUS_PEC | FEATURE_BLOCK_BUFFER | \ - FEATURE_HOST_NOTIFY) +#define FEATURES_ICH4 (FEATURE_SMBUS_PEC | FEATURE_BLOCK_BUFFER | FEATURE_HOST_NOTIFY) +#define FEATURES_ICH5 (FEATURES_ICH4 | FEATURE_BLOCK_PROC) static const struct pci_device_id i801_ids[] = { { PCI_DEVICE_DATA(INTEL, 82801AA_3, 0) }, @@ -1665,7 +1662,8 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id) mutex_init(&priv->acpi_lock); priv->pci_dev = dev; - priv->features = id->driver_data; + priv->features = FEATURE_IRQ | FEATURE_I2C_BLOCK_READ; + priv->features |= id->driver_data; /* Disable features on user request */ for (i = 0; i < ARRAY_SIZE(i801_feature_names); i++) {
According to the datasheets interrupt mode and i2c block read are supported on all chip versions. Therefore set both feature flags for all chip versions. Note: Don't remove the two feature flags as such (at least for now), so that in case of a problem users can use the disable_features module parameter to disable a problematic feature. Patch is based solely on the datasheets. I don't have old enough test hw, therefore the patch is compile-tested only. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/i2c/busses/i2c-i801.c | 140 +++++++++++++++++----------------- 1 file changed, 69 insertions(+), 71 deletions(-)