[PATCH V5 5/9] USB: OHCI: make ohci-at91 a separate driver

Subsystems: the rest, usb ohci driver, usb subsystem

STALE4746d

5 messages, 3 authors, 2013-09-27 · open the first message on its own page

[PATCH V5 5/9] USB: OHCI: make ohci-at91 a separate driver

From: Kevin Hilman <hidden>
Date: 2013-09-27 15:10:32 

Manjunath,

Manjunath Goudar [off-list ref] writes:

Separate the  TI OHCI Atmel 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.
This broke booting on atmel sama5 boards (and likely others with the
same conversion)...

[...]

+static int __init ohci_at91_init(void)
+{
+	if (usb_disabled())
+		return -ENODEV;
+
+	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+	ohci_init_driver(&ohci_at91_hc_driver, NULL);
ohci_init_driver() doesn't have any sanity checks for NULL overrides, so
it blindly dereferences and faults.

Some of the other conversions have this same problem (at least omap3).
Did anyone test this series on hardware?

I'm not too familiar with OHCI, but something like the patch below is
probably needed along with this series.

Kevin


From a3b5cc90e74038a6449fbd25e0d720ea02884f30 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <redacted>
Date: Fri, 27 Sep 2013 08:07:19 -0700
Subject: [PATCH] USB: OHCI: ohci_init_driver(): sanity check overrides

Check for non-NULL overrides before dereferencing since platforms may
pass in NULL overrides.

Signed-off-by: Kevin Hilman <redacted>
---
 drivers/usb/host/ohci-hcd.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 21d937a..8ada13f 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1161,10 +1161,12 @@ void ohci_init_driver(struct hc_driver *drv,
 	/* Copy the generic table to drv and then apply the overrides */
 	*drv = ohci_hc_driver;
 
-	drv->product_desc = over->product_desc;
-	drv->hcd_priv_size += over->extra_priv_size;
-	if (over->reset)
-		drv->reset = over->reset;
+	if (over) {
+		drv->product_desc = over->product_desc;
+		drv->hcd_priv_size += over->extra_priv_size;
+		if (over->reset)
+			drv->reset = over->reset;
+	}
 }
 EXPORT_SYMBOL_GPL(ohci_init_driver);
 
-- 
1.8.3

[PATCH V5 5/9] USB: OHCI: make ohci-at91 a separate driver

From: Greg KH <hidden>
Date: 2013-09-27 15:25:21 

On Fri, Sep 27, 2013 at 08:10:32AM -0700, Kevin Hilman wrote:
Manjunath,

Manjunath Goudar [off-list ref] writes:
quoted
Separate the  TI OHCI Atmel 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.
This broke booting on atmel sama5 boards (and likely others with the
same conversion)...
Crap.

And it looks like Manjunath is now no longer working at Linaro, so these
patches are "abandoned" :(

Unless someone at Linaro steps up in the next few days to fix these,
I'll have to go revert them all.  Again.

bleah.

greg k-h

[PATCH V5 5/9] USB: OHCI: make ohci-at91 a separate driver

From: stern@rowland.harvard.edu (Alan Stern)
Date: 2013-09-27 15:37:16 

On Fri, 27 Sep 2013, Greg KH wrote:

On Fri, Sep 27, 2013 at 08:10:32AM -0700, Kevin Hilman wrote:
quoted
Manjunath,

Manjunath Goudar [off-list ref] writes:
quoted
Separate the  TI OHCI Atmel 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.
This broke booting on atmel sama5 boards (and likely others with the
same conversion)...
Crap.

And it looks like Manjunath is now no longer working at Linaro, so these
patches are "abandoned" :(

Unless someone at Linaro steps up in the next few days to fix these,
I'll have to go revert them all.  Again.

bleah.
It's not so bad as all that.  The patch Kevin wrote duplicates what 
ehci-hcd does.  It's an obvious oversight; I need to work harder at 
keeping the two drivers in sync.

Alan Stern

[PATCH V5 5/9] USB: OHCI: make ohci-at91 a separate driver

From: stern@rowland.harvard.edu (Alan Stern)
Date: 2013-09-27 15:38:30 

On Fri, 27 Sep 2013, Kevin Hilman wrote:
quoted hunk
Manjunath,

Manjunath Goudar [off-list ref] writes:
quoted
Separate the  TI OHCI Atmel 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.
This broke booting on atmel sama5 boards (and likely others with the
same conversion)...

[...]
quoted
+static int __init ohci_at91_init(void)
+{
+	if (usb_disabled())
+		return -ENODEV;
+
+	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+	ohci_init_driver(&ohci_at91_hc_driver, NULL);
ohci_init_driver() doesn't have any sanity checks for NULL overrides, so
it blindly dereferences and faults.

Some of the other conversions have this same problem (at least omap3).
Did anyone test this series on hardware?

I'm not too familiar with OHCI, but something like the patch below is
probably needed along with this series.

Kevin


From a3b5cc90e74038a6449fbd25e0d720ea02884f30 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <redacted>
Date: Fri, 27 Sep 2013 08:07:19 -0700
Subject: [PATCH] USB: OHCI: ohci_init_driver(): sanity check overrides

Check for non-NULL overrides before dereferencing since platforms may
pass in NULL overrides.

Signed-off-by: Kevin Hilman <redacted>
---
 drivers/usb/host/ohci-hcd.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 21d937a..8ada13f 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1161,10 +1161,12 @@ void ohci_init_driver(struct hc_driver *drv,
 	/* Copy the generic table to drv and then apply the overrides */
 	*drv = ohci_hc_driver;
 
-	drv->product_desc = over->product_desc;
-	drv->hcd_priv_size += over->extra_priv_size;
-	if (over->reset)
-		drv->reset = over->reset;
+	if (over) {
+		drv->product_desc = over->product_desc;
+		drv->hcd_priv_size += over->extra_priv_size;
+		if (over->reset)
+			drv->reset = over->reset;
+	}
 }
 EXPORT_SYMBOL_GPL(ohci_init_driver);
Yes, this is exactly what ehci-hcd does and it clearly is necessary.

Acked-by: Alan Stern <stern@rowland.harvard.edu>

[PATCH V5 5/9] USB: OHCI: make ohci-at91 a separate driver

From: Greg KH <hidden>
Date: 2013-09-27 15:40:20 

On Fri, Sep 27, 2013 at 11:37:16AM -0400, Alan Stern wrote:
On Fri, 27 Sep 2013, Greg KH wrote:
quoted
On Fri, Sep 27, 2013 at 08:10:32AM -0700, Kevin Hilman wrote:
quoted
Manjunath,

Manjunath Goudar [off-list ref] writes:
quoted
Separate the  TI OHCI Atmel 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.
This broke booting on atmel sama5 boards (and likely others with the
same conversion)...
Crap.

And it looks like Manjunath is now no longer working at Linaro, so these
patches are "abandoned" :(

Unless someone at Linaro steps up in the next few days to fix these,
I'll have to go revert them all.  Again.

bleah.
It's not so bad as all that.  The patch Kevin wrote duplicates what 
ehci-hcd does.  It's an obvious oversight; I need to work harder at 
keeping the two drivers in sync.
Ah, ok, so Kevin's patch should resolve it all?  That's good, I'll go
queue it up.

greg k-h
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help