shared/tinyusb: Add common cdc tx/rx functions.

[andrewleech]

There are a few tinyusb CDC functions used for stdio that are currently replicated across a number of ports.
Not surprisingly in a couple of cases these have started to diverge slightly, with additional features added to one of them.

This PR consolidates a couple of key shared functions used directly by tinyusb based ports.

[github-actions]

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:   +16 +0.004% TEENSY40[incl -8(bss)]
        rp2:    +4 +0.001% RPI_PICO[incl -4(bss)]
       samd:   +28 +0.011% ADAFRUIT_ITSYBITSY_M4_EXPRESS

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.39%. Comparing base (3613ad9) to head (c11efc7).

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #14462   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         161      161           
  Lines       21204    21204           
=======================================
  Hits        20864    20864           
  Misses        340      340           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[projectgus]

This is really useful, thanks @andrewleech! One small change suggestion, but I think we should merge it with or without the additional name change.

[andrewleech]

Note: I've only been able to test this on RP2, I don't own samd / mimxrt / renesas-ra boards.

edit: I've got my hands on samd, mimxrt and renesas-ra boards ( thanks @mattytrentini ) but haven't been able to figure out how to enable usb on renesas yet... renesas#2

So now rp2, samd21, samd51, mimxrt have been tested and appear to work as expected!

[projectgus]

Note: I've only been able to test this on RP2, I don't own samd / mimxrt / renesas-ra boards.

It may be redundant as I saw the follow-on PR has been tested on samd already, but I've tested this PR on samd and it seems to work as expected. 👍

[andrewleech]

Note: I've only been able to test this on RP2, I don't own samd / mimxrt / renesas-ra boards.

It may be redundant as I saw the follow-on PR has been tested on samd already, but I've tested this PR on samd and it seems to work as expected. 👍

Thanks I've just updated my previous post to reflect what I've now been able to test! Regardless, I certainly appreciate any additional testing as I'm not very familiar with some of these ports!

FYI thanks to this incredibly helpful tidbit of info, I'm also looking at bringing esp32s2 and s3 into using this same shared tinyusb implementation, as well as fixing the auto-bootloader jump there for S3 which is currently incredibly frustrating!
hathach/tinyusb#2647 (comment)
That should also make it easier / possible to get the runtime usb definition stuff working there too!

It's probably worth saving the esp stuff for a follow up PR though, it's a larger parcel of work though!

[projectgus]

@andrewleech Oh wow, that's fantastic news! I had anticipated this being a much bigger task, very exciting.

[andrewleech]

So now I've tested everything here except the renesas port changes.

Can anyone provide guidance on how to update the auto generated files in a renesas board (in particular the interrupt vector header) to enable USB on a evk board? @TakeoTakahashi2020 @dpgeorge

Alternatively @iabdalkader could you perhaps test this PR doesn't break anything on a Portenta C33 (I can really afford one myself for testing with).

[iabdalkader]

@andrewleech Yes I can test it, perhaps later today. What do I need to test ?

[andrewleech]

What do I need to test ?

Thanks, on this PR there's no intended functional change so it's just a basic check that stdio / repl still seems to work.

If possible, the bigger change is in the follow on PR #14485
For that one is checking at startup whether the banner is buffered and shown on initial connection. The description there should show pretty clearly.

If you know, I'd love some pointers on how to create (or better, modify) the auto generated code in a renesas board so I can test / document it myself too.

[iabdalkader]

it's just a basic check that stdio / repl still seems to work

REPL is still functional on the Portenta C33.

If you know, I'd love some pointers on how to create (or better, modify) the auto generated code in a renesas board so I can test / document it myself too.

You'll need to install the e2studio, load the configuration, edit and then generate the code. For this you need the configuration.xml, I think it's this one: configuration.xml.zip

[andrewleech]

You'll need to install the e2studio, load the configuration, edit and then generate the code. For this you need the configuration.xml, I think it's this one: configuration.xml.zip

Ok thanks! I previously installed RA Smart Configurator which can open a "project", not sure if that will be the XML. I'll try tomorrow.

I really think this XML/project file needs to be included in the board folder as the source of the auto gen files.

[andrewleech]

REPL is still functional on the Portenta C33.

That's great thanks!

[iabdalkader]

not sure if that will be the XML. I'll try tomorrow.

There's a cfg.txt file too, those are the only two non-generated files.
ra_cfg.txt

[andrewleech]

Thanks @iabdalkader that "configuration.xml" file opened just fine in the standalone "RA Smart Configurator" application!
Looking at how everything for your board was configured in that cleared up a lot for me.
I was then able to start with a basic new configuration for my ek board and enable the USB pins and interrupts in that, following the USB FS configuration of your project as a guide.
Running the "generate files" before and after making my USB config showed only changes to pin and vector files, I was then able to use these changes as a reference to modify the existing EK board generated files to enable USB there. I needed to disable a couple of existing interrupts as that chip only supports 32 user interrupts but that's fine for this test.

That got it going for me, with a cut up usb cable wired to the USB pins and a jumper from 3v to vbus to enable it I had cdc working (quicker than wiring resister divider from USB 5v to vbus pin).

More than just replicating your testing on this PR, this more importantly helped me finish off the updates in tinyusb to support my startup buffer feature.

Thanks again! I'll try to get a more succinct version of these notes into a readme update pr to help the next person. Would you mind me also including your configuration.xml in that pr (in the matching board folder) to reference as an example?

[iabdalkader]

Would you mind me also including your configuration.xml in that pr (in the matching board folder) to reference as an example?

No please go ahead. The configuration should have been there with the board files.

[andrewleech]

I found a further consolidation that made sense in moving more of the cdc stdin handling into the mp_usbd_cdc_poll_interfaces() function, de-duplicating this code everywhere else.

With that change, I've restested on all platforms covered here and all appear to be working (basic read/write on repl and mpremote mount still works).

I previously had nrf also included here but it was a larger change which grew even larger as I tested and cleaned it up so moved it to its own PR.

With this, I consider this PR complete and ready for final review.

[dpgeorge]

Thanks, this looks really good. It was always on my list to eventually factor these functions, and I'm glad someone else got to it first :)

[ricksorensen]

Using pull from 31 may 2024, and master branch, NRF52840 does not compile with these changes on my system.

It cannot find the ringbuf functions. Looked at the samd mods, and updated:

diff --git a/ports/nrf/mphalport.h b/ports/nrf/mphalport.h
index 7efe05a15..a51f84ef5 100644
--- a/ports/nrf/mphalport.h
+++ b/ports/nrf/mphalport.h
@@ -28,6 +28,7 @@
 #define __NRF52_HAL
 
 #include "py/mpconfig.h"
+#include "py/ringbuf.h"   // RJS
 #include <nrfx.h>
 #include "pin.h"
 #include "nrf_gpio.h"
@@ -42,6 +43,8 @@ typedef enum
 } HAL_StatusTypeDef;
 
 extern const unsigned char mp_hal_status_to_errno_table[4];
+extern int mp_interrupt_char;     //RJS
+extern ringbuf_t stdin_ringbuf;   //RJS
 
 NORETURN void mp_hal_raise(HAL_StatusTypeDef status);
 void mp_hal_set_interrupt_char(int c); // -1 to disable

and

diff --git a/ports/nrf/mphalport.c b/ports/nrf/mphalport.c
index 06c6ba5cc..682ceff80 100644
--- a/ports/nrf/mphalport.c
+++ b/ports/nrf/mphalport.c
@@ -40,6 +40,12 @@
 #include "nrf_clock.h"
 #endif
 
+// RJS 
+#ifndef MICROPY_HW_STDIN_BUFFER_LEN
+#define MICROPY_HW_STDIN_BUFFER_LEN 128
+#endif
+static uint8_t stdin_ringbuf_array[MICROPY_HW_STDIN_BUFFER_LEN];  //RJS
+ringbuf_t stdin_ringbuf = { stdin_ringbuf_array, sizeof(stdin_ringbuf_array), 0, 0 };   //RJS
 #if !defined(USE_WORKAROUND_FOR_ANOMALY_132) && \
     (defined(NRF52832_XXAA) || defined(NRF52832_XXAB))
 // ANOMALY 132 - LFCLK needs to avoid frame from 66us to 138us after LFCLK stop.

My code now compiles (BOARD=SEEED_XIAO_NRF52), and the REPL works with new build.

Note that the 1.23.0 tag compiles and runs without modification.

I do not think these are the optimal patches as I did them simply to resolve link/compile errors based on the samd code... and I have little knowledge of tinyusb. I am particularly concerned with the extern definitions.

[robert-hh]

Not surprisingly, the ARDUINO_NANO_33_BLE_SENSE build shows the same problem. Looking into the code the implementations seems somewhat confusing, because there in Makefile both drivers/usb/usb_cdc.c and lib/tinyusb/src/class/cdc/cdc_device.c are referred. The former uses properly ringbuffers.
So maybe the removal of the preliminary attempt to use the common USB functions failed.

@andrewleech
Edit: It's sufficient to remove the line (188):
tinyusb/mp_usbd_cdc.c \
from Makefile.