Re: [linux-usb-devel] [PATCH 2.6.21] Work around for PPC440EPX USBH_23 errrata
On Sunday, May 20, 2007, Pete Zaitcev wrote: On Fri, 18 May 2007 14:23:43 -0700, Mark Miesfeld [EMAIL PROTECTED] wrote: There is a published errata for the PPC440EPX, USBH_23: EHCI and OHCI Linux module contention. So, you're set for the classic (and unsoluble) inter-module problem, but only if you attempt to keep parts accessing OHCI inside the ohci-hcd. In short, this is the part which knows when to poke OHCI: - } else +#ifdef CONFIG_USB_PPC440EPX_USBH_23_ERRATA + /* ensure 440EPX ohci controller state is operational */ + ohci_ppc_set_hcfs(HCFS_OPERATIONAL); +#endif And this is the part which implements poking: + + /* write the new state and flush the write. */ + ohci_writel(cached_ohci, hc_control, cached_ohci-regs-control); + (void) ohci_readl(cached_ohci, cached_ohci-regs-control); It looks to me that the second part does not have to be located in ohci-hcd. On PPC, ohci_writel only uses the ohci private struct to determine the endianness, presumably known in your case. So, plain readl can be used. This only leaves the address for readl, which comes from ioremap. If PPC allowed aliased ioremaps, you'd be golden: just do a second ioremap. Can you do that? Please give it a thought. Yes, that would be much better. I'll look into doing that. If I can't, I'll use your idea below. +struct ohci_hcd*cached_ohci = NULL; This is just nasty. If aliased ioremaps are not possible, you better create a global symbol for this in the hcd.c or other core part. Fill the word when ohci probes, let ehci use it. Then, see above. Thanks for the good suggestions. -- Mark - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] [PATCH 2.6.21] Work around for PPC440EPX USBH_23 errrata
On Sunday, May 20, 2007, Pete Zaitcev wrote: On Sun, 20 May 2007 21:55:58 -0700, David Brownell [EMAIL PROTECTED] wrote: If you're going to turn on the OHCI hardware, do it the normal way; don't bypass its driver. How exactly do you suggest we do this? The problem is not quite dissimilar to that of BIOS handoff. There, we solved the issue by having a compilation unit (pci-quirks.c) which textually belongs to the driver, yet is built into kernel. What I proposed is more like how the handoff quirk worked before. But if you know an sane alternative, I'll be happy (ok, Mark may be happier). Yes, I would be happier. But, I don't see how to do it. -- Mark - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] [PATCH 2.6.21] Work around for PPC440EPX USBH_23 errrata
On Sunday, May 20, 2007, David Brownell
On Sunday 20 May 2007, Pete Zaitcev wrote:
On Fri, 18 May 2007 14:23:43 -0700, Mark Miesfeld [EMAIL PROTECTED]
wrote:
And this is the part which implements poking:
+++ b/drivers/usb/host/ohci-ppc-soc.c
@@ -20,6 +20,37 @@ #include linux/signal.h
/* always called with process context; sleeping is OK */
+#ifdef CONFIG_USB_PPC440EPX_USBH_23_ERRATA
+struct ohci_hcd *cached_ohci = NULL;
+#define HCFS_SUSPEND 0
+#define HCFS_OPERATIONAL 1
+void ohci_ppc_set_hcfs(int state) {
+ u32 hc_control;
+
+ hc_control = (ohci_readl(cached_ohci, cached_ohci-regs-control)
+~OHCI_CTRL_HCFS);
+
+ switch ( state ) {
+ case HCFS_SUSPEND:
+ hc_control |= OHCI_USB_SUSPEND;
+ break;
+ case HCFS_OPERATIONAL :
+ hc_control |= OHCI_USB_OPER;
So, basically you'll turn on OHCI hardware without having the
driver know that it's on ... ?
I realize this is not ideal, but because of the errata, the situation
is not ideal.
At the point that the hardware is turned on, the driver thinks that
the hardware is on.
After the ohci driver is completely set up, the HCFS field in the contrller
core is set to suspend. As near as I can tell, the ohci driver will not
do anything until it gets an interrupt and it will not get an interrupt until
the ehci driver explictly turns over control of the port.
Which it only does when it detects that a non-high speed device is connected.
Then, if the ehci driver detects a new connection, and it is a high speed
device, the HCFS field in the ohci controller core is set back to suspend.
This looks almost certain to break something -- deeply.
If you're going to turn on the OHCI hardware, do it the
normal way; don't bypass its driver.
I don't see how to do that. Because of the errata, the ohci driver
needs to be completely set up and the HCFS field in the core set to
suspend until, and only until, a full speed device is connected. Then,
if that device is disconnected and a high speed device connected, the
HCFS field needs to be set to suspend.
--
Mark
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] [PATCH 2.6.21] Work around for PPC440EPX USBH_23 errrata
On Fri, 18 May 2007 14:23:43 -0700, Mark Miesfeld [EMAIL PROTECTED] wrote:
There is a published errata for the PPC440EPX, USBH_23: EHCI and OHCI
Linux module contention. The overview states: 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 Host controller.
The EHCI module functions normally when the OHCI module is not loaded.
So, you're set for the classic (and unsoluble) inter-module problem,
but only if you attempt to keep parts accessing OHCI inside the ohci-hcd.
In short, this is the part which knows when to poke OHCI:
+++ b/drivers/usb/host/ehci-hub.c
@@ -319,8 +325,17 @@ static int check_reset_complete (
port_status = ~PORT_RWC_BITS;
ehci_writel(ehci, port_status, status_reg);
- } else
+#ifdef CONFIG_USB_PPC440EPX_USBH_23_ERRATA
+ /* ensure 440EPX ohci controller state is operational */
+ ohci_ppc_set_hcfs(HCFS_OPERATIONAL);
+#endif
And this is the part which implements poking:
+++ b/drivers/usb/host/ohci-ppc-soc.c
@@ -20,6 +20,37 @@ #include linux/signal.h
/* always called with process context; sleeping is OK */
+#ifdef CONFIG_USB_PPC440EPX_USBH_23_ERRATA
+struct ohci_hcd *cached_ohci = NULL;
+#define HCFS_SUSPEND 0
+#define HCFS_OPERATIONAL 1
+void ohci_ppc_set_hcfs(int state) {
+ u32 hc_control;
+
+ hc_control = (ohci_readl(cached_ohci, cached_ohci-regs-control)
+~OHCI_CTRL_HCFS);
+
+ switch ( state ) {
+ case HCFS_SUSPEND:
+ hc_control |= OHCI_USB_SUSPEND;
+ break;
+ case HCFS_OPERATIONAL :
+ hc_control |= OHCI_USB_OPER;
+ break;
+ default:
+ /* this is unexpected, shoud never happen */
+ hc_control |= OHCI_USB_SUSPEND;
+ break;
+ }
+
+ /* write the new state and flush the write. */
+ ohci_writel(cached_ohci, hc_control, cached_ohci-regs-control);
+ (void) ohci_readl(cached_ohci, cached_ohci-regs-control);
+}
+EXPORT_SYMBOL (ohci_ppc_set_hcfs);
It looks to me that the second part does not have to be located in
ohci-hcd. On PPC, ohci_writel only uses the ohci private struct
to determine the endianness, presumably known in your case. So,
plain readl can be used. This only leaves the address for readl,
which comes from ioremap. If PPC allowed aliased ioremaps, you'd
be golden: just do a second ioremap. Can you do that? Please give
it a thought.
+struct ohci_hcd *cached_ohci = NULL;
This is just nasty.
If aliased ioremaps are not possible, you better create a global
symbol for this in the hcd.c or other core part. Fill the word
when ohci probes, let ehci use it. Then, see above.
The patch was linewrapped, the coding style was horrible, but
it was all secondary to the overall architecture. Once you get
rid of inter-module interaction, we can look at small things.
They are easy.
Best wishes,
-- Pete
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] [PATCH 2.6.21] Work around for PPC440EPX USBH_23 errrata
On Sunday 20 May 2007, Pete Zaitcev wrote:
On Fri, 18 May 2007 14:23:43 -0700, Mark Miesfeld [EMAIL PROTECTED] wrote:
And this is the part which implements poking:
+++ b/drivers/usb/host/ohci-ppc-soc.c
@@ -20,6 +20,37 @@ #include linux/signal.h
/* always called with process context; sleeping is OK */
+#ifdef CONFIG_USB_PPC440EPX_USBH_23_ERRATA
+struct ohci_hcd*cached_ohci = NULL;
+#define HCFS_SUSPEND 0
+#define HCFS_OPERATIONAL 1
+void ohci_ppc_set_hcfs(int state) {
+ u32 hc_control;
+
+ hc_control = (ohci_readl(cached_ohci, cached_ohci-regs-control)
+ ~OHCI_CTRL_HCFS);
+
+ switch ( state ) {
+ case HCFS_SUSPEND:
+ hc_control |= OHCI_USB_SUSPEND;
+ break;
+ case HCFS_OPERATIONAL :
+ hc_control |= OHCI_USB_OPER;
So, basically you'll turn on OHCI hardware without having the
driver know that it's on ... ?
This looks almost certain to break something -- deeply.
If you're going to turn on the OHCI hardware, do it the
normal way; don't bypass its driver.
- Dave
+ break;
+ default:
+ /* this is unexpected, shoud never happen */
+ hc_control |= OHCI_USB_SUSPEND;
+ break;
+ }
+
+ /* write the new state and flush the write. */
+ ohci_writel(cached_ohci, hc_control, cached_ohci-regs-control);
+ (void) ohci_readl(cached_ohci, cached_ohci-regs-control);
+}
+EXPORT_SYMBOL (ohci_ppc_set_hcfs);
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] [PATCH 2.6.21] Work around for PPC440EPX USBH_23 errrata
On Sun, 20 May 2007 21:55:58 -0700, David Brownell [EMAIL PROTECTED] wrote: If you're going to turn on the OHCI hardware, do it the normal way; don't bypass its driver. How exactly do you suggest we do this? The problem is not quite dissimilar to that of BIOS handoff. There, we solved the issue by having a compilation unit (pci-quirks.c) which textually belongs to the driver, yet is built into kernel. What I proposed is more like how the handoff quirk worked before. But if you know an sane alternative, I'll be happy (ok, Mark may be happier). -- Pete - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
