On Nov 24, 2023 / 17:22, Andy Shevchenko wrote:
> On Fri, Nov 24, 2023 at 10:49:45AM +0000, Shinichiro Kawasaki wrote:
> > On Nov 14, 2023 / 19:58, Andy Shevchenko wrote:
>
> ...
>
> > I created a patch below and confirmed it avoided the lockdep WARN. The
> > i2c-i801 probe was ok at system boot.
>
> Another possible solution I was thinking about is to have a local cache,
> so, make p2sb.c to be called just after PCI enumeration at boot time
> to cache the P2SB device's bar, and then cache the bar based on the device
> in question at the first call. Yet it may not remove all theoretical /
> possible scenarios with dead lock (taking into account hotpluggable
> devices), but won't fail the i801 driver in the above use case IIUC.
Thanks for the idea. I created an experimental patch below (it does not guard
list nor free the list elements, so it is incomplete). I confirmed that this
patch avoids the deadlock. So your idea looks working. I still observe the
deadlock WARN, but it looks better than the hang by the deadlock.
Having said that, Heiner says in another mail that "A solution has to support
pci drivers using p2sb_bar() in probe()". This idea does not fulfill it. Hmm.
diff --git a/drivers/platform/x86/p2sb.c b/drivers/platform/x86/p2sb.c
index 1cf2471d54d..0e7057979a2 100644
--- a/drivers/platform/x86/p2sb.c
+++ b/drivers/platform/x86/p2sb.c
@@ -26,6 +26,15 @@ static const struct x86_cpu_id p2sb_cpu_ids[] = {
{}
};
+struct p2sb_res_cache {
+ struct list_head entry;
+ u32 bus_dev_id;
+ unsigned int devfn;
+ struct resource res;
+};
+
+static LIST_HEAD(p2sb_res_list);
+
static int p2sb_get_devfn(unsigned int *devfn)
{
unsigned int fn = P2SB_DEVFN_DEFAULT;
@@ -75,26 +84,8 @@ static int p2sb_scan_and_read(struct pci_bus *bus, unsigned int devfn, struct re
return ret;
}
-/**
- * p2sb_bar - Get Primary to Sideband (P2SB) bridge device BAR
- * @bus: PCI bus to communicate with
- * @devfn: PCI slot and function to communicate with
- * @mem: memory resource to be filled in
- *
- * The BIOS prevents the P2SB device from being enumerated by the PCI
- * subsystem, so we need to unhide and hide it back to lookup the BAR.
- *
- * if @bus is NULL, the bus 0 in domain 0 will be used.
- * If @devfn is 0, it will be replaced by devfn of the P2SB device.
- *
- * Caller must provide a valid pointer to @mem.
- *
- * Locking is handled by pci_rescan_remove_lock mutex.
- *
- * Return:
- * 0 on success or appropriate errno value on error.
- */
-int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem)
+static int __p2sb_bar(struct pci_bus *bus, unsigned int devfn,
+ struct resource *mem)
{
struct pci_dev *pdev_p2sb;
unsigned int devfn_p2sb;
@@ -141,4 +132,54 @@ int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem)
return 0;
}
+
+/**
+ * p2sb_bar - Get Primary to Sideband (P2SB) bridge device BAR
+ * @bus: PCI bus to communicate with
+ * @devfn: PCI slot and function to communicate with
+ * @mem: memory resource to be filled in
+ *
+ * The BIOS prevents the P2SB device from being enumerated by the PCI
+ * subsystem, so we need to unhide and hide it back to lookup the BAR.
+ *
+ * if @bus is NULL, the bus 0 in domain 0 will be used.
+ * If @devfn is 0, it will be replaced by devfn of the P2SB device.
+ *
+ * Caller must provide a valid pointer to @mem.
+ *
+ * Locking is handled by pci_rescan_remove_lock mutex.
+ *
+ * Return:
+ * 0 on success or appropriate errno value on error.
+ */
+int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem)
+{
+ int ret;
+ struct p2sb_res_cache *cache;
+ struct resource res;
+
+ list_for_each_entry(cache, &p2sb_res_list, entry) {
+ if (cache->bus_dev_id == bus->dev.id &&
+ cache->devfn == devfn) {
+ memcpy(mem, &cache->res, sizeof(*mem));
+ return 0;
+ }
+ }
+
+ ret = __p2sb_bar(bus, devfn, &res);
+ if (ret)
+ return ret;
+
+ memcpy(mem, &res, sizeof(res));
+
+ cache = kmalloc(sizeof(*cache), GFP_KERNEL);
+ if (cache) {
+ cache->bus_dev_id = bus->dev.id;
+ cache->devfn = devfn;
+ memcpy(&cache->res, &res, sizeof(res));
+ list_add(&cache->entry, &p2sb_res_list);
+ }
+
+ return 0;
+}
EXPORT_SYMBOL_GPL(p2sb_bar);
