[PATCH 0/6] PCI/portdrv: Use bus-type functions

[PATCH 0/6] PCI/portdrv: Use bus-type functions

[Uwe Kleine-König]

Hello,

with the eventual goal to remore .probe(), .remove() and .shutdown()
from struct device_driver convert pcie portdrv to use bus-type
callbacks.

The first patch is a fix, but I think it's not relevant as I didn't find
a pcie driver without a remove callback. Feel free to drop the Fixes
line if you think it's not justified and decide yourself if you want it
backported to stable. I have no strong opinion here.

For the complete series there is no intended change in behaviour (apart
from the fix in the first patch :-).

Best regards
Uwe

Uwe Kleine-König (6):
  PCI/portdrv: Fix potential resource leak
  PCI/portdrv: Drop empty shutdown callback
  PCI/portdrv: Don't check for the driver's and device's bus
  PCI/portdrv: Move pcie_port_bus_type to pcie source file
  PCI/portdrv: Don't check for valid device and driver in bus callbacks
  PCI/portdrv: Use bus-type functions

 drivers/pci/pci-driver.c   | 28 -------------------
 drivers/pci/pcie/portdrv.c | 55 +++++++++++++++++++-------------------
 2 files changed, 28 insertions(+), 55 deletions(-)


base-commit: 7d0a66e4bb9081d75c82ec4957c50034cb0ea449
-- 
2.47.3

[PATCH 1/6] PCI/portdrv: Fix potential resource leak

[Uwe Kleine-König]

pcie_port_probe_service() unconditionally calls get_device() (unless it
fails). So drop that reference also unconditionally as it's fine for a
pcie driver to not have a remove callback.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
This isn't very urgent as I think there is no pcie driver without a
remove callback
---
 drivers/pci/pcie/portdrv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index d1b68c18444f..f13039378908 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -557,10 +557,10 @@ static int pcie_port_remove_service(struct device *dev)
 
 	pciedev = to_pcie_device(dev);
 	driver = to_service_driver(dev->driver);
-	if (driver && driver->remove) {
+	if (driver && driver->remove)
 		driver->remove(pciedev);
-		put_device(dev);
-	}
+
+	put_device(dev);
 	return 0;
 }
 
-- 
2.47.3

[PATCH 2/6] PCI/portdrv: Drop empty shutdown callback

[Uwe Kleine-König]

.shutdown() is an optional callback and the core only calls it if the
pointer in struct device_driver is non-NULL. So make nothing in a bit
shorter time and remove the empty function.

Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
---
 drivers/pci/pcie/portdrv.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index f13039378908..63492c3d3565 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -564,17 +564,6 @@ static int pcie_port_remove_service(struct device *dev)
 	return 0;
 }
 
-/**
- * pcie_port_shutdown_service - shut down given PCI Express port service
- * @dev: PCI Express port service device to handle
- *
- * If PCI Express port service driver is registered with
- * pcie_port_service_register(), this function will be called by the driver core
- * when device_shutdown() is called for the port service device associated
- * with the driver.
- */
-static void pcie_port_shutdown_service(struct device *dev) {}
-
 /**
  * pcie_port_service_register - register PCI Express port service driver
  * @new: PCI Express port service driver to register
@@ -588,7 +577,6 @@ int pcie_port_service_register(struct pcie_port_service_driver *new)
 	new->driver.bus = &pcie_port_bus_type;
 	new->driver.probe = pcie_port_probe_service;
 	new->driver.remove = pcie_port_remove_service;
-	new->driver.shutdown = pcie_port_shutdown_service;
 
 	return driver_register(&new->driver);
 }
-- 
2.47.3

[PATCH 3/6] PCI/portdrv: Don't check for the driver's and device's bus

[Uwe Kleine-König]

The driver core ensures that the match function is only called for
drivers and devices of the right bus. So drop the useless check.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pci/pci-driver.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 302d61783f6c..2b6a628fc7d0 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1702,14 +1702,8 @@ EXPORT_SYMBOL(pci_bus_type);
 #ifdef CONFIG_PCIEPORTBUS
 static int pcie_port_bus_match(struct device *dev, const struct device_driver *drv)
 {
-	struct pcie_device *pciedev;
-	const 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);
+	struct pcie_device *pciedev = to_pcie_device(dev);
+	const struct pcie_port_service_driver *driver = to_service_driver(drv);
 
 	if (driver->service != pciedev->service)
 		return 0;
-- 
2.47.3

[PATCH 4/6] PCI/portdrv: Move pcie_port_bus_type to pcie source file

[Uwe Kleine-König]

Conceptually the pci_express bus doesn't belong in generic pci code.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pci/pci-driver.c   | 22 ----------------------
 drivers/pci/pcie/portdrv.c | 20 ++++++++++++++++++++
 2 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 2b6a628fc7d0..addb1d226a25 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1699,28 +1699,6 @@ const struct bus_type pci_bus_type = {
 };
 EXPORT_SYMBOL(pci_bus_type);
 
-#ifdef CONFIG_PCIEPORTBUS
-static int pcie_port_bus_match(struct device *dev, const struct device_driver *drv)
-{
-	struct pcie_device *pciedev = to_pcie_device(dev);
-	const struct pcie_port_service_driver *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;
-}
-
-const struct bus_type pcie_port_bus_type = {
-	.name		= "pci_express",
-	.match		= pcie_port_bus_match,
-};
-#endif
-
 static int __init pci_driver_init(void)
 {
 	int ret;
diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index 63492c3d3565..5cb0daf6781b 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -508,6 +508,21 @@ static void pcie_port_device_remove(struct pci_dev *dev)
 	pci_free_irq_vectors(dev);
 }
 
+static int pcie_port_bus_match(struct device *dev, const struct device_driver *drv)
+{
+	struct pcie_device *pciedev = to_pcie_device(dev);
+	const struct pcie_port_service_driver *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;
+}
+
 /**
  * pcie_port_probe_service - probe driver for given PCI Express port service
  * @dev: PCI Express port service device to probe against
@@ -564,6 +579,11 @@ static int pcie_port_remove_service(struct device *dev)
 	return 0;
 }
 
+const struct bus_type pcie_port_bus_type = {
+	.name = "pci_express",
+	.match = pcie_port_bus_match,
+};
+
 /**
  * pcie_port_service_register - register PCI Express port service driver
  * @new: PCI Express port service driver to register
-- 
2.47.3

[PATCH 5/6] PCI/portdrv: Don't check for valid device and driver in bus callbacks

[Uwe Kleine-König]

The driver core ensures that in .probe() and .remove() both dev and
dev->driver are valid. So drop the respective check.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pci/pcie/portdrv.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index 5cb0daf6781b..f164fbd0884c 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -537,9 +537,6 @@ static int pcie_port_probe_service(struct device *dev)
 	struct pcie_port_service_driver *driver;
 	int status;
 
-	if (!dev || !dev->driver)
-		return -ENODEV;
-
 	driver = to_service_driver(dev->driver);
 	if (!driver || !driver->probe)
 		return -ENODEV;
@@ -567,9 +564,6 @@ static int pcie_port_remove_service(struct device *dev)
 	struct pcie_device *pciedev;
 	struct pcie_port_service_driver *driver;
 
-	if (!dev || !dev->driver)
-		return 0;
-
 	pciedev = to_pcie_device(dev);
 	driver = to_service_driver(dev->driver);
 	if (driver && driver->remove)
-- 
2.47.3

[PATCH 6/6] PCI/portdrv: Use bus-type functions

[Uwe Kleine-König]

Instead of assigning the probe function for each driver individually,
use .probe and .remove from the pci_express bus. Rename the functions
for consistency.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pci/pcie/portdrv.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index f164fbd0884c..87149075bc5a 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -524,14 +524,14 @@ static int pcie_port_bus_match(struct device *dev, const struct device_driver *d
 }
 
 /**
- * pcie_port_probe_service - probe driver for given PCI Express port service
+ * pcie_port_bus_probe - probe driver for given PCI Express port service
  * @dev: PCI Express port service device to probe against
  *
  * If PCI Express port service driver is registered with
  * pcie_port_service_register(), this function will be called by the driver core
  * whenever match is found between the driver and a port service device.
  */
-static int pcie_port_probe_service(struct device *dev)
+static int pcie_port_bus_probe(struct device *dev)
 {
 	struct pcie_device *pciedev;
 	struct pcie_port_service_driver *driver;
@@ -551,7 +551,7 @@ static int pcie_port_probe_service(struct device *dev)
 }
 
 /**
- * pcie_port_remove_service - detach driver from given PCI Express port service
+ * pcie_port_bus_remove - detach driver from given PCI Express port service
  * @dev: PCI Express port service device to handle
  *
  * If PCI Express port service driver is registered with
@@ -559,7 +559,7 @@ static int pcie_port_probe_service(struct device *dev)
  * when device_unregister() is called for the port service device associated
  * with the driver.
  */
-static int pcie_port_remove_service(struct device *dev)
+static void pcie_port_bus_remove(struct device *dev)
 {
 	struct pcie_device *pciedev;
 	struct pcie_port_service_driver *driver;
@@ -570,12 +570,13 @@ static int pcie_port_remove_service(struct device *dev)
 		driver->remove(pciedev);
 
 	put_device(dev);
-	return 0;
 }
 
 const struct bus_type pcie_port_bus_type = {
 	.name = "pci_express",
 	.match = pcie_port_bus_match,
+	.probe = pcie_port_bus_probe,
+	.remove = pcie_port_bus_remove,
 };
 
 /**
@@ -589,8 +590,6 @@ int pcie_port_service_register(struct pcie_port_service_driver *new)
 
 	new->driver.name = new->name;
 	new->driver.bus = &pcie_port_bus_type;
-	new->driver.probe = pcie_port_probe_service;
-	new->driver.remove = pcie_port_remove_service;
 
 	return driver_register(&new->driver);
 }
-- 
2.47.3

Re: [PATCH 2/6] PCI/portdrv: Drop empty shutdown callback

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]

On Tue, Dec 02, 2025 at 04:13:50PM +0100, Uwe Kleine-König wrote:
> .shutdown() is an optional callback and the core only calls it if the
> pointer in struct device_driver is non-NULL. So make nothing in a bit
> shorter time and remove the empty function.
> 
> Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>

I got the email address wrong here. If this version is picked up, can
you please make this 

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

to match the author? I'll fix it in my tree to get this right for v2 (if
that will happen).

Thanks
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/6] PCI/portdrv: Fix potential resource leak

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]

On Tue, 2 Dec 2025, Uwe Kleine-König wrote:

> pcie_port_probe_service() unconditionally calls get_device() (unless it
> fails). So drop that reference also unconditionally as it's fine for a
> pcie driver to not have a remove callback.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> This isn't very urgent as I think there is no pcie driver without a
> remove callback
> ---
>  drivers/pci/pcie/portdrv.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index d1b68c18444f..f13039378908 100644
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -557,10 +557,10 @@ static int pcie_port_remove_service(struct device *dev)
>  
>  	pciedev = to_pcie_device(dev);
>  	driver = to_service_driver(dev->driver);
> -	if (driver && driver->remove) {
> +	if (driver && driver->remove)
>  		driver->remove(pciedev);
> -		put_device(dev);
> -	}
> +
> +	put_device(dev);
>  	return 0;
>  }
>  
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH 2/6] PCI/portdrv: Drop empty shutdown callback

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]

On Tue, 2 Dec 2025, Uwe Kleine-König wrote:

> .shutdown() is an optional callback and the core only calls it if the
> pointer in struct device_driver is non-NULL. So make nothing in a bit
> shorter time and remove the empty function.
> 
> Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
> ---
>  drivers/pci/pcie/portdrv.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index f13039378908..63492c3d3565 100644
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -564,17 +564,6 @@ static int pcie_port_remove_service(struct device *dev)
>  	return 0;
>  }
>  
> -/**
> - * pcie_port_shutdown_service - shut down given PCI Express port service
> - * @dev: PCI Express port service device to handle
> - *
> - * If PCI Express port service driver is registered with
> - * pcie_port_service_register(), this function will be called by the driver core
> - * when device_shutdown() is called for the port service device associated
> - * with the driver.
> - */
> -static void pcie_port_shutdown_service(struct device *dev) {}
> -
>  /**
>   * pcie_port_service_register - register PCI Express port service driver
>   * @new: PCI Express port service driver to register
> @@ -588,7 +577,6 @@ int pcie_port_service_register(struct pcie_port_service_driver *new)
>  	new->driver.bus = &pcie_port_bus_type;
>  	new->driver.probe = pcie_port_probe_service;
>  	new->driver.remove = pcie_port_remove_service;
> -	new->driver.shutdown = pcie_port_shutdown_service;
>  
>  	return driver_register(&new->driver);
>  }
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH 4/6] PCI/portdrv: Move pcie_port_bus_type to pcie source file

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2829 bytes --]

On Tue, 2 Dec 2025, Uwe Kleine-König wrote:

> Conceptually the pci_express bus doesn't belong in generic pci code.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
>  drivers/pci/pci-driver.c   | 22 ----------------------
>  drivers/pci/pcie/portdrv.c | 20 ++++++++++++++++++++
>  2 files changed, 20 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 2b6a628fc7d0..addb1d226a25 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1699,28 +1699,6 @@ const struct bus_type pci_bus_type = {
>  };
>  EXPORT_SYMBOL(pci_bus_type);
>  
> -#ifdef CONFIG_PCIEPORTBUS
> -static int pcie_port_bus_match(struct device *dev, const struct device_driver *drv)
> -{
> -	struct pcie_device *pciedev = to_pcie_device(dev);
> -	const struct pcie_port_service_driver *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;
> -}
> -
> -const struct bus_type pcie_port_bus_type = {
> -	.name		= "pci_express",
> -	.match		= pcie_port_bus_match,
> -};
> -#endif
> -
>  static int __init pci_driver_init(void)
>  {
>  	int ret;
> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index 63492c3d3565..5cb0daf6781b 100644
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -508,6 +508,21 @@ static void pcie_port_device_remove(struct pci_dev *dev)
>  	pci_free_irq_vectors(dev);
>  }
>  
> +static int pcie_port_bus_match(struct device *dev, const struct device_driver *drv)
> +{
> +	struct pcie_device *pciedev = to_pcie_device(dev);
> +	const struct pcie_port_service_driver *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;
> +}
> +
>  /**
>   * pcie_port_probe_service - probe driver for given PCI Express port service
>   * @dev: PCI Express port service device to probe against
> @@ -564,6 +579,11 @@ static int pcie_port_remove_service(struct device *dev)
>  	return 0;
>  }
>  
> +const struct bus_type pcie_port_bus_type = {
> +	.name = "pci_express",
> +	.match = pcie_port_bus_match,
> +};
> +
>  /**
>   * pcie_port_service_register - register PCI Express port service driver
>   * @new: PCI Express port service driver to register

I wonder if you should also relocate that #ifdef region 
bus_register(&pcie_port_bus_type); is in and make pcie_port_bus_type 
static? As is, this move feels somewhat incomplete.

-- 
 i.

Re: [PATCH 4/6] PCI/portdrv: Move pcie_port_bus_type to pcie source file

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]

Hello Ilpo,

On Thu, Dec 11, 2025 at 07:28:45PM +0200, Ilpo Järvinen wrote:
> On Tue, 2 Dec 2025, Uwe Kleine-König wrote:
> > @@ -564,6 +579,11 @@ static int pcie_port_remove_service(struct device *dev)
> >  	return 0;
> >  }
> >  
> > +const struct bus_type pcie_port_bus_type = {
> > +	.name = "pci_express",
> > +	.match = pcie_port_bus_match,
> > +};
> > +
> >  /**
> >   * pcie_port_service_register - register PCI Express port service driver
> >   * @new: PCI Express port service driver to register
> 
> I wonder if you should also relocate that #ifdef region 
> bus_register(&pcie_port_bus_type); is in and make pcie_port_bus_type 
> static? As is, this move feels somewhat incomplete.

Yeah, I agree, I had the same feeling. I decided against that because
I'd need a new function that just calls bus_register and that has to be
called from pci_driver_init(). I thought this to be hardly better than
the status quo. Looking again pcieportdrv.o is a separate module that
doesn't have an initcall, so this could be done in there. Will have a
look early next week.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/6] PCI/portdrv: Fix potential resource leak

[Jonathan Cameron]

On Tue,  2 Dec 2025 16:13:49 +0100
Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote:

> pcie_port_probe_service() unconditionally calls get_device() (unless it
> fails). So drop that reference also unconditionally as it's fine for a
> pcie driver to not have a remove callback.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> This isn't very urgent as I think there is no pcie driver without a
> remove callback

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> ---
>  drivers/pci/pcie/portdrv.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index d1b68c18444f..f13039378908 100644
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -557,10 +557,10 @@ static int pcie_port_remove_service(struct device *dev)
>  
>  	pciedev = to_pcie_device(dev);
>  	driver = to_service_driver(dev->driver);
> -	if (driver && driver->remove) {
> +	if (driver && driver->remove)
>  		driver->remove(pciedev);
> -		put_device(dev);
> -	}
> +
> +	put_device(dev);
>  	return 0;
>  }
>  

Re: [PATCH 2/6] PCI/portdrv: Drop empty shutdown callback

[Jonathan Cameron]

On Tue,  2 Dec 2025 16:13:50 +0100
Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote:

> .shutdown() is an optional callback and the core only calls it if the
> pointer in struct device_driver is non-NULL. So make nothing in a bit
> shorter time and remove the empty function.
> 
> Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Re: [PATCH 3/6] PCI/portdrv: Don't check for the driver's and device's bus

[Jonathan Cameron]

On Tue,  2 Dec 2025 16:13:51 +0100
Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote:

> The driver core ensures that the match function is only called for
> drivers and devices of the right bus. So drop the useless check.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

A lot of stuff would be broken if match functions were called 
on wrong types of devices :(

Re: [PATCH 5/6] PCI/portdrv: Don't check for valid device and driver in bus callbacks

[Jonathan Cameron]

On Tue,  2 Dec 2025 16:13:53 +0100
Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote:

> The driver core ensures that in .probe() and .remove() both dev and
> dev->driver are valid. So drop the respective check.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Re: [PATCH 6/6] PCI/portdrv: Use bus-type functions

[Jonathan Cameron]

On Tue,  2 Dec 2025 16:13:54 +0100
Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote:

> Instead of assigning the probe function for each driver individually,
> use .probe and .remove from the pci_express bus. Rename the functions
> for consistency.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Makes sense.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Nice little cleanup series.  Thanks!

Re: [PATCH 4/6] PCI/portdrv: Move pcie_port_bus_type to pcie source file

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2859 bytes --]

On Fri, Dec 12, 2025 at 11:35:40PM +0100, Uwe Kleine-König wrote:
> Hello Ilpo,
> 
> On Thu, Dec 11, 2025 at 07:28:45PM +0200, Ilpo Järvinen wrote:
> > On Tue, 2 Dec 2025, Uwe Kleine-König wrote:
> > > @@ -564,6 +579,11 @@ static int pcie_port_remove_service(struct device *dev)
> > >  	return 0;
> > >  }
> > >  
> > > +const struct bus_type pcie_port_bus_type = {
> > > +	.name = "pci_express",
> > > +	.match = pcie_port_bus_match,
> > > +};
> > > +
> > >  /**
> > >   * pcie_port_service_register - register PCI Express port service driver
> > >   * @new: PCI Express port service driver to register
> > 
> > I wonder if you should also relocate that #ifdef region 
> > bus_register(&pcie_port_bus_type); is in and make pcie_port_bus_type 
> > static? As is, this move feels somewhat incomplete.
> 
> Yeah, I agree, I had the same feeling. I decided against that because
> I'd need a new function that just calls bus_register and that has to be
> called from pci_driver_init(). I thought this to be hardly better than
> the status quo. Looking again pcieportdrv.o is a separate module that
> doesn't have an initcall, so this could be done in there. Will have a
> look early next week.

This is wrong for two reasons. a) there is an initcall in
drivers/pci/pcie/portdrv.c, and b) it took me longer than "next week".

So the move could be completed by:

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 2cc4e9e6f5ef..23316b5f1491 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1709,11 +1709,6 @@ static int __init pci_driver_init(void)
 	if (ret)
 		return ret;
 
-#ifdef CONFIG_PCIEPORTBUS
-	ret = bus_register(&pcie_port_bus_type);
-	if (ret)
-		return ret;
-#endif
 	dma_debug_add_bus(&pci_bus_type);
 	return 0;
 }
diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index 88af0dacf351..3382d50ecb3e 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -838,9 +838,15 @@ static void __init pcie_init_services(void)
 
 static int __init pcie_portdrv_init(void)
 {
+	int ret;
+
 	if (pcie_ports_disabled)
 		return -EACCES;
 
+	ret = bus_register(&pcie_port_bus_type);
+	if (ret)
+		return ret;
+
 	pcie_init_services();
 	dmi_check_system(pcie_portdrv_dmi_table);
 
But I'd like to have this postponed and in a separate patch because
pci_driver_init() is a postcore_initcall and pcie_portdrv_init is only a
normal device_initcall(). So it definitively changes ordering and I'd
fear regressions. It would be sad if patches #5 and #6 would need to be
reverted to fix a regression in this area.

So given this is the only open issue in this series I'm aware of, can we
please get this in? Then I'll send a patch for the above change during
the next development cycle.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/6] PCI/portdrv: Use bus-type functions

[Bjorn Helgaas]

On Tue, Dec 02, 2025 at 04:13:48PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> with the eventual goal to remore .probe(), .remove() and .shutdown()
> from struct device_driver convert pcie portdrv to use bus-type
> callbacks.
> 
> The first patch is a fix, but I think it's not relevant as I didn't find
> a pcie driver without a remove callback. Feel free to drop the Fixes
> line if you think it's not justified and decide yourself if you want it
> backported to stable. I have no strong opinion here.
> 
> For the complete series there is no intended change in behaviour (apart
> from the fix in the first patch :-).
> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (6):
>   PCI/portdrv: Fix potential resource leak
>   PCI/portdrv: Drop empty shutdown callback
>   PCI/portdrv: Don't check for the driver's and device's bus
>   PCI/portdrv: Move pcie_port_bus_type to pcie source file
>   PCI/portdrv: Don't check for valid device and driver in bus callbacks
>   PCI/portdrv: Use bus-type functions
> 
>  drivers/pci/pci-driver.c   | 28 -------------------
>  drivers/pci/pcie/portdrv.c | 55 +++++++++++++++++++-------------------
>  2 files changed, 28 insertions(+), 55 deletions(-)

Applied to pci/portdrv for v6.20, thanks for doing this!