From 048e0448c0b8edf4cdd82f72e094b29c2f094281 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Wed, 25 Sep 2019 20:17:53 -0400 Subject: [PATCH 1/4] ST FSDEV:Remove setting the EP kind, as I think it was causing issues during enumeration (sometimes). Also move a membar. --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 00f5b123..890dffcf 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -252,7 +252,9 @@ void dcd_init (uint8_t rhport) void dcd_int_enable (uint8_t rhport) { (void)rhport; - + // Member here forces write to RAM before allowing ISR to execute + __DSB(); + __ISB(); #if CFG_TUSB_MCU == OPT_MCU_STM32F0 || CFG_TUSB_MCU == OPT_MCU_STM32L0 NVIC_EnableIRQ(USB_IRQn); #elif CFG_TUSB_MCU == OPT_MCU_STM32F3 @@ -276,10 +278,7 @@ void dcd_int_disable(uint8_t rhport) #else #error Unknown arch in USB driver #endif - // I'm not convinced that memory synchronization is completely necessary, but - // it isn't a bad idea. - __DSB(); - __ISB(); + // CMSIS has a membar after disabling interrupts } // Receive Set Address request, mcu port must also include status IN response @@ -440,10 +439,10 @@ static uint16_t dcd_ep_ctr_handler(void) } /* Process Control Data OUT status Packet*/ - if(EPindex == 0u && xfer->total_len == 0u) + /*if(EPindex == 0u && xfer->total_len == 0u) { pcd_clear_ep_kind(USB,0); // Good, so allow non-zero length packets now. - } + }*/ dcd_event_xfer_complete(0, EPindex, xfer->total_len, XFER_RESULT_SUCCESS, true); pcd_set_ep_rx_cnt(USB, EPindex, CFG_TUD_ENDPOINT0_SIZE); @@ -627,7 +626,9 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc } pcd_set_ep_address(USB, epnum, epnum); - pcd_clear_ep_kind(USB,0); // Be normal, for now, instead of only accepting zero-byte packets + // Be normal, for now, instead of only accepting zero-byte packets (on control endpoint) + // or being double-buffered (bulk endpoints) + pcd_clear_ep_kind(USB,0); if(dir == TUSB_DIR_IN) { @@ -688,7 +689,7 @@ bool dcd_edpt_xfer (uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t if (epnum == 0 && buffer == NULL) { xfer->buffer = (uint8_t*)_setup_packet; - pcd_set_ep_kind(USB,0); // Expect a zero-byte INPUT + //pcd_set_ep_kind(USB,0); // Expect a zero-byte INPUT } if(total_bytes > xfer->max_packet_size) { From f19082f02d1ff3cd9f458476fd558a7e7fabc1bf Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Wed, 25 Sep 2019 20:53:22 -0400 Subject: [PATCH 2/4] Reset TX and RX endpoints to NAK when receiving setup packet. --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 890dffcf..16bef6c3 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -418,10 +418,15 @@ static uint16_t dcd_ep_ctr_handler(void) uint8_t userMemBuf[8]; /* Get SETUP Packet*/ count = pcd_get_ep_rx_cnt(USB, EPindex); - //TU_ASSERT_ERR(count == 8); - dcd_read_packet_memory(userMemBuf, *pcd_ep_rx_address_ptr(USB,EPindex), 8); + if(count == 8) // Setup packet should always be 8 bits. If not, ignore it, and try again. + { + // Must reset EP to NAK (in case it had been stalling) (though, maybe too late here) + pcd_set_ep_rx_status(USB,0u,USB_EP_RX_NAK); + pcd_set_ep_tx_status(USB,0u,USB_EP_TX_NAK); + dcd_read_packet_memory(userMemBuf, *pcd_ep_rx_address_ptr(USB,EPindex), 8); + dcd_event_setup_received(0, (uint8_t*)userMemBuf, true); + } /* SETUP bit kept frozen while CTR_RX = 1*/ - dcd_event_setup_received(0, (uint8_t*)userMemBuf, true); pcd_clear_rx_ep_ctr(USB, EPindex); } else if ((wEPVal & USB_EP_CTR_RX) != 0U) // OUT From e5f38e3e861cb20046a81e666e8de9a7fdc3919d Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Wed, 2 Oct 2019 00:11:16 -0400 Subject: [PATCH 3/4] Remove references to EP kind. --- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 9343b6d0..1fd3e8ba 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -444,10 +444,6 @@ static uint16_t dcd_ep_ctr_handler(void) } /* Process Control Data OUT status Packet*/ - /*if(EPindex == 0u && xfer->total_len == 0u) - { - pcd_clear_ep_kind(USB,0); // Good, so allow non-zero length packets now. - }*/ dcd_event_xfer_complete(0, EPindex, xfer->total_len, XFER_RESULT_SUCCESS, true); pcd_set_ep_rx_cnt(USB, EPindex, CFG_TUD_ENDPOINT0_SIZE); @@ -694,7 +690,6 @@ bool dcd_edpt_xfer (uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t if (epnum == 0 && buffer == NULL) { xfer->buffer = (uint8_t*)_setup_packet; - //pcd_set_ep_kind(USB,0); // Expect a zero-byte INPUT } if(total_bytes > xfer->max_packet_size) { From 1e193212d7f93a55e7ec857d0ad8dc4eaea517de Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Wed, 2 Oct 2019 00:31:47 -0400 Subject: [PATCH 4/4] Add testcase for EP0 stall recovery to USBTMC test script. --- examples/device/usbtmc/visaQuery.py | 18 +++++++++++++++++- src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/examples/device/usbtmc/visaQuery.py b/examples/device/usbtmc/visaQuery.py index 44b5fd87..b5bf8d48 100644 --- a/examples/device/usbtmc/visaQuery.py +++ b/examples/device/usbtmc/visaQuery.py @@ -1,9 +1,10 @@ +#!/usr/bin/env python3 + import visa import time import sys - def test_idn(): idn = inst.query("*idn?"); assert (idn == "TinyUSB,ModelNumber,SerialNumber,FirmwareVer123456\r\n") @@ -129,6 +130,18 @@ def test_multi_read(): assert (x + "\r\n" == y) #inst.chunk_size = old_chunk_size +def test_stall_ep0(): + usb_iface = inst.get_visa_attribute(visa.constants.VI_ATTR_USB_INTFC_NUM) + inst.read_stb() + # This is an invalid request, should create stall. + try: + retv = inst.control_in(request_type_bitmap_field=0xA1, request_id=60, request_value=0x0000, index=usb_iface, length=0x0001) + assert false + except visa.VisaIOError: + pass + + assert (inst.read_stb() == 0) + rm = visa.ResourceManager("/c/Windows/system32/visa64.dll") reslist = rm.list_resources("USB?::?*::INSTR") @@ -171,6 +184,9 @@ test_echo(165,170) print("+ Read timeout (no MAV)") test_read_timeout() +print("+ Test EP0 stall recovery") +test_stall_ep0() + print("+ MAV") test_mav() diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c index 1fd3e8ba..c33f9b86 100644 --- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c +++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c @@ -418,7 +418,7 @@ static uint16_t dcd_ep_ctr_handler(void) uint8_t userMemBuf[8]; /* Get SETUP Packet*/ count = pcd_get_ep_rx_cnt(USB, EPindex); - if(count == 8) // Setup packet should always be 8 bits. If not, ignore it, and try again. + if(count == 8) // Setup packet should always be 8 bytes. If not, ignore it, and try again. { // Must reset EP to NAK (in case it had been stalling) (though, maybe too late here) pcd_set_ep_rx_status(USB,0u,USB_EP_RX_NAK);