Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756637AbcKWU4q (ORCPT ); Wed, 23 Nov 2016 15:56:46 -0500 Received: from netrider.rowland.org ([192.131.102.5]:53719 "HELO netrider.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1756507AbcKWU4o (ORCPT ); Wed, 23 Nov 2016 15:56:44 -0500 Date: Wed, 23 Nov 2016 15:56:43 -0500 (EST) From: Alan Stern X-X-Sender: stern@netrider.rowland.org To: csmanjuvijay@gmail.com cc: Arnd Bergmann , Greg KH , Wan ZongShun , , Subject: Re: [PATCH] USB: EHCI: use module_platform_driver macro In-Reply-To: <1479816762-18162-1-git-send-email-csmanjuvijay@gmail.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4203 Lines: 132 On Tue, 22 Nov 2016 csmanjuvijay@gmail.com wrote: > From: Majunath Goudar > > Use the module_platform_driver macro to do module init/exit. > This eliminates a lot of boilerplate.This also removes redundant > code and overhead of a function call. I really don't like this patch, or the corresponding patch for ohci-nxp.c. By moving the contents of ehci_w90X900_init() into ehci_w90x900_probe(), you cause it to run at the wrong time and possibly run more than once. (Also, the title of this patch is wrong. You are not changing the EHCI core files; you are changing the specific ehci-w90x900 driver.) On the other hand, I do like the fact that the patch removes two useless functions in the probe and remove paths. But that can be done separately. Alan Stern > Signed-off-by: Manjunath Goudar > Cc: Arnd Bergmann > Cc: Greg KH > Cc: Alan Stern > Cc: Wan ZongShun > Cc: linux-usb@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > drivers/usb/host/ehci-w90x900.c | 55 ++++++++++++----------------------------- > 1 file changed, 16 insertions(+), 39 deletions(-) > > diff --git a/drivers/usb/host/ehci-w90x900.c b/drivers/usb/host/ehci-w90x900.c > index e42a29e..4cb2651 100644 > --- a/drivers/usb/host/ehci-w90x900.c > +++ b/drivers/usb/host/ehci-w90x900.c > @@ -33,8 +33,7 @@ static const char hcd_name[] = "ehci-w90x900 "; > > static struct hc_driver __read_mostly ehci_w90x900_hc_driver; > > -static int usb_w90x900_probe(const struct hc_driver *driver, > - struct platform_device *pdev) > +static int ehci_w90x900_probe(struct platform_device *pdev) > { > struct usb_hcd *hcd; > struct ehci_hcd *ehci; > @@ -42,7 +41,15 @@ static int usb_w90x900_probe(const struct hc_driver *driver, > int retval = 0, irq; > unsigned long val; > > - hcd = usb_create_hcd(driver, &pdev->dev, "w90x900 EHCI"); > + if (usb_disabled()) > + return -ENODEV; > + > + pr_info("%s: " DRIVER_DESC "\n", hcd_name); > + > + ehci_init_driver(&ehci_w90x900_hc_driver, NULL); > + > + hcd = usb_create_hcd(&ehci_w90x900_hc_driver, &pdev->dev, > + "w90x900 EHCI"); > if (!hcd) { > retval = -ENOMEM; > goto err1; > @@ -63,9 +70,9 @@ static int usb_w90x900_probe(const struct hc_driver *driver, > HC_LENGTH(ehci, ehci_readl(ehci, &ehci->caps->hc_capbase)); > > /* enable PHY 0,1,the regs only apply to w90p910 > - * 0xA4,0xA8 were offsets of PHY0 and PHY1 controller of > - * w90p910 IC relative to ehci->regs. > - */ > + * 0xA4,0xA8 were offsets of PHY0 and PHY1 controller of > + * w90p910 IC relative to ehci->regs. > + */ > val = __raw_readl(ehci->regs+PHY0_CTR); > val |= ENPHY; > __raw_writel(val, ehci->regs+PHY0_CTR); > @@ -92,26 +99,12 @@ static int usb_w90x900_probe(const struct hc_driver *driver, > return retval; > } > > -static void usb_w90x900_remove(struct usb_hcd *hcd, > - struct platform_device *pdev) > -{ > - usb_remove_hcd(hcd); > - usb_put_hcd(hcd); > -} > - > -static int ehci_w90x900_probe(struct platform_device *pdev) > -{ > - if (usb_disabled()) > - return -ENODEV; > - > - return usb_w90x900_probe(&ehci_w90x900_hc_driver, pdev); > -} > - > static int ehci_w90x900_remove(struct platform_device *pdev) > { > struct usb_hcd *hcd = platform_get_drvdata(pdev); > > - usb_w90x900_remove(hcd, pdev); > + usb_remove_hcd(hcd); > + usb_put_hcd(hcd); > > return 0; > } > @@ -124,23 +117,7 @@ static struct platform_driver ehci_hcd_w90x900_driver = { > }, > }; > > -static int __init ehci_w90X900_init(void) > -{ > - if (usb_disabled()) > - return -ENODEV; > - > - pr_info("%s: " DRIVER_DESC "\n", hcd_name); > - > - ehci_init_driver(&ehci_w90x900_hc_driver, NULL); > - return platform_driver_register(&ehci_hcd_w90x900_driver); > -} > -module_init(ehci_w90X900_init); > - > -static void __exit ehci_w90X900_cleanup(void) > -{ > - platform_driver_unregister(&ehci_hcd_w90x900_driver); > -} > -module_exit(ehci_w90X900_cleanup); > +module_platform_driver(ehci_hcd_w90x900_driver); > > MODULE_DESCRIPTION(DRIVER_DESC); > MODULE_AUTHOR("Wan ZongShun "); >