From 6ab58fe93d47d822b351aa33b50a9b28a60ac67b Mon Sep 17 00:00:00 2001 From: Eric Price Date: Fri, 12 Oct 2018 15:54:58 +0200 Subject: [PATCH 1/8] LP-602 bugfix in STM32_USB_OTG_Driver to prevent IRQ hammering on USB replug --- .../stm32f4xx/libraries/STM32_USB_OTG_Driver/src/usb_dcd_int.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flight/pios/stm32f4xx/libraries/STM32_USB_OTG_Driver/src/usb_dcd_int.c b/flight/pios/stm32f4xx/libraries/STM32_USB_OTG_Driver/src/usb_dcd_int.c index d6e52c604..1c5e33d4a 100644 --- a/flight/pios/stm32f4xx/libraries/STM32_USB_OTG_Driver/src/usb_dcd_int.c +++ b/flight/pios/stm32f4xx/libraries/STM32_USB_OTG_Driver/src/usb_dcd_int.c @@ -681,7 +681,7 @@ static uint32_t DCD_WriteEmptyTxFifo(USB_OTG_CORE_HANDLE *pdev, uint32_t epnum) } // --- start fix uint32_t fifoemptymsk; - if (len < ep->maxpacket) + if (len <= ep->maxpacket) { // FIFO empty fifoemptymsk = 0x1 << epnum; From 3f2d89fc11746b03b525bef564369cc4fc00dcba Mon Sep 17 00:00:00 2001 From: Eric Price Date: Mon, 15 Oct 2018 10:40:45 +0200 Subject: [PATCH 2/8] LP-602 better fix for USB IRQ lockup issue --- .../stm32f4xx/libraries/STM32_USB_OTG_Driver/src/usb_dcd_int.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flight/pios/stm32f4xx/libraries/STM32_USB_OTG_Driver/src/usb_dcd_int.c b/flight/pios/stm32f4xx/libraries/STM32_USB_OTG_Driver/src/usb_dcd_int.c index 1c5e33d4a..b572b9550 100644 --- a/flight/pios/stm32f4xx/libraries/STM32_USB_OTG_Driver/src/usb_dcd_int.c +++ b/flight/pios/stm32f4xx/libraries/STM32_USB_OTG_Driver/src/usb_dcd_int.c @@ -681,7 +681,7 @@ static uint32_t DCD_WriteEmptyTxFifo(USB_OTG_CORE_HANDLE *pdev, uint32_t epnum) } // --- start fix uint32_t fifoemptymsk; - if (len <= ep->maxpacket) + if (len < ep->maxpacket || ep->xfer_count==0) { // FIFO empty fifoemptymsk = 0x1 << epnum; From ca8231797085efedef5d58ddaaac5f9959bcdb38 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Tue, 16 Oct 2018 11:10:47 +0200 Subject: [PATCH 3/8] LP-602 Bugfix in pios_usb_cdc: reset tx_start on device init to prevent stalled tx --- flight/pios/stm32f4xx/pios_usb_cdc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/flight/pios/stm32f4xx/pios_usb_cdc.c b/flight/pios/stm32f4xx/pios_usb_cdc.c index 803d653c6..2da23bc87 100644 --- a/flight/pios/stm32f4xx/pios_usb_cdc.c +++ b/flight/pios/stm32f4xx/pios_usb_cdc.c @@ -607,6 +607,7 @@ static void PIOS_USB_CDC_DATA_IF_Init(uint32_t usb_cdc_id) PIOS_USB_CDC_DATA_EP_OUT_Callback, (uint32_t)usb_cdc_dev); usb_cdc_dev->usb_data_if_enabled = true; + usb_cdc_dev->tx_active = false; } static void PIOS_USB_CDC_DATA_IF_DeInit(uint32_t usb_cdc_id) From a4e20499c0c2da6c3bd13ce8b1c36db06ee15df0 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Tue, 16 Oct 2018 11:11:30 +0200 Subject: [PATCH 4/8] LP-602 significant change to USB layer. force complete USB stack reset on replug/reset from host device --- flight/pios/stm32f4xx/pios_usb.c | 34 +++++++++------------------- flight/pios/stm32f4xx/pios_usbhook.c | 25 ++++++++++++++++---- 2 files changed, 32 insertions(+), 27 deletions(-) diff --git a/flight/pios/stm32f4xx/pios_usb.c b/flight/pios/stm32f4xx/pios_usb.c index 686120069..fb1c7b3ff 100644 --- a/flight/pios/stm32f4xx/pios_usb.c +++ b/flight/pios/stm32f4xx/pios_usb.c @@ -38,7 +38,7 @@ #include /* Rx/Tx status */ -static uint8_t transfer_possible = 0; +static volatile uint8_t transfer_possible = 0; #ifdef PIOS_INCLUDE_FREERTOS static void(*disconnection_cb_list[3]) (void); @@ -148,6 +148,9 @@ out_fail: */ int32_t PIOS_USB_ChangeConnectionState(bool connected) { +#ifdef PIOS_INCLUDE_FREERTOS + static volatile uint8_t lastStatus = 2; // 2 is "no last status" +#endif // In all cases: re-initialise USB HID driver if (connected) { transfer_possible = 1; @@ -168,6 +171,12 @@ int32_t PIOS_USB_ChangeConnectionState(bool connected) #ifdef PIOS_INCLUDE_FREERTOS raiseConnectionStateCallback(connected); + if (lastStatus != transfer_possible) { + if (lastStatus == 1) { + raiseDisconnectionCallbacks(); + } + lastStatus = transfer_possible; + } #endif return 0; @@ -187,28 +196,7 @@ bool PIOS_USB_CheckAvailable(__attribute__((unused)) uint32_t id) return false; } - usb_found = ((usb_dev->cfg->vsense.gpio->IDR & usb_dev->cfg->vsense.init.GPIO_Pin) != 0) ^ usb_dev->cfg->vsense_active_low; -// Please note that checks of transfer_possible and the reconnection handling is -// suppressed for non freertos mode (aka bootloader) as this is causing problems detecting connection and -// broken communications. -#ifdef PIOS_INCLUDE_FREERTOS - static bool lastStatus = false; - bool status = usb_found != 0 && transfer_possible ? 1 : 0; - bool reconnect = false; - if (xSemaphoreTakeFromISR(usb_dev->statusCheckSemaphore, NULL) == pdTRUE) { - reconnect = (lastStatus && !status); - lastStatus = status; - xSemaphoreGiveFromISR(usb_dev->statusCheckSemaphore, NULL); - } - if (reconnect) { - raiseDisconnectionCallbacks(); - } - return status; - -#else - return usb_found; - -#endif + return transfer_possible; } /* diff --git a/flight/pios/stm32f4xx/pios_usbhook.c b/flight/pios/stm32f4xx/pios_usbhook.c index 0d58a5e10..d587905db 100644 --- a/flight/pios/stm32f4xx/pios_usbhook.c +++ b/flight/pios/stm32f4xx/pios_usbhook.c @@ -280,7 +280,9 @@ static USBD_DEVICE device_callbacks = { static void PIOS_USBHOOK_USR_Init(void) { PIOS_USB_ChangeConnectionState(false); - reconnect(); + // reconnect dev logically on init (previously a call to reconnect()) + DCD_DevDisconnect(&pios_usb_otg_core_handle); + DCD_DevConnect(&pios_usb_otg_core_handle); } static void PIOS_USBHOOK_USR_DeviceReset(__attribute__((unused)) uint8_t speed) @@ -491,9 +493,24 @@ static USBD_Class_cb_TypeDef class_callbacks = { static void reconnect(void) { - /* Force a physical disconnect/reconnect */ - DCD_DevDisconnect(&pios_usb_otg_core_handle); - DCD_DevConnect(&pios_usb_otg_core_handle); + static volatile bool in_reconnect = false; + + /* Force a complete device reset. This can trigger a call to reconnect() so prevent recursion */ + if (!in_reconnect) { + in_reconnect = true; // save since volatile and STM32F4 is single core + // disable USB device + DCD_DevDisconnect(&pios_usb_otg_core_handle); + USBD_DeInit(&pios_usb_otg_core_handle); + USB_OTG_StopDevice(&pios_usb_otg_core_handle); + // enable USB device + USBD_Init(&pios_usb_otg_core_handle, + USB_OTG_FS_CORE_ID, + &device_callbacks, + &class_callbacks, + &user_callbacks); + + in_reconnect = false; + } } #endif /* PIOS_INCLUDE_USB */ From 868d78791228b6213924f9fc6eca3d52885fc337 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Tue, 16 Oct 2018 11:41:27 +0200 Subject: [PATCH 5/8] LP-602 Fix regression of bootloader which checks for USB availability without initialising it, so it really needs to check if the cable is present (gpio check) --- flight/pios/stm32f4xx/pios_usb.c | 12 ++++++++++++ flight/targets/boards/revolution/bootloader/main.c | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/flight/pios/stm32f4xx/pios_usb.c b/flight/pios/stm32f4xx/pios_usb.c index fb1c7b3ff..158fd8128 100644 --- a/flight/pios/stm32f4xx/pios_usb.c +++ b/flight/pios/stm32f4xx/pios_usb.c @@ -199,6 +199,18 @@ bool PIOS_USB_CheckAvailable(__attribute__((unused)) uint32_t id) return transfer_possible; } +/** + * This function returns wether a USB cable (5V pin) has been detected + * \return true: cable connected + * \return false: cable not detected (no cable or cable with no power) + */ +bool PIOS_USB_CableConnected(__attribute__((unused)) uint8_t id) +{ + struct pios_usb_dev *usb_dev = (struct pios_usb_dev *)pios_usb_id; + + return ((usb_dev->cfg->vsense.gpio->IDR & usb_dev->cfg->vsense.init.GPIO_Pin) != 0) ^ usb_dev->cfg->vsense_active_low; +} + /* * * Register a physical disconnection callback diff --git a/flight/targets/boards/revolution/bootloader/main.c b/flight/targets/boards/revolution/bootloader/main.c index aa5c7999a..c85ef675e 100644 --- a/flight/targets/boards/revolution/bootloader/main.c +++ b/flight/targets/boards/revolution/bootloader/main.c @@ -82,7 +82,7 @@ int main() // is 2.7 volts check_bor(); - USB_connected = PIOS_USB_CheckAvailable(0); + USB_connected = PIOS_USB_CableConnected(0); if (PIOS_IAP_CheckRequest() == true) { PIOS_DELAY_WaitmS(1000); From 8793aaa387a1b10500d046db5fa183acdeb9368a Mon Sep 17 00:00:00 2001 From: Eric Price Date: Tue, 16 Oct 2018 15:51:46 +0200 Subject: [PATCH 6/8] LP-602 Re-enable rx_status checking in usb_csc to optionally reenable rx after reset This partially reverts 1203fa9e665b1ed0236c7456e1b5f064e3366460 LP-495 F4 USB CDC: remove internal rx_active state tracking and use actual endpoint status instead (like F1) However rx_status is only used in the reinit case, without reintroducing issue LP-495 --- flight/pios/stm32f4xx/pios_usb_cdc.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/flight/pios/stm32f4xx/pios_usb_cdc.c b/flight/pios/stm32f4xx/pios_usb_cdc.c index 2da23bc87..52513f8bd 100644 --- a/flight/pios/stm32f4xx/pios_usb_cdc.c +++ b/flight/pios/stm32f4xx/pios_usb_cdc.c @@ -91,8 +91,9 @@ struct pios_usb_cdc_dev { * that are strictly < maxPacketSize for this interface which means we never have * to bother with zero length packets (ZLP). */ - uint8_t tx_packet_buffer[PIOS_USB_BOARD_CDC_DATA_LENGTH - 1] __attribute__((aligned(4))); + uint8_t tx_packet_buffer[PIOS_USB_BOARD_CDC_DATA_LENGTH - 1] __attribute__((aligned(4))); volatile bool tx_active; + volatile bool rx_active; uint8_t ctrl_tx_packet_buffer[PIOS_USB_BOARD_CDC_MGMT_LENGTH] __attribute__((aligned(4))); @@ -191,8 +192,9 @@ int32_t PIOS_USB_CDC_Init(uint32_t *usbcdc_id, const struct pios_usb_cdc_cfg *cf pios_usb_cdc_id = (uint32_t)usb_cdc_dev; - /* Tx is not active yet */ + /* Tx and Rx are not active yet */ usb_cdc_dev->tx_active = false; + usb_cdc_dev->rx_active = false; /* Clear stats */ usb_cdc_dev->rx_dropped = 0; @@ -279,6 +281,7 @@ static void PIOS_USB_CDC_RxStart(uint32_t usbcdc_id, uint16_t rx_bytes_avail) PIOS_USBHOOK_EndpointRx(usb_cdc_dev->cfg->data_rx_ep, usb_cdc_dev->rx_packet_buffer, sizeof(usb_cdc_dev->rx_packet_buffer)); + usb_cdc_dev->rx_active = true; } } @@ -608,6 +611,12 @@ static void PIOS_USB_CDC_DATA_IF_Init(uint32_t usb_cdc_id) (uint32_t)usb_cdc_dev); usb_cdc_dev->usb_data_if_enabled = true; usb_cdc_dev->tx_active = false; + /* Check if rx was previously active, if so we need to reactivate */ + if (usb_cdc_dev->rx_active) { + PIOS_USBHOOK_EndpointRx(usb_cdc_dev->cfg->data_rx_ep, + usb_cdc_dev->rx_packet_buffer, + sizeof(usb_cdc_dev->rx_packet_buffer)); + } } static void PIOS_USB_CDC_DATA_IF_DeInit(uint32_t usb_cdc_id) @@ -711,9 +720,11 @@ static bool PIOS_USB_CDC_DATA_EP_OUT_Callback( usb_cdc_dev->rx_packet_buffer, sizeof(usb_cdc_dev->rx_packet_buffer)); rc = true; + usb_cdc_dev->rx_active = true; } else { /* Not enough room left for a message, apply backpressure */ rc = false; + usb_cdc_dev->rx_active = false; } #if defined(PIOS_INCLUDE_FREERTOS) From 4c9b17903ec7aca4f8200d4d60a19e56921e9f8f Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 18 Oct 2018 13:13:06 +0200 Subject: [PATCH 7/8] LP-602 Update all bootloaders that run on STM32F4 hardware to use correct USB connected check --- flight/targets/boards/discoveryf4bare/bootloader/main.c | 2 +- flight/targets/boards/osd/bootloader/main.c | 2 +- flight/targets/boards/revonano/bootloader/main.c | 2 +- flight/targets/boards/revoproto/bootloader/main.c | 2 +- flight/targets/boards/sparky2/bootloader/main.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/flight/targets/boards/discoveryf4bare/bootloader/main.c b/flight/targets/boards/discoveryf4bare/bootloader/main.c index aa5c7999a..c85ef675e 100644 --- a/flight/targets/boards/discoveryf4bare/bootloader/main.c +++ b/flight/targets/boards/discoveryf4bare/bootloader/main.c @@ -82,7 +82,7 @@ int main() // is 2.7 volts check_bor(); - USB_connected = PIOS_USB_CheckAvailable(0); + USB_connected = PIOS_USB_CableConnected(0); if (PIOS_IAP_CheckRequest() == true) { PIOS_DELAY_WaitmS(1000); diff --git a/flight/targets/boards/osd/bootloader/main.c b/flight/targets/boards/osd/bootloader/main.c index c333ab9af..36252cc83 100644 --- a/flight/targets/boards/osd/bootloader/main.c +++ b/flight/targets/boards/osd/bootloader/main.c @@ -76,7 +76,7 @@ int main() PIOS_Board_Init(); PIOS_IAP_Init(); - USB_connected = PIOS_USB_CheckAvailable(0); + USB_connected = PIOS_USB_CableConnected(0); if (PIOS_IAP_CheckRequest() == true) { PIOS_DELAY_WaitmS(1000); diff --git a/flight/targets/boards/revonano/bootloader/main.c b/flight/targets/boards/revonano/bootloader/main.c index aa5c7999a..c85ef675e 100644 --- a/flight/targets/boards/revonano/bootloader/main.c +++ b/flight/targets/boards/revonano/bootloader/main.c @@ -82,7 +82,7 @@ int main() // is 2.7 volts check_bor(); - USB_connected = PIOS_USB_CheckAvailable(0); + USB_connected = PIOS_USB_CableConnected(0); if (PIOS_IAP_CheckRequest() == true) { PIOS_DELAY_WaitmS(1000); diff --git a/flight/targets/boards/revoproto/bootloader/main.c b/flight/targets/boards/revoproto/bootloader/main.c index aa5c7999a..c85ef675e 100644 --- a/flight/targets/boards/revoproto/bootloader/main.c +++ b/flight/targets/boards/revoproto/bootloader/main.c @@ -82,7 +82,7 @@ int main() // is 2.7 volts check_bor(); - USB_connected = PIOS_USB_CheckAvailable(0); + USB_connected = PIOS_USB_CableConnected(0); if (PIOS_IAP_CheckRequest() == true) { PIOS_DELAY_WaitmS(1000); diff --git a/flight/targets/boards/sparky2/bootloader/main.c b/flight/targets/boards/sparky2/bootloader/main.c index 02d53d948..6ed56133f 100644 --- a/flight/targets/boards/sparky2/bootloader/main.c +++ b/flight/targets/boards/sparky2/bootloader/main.c @@ -83,7 +83,7 @@ int main() // is 2.7 volts check_bor(); - USB_connected = PIOS_USB_CheckAvailable(0); + USB_connected = PIOS_USB_CableConnected(0); if (PIOS_IAP_CheckRequest() == true) { PIOS_DELAY_WaitmS(1000); From f162bbf331a15ca91a14a52f2c04d6951cd64df8 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Fri, 19 Oct 2018 16:25:27 +0200 Subject: [PATCH 8/8] LP-602 Improvement to usb_dcd. Activate rx handler always, even if it seems unneeded. Otherwise reading from DCD will fail if a module tries to read data before USB is connected. --- flight/pios/stm32f4xx/pios_usb_cdc.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/flight/pios/stm32f4xx/pios_usb_cdc.c b/flight/pios/stm32f4xx/pios_usb_cdc.c index 52513f8bd..e8e847065 100644 --- a/flight/pios/stm32f4xx/pios_usb_cdc.c +++ b/flight/pios/stm32f4xx/pios_usb_cdc.c @@ -91,9 +91,8 @@ struct pios_usb_cdc_dev { * that are strictly < maxPacketSize for this interface which means we never have * to bother with zero length packets (ZLP). */ - uint8_t tx_packet_buffer[PIOS_USB_BOARD_CDC_DATA_LENGTH - 1] __attribute__((aligned(4))); + uint8_t tx_packet_buffer[PIOS_USB_BOARD_CDC_DATA_LENGTH - 1] __attribute__((aligned(4))); volatile bool tx_active; - volatile bool rx_active; uint8_t ctrl_tx_packet_buffer[PIOS_USB_BOARD_CDC_MGMT_LENGTH] __attribute__((aligned(4))); @@ -194,7 +193,6 @@ int32_t PIOS_USB_CDC_Init(uint32_t *usbcdc_id, const struct pios_usb_cdc_cfg *cf /* Tx and Rx are not active yet */ usb_cdc_dev->tx_active = false; - usb_cdc_dev->rx_active = false; /* Clear stats */ usb_cdc_dev->rx_dropped = 0; @@ -281,7 +279,6 @@ static void PIOS_USB_CDC_RxStart(uint32_t usbcdc_id, uint16_t rx_bytes_avail) PIOS_USBHOOK_EndpointRx(usb_cdc_dev->cfg->data_rx_ep, usb_cdc_dev->rx_packet_buffer, sizeof(usb_cdc_dev->rx_packet_buffer)); - usb_cdc_dev->rx_active = true; } } @@ -611,12 +608,10 @@ static void PIOS_USB_CDC_DATA_IF_Init(uint32_t usb_cdc_id) (uint32_t)usb_cdc_dev); usb_cdc_dev->usb_data_if_enabled = true; usb_cdc_dev->tx_active = false; - /* Check if rx was previously active, if so we need to reactivate */ - if (usb_cdc_dev->rx_active) { - PIOS_USBHOOK_EndpointRx(usb_cdc_dev->cfg->data_rx_ep, - usb_cdc_dev->rx_packet_buffer, - sizeof(usb_cdc_dev->rx_packet_buffer)); - } + /* Activate rx prophylactically */ + PIOS_USBHOOK_EndpointRx(usb_cdc_dev->cfg->data_rx_ep, + usb_cdc_dev->rx_packet_buffer, + sizeof(usb_cdc_dev->rx_packet_buffer)); } static void PIOS_USB_CDC_DATA_IF_DeInit(uint32_t usb_cdc_id) @@ -720,11 +715,9 @@ static bool PIOS_USB_CDC_DATA_EP_OUT_Callback( usb_cdc_dev->rx_packet_buffer, sizeof(usb_cdc_dev->rx_packet_buffer)); rc = true; - usb_cdc_dev->rx_active = true; } else { /* Not enough room left for a message, apply backpressure */ rc = false; - usb_cdc_dev->rx_active = false; } #if defined(PIOS_INCLUDE_FREERTOS)