From c1ba580ae2214427db92900c6759337d7e062482 Mon Sep 17 00:00:00 2001 From: Wojciech Zajac Date: Tue, 17 Jun 2014 12:12:11 +0200 Subject: [PATCH] Generalization of USB transfer's error handling No longer assumes EP0 for control transfer, in general HCD code. Some additional cleanup. --- drivers/usbd/README.txt | 2 +- drivers/usbd/hcd/hcd.c | 6 +++ drivers/usbd/hcd/musb/musb_core.c | 65 +++++++++++++++++------- drivers/usbd/hcd/musb/musb_core.h | 2 +- drivers/usbd/include/usb/hcd_interface.h | 3 +- 5 files changed, 57 insertions(+), 21 deletions(-) diff --git a/drivers/usbd/README.txt b/drivers/usbd/README.txt index 25fd63da1..de58a3e86 100755 --- a/drivers/usbd/README.txt +++ b/drivers/usbd/README.txt @@ -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 diff --git a/drivers/usbd/hcd/hcd.c b/drivers/usbd/hcd/hcd.c index 7a072b714..934e2e2e8 100755 --- a/drivers/usbd/hcd/hcd.c +++ b/drivers/usbd/hcd/hcd.c @@ -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; diff --git a/drivers/usbd/hcd/musb/musb_core.c b/drivers/usbd/hcd/musb/musb_core.c index 20c2905da..cc05066fa 100755 --- a/drivers/usbd/hcd/musb/musb_core.c +++ b/drivers/usbd/hcd/musb/musb_core.c @@ -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; } diff --git a/drivers/usbd/hcd/musb/musb_core.h b/drivers/usbd/hcd/musb/musb_core.h index 899f6eea9..48f5d0bdb 100755 --- a/drivers/usbd/hcd/musb/musb_core.h +++ b/drivers/usbd/hcd/musb/musb_core.h @@ -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_ */ diff --git a/drivers/usbd/include/usb/hcd_interface.h b/drivers/usbd/include/usb/hcd_interface.h index 332f2819c..66a92665b 100755 --- a/drivers/usbd/include/usb/hcd_interface.h +++ b/drivers/usbd/include/usb/hcd_interface.h @@ -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; -- 2.44.0