tinyusb icon indicating copy to clipboard operation
tinyusb copied to clipboard

Add CH32V20x USB OTG/FS Driver

Open dragonlock2 opened this issue 2 years ago • 15 comments

Describe the PR Added support for the USB OTG/FS driver found in the CH32V20x microcontrollers, heavily inspired by the CH32V307 driver. While not tested, this driver should also work for the matching IP in the CH32V307.

Note there is an older incomplete PR https://github.com/hathach/tinyusb/pull/1995 that was trying to accomplish something similar.

Additional context Driver does appear to be working well, I'm currently using it to implement a basic RPC using bulk transfers and 8-channel 16-bit 48kHz audio streaming in dragonlock2/miscboards/wch/rvice_adc.

dragonlock2 avatar Dec 06 '23 07:12 dragonlock2

I did a quick test with CH32V30X chips, it seems to work also there (very minor changes needed because i'm not using wch SDK) Good job!

mean00 avatar Feb 19 '24 19:02 mean00

I'm just trying to compile the Pull request. I'm documenting here my process.

  1. I'm getting an error in dcd_int_handler(uint8_t rhport) . That a semicolon is missing after a declaration. switch (token) { case PID_OUT:; uint16_t rx_len = USBOTG_FS->RX_LEN; update_out(rhport, ep, rx_len); break;

greenscreenflicker avatar Mar 23 '24 20:03 greenscreenflicker

I'm just trying to compile the Pull request. I'm documenting here my process.

  1. I'm getting an error in dcd_int_handler(uint8_t rhport) . That a semicolon is missing after a declaration. switch (token) { case PID_OUT:; uint16_t rx_len = USBOTG_FS->RX_LEN; update_out(rhport, ep, rx_len); break;

Some compilers don't like declaring a variable inside a switch/case. Added brackets to fix.

dragonlock2 avatar Mar 23 '24 20:03 dragonlock2

I'm just trying to compile the Pull request. I'm documenting here my process.

  1. I'm getting an error in dcd_int_handler(uint8_t rhport) . That a semicolon is missing after a declaration. switch (token) { case PID_OUT:; uint16_t rx_len = USBOTG_FS->RX_LEN; update_out(rhport, ep, rx_len); break;

Some compilers don't like declaring a variable inside a switch/case. Added brackets to fix.

Fixed by additional semicolon... see PID_OUT... Thanks for the support so far!

greenscreenflicker avatar Mar 23 '24 20:03 greenscreenflicker

@dragonlock2 does it support CDC also?

I have the problem that I don't get CDC enumeration. But that's probabbly me beeing a noob on tinyusb.

greenscreenflicker avatar Mar 23 '24 21:03 greenscreenflicker

I prefer brackets for readability, didn't know a semicolon also works. The driver supports all of the transfer types needed in USB, so this should work for CDC too.

dragonlock2 avatar Mar 23 '24 21:03 dragonlock2

Can you please give me some hints why I don't get enumeration. What could I do wrong? I run it in Freertos, in a seperate task. It looks like this:

void task1_task(void *pvParameters)
{

    board_init();

    // init device stack on configured roothub port
    tud_init(BOARD_TUD_RHPORT);

    if (board_init_after_tusb) {
      board_init_after_tusb();
    }

    while (1) {
      tud_task(); // tinyusb device task
      tud_cdc_n_write_str(0,"Hello, World!\r\n");
      vTaskDelay(10);
    }
}

EDIT: I copied the rest of the code from https://github.com/hathach/tinyusb/tree/master/examples/device/cdc_dual_ports/src

EDIT2: I can assure that it's not the hardware, as it works with a non-tinyusb code.

greenscreenflicker avatar Mar 23 '24 21:03 greenscreenflicker

@greenscreenflicker let's continue this part of the discussion on https://github.com/hathach/tinyusb/discussions/2525 and keep comments here focused on the PR

dragonlock2 avatar Mar 23 '24 22:03 dragonlock2

Please mention in the files, that it only works with some of the devices, having the right module. (H/D in the grafics below)

grafik

greenscreenflicker avatar Mar 26 '24 10:03 greenscreenflicker

Not sure what @hathach would prefer, but I think the driver names are enough documentation. dcd_ch32_usbfs.c is for the USBFS/OTG_FS IP and dcd_ch32_usbhs.c is for the USBHD IP.

Notably some CH32V20x only have the USBD IP which I believe may be compatible with the STM32 driver.

dragonlock2 avatar Mar 26 '24 10:03 dragonlock2

I would be very interested into this as have 2 boards with v203(CH32V203G6U6 and CH32V203C8U6) If you need any test let me know but love to have normal USB access with tinyUSB

ddB0515 avatar Mar 26 '24 15:03 ddB0515

@ddB0515 Note that CH32V203G6U6 is not supported, while CH32V203C8U6 probbably is. The naming by wch (@openwch) of the devices in unfortunate.

greenscreenflicker avatar Mar 26 '24 16:03 greenscreenflicker

@greenscreenflicker will CH32V203G6U6 work at least as HID/CDC? as that is supported as USB Device or I'm wrong?

ddB0515 avatar Mar 26 '24 16:03 ddB0515

@ddB0515 CH32V203G6U6 doesn't work with CDC/HID with the proposed module (see image from me above). I use that MCU also, and have the problem. See here: https://github.com/hathach/tinyusb/discussions/2525#discussioncomment-8912923

greenscreenflicker avatar Mar 26 '24 16:03 greenscreenflicker

@ddB0515 @greenscreenflicker as I expected the existing dcd_stm32_fsdev.c works for the USBD of CH32V20x. See dragonlock2/miscboards/wch/mouse_jiggler. Let's continue discussion on https://github.com/hathach/tinyusb/discussions/2525 and keep this PR focused on the USB OTG/FS driver.

dragonlock2 avatar Mar 28 '24 03:03 dragonlock2

thank you for your PR. I am testing this out, rebased (with latest) and also fix ci build and other minor change to cmake build https://github.com/hathach/tinyusb/tree/pr2362_rebased , but I couldn push to this PR since you haven't allow maintainner to update this. Would you mind changing the PR to give me the write permission.

hathach avatar May 16 '24 15:05 hathach

try it now, you should be able to push to my fork

dragonlock2 avatar May 16 '24 16:05 dragonlock2

try it now, you should be able to push to my fork

Thank you, I have push the rebased to your fork. Though it isnt quite working, it keep stalling the get configuration. (stall is set with device qualifier previous, should be clear on setup). I may be due to recent chagnes on master, though it shows that this driver is partially working. I am doing more test and try to fix it if I could.

image

hathach avatar May 17 '24 07:05 hathach

Interesting, it's still working well on my end with my mouse_jiggler example, but I do set bcdUSB to 0x0110 because I had issues enumerating when using 0x0200. This is probably related.

I can take a look too, which example are you running?

dragonlock2 avatar May 17 '24 08:05 dragonlock2

I am testing with cdc_msc stock example. yeah, setting bcdUSB to 2.0 with fullspeed will cause host to request device qualifier --> which tinyusb will stall as an not supported. I think the current driver didn't clear stall correctly. I will revise the driver and fix that later, I think we are getting closed to merge this. I will also cross-checking/cherry-pick with code in #1995 as well.

hathach avatar May 17 '24 14:05 hathach

Try it now, I didn't realize dcd_edpt_xfer should clear the stall, added a call to dcd_edpt_clear_stall. Looks like it's enumerating now on my end with bcdUSB set to 0x0200.

dragonlock2 avatar May 17 '24 16:05 dragonlock2

@dragonlock2 no, it is not correct behavior. We should only clear EP0 stalled status (both in and out) when receiving SETUP token. For non-control endpoint, the stalled status remain until it is explicitly cleared by host.

hathach avatar May 20 '24 04:05 hathach

I see, I just moved the dcd_edpt_clear_stall calls to the interrupt when the setup token is received

dragonlock2 avatar May 20 '24 04:05 dragonlock2

I see, I just moved the dcd_edpt_clear_stall calls to the interrupt when the setup token is received

perfect, I am about to push the update, but you are faster than me. It is enumerated now, it is now safe to do some "minor" moving and/or generalizing the driver. Will try to merge this soon enough.

hathach avatar May 20 '24 04:05 hathach