On 18 July 2013 21:20, Alan Stern stern@rowland.harvard.edu wrote:
On Tue, 25 Jun 2013, Manjunath Goudar wrote:
Separate the TI OHCI OMAP1/2 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.
I'm sorry it took so long to review this; I have been very busy.
This patch is almost right. There are just a couple of problems involving the clock_power calls.
It's ok. Thank you very much for your valuable comments. These issue I will fix in next version of this series.
@@ -354,12 +358,6 @@ static int usb_hcd_omap_probe (const struct
hc_driver *driver,
goto err2; }
ohci = hcd_to_ohci(hcd);
ohci_hcd_init(ohci);
host_initialized = 0;
host_enabled = 1;
irq = platform_get_irq(pdev, 0); if (irq < 0) { retval = -ENXIO;
@@ -369,11 +367,7 @@ static int usb_hcd_omap_probe (const struct
hc_driver *driver,
if (retval) goto err3;
host_initialized = 1;
if (!host_enabled)
omap_ohci_clock_power(0);
omap_ohci_clock_power(0);
Since host_enabled was always 1 at this point, omap_ohci_clock_power() would never be called. You should remove it.
@@ -402,6 +396,8 @@ err0: static inline void usb_hcd_omap_remove (struct usb_hcd *hcd, struct platform_device *pdev) {
dev_dbg(hcd->self.controller, "stopping USB Controller\n");
omap_ohci_clock_power(0); usb_remove_hcd(hcd); if (!IS_ERR_OR_NULL(hcd->phy)) { (void) otg_set_host(hcd->phy->otg, 0);
omap_ohci_clock_power() should be called after usb_remove_hcd(), not before. This reason is simple: Until usb_remove_hcd() is called, the controller is still running and therefore needs to have a valid clock signal.
Alan Stern
Manjunath Goudar