nuttx icon indicating copy to clipboard operation
nuttx copied to clipboard

EHCI: *_cancel() failed to free qH and qTD

Open taikoyaP opened this issue 3 years ago • 2 comments

I use SAMA5D2 EHCI host driver with USB CDC-ACM device (AMTelecom AML574). Calling open() and read() for device (/dev/ttyACM0) works good. But when calling close(), DRVR_TRANSFER(for bulkin EP) in reading work queue job (usbhost_rxdata_work()) does not return, so when calling next open(), newly data is not came. So I add calling DRVR_CANCEL(for bulkin EP) in usbhost_shutdown().

When I repeated open() -> read() -> close() for device, USB error occurred.

(calling open())
[   32.660000] usbhost_setup: Entry
[   32.660000] sam_ctrlin: RHPort1 type: 21 req: 22 value: 0003 index: 0000 len: 0000
[   32.660000] sam_async_setup: RHport1 EP0: buffer=0, buflen=0, req=0x2006ce60
[   32.660000] OHCI Interrupts pending: 000000
[   32.660000] sam_ehci_tophalf: USBSTS: 0000c009 USBINTR: 00000037
[   32.660000] EHCI USB Interrupt (USBINT) Interrupt: 000001
[   32.660000] EHCI IOC EP0 TOKEN=8d00
[   32.660000] EHCI IOC EP1 TOKEN=8d80
[   32.660000] EHCI IOC EP1 TOKEN=8d80
[   32.660000] EHCI IOC EP2 TOKEN=8d80
[   32.660000] usbhost_ioctl: Entry
[   32.660000] sam_async_setup: RHport1 EP1: buffer=0x2006d000, buflen=512, req=0
[   32.660000] sam_async_setup: RHport1 EP1: buffer=0x2006d220, buflen=4, req=0
[   32.660000] EHCI ERROR: Failed to allocate a QH
[   32.660000] EHCI ERROR: sam_qh_create failed
[   32.660000] sam_transfer: ERROR: Transfer setup failed: -12
[   32.660000] usbhost_txdata_work: ERROR: DRVR_TRANSFER for packet failed: -12

I read sam_echi.c, and found two problems. But all other ECHI host drivers contains same code, so it may be a mistake of me. I want to confirm this.

Problem 1

In sam_cancel(),

/* If the asynchronous queue is empty, then the forward point in
 * the asynchronous queue head will point back to the queue
 * head.
 */

if (qh && qh != &g_asynchead)
  {
    /* Claim that we successfully cancelled the transfer */
    ret = OK;
    goto exit_terminate;
  }

....

  /* Find and remove the QH. */

  ret = sam_qh_foreach(qh, &bp, sam_qh_cancel, epinfo);

....
exit_terminate:
  epinfo->result = -ESHUTDOWN;

It seems to check queue is empty, so it must be if (qh && qh == &g_asynchead). Is that right?

(In sam_ioc_bottomhalf(), it seems to check queue is not empty.)

/* If the asynchronous queue is empty, then the forward point in the
 * asynchronous queue head will point back to the queue head.
 */

if (qh && qh != &g_asynchead)
  {
    /* Then traverse and operate on every QH and qTD in the asynchronous
     * queue
     */

    ret = sam_qh_foreach(qh, &bp, sam_qh_ioccheck, NULL);
  }

Problem 2

In sam_qh_cancel(),

/* Check if this is the QH that we are looking for */

if (qh->epinfo == epinfo)
  {
    /* No... keep looking */
    return OK;
  }

(calling sam_qtd_cancel() and sam_qh_free())

This code seems that when qh->epinfo is epinfo (looking for), qtd and qh is not freed. So it must be if (qh->epinfo != epinfo). Is that right?

Thanks.

taikoyaP avatar Jun 20 '22 00:06 taikoyaP

Yes, I think you are right! @pkarashchenko did you test USB on SAM MCUs?

acassis avatar Jun 23 '22 22:06 acassis

Unfortunately no. I've been working with SAMe70 based board that does not have USB interface

pkarashchenko avatar Jun 26 '22 11:06 pkarashchenko