taikoyaP opened a new issue, #6483:
URL: https://github.com/apache/incubator-nuttx/issues/6483

   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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to