On 10/12/2024 19:13, Théo Lebrun wrote: > The XHCI_RESET_ON_RESUME quirk allows wrappers to signal that they > expect a reset after resume. It is also used by some to enforce a XHCI > reset on resume (see needs-reset-on-resume DT prop). > > Some wrappers are unsure beforehands if they will reset. Add a mechanism > to signal *at resume* if power has been lost. Parent devices can set > this flag, that defaults to false. > > The XHCI_RESET_ON_RESUME quirk still triggers a runtime_pm_get() on the > controller. This is required as we do not know if a suspend will > trigger a reset, so the best guess is to avoid runtime PM. > > Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx> > --- > drivers/usb/host/xhci.c | 3 ++- > drivers/usb/host/xhci.h | 6 ++++++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 5ebde8cae4fc44cdb997b0f61314e309bda56c0d..ae2c8daa206a87da24d58a62b0a0485ebf68cdd6 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -1017,7 +1017,8 @@ int xhci_resume(struct xhci_hcd *xhci, pm_message_t msg) > > spin_lock_irq(&xhci->lock); > > - if (hibernated || xhci->quirks & XHCI_RESET_ON_RESUME || xhci->broken_suspend) > + if (hibernated || xhci->quirks & XHCI_RESET_ON_RESUME || > + xhci->broken_suspend || xhci->lost_power) > reinit_xhc = true; > > if (!reinit_xhc) { > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > index 4914f0a10cff42dbc1448dcf7908534d582c848e..32526df75925989d40cbe7d59a187c945f498a30 100644 > --- a/drivers/usb/host/xhci.h > +++ b/drivers/usb/host/xhci.h > @@ -1645,6 +1645,12 @@ struct xhci_hcd { > unsigned broken_suspend:1; > /* Indicates that omitting hcd is supported if root hub has no ports */ > unsigned allow_single_roothub:1; > + /* > + * Signal from upper stacks that we lost power during system-wide > + * suspend. Its default value is based on XHCI_RESET_ON_RESUME, meaning > + * it is safe for wrappers to not modify lost_power at resume. > + */ > + unsigned lost_power:1; I suppose this is private to XHCI driver and not legitimate to be accessed by another driver after HCD is instantiated? Doesn't access to xhci_hcd need to be serialized via xhci->lock? Just curious, what happens if you don't include patch 4 and 5? Is USB functionality still broken for you? Doesn't XHCI driver detect that power was lost and issue a reset in any case via the below code /* re-initialize the HC on Restore Error, or Host Controller Error */ if ((temp & (STS_SRE | STS_HCE)) && !(xhci->xhc_state & XHCI_STATE_REMOVING)) { reinit_xhc = true; if (!xhci->broken_suspend) xhci_warn(xhci, "xHC error in resume, USBSTS 0x%x, Reinit\n", temp); } > /* cached extended protocol port capabilities */ > struct xhci_port_cap *port_caps; > unsigned int num_port_caps; > -- cheers, -roger