| Message ID | 20231020092107.2148311-1-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: host-generic: Convert to platform remove callback returning void | expand |
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
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
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
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
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,
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
[+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
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 >
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
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
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(-) base-commit: 2030579113a1b1b5bfd7ff24c0852847836d8fd1