[RFC 0/2] platform/x86: p2sb: Cache SPI controller BAR on Goldmont

[RFC 0/2] platform/x86: p2sb: Cache SPI controller BAR on Goldmont

[Hans de Goede]

Hi All,

Here are 2 follow-up patches to "[PATCH v3] platform/x86: p2sb: Defer P2SB
device scan when P2SB device has func 0".

These are marked as RFC because I hope that we can replace that entire
patch with the patch from "[RFC 1/1] platform/x86: p2sb: On Goldmont only
cache P2SB and SPI devfn BAR", but it is not sure yet if that approach
to fixing the issue at hand actually works :)

Regards,

Hans


Hans de Goede (2):
  platform/x86: p2sb: Cache SPI controller resources on Goldmont
    platforms
  platform/x86: p2sb: Make p2sb_get_devfn() return void

 drivers/platform/x86/p2sb.c | 59 +++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

-- 
2.44.0

[RFC 1/2] platform/x86: p2sb: Cache SPI controller resources on Goldmont platforms

[Hans de Goede]

Instead of skipping all fs_initcall() caching of resources on Goldmont
platforms, cache the SPI controller BAR. Caching the SPI controller BAR
seems safe to do (unlike the P2SB BAR which is known to cause issues
on Goldmont).

Caching the SPI controller BAR is useful since p2sb_bar() normally
is always called for the SPI devfn on Goldmont platforms.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/p2sb.c | 41 ++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/p2sb.c b/drivers/platform/x86/p2sb.c
index c829dbd8f058..caf037b44487 100644
--- a/drivers/platform/x86/p2sb.c
+++ b/drivers/platform/x86/p2sb.c
@@ -20,9 +20,11 @@
 #define P2SBC_HIDE		BIT(8)
 
 #define P2SB_DEVFN_DEFAULT	PCI_DEVFN(31, 1)
+#define P2SB_DEVFN_GOLDMONT	PCI_DEVFN(13, 0)
+#define SPI_DEVFN_GOLDMONT	PCI_DEVFN(13, 2)
 
 static const struct x86_cpu_id p2sb_cpu_ids[] = {
-	X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT,	PCI_DEVFN(13, 0)),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT, P2SB_DEVFN_GOLDMONT),
 	{}
 };
 
@@ -109,7 +111,7 @@ static struct pci_bus *p2sb_get_bus(struct pci_bus *bus)
 	return p2sb_bus;
 }
 
-static int p2sb_cache_resources(unsigned int devfn_to_cache, bool from_fs_init)
+static int p2sb_cache_resources(unsigned int devfn_to_cache)
 {
 	unsigned int devfn_p2sb;
 	u32 value = P2SBC_HIDE;
@@ -126,18 +128,6 @@ static int p2sb_cache_resources(unsigned int devfn_to_cache, bool from_fs_init)
 	if (!bus)
 		return -ENODEV;
 
-	/*
-	 * On ASUS VivoBook D540NV-GQ065T which has Goldmont CPU family Pentium
-	 * N4200, P2SB device scan including function 0 at fs_initcall() step
-	 * causes ACPI errors. To avoid the errors, defer P2SB device scan and
-	 * cache when P2SB devices has function 0.
-	 */
-	if (PCI_FUNC(devfn_p2sb) == 0 && from_fs_init)
-		return -EBUSY;
-
-	if (devfn_to_cache == 0)
-		devfn_to_cache = devfn_p2sb;
-
 	/*
 	 * When a device with same devfn exists and its device class is not
 	 * PCI_CLASS_MEMORY_OTHER for P2SB, do not touch it.
@@ -205,7 +195,7 @@ int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem)
 
 	/* Scan and cache P2SB device if it was deferred at fs_initcall() */
 	if (!p2sb_valid_resource(&cache->res))
-		p2sb_cache_resources(devfn, false);
+		p2sb_cache_resources(devfn);
 
 	if (cache->bus_dev_id != bus->dev.id)
 		return -ENODEV;
@@ -220,7 +210,26 @@ EXPORT_SYMBOL_GPL(p2sb_bar);
 
 static int __init p2sb_fs_init(void)
 {
-	p2sb_cache_resources(0, true);
+	unsigned int devfn;
+
+	/*
+	 * On most platforms p2sb_bar() is only called with the PCI devfn
+	 * of the P2SB PCI device itself, cache the P2SB BAR.
+	 */
+	p2sb_get_devfn(&devfn);
+
+	/*
+	 * On Goldmont platforms p2sb_bar() may be called with 2 different PCI
+	 * devfn values, one for the P2SB itself and one for the SPI controller.
+	 * Unhiding the P2SB is known to cause ACPI errors on some devices
+	 * where p2sb_bar() is never called for the P2SB devfn. This happens
+	 * on e.g. the ASUS VivoBook D540NV-GQ065T with a Pentium N4200 CPU.
+	 * To avoid this issue only cache the SPI BAR at fs_initcall() time.
+	 */
+	if (devfn == P2SB_DEVFN_GOLDMONT)
+		devfn = SPI_DEVFN_GOLDMONT;
+
+	p2sb_cache_resources(devfn);
 	return 0;
 }
 
-- 
2.44.0

[RFC 2/2] platform/x86: p2sb: Make p2sb_get_devfn() return void

[Hans de Goede]

p2sb_get_devfn() always succeeds, make it return void and
remove error checking from its callers.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/p2sb.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/p2sb.c b/drivers/platform/x86/p2sb.c
index caf037b44487..3293c2a4ed6c 100644
--- a/drivers/platform/x86/p2sb.c
+++ b/drivers/platform/x86/p2sb.c
@@ -43,7 +43,7 @@ struct p2sb_res_cache {
 
 static struct p2sb_res_cache p2sb_resources[NR_P2SB_RES_CACHE];
 
-static int p2sb_get_devfn(unsigned int *devfn)
+static void p2sb_get_devfn(unsigned int *devfn)
 {
 	unsigned int fn = P2SB_DEVFN_DEFAULT;
 	const struct x86_cpu_id *id;
@@ -53,7 +53,6 @@ static int p2sb_get_devfn(unsigned int *devfn)
 		fn = (unsigned int)id->driver_data;
 
 	*devfn = fn;
-	return 0;
 }
 
 static bool p2sb_valid_resource(struct resource *res)
@@ -117,12 +116,9 @@ static int p2sb_cache_resources(unsigned int devfn_to_cache)
 	u32 value = P2SBC_HIDE;
 	struct pci_bus *bus;
 	u16 class;
-	int ret;
 
 	/* Get devfn for P2SB device itself */
-	ret = p2sb_get_devfn(&devfn_p2sb);
-	if (ret)
-		return ret;
+	p2sb_get_devfn(&devfn_p2sb);
 
 	bus = p2sb_get_bus(NULL);
 	if (!bus)
@@ -159,7 +155,7 @@ static int p2sb_cache_resources(unsigned int devfn_to_cache)
 
 	pci_unlock_rescan_remove();
 
-	return ret;
+	return 0;
 }
 
 /**
@@ -179,17 +175,13 @@ static int p2sb_cache_resources(unsigned int devfn_to_cache)
 int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem)
 {
 	struct p2sb_res_cache *cache;
-	int ret;
 
 	bus = p2sb_get_bus(bus);
 	if (!bus)
 		return -ENODEV;
 
-	if (!devfn) {
-		ret = p2sb_get_devfn(&devfn);
-		if (ret)
-			return ret;
-	}
+	if (!devfn)
+		p2sb_get_devfn(&devfn);
 
 	cache = &p2sb_resources[PCI_FUNC(devfn)];
 
-- 
2.44.0