Skip to content

ch552: add optional bulk-transfer DEBUG endpoint mode and FPGA CTS gating - #420

Open
jthornblad wants to merge 8 commits into
resetfrom
ch552_bulk
Open

ch552: add optional bulk-transfer DEBUG endpoint mode and FPGA CTS gating#420
jthornblad wants to merge 8 commits into
resetfrom
ch552_bulk

Conversation

@jthornblad

@jthornblad jthornblad commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description

  • Add BULK make flag to build DEBUG interface with bulk endpoints instead of interrupt, reducing descriptor overhead and skipping HID-only config/report descriptor requests when enabled
  • Increase UART RX buffer 140 -> 220 bytes and update CTS threshold (95%) to match
  • Only forward buffered USB data (EP2/EP3/EP4) to UART when FPGA asserts CTS (P1.4 low), preventing deadlocks when the FPGA isn't ready to receive

Type of change

Please tick any that are relevant to this PR and remove any that aren't.

  • Bugfix (non breaking change which resolve an issue)
  • Feature (non breaking change which adds functionality)
  • Breaking Change (a change which would cause existing functionality to not work as expected)
  • Documentation (a change to documentation)

Submission checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my changes
  • I have tested and verified my changes on target
  • My changes are well written and CI is passing
  • I have squashed my work to relevant commits and rebased on main for linear history
  • I have added a "Co-authored-by: x" if several people contributed, either pair programming or by squashing commits from different authors.
  • I have updated the documentation where relevant (readme, dev.tillitis.se etc.)
  • QEMU is updated to reflect changes

@jthornblad
jthornblad requested review from agren and dehanj July 8, 2026 14:49
agren added 5 commits July 9, 2026 09:47
Instead of always enabling USB endpoints at boot we let the firmware or
app that intend to receive data decide when to enable them.

This allows a client to know when to send data after a reset.
On Windows, the go-serial library used in our client side apps discards
USB serial number strings that contain characters other than
'[0-9a-zA-Z_]'. We work around this by replacing '-' by '_' here.
To provide uniform behavior from a device app point of view we disable
all USB IO before starting an app loaded from client. This makes an app
start with disabled USB IO when loaded from client or flash.
@jthornblad
jthornblad force-pushed the ch552_bulk branch 2 times, most recently from 48f34c3 to 82ebd95 Compare July 9, 2026 14:47

// Check if Endpoint 2 (CDC) has received data
if (UsbEp2ByteCount) {
if (UsbEp2ByteCount && (gpio_p1_4_get() == 0)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is good improvement, since we don't wait doing nothing until the FPGA is ready but can keep processing other stuff. The same check inside made inside the CH554UART1SendByte(), should we remove it So the contract becomes that one has to check for CTS, and then send the byte?

It does not hold for CH554UART1SendBuffer() however, which has to to the check..

I would also make a helper function looks something like,

bool fpga_uart_ready (
   return gpio_p1_4_get() == 0;
}

to reduce at least three duplicate code parts in main.c

inline void check_cts_stop(void)
{
if (uart_byte_count() >= 133) // UartRxBuf is filled to 95% or more
if (uart_byte_count() >= 209) // UartRxBuf is filled to 95% or more

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why specifically 95%? Feels like we will have 11 unused bytes.
Wouldn't it be enough with like 2 bytes, or maybe 8 bytes to be conservative?

@dehanj dehanj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should probably be three commits, since it is three different things.

- Add BULK make flag to define BULK_TRANSFER, switching the DEBUG
  interface from interrupt to bulk endpoints (0 ms bInterval instead
  of 16 ms)
- Add a bulk-only DebugDesc variant (23 bytes vs. 32) alongside the
  existing interrupt version, selected via #ifdef BULK_TRANSFER
- Skip DebugCfgDesc/DebugReportDesc HID requests when built with
  BULK_TRANSFER, since the vendor-specific bulk interface has no
  HID report descriptor
- Fold DEBUG_REPORT_DESC_SIZE into the interrupt/bulk #ifdef and
  correct it to match the actual descriptor size (was hardcoded to
  34, actual size is 32); MAX_CFG_DESC_SIZE now derives from it
  instead of duplicating the constant
- Bump UART_RX_BUF_SIZE from 140 to 220 bytes
- Update CTS stop threshold from 133 to 209 to match (95% of 220)
- Only forward buffered USB data (EP2 CDC, EP3 FIDO/CCID, EP4 DEBUG)
  to the UART when the FPGA asserts CTS (P1.4 low), preventing
  deadlocks when the FPGA isn't ready to receive
@jthornblad
jthornblad requested a review from dehanj July 10, 2026 14:00
@agren
agren force-pushed the reset branch 2 times, most recently from 3dbfb17 to 30b1642 Compare July 13, 2026 12:17
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