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 CH32V20x USB OTG/FS Driver #2362

Merged
merged 9 commits into from
May 20, 2024
Merged

Add CH32V20x USB OTG/FS Driver #2362

merged 9 commits into from
May 20, 2024

Conversation

dragonlock2
Copy link
Contributor

@dragonlock2 dragonlock2 commented Dec 6, 2023

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 #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.

Copy link
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 still haven't got time to test this out. I am off for TET holidays for a couple of weeks. Will try to review afterwards, thank you for your patient.

@mean00
Copy link

mean00 commented Feb 19, 2024

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!

@greenscreenflicker
Copy link

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;

@dragonlock2 dragonlock2 force-pushed the master branch 2 times, most recently from b2b1565 to 850cd12 Compare March 23, 2024 20:27
@dragonlock2
Copy link
Contributor Author

dragonlock2 commented Mar 23, 2024

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 dragonlock2 reopened this Mar 23, 2024
@greenscreenflicker
Copy link

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
Copy link

greenscreenflicker commented Mar 23, 2024

@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.

@dragonlock2
Copy link
Contributor Author

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.

@greenscreenflicker
Copy link

greenscreenflicker commented Mar 23, 2024

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.

@dragonlock2
Copy link
Contributor Author

@greenscreenflicker let's continue this part of the discussion on #2525 and keep comments here focused on the PR

@greenscreenflicker
Copy link

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

@dragonlock2
Copy link
Contributor Author

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.

@ddB0515
Copy link

ddB0515 commented Mar 26, 2024

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

@greenscreenflicker
Copy link

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

@ddB0515
Copy link

ddB0515 commented Mar 26, 2024

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

@greenscreenflicker
Copy link

greenscreenflicker commented Mar 26, 2024

@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:
#2525 (reply in thread)

@dragonlock2
Copy link
Contributor Author

@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 #2525 and keep this PR focused on the USB OTG/FS driver.

dragonlock2 and others added 2 commits May 15, 2024 20:01
add cmake for ch32v20x, skip freertos examples for CH32V20X, also skip net webserver due to lack of RAM
update to use openocd with wlinke adapter
@hathach
Copy link
Owner

hathach commented May 16, 2024

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.

@dragonlock2
Copy link
Contributor Author

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

@hathach
Copy link
Owner

hathach commented May 17, 2024

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

@dragonlock2
Copy link
Contributor Author

dragonlock2 commented May 17, 2024

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?

@hathach
Copy link
Owner

hathach commented May 17, 2024

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.

@dragonlock2
Copy link
Contributor Author

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.

@hathach
Copy link
Owner

hathach commented May 20, 2024

@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.

@dragonlock2
Copy link
Contributor Author

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

@hathach
Copy link
Owner

hathach commented May 20, 2024

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.

- add ch32v203_ro_1v0 board
- CFG_EXAMPLE_MSC_DUAL_READONLY to build msc_dual_lun for ch32v203
 add missing MIT license
Copy link
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 you very much for your effort and patient. This works perfectly well with both my v203 and v307 dev board. I have update the linker for v20x to support different flash/ram size (by symdef __flash/ram_size), which should work better for more boards than default 64k/20k.

I also change the usbhs and usbfs driver a bit so that it is used correctly with CFG_TUD_MAX_SPEED since high-end ch32 such as v307 support both controller.

Note: v203 also support both usbfs and stm32 fsdev, but that selection will be added later as follow up PR.

This should also close #1995

@hathach hathach merged commit a068b81 into hathach:master May 20, 2024
76 checks passed
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.

None yet

5 participants