lowRISC / opentitan

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

[usbdev] prim_cdc_rand_delay induces sampling errors with usb_fs_rx #23839

Open alees24 opened 1 week ago

alees24 commented 1 week ago

Description

The CDC modeling in prim_cdc_rand_delay has a massive impact upon the ability of the usbdev IP block (specifically the logic in usb_fs_rx) to sample the input data correctly, whether or not the external differential receiver is utilized. Even a small difference between the host and device frequency will lead to long OUT DATA packets being rejected with CRC16 mismatch, and a substantial frequency difference can readily lead to even SOF packets being mis-sampled as the usbdev receiver logic tries to accommodate a frequency mismatch but is actually responding to the variable delay on the DP and DN inputs to the logic.

This impacts non-differential reception too, where there is an external receiver, because the DP/DN lines are still consulted to look for transition states and to attempt to accommodate frequency mismatches.

At the very least this is going to present a problem for V2/3 sign-off, but there is also reason to be concerned if the CDC modeling cannot fairly be considered overly-conservative and/or unrealistic.

With usbdev employing (only?) 4 x oversampling, a cycle delay on just one signal of a DP/DN transition, followed by a cycle delay only on the other signal one bit interval (4 clocks) later, can result in one signal being - say - high for 3 cycles - whilst its counterpart is low for 5 cycles, centred on those 3.

LLLHHHLLL
HHLLLLLHH
andreaskurth commented 1 week ago

Can't we just disable these random additional cycles in this instance of prim_rand_cdc_delay, because the USB protocol mandates delay balancing and clock tuning such that the modeled problems cannot happen -- right?

a-will commented 1 week ago

Can't we just disable these random additional cycles in this instance of prim_rand_cdc_delay, because the USB protocol mandates delay balancing and clock tuning such that the modeled problems cannot happen -- right?

We can sample a transition on DP on a different cycle from DM, but they definitely can't be two cycles away . usbdev specifically requires DP/DM matching within tight enough bounds to make that true (to accommodate T_FST https://github.com/lowRISC/opentitan/blob/84c5f699f6d859b35c1f1c7317dc0a0d36c1ee8e/hw/ip/usbdev/rtl/usb_fs_rx.sv#L96-L102)

If the USB host agent is already playing with clock phases and DP/DM mismatch, then I'd certainly agree with disabling the randomized delay model in prim_rand_cdc_delay.

andreaskurth commented 3 days ago

Just discussed in triage and agreed to move to M7 because there's no evidence of an RTL bug; this is a DV improvement.