On Jan 03, 2024 / 22:31, Klara Modin wrote: > Hi, > > With this patch, ata_piix fails to detect my IDE controller (older P4 > Xeon board). Reverting this on top of 6.7-rc8 resolves the issue for > me. Thanks for the report. According the dmesg and lspci logs, it looks the IDE controller has the PCI DEVFN 31:1 (0000:00:1f.1). So, I guess as follows: - Klara's system has the old ICH (ICH4) which has IDE controller at DEVFN 31:1. This DEVFN is same as P2SB device that newer PCH Series 100 or later has. - The system has ISA bridge 82801BA for LPC, then LPC_ICH driver is loaded. This driver depends on P2SB and calls p2sb_bar(). But p2sb_bar() is called only for new features of LPC_ICH driver (SPI flash and GPIO). Then p2sb_bar() is not called on the system, probably (I want to confirm it). - Before the commit b28ff7a7c324 ("platform/x86: p2sb: Allow p2sb_bar() calls during PCI device probe"), the DEVFN 31:1 is accessed as P2SB only when p2sb_bar() is called. After the commit, if P2SB is enabled in Kconfig, DEVFN 31:1 is accessed as P2SB to cache P2SB resources. However, the device is not P2SB devcie but IDE controller, then it casued the IDE controller detection failure. > > Please tell me if there's anything else you need. I'm willing to test > any new patches. Could you confirm that p2sb_bar() is not called during boot process on your system? Applying the one liner printk patch below, let's confirm that the string "p2sb_bar" does not appear in the dmesg log. diff --git a/drivers/platform/x86/p2sb.c b/drivers/platform/x86/p2sb.c index 99a0e456f075..e034a58d7531 100644 --- a/drivers/platform/x86/p2sb.c +++ b/drivers/platform/x86/p2sb.c @@ -195,6 +195,7 @@ int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem) struct p2sb_res_cache *cache; int ret; + printk("%s\n", __func__); bus = p2sb_get_bus(bus); if (!bus) return -ENODEV; If p2sb_bar() is not called on the system, I think P2SB device scan and resource cache shall not run on such old systems. Before the commit 53eb64c88f17 ("platform/x86: p2sb: Don't fail if unknown CPU is found"), p2sb.c had a whitelist of Intel CPU IDs to access P2SB device. Its commit message indicates that we can add blacklist of CPU IDs if needed. I think this blacklist will help to avoid the failure. So next question is: how to list the CPUs which do not need P2SB resource cache? I don't have clear answer yet. According a P2SB document [1], P2SB support was introduced since PCH Series 100, around 2010. Looking at ICH history [2], ICH9 removed PATA support, so I guess DEVFN 31:1 for IDE controller was removed since ICH9, around 2007. So the end of the Intel CPU blacklist could be the CPU introduced between 2007 and 2010. [1] https://lab.whitequark.org/notes/2017-11-08/accessing-intel-ich-pch-gpios/ [2] https://en.wikipedia.org/wiki/I/O_Controller_Hub#ICH8 My mere idea was to just blacklist Intel CPUs with family != 6. If my guesses are all correct, following patch will avoid the failure on the reported system, since the system has CPU with family == 0xf. This will cover certain amount of old CPUs which should not access DEVFN 31:1 as P2SB. But family 6 started around 2006, then it is not the complete blacklist, probably. I will think about the better way. diff --git a/drivers/platform/x86/p2sb.c b/drivers/platform/x86/p2sb.c index 99a0e456f075..c9bcef8e2c4c 100644 --- a/drivers/platform/x86/p2sb.c +++ b/drivers/platform/x86/p2sb.c @@ -46,6 +46,10 @@ static int p2sb_get_devfn(unsigned int *devfn) unsigned int fn = P2SB_DEVFN_DEFAULT; const struct x86_cpu_id *id; + /* ICH/PCHs for old CPUs do not have P2SB */ + if (boot_cpu_data.x86 != 6) + return -ENODEV; + id = x86_match_cpu(p2sb_cpu_ids); if (id) fn = (unsigned int)id->driver_data;