Skip to content

fix(dcd): Fixed race condition on device disconnect #3109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+49 −0
Open

fix(dcd): Fixed race condition on device disconnect #3109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/portable/synopsys/dwc2/dcd_dwc2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

#include "device/dcd.h"
#include "dwc2_common.h"
#include "dwc2_critical.h"

//--------------------------------------------------------------------+
// MACRO TYPEDEF CONSTANT ENUM
Expand All @@ -52,6 +53,9 @@ typedef struct {
uint8_t interval;
} xfer_ctl_t;

/*
This variable is modified from ISR context, so it must be protected by critical section
*/
static xfer_ctl_t xfer_status[DWC2_EP_MAX][2];
#define XFER_CTL_BASE(_ep, _dir) (&xfer_status[_ep][_dir])

Expand Down Expand Up @@ -321,6 +325,9 @@ static void edpt_disable(uint8_t rhport, uint8_t ep_addr, bool stall) {
}
}

// Since this function returns void, it is not possible to return a boolean success message
// We must make sure that this function is not called when the EP is disabled
// Must be called from critical section
static void edpt_schedule_packets(uint8_t rhport, const uint8_t epnum, const uint8_t dir) {
dwc2_regs_t* dwc2 = DWC2_REG(rhport);
xfer_ctl_t* const xfer = XFER_CTL_BASE(epnum, dir);
Expand Down Expand Up @@ -531,6 +538,7 @@ void dcd_edpt_close_all(uint8_t rhport) {
dwc2_regs_t* dwc2 = DWC2_REG(rhport);
uint8_t const ep_count = _dwc2_controller[rhport].ep_count;

DCD_ENTER_CRITICAL();
_dcd_data.allocated_epin_count = 0;

// Disable non-control interrupt
Expand All @@ -550,6 +558,7 @@ void dcd_edpt_close_all(uint8_t rhport) {
dfifo_flush_rx(dwc2);

dfifo_device_init(rhport); // re-init dfifo
DCD_EXIT_CRITICAL();
}

bool dcd_edpt_iso_alloc(uint8_t rhport, uint8_t ep_addr, uint16_t largest_packet_size) {
Expand All @@ -568,7 +577,12 @@ bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t* buffer, uint16_t to
uint8_t const epnum = tu_edpt_number(ep_addr);
uint8_t const dir = tu_edpt_dir(ep_addr);

DCD_ENTER_CRITICAL();
xfer_ctl_t* xfer = XFER_CTL_BASE(epnum, dir);
if (xfer->max_size == 0) {
DCD_EXIT_CRITICAL();
return false; // Endpoint is closed
}
xfer->buffer = buffer;
xfer->ff = NULL;
xfer->total_len = total_bytes;
Expand All @@ -580,6 +594,7 @@ bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t* buffer, uint16_t to

// Schedule packets to be sent within interrupt
edpt_schedule_packets(rhport, epnum, dir);
DCD_EXIT_CRITICAL();

return true;
}
Expand All @@ -595,14 +610,20 @@ bool dcd_edpt_xfer_fifo(uint8_t rhport, uint8_t ep_addr, tu_fifo_t* ff, uint16_t
uint8_t const epnum = tu_edpt_number(ep_addr);
uint8_t const dir = tu_edpt_dir(ep_addr);

DCD_ENTER_CRITICAL();
xfer_ctl_t* xfer = XFER_CTL_BASE(epnum, dir);
if (xfer->max_size == 0) {
DCD_EXIT_CRITICAL();
return false; // Endpoint is closed
}
xfer->buffer = NULL;
xfer->ff = ff;
xfer->total_len = total_bytes;

// Schedule packets to be sent within interrupt
// TODO xfer fifo may only available for slave mode
edpt_schedule_packets(rhport, epnum, dir);
DCD_EXIT_CRITICAL();

return true;
}
Expand Down Expand Up @@ -631,6 +652,7 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) {
//--------------------------------------------------------------------

// 7.4.1 Initialization on USB Reset
// Must be called from critical section
static void handle_bus_reset(uint8_t rhport) {
dwc2_regs_t *dwc2 = DWC2_REG(rhport);
const uint8_t ep_count = dwc2_ep_count(dwc2);
Expand Down Expand Up @@ -989,8 +1011,10 @@ void dcd_int_handler(uint8_t rhport) {

if (gintsts & GINTSTS_USBRST) {
// USBRST is start of reset.
DCD_ENTER_CRITICAL();
dwc2->gintsts = GINTSTS_USBRST;
handle_bus_reset(rhport);
DCD_EXIT_CRITICAL();
}

if (gintsts & GINTSTS_ENUMDNE) {
Expand Down
25 changes: 25 additions & 0 deletions src/portable/synopsys/dwc2/dwc2_critical.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/

#ifndef TUSB_DWC2_CRITICAL_H_
#define TUSB_DWC2_CRITICAL_H_

#include "common/tusb_mcu.h"

#if defined(TUP_USBIP_DWC2_ESP32)
#include "freertos/FreeRTOS.h"
static portMUX_TYPE dcd_lock = portMUX_INITIALIZER_UNLOCKED;
#define DCD_ENTER_CRITICAL() portENTER_CRITICAL(&dcd_lock)
#define DCD_EXIT_CRITICAL() portEXIT_CRITICAL(&dcd_lock)

#else
// Define critical section macros for DWC2 as no-op if not defined
// This is to avoid breaking existing code that does not use critical section
#define DCD_ENTER_CRITICAL() // no-op
#define DCD_EXIT_CRITICAL() // no-op
#endif

#endif // TUSB_DWC2_CRITICAL_H_
Loading