lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.57k stars 771 forks source link

[CI] Add USB tests to FPGA CI #4743

Open imphil opened 3 years ago

imphil commented 3 years ago

Add tests to CI which utilize the USB peripheral on an FPGA board. See also https://github.com/lowRISC/opentitan/pull/4330

alistair23 commented 3 years ago

Just a note that USB is still broken on the FPGA, see https://github.com/lowRISC/opentitan/issues/2598 and https://github.com/lowRISC/opentitan/issues/3388

GregAC commented 2 years ago

@a-will any thoughts on this?

a-will commented 2 years ago

The referenced issues are obsolete at this point, but it would definitely be nice to get CW310's OT USB devices hooked up in CI. Initially, we would likely reproduce the usbdev_test on FPGA (use the OT USB as a couple UARTs and send / receive data). We'll probably want more coverage at some point, though.

Tests will need the CW310 physically connected, and if the FPGA runner uses containers, we'll need to figure out a good strategy for passing through the devices. For the case of multiple boards connected to a single host, most likely the physical bus tree would need to be examined to associate FPGA USB devices with CW310 controller USB devices (so everything related to one board goes to the right place).

alees24 commented 1 year ago

estimate 8

moidx commented 1 year ago

Moving to CI bucket

drewmacrae commented 1 year ago

Is this why the //sw/device/tests:usbdev_test_fpga_cw310_test_rom test fails when I run it, and should it be marked as broken or tagged to reflect an unfulfilled hardware requirement like we do for the jtag tests?

alees24 commented 1 year ago

Is this why the //sw/device/tests:usbdev_test_fpga_cw310_test_rom test fails when I run it, and should it be marked as broken or tagged to reflect an unfulfilled hardware requirement like we do for the jtag tests?

It requires the DPI model and operates only in Verilator t-l simulation. The test is tagged as 'skip_in_ci' but I added the cw310_test_rom target because I do sometimes run it manually on FPGA. Is the 'skip_in_ci' tag incorrect or inadequate?

a-will commented 1 year ago

@drewmacrae For the FPGA variant to work, that test needs some host code to drive it--Specifically, it expects to see "Hi!Hi!" over the USB serial interface.

@alees24 Because that folder is intended for automated tests only, we should probably remove the target to avoid confusion. skip_in_ci only helps CI, but our teammates use these targets locally as well. Instead, perhaps consider using the very similar //sw/device/examples/hello_usbdev for non-automated purposes. We can add the FPGA target back once host code is available.

Edit: @cfrantz suggested that if we are planning to create that host code for this test, we can add the manual tag for now.

alees24 commented 1 year ago

I'd prefer that it be tagged as manual then; I think usbdev_test serves as a useful smoke test and that it should become a usable regression test both in FPGA and in sim.

johngt commented 1 year ago

From @alees24 effort estimate 8 integration of t-l tests into CI, with host app <..8>

estimate 2