Add Rx and interrupts to UsbSerialJtag

[raiker]

This is a first cut of adding USB serial Rx and the ability to trigger interrupts when packets are received. I have attempted to make the interface as similar as possible to regular serial UART where it makes sense.
This does include a breaking change - now you actually have to create an instance of UsbSerialJtag with a peripheral instead of just using the registers directly.
I have updated the esp32c3 and esp32s3 examples to demonstrate some of the new features and tested the change on my ESP32-C3-DevKit-RUST-1. I don't have an esp32s3 handy to test its example, but it at least builds. Would appreciate if someone else could test it for me please.
To note:

• I have slightly changed the name of the Rx interrupt from "serial_out_recv_pkt" to "rx_packet_recv" because the TRM names them from the point of view of the USB host and I find it really confusing.

Possible future work:

• It looks like the esp32s3 has the ability to map the USB D+ and D- lines to arbitrary GPIO pins using the GPIO matrix. I don't have any way to test that so I've left it for now.
• I also didn't implement the Tx interrupt because I'm not sure of its utility.

Would love to hear if anyone has any useful feedback on this change.

[bjoernQ]

Thanks for your contribution

Code looks fine to me - any specific reason implementing Read from embedded-hal but not Write ?

My testing on ESP32-C3 went also fine but not on ESP32-S3

• it starts printing Hello World! repeatedly
• if I enter two characters on ESP32-S3 in espflash --monitor is freezes - I cannot even use CTRL-C or CTRL-R in espflash at that point and need to unplug the ESP32-S3

I'm also not sure if USB_DEVICE is the right interrupt - maybe USB_INTR (38) but that is currently not in the PAC as it seems

BTW the problem with freezing the application is also present on S3 with current main branch - so it's not really something you introduced

[raiker]

Thanks for taking a look at my pull request @bjoernQ. The missing embedded_hal::serial::Write implementation was an oversight, so I have added a commit with that.
Non-blocking interacts a little unusually with the USB serial since the hardware doesn't flush its Tx FIFO until it fills (64 bytes). There's a flush() method on the trait for that, but it isn't called automatically at the end of an operation. It fits the API but the behaviour is a bit unusual.
As for the ESP32-S3... I suspect you're right with the IRQ assignment. From your description it's unclear to me where the bug lies, whether it's in esp-monitor, esp-hal, the example or the hardware. Perhaps multiple. I understand if you want to hold off merging this change until you work out what's going on there.

[bjoernQ]

There is an issue regarding the problem on ESP32-S3 now: #269

[bjoernQ]

@jessebraham any preference how to handle this?

I see basically two options

• remove the Rx part of the S3 example, add a note that Rx doesn't work for S3 with a link to the issue and merge it
• try to get ESP32-S3 crashes when connected via Serial/Jtag port and sending two bytes of data #269 fixed first and keep this unmerged until then (I looked into ESP32-S3 crashes when connected via Serial/Jtag port and sending two bytes of data #269 but have no idea what is causing that)

[jessebraham]

I think either approach is probably fine. If we're unsure of what the underlying issue is then I don't think we should hold up on merging the changes if they're otherwise working on the other device. As you said we can just link the issue in a comment or something and update it when we're able.

[bjoernQ]

Agree - let's get this merged

Seems #271 causes merge conflicts - also maybe revert the S3 example changes or add a comment with a link to the issue - then it should be fine

Thanks for the effort

[raiker]

Sorry for the delay.
I've rebased on top of main and added a comment with a link to the issue to the esp32s3 example. In the process I inadvertently closed the pull request; I think I've got it into a mergeable state again.
#271 is completely subsumed by my change, apologies to TheButlah

[bjoernQ]

LGTM - thanks for the contribution