lowRISC / lowRISC/opentitan

[usbdev] aon_wake maintains pull up assertion over VBUS disconnection

Open
#18,562 11 comments 0 reactions 1 assignee Claimed by @alees24 View on GitHub
IP:usbdev Type:Enhancement Type:FutureRelease
Dominant language
SystemVerilog
Stars
3.6k
Forks
1.1k
Avg merge
2d 22h
Merged PRs (30d)
141

Description

### 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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.