[usbdev] aon_wake maintains pull up assertion over VBUS disconnection

[alees24]

Description

Re Disconnection/Interruption to VBUS/SENSE when OT is in Deep Sleep (Imperfection rather than fault)

In the event that aon_wake has control of the pull ups and VBUS/SENSE is removed for a period, the pull up remains asserted and will be driven at the point of VBUS reappearing. If the host controller has spotted the disconnection because D+ was also affected (no longer pulled up) then it will discover the device presence and attempt to configure the device.

If this same interruption happens whilst usbdev has control, the pull up is removed when VBUS is lost (in usbdev_usbif qualifies the assertion of usb_pullup_en_o with the presence of usb_sense_i) and software can be expected to respond promptly and deassert the pull up, await the return of VBUS and then go through the normal startup/configuration sequence again.

In chip sim, with current test code, an observed delay of 2ms occurs between VBUS disconnection and software being ready to access/reconfigure usbdev. A shorter interruption could lead to unintended/unexpected behavior, although it's likely that the host controller will just keep retrying.

Suggest when software returns from Deep Sleep and discovers a Disconnection event, it should be aware that the host may or may not have spotted a disconnection, and thus introduce a deliberate disconnect period by ensuring that usbdev pull up is disabled before deactivating the aon_wake module. Only after a deliberate delay should the pull up then be enabled to indicate presence; TBC - this delay need only be of the order of microseconds to milliseconds, not even tens of milliseconds, and only in the case of a Disconnection having been reported.

Re future RTL, I would suggest these pull up assignments:

assign aon_dppullup_en_d = wake_detect_active_q ? aon_dppullup_en_q : usbdev_dppullup_en_aon; assign aon_dnpullup_en_d = wake_detect_active_q ? aon_dnpullup_en_q : usbdev_dnpullup_en_aon;

should be: assign aon_dppullup_en_d = wake_detect_active_q ? (aon_dppullup_en_q & ~event_sense_lost) : usbdev_dppullup_en_aon; assign aon_dnpullup_en_d = wake_detect_active_q ? (aon_dnpullup_en_q & ~event_sense_lost) : usbdev_dnpullup_en_aon;

Pull up(s) that are asserted whilst the module is active will then become deasserted when the (filtered) sense signal disappears and the module decides to request wakeup.

[moidx]

@GregAC to assign priority and identify potential workarounds with @alees24.

[GregAC]

Following discussion with @alees24 there is no urgency with this change. It is nice to have but isn't a concern for M2.5.1

[moidx]

Moving this to M2.5.Backlog so that we can pick it up in a future release.

[alees24]

After a bit more thought I think even the software workaround can be largely zero-impact. Upon waking from sleep and discovering a report of a Disconnection, check for the presence of VBUS/SENSE and immediately disconnect the pull up.

If VBUS/SENSE was discovered to be absent - the usual case, since the most probable cause is becoming physically unplugged - there is no further action required; we will just be notified in the event of any subsequent VBUS assertion.

If VBUS was asserted, then we could have detected a harmless interruption of VBUS from an electrical problem/similar, but we cannot trust the connection, and must de-assert the pullup for a long enough interval for the host/hubs to recognize a reconnection.

I think it's still worth including the proposed hardware modification in a subsequent revision of the RTL (once tested, but as far as I can see,it's robust).

[johngt]

Covered by draft PR - https://github.com/lowRISC/opentitan/pull/19270 Leaving open until that is merged. Setting effort spent such that remaining is 0.

[andreaskurth]

Current status captured in the D2 signoff issue:

• [usbdev] aon_wake maintains pull up assertion over VBUS disconnection #18562
  • draft PR DRAFT [usb_aon] Deassert pullups if SENSE lost when active #19270 non-essential, software could work around.
  • open related PR [usb,aon_wake] Report 'bus not idle' status to usbdev #22019 more important; presents software with additional information to permit work around.

@alees24: Now that #22019 has been merged and thus AFAIU the SW workaround is possible, do you plan to also complete #19270 for M2 or should that come in later? Put differently, should this issue stay in M2 or be postponed to M4?

[alees24]

My feeling is that none of us has really worked out the full set of possible event sequences to be sure that the hardware behavior change would overall be an improvement. Hopefully we'll get a chance to think fully about this and test it properly before M4, and if the hardware is changed it can be regarded as a bug fix; it would not necessitate changes to the register API etc, just annotation in the programmer's guide documentation.

[andreaskurth]

@andreaskurth to check this in light of CDC investigations

[andreaskurth]

@johngt to raise this in discussions next Monday

[andreaskurth]

discussed at the SiVal meeting, still needed for M4; Adrian is on it

[moidx]

Marking this as a future release as we won't be able to land an RTL change for Earlgrey-PROD.M4.

@a-will: Response time after wake up is in the order of 100s of milliseconds due to the specification of V-BUS.