Re: [PATCH] [RFC] USB: EHCI: add support for the EV-OXU210-PCI board

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 23, 2010 at 01:08:43PM -0400, Alan Stern wrote:
> On Wed, 23 Jun 2010, Andrea Righi wrote:
> 
> > ffff88007a862cc0 2028329666 S Bi:2:002:1 -115 4096 <
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > device stalls here (in this case with a large URB)...
> > 
> > ffff88007a862cc0 2058694464 C Bi:2:002:1 -104 0
> > 
> > 
> > > I'm about out of ideas.  You could look at the qh status values in the
> > > "async" debugging file while waiting for the URB to time out, but they
> > > probably won't tell you anything new.
> > 
> > for this last case:
> > 
> > $ cat /sys/kernel/debug/usb/ehci/oxu210hp_sph/async
> > qh/ffff88007b051800 dev2 hs ep1 42002102 40000000 (90008d80* data1 nak0)
> > 	ffffc90005001120*in len=4096 10008d80 urb ffff88007a862cc0
> 
> Nothing stands out as significant.  The values indicate that the 
> controller hasn't received any data from the device, but that's 
> probably not true in reality.

I did many tests in these past days, unfortunately without any
significant result. :(

The only "bad" thing I've found is that the communication via
usb_stor_bulk_transfer_sglist() seems to fail only with "large"
transfers (even 192 is sufficient to trigger the problem, while small
transfers seems to work fine):

$ dmesg | grep "Status code\|bulk_transfer"
[ 3199.220552] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.220761] usb-storage: Status code 0; transferred 31/31
[ 3199.220767] usb-storage: usb_stor_bulk_transfer_sglist: xfer 36 bytes, 1 entries
[ 3199.257161] usb-storage: Status code 0; transferred 36/36
[ 3199.257168] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.257380] usb-storage: Status code 0; transferred 13/13
[ 3199.269690] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.269896] usb-storage: Status code 0; transferred 31/31
[ 3199.269903] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.270141] usb-storage: Status code 0; transferred 13/13
[ 3199.270173] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.270390] usb-storage: Status code 0; transferred 31/31
[ 3199.270396] usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
[ 3199.647916] usb-storage: Status code 0; transferred 18/18
[ 3199.647924] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.648143] usb-storage: Status code 0; transferred 13/13
[ 3199.648223] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.648393] usb-storage: Status code 0; transferred 31/31
[ 3199.648399] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.648642] usb-storage: Status code 0; transferred 13/13
[ 3199.648702] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.648892] usb-storage: Status code 0; transferred 31/31
[ 3199.648897] usb-storage: usb_stor_bulk_transfer_sglist: xfer 8 bytes, 1 entries
[ 3199.649648] usb-storage: Status code 0; transferred 8/8
[ 3199.649655] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.649891] usb-storage: Status code 0; transferred 13/13
[ 3199.649952] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.658891] usb-storage: Status code 0; transferred 31/31
[ 3199.658897] usb-storage: usb_stor_bulk_transfer_sglist: xfer 192 bytes, 1 entries
[ 3199.659233] usb-storage: Status code -121; transferred 36/192
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[ 3199.659240] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.659401] usb-storage: Status code 0; transferred 13/13
[ 3199.659544] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.659787] usb-storage: Status code 0; transferred 31/31
[ 3199.659794] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.660054] usb-storage: Status code 0; transferred 13/13
[ 3199.660914] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.661184] usb-storage: Status code 0; transferred 31/31
[ 3199.661191] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.661793] usb-storage: Status code 0; transferred 13/13
[ 3199.661824] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.662027] usb-storage: Status code 0; transferred 31/31
[ 3199.662032] usb-storage: usb_stor_bulk_transfer_sglist: xfer 18 bytes, 1 entries
[ 3199.662289] usb-storage: Status code 0; transferred 18/18
[ 3199.662296] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.662526] usb-storage: Status code 0; transferred 13/13
[ 3199.662614] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.662775] usb-storage: Status code 0; transferred 31/31
[ 3199.662782] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.663025] usb-storage: Status code 0; transferred 13/13
[ 3199.663086] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.663273] usb-storage: Status code 0; transferred 31/31
[ 3199.663279] usb-storage: usb_stor_bulk_transfer_sglist: xfer 8 bytes, 1 entries
[ 3199.664035] usb-storage: Status code 0; transferred 8/8
[ 3199.664042] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.664277] usb-storage: Status code 0; transferred 13/13
[ 3199.664338] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.664522] usb-storage: Status code 0; transferred 31/31
[ 3199.664528] usb-storage: usb_stor_bulk_transfer_sglist: xfer 192 bytes, 1 entries
[ 3199.664840] usb-storage: Status code -121; transferred 36/192
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[ 3199.664848] usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
[ 3199.665043] usb-storage: Status code 0; transferred 13/13
[ 3199.665149] usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
[ 3199.665273] usb-storage: Status code 0; transferred 31/31
[ 3199.665279] usb-storage: usb_stor_bulk_transfer_sglist: xfer 4096 bytes, 1 entries
[ 3230.081270] usb-storage: Status code -104; transferred 0/4096
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The error pattern seems quite clear looking at this particular log.

Maybe the controller is just buggy and it's not able to handle "large"
bulk transfers... I don't see a quick way to limit the size of bulk
transfers, so I'd like to do a test adding an intermediate layer to
split large bulk transfers into smaller one.

BTW I also found this issue in the current scatter-gather communication
in the EHCI layer:

---
USB: EHCI: fix NULL pointer dererence in HCDs that use HCD_LOCAL_MEM

If we use HCD_LOCAL_MEM flag and dma_declare_coherent_memory() to
enforce local memory utilization we also need to disable native
scatter-gather support, otherwise hcd_alloc_coherent() in
map_urb_for_dma() is called with urb->transfer_buffer == NULL, that
triggers a NULL pointer dereference.

We can also consider to add some BUG_ON() to better catch this problem
in the future.

[ At the moment this bug is not triggered by any driver, and probably
this is not the best way to fix it, but at least this is sufficient to
avoid annoying kernel oops for future drivers. ]

Signed-off-by: Andrea Righi <arighi@xxxxxxxxxxx>
---
 drivers/usb/core/hcd.c      |    4 ++++
 drivers/usb/host/ehci-hcd.c |    3 ++-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 12742f1..4f1d9a2 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1219,6 +1219,8 @@ static int hcd_alloc_coherent(struct usb_bus *bus,
 {
 	unsigned char *vaddr;
 
+	BUG_ON(*vaddr_handle == NULL);
+
 	vaddr = hcd_buffer_alloc(bus, size + sizeof(vaddr),
 				 mem_flags, dma_handle);
 	if (!vaddr)
@@ -1248,6 +1250,8 @@ static void hcd_free_coherent(struct usb_bus *bus, dma_addr_t *dma_handle,
 {
 	unsigned char *vaddr = *vaddr_handle;
 
+	BUG_ON(*vaddr_handle == NULL);
+
 	vaddr = (void *)get_unaligned((unsigned long *)(vaddr + size));
 
 	if (dir == DMA_FROM_DEVICE)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index a3ef2a9..27e0351 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -606,7 +606,8 @@ static int ehci_init(struct usb_hcd *hcd)
 	ehci->command = temp;
 
 	/* Accept arbitrarily long scatter-gather lists */
-	hcd->self.sg_tablesize = ~0;
+	if (!(hcd->driver->flags & HCD_LOCAL_MEM))
+		hcd->self.sg_tablesize = ~0;
 	return 0;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux