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
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
