tinyvision-ai-inc / pico-ice

Raspberry Pi PICO board + Lattice iCE40 FPGA's
MIT License
142 stars 26 forks source link

Modified main.c to enable uart1 when UART1_CDC is set #45

Open MrJake222 opened 4 months ago

MrJake222 commented 4 months ago

Fixes #44

MrJake222 commented 4 months ago

also encountered a weird bug that linux kernel failed to initialize the device if CFG_TUD_CDC was set to 4

[20399.979950] usb 1-2: new full-speed USB device number 65 using xhci_hcd
[20400.107901] usb 1-2: config 1 has an invalid descriptor of length 0, skipping remainder of the config
[20400.108892] usb 1-2: New USB device found, idVendor=1209, idProduct=b1c0, bcdDevice= 1.00
[20400.108909] usb 1-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[20400.108946] usb 1-2: Product: pico-ice
[20400.108953] usb 1-2: Manufacturer: tinyVision.ai Inc.
[20400.108958] usb 1-2: SerialNumber: DE6268F24F3D6C21
[20400.116983] usb 1-2: can't set config #1, error -32
josuah commented 4 months ago

There are also other fields to modify if wanting to increase the number of CDC interfaces. I tried to summarize them here: https://pico-ice.tinyvision.ai/group__ice__usb.html#autotoc_md1

  1. Define ICE_USB_UART0_CDC or ICE_USB_UART1_CDC to the CDC interface number to use.
  2. Adjust these as needed: ITF_NUM_CDCx, ITF_NUM_DATAx, CFG_TUD_CDC, TUD_CDC_DESCRIPTOR, STRID_CDC+x

Hopefully this is just this...

MrJake222 commented 4 months ago

Oh I assumed they're used for different things, thanks.

What about the changes I propose?

josuah commented 4 months ago

I am split between making the default firmware more configurable, or instead using separate examples with no #ifdef, keeping all the configuration in the SDK.

The goal is to avoid a lot of cascaded configurations to keep TinyUSB config as direct as possible, to help with troubleshooting.

I will be able to come back to it mid-week, is it ok? First trying an example with both UART enabled at once, which will show the needed configuration, this should help discovering what is missing here...

Many thanks for pointing this out! Maybe having both UARTs enabled by default is the way to go!

josuah commented 4 months ago

also encountered a weird bug that linux kernel failed to initialize the device if CFG_TUD_CDC was set to 4

Since you removed CDC SPI, maybe you need to keep it to 3 instead of incrementing it to 4: Removing one CDC ACM interface, adding one CDC ACM interface => keeping the same config?

MrJake222 commented 4 months ago

Since you removed CDC SPI, maybe you need to keep it to 3 instead of incrementing it to 4

Yes, that's my current config. It works :)

Many thanks for pointing this out! Maybe having both UARTs enabled by default is the way to go!

I find it useful, because I define one UART block in FPGA for debugging (mainly programming the SPRAM) and the other for regular I/O.

josuah commented 4 months ago

Ah right, that sounds like the way forward for everyone working with UART as a debug protocol as well as control protocol!

I think this will be appreciated: a single UART core to write, and reusing it across for various communication purposes. Maybe debugging/controlling two different cores at once that way.