From eda332cbf82ed0c21baff30990577e6b53c4f709 Mon Sep 17 00:00:00 2001 From: Russ Butler Date: Mon, 15 Jan 2018 21:09:31 -0600 Subject: [PATCH 1/2] Fix LPC17XX and LPC40XX USB race condition If a SETUP packet arrives shortly after an IN then the packets will be processed in the wrong order - SETUP first then IN. This causes the subsequent control transfer to fail. Fix this problem by processing IN packets before processing SETUP packets. --- .../targets/TARGET_NXP/USBHAL_LPC17.cpp | 25 ++++++++++++++----- .../targets/TARGET_NXP/USBHAL_LPC40.cpp | 25 ++++++++++++++----- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC17.cpp b/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC17.cpp index 61dc87e0aa..5d516f5304 100644 --- a/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC17.cpp +++ b/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC17.cpp @@ -590,6 +590,25 @@ void USBHAL::usbisr(void) { if (LPC_USB->USBDevIntSt & EP_SLOW) { // (Slow) Endpoint Interrupt + // Process IN packets before SETUP packets + // Note - order of OUT and SETUP does not matter as OUT packets + // are clobbered by SETUP packets and thus ignored. + // + // A SETUP packet can arrive at any time where as an IN packet is + // only sent after calling EP0write and an OUT packet after EP0read. + // The functions EP0write and EP0read are called only in response to + // a setup packet or IN/OUT packets sent in response to that + // setup packet. Therefore, if an IN or OUT packet is pending + // at the same time as a SETUP packet, the IN or OUT packet belongs + // to the previous control transfer and should either be processed + // before the SETUP packet (in the case of IN) or dropped (in the + // case of OUT as SETUP clobbers the OUT data). + if (LPC_USB->USBEpIntSt & EP(EP0IN)) { + selectEndpointClearInterrupt(EP0IN); + LPC_USB->USBDevIntClr = EP_SLOW; + EP0in(); + } + // Process each endpoint interrupt if (LPC_USB->USBEpIntSt & EP(EP0OUT)) { if (selectEndpointClearInterrupt(EP0OUT) & SIE_SE_STP) { @@ -601,12 +620,6 @@ void USBHAL::usbisr(void) { LPC_USB->USBDevIntClr = EP_SLOW; } - if (LPC_USB->USBEpIntSt & EP(EP0IN)) { - selectEndpointClearInterrupt(EP0IN); - LPC_USB->USBDevIntClr = EP_SLOW; - EP0in(); - } - for (uint8_t num = 2; num < 16*2; num++) { if (LPC_USB->USBEpIntSt & EP(num)) { selectEndpointClearInterrupt(num); diff --git a/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC40.cpp b/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC40.cpp index a5d7b44401..7147366239 100644 --- a/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC40.cpp +++ b/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC40.cpp @@ -595,6 +595,25 @@ void USBHAL::usbisr(void) { if (LPC_USB->DevIntSt & EP_SLOW) { // (Slow) Endpoint Interrupt + // Process IN packets before SETUP packets + // Note - order of OUT and SETUP does not matter as OUT packets + // are clobbered by SETUP packets and thus ignored. + // + // A SETUP packet can arrive at any time where as an IN packet is + // only sent after calling EP0write and an OUT packet after EP0read. + // The functions EP0write and EP0read are called only in response to + // a setup packet or IN/OUT packets sent in response to that + // setup packet. Therefore, if an IN or OUT packet is pending + // at the same time as a SETUP packet, the IN or OUT packet belongs + // to the previous control transfer and should either be processed + // before the SETUP packet (in the case of IN) or dropped (in the + // case of OUT as SETUP clobbers the OUT data). + if (LPC_USB->EpIntSt & EP(EP0IN)) { + selectEndpointClearInterrupt(EP0IN); + LPC_USB->DevIntClr = EP_SLOW; + EP0in(); + } + // Process each endpoint interrupt if (LPC_USB->EpIntSt & EP(EP0OUT)) { if (selectEndpointClearInterrupt(EP0OUT) & SIE_SE_STP) { @@ -606,12 +625,6 @@ void USBHAL::usbisr(void) { LPC_USB->DevIntClr = EP_SLOW; } - if (LPC_USB->EpIntSt & EP(EP0IN)) { - selectEndpointClearInterrupt(EP0IN); - LPC_USB->DevIntClr = EP_SLOW; - EP0in(); - } - for (uint8_t num = 2; num < 16*2; num++) { if (LPC_USB->EpIntSt & EP(num)) { selectEndpointClearInterrupt(num); From 6fe0aa0074fbb5c1522fe311548a625b92e4a31c Mon Sep 17 00:00:00 2001 From: Russ Butler Date: Mon, 15 Jan 2018 21:18:32 -0600 Subject: [PATCH 2/2] Fix LPC17XX and LPC40XX USB data throttling Only clear the USB read buffer when endpointRead is called. This allows data to be read with endpointReadResult without also allowing USB to transfer more data. Instead additional data is transferred explicitly with a call to endpointRead. --- .../USBDevice/targets/TARGET_NXP/USBHAL_LPC17.cpp | 12 ++++++------ .../USBDevice/targets/TARGET_NXP/USBHAL_LPC40.cpp | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC17.cpp b/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC17.cpp index 5d516f5304..6c047e8363 100644 --- a/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC17.cpp +++ b/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC17.cpp @@ -275,11 +275,6 @@ uint32_t USBHAL::endpointReadcore(uint8_t endpoint, uint8_t *buffer) { LPC_USB->USBCtrl = 0; - if ((endpoint >> 1) % 3 || (endpoint >> 1) == 0) { - SIEselectEndpoint(endpoint); - SIEclearBuffer(); - } - return size; } @@ -431,7 +426,7 @@ void USBHAL::EP0setup(uint8_t *buffer) { } void USBHAL::EP0read(void) { - // Not required + endpointRead(EP0OUT, MAX_PACKET_SIZE_EP0); } void USBHAL::EP0readStage(void) { @@ -456,6 +451,11 @@ void USBHAL::EP0stall(void) { } EP_STATUS USBHAL::endpointRead(uint8_t endpoint, uint32_t maximumSize) { + // Don't clear isochronous endpoints + if ((endpoint >> 1) % 3 || (endpoint >> 1) == 0) { + SIEselectEndpoint(endpoint); + SIEclearBuffer(); + } return EP_PENDING; } diff --git a/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC40.cpp b/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC40.cpp index 7147366239..248dd63b19 100644 --- a/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC40.cpp +++ b/features/unsupported/USBDevice/targets/TARGET_NXP/USBHAL_LPC40.cpp @@ -276,11 +276,6 @@ uint32_t USBHAL::endpointReadcore(uint8_t endpoint, uint8_t *buffer) { LPC_USB->Ctrl = 0; - if ((endpoint >> 1) % 3 || (endpoint >> 1) == 0) { - SIEselectEndpoint(endpoint); - SIEclearBuffer(); - } - return size; } @@ -436,7 +431,7 @@ void USBHAL::EP0setup(uint8_t *buffer) { } void USBHAL::EP0read(void) { - // Not required + endpointRead(EP0OUT, MAX_PACKET_SIZE_EP0); } void USBHAL::EP0readStage(void) { @@ -461,6 +456,11 @@ void USBHAL::EP0stall(void) { } EP_STATUS USBHAL::endpointRead(uint8_t endpoint, uint32_t maximumSize) { + // Don't clear isochronous endpoints + if ((endpoint >> 1) % 3 || (endpoint >> 1) == 0) { + SIEselectEndpoint(endpoint); + SIEclearBuffer(); + } return EP_PENDING; }