diff mbox series

[4/8] i2c: i801: enable FEATURE_IRQ and FEATURE_I2C_BLOCK_READ on all chip versions

Message ID 1f81a126-11b4-aa22-1e2c-9824e0ad730c@gmail.com
State New
Headers show
Series i2c: i801: Series with minor improvements | expand

Commit Message

Heiner Kallweit April 15, 2022, 4:56 p.m. UTC
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(-)

Comments

Jean Delvare June 13, 2022, 5:08 p.m. UTC | #1
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).
Jean Delvare June 14, 2022, 12:59 p.m. UTC | #2
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...
Heiner Kallweit Dec. 16, 2022, 9:36 p.m. UTC | #3
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 mbox series

Patch

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++) {