Skip to content

cdc: Fix autoflush for FIFO < MPS#1487

Merged
hathach merged 1 commit intohathach:masterfrom
tore-espressif:fix/cdc_autoflush
Dec 6, 2022
Merged

cdc: Fix autoflush for FIFO < MPS#1487
hathach merged 1 commit intohathach:masterfrom
tore-espressif:fix/cdc_autoflush

Conversation

@tore-espressif
Copy link
Copy Markdown
Contributor

@tore-espressif tore-espressif commented Jun 2, 2022

Describe the PR
If CDC TX buffer size is configured to be less than bulk packet size, the autoflushing condition is never reached.

Changes:

  1. TX buff is automatically flushed when TX FIFO is full

Additional context
Found out during espressif/esp-idf#9040

@tore-espressif
Copy link
Copy Markdown
Contributor Author

tore-espressif commented Jun 2, 2022

This is in great extent reverting #547

Copy link
Copy Markdown
Collaborator

@perigoso perigoso left a comment

Choose a reason for hiding this comment

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

LGTM

@tore-espressif
Copy link
Copy Markdown
Contributor Author

tore-espressif commented Oct 18, 2022

@PanRe @perigoso @hathach any update about this?

Copy link
Copy Markdown
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

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

sorry for late response, I was too busy with other works. The reason for change request: although it has more code, the additional condition CFG_TUD_CDC_TX_BUFSIZE < BULK_PACKET_SIZE is constant expression and will help the compiler to optimize it out.

Therefore we don't have to run tu_fifo_full() when bufsize is larger than the packet size (and const expression also not executed at all in other case)

Copy link
Copy Markdown
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

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

thank for the update

@hathach hathach merged commit d9817eb into hathach:master Dec 6, 2022
7FM pushed a commit to 7FM/tinyusb that referenced this pull request Aug 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants