powerpc: Workaround for the PPC440EPX USBH_23 errata

A published errata for ppc440epx states, that when running Linux with both EHCI and OHCI modules loaded, the EHCI module experiences a fatal error when a high-speed device is connected to the USB2.0, and functions normally if OHCI module is not loaded.

Quote from original descriprion:

The 440EPx USB 2.0 Host controller is an EHCI compliant controller. In USB 2.0 Host controllers, each EHCI controller has one or more companion controllers, which may be OHCI or UHCI. An USB 2.0 Host controller will contain one or more ports. For each port, only one of the controllers is connected at any one time. In the 440EPx, there is only one OHCI companion controller, and only one USB 2.0 Host port.
All ports on an USB 2.0 controller default to the companion controller. If you load only an ohci driver, it will have control of the ports and any deviceplugged in will operate, although high speed devices will be forced to operate at full speed. When an ehci driver is loaded, it explicitly takes control of the ports. If there is a device connected, and / or every time there is a new device connected, the ehci driver determines if the device is high speed or not. If it is high speed, the driver retains control of the port. If it is not, the driver explicitly gives the companion controller control of the port.

The is a software workaround that uses
Initial version of the software workaround was posted to linux-usb-devel:

http://www.mail-archive.com/[email protected]/msg54019.html

and later available from amcc.com:
http://www.amcc.com/Embedded/Downloads/download.html?cat=1&family=15&ins=2

The patch below is generally based on the latter, but reworked to powerpc/of_device USB drivers, and uses a few devicetree inquiries to get rid of all the hardcoded defines and CONFIG_* stuff, in favor to defining specific quirk. The latter required to add more accurate description into compatible field of USB node for 'sequioia' board.

drivers/usb/host/ehci-ppc-of.c +3 -2

@@ -107,16 +107,17 @@ ehci_hcd_ppc_of_probe(struct of_device *op, const struct of_device_id *match)
 {
 	struct device_node *dn = op->node;
 	struct usb_hcd *hcd;
-	struct ehci_hcd	*ehci;
+	struct ehci_hcd	*ehci = NULL;
 	struct resource res;
 	int irq;
 	int rv;
 
+	struct device_node *np;
+
 	if (usb_disabled())
 		return -ENODEV;
 
 	dev_dbg(&op->dev, "initializing PPC-OF USB Controller\n");
-
 	rv = of_address_to_resource(dn, 0, &res);
 	if (rv)
 		return rv;

drivers/usb/host/ehci-ppc-of.c +2 -0

 	hcd->rsrc_start = res.start;
 	hcd->rsrc_len = res.end - res.start + 1;
 

@@ -172,6 +174,21 @@ ehci_hcd_ppc_of_probe(struct of_device *op, const struct of_device_id *match)
 				rv ? "NOT ": "");
 	}
 
+	np = of_find_compatible_node(NULL, NULL, "ibm,usb-ohci-440epx");
+	if (np != NULL) {

drivers/usb/host/ehci.h +4 -0

@@ -809,6 +819,20 @@ static inline u32 hc32_to_cpup (const struct ehci_hcd *ehci, const __hc32 *x)
 		le32_to_cpup((__force __le32 *)x);
 }
 
+static inline void set_ohci_hcfs(struct ehci_hcd *ehci, int operational)
+{
+	u32 hc_control;
+

drivers/usb/host/ohci-ppc-of.c +9 -0

@@ -148,6 +148,31 @@ ohci_hcd_ppc_of_probe(struct of_device *op, const struct of_device_id *match)
 	if (rv == 0)
 		return 0;
 
+	/* by now, 440epx is known to show usb_23 erratum */
+	np = of_find_compatible_node(NULL, NULL, "ibm,usb-ehci-440epx");
+
+	/* Work around - At this point ohci_run has executed, the
+	* controller is running, everything, the root ports, etc., is
+	* set up.  If the ehci driver is loaded, put the ohci core in
+	* the suspended state.  The ehci driver will bring it out of
+	* suspended state when / if a non-high speed USB device is
+	* attached to the USB Host port.  If the ehci driver is not

Quoted 17 lines

A published errata for ppc440epx states, that when running Linux with both EHCI and OHCI modules loaded, the EHCI module experiences a fatal error when a high-speed device is connected to the USB2.0, and functions normally if OHCI module is not loaded.

Quote from original descriprion:

The 440EPx USB 2.0 Host controller is an EHCI compliant controller. In USB 2.0 Host controllers, each EHCI controller has one or more companion controllers, which may be OHCI or UHCI. An USB 2.0 Host controller will contain one or more ports. For each port, only one of the controllers is connected at any one time. In the 440EPx, there is only one OHCI companion controller, and only one USB 2.0 Host port. All ports on an USB 2.0 controller default to the companion controller. If you load only an ohci driver, it will have control of the ports and any deviceplugged in will

Quoted 8 lines

operate, although high speed devices will be forced to operate at full speed. When an ehci driver is loaded, it explicitly takes control of the ports. If there is a device connected, and / or every time there is a new device connected, the ehci driver determines if the device is high speed or not. If it is high speed, the driver retains control of the port. If it is not, the driver explicitly gives the companion controller control of the port.

This doesn't explain why the fatal error occurs.

Quoted 9 lines

The is a software workaround that uses
Initial version of the software workaround was posted to linux-usb-devel:

http://www.mail-archive.com/[email protected]/msg54019.html

and later available from amcc.com:
http://www.amcc.com/Embedded/Downloads/download.html?cat=1&family=15&ins=2

Quoted 6 lines

The patch below is generally based on the latter, but reworked to powerpc/of_device USB drivers, and uses a few devicetree inquiries to get rid of all the hardcoded defines and CONFIG_* stuff, in favor to defining specific quirk. The latter required to add more accurate description into compatible field of USB node for 'sequioia' board.

I have some doubts about parts of this patch.

Quoted 24 lines

drivers/usb/host/ehci-ppc-of.c +3 -2

@@ -107,16 +107,17 @@ ehci_hcd_ppc_of_probe(struct of_device *op,
 const struct of_device_id *match) {
 	struct device_node *dn = op->node;
 	struct usb_hcd *hcd;
-	struct ehci_hcd	*ehci;
+	struct ehci_hcd	*ehci = NULL;
 	struct resource res;
 	int irq;
 	int rv;
 
+	struct device_node *np;
+
 	if (usb_disabled())
 		return -ENODEV;
 
 	dev_dbg(&op->dev, "initializing PPC-OF USB Controller\n");
-
 	rv = of_address_to_resource(dn, 0, &res);
 	if (rv)
 		return rv;

drivers/usb/host/ehci-ppc-of.c ehci_hcd_ppc_of_probe(struct of_device *op, · line 129

Quoted 4 lines

drivers/usb/host/ehci-ppc-of.c +1 -0

@@ -125,6 +126,7 @@ ehci_hcd_ppc_of_probe(struct of_device *op,
 const struct of_device_id *match) if (!hcd)
 		return -ENOMEM;
 
+

Is there any reason for these apparently gratuitous whitespace changes?

Quoted 22 lines

drivers/usb/host/ehci-ppc-of.c +11 -0

 	hcd->rsrc_start = res.start;
 	hcd->rsrc_len = res.end - res.start + 1;
 

@@ -172,6 +174,21 @@ ehci_hcd_ppc_of_probe(struct of_device *op,
 const struct of_device_id *match) rv ? "NOT ": "");
 	}
 
+	np = of_find_compatible_node(NULL, NULL,
 "ibm,usb-ohci-440epx");
+	if (np != NULL) {
+	/* claim we really affected by usb23 erratum */
+		ehci->has_amcc_usb23 = 1;
+		if (!of_address_to_resource(np, 0, &res))
+			ehci->ohci_hcctrl_reg = ioremap(res.start +
+					OHCI_HCCTRL_OFFSET,
 OHCI_HCCTRL_LEN);
+		else
+			pr_debug(__FILE__ ": no ohci offset in
 fdt\n");
+		if (!ehci->ohci_hcctrl_reg) {
+			pr_debug(__FILE__ ": ioremap for ohci

hcctrl failed\n");

drivers/usb/host/ehci-ppc-of.c ehci_hcd_ppc_of_probe(struct of_device *op, const struct of_device_id *match) · line 188

Quoted 1 line

drivers/usb/host/ehci-ppc-of.c +1 -0

+			return -ENOMEM;

This is a memory leak.

Quoted 16 lines

drivers/usb/host/ehci.h +8 -0

@@ -809,6 +819,20 @@ static inline u32 hc32_to_cpup (const struct
 ehci_hcd *ehci, const __hc32 *x) le32_to_cpup((__force __le32 *)x);
 }
 
+static inline void set_ohci_hcfs(struct ehci_hcd *ehci, int
 operational) +{
+	u32 hc_control;
+
+	hc_control = (readl_be(ehci->ohci_hcctrl_reg) &
 ~OHCI_CTRL_HCFS);
+	if (operational)
+		hc_control |= OHCI_USB_OPER;
+	else
+		hc_control |= OHCI_USB_SUSPEND;

drivers/usb/host/ehci.h static inline u32 hc32_to_cpup (const struct ehci_hcd *ehci, const __hc32 *x) · line 835

Quoted 5 lines

drivers/usb/host/ehci.h +5 -0

+
+	writel_be(hc_control, ehci->ohci_hcctrl_reg);
+	(void) readl_be(ehci->ohci_hcctrl_reg);
+}
+

This should be protected by a preprocessor test so that the code doesn't get compiled on architectures that don't need it.

Quoted 32 lines

drivers/usb/host/ohci-ppc-of.c +2 -0

@@ -148,6 +148,31 @@ ohci_hcd_ppc_of_probe(struct of_device *op,
 const struct of_device_id *match) if (rv == 0)
 		return 0;
 
+	/* by now, 440epx is known to show usb_23 erratum */
+	np = of_find_compatible_node(NULL, NULL,

"ibm,usb-ehci-440epx"); +

drivers/usb/host/ohci-ppc-of.c +2 -0

+	/* Work around - At this point ohci_run has executed, the
+	* controller is running, everything, the root ports, etc.,

is

drivers/usb/host/ohci-ppc-of.c +1 -0

+	* set up.  If the ehci driver is loaded, put the ohci core

in

drivers/usb/host/ohci-ppc-of.c +1 -0

+	* the suspended state.  The ehci driver will bring it out

of

drivers/usb/host/ohci-ppc-of.c +9 -0

+	* suspended state when / if a non-high speed USB device is
+	* attached to the USB Host port.  If the ehci driver is not
+	* loaded, do nothing. request_mem_region is used to test if
+	* the ehci driver is loaded.
+	*/
+	if (np !=  NULL) {
+		ohci->flags |= OHCI_QUIRK_AMCC_USB23;
+		if (!of_address_to_resource(np, 0, &res))
+			if (!request_mem_region(res.start, 0x4,

hcd_name)) {
+
writel_be((readl_be(&ohci->regs->control) |

drivers/usb/host/ohci-ppc-of.c +1 -0

+				    OHCI_USB_SUSPEND),

&ohci->regs->control);

drivers/usb/host/ohci-ppc-of.c +1 -0

+				    (void)

Quoted 1 line

readl_be(&ohci->regs->control);

Wrong indentation level.

Quoted 6 lines

drivers/usb/host/ohci-ppc-of.c +5 -0

+			} else  {
+				release_mem_region(res.start, 0x4);
+			}
+		else
+		    pr_debug(__FILE__ ": cannot get ehci offset

from fdt\n");

Wrong indentation amount.

drivers/usb/host/ohci-ppc-of.c ohci_hcd_ppc_of_probe(struct of_device *op, const struct of_device_id *match) · line 178

Quoted 4 lines

drivers/usb/host/ohci-ppc-of.c +1 -0

+	}
 	iounmap(hcd->regs);
 err_ioremap:
 	irq_dispose_mapping(irq);

I doubt this will interact properly with usbcore and the rest of ohci-hcd. They do not expect the root hub to be suspended unless they know about it.

Quoted 2 lines

This doesn't explain why the fatal error occurs.

On certain 44x set of SoCs, only one controller is able to function, e.g. technically they are mutually exclusive.

There used to be recommendation to use only hi-speed or full-speed devices with specific conditions, when respective module was unloaded. Later, it was observed that ohci suspend is enough to keep things going, and it was turned into workaround, as explained below.

Quoted 1 line

I have some doubts about parts of this patch.

What stands about noted gotchas, I agree and will fix them, thanks for looking through the patch. I agree this doesn't look pleasant, but unfortunately is the only way to say use USB keyboard, and hi-speed memory stick at the same time.

Quoted 3 lines

I doubt this will interact properly with usbcore and the rest of ohci-hcd. They do not expect the root hub to be suspended unless they know about it.

I need to reemphasize, that upper is not "normal", but unfortunately the only way to have both full and hi-speed usb live in peace with 44xEPX family. Quirks are not going to be executed on other chip anyway, and on chip in question, it was tested and works stable enough.

I can add an explicit warning, that workaround is being utilised, to make things clear if it will be considered appropriate.

Your original post mentioned that the 440EPx has only one USB 2.0 host port. Then how can you use a keyboard and memory stick at the same time? You'd have to plug them into a hub -- in which case only one controller would be needed, the one driving the hub. The patch would be unnecessary.

Quoted 11 lines

Quoted 4 lines

I doubt this will interact properly with usbcore and the rest of ohci-hcd. They do not expect the root hub to be suspended unless they know about it.

I need to reemphasize, that upper is not "normal", but unfortunately the only way to have both full and hi-speed usb live in peace with 44xEPX family. Quirks are not going to be executed on other chip anyway, and on chip in question, it was tested and works stable enough.

I can add an explicit warning, that workaround is being utilised, to make things clear if it will be considered appropriate.

Do these systems support CONFIG_PM? If they do then your patch shouldn't be needed, because then ohci-hcd will automatically suspend the OHCI root hub 1 second after the last full/low-speed device is unplugged. You could reduce that 1 second value if you wanted to decrease the probability of conflicts.

Alan Stern

_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Quoted 5 lines · Alan Stern

Your original post mentioned that the 440EPx has only one USB 2.0 host port. Then how can you use a keyboard and memory stick at the same time? You'd have to plug them into a hub -- in which case only one controller would be needed, the one driving the hub. The patch would be unnecessary.

I have one of these processors on a Sequoia board. What happens is that if you build the kernel with both EHCI and OHCI support, then plug in a modern USB memory stick, it initially tries EHCI, the driver fails, and the whole thing falls back to OHCI. So you wind up running at 12 Mbps. The only way to make high speed work is to turn off the OHCI driver, and then you cannot support slow devices with that kernel.

So, hile you cannot plug two devices in at one time, you can plug in different speed devices at different times, and my understanding is that this patch will let that work seamlessly.

Is there some reason why it doesn't work already? All the patch does is suspend the OHCI root hub when you plug in the memory stick -- but the root hub should already be suspended.

Unless the memory stick is already plugged in when the kernel boots. In which case the root hub won't be suspended -- it won't suspend until
1 second after ohci-hcd initializes the controller. Is that the scenario you're worried about?

I suppose some embedded platforms don't use CONFIG_PM, is this still a requirement for autosuspend ? Or do that happen always on an empty port nowadays ?

Quoted 4 lines

I suppose some embedded platforms don't use CONFIG_PM, is this still a requirement for autosuspend ? Or do that happen always on an empty port nowadays ?

ohci-hcd doesn't automatically suspend the root hub if CONFIG_PM isn't set. However it isn't necessary to set CONFIG_SLEEP, CONFIG_HIBERNATION, or CONFIG_USB_SUSPEND.

But even assuming PM set, common use-case of
embedded systems to have stuff on USB bus that is never plugged off; and in case of compiled-in ehci and ohci there is just

Initializing USB Mass Storage driver...
usb 1-1: new high speed USB device using ppc-of-ehci and address 2 usb 1-1: device descriptor read/all, error -110
hub 1-0:1.0: unable to enumerate USB device on port 1

(not touching PM here to be clear)

even 1-second delay would be enough for root hub to be hosed... So, is the patch (with all the issues addressed) acceptable for mainline?

Quoted 10 lines

But even assuming PM set, common use-case of
embedded systems to have stuff on USB bus that is never plugged off; and in case of compiled-in ehci and ohci there is just

Initializing USB Mass Storage driver...
usb 1-1: new high speed USB device using ppc-of-ehci and address 2 usb 1-1: device descriptor read/all, error -110
hub 1-0:1.0: unable to enumerate USB device on port 1

Quoted 5 lines

(not touching PM here to be clear)

even 1-second delay would be enough for root hub to be hosed... So, is the patch (with all the issues addressed) acceptable for mainline?

Try doing this instead. First, make sure CONFIG_PM is set! :-) Then replace your patch with something that adds a 2-second delay to the end of ehci_start() when running on a 440EPx system. That should give the OHCI controller time to suspend before the EHCI root hub is registered.

A little more tweaking will be needed to handle system sleeps. But this should be a good start.

What to do when CONFIG_PM is off is a separate matter. Let's not worry about it for now -- especially since, as Matthias suggested, you can use a USB 2.0 hub.

I've started looking this way. Sorry, but this approach is a little bit fragile, and unreliable.

First, it just did not work out in case of usb2 hub with memory stick and keybord plugged - OHCI did not suspend, ehci is hosed and we're happily using hi-speed device on full-speed:

Secondly, we may *not* rely on the fact, that OHCI will always have the same suspend policy. Even kicking the code up to the shape when it will automagically suspend in proper timing to get the HW issue around, we cannot be sure that it will persist along kernel lifecycle, and won't require concerned people to kick suspend timings back to the working state subsequently each rc release.

Thirdly, PM is disabled by Kconfig explicitly in case of 44x. Reasoning is not clear at the moment, but I believe that isn't there just in case.

Quoted 3 lines

What to do when CONFIG_PM is off is a separate matter. Let's not worry about it for now -- especially since, as Matthias suggested, you can use a USB 2.0 hub.

Not every hub will work (none of available did so far), and it is often not an option for embedded device without rewiring.

As this touches powerpc stuff only, are there any objections to let powerpc peolple consider if approach suggested earlier is applicable or not?

I assume that's the reason the suggested approach failed.

I don't mind doing that, provided the changes are cleaned up so that they don't affect people who aren't building kernels for 44x systems.

Quoted 15 lines

I've started looking this way. Sorry, but this approach is a little bit fragile, and unreliable.

First, it just did not work out in case of usb2 hub with memory stick and keybord plugged - OHCI did not suspend, ehci is hosed and we're happily using hi-speed device on full-speed:

Secondly, we may *not* rely on the fact, that OHCI will always have the same suspend policy. Even kicking the code up to the shape when it will automagically suspend in proper timing to get the HW issue around, we cannot be sure that it will persist along kernel lifecycle, and won't require concerned people to kick suspend timings back to the working state subsequently each rc release.

Quoted 3 lines

Thirdly, PM is disabled by Kconfig explicitly in case of 44x. Reasoning is not clear at the moment, but I believe that isn't there just in case.

I assume that's the reason the suggested approach failed.

Quoted 7 lines

Quoted 4 lines

What to do when CONFIG_PM is off is a separate matter. Let's not worry about it for now -- especially since, as Matthias suggested, you can use a USB 2.0 hub.

Not every hub will work (none of available did so far), and it is often not an option for embedded device without rewiring.

It's odd that your hubs don't work. What's wrong with them?

Quoted 3 lines

As this touches powerpc stuff only, are there any objections to let powerpc peolple consider if approach suggested earlier is applicable or not?

I don't mind doing that, provided the changes are cleaned up so that they don't affect people who aren't building kernels for 44x systems.

Quoted 5 lines

Quoted 2 lines

Not every hub will work (none of available did so far), and it is often not an option for embedded device without rewiring.

It's odd that your hubs don't work. What's wrong with them?

well, they do not have transaction translators then. Nothing really wrong

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

Not every hub will work (none of available did so far), and it is often not an option for embedded device without rewiring.

It's odd that your hubs don't work. What's wrong with them?

Quoted 2 lines

well, they do not have transaction translators then. Nothing really wrong

No. If the hubs run at high speed then they must have transaction translators; it's required by the USB 2.0 specification.

Did you try using them with ohci-hcd not loaded?

Alan Stern

Quoted 13 lines · Steven A. Falco

Quoted 5 lines · Alan Stern

Your original post mentioned that the 440EPx has only one USB 2.0 host port. Then how can you use a keyboard and memory stick at the same time? You'd have to plug them into a hub -- in which case only one controller would be needed, the one driving the hub. The patch would be unnecessary.

I have one of these processors on a Sequoia board. What happens is that if you build the kernel with both EHCI and OHCI support, then plug in a modern USB memory stick, it initially tries EHCI, the driver fails, and the whole thing falls back to OHCI. So you wind up running at 12 Mbps. The only way to make high speed work is to turn off the OHCI driver, and then you cannot support slow devices with that kernel.

Quoted 3 lines

So, hile you cannot plug two devices in at one time, you can plug in different speed devices at different times, and my understanding is that this patch will let that work seamlessly.

Is there some reason why it doesn't work already? All the patch does is suspend the OHCI root hub when you plug in the memory stick -- but the root hub should already be suspended.

Unless the memory stick is already plugged in when the kernel boots. In which case the root hub won't be suspended -- it won't suspend until 1 second after ohci-hcd initializes the controller. Is that the scenario you're worried about?

Alan Stern

Quoted 5 lines · Alan Stern

Your original post mentioned that the 440EPx has only one USB 2.0 host port. Then how can you use a keyboard and memory stick at the same time? You'd have to plug them into a hub -- in which case only one controller would be needed, the one driving the hub. The patch would be unnecessary.

I have one of these processors on a Sequoia board. What happens is that if you build the kernel with both EHCI and OHCI support, then plug in a modern USB memory stick, it initially tries EHCI, the driver fails, and the whole thing falls back to OHCI. So you wind up running at 12 Mbps. The only way to make high speed work is to turn off the OHCI driver, and then you cannot support slow devices with that kernel.

So, hile you cannot plug two devices in at one time, you can plug in different speed devices at different times, and my understanding is that this patch will let that work seamlessly.