From 35cd8c49d5dc267d9387a7e7d1e99b523a6bca2c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 May 2026 01:53:30 +0000 Subject: [PATCH 1/2] fix: correct CH32X035 RX_RES bit positions and shared register handling - ch32_usbfs_reg.h: Fix USBFS_EP_R_RES_* values from (x<<0) to (x<<2) to match USBFS_EP_R_RES_MASK at bits [3:2] for CH32X035 - dcd_ch32_usbfs.c: Add #if defined(CH32X035) guards throughout driver to handle the shared TX/RX control register in CH32X035: - dcd_init: combine TX+RX writes for EP0 and EP1-7 initialization - update_in: preserve RX bits when updating TX for EP0 - update_out: preserve TX bits when setting RX=ACK for EP0 - dcd_int_handler: fix SETUP and BUS_RST EP0 register writes - dcd_edpt0_status_complete: combine TX+RX writes - dcd_edpt_open: use read-modify-write to preserve other direction - dcd_edpt_stall/clear_stall: use read-modify-write for EP0 Agent-Logs-Url: https://github.com/21km43/Adafruit_TinyUSB_Arduino/sessions/5141344a-6785-407d-8fca-757e32bbc9c4 Co-authored-by: 21km43 <48169975+21km43@users.noreply.github.com> --- src/portable/wch/ch32_usbfs_reg.h | 11 +++-- src/portable/wch/dcd_ch32_usbfs.c | 67 +++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/src/portable/wch/ch32_usbfs_reg.h b/src/portable/wch/ch32_usbfs_reg.h index ecd7a5eb..8f2f2d96 100644 --- a/src/portable/wch/ch32_usbfs_reg.h +++ b/src/portable/wch/ch32_usbfs_reg.h @@ -161,14 +161,17 @@ #define USBFS_EP_T_RES_STALL (3 << 0) // RX_CTRL +// In CH32X035, TX and RX share the same 8-bit control register (UEP0_CTRL_H): +// bits [1:0] = TX response, bit [4] = AUTO_TOG (shared), bit [6] = TX toggle +// bits [3:2] = RX response, bit [7] = RX toggle #define USBFS_EP_R_RES_MASK (3 << 2) #define USBFS_EP_R_TOG (1 << 7) #define USBFS_EP_R_AUTO_TOG (1 << 4) -#define USBFS_EP_R_RES_ACK (0 << 0) -#define USBFS_EP_R_RES_NYET (1 << 0) -#define USBFS_EP_R_RES_NAK (2 << 0) -#define USBFS_EP_R_RES_STALL (3 << 0) +#define USBFS_EP_R_RES_ACK (0 << 2) +#define USBFS_EP_R_RES_NYET (1 << 2) +#define USBFS_EP_R_RES_NAK (2 << 2) +#define USBFS_EP_R_RES_STALL (3 << 2) #else // TX_CTRL #define USBFS_EP_T_RES_MASK (3 << 0) diff --git a/src/portable/wch/dcd_ch32_usbfs.c b/src/portable/wch/dcd_ch32_usbfs.c index 80f5b74a..e674b648 100644 --- a/src/portable/wch/dcd_ch32_usbfs.c +++ b/src/portable/wch/dcd_ch32_usbfs.c @@ -88,7 +88,13 @@ static void update_in(uint8_t rhport, uint8_t ep, bool force) { EP_TX_LEN(ep) = len; if (ep == 0) { +#if defined(CH32X035) + // CH32X035: TX and RX share one register; preserve RX bits when updating TX + EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_R_RES_MASK | USBFS_EP_R_TOG)) | + USBFS_EP_T_RES_ACK | (data.ep0_tog ? USBFS_EP_T_TOG : 0); +#else EP_TX_CTRL(0) = USBFS_EP_T_RES_ACK | (data.ep0_tog ? USBFS_EP_T_TOG : 0); +#endif data.ep0_tog = !data.ep0_tog; } else if (data.isochronous[ep]) { EP_TX_CTRL(ep) = (EP_TX_CTRL(ep) & ~(USBFS_EP_T_RES_MASK)) | USBFS_EP_T_RES_NYET; @@ -124,7 +130,12 @@ static void update_out(uint8_t rhport, uint8_t ep, size_t rx_len) { } if (ep == 0) { +#if defined(CH32X035) + // CH32X035: TX and RX share one register; preserve TX bits when setting RX=ACK + EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | USBFS_EP_R_RES_ACK; +#else EP_RX_CTRL(0) = USBFS_EP_R_RES_ACK; +#endif } } } @@ -143,8 +154,13 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) { // setup endpoint 0 EP_DMA(0) = (uint32_t) &data.buffer[0][0]; EP_TX_LEN(0) = 0; +#if defined(CH32X035) + // CH32X035: TX and RX share one register; write combined initial state + EP_TX_CTRL(0) = USBFS_EP_T_RES_NAK | USBFS_EP_R_RES_ACK; +#else EP_TX_CTRL(0) = USBFS_EP_T_RES_NAK; EP_RX_CTRL(0) = USBFS_EP_R_RES_ACK; +#endif // enable other endpoints but NAK everything USBFSD->UEP4_1_MOD = 0xCC; @@ -165,8 +181,14 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) { EP_DMA(ep) = (uint32_t) &data.buffer[ep][0]; #endif EP_TX_LEN(ep) = 0; +#if defined(CH32X035) + // CH32X035: TX and RX share one register; write combined initial state + EP_TX_CTRL(ep) = USBFS_EP_T_AUTO_TOG | USBFS_EP_T_RES_NAK | + USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_NAK; +#else EP_TX_CTRL(ep) = USBFS_EP_T_AUTO_TOG | USBFS_EP_T_RES_NAK; EP_RX_CTRL(ep) = USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_NAK; +#endif } EP_DMA(3) = (uint32_t) &data.ep3_buffer.out[0]; @@ -195,8 +217,13 @@ void dcd_int_handler(uint8_t rhport) { case PID_SETUP: // setup clears stall +#if defined(CH32X035) + // CH32X035: TX and RX share one register; write combined state + EP_TX_CTRL(0) = USBFS_EP_T_RES_NAK | USBFS_EP_R_RES_ACK; +#else EP_TX_CTRL(0) = USBFS_EP_T_RES_NAK; EP_RX_CTRL(0) = USBFS_EP_R_RES_ACK; +#endif data.ep0_tog = true; dcd_event_setup_received(rhport, &data.buffer[0][TUSB_DIR_OUT][0], true); @@ -213,7 +240,12 @@ void dcd_int_handler(uint8_t rhport) { dcd_event_bus_reset(rhport, (USBFSD->UDEV_CTRL & USBFS_UDEV_CTRL_LOW_SPEED) ? TUSB_SPEED_LOW : TUSB_SPEED_FULL, true); USBFSD->DEV_ADDR = 0x00; +#if defined(CH32X035) + // CH32X035: TX and RX share one register; preserve TX bits when setting RX=ACK + EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | USBFS_EP_R_RES_ACK; +#else EP_RX_CTRL(0) = USBFS_EP_R_RES_ACK; +#endif USBFSD->INT_FG = USBFS_INT_FG_BUS_RST; } else if (status & USBFS_INT_FG_SUSPEND) { @@ -267,8 +299,13 @@ void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const* req request->bRequest == TUSB_REQ_SET_ADDRESS) { USBFSD->DEV_ADDR = (uint8_t) request->wValue; } +#if defined(CH32X035) + // CH32X035: TX and RX share one register; write combined state + EP_TX_CTRL(0) = USBFS_EP_T_RES_NAK | USBFS_EP_R_RES_ACK; +#else EP_TX_CTRL(0) = USBFS_EP_T_RES_NAK; EP_RX_CTRL(0) = USBFS_EP_R_RES_ACK; +#endif } bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const* desc_ep) { @@ -283,13 +320,28 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const* desc_ep) { if (ep != 0) { if (dir == TUSB_DIR_OUT) { if (data.isochronous[ep]) { +#if defined(CH32X035) + EP_TX_CTRL(ep) = (EP_TX_CTRL(ep) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | + USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_NYET; +#else EP_RX_CTRL(ep) = USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_NYET; +#endif } else { +#if defined(CH32X035) + EP_TX_CTRL(ep) = (EP_TX_CTRL(ep) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | + USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_ACK; +#else EP_RX_CTRL(ep) = USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_ACK; +#endif } } else { EP_TX_LEN(ep) = 0; +#if defined(CH32X035) + EP_TX_CTRL(ep) = (EP_TX_CTRL(ep) & (USBFS_EP_R_RES_MASK | USBFS_EP_R_TOG)) | + USBFS_EP_T_AUTO_TOG | USBFS_EP_T_RES_NAK; +#else EP_TX_CTRL(ep) = USBFS_EP_T_AUTO_TOG | USBFS_EP_T_RES_NAK; +#endif } } return true; @@ -339,10 +391,20 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { uint8_t dir = tu_edpt_dir(ep_addr); if (ep == 0) { if (dir == TUSB_DIR_OUT) { +#if defined(CH32X035) + // CH32X035: TX and RX share one register; preserve TX bits when setting RX=STALL + EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | USBFS_EP_R_RES_STALL; +#else EP_RX_CTRL(0) = USBFS_EP_R_RES_STALL; +#endif } else { EP_TX_LEN(0) = 0; +#if defined(CH32X035) + // CH32X035: TX and RX share one register; preserve RX bits when setting TX=STALL + EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_R_RES_MASK | USBFS_EP_R_TOG)) | USBFS_EP_T_RES_STALL; +#else EP_TX_CTRL(0) = USBFS_EP_T_RES_STALL; +#endif } } else { if (dir == TUSB_DIR_OUT) { @@ -359,7 +421,12 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) { uint8_t dir = tu_edpt_dir(ep_addr); if (ep == 0) { if (dir == TUSB_DIR_OUT) { +#if defined(CH32X035) + // CH32X035: TX and RX share one register; preserve TX bits when setting RX=ACK + EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | USBFS_EP_R_RES_ACK; +#else EP_RX_CTRL(0) = USBFS_EP_R_RES_ACK; +#endif } } else { if (dir == TUSB_DIR_OUT) { From 995c4cffcfe3ddb8ed49e06407942204bd408cf8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 May 2026 01:56:58 +0000 Subject: [PATCH 2/2] fix: address code review comments - use macros and clarify AUTO_TOG comment - Extract repeated read-modify-write patterns into CH32X035_PRESERVE_TX_BITS and CH32X035_PRESERVE_RX_BITS helper macros - Use EP_RX_CTRL in PRESERVE_RX_BITS for semantic clarity - Clarify AUTO_TOG comment in ch32_usbfs_reg.h to document shared-bit behavior Agent-Logs-Url: https://github.com/21km43/Adafruit_TinyUSB_Arduino/sessions/5141344a-6785-407d-8fca-757e32bbc9c4 Co-authored-by: 21km43 <48169975+21km43@users.noreply.github.com> --- src/portable/wch/ch32_usbfs_reg.h | 6 ++++-- src/portable/wch/dcd_ch32_usbfs.c | 26 ++++++++++++++------------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/portable/wch/ch32_usbfs_reg.h b/src/portable/wch/ch32_usbfs_reg.h index 8f2f2d96..6262ec96 100644 --- a/src/portable/wch/ch32_usbfs_reg.h +++ b/src/portable/wch/ch32_usbfs_reg.h @@ -162,8 +162,10 @@ // RX_CTRL // In CH32X035, TX and RX share the same 8-bit control register (UEP0_CTRL_H): -// bits [1:0] = TX response, bit [4] = AUTO_TOG (shared), bit [6] = TX toggle -// bits [3:2] = RX response, bit [7] = RX toggle +// bits [1:0] = TX response, bit [6] = TX toggle +// bits [3:2] = RX response, bit [7] = RX toggle +// bit [4] = AUTO_TOG, shared: a single bit that enables automatic data toggle +// for both TX and RX simultaneously. #define USBFS_EP_R_RES_MASK (3 << 2) #define USBFS_EP_R_TOG (1 << 7) #define USBFS_EP_R_AUTO_TOG (1 << 4) diff --git a/src/portable/wch/dcd_ch32_usbfs.c b/src/portable/wch/dcd_ch32_usbfs.c index e674b648..a8dd75c6 100644 --- a/src/portable/wch/dcd_ch32_usbfs.c +++ b/src/portable/wch/dcd_ch32_usbfs.c @@ -40,6 +40,11 @@ #define EP_TX_LEN(ep) ((&USBFSD->UEP0_TX_LEN)[2 * ep + (ep > 4 ? 24 : 0)]) #define EP_TX_CTRL(ep) ((&USBFSD->UEP0_CTRL_H)[2 * ep + (ep > 4 ? 24 : 0)]) #define EP_RX_CTRL(ep) ((&USBFSD->UEP0_CTRL_H)[2 * ep + (ep > 4 ? 24 : 0)]) +// CH32X035: TX and RX share one 8-bit control register. These helpers preserve +// the opposite direction's bits (RES + TOG) during a read-modify-write. +// AUTO_TOG (bit 4) is excluded from both masks so the caller's value takes effect. +#define CH32X035_PRESERVE_TX_BITS(ep) (EP_TX_CTRL(ep) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) +#define CH32X035_PRESERVE_RX_BITS(ep) (EP_RX_CTRL(ep) & (USBFS_EP_R_RES_MASK | USBFS_EP_R_TOG)) #else #define EP_DMA(ep) ((&USBFSD->UEP0_DMA)[ep]) #define EP_TX_LEN(ep) ((&USBFSD->UEP0_TX_LEN)[2 * ep]) @@ -90,7 +95,7 @@ static void update_in(uint8_t rhport, uint8_t ep, bool force) { if (ep == 0) { #if defined(CH32X035) // CH32X035: TX and RX share one register; preserve RX bits when updating TX - EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_R_RES_MASK | USBFS_EP_R_TOG)) | + EP_TX_CTRL(0) = CH32X035_PRESERVE_RX_BITS(0) | USBFS_EP_T_RES_ACK | (data.ep0_tog ? USBFS_EP_T_TOG : 0); #else EP_TX_CTRL(0) = USBFS_EP_T_RES_ACK | (data.ep0_tog ? USBFS_EP_T_TOG : 0); @@ -132,7 +137,7 @@ static void update_out(uint8_t rhport, uint8_t ep, size_t rx_len) { if (ep == 0) { #if defined(CH32X035) // CH32X035: TX and RX share one register; preserve TX bits when setting RX=ACK - EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | USBFS_EP_R_RES_ACK; + EP_TX_CTRL(0) = CH32X035_PRESERVE_TX_BITS(0) | USBFS_EP_R_RES_ACK; #else EP_RX_CTRL(0) = USBFS_EP_R_RES_ACK; #endif @@ -242,7 +247,7 @@ void dcd_int_handler(uint8_t rhport) { USBFSD->DEV_ADDR = 0x00; #if defined(CH32X035) // CH32X035: TX and RX share one register; preserve TX bits when setting RX=ACK - EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | USBFS_EP_R_RES_ACK; + EP_TX_CTRL(0) = CH32X035_PRESERVE_TX_BITS(0) | USBFS_EP_R_RES_ACK; #else EP_RX_CTRL(0) = USBFS_EP_R_RES_ACK; #endif @@ -321,15 +326,13 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const* desc_ep) { if (dir == TUSB_DIR_OUT) { if (data.isochronous[ep]) { #if defined(CH32X035) - EP_TX_CTRL(ep) = (EP_TX_CTRL(ep) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | - USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_NYET; + EP_TX_CTRL(ep) = CH32X035_PRESERVE_TX_BITS(ep) | USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_NYET; #else EP_RX_CTRL(ep) = USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_NYET; #endif } else { #if defined(CH32X035) - EP_TX_CTRL(ep) = (EP_TX_CTRL(ep) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | - USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_ACK; + EP_TX_CTRL(ep) = CH32X035_PRESERVE_TX_BITS(ep) | USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_ACK; #else EP_RX_CTRL(ep) = USBFS_EP_R_AUTO_TOG | USBFS_EP_R_RES_ACK; #endif @@ -337,8 +340,7 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const* desc_ep) { } else { EP_TX_LEN(ep) = 0; #if defined(CH32X035) - EP_TX_CTRL(ep) = (EP_TX_CTRL(ep) & (USBFS_EP_R_RES_MASK | USBFS_EP_R_TOG)) | - USBFS_EP_T_AUTO_TOG | USBFS_EP_T_RES_NAK; + EP_TX_CTRL(ep) = CH32X035_PRESERVE_RX_BITS(ep) | USBFS_EP_T_AUTO_TOG | USBFS_EP_T_RES_NAK; #else EP_TX_CTRL(ep) = USBFS_EP_T_AUTO_TOG | USBFS_EP_T_RES_NAK; #endif @@ -393,7 +395,7 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { if (dir == TUSB_DIR_OUT) { #if defined(CH32X035) // CH32X035: TX and RX share one register; preserve TX bits when setting RX=STALL - EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | USBFS_EP_R_RES_STALL; + EP_TX_CTRL(0) = CH32X035_PRESERVE_TX_BITS(0) | USBFS_EP_R_RES_STALL; #else EP_RX_CTRL(0) = USBFS_EP_R_RES_STALL; #endif @@ -401,7 +403,7 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { EP_TX_LEN(0) = 0; #if defined(CH32X035) // CH32X035: TX and RX share one register; preserve RX bits when setting TX=STALL - EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_R_RES_MASK | USBFS_EP_R_TOG)) | USBFS_EP_T_RES_STALL; + EP_TX_CTRL(0) = CH32X035_PRESERVE_RX_BITS(0) | USBFS_EP_T_RES_STALL; #else EP_TX_CTRL(0) = USBFS_EP_T_RES_STALL; #endif @@ -423,7 +425,7 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) { if (dir == TUSB_DIR_OUT) { #if defined(CH32X035) // CH32X035: TX and RX share one register; preserve TX bits when setting RX=ACK - EP_TX_CTRL(0) = (EP_TX_CTRL(0) & (USBFS_EP_T_RES_MASK | USBFS_EP_T_TOG)) | USBFS_EP_R_RES_ACK; + EP_TX_CTRL(0) = CH32X035_PRESERVE_TX_BITS(0) | USBFS_EP_R_RES_ACK; #else EP_RX_CTRL(0) = USBFS_EP_R_RES_ACK; #endif