From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965451AbcJYHkF (ORCPT ); Tue, 25 Oct 2016 03:40:05 -0400 Received: from mail-qk0-f175.google.com ([209.85.220.175]:36033 "EHLO mail-qk0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933262AbcJYHkA (ORCPT ); Tue, 25 Oct 2016 03:40:00 -0400 MIME-Version: 1.0 In-Reply-To: <403743d8-dff2-3389-105b-1082b674b0b8@lechnology.com> References: <20161024164634.4330-1-ahaslam@baylibre.com> <20161024164634.4330-12-ahaslam@baylibre.com> <403743d8-dff2-3389-105b-1082b674b0b8@lechnology.com> From: Axel Haslam Date: Tue, 25 Oct 2016 09:39:19 +0200 Message-ID: Subject: Re: [PATCH/RFT v2 11/17] USB: OHCI: make ohci-da8xx a separate driver To: David Lechner Cc: Greg KH , Johan Hovold , robh+dt@kernel.org, Sekhar Nori , Alan Stern , Kevin Hilman , Sergei Shtylyov , manjunath.goudar@linaro.org, Mark Brown , Alexandre Bailon , linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 25, 2016 at 2:38 AM, David Lechner wrote: > On 10/24/2016 11:46 AM, ahaslam@baylibre.com wrote: >> >> From: Manjunath Goudar >> >> Separate the Davinci OHCI host controller driver from ohci-hcd >> host code so that it can be built as a separate driver module. >> This work is part of enabling multi-platform kernels on ARM; >> it would be nice to have in 3.11. > > > No need for comment about kernel 3.11. yes, ok. > >> >> Signed-off-by: Manjunath Goudar >> --- >> drivers/usb/host/Kconfig | 2 +- >> drivers/usb/host/Makefile | 1 + >> drivers/usb/host/ohci-da8xx.c | 185 >> +++++++++++++++++------------------------- >> drivers/usb/host/ohci-hcd.c | 18 ---- >> 4 files changed, 76 insertions(+), 130 deletions(-) >> >> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig >> index 83b6cec..642c6fe8 100644 >> --- a/drivers/usb/host/Kconfig >> +++ b/drivers/usb/host/Kconfig >> @@ -479,7 +479,7 @@ config USB_OHCI_HCD_OMAP3 >> OMAP3 and later chips. >> >> config USB_OHCI_HCD_DAVINCI >> - bool "OHCI support for TI DaVinci DA8xx" >> + tristate "OHCI support for TI DaVinci DA8xx" >> depends on ARCH_DAVINCI_DA8XX >> depends on USB_OHCI_HCD=y > > > Need to drop the "=y" here, otherwise you still can't compile this as a > module. Im able to complile it as a module, but ok ill remove it. > >> select PHY_DA8XX_USB >> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile >> index 6ef785b..2644537 100644 >> --- a/drivers/usb/host/Makefile >> +++ b/drivers/usb/host/Makefile >> @@ -61,6 +61,7 @@ obj-$(CONFIG_USB_OHCI_HCD_AT91) += ohci-at91.o >> obj-$(CONFIG_USB_OHCI_HCD_S3C2410) += ohci-s3c2410.o >> obj-$(CONFIG_USB_OHCI_HCD_LPC32XX) += ohci-nxp.o >> obj-$(CONFIG_USB_OHCI_HCD_PXA27X) += ohci-pxa27x.o >> +obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o >> >> obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o >> obj-$(CONFIG_USB_FHCI_HCD) += fhci.o >> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c >> index e98066d..5585d9e 100644 >> --- a/drivers/usb/host/ohci-da8xx.c >> +++ b/drivers/usb/host/ohci-da8xx.c >> @@ -11,16 +11,31 @@ >> * kind, whether express or implied. >> */ >> >> +#include >> +#include >> #include >> #include >> +#include >> +#include >> #include >> -#include >> #include >> #include >> +#include > > > linux/platform_device.h is listed twice > >> +#include >> +#include >> +#include >> >> -#ifndef CONFIG_ARCH_DAVINCI_DA8XX >> -#error "This file is DA8xx bus glue. Define CONFIG_ARCH_DAVINCI_DA8XX." >> -#endif >> +#include "ohci.h" >> + >> +#define DRIVER_DESC "OHCI DA8XX driver" >> + >> +static const char hcd_name[] = "ohci-da8xx"; > > > why static const char instead of #define? This is only used one time in a > pr_info, so it seems kind of pointless anyway. Other drivers are using static const for the same variable. i think static const is preferred over #define because #define doet give a type. If you dont mind ill keep it static const. > >> + >> +static struct hc_driver __read_mostly ohci_da8xx_hc_driver; >> + >> +static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq, >> + u16 wValue, u16 wIndex, char *buf, u16 wLength); >> +static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf); >> >> static struct clk *usb11_clk; >> static struct phy *usb11_phy; >> @@ -73,7 +88,7 @@ static void ohci_da8xx_ocic_handler(struct >> da8xx_ohci_root_hub *hub) >> hub->set_power(0); >> } >> >> -static int ohci_da8xx_init(struct usb_hcd *hcd) >> +static int ohci_da8xx_reset(struct usb_hcd *hcd) >> { >> struct device *dev = hcd->self.controller; >> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev); >> @@ -93,7 +108,7 @@ static int ohci_da8xx_init(struct usb_hcd *hcd) >> */ >> ohci->num_ports = 1; >> >> - result = ohci_init(ohci); >> + result = ohci_setup(hcd); >> if (result < 0) { >> ohci_da8xx_disable(); >> return result; >> @@ -121,30 +136,12 @@ static int ohci_da8xx_init(struct usb_hcd *hcd) >> return result; >> } >> >> -static void ohci_da8xx_stop(struct usb_hcd *hcd) >> -{ >> - ohci_stop(hcd); >> - ohci_da8xx_disable(); >> -} >> - >> -static int ohci_da8xx_start(struct usb_hcd *hcd) >> -{ >> - struct ohci_hcd *ohci = hcd_to_ohci(hcd); >> - int result; >> - >> - result = ohci_run(ohci); >> - if (result < 0) >> - ohci_da8xx_stop(hcd); >> - >> - return result; >> -} >> - >> /* >> * Update the status data from the hub with the over-current indicator >> change. >> */ >> static int ohci_da8xx_hub_status_data(struct usb_hcd *hcd, char *buf) >> { >> - int length = ohci_hub_status_data(hcd, buf); >> + int length = orig_ohci_hub_status_data(hcd, buf); >> >> /* See if we have OCIC flag set */ >> if (ocic_flag) { >> @@ -226,66 +223,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd >> *hcd, u16 typeReq, u16 wValue, >> } >> } >> >> - return ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, >> wLength); >> + return orig_ohci_hub_control(hcd, typeReq, wValue, >> + wIndex, buf, wLength); >> } >> >> -static const struct hc_driver ohci_da8xx_hc_driver = { >> - .description = hcd_name, >> - .product_desc = "DA8xx OHCI", >> - .hcd_priv_size = sizeof(struct ohci_hcd), >> - >> - /* >> - * generic hardware linkage >> - */ >> - .irq = ohci_irq, >> - .flags = HCD_USB11 | HCD_MEMORY, >> - >> - /* >> - * basic lifecycle operations >> - */ >> - .reset = ohci_da8xx_init, >> - .start = ohci_da8xx_start, >> - .stop = ohci_da8xx_stop, >> - .shutdown = ohci_shutdown, >> - >> - /* >> - * managing i/o requests and associated device resources >> - */ >> - .urb_enqueue = ohci_urb_enqueue, >> - .urb_dequeue = ohci_urb_dequeue, >> - .endpoint_disable = ohci_endpoint_disable, >> - >> - /* >> - * scheduling support >> - */ >> - .get_frame_number = ohci_get_frame, >> - >> - /* >> - * root hub support >> - */ >> - .hub_status_data = ohci_da8xx_hub_status_data, >> - .hub_control = ohci_da8xx_hub_control, >> - >> -#ifdef CONFIG_PM >> - .bus_suspend = ohci_bus_suspend, >> - .bus_resume = ohci_bus_resume, >> -#endif >> - .start_port_reset = ohci_start_port_reset, >> -}; >> - >> >> /*-------------------------------------------------------------------------*/ >> >> - >> -/** >> - * usb_hcd_da8xx_probe - initialize DA8xx-based HCDs >> - * Context: !in_interrupt() >> - * >> - * Allocates basic resources for this USB host controller, and >> - * then invokes the start() method for the HCD associated with it >> - * through the hotplug entry's driver_data. >> - */ >> -static int usb_hcd_da8xx_probe(const struct hc_driver *driver, >> - struct platform_device *pdev) >> +static int ohci_da8xx_probe(struct platform_device *pdev) >> { >> struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev); >> struct usb_hcd *hcd; >> @@ -295,6 +239,11 @@ static int usb_hcd_da8xx_probe(const struct hc_driver >> *driver, >> if (hub == NULL) >> return -ENODEV; >> >> + hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev, >> + dev_name(&pdev->dev)); >> + if (!hcd) >> + return -ENOMEM; >> + > > > Won't this leak hdc if there is an error later? > >> usb11_clk = devm_clk_get(&pdev->dev, "usb11"); >> if (IS_ERR(usb11_clk)) { >> if (PTR_ERR(usb11_clk) != -EPROBE_DEFER) >> @@ -309,9 +258,6 @@ static int usb_hcd_da8xx_probe(const struct hc_driver >> *driver, >> return PTR_ERR(usb11_phy); >> } >> >> - hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev)); >> - if (!hcd) >> - return -ENOMEM; > > > Why does this need to be moved? > it should not have moved this, will fix. >> >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> hcd->regs = devm_ioremap_resource(&pdev->dev, mem); >> @@ -323,13 +269,12 @@ static int usb_hcd_da8xx_probe(const struct >> hc_driver *driver, >> hcd->rsrc_start = mem->start; >> hcd->rsrc_len = resource_size(mem); >> >> - ohci_hcd_init(hcd_to_ohci(hcd)); >> - >> irq = platform_get_irq(pdev, 0); >> if (irq < 0) { >> error = -ENODEV; >> goto err; >> } >> + >> error = usb_add_hcd(hcd, irq, 0); >> if (error) >> goto err; >> @@ -348,35 +293,14 @@ static int usb_hcd_da8xx_probe(const struct >> hc_driver *driver, >> return error; >> } >> >> -/** >> - * usb_hcd_da8xx_remove - shutdown processing for DA8xx-based HCDs >> - * @dev: USB Host Controller being removed >> - * Context: !in_interrupt() >> - * >> - * Reverses the effect of usb_hcd_da8xx_probe(), first invoking >> - * the HCD's stop() method. It is always called from a thread >> - * context, normally "rmmod", "apmd", or something similar. >> - */ >> -static inline void >> -usb_hcd_da8xx_remove(struct usb_hcd *hcd, struct platform_device *pdev) >> +static int ohci_da8xx_remove(struct platform_device *pdev) >> { >> + struct usb_hcd *hcd = platform_get_drvdata(pdev); >> struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev); >> >> hub->ocic_notify(NULL); >> usb_remove_hcd(hcd); >> usb_put_hcd(hcd); >> -} >> - >> -static int ohci_hcd_da8xx_drv_probe(struct platform_device *dev) >> -{ >> - return usb_hcd_da8xx_probe(&ohci_da8xx_hc_driver, dev); >> -} >> - >> -static int ohci_hcd_da8xx_drv_remove(struct platform_device *dev) >> -{ >> - struct usb_hcd *hcd = platform_get_drvdata(dev); >> - >> - usb_hcd_da8xx_remove(hcd, dev); >> >> return 0; >> } >> @@ -426,12 +350,16 @@ static int ohci_da8xx_resume(struct platform_device >> *dev) >> } >> #endif >> >> +static const struct ohci_driver_overrides da8xx_overrides __initconst = { >> + .reset = ohci_da8xx_reset >> +}; >> + >> /* >> * Driver definition to register with platform structure. >> */ >> static struct platform_driver ohci_hcd_da8xx_driver = { >> - .probe = ohci_hcd_da8xx_drv_probe, >> - .remove = ohci_hcd_da8xx_drv_remove, >> + .probe = ohci_da8xx_probe, >> + .remove = ohci_da8xx_remove, >> .shutdown = usb_hcd_platform_shutdown, >> #ifdef CONFIG_PM >> .suspend = ohci_da8xx_suspend, > > > It would probably be a good idea to change the driver name here. Currently > it is "ohci". Although this would be better in a separate patch if the name > has to be changed to match in other files as well. > ok, ill do a separate patch for that. >> @@ -442,4 +370,39 @@ static int ohci_da8xx_resume(struct platform_device >> *dev) >> }, >> }; >> >> +static int __init ohci_da8xx_init(void) >> +{ >> + >> + if (usb_disabled()) >> + return -ENODEV; >> + >> + pr_info("%s: " DRIVER_DESC "\n", hcd_name); >> + ohci_init_driver(&ohci_da8xx_hc_driver, &da8xx_overrides); >> + >> + /* >> + * The Davinci da8xx HW has some unusual quirks, which require >> + * da8xx-specific workarounds. We override certain hc_driver >> + * functions here to achieve that. We explicitly do not enhance >> + * ohci_driver_overrides to allow this more easily, since this >> + * is an unusual case, and we don't want to encourage others to >> + * override these functions by making it too easy. >> + */ >> + >> + orig_ohci_hub_control = ohci_da8xx_hc_driver.hub_control; >> + orig_ohci_hub_status_data = ohci_da8xx_hc_driver.hub_status_data; >> + >> + ohci_da8xx_hc_driver.hub_status_data = >> ohci_da8xx_hub_status_data; >> + ohci_da8xx_hc_driver.hub_control = ohci_da8xx_hub_control; >> + >> + return platform_driver_register(&ohci_hcd_da8xx_driver); >> +} >> +module_init(ohci_da8xx_init); >> + >> +static void __exit ohci_da8xx_cleanup(void) > > > ohci_da8xx_exit would be a better name > ok. >> +{ >> + platform_driver_unregister(&ohci_hcd_da8xx_driver); >> +} >> +module_exit(ohci_da8xx_cleanup); >> +MODULE_DESCRIPTION(DRIVER_DESC); >> +MODULE_LICENSE("GPL"); >> MODULE_ALIAS("platform:ohci"); > > > this will need to be changed too if you change the driver name > >> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c >> index 1700908..8de174a 100644 >> --- a/drivers/usb/host/ohci-hcd.c >> +++ b/drivers/usb/host/ohci-hcd.c >> @@ -1219,11 +1219,6 @@ void ohci_init_driver(struct hc_driver *drv, >> #define SA1111_DRIVER ohci_hcd_sa1111_driver >> #endif >> >> -#ifdef CONFIG_USB_OHCI_HCD_DAVINCI >> -#include "ohci-da8xx.c" >> -#define DAVINCI_PLATFORM_DRIVER ohci_hcd_da8xx_driver >> -#endif >> - >> #ifdef CONFIG_USB_OHCI_HCD_PPC_OF >> #include "ohci-ppc-of.c" >> #define OF_PLATFORM_DRIVER ohci_hcd_ppc_of_driver >> @@ -1303,19 +1298,9 @@ static int __init ohci_hcd_mod_init(void) >> goto error_tmio; >> #endif >> >> -#ifdef DAVINCI_PLATFORM_DRIVER >> - retval = platform_driver_register(&DAVINCI_PLATFORM_DRIVER); >> - if (retval < 0) >> - goto error_davinci; >> -#endif >> - >> return retval; >> >> /* Error path */ >> -#ifdef DAVINCI_PLATFORM_DRIVER >> - platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER); >> - error_davinci: >> -#endif >> #ifdef TMIO_OHCI_DRIVER >> platform_driver_unregister(&TMIO_OHCI_DRIVER); >> error_tmio: >> @@ -1351,9 +1336,6 @@ static int __init ohci_hcd_mod_init(void) >> >> static void __exit ohci_hcd_mod_exit(void) >> { >> -#ifdef DAVINCI_PLATFORM_DRIVER >> - platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER); >> -#endif >> #ifdef TMIO_OHCI_DRIVER >> platform_driver_unregister(&TMIO_OHCI_DRIVER); >> #endif >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: Axel Haslam Subject: Re: [PATCH/RFT v2 11/17] USB: OHCI: make ohci-da8xx a separate driver Date: Tue, 25 Oct 2016 09:39:19 +0200 Message-ID: References: <20161024164634.4330-1-ahaslam@baylibre.com> <20161024164634.4330-12-ahaslam@baylibre.com> <403743d8-dff2-3389-105b-1082b674b0b8@lechnology.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: <403743d8-dff2-3389-105b-1082b674b0b8-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: David Lechner Cc: Greg KH , Johan Hovold , robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Sekhar Nori , Alan Stern , Kevin Hilman , Sergei Shtylyov , manjunath.goudar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, Mark Brown , Alexandre Bailon , linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: devicetree@vger.kernel.org On Tue, Oct 25, 2016 at 2:38 AM, David Lechner wrote: > On 10/24/2016 11:46 AM, ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org wrote: >> >> From: Manjunath Goudar >> >> Separate the Davinci OHCI host controller driver from ohci-hcd >> host code so that it can be built as a separate driver module. >> This work is part of enabling multi-platform kernels on ARM; >> it would be nice to have in 3.11. > > > No need for comment about kernel 3.11. yes, ok. > >> >> Signed-off-by: Manjunath Goudar >> --- >> drivers/usb/host/Kconfig | 2 +- >> drivers/usb/host/Makefile | 1 + >> drivers/usb/host/ohci-da8xx.c | 185 >> +++++++++++++++++------------------------- >> drivers/usb/host/ohci-hcd.c | 18 ---- >> 4 files changed, 76 insertions(+), 130 deletions(-) >> >> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig >> index 83b6cec..642c6fe8 100644 >> --- a/drivers/usb/host/Kconfig >> +++ b/drivers/usb/host/Kconfig >> @@ -479,7 +479,7 @@ config USB_OHCI_HCD_OMAP3 >> OMAP3 and later chips. >> >> config USB_OHCI_HCD_DAVINCI >> - bool "OHCI support for TI DaVinci DA8xx" >> + tristate "OHCI support for TI DaVinci DA8xx" >> depends on ARCH_DAVINCI_DA8XX >> depends on USB_OHCI_HCD=y > > > Need to drop the "=y" here, otherwise you still can't compile this as a > module. Im able to complile it as a module, but ok ill remove it. > >> select PHY_DA8XX_USB >> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile >> index 6ef785b..2644537 100644 >> --- a/drivers/usb/host/Makefile >> +++ b/drivers/usb/host/Makefile >> @@ -61,6 +61,7 @@ obj-$(CONFIG_USB_OHCI_HCD_AT91) += ohci-at91.o >> obj-$(CONFIG_USB_OHCI_HCD_S3C2410) += ohci-s3c2410.o >> obj-$(CONFIG_USB_OHCI_HCD_LPC32XX) += ohci-nxp.o >> obj-$(CONFIG_USB_OHCI_HCD_PXA27X) += ohci-pxa27x.o >> +obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o >> >> obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o >> obj-$(CONFIG_USB_FHCI_HCD) += fhci.o >> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c >> index e98066d..5585d9e 100644 >> --- a/drivers/usb/host/ohci-da8xx.c >> +++ b/drivers/usb/host/ohci-da8xx.c >> @@ -11,16 +11,31 @@ >> * kind, whether express or implied. >> */ >> >> +#include >> +#include >> #include >> #include >> +#include >> +#include >> #include >> -#include >> #include >> #include >> +#include > > > linux/platform_device.h is listed twice > >> +#include >> +#include >> +#include >> >> -#ifndef CONFIG_ARCH_DAVINCI_DA8XX >> -#error "This file is DA8xx bus glue. Define CONFIG_ARCH_DAVINCI_DA8XX." >> -#endif >> +#include "ohci.h" >> + >> +#define DRIVER_DESC "OHCI DA8XX driver" >> + >> +static const char hcd_name[] = "ohci-da8xx"; > > > why static const char instead of #define? This is only used one time in a > pr_info, so it seems kind of pointless anyway. Other drivers are using static const for the same variable. i think static const is preferred over #define because #define doet give a type. If you dont mind ill keep it static const. > >> + >> +static struct hc_driver __read_mostly ohci_da8xx_hc_driver; >> + >> +static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq, >> + u16 wValue, u16 wIndex, char *buf, u16 wLength); >> +static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf); >> >> static struct clk *usb11_clk; >> static struct phy *usb11_phy; >> @@ -73,7 +88,7 @@ static void ohci_da8xx_ocic_handler(struct >> da8xx_ohci_root_hub *hub) >> hub->set_power(0); >> } >> >> -static int ohci_da8xx_init(struct usb_hcd *hcd) >> +static int ohci_da8xx_reset(struct usb_hcd *hcd) >> { >> struct device *dev = hcd->self.controller; >> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev); >> @@ -93,7 +108,7 @@ static int ohci_da8xx_init(struct usb_hcd *hcd) >> */ >> ohci->num_ports = 1; >> >> - result = ohci_init(ohci); >> + result = ohci_setup(hcd); >> if (result < 0) { >> ohci_da8xx_disable(); >> return result; >> @@ -121,30 +136,12 @@ static int ohci_da8xx_init(struct usb_hcd *hcd) >> return result; >> } >> >> -static void ohci_da8xx_stop(struct usb_hcd *hcd) >> -{ >> - ohci_stop(hcd); >> - ohci_da8xx_disable(); >> -} >> - >> -static int ohci_da8xx_start(struct usb_hcd *hcd) >> -{ >> - struct ohci_hcd *ohci = hcd_to_ohci(hcd); >> - int result; >> - >> - result = ohci_run(ohci); >> - if (result < 0) >> - ohci_da8xx_stop(hcd); >> - >> - return result; >> -} >> - >> /* >> * Update the status data from the hub with the over-current indicator >> change. >> */ >> static int ohci_da8xx_hub_status_data(struct usb_hcd *hcd, char *buf) >> { >> - int length = ohci_hub_status_data(hcd, buf); >> + int length = orig_ohci_hub_status_data(hcd, buf); >> >> /* See if we have OCIC flag set */ >> if (ocic_flag) { >> @@ -226,66 +223,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd >> *hcd, u16 typeReq, u16 wValue, >> } >> } >> >> - return ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, >> wLength); >> + return orig_ohci_hub_control(hcd, typeReq, wValue, >> + wIndex, buf, wLength); >> } >> >> -static const struct hc_driver ohci_da8xx_hc_driver = { >> - .description = hcd_name, >> - .product_desc = "DA8xx OHCI", >> - .hcd_priv_size = sizeof(struct ohci_hcd), >> - >> - /* >> - * generic hardware linkage >> - */ >> - .irq = ohci_irq, >> - .flags = HCD_USB11 | HCD_MEMORY, >> - >> - /* >> - * basic lifecycle operations >> - */ >> - .reset = ohci_da8xx_init, >> - .start = ohci_da8xx_start, >> - .stop = ohci_da8xx_stop, >> - .shutdown = ohci_shutdown, >> - >> - /* >> - * managing i/o requests and associated device resources >> - */ >> - .urb_enqueue = ohci_urb_enqueue, >> - .urb_dequeue = ohci_urb_dequeue, >> - .endpoint_disable = ohci_endpoint_disable, >> - >> - /* >> - * scheduling support >> - */ >> - .get_frame_number = ohci_get_frame, >> - >> - /* >> - * root hub support >> - */ >> - .hub_status_data = ohci_da8xx_hub_status_data, >> - .hub_control = ohci_da8xx_hub_control, >> - >> -#ifdef CONFIG_PM >> - .bus_suspend = ohci_bus_suspend, >> - .bus_resume = ohci_bus_resume, >> -#endif >> - .start_port_reset = ohci_start_port_reset, >> -}; >> - >> >> /*-------------------------------------------------------------------------*/ >> >> - >> -/** >> - * usb_hcd_da8xx_probe - initialize DA8xx-based HCDs >> - * Context: !in_interrupt() >> - * >> - * Allocates basic resources for this USB host controller, and >> - * then invokes the start() method for the HCD associated with it >> - * through the hotplug entry's driver_data. >> - */ >> -static int usb_hcd_da8xx_probe(const struct hc_driver *driver, >> - struct platform_device *pdev) >> +static int ohci_da8xx_probe(struct platform_device *pdev) >> { >> struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev); >> struct usb_hcd *hcd; >> @@ -295,6 +239,11 @@ static int usb_hcd_da8xx_probe(const struct hc_driver >> *driver, >> if (hub == NULL) >> return -ENODEV; >> >> + hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev, >> + dev_name(&pdev->dev)); >> + if (!hcd) >> + return -ENOMEM; >> + > > > Won't this leak hdc if there is an error later? > >> usb11_clk = devm_clk_get(&pdev->dev, "usb11"); >> if (IS_ERR(usb11_clk)) { >> if (PTR_ERR(usb11_clk) != -EPROBE_DEFER) >> @@ -309,9 +258,6 @@ static int usb_hcd_da8xx_probe(const struct hc_driver >> *driver, >> return PTR_ERR(usb11_phy); >> } >> >> - hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev)); >> - if (!hcd) >> - return -ENOMEM; > > > Why does this need to be moved? > it should not have moved this, will fix. >> >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> hcd->regs = devm_ioremap_resource(&pdev->dev, mem); >> @@ -323,13 +269,12 @@ static int usb_hcd_da8xx_probe(const struct >> hc_driver *driver, >> hcd->rsrc_start = mem->start; >> hcd->rsrc_len = resource_size(mem); >> >> - ohci_hcd_init(hcd_to_ohci(hcd)); >> - >> irq = platform_get_irq(pdev, 0); >> if (irq < 0) { >> error = -ENODEV; >> goto err; >> } >> + >> error = usb_add_hcd(hcd, irq, 0); >> if (error) >> goto err; >> @@ -348,35 +293,14 @@ static int usb_hcd_da8xx_probe(const struct >> hc_driver *driver, >> return error; >> } >> >> -/** >> - * usb_hcd_da8xx_remove - shutdown processing for DA8xx-based HCDs >> - * @dev: USB Host Controller being removed >> - * Context: !in_interrupt() >> - * >> - * Reverses the effect of usb_hcd_da8xx_probe(), first invoking >> - * the HCD's stop() method. It is always called from a thread >> - * context, normally "rmmod", "apmd", or something similar. >> - */ >> -static inline void >> -usb_hcd_da8xx_remove(struct usb_hcd *hcd, struct platform_device *pdev) >> +static int ohci_da8xx_remove(struct platform_device *pdev) >> { >> + struct usb_hcd *hcd = platform_get_drvdata(pdev); >> struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev); >> >> hub->ocic_notify(NULL); >> usb_remove_hcd(hcd); >> usb_put_hcd(hcd); >> -} >> - >> -static int ohci_hcd_da8xx_drv_probe(struct platform_device *dev) >> -{ >> - return usb_hcd_da8xx_probe(&ohci_da8xx_hc_driver, dev); >> -} >> - >> -static int ohci_hcd_da8xx_drv_remove(struct platform_device *dev) >> -{ >> - struct usb_hcd *hcd = platform_get_drvdata(dev); >> - >> - usb_hcd_da8xx_remove(hcd, dev); >> >> return 0; >> } >> @@ -426,12 +350,16 @@ static int ohci_da8xx_resume(struct platform_device >> *dev) >> } >> #endif >> >> +static const struct ohci_driver_overrides da8xx_overrides __initconst = { >> + .reset = ohci_da8xx_reset >> +}; >> + >> /* >> * Driver definition to register with platform structure. >> */ >> static struct platform_driver ohci_hcd_da8xx_driver = { >> - .probe = ohci_hcd_da8xx_drv_probe, >> - .remove = ohci_hcd_da8xx_drv_remove, >> + .probe = ohci_da8xx_probe, >> + .remove = ohci_da8xx_remove, >> .shutdown = usb_hcd_platform_shutdown, >> #ifdef CONFIG_PM >> .suspend = ohci_da8xx_suspend, > > > It would probably be a good idea to change the driver name here. Currently > it is "ohci". Although this would be better in a separate patch if the name > has to be changed to match in other files as well. > ok, ill do a separate patch for that. >> @@ -442,4 +370,39 @@ static int ohci_da8xx_resume(struct platform_device >> *dev) >> }, >> }; >> >> +static int __init ohci_da8xx_init(void) >> +{ >> + >> + if (usb_disabled()) >> + return -ENODEV; >> + >> + pr_info("%s: " DRIVER_DESC "\n", hcd_name); >> + ohci_init_driver(&ohci_da8xx_hc_driver, &da8xx_overrides); >> + >> + /* >> + * The Davinci da8xx HW has some unusual quirks, which require >> + * da8xx-specific workarounds. We override certain hc_driver >> + * functions here to achieve that. We explicitly do not enhance >> + * ohci_driver_overrides to allow this more easily, since this >> + * is an unusual case, and we don't want to encourage others to >> + * override these functions by making it too easy. >> + */ >> + >> + orig_ohci_hub_control = ohci_da8xx_hc_driver.hub_control; >> + orig_ohci_hub_status_data = ohci_da8xx_hc_driver.hub_status_data; >> + >> + ohci_da8xx_hc_driver.hub_status_data = >> ohci_da8xx_hub_status_data; >> + ohci_da8xx_hc_driver.hub_control = ohci_da8xx_hub_control; >> + >> + return platform_driver_register(&ohci_hcd_da8xx_driver); >> +} >> +module_init(ohci_da8xx_init); >> + >> +static void __exit ohci_da8xx_cleanup(void) > > > ohci_da8xx_exit would be a better name > ok. >> +{ >> + platform_driver_unregister(&ohci_hcd_da8xx_driver); >> +} >> +module_exit(ohci_da8xx_cleanup); >> +MODULE_DESCRIPTION(DRIVER_DESC); >> +MODULE_LICENSE("GPL"); >> MODULE_ALIAS("platform:ohci"); > > > this will need to be changed too if you change the driver name > >> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c >> index 1700908..8de174a 100644 >> --- a/drivers/usb/host/ohci-hcd.c >> +++ b/drivers/usb/host/ohci-hcd.c >> @@ -1219,11 +1219,6 @@ void ohci_init_driver(struct hc_driver *drv, >> #define SA1111_DRIVER ohci_hcd_sa1111_driver >> #endif >> >> -#ifdef CONFIG_USB_OHCI_HCD_DAVINCI >> -#include "ohci-da8xx.c" >> -#define DAVINCI_PLATFORM_DRIVER ohci_hcd_da8xx_driver >> -#endif >> - >> #ifdef CONFIG_USB_OHCI_HCD_PPC_OF >> #include "ohci-ppc-of.c" >> #define OF_PLATFORM_DRIVER ohci_hcd_ppc_of_driver >> @@ -1303,19 +1298,9 @@ static int __init ohci_hcd_mod_init(void) >> goto error_tmio; >> #endif >> >> -#ifdef DAVINCI_PLATFORM_DRIVER >> - retval = platform_driver_register(&DAVINCI_PLATFORM_DRIVER); >> - if (retval < 0) >> - goto error_davinci; >> -#endif >> - >> return retval; >> >> /* Error path */ >> -#ifdef DAVINCI_PLATFORM_DRIVER >> - platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER); >> - error_davinci: >> -#endif >> #ifdef TMIO_OHCI_DRIVER >> platform_driver_unregister(&TMIO_OHCI_DRIVER); >> error_tmio: >> @@ -1351,9 +1336,6 @@ static int __init ohci_hcd_mod_init(void) >> >> static void __exit ohci_hcd_mod_exit(void) >> { >> -#ifdef DAVINCI_PLATFORM_DRIVER >> - platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER); >> -#endif >> #ifdef TMIO_OHCI_DRIVER >> platform_driver_unregister(&TMIO_OHCI_DRIVER); >> #endif >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: ahaslam@baylibre.com (Axel Haslam) Date: Tue, 25 Oct 2016 09:39:19 +0200 Subject: [PATCH/RFT v2 11/17] USB: OHCI: make ohci-da8xx a separate driver In-Reply-To: <403743d8-dff2-3389-105b-1082b674b0b8@lechnology.com> References: <20161024164634.4330-1-ahaslam@baylibre.com> <20161024164634.4330-12-ahaslam@baylibre.com> <403743d8-dff2-3389-105b-1082b674b0b8@lechnology.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Oct 25, 2016 at 2:38 AM, David Lechner wrote: > On 10/24/2016 11:46 AM, ahaslam at baylibre.com wrote: >> >> From: Manjunath Goudar >> >> Separate the Davinci OHCI host controller driver from ohci-hcd >> host code so that it can be built as a separate driver module. >> This work is part of enabling multi-platform kernels on ARM; >> it would be nice to have in 3.11. > > > No need for comment about kernel 3.11. yes, ok. > >> >> Signed-off-by: Manjunath Goudar >> --- >> drivers/usb/host/Kconfig | 2 +- >> drivers/usb/host/Makefile | 1 + >> drivers/usb/host/ohci-da8xx.c | 185 >> +++++++++++++++++------------------------- >> drivers/usb/host/ohci-hcd.c | 18 ---- >> 4 files changed, 76 insertions(+), 130 deletions(-) >> >> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig >> index 83b6cec..642c6fe8 100644 >> --- a/drivers/usb/host/Kconfig >> +++ b/drivers/usb/host/Kconfig >> @@ -479,7 +479,7 @@ config USB_OHCI_HCD_OMAP3 >> OMAP3 and later chips. >> >> config USB_OHCI_HCD_DAVINCI >> - bool "OHCI support for TI DaVinci DA8xx" >> + tristate "OHCI support for TI DaVinci DA8xx" >> depends on ARCH_DAVINCI_DA8XX >> depends on USB_OHCI_HCD=y > > > Need to drop the "=y" here, otherwise you still can't compile this as a > module. Im able to complile it as a module, but ok ill remove it. > >> select PHY_DA8XX_USB >> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile >> index 6ef785b..2644537 100644 >> --- a/drivers/usb/host/Makefile >> +++ b/drivers/usb/host/Makefile >> @@ -61,6 +61,7 @@ obj-$(CONFIG_USB_OHCI_HCD_AT91) += ohci-at91.o >> obj-$(CONFIG_USB_OHCI_HCD_S3C2410) += ohci-s3c2410.o >> obj-$(CONFIG_USB_OHCI_HCD_LPC32XX) += ohci-nxp.o >> obj-$(CONFIG_USB_OHCI_HCD_PXA27X) += ohci-pxa27x.o >> +obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o >> >> obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o >> obj-$(CONFIG_USB_FHCI_HCD) += fhci.o >> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c >> index e98066d..5585d9e 100644 >> --- a/drivers/usb/host/ohci-da8xx.c >> +++ b/drivers/usb/host/ohci-da8xx.c >> @@ -11,16 +11,31 @@ >> * kind, whether express or implied. >> */ >> >> +#include >> +#include >> #include >> #include >> +#include >> +#include >> #include >> -#include >> #include >> #include >> +#include > > > linux/platform_device.h is listed twice > >> +#include >> +#include >> +#include >> >> -#ifndef CONFIG_ARCH_DAVINCI_DA8XX >> -#error "This file is DA8xx bus glue. Define CONFIG_ARCH_DAVINCI_DA8XX." >> -#endif >> +#include "ohci.h" >> + >> +#define DRIVER_DESC "OHCI DA8XX driver" >> + >> +static const char hcd_name[] = "ohci-da8xx"; > > > why static const char instead of #define? This is only used one time in a > pr_info, so it seems kind of pointless anyway. Other drivers are using static const for the same variable. i think static const is preferred over #define because #define doet give a type. If you dont mind ill keep it static const. > >> + >> +static struct hc_driver __read_mostly ohci_da8xx_hc_driver; >> + >> +static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq, >> + u16 wValue, u16 wIndex, char *buf, u16 wLength); >> +static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf); >> >> static struct clk *usb11_clk; >> static struct phy *usb11_phy; >> @@ -73,7 +88,7 @@ static void ohci_da8xx_ocic_handler(struct >> da8xx_ohci_root_hub *hub) >> hub->set_power(0); >> } >> >> -static int ohci_da8xx_init(struct usb_hcd *hcd) >> +static int ohci_da8xx_reset(struct usb_hcd *hcd) >> { >> struct device *dev = hcd->self.controller; >> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev); >> @@ -93,7 +108,7 @@ static int ohci_da8xx_init(struct usb_hcd *hcd) >> */ >> ohci->num_ports = 1; >> >> - result = ohci_init(ohci); >> + result = ohci_setup(hcd); >> if (result < 0) { >> ohci_da8xx_disable(); >> return result; >> @@ -121,30 +136,12 @@ static int ohci_da8xx_init(struct usb_hcd *hcd) >> return result; >> } >> >> -static void ohci_da8xx_stop(struct usb_hcd *hcd) >> -{ >> - ohci_stop(hcd); >> - ohci_da8xx_disable(); >> -} >> - >> -static int ohci_da8xx_start(struct usb_hcd *hcd) >> -{ >> - struct ohci_hcd *ohci = hcd_to_ohci(hcd); >> - int result; >> - >> - result = ohci_run(ohci); >> - if (result < 0) >> - ohci_da8xx_stop(hcd); >> - >> - return result; >> -} >> - >> /* >> * Update the status data from the hub with the over-current indicator >> change. >> */ >> static int ohci_da8xx_hub_status_data(struct usb_hcd *hcd, char *buf) >> { >> - int length = ohci_hub_status_data(hcd, buf); >> + int length = orig_ohci_hub_status_data(hcd, buf); >> >> /* See if we have OCIC flag set */ >> if (ocic_flag) { >> @@ -226,66 +223,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd >> *hcd, u16 typeReq, u16 wValue, >> } >> } >> >> - return ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, >> wLength); >> + return orig_ohci_hub_control(hcd, typeReq, wValue, >> + wIndex, buf, wLength); >> } >> >> -static const struct hc_driver ohci_da8xx_hc_driver = { >> - .description = hcd_name, >> - .product_desc = "DA8xx OHCI", >> - .hcd_priv_size = sizeof(struct ohci_hcd), >> - >> - /* >> - * generic hardware linkage >> - */ >> - .irq = ohci_irq, >> - .flags = HCD_USB11 | HCD_MEMORY, >> - >> - /* >> - * basic lifecycle operations >> - */ >> - .reset = ohci_da8xx_init, >> - .start = ohci_da8xx_start, >> - .stop = ohci_da8xx_stop, >> - .shutdown = ohci_shutdown, >> - >> - /* >> - * managing i/o requests and associated device resources >> - */ >> - .urb_enqueue = ohci_urb_enqueue, >> - .urb_dequeue = ohci_urb_dequeue, >> - .endpoint_disable = ohci_endpoint_disable, >> - >> - /* >> - * scheduling support >> - */ >> - .get_frame_number = ohci_get_frame, >> - >> - /* >> - * root hub support >> - */ >> - .hub_status_data = ohci_da8xx_hub_status_data, >> - .hub_control = ohci_da8xx_hub_control, >> - >> -#ifdef CONFIG_PM >> - .bus_suspend = ohci_bus_suspend, >> - .bus_resume = ohci_bus_resume, >> -#endif >> - .start_port_reset = ohci_start_port_reset, >> -}; >> - >> >> /*-------------------------------------------------------------------------*/ >> >> - >> -/** >> - * usb_hcd_da8xx_probe - initialize DA8xx-based HCDs >> - * Context: !in_interrupt() >> - * >> - * Allocates basic resources for this USB host controller, and >> - * then invokes the start() method for the HCD associated with it >> - * through the hotplug entry's driver_data. >> - */ >> -static int usb_hcd_da8xx_probe(const struct hc_driver *driver, >> - struct platform_device *pdev) >> +static int ohci_da8xx_probe(struct platform_device *pdev) >> { >> struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev); >> struct usb_hcd *hcd; >> @@ -295,6 +239,11 @@ static int usb_hcd_da8xx_probe(const struct hc_driver >> *driver, >> if (hub == NULL) >> return -ENODEV; >> >> + hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev, >> + dev_name(&pdev->dev)); >> + if (!hcd) >> + return -ENOMEM; >> + > > > Won't this leak hdc if there is an error later? > >> usb11_clk = devm_clk_get(&pdev->dev, "usb11"); >> if (IS_ERR(usb11_clk)) { >> if (PTR_ERR(usb11_clk) != -EPROBE_DEFER) >> @@ -309,9 +258,6 @@ static int usb_hcd_da8xx_probe(const struct hc_driver >> *driver, >> return PTR_ERR(usb11_phy); >> } >> >> - hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev)); >> - if (!hcd) >> - return -ENOMEM; > > > Why does this need to be moved? > it should not have moved this, will fix. >> >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> hcd->regs = devm_ioremap_resource(&pdev->dev, mem); >> @@ -323,13 +269,12 @@ static int usb_hcd_da8xx_probe(const struct >> hc_driver *driver, >> hcd->rsrc_start = mem->start; >> hcd->rsrc_len = resource_size(mem); >> >> - ohci_hcd_init(hcd_to_ohci(hcd)); >> - >> irq = platform_get_irq(pdev, 0); >> if (irq < 0) { >> error = -ENODEV; >> goto err; >> } >> + >> error = usb_add_hcd(hcd, irq, 0); >> if (error) >> goto err; >> @@ -348,35 +293,14 @@ static int usb_hcd_da8xx_probe(const struct >> hc_driver *driver, >> return error; >> } >> >> -/** >> - * usb_hcd_da8xx_remove - shutdown processing for DA8xx-based HCDs >> - * @dev: USB Host Controller being removed >> - * Context: !in_interrupt() >> - * >> - * Reverses the effect of usb_hcd_da8xx_probe(), first invoking >> - * the HCD's stop() method. It is always called from a thread >> - * context, normally "rmmod", "apmd", or something similar. >> - */ >> -static inline void >> -usb_hcd_da8xx_remove(struct usb_hcd *hcd, struct platform_device *pdev) >> +static int ohci_da8xx_remove(struct platform_device *pdev) >> { >> + struct usb_hcd *hcd = platform_get_drvdata(pdev); >> struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev); >> >> hub->ocic_notify(NULL); >> usb_remove_hcd(hcd); >> usb_put_hcd(hcd); >> -} >> - >> -static int ohci_hcd_da8xx_drv_probe(struct platform_device *dev) >> -{ >> - return usb_hcd_da8xx_probe(&ohci_da8xx_hc_driver, dev); >> -} >> - >> -static int ohci_hcd_da8xx_drv_remove(struct platform_device *dev) >> -{ >> - struct usb_hcd *hcd = platform_get_drvdata(dev); >> - >> - usb_hcd_da8xx_remove(hcd, dev); >> >> return 0; >> } >> @@ -426,12 +350,16 @@ static int ohci_da8xx_resume(struct platform_device >> *dev) >> } >> #endif >> >> +static const struct ohci_driver_overrides da8xx_overrides __initconst = { >> + .reset = ohci_da8xx_reset >> +}; >> + >> /* >> * Driver definition to register with platform structure. >> */ >> static struct platform_driver ohci_hcd_da8xx_driver = { >> - .probe = ohci_hcd_da8xx_drv_probe, >> - .remove = ohci_hcd_da8xx_drv_remove, >> + .probe = ohci_da8xx_probe, >> + .remove = ohci_da8xx_remove, >> .shutdown = usb_hcd_platform_shutdown, >> #ifdef CONFIG_PM >> .suspend = ohci_da8xx_suspend, > > > It would probably be a good idea to change the driver name here. Currently > it is "ohci". Although this would be better in a separate patch if the name > has to be changed to match in other files as well. > ok, ill do a separate patch for that. >> @@ -442,4 +370,39 @@ static int ohci_da8xx_resume(struct platform_device >> *dev) >> }, >> }; >> >> +static int __init ohci_da8xx_init(void) >> +{ >> + >> + if (usb_disabled()) >> + return -ENODEV; >> + >> + pr_info("%s: " DRIVER_DESC "\n", hcd_name); >> + ohci_init_driver(&ohci_da8xx_hc_driver, &da8xx_overrides); >> + >> + /* >> + * The Davinci da8xx HW has some unusual quirks, which require >> + * da8xx-specific workarounds. We override certain hc_driver >> + * functions here to achieve that. We explicitly do not enhance >> + * ohci_driver_overrides to allow this more easily, since this >> + * is an unusual case, and we don't want to encourage others to >> + * override these functions by making it too easy. >> + */ >> + >> + orig_ohci_hub_control = ohci_da8xx_hc_driver.hub_control; >> + orig_ohci_hub_status_data = ohci_da8xx_hc_driver.hub_status_data; >> + >> + ohci_da8xx_hc_driver.hub_status_data = >> ohci_da8xx_hub_status_data; >> + ohci_da8xx_hc_driver.hub_control = ohci_da8xx_hub_control; >> + >> + return platform_driver_register(&ohci_hcd_da8xx_driver); >> +} >> +module_init(ohci_da8xx_init); >> + >> +static void __exit ohci_da8xx_cleanup(void) > > > ohci_da8xx_exit would be a better name > ok. >> +{ >> + platform_driver_unregister(&ohci_hcd_da8xx_driver); >> +} >> +module_exit(ohci_da8xx_cleanup); >> +MODULE_DESCRIPTION(DRIVER_DESC); >> +MODULE_LICENSE("GPL"); >> MODULE_ALIAS("platform:ohci"); > > > this will need to be changed too if you change the driver name > >> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c >> index 1700908..8de174a 100644 >> --- a/drivers/usb/host/ohci-hcd.c >> +++ b/drivers/usb/host/ohci-hcd.c >> @@ -1219,11 +1219,6 @@ void ohci_init_driver(struct hc_driver *drv, >> #define SA1111_DRIVER ohci_hcd_sa1111_driver >> #endif >> >> -#ifdef CONFIG_USB_OHCI_HCD_DAVINCI >> -#include "ohci-da8xx.c" >> -#define DAVINCI_PLATFORM_DRIVER ohci_hcd_da8xx_driver >> -#endif >> - >> #ifdef CONFIG_USB_OHCI_HCD_PPC_OF >> #include "ohci-ppc-of.c" >> #define OF_PLATFORM_DRIVER ohci_hcd_ppc_of_driver >> @@ -1303,19 +1298,9 @@ static int __init ohci_hcd_mod_init(void) >> goto error_tmio; >> #endif >> >> -#ifdef DAVINCI_PLATFORM_DRIVER >> - retval = platform_driver_register(&DAVINCI_PLATFORM_DRIVER); >> - if (retval < 0) >> - goto error_davinci; >> -#endif >> - >> return retval; >> >> /* Error path */ >> -#ifdef DAVINCI_PLATFORM_DRIVER >> - platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER); >> - error_davinci: >> -#endif >> #ifdef TMIO_OHCI_DRIVER >> platform_driver_unregister(&TMIO_OHCI_DRIVER); >> error_tmio: >> @@ -1351,9 +1336,6 @@ static int __init ohci_hcd_mod_init(void) >> >> static void __exit ohci_hcd_mod_exit(void) >> { >> -#ifdef DAVINCI_PLATFORM_DRIVER >> - platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER); >> -#endif >> #ifdef TMIO_OHCI_DRIVER >> platform_driver_unregister(&TMIO_OHCI_DRIVER); >> #endif >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html