Re: [PATCH v5 1/2] platform/x86: p2sb: Allow p2sb_bar() calls during PCI device probe

From: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
To: Lukas Wunner <lukas@wunner.de>
Cc: Klara Modin <klarasmodin@gmail.com>,
	"andriy.shevchenko@linux.intel.com"
	<andriy.shevchenko@linux.intel.com>,
	"hdegoede@redhat.com" <hdegoede@redhat.com>,
	"ilpo.jarvinen@linux.intel.com" <ilpo.jarvinen@linux.intel.com>,
	"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"platform-driver-x86@vger.kernel.org"
	<platform-driver-x86@vger.kernel.org>
Subject: Re: [PATCH v5 1/2] platform/x86: p2sb: Allow p2sb_bar() calls during PCI device probe
Date: Fri, 5 Jan 2024 08:18:05 +0000	[thread overview]
Message-ID: <b565j7nbqu67pjhjw6ei7i3nkazazirl4dyxhaem3z7ghii3gs@dngmvjcylrjp> (raw)
In-Reply-To: <20240104123621.GA4876@wunner.de>

On Jan 04, 2024 / 13:36, Lukas Wunner wrote:
> On Thu, Jan 04, 2024 at 08:41:28AM +0000, Shinichiro Kawasaki wrote:
> > My mere idea was to just blacklist Intel CPUs with family != 6.
> 
> The P2SB device has Vendor ID 0x8086, Device ID 0xc5c5, so just match
> for that?  The IDE controller in question has [8086:244b].  Class codes
> also differ, so that would be another suitable method for differentiation.

Lukas,

Thank you for the idea. I refereed ICH/PCH documents and found that Device IDs
of IDE controller and P2SB at DEVFN(31,1) are different depending on ICH/PCH
version. It looks troublesome to list them all. On the other hand, class code of
P2SB looks same across PCH versions: 58000h is the class code. So I think and
hope that the class code check will be the best solution. If anyone in Intel can
confirm that all P2SB devices have the same class code, it will be great.

Klara,

Thank you very much for confirming that p2sb_bar() is not called on your system.
As the next step, I would like to try out the solution idea by Lukas. Could you
apply the patch below on top of the kernel v6.7-rc8 and see if the IDE
controller detection failure is resolved?

diff --git a/drivers/platform/x86/p2sb.c b/drivers/platform/x86/p2sb.c
index fcf1ce8bbdc5..e82ab2ddd74b 100644
--- a/drivers/platform/x86/p2sb.c
+++ b/drivers/platform/x86/p2sb.c
@@ -26,6 +26,8 @@ static const struct x86_cpu_id p2sb_cpu_ids[] = {
 	{}
 };
 
+#define P2SB_CLASS_CODE		0x58000
+
 /*
  * Cache BAR0 of P2SB device functions 0 to 7.
  * TODO: The constant 8 is the number of functions that PCI specification
@@ -136,9 +138,10 @@ static struct pci_bus *p2sb_get_bus(struct pci_bus *bus)
 
 static int p2sb_cache_resources(void)
 {
-	struct pci_bus *bus;
 	unsigned int devfn_p2sb;
 	u32 value = P2SBC_HIDE;
+	struct pci_bus *bus;
+	u32 class;
 	int ret;
 
 	/* Get devfn for P2SB device itself */
@@ -150,6 +153,14 @@ static int p2sb_cache_resources(void)
 	if (!bus)
 		return -ENODEV;
 
+	/*
+	 * When a device with same devfn exists and it is not P2SB, do not
+	 * touch it.
+	 */
+	pci_bus_read_config_dword(bus, devfn_p2sb, PCI_CLASS_REVISION, &class);
+	if (!PCI_POSSIBLE_ERROR(class) && class >> 8 != P2SB_CLASS_CODE)
+		return -ENODEV;
+
 	/*
 	 * Prevent concurrent PCI bus scan from seeing the P2SB device and
 	 * removing via sysfs while it is temporarily exposed.


