-
Notifications
You must be signed in to change notification settings - Fork 914
[rescue,test] enhance rescue error handling tests #28789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: earlgrey_1.0.0
Are you sure you want to change the base?
[rescue,test] enhance rescue error handling tests #28789
Conversation
5350817 to
655f78e
Compare
| opentitan_test( | ||
| name = "usbdfu_in_transaction_cancel", | ||
| exec_env = { | ||
| # Unsupported on cw310: USB control request with a minimal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on this? As far as I know, there is no difference at the IP or SW level between the cw310 and the cw340 so it's not clear to me what is "unsupported" on the cw310?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test aims to verify the USB IN transaction cancellation handling on the device side:
opentitan/sw/device/silicon_creator/lib/drivers/usb.c
Lines 333 to 335 in 382e702
| if (bitfield_bit32_read(reg, USBDEV_CONFIGIN_0_PEND_0_BIT)) { | |
| // Was there a cancelled transaction? | |
| error = kUsbTransferFlagsError; |
rusb::DeviceHandle to trigger a USB IN transaction cancellation event.
As a workaround, I implemented an approach where I issue a USB control request with a very short timeout (1ms) from the host side. In my development environment (cw340 board), this successfully triggers the desired corner case, and the device-side cancellation logic is exercised.
However, this test fails in the CI environment (cw310). It appears that for the cw310, this workaround of a minimal timeout request from the host doesn't lead to the device considering the transaction as cancelled. While I don't have deep insight into the hardware differences, I suspect this might be due to subtle timing differences and decided to only enable this test on cw340 platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've observed that this test is flaky in the CI, even on the cw340 platform. I'm removing it from this PR for now and will investigate a more reliable way to cover this test scenario.
655f78e to
9176a8e
Compare
This commit expands the end-to-end tests for rescue error handling: - Adds a rescue_image_too_big test for Xmodem to check the handling of oversized firmware images during rescue, ensuring the device correctly cancels the operation and remains functional. - Modifies the `usb_dfu_out_chunk_too_big` test to first perform a successful chunk download before attempting an oversized one, improving test robustness. Signed-off-by: Anthony Chen <[email protected]>
9176a8e to
2b4f7f0
Compare
This expands end-to-end tests for rescue error handling to increase test coverage:
usb_dfu_out_chunk_too_bigtest to first perform a successful chunk download before attempting an oversized one, improving test robustness.