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...
--
Jean Delvare
SUSE L3 Support
