[PATCH] powerpc/usb: Fix 440EPx USBH_3 & USBH_5 EHCI errata

[- Reyneke]

> Please provide a valid email address here.

New email client helpfully stripped out the address...

>> called in a context where you can't iomap ? (ie with a spinlock held).

That is indeed the case. Keeping ehci generic is a valid point, but because of the above I don't think adding a "hook" is going to work. I'll take a look at the *platform_private option and re-post - this would also allow for better runtime checking, as you suggested.

 Cheers
  Jan




----------------------------------
> On Fri, 2009-03-06 at 11:30 +0000, - Reyneke wrote:
>> Patch applies to 440EPx devices in USB EHCI host mode (USB 2.0).
>>
>>>From the 440EPx errata:
>>
>> USBH_3: Host hangs after underrun or overrun occurs
>> USBH_5: EHCI0_INSNREGxx registers are reset by a Soft or Light Host Controller Reset
>>
>> Workround for USBH_3 is to enable Break Memory Transfer (BMT) in INSNREG3. But the controller is reset after this fix is applied, and thus the current workround is lost. The following short patch ensures INSNREG3 is correctly set after reset.
>
>> Signed-off-by: Jan Reyneke
>
> Please provide a valid email address here.
>
>> ---

>
> A few issues here. First, it would be preferable to have this in the
> ehci-ppc-of.c file. If you can't stick that in such a place that it will
> be called after ehci_reset, then maybe you can add a reset "hook" so
> that ehci-ppc-of.c gets to wrap the real ehci_reset().
>
> Also, while the ifdef CONFIG_440EPX is good to prevent building the code
> on machines that don't need it, it's also not enough. We allow building
> kernels that support multiple boards and SoC's within the same major CPU
> family and thus you -also- need runtime detection. Either using a quirk
> (I think the USB drivers have quirk flags) or just always doing the
> of_device_is_compatible() thingy which is yet another reason for finding
> a way to move that up into ehci-ppc-of.c
>
> That would also avoid some duplication...
>

>
> So if you manage to move the quirk here, you can thus re-use the
> existing code, or is the reset always called in a context where you
> can't iomap ? (ie with a spinlock held).
>
> In any case, I don't like adding a specific field to the generic ehci
> structure like that. If that's what it takes, add a void
> *platform_private to it, and use -that- to stick a host specific data
> structure, but for something not performance sensitive such as a reset,
> if you can get away with always mapping/unmapping, it's probably better.
>
> Cheers,
> Ben.
>
>

_________________________________________________________________
View your Twitter and Flickr updates from one place – Learn more!
http://clk.atdmt.com/UKM/go/137984870/direct/01/