[psas-software] Fwd: [linux-usb-devel] [PATCH/RFC] EHCI and OHCI big endian registers only support
Barton C Massey
bart
Thu Nov 9 13:00:30 PST 2006
Doesn't look too horrible to fix, anyhow---thanks much for
catching this!
Bart
In message <4c8f91e80611091013h3dab5376mfc09d6b15b2293be at mail.gmail.com> you wrote:
> 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
>
> _______________________________________________
> psas-software mailing list
> psas-software at lists.psas.pdx.edu
> http://lists.psas.pdx.edu/mailman/listinfo/psas-software
More information about the psas-avionics
mailing list