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

Reply via email to