Hi,

Boris Brezillon <boris.brezil...@free-electrons.com> writes:
> Hi Douglas,
>
> On Sat, 14 Nov 2015 14:15:30 -0500
> Douglas Gilbert <dgilb...@interlog.com> wrote:
>
>> On 15-11-12 05:18 PM, Alan Stern wrote:
>> > On Thu, 12 Nov 2015, Douglas Gilbert wrote:
>> >
>> >> Yes, the X201 has USB 2.0 host ports. It is running a stock Ubuntu
>> >> 15.10 kernel: 4.2.0-18-generic and the log indicates that the
>> >> ehci_pci driver is being used. Part of the X201's syslog is
>> >> attached in which a driver complains about the invalid maxpacket
>> >> values of 64.
>> >>
>> >> So its seems that the ehci drivers as used on the X201 can work
>> >> around the invalid maxpacket value (64) while the xhci drivers
>> >> used by the X240 (due to the USB 3.0 host ports) get tripped up.
>> >
>> > Yes, I think that's right.  The restriction that high speed bulk
>> > endpoints must have a maxpacket size of 512 is enforced by the xHCI
>> > hardware but not by the EHCI hardware; this explains why ehci-hcd is
>> > able to work around such violations while xhci-hcd isn't.
>> >
>> >> Still looking at drivers/usb/gadget/udc/atmel_usba_udc.c which
>> >> has lots of changes between lk 3.19.0-rc4 and 4.0.0-rc4 . The
>> >> maxpacket value seems (to me) to be related to the fifo-size
>> >> in the gadget section of this dts include file:
>> >>     arch/arm/boot/dts/at91sam9x5.dtsi
>> >> which has 1024 for ep1 through ep5 and 64 for ep0.
>> >
>> > The assignment of endpoints isn't done in the UDC driver; it is carried
>> > out by epautoconf.c in drivers/usb/gadget/.  So you may need to expand
>> > your bisection search beyond the single UDC driver source file.
>> >
>> > Have you tried enabling debugging in the gadget drivers and checking
>> > out the kernel log on the gadget?
>> >
>> >> So it looks like 1.5 bugs:
>> >>     - one in atmel's udc driver for the at91sam9x5 family, and
>> >>     - the inconsistency between the ehci driver working around
>> >>       invalid maxpacket values and the xhci driver behaving
>> >>       badly (lots of bus resets and a badly made SCSI storage
>> >>       device [e.g. INQUIRY works but READ(10) fails]).
>> >
>> > The first is clearly a bug, although at the moment we can't be sure
>> > where.  The second is an unavoidable hardware restriction, not a bug.
>> > Anyway, if you fix the first problem then the second won't be an issue.
>> 
>> Found the udc driver bug. A shadow register value was introduced
>> around lk 4.0 for the Atmel 9x5/sama5d3 UDPHS driver
>> (atmel_usba_udc.c) for the interrupt status register. It used the
>> interrupt enable register (last written) value as a mask. At least
>> for the at91sam9g25 that works apart from the SPEED bit (bit 0)
>> which is only present in the interrupt status register.
>> 
>> It seems that USB negotiates the link speed during resets and at
>> the G25 end, even though the hardware had negotiated a "high
>> speed" link with the host, the logic in usba_udc_irq() deduced it
>> was only a full speed link (due to the above bug). Thereafter
>> there was confusion which the ehci_hcd host driver could handle
>> but the xhci_pci driver could not. In the xhci_pci case there
>> were multiple high speed link resets in the host log, matched
>> at the device (G25) end with a similar number of reported _full_
>> speed resets.
>> 
>> The author of the changes to the code that caused this is
>> cc-ed on this post. He might like to consider the attached
>> patch which fixed my problem. However the shadow mask register
>> technique might have other subtle issues that I'm not
>> qualified to address.
>
> Looks good to me, and sorry for the inconvenience.
>
>> 
>> If I don't hear anything on this issue then I can produce
>> a patch. Does it go through the ARM or USB (or both) trees?
>
> You can go ahead and send a patch to the ARM and USB MLs (+
> appropriate maintainers), unless you want me to do it.
>
>> 
>> If my patch is sufficient, then perhaps it should also be
>> issued against the lk 4.0, 4.1, 4.2 and 4.3 kernels that are
>> still actively maintained.
>
> Yep, adding the following line after your SoB should do the trick:
>
> Cc: <sta...@vger.kernel.org> #4.0+

please send this out as a real patch, otherwise I can't apply.

-- 
balbi

Attachment: signature.asc
Description: PGP signature

Reply via email to