[PATCH] PCI: host-generic: Convert to platform remove callback returning void

[PATCH] PCI: host-generic: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code.  However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

pci_host_common_remove() returned zero unconditionally. With that
converted to return void instead, the generic pci host driver can be
switched to .remove_new trivially.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pci/controller/pci-host-common.c  | 4 +---
 drivers/pci/controller/pci-host-generic.c | 2 +-
 include/linux/pci-ecam.h                  | 2 +-
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index 6be3266cd7b5..45b71806182d 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -85,7 +85,7 @@ int pci_host_common_probe(struct platform_device *pdev)
 }
 EXPORT_SYMBOL_GPL(pci_host_common_probe);
 
-int pci_host_common_remove(struct platform_device *pdev)
+void pci_host_common_remove(struct platform_device *pdev)
 {
 	struct pci_host_bridge *bridge = platform_get_drvdata(pdev);
 
@@ -93,8 +93,6 @@ int pci_host_common_remove(struct platform_device *pdev)
 	pci_stop_root_bus(bridge->bus);
 	pci_remove_root_bus(bridge->bus);
 	pci_unlock_rescan_remove();
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(pci_host_common_remove);
 
diff --git a/drivers/pci/controller/pci-host-generic.c b/drivers/pci/controller/pci-host-generic.c
index 63865aeb636b..41cb6a057f6e 100644
--- a/drivers/pci/controller/pci-host-generic.c
+++ b/drivers/pci/controller/pci-host-generic.c
@@ -82,7 +82,7 @@ static struct platform_driver gen_pci_driver = {
 		.of_match_table = gen_pci_of_match,
 	},
 	.probe = pci_host_common_probe,
-	.remove = pci_host_common_remove,
+	.remove_new = pci_host_common_remove,
 };
 module_platform_driver(gen_pci_driver);
 
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
index 6b1301e2498e..3a4860bd2758 100644
--- a/include/linux/pci-ecam.h
+++ b/include/linux/pci-ecam.h
@@ -93,6 +93,6 @@ extern const struct pci_ecam_ops loongson_pci_ecam_ops; /* Loongson PCIe */
 #if IS_ENABLED(CONFIG_PCI_HOST_COMMON)
 /* for DT-based PCI controllers that support ECAM */
 int pci_host_common_probe(struct platform_device *pdev);
-int pci_host_common_remove(struct platform_device *pdev);
+void pci_host_common_remove(struct platform_device *pdev);
 #endif
 #endif

base-commit: 2030579113a1b1b5bfd7ff24c0852847836d8fd1
-- 
2.40.0.353.g56adddaa06d3

Re: [PATCH] PCI: host-generic: Convert to platform remove callback returning void

[Will Deacon]

On Fri, Oct 20, 2023 at 11:21:07AM +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code.  However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> pci_host_common_remove() returned zero unconditionally. With that
> converted to return void instead, the generic pci host driver can be
> switched to .remove_new trivially.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/pci/controller/pci-host-common.c  | 4 +---
>  drivers/pci/controller/pci-host-generic.c | 2 +-
>  include/linux/pci-ecam.h                  | 2 +-
>  3 files changed, 3 insertions(+), 5 deletions(-)

Acked-by: Will Deacon <will@kernel.org>

Will

Re: [PATCH] PCI: host-generic: Convert to platform remove callback returning void

[Uwe Kleine-König]

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

Hello,

On Fri, Oct 20, 2023 at 11:21:07AM +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code.  However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> pci_host_common_remove() returned zero unconditionally. With that
> converted to return void instead, the generic pci host driver can be
> switched to .remove_new trivially.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

who feels responsible to apply this patch?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH] PCI: host-generic: Convert to platform remove callback returning void

[Bjorn Helgaas]

On Mon, Nov 20, 2023 at 10:22:24PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> On Fri, Oct 20, 2023 at 11:21:07AM +0200, Uwe Kleine-König wrote:
> > The .remove() callback for a platform driver returns an int which makes
> > many driver authors wrongly assume it's possible to do error handling by
> > returning an error code.  However the value returned is (mostly) ignored
> > and this typically results in resource leaks. To improve here there is a
> > quest to make the remove callback return void. In the first step of this
> > quest all drivers are converted to .remove_new() which already returns
> > void.
> > 
> > pci_host_common_remove() returned zero unconditionally. With that
> > converted to return void instead, the generic pci host driver can be
> > switched to .remove_new trivially.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> who feels responsible to apply this patch?

If you're ready to rename .remove_new() back to .remove(), you can
include this as part of that series with my ack:

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Or we can take it via the PCI tree for v6.8.

Bjorn

Re: [PATCH] PCI: host-generic: Convert to platform remove callback returning void

[Uwe Kleine-König]

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

Hello Bjorn,

On Mon, Nov 20, 2023 at 03:30:07PM -0600, Bjorn Helgaas wrote:
> On Mon, Nov 20, 2023 at 10:22:24PM +0100, Uwe Kleine-König wrote:
> > On Fri, Oct 20, 2023 at 11:21:07AM +0200, Uwe Kleine-König wrote:
> > > The .remove() callback for a platform driver returns an int which makes
> > > many driver authors wrongly assume it's possible to do error handling by
> > > returning an error code.  However the value returned is (mostly) ignored
> > > and this typically results in resource leaks. To improve here there is a
> > > quest to make the remove callback return void. In the first step of this
> > > quest all drivers are converted to .remove_new() which already returns
> > > void.
> > > 
> > > pci_host_common_remove() returned zero unconditionally. With that
> > > converted to return void instead, the generic pci host driver can be
> > > switched to .remove_new trivially.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 
> > who feels responsible to apply this patch?
> 
> If you're ready to rename .remove_new() back to .remove(), you can
> include this as part of that series with my ack:
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> Or we can take it via the PCI tree for v6.8.

The idea is that all drivers are converted to .remove_new() before
changing .remove() to return void. This way the commit changing struct
platform_driver doesn't has to touch the 1000+ platform drivers. So if
you take this patch via pci in the next merge window, that would be
good.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH] PCI: host-generic: Convert to platform remove callback returning void

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>


On Fri, 20 Oct 2023 11:21:07 +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code.  However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> [...]

Applied to "enumeration" for v6.8, thanks!

[1/1] PCI: host-generic: Convert to platform remove callback returning void
      commit: d9dcdb4531fe39ce48919ef8c2c9369ee49f3ad2

Best regards,
-- 
Bjorn Helgaas <bhelgaas@google.com>

Re: [PATCH] PCI: host-generic: Convert to platform remove callback returning void

[Uwe Kleine-König]

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

On Mon, Nov 20, 2023 at 03:56:18PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> 
> On Fri, 20 Oct 2023 11:21:07 +0200, Uwe Kleine-König wrote:
> > The .remove() callback for a platform driver returns an int which makes
> > many driver authors wrongly assume it's possible to do error handling by
> > returning an error code.  However the value returned is (mostly) ignored
> > and this typically results in resource leaks. To improve here there is a
> > quest to make the remove callback return void. In the first step of this
> > quest all drivers are converted to .remove_new() which already returns
> > void.
> > 
> > [...]
> 
> Applied to "enumeration" for v6.8, thanks!
> 
> [1/1] PCI: host-generic: Convert to platform remove callback returning void
>       commit: d9dcdb4531fe39ce48919ef8c2c9369ee49f3ad2

Thanks! This branch isn't included in next. Is this on purpose?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH] PCI: host-generic: Convert to platform remove callback returning void

[Bjorn Helgaas]

[+to lkp]

On Thu, Nov 23, 2023 at 06:12:36PM +0100, Uwe Kleine-König wrote:
> On Mon, Nov 20, 2023 at 03:56:18PM -0600, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> > On Fri, 20 Oct 2023 11:21:07 +0200, Uwe Kleine-König wrote:
> > > The .remove() callback for a platform driver returns an int which makes
> > > many driver authors wrongly assume it's possible to do error handling by
> > > returning an error code.  However the value returned is (mostly) ignored
> > > and this typically results in resource leaks. To improve here there is a
> > > quest to make the remove callback return void. In the first step of this
> > > quest all drivers are converted to .remove_new() which already returns
> > > void.
> > > 
> > > [...]
> > 
> > Applied to "enumeration" for v6.8, thanks!
> > 
> > [1/1] PCI: host-generic: Convert to platform remove callback returning void
> >       commit: d9dcdb4531fe39ce48919ef8c2c9369ee49f3ad2
> 
> Thanks! This branch isn't included in next. Is this on purpose?

To reduce the workload of the folks maintaining "next", I wait for the
0-day bot to build test a branch before merging it into the PCI "next"
branch.

That usually takes a couple days after I push a branch to the
kernel.org repo.  I have these branches that are currently waiting to
be put into next:

  ecam            pushed 11/20     bot response 11/22
  resource        pushed 11/20     no bot response yet
  enumeration     pushed 11/20     no bot response yet
  p2pdma          pushed 11/20     no bot response yet
  switchtec       pushed 11/22     no bot response yet

Not sure if the 0-day bot is slower than usual right now, but I cc'd
the LKP folks in case something is wrong.

Bjorn

Re: [PATCH] PCI: host-generic: Convert to platform remove callback returning void

[Philip Li]

On Thu, Nov 23, 2023 at 09:25:35PM -0600, Bjorn Helgaas wrote:
> [+to lkp]
> 
> On Thu, Nov 23, 2023 at 06:12:36PM +0100, Uwe Kleine-König wrote:
> > On Mon, Nov 20, 2023 at 03:56:18PM -0600, Bjorn Helgaas wrote:
> > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > On Fri, 20 Oct 2023 11:21:07 +0200, Uwe Kleine-König wrote:
> > > > The .remove() callback for a platform driver returns an int which makes
> > > > many driver authors wrongly assume it's possible to do error handling by
> > > > returning an error code.  However the value returned is (mostly) ignored
> > > > and this typically results in resource leaks. To improve here there is a
> > > > quest to make the remove callback return void. In the first step of this
> > > > quest all drivers are converted to .remove_new() which already returns
> > > > void.
> > > > 
> > > > [...]
> > > 
> > > Applied to "enumeration" for v6.8, thanks!
> > > 
> > > [1/1] PCI: host-generic: Convert to platform remove callback returning void
> > >       commit: d9dcdb4531fe39ce48919ef8c2c9369ee49f3ad2
> > 
> > Thanks! This branch isn't included in next. Is this on purpose?
> 
> To reduce the workload of the folks maintaining "next", I wait for the
> 0-day bot to build test a branch before merging it into the PCI "next"
> branch.
> 
> That usually takes a couple days after I push a branch to the
> kernel.org repo.  I have these branches that are currently waiting to
> be put into next:
> 
>   ecam            pushed 11/20     bot response 11/22
>   resource        pushed 11/20     no bot response yet
>   enumeration     pushed 11/20     no bot response yet
>   p2pdma          pushed 11/20     no bot response yet
>   switchtec       pushed 11/22     no bot response yet
> 
> Not sure if the 0-day bot is slower than usual right now, but I cc'd
> the LKP folks in case something is wrong.

Sorry about the recent slowness, Bjorn. This is our side problem, we met some
issues and the whole cluster can't achieve its usual utilization. We are still
working on resolving the issues, the report time would be delayed, though hard
to tell right now the accurate time.

We will check the status of these branches, if they are in low risk, we will
send our the current test status.

> 
> Bjorn
> 

Re: [PATCH] PCI: host-generic: Convert to platform remove callback returning void

[Uwe Kleine-König]

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

Hello Bjorn,

On Thu, Nov 23, 2023 at 09:25:35PM -0600, Bjorn Helgaas wrote:
> On Thu, Nov 23, 2023 at 06:12:36PM +0100, Uwe Kleine-König wrote:
> > On Mon, Nov 20, 2023 at 03:56:18PM -0600, Bjorn Helgaas wrote:
> > > Applied to "enumeration" for v6.8, thanks!
> > > 
> > > [1/1] PCI: host-generic: Convert to platform remove callback returning void
> > >       commit: d9dcdb4531fe39ce48919ef8c2c9369ee49f3ad2
> > 
> > Thanks! This branch isn't included in next. Is this on purpose?
> 
> To reduce the workload of the folks maintaining "next", I wait for the
> 0-day bot to build test a branch before merging it into the PCI "next"
> branch.

If this isn't a mistake (or a hint to me that there are problems with
the patch) everything is fine on my side.

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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