[psas-software] Fwd: [linux-usb-devel] [PATCH/RFC] EHCI and OHCI big endian registers only support

Sarah A Bailey saharabeara at gmail.com
Thu Nov 9 10:13:13 PST 2006


I was browsing around the USB mailing list when I found a reference to
the flight computer chip.  We might have some USB issues with it. :(

Sarah

---------- Forwarded message ----------
From: Benjamin Herrenschmidt <benh at kernel.crashing.org>
Date: Nov 6, 2006 12:29 PM
Subject: Re: [linux-usb-devel] [PATCH/RFC] EHCI and OHCI big endian
registers only support
To: David Brownell <david-b at pacbell.net>
Cc: Greg KH <greg at kroah.com>, Alan Stern <stern at rowland.harvard.edu>,
linux-usb-devel at lists.sourceforge.net, Kou Ishizaki
<kou.ishizaki at toshiba.co.jp>


On Mon, 2006-11-06 at 10:58 -0800, David Brownell wrote:
> On Saturday 04 November 2006 10:42 pm, Benjamin Herrenschmidt wrote:
>
> > The issue is that those have an EHCI/OHCI pair which has big endian
> > registers but little manipulates DMA data structures in little endian
> > form (some would say they are only half broken :-)
>
> I don't actually understand why chip vendors do this kind of stuff:
> needlessly byteswapping PCI reads/writes.  Just to annoy people who
> want to use portable drivers?

I don't understand neither :-( If I could get them to fix the HW,
beleive me, I would have done it a long time ago.

> > The OHCI change is trivial.
>
> ... since the driver already has support for this type of chip braindamage.
> Well, _similar_ braindamage.

Yes.

> > The EHCI change is more invasive as EHCI
> > didn't have any support for big-endian cells so far, but it's also
> > pretty much on the trivial side of things, mostly replacing readl/writel
> > with ehci_readl/ehci_writel.
>
> Right.  In general I have no issues with this, except the OHCI bit noted
> below.  It's basically following the model we used for similar OHCI issues,
> and the OHCI issue relates to some cleanup I've not yet pushed.
>
> But you'd make Andrew Morton a bit happier if you were switching
>
>       readl (...)
> to
>       ehci_readl(ehci, ...)
>
> That is, remove whitespace-for-readability in the "new" code.

Hrm... you want me to remove the whitespace ? Or do you want me to -not-
remove it ? :) Because, afaik, the toshiba patch does remove it, I just
noticed.

> > --- linux-cell.orig/drivers/usb/host/ehci-fsl.c     2006-10-21 16:37:03.000000000 +1000
> > +++ linux-cell/drivers/usb/host/ehci-fsl.c  2006-11-05 17:31:44.000000000 +1100
> > @@ -177,7 +177,7 @@
> >     case FSL_USB2_PHY_NONE:
> >             break;
> >     }
> > -   writel(portsc, &ehci->regs->port_status[port_offset]);
> > +   ehci_writel(ehci, portsc, &ehci->regs->port_status[port_offset]);
>
> Here's an example of a vendor which implemented EHCI in a big-endian
> environment and didn't feel compelled to add gratuitous byteswapping
> for the standard register set ... even though, as non-PCI silicon,
> it would be more natural to do it there.
>
> Just sayin', you know.  :)

And that same vendor got it wrong in a different chip (the MPC52xx) :-)

> > --- linux-cell.orig/drivers/usb/host/ohci-pci.c     2006-10-21 16:37:04.000000000 +1000
> > +++ linux-cell/drivers/usb/host/ohci-pci.c  2006-11-05 17:31:44.000000000 +1100
> > @@ -25,6 +25,22 @@
> >  {
> >     struct ohci_hcd *ohci = hcd_to_ohci (hcd);
> >
> > +#ifdef CONFIG_USB_OHCI_BIG_ENDIAN_MMIO
> > +   if (hcd->self.controller) {
> > +           struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
>
> It'd actually be a BUG() if self->controller is null, so don't
> bother to test it ...

Ok.

> > +           /* for TOSHIBA CELLEB
> > +            *
> > +            *
> > +            */
> > +           if (pdev->vendor == PCI_VENDOR_ID_TOSHIBA_2
> > +                           && pdev->device  == 0x01b6) {
> > +                   ohci->flags |= OHCI_BIG_ENDIAN_MMIO;
> > +                   ohci_dbg (ohci,
> > +                           "enabled OHCI_BIG_ENDIAN_MMIO\n");
> > +           }
> > +   }
> > +#endif
> > +
> >     ohci_hcd_init (ohci);
> >     return ohci_init (ohci);
> >  }
>
> And that OHCI bit is the thing I'd rather not see done that way.
> Appended is a patch that shows how I'd like to start handling new
> OHCI quirks.  It's not quite ready to merge as-is, for two reasons:

Ok.

>   - ISTR it doesn't apply to the latest kernels;
>
>   - That _different_ Toshiba-specific quirk (ahem) isn't resolved
>     by the patch, for some reason yet tbd.
>
> So, instead of adding more if/else/... please start building this
> kind of table-based infrastructure.  The other quirks can start
> to switch over to this approach later.

I'll give that a go.

> Add a more formal quirk infrastructure to the OHCI driver, using pci_device_id
> and associated utilities.
>
> Use it for a quirk specific to the Portege 4000, listed in osdl buzilla as
> http://bugzilla.kernel.org/show_bug.cgi?id=6723 ... the other quirks can be
> moved over to use this infrastructure later.

Thanks !

Ben.



-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
linux-usb-devel at lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel




More information about the psas-avionics mailing list