Skip to content
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.

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

Add check for max number of endpoints #20

Merged
merged 1 commit into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 60 additions & 1 deletion src/portable/synopsys/dwc2/dcd_dwc2.c
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,48 @@ void dcd_sof_enable(uint8_t rhport, bool en)
/* DCD Endpoint port
*------------------------------------------------------------------*/

#if TU_CHECK_MCU(OPT_MCU_ESP32S2, OPT_MCU_ESP32S3, OPT_MCU_ESP32P4)

// Check if IN/OUT endpoint is avaliable before opening it
static bool dcd_edpt_available(uint8_t rhport, uint8_t dir)
{
// Verify that we have a vacant EP
if ((dwc_ep_config[rhport].in_ep + dwc_ep_config[rhport].out_ep) > (dwc_ep_config[rhport].ep_max_count - 1)) {
return false;
}
// Get the ep_count amount at the moment as a temporal variable and update it
uint8_t new_ep_count = (dir) ? dwc_ep_config[rhport].in_ep : dwc_ep_config[rhport].out_ep;
peter-marcisovsky marked this conversation as resolved.
Show resolved Hide resolved
new_ep_count++;

// Verify overflow for IN EP ESP32Sx
#if TU_CHECK_MCU(OPT_MCU_ESP32S2, OPT_MCU_ESP32S3)
// ESP32Sx has 6 endpoints, from which only 5 can be confiugred as IN
if ((dir) && (new_ep_count > (dwc_ep_config[rhport].ep_max_count - 1))) {
return false;
}
#endif // TU_CHECK_MCU(OPT_MCU_ESP32S2, OPT_MCU_ESP32S3)
// Write new value back
if(dir) {
dwc_ep_config[rhport].in_ep = new_ep_count;
} else {
dwc_ep_config[rhport].out_ep = new_ep_count;
}
return true;
}

// Release an endpoint.
static void dcd_edpt_release(uint8_t rhport, uint8_t dir)
{
if (dir) {
TU_ASSERT(dwc_ep_config[rhport].in_ep == 0); // Check if number of opened EPs is not zero
dwc_ep_config[rhport].in_ep--; // Release in_ep
} else {
TU_ASSERT(dwc_ep_config[rhport].out_ep == 0);
dwc_ep_config[rhport].out_ep--;
}
}
#endif

bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * desc_edpt)
{
(void) rhport;
Expand All @@ -640,7 +682,12 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * desc_edpt)
uint8_t const epnum = tu_edpt_number(desc_edpt->bEndpointAddress);
uint8_t const dir = tu_edpt_dir(desc_edpt->bEndpointAddress);

TU_ASSERT(epnum < ep_count);
#if TU_CHECK_MCU(OPT_MCU_ESP32S2, OPT_MCU_ESP32S3, OPT_MCU_ESP32P4)
if (!dcd_edpt_available(rhport, dir)) {
TU_LOG(1, "No endpoints available (ep_max=%d) \r\n", dwc_ep_config[rhport].ep_max_count);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would have been good to make this an error, spent some time to figure out I ran out of endpoints as this 'silently' passes... Before there was an assert.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bollenn,
Sorry for the inconveniences.

Actually, the whole idea of that MR was exactly as you have said: to display the message when there are no more EPs available.
Before, there was the assert, but the comparison itself was between wrong values, so the assert never worked, even in case when there no more vacant EPs available.

May I ask, why is was "silent" in your case? TU_LOG was configured to 0?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't remember, LOG level to low or return value check missing and this log just passing by in between a lot of other logs...

return false;
}
#endif

xfer_ctl_t * xfer = XFER_CTL_BASE(epnum, dir);
xfer->max_size = tu_edpt_packet_size(desc_edpt);
Expand Down Expand Up @@ -721,6 +768,11 @@ 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;

#if TU_CHECK_MCU(OPT_MCU_ESP32S2, OPT_MCU_ESP32S3, OPT_MCU_ESP32P4)
dwc_ep_config[rhport].in_ep = 0;
dwc_ep_config[rhport].out_ep = 0;
#endif

// Disable non-control interrupt
dwc2->daintmsk = (1 << DAINTMSK_OEPM_Pos) | (1 << DAINTMSK_IEPM_Pos);

Expand Down Expand Up @@ -878,6 +930,13 @@ void dcd_edpt_close (uint8_t rhport, uint8_t ep_addr)

dcd_edpt_disable(rhport, ep_addr, false);

#if TU_CHECK_MCU(OPT_MCU_ESP32S2, OPT_MCU_ESP32S3, OPT_MCU_ESP32P4)
peter-marcisovsky marked this conversation as resolved.
Show resolved Hide resolved
// Release an endpoint if it is not the 0 EP
if (epnum) {
dcd_edpt_release(rhport, dir);
}
#endif

// Update max_size
xfer_status[epnum][dir].max_size = 0; // max_size = 0 marks a disabled EP - required for changing FIFO allocation

Expand Down
12 changes: 11 additions & 1 deletion src/portable/synopsys/dwc2/dwc2_esp32.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
#endif

#if DWC2_FS_PERIPH_BASE
#define DWC2_FS_EP_MAX 6 // USB_OUT_EP_NUM. TODO ESP32Sx only has 5 tx fifo (5 endpoint IN)
#define DWC2_FS_EP_MAX 6 // USB_OUT_EP_NUM
#define DWC2_FS_EP_FIFO_SIZE 1024
#endif

Expand All @@ -82,6 +82,16 @@ static const dwc2_controller_t _dwc2_controller[] =
#endif
};

static dwc_ep_config_t dwc_ep_config[] =
{
#ifdef DWC2_FS_PERIPH_BASE
{ .out_ep = 0, .in_ep = 0, .ep_max_count = DWC2_FS_EP_MAX },
#endif
#ifdef DWC2_HS_PERIPH_BASE
{ .out_ep = 0, .in_ep = 0, .ep_max_count = DWC2_HS_EP_MAX },
#endif
};

static intr_handle_t usb_ih[DWC2_PERIPH_COUNT];

static void dcd_int_handler_wrap(void* arg)
Expand Down
8 changes: 8 additions & 0 deletions src/portable/synopsys/dwc2/dwc2_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ typedef struct
uint32_t ep_fifo_size;
}dwc2_controller_t;

#if TU_CHECK_MCU(OPT_MCU_ESP32S2, OPT_MCU_ESP32S3, OPT_MCU_ESP32P4)
typedef struct {
uint8_t out_ep;
uint8_t in_ep;
const uint8_t ep_max_count;
}dwc_ep_config_t;
#endif

/* DWC OTG HW Release versions */
#define DWC2_CORE_REV_2_71a 0x4f54271a
#define DWC2_CORE_REV_2_72a 0x4f54272a
Expand Down