Notes from porting TinyUSB

[rajivr]

Hi @hathach

Belated Lunar New Year Greetings and thanks again for TinyUSB. :-)

I managed to get a basic port of TinyUSB working. In our case the DCD layer is in Rust. There are some issues that came up during the port that I wanted to share with you.

I don't know a good way to create a PR for these changes, so I am providing links to the patches that we currently are maintaining in the tree.

1. We needed to remove static inline from tu_edpt_* functions. The patch is here.

   These functions are called from Rust DCD code.

2. We needed to remove TU_ATTR_WEAK from a few functions. The patch is here

   What was happening in this case was that the linker was picking up the weak definition from TinyUSB instead of the function defined in our crate (Rust's equivalent of a library).

3. Introduce tud_task_is_queue_empty() function. The patch is here

   We introduced this function because in the OS main loop, we needed some way to determine when TinyUSB did not have any more work left to do, so we can safely execute wfi instruction.

4. Because Rust does not have native bitfield support, I had to a workaround by having a dcd_edpt_open function written in C that extracted out data from endpoint descriptor before calling Rust DCD function. The code is here

I hope this is going to be helpful to other TinyUSB/Rust users.

[cr1901]

I'm not going to profile the static code (mainly because I don't know how :P), but I would rather see a Rust-api specific define (call it TU_RUST_API) that changes the definition of TU_ATTR_WEAK to nothing, e.g:

#ifdef TU_RUST_API
#define TU_ATTR_WEAK      
#else
#define TU_ATTR_WEAK                  __attribute__ ((weak))
#endif

And gates whether currently static functions remain static, e.g. in tusb_types.h:

#ifndef TU_RUST_API
static inline tusb_dir_t tu_edpt_dir(uint8_t addr)
{
    return (addr & TUSB_DIR_IN_MASK) ? TUSB_DIR_IN : TUSB_DIR_OUT;
}
#endif

tusb_types.c remains, but is only used by the cc crate when compiling tinyusb-sys or whatever equivalent you have. If compiling for a C project, tusb_types.c is never seen by your project.

[hathach]

Hi @hathach

Belated Lunar New Year Greetings and thanks again for TinyUSB. :-)

I managed to get a basic port of TinyUSB working. In our case the DCD layer is in Rust. There are some issues that came up during the port that I wanted to share with you.

@rajivr Thank you for your New Year Greetings, I am glad you find TinyUSB useful.

I don't know a good way to create a PR for these changes, so I am providing links to the patches that we currently are maintaining in the tree.

1. We needed to remove static inline from tu_edpt_* functions. The patch is here.
   These functions are called from Rust DCD code.

These are helper function, you can simply skip it and define your rust_edpt_* and copy its implementation, those won't be changed anytime soon.

2. We needed to remove TU_ATTR_WEAK from a few functions. The patch is here
   What was happening in this case was that the linker was picking up the weak definition from TinyUSB instead of the function defined in our crate (Rust's equivalent of a library).

as @cr1901 point out, you can use tusb_compiler.h to overwrite the definition.

3. Introduce tud_task_is_queue_empty() function. The patch is here
   We introduced this function because in the OS main loop, we needed some way to determine when TinyUSB did not have any more work left to do, so we can safely execute wfi instruction.

I thought you used Tock RTOS for rust embedded ? most RTOS will automatically to put the caller into suspended mode when queue is empty. For OS NONE configure, tud_task() only return after the event queue is empty. You can sleep after calling tud_task().

4. Because Rust does not have native bitfield support, I had to a workaround by having a dcd_edpt_open function written in C that extracted out data from endpoint descriptor before calling Rust DCD function. The code is here

I hope this is going to be helpful to other TinyUSB/Rust users.

I don't know Rust, could you just use the bitwise op ( & then shift) to extract the needed field. Sorry if it is dumb question, I have no idea how it works.

To be honest, I don't see the need to merge any of your proposed changes just yet. Maybe since I don't understand how Rust works.

[rajivr]

1. Introduce tud_task_is_queue_empty() function. The patch is here
   We introduced this function because in the OS main loop, we needed some way to determine when TinyUSB did not have any more work left to do, so we can safely execute wfi instruction.

I thought you used Tock RTOS for rust embedded ? most RTOS will automatically to put the caller into suspended mode when queue is empty. For OS NONE configure, tud_task() only return after the event queue is empty. You can sleep after calling tud_task().

TockOS, like a lot of other operating systems divides interrupt processing in two halves. The top-half runs in interrupt context, which is mapped to ARMv7-M's handler mode. The bottom-half processing happens in kernel context, which is mapped to ARMv7-M's privileged thread mode.

The main USB interrupt processing happens in the bottom half, and within the bottom half we add to TinyUSB task queue. Therefore we can call wfi, only when both TinyUSB task queue is empty and no more bottom half interrupt processing is left to do.

Hence the need for tud_task_is_queue_empty() function.

[hathach]

1. Introduce tud_task_is_queue_empty() function. The patch is here
   We introduced this function because in the OS main loop, we needed some way to determine when TinyUSB did not have any more work left to do, so we can safely execute wfi instruction.

I thought you used Tock RTOS for rust embedded ? most RTOS will automatically to put the caller into suspended mode when queue is empty. For OS NONE configure, tud_task() only return after the event queue is empty. You can sleep after calling tud_task().

TockOS, like a lot of other operating systems divides interrupt processing in two halves. The top-half runs in interrupt context, which is mapped to ARMv7-M's handler mode. The bottom-half processing happens in kernel context, which is mapped to ARMv7-M's privileged thread mode.

The main USB interrupt processing happens in the bottom half, and within the bottom half we add to TinyUSB task queue. Therefore we can call wfi, only when both TinyUSB task queue is empty and no more bottom half interrupt processing is left to do.

Hence the need for tud_task_is_queue_empty() function.

Tock should implement its own osal_tock port, instead of using osal_none. Like freeRTOS, I would expect the native queue API from RTOS will put the caller into suspended mode if the queue is empty https://github.com/hathach/tinyusb/blob/master/src/osal/osal_freertos.h#L123

[cr1901]

@rajivr Has there been any forward progress with your TockOS port? I'm looking forward to seeing what you've come up with! :)

[hathach]

Introduce tud_task_is_queue_empty() function. The patch is here We introduced this function because in the OS main loop, we needed some way to determine when TinyUSB did not have any more work left to do, so we can safely execute wfi instruction

@rajivr my bads,I finally understand the problems with WFI, this is new to me, Your suggestion is spot-on, and it is indeed needed to safely execute wfi() . Here is the issue with tinyusb with wfi issue on circuitpython adafruit/circuitpython#2868

Update: implemented in #414 the tud API got name changed from the suggestion tud_task_is_queue_empty() since we never tell there is a queue inside a task from application pov