]> Zhao Yanbai Git Server - minix.git/commitdiff
Generalization of USB transfer's error handling
authorWojciech Zajac <wzajac@jpembedded.eu>
Tue, 17 Jun 2014 10:12:11 +0000 (12:12 +0200)
committerLionel Sambuc <lionel@minix3.org>
Mon, 28 Jul 2014 15:05:53 +0000 (17:05 +0200)
No longer assumes EP0 for control transfer, in general HCD code.

Some additional cleanup.

drivers/usbd/README.txt
drivers/usbd/hcd/hcd.c
drivers/usbd/hcd/musb/musb_core.c
drivers/usbd/hcd/musb/musb_core.h
drivers/usbd/include/usb/hcd_interface.h

index 25fd63da1a3f22b3bd03469f59764567afcdcd5b..de58a3e86c7d51adb5b64f180a6e963f96dfbf40 100755 (executable)
@@ -16,4 +16,4 @@ created march-june 2014, JPEmbedded (info@jpembedded.eu)
   formatting for it, is therefore hardcoded into USBD
 - Waiting for USB0 clock to leave IDLEST.Disable state, by nanosleep, was
   removed, as this should be implemented for all clocks in clkconf_set
-- Control transfers can only be performed with EP0
\ No newline at end of file
+- Control transfers can only be performed with EP0
index 7a072b7147c76af19e69f72cfa859f1479bf606a..934e2e2e816f3a67891628e2aa378ae841a68988 100755 (executable)
@@ -673,6 +673,7 @@ hcd_setup_packet(hcd_device_state * this_device, hcd_ctrlrequest * setup,
        /* Check response */
        if (EXIT_SUCCESS != d->check_error(d->private_data,
                                        HCD_TRANSFER_CONTROL,
+                                       ep,
                                        HCD_DIRECTION_UNUSED))
                return EXIT_FAILURE;
 
@@ -696,6 +697,7 @@ hcd_setup_packet(hcd_device_state * this_device, hcd_ctrlrequest * setup,
                                if (EXIT_SUCCESS != d->check_error(
                                                        d->private_data,
                                                        HCD_TRANSFER_CONTROL,
+                                                       ep,
                                                        HCD_DIRECTION_UNUSED))
                                        return EXIT_FAILURE;
 
@@ -746,6 +748,7 @@ hcd_setup_packet(hcd_device_state * this_device, hcd_ctrlrequest * setup,
                /* Check response */
                if (EXIT_SUCCESS != d->check_error(d->private_data,
                                                HCD_TRANSFER_CONTROL,
+                                               ep,
                                                HCD_DIRECTION_UNUSED))
                        return EXIT_FAILURE;
 
@@ -760,6 +763,7 @@ hcd_setup_packet(hcd_device_state * this_device, hcd_ctrlrequest * setup,
                /* Check response */
                if (EXIT_SUCCESS != d->check_error(d->private_data,
                                                HCD_TRANSFER_CONTROL,
+                                               ep,
                                                HCD_DIRECTION_UNUSED))
                        return EXIT_FAILURE;
 
@@ -851,6 +855,7 @@ hcd_data_transfer(hcd_device_state * this_device, hcd_datarequest * request)
                        /* Check response */
                        if (EXIT_SUCCESS != d->check_error(d->private_data,
                                                        request->type,
+                                                       request->endpoint,
                                                        HCD_DIRECTION_IN))
                                return EXIT_FAILURE;
 
@@ -898,6 +903,7 @@ hcd_data_transfer(hcd_device_state * this_device, hcd_datarequest * request)
                        /* Check response */
                        if (EXIT_SUCCESS != d->check_error(d->private_data,
                                                        request->type,
+                                                       request->endpoint,
                                                        HCD_DIRECTION_OUT))
                                return EXIT_FAILURE;
 
index 20c2905daf6a077e013a0056ae6d7acd889d5b4d..cc05066fa3586489514b0a6eab71c41e9026a1af 100755 (executable)
@@ -235,7 +235,7 @@ musb_read_fifo(void * cfg, void * output, int size, hcd_reg1 fifo_num)
 
 
 /*===========================================================================*
- *    musb_write_fifo                                                         *
+ *    musb_write_fifo                                                        *
  *===========================================================================*/
 static void
 musb_write_fifo(void * cfg, void * input, int size, hcd_reg1 fifo_num)
@@ -524,9 +524,9 @@ musb_rx_stage(void * cfg, hcd_datarequest * request)
 #endif
 
        /* TODO: One reusable FIFO, no double buffering */
-       /* TODO: With this, only one device can work at a time but it
-        * may be impossible to have MUSB work reasonably with multiple
-        * EP interrupts anyway */
+       /* TODO: With this, only one device/EP can work at a time. Should this
+        * be changed, hcd_device_wait/hcd_device_continue calls must be fixed
+        * accordingly to allow handling multiple EP interrupts */
        /* Assign FIFO */
        HCD_WR2(r, MUSB_REG_RXFIFOADDR, MUSB_VAL_XXFIFOADDR_EP0_END);
        HCD_WR1(r, MUSB_REG_RXFIFOSZ, MUSB_VAL_XXFIFOSZ_4096);
@@ -618,9 +618,9 @@ musb_tx_stage(void * cfg, hcd_datarequest * request)
 #endif
 
        /* TODO: One reusable FIFO, no double buffering */
-       /* TODO: With this, only one device can work at a time but it
-        * may be impossible to have MUSB work reasonably with multiple
-        * EP interrupts anyway */
+       /* TODO: With this, only one device/EP can work at a time. Should this
+        * be changed, hcd_device_wait/hcd_device_continue calls must be fixed
+        * accordingly to allow handling multiple EP interrupts */
        /* Assign FIFO */
        HCD_WR2(r, MUSB_REG_TXFIFOADDR, MUSB_VAL_XXFIFOADDR_EP0_END);
        HCD_WR1(r, MUSB_REG_TXFIFOSZ, MUSB_VAL_XXFIFOSZ_4096);
@@ -781,15 +781,26 @@ musb_read_data(void * cfg, hcd_reg1 * buffer, hcd_reg1 ep_num)
  *    musb_check_error                                                       *
  *===========================================================================*/
 int
-musb_check_error(void * cfg, hcd_transfer transfer, hcd_direction dir)
+musb_check_error(void * cfg, hcd_transfer xfer, hcd_reg1 ep, hcd_direction dir)
 {
+       /* Possible error handling schemes for MUSB */
+       typedef enum {
+
+               MUSB_INVALID_ERROR_CASE,
+               MUSB_EP0_ERROR_CASE,
+               MUSB_OUT_ERROR_CASE,
+               MUSB_IN_ERROR_CASE,
+       }
+       musb_error_case;
+
        void * r;
        hcd_reg2 host_csr;
+       musb_error_case error_case;
 
        DEBUG_DUMP;
 
        /* TODO: ISO transfer */
-       USB_ASSERT(HCD_TRANSFER_ISOCHRONOUS != transfer,
+       USB_ASSERT(HCD_TRANSFER_ISOCHRONOUS != xfer,
                "ISO transfer not supported");
 
        r = ((musb_core_config *)cfg)->regs;
@@ -797,9 +808,28 @@ musb_check_error(void * cfg, hcd_transfer transfer, hcd_direction dir)
        /* Set EP and device address to be used in this command */
        musb_set_state((musb_core_config *)cfg);
 
-       /* TODO: More than one control EP? */
-       /* In MUSB EP0 has it's own registers for error handling */
-       if (HCD_TRANSFER_CONTROL == transfer) {
+       /* Resolve which error needs to be checked
+        * so we can use proper MUSB registers */
+       error_case = MUSB_INVALID_ERROR_CASE;
+
+       if (HCD_DEFAULT_EP == ep) {
+               if (HCD_TRANSFER_CONTROL == xfer)
+                       /* EP0, control, any direction */
+                       error_case = MUSB_EP0_ERROR_CASE;
+       } else {
+               if (HCD_TRANSFER_CONTROL != xfer) {
+                       if (HCD_DIRECTION_OUT == dir)
+                               /* EP>0, non-control, out */
+                               error_case = MUSB_OUT_ERROR_CASE;
+                       else if (HCD_DIRECTION_IN == dir)
+                               /* EP>0, non-control, in */
+                               error_case = MUSB_IN_ERROR_CASE;
+               }
+       }
+
+       /* In MUSB, EP0 has it's own registers for error handling and it also
+        * seems to be the only way to handle errors for control transfers */
+       if (MUSB_EP0_ERROR_CASE == error_case) {
                /* Get control status register */
                host_csr = HCD_RD2(r, MUSB_REG_HOST_CSR0);
 
@@ -828,10 +858,8 @@ musb_check_error(void * cfg, hcd_transfer transfer, hcd_direction dir)
                return EXIT_SUCCESS;
        }
 
-       /* Non-control transfer error check,
-        * is based on transfer direction */
-       if (HCD_DIRECTION_OUT == dir) {
-               /* Get RX status register */
+       if (MUSB_OUT_ERROR_CASE == error_case) {
+               /* Get TX status register */
                host_csr = HCD_RD2(r, MUSB_REG_HOST_TXCSR);
 
                /* Check for common errors */
@@ -859,7 +887,7 @@ musb_check_error(void * cfg, hcd_transfer transfer, hcd_direction dir)
                return EXIT_SUCCESS;
        }
 
-       if (HCD_DIRECTION_IN == dir) {
+       if (MUSB_IN_ERROR_CASE == error_case) {
                /* Get RX status register */
                host_csr = HCD_RD2(r, MUSB_REG_HOST_RXCSR);
 
@@ -888,6 +916,7 @@ musb_check_error(void * cfg, hcd_transfer transfer, hcd_direction dir)
                return EXIT_SUCCESS;
        }
 
-       USB_MSG("Invalid USB transfer 0x%X:0x%X", (int)transfer, (int)dir);
+       USB_MSG("Invalid USB transfer error check: 0x%X, 0x%X, 0x%X",
+               (int)xfer, (int)ep, (int)dir);
        return EXIT_FAILURE;
 }
index 899f6eea9b1bf9eca05c89837726906fa6092abf..48f5d0bdba43741c1e2b03ac71dd104018bdaf95 100755 (executable)
@@ -53,7 +53,7 @@ void musb_out_data_stage(void *);
 void musb_in_status_stage(void *);
 void musb_out_status_stage(void *);
 int musb_read_data(void *, hcd_reg1 *, hcd_reg1);
-int musb_check_error(void *, hcd_transfer, hcd_direction);
+int musb_check_error(void *, hcd_transfer, hcd_reg1, hcd_direction);
 
 
 #endif /* !_MUSB_CORE_H_ */
index 332f2819c467bce5a0751d20bde4f30d884a12a2..66a92665b0c59c856a5a8f4e5249722a08fbdf64 100755 (executable)
@@ -33,7 +33,8 @@ struct hcd_driver_state {
        void    (*in_status_stage)      (void *);
        void    (*out_status_stage)     (void *);
        int     (*read_data)            (void *, hcd_reg1 *, hcd_reg1);
-       int     (*check_error)          (void *, hcd_transfer, hcd_direction);
+       int     (*check_error)          (void *, hcd_transfer, hcd_reg1,
+                                       hcd_direction);
 
        /* Controller's private data (like mapped registers) */
        void *          private_data;