Re: [PATCH v1 5/9] PCI/portdrv: Remove pcie_port_bus_type link order dependency
On Wednesday, March 7, 2018 7:13:51 AM CET Bjorn Helgaas wrote:
> From: Bjorn Helgaas
>
> The pcie_port_bus_type must be registered before drivers that depend on it
> can be registered. Those drivers include:
>
> pcied_init()# PCIe native hotplug driver
> aer_service_init() # AER driver
> dpc_service_init() # DPC driver
> pcie_pme_service_init() # PME driver
>
> Previously we registered pcie_port_bus_type from pcie_portdrv_init(), a
> device_initcall. The callers of pcie_port_service_register() (above) are
> also device_initcalls. This is fragile because the device_initcall
> ordering depends on link order, which is not explicit.
>
> Register pcie_port_bus_type from pci_driver_init() along with pci_bus_type.
> This removes the link order dependency between portdrv and the pciehp, AER,
> DPC, and PCIe PME drivers.
>
> Signed-off-by: Bjorn Helgaas
> ---
> drivers/pci/pci-driver.c | 45 +++-
> drivers/pci/pcie/Makefile |2 +
> drivers/pci/pcie/portdrv_bus.c | 56
>
> drivers/pci/pcie/portdrv_pci.c | 13 +
> 4 files changed, 46 insertions(+), 70 deletions(-)
> delete mode 100644 drivers/pci/pcie/portdrv_bus.c
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 38ee7c8b4d1a..4db85a0faf34 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -19,6 +20,7 @@
> #include
> #include
> #include "pci.h"
> +#include "pcie/portdrv.h"
>
> struct pci_dynid {
> struct list_head node;
> @@ -1553,8 +1555,49 @@ struct bus_type pci_bus_type = {
> };
> EXPORT_SYMBOL(pci_bus_type);
>
> +#ifdef CONFIG_PCIEPORTBUS
> +static int pcie_port_bus_match(struct device *dev, struct device_driver *drv)
> +{
> + struct pcie_device *pciedev;
> + struct pcie_port_service_driver *driver;
> +
> + if (drv->bus != &pcie_port_bus_type || dev->bus != &pcie_port_bus_type)
> + return 0;
> +
> + pciedev = to_pcie_device(dev);
> + driver = to_service_driver(drv);
> +
> + if (driver->service != pciedev->service)
> + return 0;
> +
> + if ((driver->port_type != PCIE_ANY_PORT) &&
> + (driver->port_type != pci_pcie_type(pciedev->port)))
> + return 0;
> +
> + return 1;
> +}
> +
> +struct bus_type pcie_port_bus_type = {
> + .name = "pci_express",
> + .match = pcie_port_bus_match,
> +};
> +EXPORT_SYMBOL_GPL(pcie_port_bus_type);
> +#endif
> +
> static int __init pci_driver_init(void)
> {
> - return bus_register(&pci_bus_type);
> + int ret;
> +
> + ret = bus_register(&pci_bus_type);
> + if (ret)
> + return ret;
> +
> +#ifdef CONFIG_PCIEPORTBUS
> + ret = bus_register(&pcie_port_bus_type);
> + if (ret)
> + return ret;
> +#endif
> +
> + return 0;
> }
> postcore_initcall(pci_driver_init);
> diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
> index 223e4c34c29a..e01c10c97b95 100644
> --- a/drivers/pci/pcie/Makefile
> +++ b/drivers/pci/pcie/Makefile
> @@ -6,7 +6,7 @@
> # Build PCI Express ASPM if needed
> obj-$(CONFIG_PCIEASPM) += aspm.o
>
> -pcieportdrv-y:= portdrv_core.o portdrv_pci.o
> portdrv_bus.o
> +pcieportdrv-y:= portdrv_core.o portdrv_pci.o
> pcieportdrv-$(CONFIG_ACPI) += portdrv_acpi.o
>
> obj-$(CONFIG_PCIEPORTBUS)+= pcieportdrv.o
> diff --git a/drivers/pci/pcie/portdrv_bus.c b/drivers/pci/pcie/portdrv_bus.c
> deleted file mode 100644
> index f0fba552a0e2..
> --- a/drivers/pci/pcie/portdrv_bus.c
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * File: portdrv_bus.c
> - * Purpose: PCI Express Port Bus Driver's Bus Overloading Functions
> - *
> - * Copyright (C) 2004 Intel
> - * Copyright (C) Tom Long Nguyen (tom.l.ngu...@intel.com)
> - */
> -
> -#include
> -#include
> -#include
> -#include
> -#include
> -
> -#include
> -#include "portdrv.h"
> -
> -static int pcie_port_bus_match(struct device *dev, struct device_driver
> *drv);
> -
> -struct bus_type pcie_port_bus_type = {
> - .name = "pci_express",
> - .match = pcie_port_bus_match,
> -};
> -EXPORT_SYMBOL_GPL(pcie_port_bus_type);
> -
> -static int pcie_port_bus_match(struct device *dev, struct device_driver *drv)
> -{
> - struct pcie_device *pciedev;
> - struct pcie_port_service_driver *driver;
> -
> - if (drv->bus != &pcie_port_bus_type || dev->bus != &pcie_port_bus_type)
> - return 0;
> -
> - pciedev = to_pcie_device(dev);
> - driver = to_service_driver(drv);
> -
> - if (driver->service != pciedev->service)
> - return 0;
> -
> - if ((driver->port_type != PCIE_ANY_PORT) &&
> - (driver->port_type != pci_pci
[PATCH v1 5/9] PCI/portdrv: Remove pcie_port_bus_type link order dependency
From: Bjorn Helgaas
The pcie_port_bus_type must be registered before drivers that depend on it
can be registered. Those drivers include:
pcied_init()# PCIe native hotplug driver
aer_service_init() # AER driver
dpc_service_init() # DPC driver
pcie_pme_service_init() # PME driver
Previously we registered pcie_port_bus_type from pcie_portdrv_init(), a
device_initcall. The callers of pcie_port_service_register() (above) are
also device_initcalls. This is fragile because the device_initcall
ordering depends on link order, which is not explicit.
Register pcie_port_bus_type from pci_driver_init() along with pci_bus_type.
This removes the link order dependency between portdrv and the pciehp, AER,
DPC, and PCIe PME drivers.
Signed-off-by: Bjorn Helgaas
---
drivers/pci/pci-driver.c | 45 +++-
drivers/pci/pcie/Makefile |2 +
drivers/pci/pcie/portdrv_bus.c | 56
drivers/pci/pcie/portdrv_pci.c | 13 +
4 files changed, 46 insertions(+), 70 deletions(-)
delete mode 100644 drivers/pci/pcie/portdrv_bus.c
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 38ee7c8b4d1a..4db85a0faf34 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#include
#include
@@ -19,6 +20,7 @@
#include
#include
#include "pci.h"
+#include "pcie/portdrv.h"
struct pci_dynid {
struct list_head node;
@@ -1553,8 +1555,49 @@ struct bus_type pci_bus_type = {
};
EXPORT_SYMBOL(pci_bus_type);
+#ifdef CONFIG_PCIEPORTBUS
+static int pcie_port_bus_match(struct device *dev, struct device_driver *drv)
+{
+ struct pcie_device *pciedev;
+ struct pcie_port_service_driver *driver;
+
+ if (drv->bus != &pcie_port_bus_type || dev->bus != &pcie_port_bus_type)
+ return 0;
+
+ pciedev = to_pcie_device(dev);
+ driver = to_service_driver(drv);
+
+ if (driver->service != pciedev->service)
+ return 0;
+
+ if ((driver->port_type != PCIE_ANY_PORT) &&
+ (driver->port_type != pci_pcie_type(pciedev->port)))
+ return 0;
+
+ return 1;
+}
+
+struct bus_type pcie_port_bus_type = {
+ .name = "pci_express",
+ .match = pcie_port_bus_match,
+};
+EXPORT_SYMBOL_GPL(pcie_port_bus_type);
+#endif
+
static int __init pci_driver_init(void)
{
- return bus_register(&pci_bus_type);
+ int ret;
+
+ ret = bus_register(&pci_bus_type);
+ if (ret)
+ return ret;
+
+#ifdef CONFIG_PCIEPORTBUS
+ ret = bus_register(&pcie_port_bus_type);
+ if (ret)
+ return ret;
+#endif
+
+ return 0;
}
postcore_initcall(pci_driver_init);
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index 223e4c34c29a..e01c10c97b95 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -6,7 +6,7 @@
# Build PCI Express ASPM if needed
obj-$(CONFIG_PCIEASPM) += aspm.o
-pcieportdrv-y := portdrv_core.o portdrv_pci.o portdrv_bus.o
+pcieportdrv-y := portdrv_core.o portdrv_pci.o
pcieportdrv-$(CONFIG_ACPI) += portdrv_acpi.o
obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o
diff --git a/drivers/pci/pcie/portdrv_bus.c b/drivers/pci/pcie/portdrv_bus.c
deleted file mode 100644
index f0fba552a0e2..
--- a/drivers/pci/pcie/portdrv_bus.c
+++ /dev/null
@@ -1,56 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * File: portdrv_bus.c
- * Purpose:PCI Express Port Bus Driver's Bus Overloading Functions
- *
- * Copyright (C) 2004 Intel
- * Copyright (C) Tom Long Nguyen (tom.l.ngu...@intel.com)
- */
-
-#include
-#include
-#include
-#include
-#include
-
-#include
-#include "portdrv.h"
-
-static int pcie_port_bus_match(struct device *dev, struct device_driver *drv);
-
-struct bus_type pcie_port_bus_type = {
- .name = "pci_express",
- .match = pcie_port_bus_match,
-};
-EXPORT_SYMBOL_GPL(pcie_port_bus_type);
-
-static int pcie_port_bus_match(struct device *dev, struct device_driver *drv)
-{
- struct pcie_device *pciedev;
- struct pcie_port_service_driver *driver;
-
- if (drv->bus != &pcie_port_bus_type || dev->bus != &pcie_port_bus_type)
- return 0;
-
- pciedev = to_pcie_device(dev);
- driver = to_service_driver(drv);
-
- if (driver->service != pciedev->service)
- return 0;
-
- if ((driver->port_type != PCIE_ANY_PORT) &&
- (driver->port_type != pci_pcie_type(pciedev->port)))
- return 0;
-
- return 1;
-}
-
-int pcie_port_bus_register(void)
-{
- return bus_register(&pcie_port_bus_type);
-}
-
-void pcie_port_bus_unregister(void)
-{
- bus_unregister(&pcie_port_bus_type);
-}
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
