Hello,
On Thu, May 18, 2023 at 02:28:44AM +0000, Yoshihiro Shimoda wrote:
> > From: Uwe Kleine-König, Sent: Thursday, May 18, 2023 8:02 AM
> >
> > 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 ignored (apart from
> > emitting a warning) 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. Eventually after all drivers are converted, .remove_new() is
> > renamed to .remove().
> >
> > Trivially convert this driver from always returning zero in the remove
> > callback to the void returning variant.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> > ---
> > drivers/usb/host/xhci-plat.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> > index b0c8e8efc43b..523e3843db5e 100644
> > --- a/drivers/usb/host/xhci-plat.c
> > +++ b/drivers/usb/host/xhci-plat.c
> > @@ -399,7 +399,7 @@ static int xhci_generic_plat_probe(struct platform_device *pdev)
> > return xhci_plat_probe(pdev, sysdev, priv_match);
> > }
> >
> > -int xhci_plat_remove(struct platform_device *dev)
> > +void xhci_plat_remove(struct platform_device *dev)
>
> We should change the prototype in the xhci-plat.h. Otherwise
> the following build error happens:
> ---
> drivers/usb/host/xhci-plat.c:398:6: error: conflicting types for 'xhci_plat_remove'; have 'void(struct platform_device *)'
> 398 | void xhci_plat_remove(struct platform_device *dev)
> | ^~~~~~~~~~~~~~~~
> In file included from drivers/usb/host/xhci-plat.c:25:
> drivers/usb/host/xhci-plat.h:28:5: note: previous declaration of 'xhci_plat_remove' with type 'int(struct platform_device *)'
> 28 | int xhci_plat_remove(struct platform_device *dev);
> | ^~~~~~~~~~~~~~~~
> In file included from ./include/linux/linkage.h:7,
> from ./include/linux/kernel.h:17,
> from ./include/linux/clk.h:13,
> from drivers/usb/host/xhci-plat.c:11:
> drivers/usb/host/xhci-plat.c:430:19: error: conflicting types for 'xhci_plat_remove'; have 'void(struct platform_device *)'
> ---
>
> Since the xhci-rcar.c uses xhci_plat_remove(), we should change
> the xhci-rcar.c in this patch too, I think.
indeed, I squashed the following change into this patch in my tree:
diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
index 83b5b5aa9f8e..2d15386f2c50 100644
--- a/drivers/usb/host/xhci-plat.h
+++ b/drivers/usb/host/xhci-plat.h
@@ -25,7 +25,7 @@ struct xhci_plat_priv {
int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev,
const struct xhci_plat_priv *priv_match);
-int xhci_plat_remove(struct platform_device *dev);
+void xhci_plat_remove(struct platform_device *dev);
extern const struct dev_pm_ops xhci_plat_pm_ops;
#endif /* _XHCI_PLAT_H */
diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
index ad966b797b89..bf5261fed32c 100644
--- a/drivers/usb/host/xhci-rcar.c
+++ b/drivers/usb/host/xhci-rcar.c
@@ -276,10 +276,10 @@ static int xhci_renesas_probe(struct platform_device *pdev)
}
static struct platform_driver usb_xhci_renesas_driver = {
- .probe = xhci_renesas_probe,
- .remove = xhci_plat_remove,
+ .probe = xhci_renesas_probe,
+ .remove_new = xhci_plat_remove,
.shutdown = usb_hcd_platform_shutdown,
- .driver = {
+ .driver = {
.name = "xhci-renesas-hcd",
.pm = &xhci_plat_pm_ops,
.of_match_table = usb_xhci_of_match,
(which also "fixes" the alignments of =).
@Greg: Assuming it will be you who picks up this series, I suggest you
skip this patch for now and I address this together with the other
changes that need still be done for drivers/usb before we switch back to
.remove() with the fixed prototype. (There are a few drivers[1] that
might return an error code in .remove(), didn't look into those yet.)
Best regards
Uwe
[1] drivers/usb/gadget/udc/aspeed_udc.c
drivers/usb/gadget/udc/at91_udc.c
drivers/usb/gadget/udc/fsl_udc_core.c
drivers/usb/gadget/udc/gr_udc.c
drivers/usb/gadget/udc/lpc32xx_udc.c
drivers/usb/gadget/udc/pxa25x_udc.c
drivers/usb/misc/onboard_usb_hub.c
drivers/usb/mtu3/mtu3_plat.c
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
