Implement EPx-only RP2040 USB Host

[rppicomidi]

This commit fixes issue #2776
#2776.
The HCD now routes all USB transfers through the EPx endpoint. It no longer uses the "interrupt" endpoint hardware to handle INTERRUPT and BULK endpoints. The fix avoids the data sequence error handling bug in the RP2040 USB IP's "interrupt" endpoint hardware
and allows the host to correctly drop the IN packet with the error without locking up. That fixes
rppicomidi/usb_midi_host#14

This fix requires the CPU to handle the SOF interrupt (every ms). That might be an issue for some systems.

A benefit of this fix is that BULK transfers are
more than 2x faster. There is an opportunity to speed them up further by forcing BULK transfer to begin
immediately instead of waiting for the next SOF interrupt.

This code has been tested with MSC flash drives, HID devices, and USB Hubs. It works with full speed and low speed HID devices connected at the same time through a hub. With the usb_midi_host application host driver, 4 MIDI devices plugged to a hub can send messages to each other (see the [midi2usbhub](https://github.com/rppicomidi/midi2usbhub project).

Describe the PR
See description, above

Additional context
rppicomidi/usb_midi_host#14
raspberrypi/pico-feedback#394

[rppicomidi]

@hathach The hardware test seems to fail because the flash download failed. Is that a problem with the build system? Flash download worked fine on my system.

[hathach]

that is my bad, I was migrating HIL machine from pi5 to an VM. Merged with latest should fix this. Please give me a bit of time, I will review and test this as soon as I could

[shreeve]

Anything helpful in this young repo? => https://github.com/shreeve/picousb

[rppicomidi]

@shreeve Helpful to this code, or to picousb? My testing shows that this code works better than the TinyUSB host controller driver code for the RP2040 that is currently in TinyUSB. It works around a RP2040 hardware issue and it makes bulk transfers faster than what you get with the current HCD. Do you see an area for improvement in this pull request?

[shreeve]

@rppicomidi - It's hard for me to grok the code since it is very compact and there is a lot going on. I've started the other project to try to create a simpler and dynamic version of USB Host. Ha Thach has done an AMAZING job with TinyUSB though and has had a huge impact. My small little library is primarily nowhere near close to a fraction of what he has done, but it is a small effort to understand how the rp2040 USB Host mode works and hopefully it will inspire someone else. Thank you for your work on this patch!

[ctag-fh-kiel]

The PR works on my test system.

[ctag-fh-kiel]

that is my bad, I was migrating HIL machine from pi5 to an VM. Merged with latest should fix this. Please give me a bit of time, I will review and test this as soon as I could

is still PR still on the test list? would be great, thx :)

[rppicomidi]

@hathach I pushed some changes to remove dead code and make this HCD even faster. Even if you don't care about the original data sequence error bug, the speed improvement should make this worth the time to merge. I do understand that the pull request is complicated, that it will take time to review, and that you are very busy, so I will remain patient if you do not have time. Thank you for this project.

[wbcbz7]

Hello everyone, and thank you all for the effort of speeding up USB host on the Raspberry Pi Pico! Unfortunately, after testing the proposed re-implementation of the RP2xxx HCD, I've found a couple of bugs and potentional issues that would be nice to have fixed:

• This code assumes that there can be only one USB hub attached (see https://github.com/rppicomidi/tinyusb/blob/f981f5c9fdd8ea227650f145af43829d836c1534/src/portable/raspberrypi/rp2040/hcd_rp2040.c#L113), while in fact the number of hubs is defined by CFG_TUH_HUB in tusb_config.h
• The epx endpoint structure is simultaenously used for storing control transfer data and an "actual" EPx endpoint storage for performing all transfers. During my tests of simultaneous USB Mass Storage and HID devices, it turns out control transfers can get sometimes corrupted by current EPx transfer control structures, and vice versa, so it makes sense to store current EPx info in the separate hw_endpoint structure, and store endpoint_control/buffer_control for ep_pool somewhere in the RAM instead of DPRAM.
• The RP2040 seems to always auto-retry bulk/interrupt transfers if NAK is received, without any ability of raising the IRQ or stopping the transfer - for example, Mass Storage devices can "throttle" the transaction this way until data is ready for transmission. This not only has a negative impact on the available USB bandwidth (as the HCD cannot schedule next transaction until at least SOF interrupt is raised, so it can observe the IRQ status there), but can also effectively halt the USB bus if (badly designed) device is stuck in an NAK loop. Moreover, one cannot just drop the packet solely on SIE_STATUS.NAK_REC flag, as it only means that the current transaction was NAKed at least once. At the same time, if it was eventually ACKed, the USB controller also raises the INTR.BUFF_STATUS flag (with additional SIE_STATUS.TRANS_COMPLETE if it was the last packet of a transaction). As for your patch, the ISR unconditionally drops data for non-control transfers if SIE_STATUS.NAK_REC if set, and this results in current transaction being "frozen". RP2350 adds a feature to stop current transaction if NAK is received, optionally raising the IRQ, so there should be no such issues, but for RP2040 some sort of a workaround should be provided.
  UPD: so far the correct NAK handling (which do not lead to Data Sequence errors) is to ignore NAK status for bulk transfers if BUFF_STATUS was asserted, and drop data on NAK for interrupt.

I really hope these issues will be addressed and resolved, as this patch already shows a vast improvement on bulk transactions on the RP2040 USB Host - my tests already show around 1.0-1.1 MBytes/second for reading 4K/8K sectors from the USB stick, so it would be awesome to have this patch fixed and eventually merged to the mainstream TinyUSB. Thanks in advance! :)

[rppicomidi]

@wbcbz7 Thank you for taking the time to review this in detail.

This code assumes that there can be only one USB hub attached

Yes, that is because multiple hub support is a relatively new feature. I will address that.

During my tests of simultaneous USB Mass Storage and HID devices, it turns out control transfers can get sometimes corrupted by current EPx transfer control structures, and vice versa

Can you post evidence of that happening? For example, a USB sniffer trace, a log, or a file diff? It will help me root cause the issues you are seeing.

The RP2040 seems to always auto-retry bulk/interrupt transfers if NAK is received, without any ability of raising the IRQ or stopping the transfer

Auto-retry is a "feature" of the RP2040 chip for Control endpoint transfers that I can do nothing about. This HCD has to assume Control endpoint transfers are rare. I agree that the RP2350 has a workaround feature for this, but this is a RP2040 HCD. If that gets merged, I (or you) can implement a RP2350 HCD.

The RP2040 will also auto-retry Bulk endpoint transfers; however, the workaround for that is supposed to be to configure Bulk endpoint transfers as Interrupt endpoint transfers; Interrrupt endpoint transfers to EPx are not supposed to auto-retry after NAK. I thought I already did that, but if not, I will fix it.

[wbcbz7]

Can you post evidence of that happening? For example, a USB sniffer trace, a log, or a file diff? It will help me root cause the issues you are seeing.
I'll try to hook an logic analyzer and dump a trace at weekends, so far you can try to reproduce a bug this way:

• Clone a host_cdc_msc_hid example from Pico-SDK, then add a simple routine to msc_app.c which will repeatedly read or write data from the mass storage device (i.e. from this gist), then compile it.
• Plug a USB hub to the Pico USB port, then plug any USB Mass Storage device (e.g. an USB stick) to the hub, then turn the Pico on and watch for messages in UART - if all is ok you should see a R (or W if write is enabled) characters repeatedly sent for each transfer complete.
• Plug any other USB device (i.e. a HID mouse) to the hub, then make sure it will send data on HID report requests - for instance you can wilggle the mouse to make it report the movement data.
• Try plugging other devices in during a transfer, or disconnecting them, or rebooting the Pico with multiple devices connected to the hub.

This code seems to work fine with the mainline TunyUSB, but with your branch, all transfers, including MSC reads/writes, ocassionally stop until reboot after new device being connected, or an assert in sync_ep_buffer is triggered (example log):

TinyUSB Host CDC MSC HID Example
[0:] USBH Device Attach
[0:] USBH Device Attach
A MassStorage device is mounted
A device with address 1 is mounted
         USB DISK 2.0     rev PMAP
Disk Size: 7389 MB
Block Count = 15132672, Block Size: 512
RRRRRRR[0:] USBH Device Attach
Rassertion "!(buf_ctrl & USB_BUF_CTRL_FULL)" failed: file "D:/SRC/pico2/!test_crap/tinyusb_rppicomidi/src/portable/raspberrypi/rp2040/rp2040_usb.c", line 231, function: sync_ep_buffer
TinyUSB Host CDC MSC HID Example
[0:] USBH Device Attach
[0:] USBH Device Attach
A MassStorage device is mounted
A device with address 1 is mounted
         USB DISK 2.0     rev PMAP
Disk Size: 7389 MB
Block Count = 15132672, Block Size: 512
RRRRRRR[0:] USBH Device Attach
R

(please ignore the pico2 in source file path - this code was compiled for the RP2040 and tested on the Pi Pico)
You can change the MSC_BLOCK_COUNT to select smaller or larger transfer sizes - it also appears that multi-USB-packet bulk transfers gets uninterrupted by i.e interrupt transfers by HID requests, so if one sets a repeating large (>16-32 kB) transfers, the HID polling rate will be negatively impacted, but I suppose that's by the current HCD driver design.

I've been tinkering with your branch's code in a local repo - unfortunately I've first merged it with latest TinyUSB by manually copying the driver files so original commit history is lost. If you're interested, I can put it online :)

[rppicomidi]

@wbcbz7 Thank you for the additional information. I will try your experiment.

By design, the HCD first tries pending Control transfers, then pending Interrupt transfers, and only if there is any time left in the frame does it try pending Bulk transfers. If you are seeing large bulk transfers slowing down HID interrupt polling rate, then there is a problem. Control transfers that NAK for a long time could potentially slow down the polling interval because RP2040 auto-retry cannot be disabled for Control endpoints, but Bulk transfers should not.

The function that tells the RP2040 that both Interrupt and Bulk transfers are to be handled as Interrupt endpoint transfers is _hw_setup_epx_from_ep() (around line 288 in hcd_rp2040.c). This function is called before using the EPx hardware to do a transfer. Do you have evidence that the RP2040 is auto-retrying Bulk or Interrupt transfers? I use usb-sniffer-lite for USB traces and will test on my system with your example code also.

[rppicomidi]

@wbcbz7 I have addressed the hub issue and have done a git merge with the latest TinyUSB and pushed it up. I am trying your test code now and find no evidence of issues yet. I am using a powered 4-port hub, a USB mouse plus 2 USB flash drives. Is there a particular block size or blocks per transfer number that causes issues in your testing? Is it possible that the merged code you created introduced some other issue?

[wbcbz7]

@rppicomidi, I've just linked the test application with your latest commit, and while it seems to bie slightly more stable, it still appears to occasionally hang during bulk transfers if control transfers are executed concurrently. I've tried usb-sniffer-lite to capture USB frame data but even with an external trigger it seems to not have enough buffer memory to capture the exact moment of hang, so I had to revert back to my logic analyzer. Attached there are two PulseView sessions with bulk in transfers gets intervened by control transfers, which leads to former being stuck - i've also attached USB packet text annotations exported.
It actaully seems that NAKed bulk transfers can be in fact preempted by other transfers, but for some reason it doesn't play well. There is one notable moment in the second capture - first SETUP transfer at 307ms leads to successive bluk transfer being stuck after a while (at 338 ms). Meanwhile, at 760ms, another SETUP packet is sent, and then there seems to be another bulk transfer being scheduled. For some reason, it seems to also end up in a "stuck" state, as there seems to be no further bulk transfers scheduled. Hope these dumps will be useful for you.

rp2040_tinyusb_bulk_hang_after_setup_2.zip
rp2040_tinyusb_bulk_hang_after_setup.zip

[bflorac]

I installed the changed files in a PICO 2W with FreeRTOS (running on 2 cores) and it does seem to prevent the run away NAKs.
I had to make sure all the USB related tasks (including the routine that calls tusb_init() were on the same core (I used core 1) or it got a panic in function hw_endpoint_lock_update() of rp2040_usb.c.