Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

i2c: i801: Do not add ICH_RES_IO_SMI for the iTCO_wdt device

Martin noticed that nct6775 driver does not load properly on his system
in v5.4+ kernels. The issue was bisected to commit b84398d6d7f9 ("i2c:
i801: Use iTCO version 6 in Cannon Lake PCH and beyond") but it is
likely not the culprit because the faulty code has been in the driver
already since commit 9424693035a5 ("i2c: i801: Create iTCO device on
newer Intel PCHs"). So more likely some commit that added PCI IDs of
recent chipsets made the driver to create the iTCO_wdt device on Martins
system.

The issue was debugged to be PCI configuration access to the PMC device
that is not present. This returns all 1's when read and this caused the
iTCO_wdt driver to accidentally request resourses used by nct6775.

It turns out that the SMI resource is only required for some ancient
systems, not the ones supported by this driver. For this reason do not
populate the SMI resource at all and drop all the related code. The
driver now always populates the main I/O resource and only in case of SPT
(Intel Sunrisepoint) compatible devices it adds another resource for the
NO_REBOOT bit. These two resources are of different types so
platform_get_resource() used by the iTCO_wdt driver continues to find
the both resources at index 0.

Link: https://lore.kernel.org/linux-hwmon/CAM1AHpQ4196tyD=HhBu-2donSsuogabkfP03v1YF26Q7_BgvgA@mail.gmail.com/
Fixes: 9424693035a5 ("i2c: i801: Create iTCO device on newer Intel PCHs")
[wsa: complete fix needs all of http://patchwork.ozlabs.org/project/linux-i2c/list/?series=160959&state=*]
Reported-by: Martin Volf <martin.volf.42@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>

authored by

Mika Westerberg and committed by
Wolfram Sang
04bbb97d e42b0c24

+12 -33
+12 -33
drivers/i2c/busses/i2c-i801.c
··· 132 132 #define TCOBASE 0x050 133 133 #define TCOCTL 0x054 134 134 135 - #define ACPIBASE 0x040 136 - #define ACPIBASE_SMI_OFF 0x030 137 - #define ACPICTRL 0x044 138 - #define ACPICTRL_EN 0x080 139 - 140 135 #define SBREG_BAR 0x10 141 136 #define SBREG_SMBCTRL 0xc6000c 142 137 #define SBREG_SMBCTRL_DNV 0xcf000c ··· 1548 1553 pci_bus_write_config_byte(pci_dev->bus, devfn, 0xe1, hidden); 1549 1554 spin_unlock(&p2sb_spinlock); 1550 1555 1551 - res = &tco_res[ICH_RES_MEM_OFF]; 1556 + res = &tco_res[1]; 1552 1557 if (pci_dev->device == PCI_DEVICE_ID_INTEL_DNV_SMBUS) 1553 1558 res->start = (resource_size_t)base64_addr + SBREG_SMBCTRL_DNV; 1554 1559 else ··· 1558 1563 res->flags = IORESOURCE_MEM; 1559 1564 1560 1565 return platform_device_register_resndata(&pci_dev->dev, "iTCO_wdt", -1, 1561 - tco_res, 3, &spt_tco_platform_data, 1566 + tco_res, 2, &spt_tco_platform_data, 1562 1567 sizeof(spt_tco_platform_data)); 1563 1568 } 1564 1569 ··· 1571 1576 i801_add_tco_cnl(struct i801_priv *priv, struct pci_dev *pci_dev, 1572 1577 struct resource *tco_res) 1573 1578 { 1574 - return platform_device_register_resndata(&pci_dev->dev, "iTCO_wdt", -1, 1575 - tco_res, 2, &cnl_tco_platform_data, 1576 - sizeof(cnl_tco_platform_data)); 1579 + return platform_device_register_resndata(&pci_dev->dev, 1580 + "iTCO_wdt", -1, tco_res, 1, &cnl_tco_platform_data, 1581 + sizeof(cnl_tco_platform_data)); 1577 1582 } 1578 1583 1579 1584 static void i801_add_tco(struct i801_priv *priv) 1580 1585 { 1581 - u32 base_addr, tco_base, tco_ctl, ctrl_val; 1582 1586 struct pci_dev *pci_dev = priv->pci_dev; 1583 - struct resource tco_res[3], *res; 1584 - unsigned int devfn; 1587 + struct resource tco_res[2], *res; 1588 + u32 tco_base, tco_ctl; 1585 1589 1586 1590 /* If we have ACPI based watchdog use that instead */ 1587 1591 if (acpi_has_watchdog()) ··· 1595 1601 return; 1596 1602 1597 1603 memset(tco_res, 0, sizeof(tco_res)); 1598 - 1599 - res = &tco_res[ICH_RES_IO_TCO]; 1604 + /* 1605 + * Always populate the main iTCO IO resource here. The second entry 1606 + * for NO_REBOOT MMIO is filled by the SPT specific function. 1607 + */ 1608 + res = &tco_res[0]; 1600 1609 res->start = tco_base & ~1; 1601 1610 res->end = res->start + 32 - 1; 1602 1611 res->flags = IORESOURCE_IO; 1603 - 1604 - /* 1605 - * Power Management registers. 1606 - */ 1607 - devfn = PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 2); 1608 - pci_bus_read_config_dword(pci_dev->bus, devfn, ACPIBASE, &base_addr); 1609 - 1610 - res = &tco_res[ICH_RES_IO_SMI]; 1611 - res->start = (base_addr & ~1) + ACPIBASE_SMI_OFF; 1612 - res->end = res->start + 3; 1613 - res->flags = IORESOURCE_IO; 1614 - 1615 - /* 1616 - * Enable the ACPI I/O space. 1617 - */ 1618 - pci_bus_read_config_dword(pci_dev->bus, devfn, ACPICTRL, &ctrl_val); 1619 - ctrl_val |= ACPICTRL_EN; 1620 - pci_bus_write_config_dword(pci_dev->bus, devfn, ACPICTRL, ctrl_val); 1621 1612 1622 1613 if (priv->features & FEATURE_TCO_CNL) 1623 1614 priv->tco_pdev = i801_add_tco_cnl(priv, pci_dev, tco_res);