Renesas ra hs rebased from #2052

[hathach]

Describe the PR

Supersede #2052 (once this Pr is merged, the other will be marked as merged as well). Thank you @facchinm for original PR to add support for renesas highspeed USB. This PR rebased, merged with latest changes also add following changes to renesas port

• better multiple ports support, the port and speed support is dynamic instead of depending on CFG_TUSB_RHPORT0/1_MODE. This make otg support and switching role support easier in the future.
• move module start to part of the dcd/hcd init
• update fsp to 4.5.0
• refactor bsp support for RA family, also add cmake build
• Tested with following ek: ra 4m1 6m1 6m5 both fullspeed and highspeed device and host
• Note: for ra6m5 evk which has 2 USB (FS and HS), PORT=1 highspeed is used as default. To test with fullspeed port run build command with PORT=0. e.g make BOARD=ra6m5_ek PORT=0 or cmake -DBOARD=ra6m5_ek -DPORT=0
• Also tested with rx65n to make sure it does not break rx port
• Uno R4 is also added, board_test with led blinky is running, but I couldnn't get usb working on it. Probably some missing reset/setup. I have to call __enable_irq() in application since bootloader probably disable it before giving cpu control to application. Will probably test with this and portenta C33 later on when having more time.

Note: There is quite a bit of shared code between hcd and dcd for rusb2. We should refactor to have them in common files, but that can be saved for following-up PRs.

@facchinm I may miss a thing or two, please review and/or test this PR out if you have some time.

[facchinm]

Sure thing! I'm testing it right now inside the core with our test suite 🙂
A couple of things I already spotted:
UNO R4 Minima doesn't work because the internal LDO must be enabled via

#define USB_VDCEN 0x80
RUSB2->USBMC = (uint16_t) (RUSB2->USBMC | (USB_VDCEN));

I agree that this in not to be part of the actual driver but a configuration at board level, so we'll add the call in the core

[hathach]

HS FIFO is 32-bit but its addressing is a bit odd. Strangely this isn't needd for host driver. In the future we should use 32-bit mode for HS since it is faster.

[hathach]

Sure thing! I'm testing it right now inside the core with our test suite 🙂 A couple of things I already spotted: UNO R4 Minima doesn't work because the internal LDO must be enabled via

#define USB_VDCEN 0x80
RUSB2->USBMC = (uint16_t) (RUSB2->USBMC | (USB_VDCEN));

I agree that this in not to be part of the actual driver but a configuration at board level, so we'll add the call in the core

oh really, let me test it out, maybe there is an BSP macro that give out the hint and we could use that. Let me double check

Update: wowla it works like a charm, I totall miss this. In my defense, I am not a really good hw :D. This seems to be indeed hw decision sinnce this requires schematics changes to pin VCC_USB_LDO. I have updated the example to enable this specifically for UNO R4 after tud_init().

[iabdalkader]

This is no longer working for MicroPython/RA6M5 (the device just doesn't enumerate) is the following config still valid ?

CFLAGS += -DCFG_TUH_RHPORT=0 \
          -DCFG_TUD_RHPORT=1 \
          -DCFG_TUH_MAX_SPEED=OPT_MODE_FULL_SPEED \
          -DCFG_TUD_MAX_SPEED=OPT_MODE_HIGH_SPEED \
          -DCFG_TUSB_RHPORT0_MODE=0\
          -DCFG_TUSB_RHPORT1_MODE=OPT_MODE_DEVICE

[hathach]

This is no longer working for MicroPython/RA6M5 (the device just doesn't enumerate) is the following config still valid ?

CFLAGS += -DCFG_TUH_RHPORT=0 \
          -DCFG_TUD_RHPORT=1 \
          -DCFG_TUH_MAX_SPEED=OPT_MODE_FULL_SPEED \
          -DCFG_TUD_MAX_SPEED=OPT_MODE_HIGH_SPEED \
          -DCFG_TUSB_RHPORT0_MODE=0\
          -DCFG_TUSB_RHPORT1_MODE=OPT_MODE_DEVICE

Let me pull out the micropython/micropython#11405 to test with, may need some time since I am not familliar with building mp. Meanwhile I think you could just use

> CFLAGS += -DCFG_TUH_RHPORT=0 \
>           -DCFG_TUD_RHPORT=1 \
>           -DCFG_TUH_MAX_SPEED=OPT_MODE_FULL_SPEED \
>           -DCFG_TUD_MAX_SPEED=OPT_MODE_HIGH_SPEED \

CFG_TUSB_RHPORT0/1_MODE can be actually dropped now, but let me double check

[iabdalkader]

CFG_TUSB_RHPORT0/1_MODE can be actually dropped now, but let me double check

I tried a few different things but none of them seemed to work. To build the board just:

make BOARD=ARDUINO_PORTENTA_C33

Double tap to enter the bootloader , flash and reset:

dfu-util -a 0 -d 2341:0368 -D build-ARDUINO_PORTENTA_C33/firmware.bin

[hathach]

added portenta_c33 to tinyub's bsp. Example works well, maybe it is mp configure. Pulling to test ...

[hathach]

@iabdalkader just pull and tested, you need update to correctly pass rhport to interrupt handler (1 instead of 0). It previously works, since the device stack is fixed to either port0 or port1 with CFG_TUSB_RHPORT0/1_MODE, but this rebased update to get it work dynnamically.

void usbhs_interrupt_handler(void) {
    IRQn_Type irq = R_FSP_CurrentIrqGet();
    R_BSP_IrqStatusClear(irq);

    #if CFG_TUSB_RHPORT1_MODE & OPT_MODE_HOST
    tuh_int_handler(1);
    #endif

    #if CFG_TUSB_RHPORT1_MODE & OPT_MODE_DEVICE
    tud_int_handler(1);
    #endif
}

void usbhs_d0fifo_handler(void) {
    IRQn_Type irq = R_FSP_CurrentIrqGet();
    R_BSP_IrqStatusClear(irq);

    #if CFG_TUSB_RHPORT1_MODE & OPT_MODE_HOST
    tuh_int_handler(1);
    #endif

    #if CFG_TUSB_RHPORT1_MODE & OPT_MODE_DEVICE
    tud_int_handler(1);
    #endif
}

void usbhs_d1fifo_handler(void) {
    IRQn_Type irq = R_FSP_CurrentIrqGet();
    R_BSP_IrqStatusClear(irq);

    #if CFG_TUSB_RHPORT1_MODE & OPT_MODE_HOST
    tuh_int_handler(1);
    #endif

    #if CFG_TUSB_RHPORT1_MODE & OPT_MODE_DEVICE
    tud_int_handler(1);
    #endif
}

You can also remove CFG_TUH_RHPORT/CFG_TUD_RHPORT since aren't part of the tinyusb configuration.

CFLAGS += -DCFG_TUH_MAX_SPEED=OPT_MODE_FULL_SPEED \
          -DCFG_TUD_MAX_SPEED=OPT_MODE_HIGH_SPEED \
          -DCFG_TUSB_RHPORT0_MODE=0\
          -DCFG_TUSB_RHPORT1_MODE=OPT_MODE_DEVICE

If you use tud_init(rhport) (initialize port as device) instead of tusb_init(void), you could actually remove CFG_TUSB_RHPORT0/1_MODE. The reason is tinyusb is moving to be more dynamic role for each rhport (can be inited as device or host and also switch role (otg/drd) in the future). Hard coded with macro CFG_TUSB_RHPORT0_MODE isn't sufficient enough. However those are still supported as backward-compatible.

[iabdalkader]

@hathach Thanks for the insight, it now works again!

If you use tud_init(rhport) (initialize port as device) instead of tusb_init(void), you could actually remove CFG_TUSB_RHPORT0/1_MODE

Is there a way to know/query if a port is configured as host or device, if I use tud_init(rhport) and remove CFG_TUSB_RHPORTx_MODE ? Or a common irq handler that knows which mode the port is configured for and calls either tuh_int_handler or tud_int_handler ?

[facchinm]

@hathach I spotted one issue in the dynamic IRQ handling; since rusb2_controller_t rusb2_controller[] is static in a .h you get a different copy every time and tud_rusb2_set_irqnum() doesn't really work for multiple ports.
Where should I put the fix? The WIP core branch is https://github.com/facchinm/ArduinoCore-renesas/pull/new/tinyusb_mainline

EDIT: patch here arduino@965627e

[hathach]

@hathach Thanks for the insight, it now works again!

If you use tud_init(rhport) (initialize port as device) instead of tusb_init(void), you could actually remove CFG_TUSB_RHPORT0/1_MODE

Is there a way to know/query if a port is configured as host or device, if I use tud_init(rhport) and remove CFG_TUSB_RHPORTx_MODE ? Or a common irq handler that knows which mode the port is configured for and calls either tuh_int_handler or tud_int_handler ?

tud_init(rhport) is called by application, so it should be possible for application to know which port is configured for which role. Though, you are right having an API to check if which role is running or to have an common tinusb handler to just either call tud/tuh_int_hanlder() depending on how the are initialized would be nice. Though I am still waiting for otg/drd layer to do that (haven't working on this eyt).

Meanwhile you could define CFG_TUD_RHPORT or MP_TUD_RHPORT etc.. macro or a variable to call correct int handler.

Note: For renesas port USB FS is port0 and HS is port1.

[hathach]

@hathach I spotted one issue in the dynamic IRQ handling; since rusb2_controller_t rusb2_controller[] is static in a .h you get a different copy every time and tud_rusb2_set_irqnum() doesn't really work for multiple ports. Where should I put the fix? The WIP core branch is https://github.com/facchinm/ArduinoCore-renesas/pull/new/tinyusb_mainline

ah, you are right, hcd and dcd will have different copies of the controllers. I think we shoud add an common rusb2_common.c like rp2040 since both hcd and dcd seems to share quite a bit of code anyway.

PS: just saw your patch, exactly like my thought, let me copy that over :)

[hathach]

@facchinm I just realized we are doing the same thing in different PR 😄 . I merged your commits from 2052, but also add some additional changes:

• rename new file to rusb2_common.c since it maybe handy for other port. We will probably move some common code (pipe write/read) to this file as well
• rename tud_rub2_set_irqnum to tusb_() since this function is called for both host and devie stack (sorry for more renaming)
• add #if defined(TUP_USBIP_RUSB2) && (CFG_TUH_ENABLED || CFG_TUD_ENABLED) check, since some auto-makefile IDE like eclipse like to add all files recursively.

[facchinm]

@hathach I checked the host functionality without the "retry" logic and it works fine across all the carriers, patch for removal submitted here arduino@edee46e

[hathach]

@hathach I checked the host functionality without the "retry" logic and it works fine across all the carriers, patch for removal submitted here arduino@edee46e

Perfect, thank you