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.
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?
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.
Doug Gilbert
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index f0f2b06..f92f5af 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1633,7 +1633,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
spin_lock(&udc->lock);
int_enb = usba_int_enb_get(udc);
- status = usba_readl(udc, INT_STA) & int_enb;
+ status = usba_readl(udc, INT_STA) & (int_enb | USBA_HIGH_SPEED);
DBG(DBG_INT, "irq, status=%#08x\n", status);
if (status & USBA_DET_SUSPEND) {
