When xHCI controller hibernated, the root hub lost power, if controller
support Port Power Control (PPC), PP is not set at xhci_resume() and
set by hub_reset_resume() later, so no need check pending port event.
If PPC is not supported, device is disconneced, seems do not send out
U3 LFPS wake signal, no need re-check again and drop 120ms delay to
save resume time.
Reported-by: Yun-Chien Yu <[email protected]>
Signed-off-by: Chunfeng Yun <[email protected]>
---
drivers/usb/host/xhci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 902f410874e8..686d8e6f03f6 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1235,7 +1235,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
* the first wake signalling failed, give it that chance.
*/
pending_portevent = xhci_pending_portevent(xhci);
- if (!pending_portevent) {
+ if (!pending_portevent && !hibernated) {
msleep(120);
pending_portevent = xhci_pending_portevent(xhci);
}
--
2.18.0
On 9.12.2021 9.22, Chunfeng Yun wrote:
> When xHCI controller hibernated, the root hub lost power, if controller
> support Port Power Control (PPC), PP is not set at xhci_resume() and
> set by hub_reset_resume() later, so no need check pending port event.
> If PPC is not supported, device is disconneced, seems do not send out
> U3 LFPS wake signal, no need re-check again and drop 120ms delay to
> save resume time.
>
> Reported-by: Yun-Chien Yu <[email protected]>
> Signed-off-by: Chunfeng Yun <[email protected]>
> ---
Thanks, adding
-Mathias
Mathias Nyman <[email protected]> 于2021年12月11日周六 01:56写道：
>
> On 9.12.2021 9.22, Chunfeng Yun wrote:
> > When xHCI controller hibernated, the root hub lost power, if controller
> > support Port Power Control (PPC), PP is not set at xhci_resume() and
> > set by hub_reset_resume() later, so no need check pending port event.
> > If PPC is not supported, device is disconneced, seems do not send out
> > U3 LFPS wake signal, no need re-check again and drop 120ms delay to
> > save resume time.
> >
> > Reported-by: Yun-Chien Yu <[email protected]>
> > Signed-off-by: Chunfeng Yun <[email protected]>
> > ---
>
> Thanks, adding
Hi Mathias, Chunfeng
I have a question on this, if there is no any usb devices connected
before suspend, do we need this 120ms delay to check again?
So do we need one more condition to limit this like?
if (!pending_portevent && !hibernated && xhci_has_child_device())
thanks
Li Jun
>
> -Mathias
On 14.12.2021 10.00, Jun Li wrote:
> Mathias Nyman <[email protected]> 于2021年12月11日周六 01:56写道：
>>
>> On 9.12.2021 9.22, Chunfeng Yun wrote:
>>> When xHCI controller hibernated, the root hub lost power, if controller
>>> support Port Power Control (PPC), PP is not set at xhci_resume() and
>>> set by hub_reset_resume() later, so no need check pending port event.
>>> If PPC is not supported, device is disconneced, seems do not send out
>>> U3 LFPS wake signal, no need re-check again and drop 120ms delay to
>>> save resume time.
>>>
>>> Reported-by: Yun-Chien Yu <[email protected]>
>>> Signed-off-by: Chunfeng Yun <[email protected]>
>>> ---
>>
>> Thanks, adding
>
> Hi Mathias, Chunfeng
>
> I have a question on this, if there is no any usb devices connected
> before suspend, do we need this 120ms delay to check again?
> So do we need one more condition to limit this like?
> if (!pending_portevent && !hibernated && xhci_has_child_device())
The 120ms delay was added to make sure we catch the second wake signal
from a device in case host missed the first U3 exit LFPS wakeup signal.
Even if no devices are connected this might be helpful if a device is
connected while host is suspended.
I haven't checked any timing for the link training during enumeration,
but it also uses LFPS signalling, and connected device isn't visible to
driver until link is successfully trained.
So the original 120ms delay patch might as a positive side effect ensure
driver doesn't suspend host mid device enumeration.
Could be looked into more, but I don't think we should this patch by
Chunfeng
Thanks
Mathias
Mathias Nyman <[email protected]> 于2021年12月14日周二 18:03写道：
>
> On 14.12.2021 10.00, Jun Li wrote:
> > Mathias Nyman <[email protected]> 于2021年12月11日周六 01:56写道：
> >>
> >> On 9.12.2021 9.22, Chunfeng Yun wrote:
> >>> When xHCI controller hibernated, the root hub lost power, if controller
> >>> support Port Power Control (PPC), PP is not set at xhci_resume() and
> >>> set by hub_reset_resume() later, so no need check pending port event.
> >>> If PPC is not supported, device is disconneced, seems do not send out
> >>> U3 LFPS wake signal, no need re-check again and drop 120ms delay to
> >>> save resume time.
> >>>
> >>> Reported-by: Yun-Chien Yu <[email protected]>
> >>> Signed-off-by: Chunfeng Yun <[email protected]>
> >>> ---
> >>
> >> Thanks, adding
> >
> > Hi Mathias, Chunfeng
> >
> > I have a question on this, if there is no any usb devices connected
> > before suspend, do we need this 120ms delay to check again?
> > So do we need one more condition to limit this like?
> > if (!pending_portevent && !hibernated && xhci_has_child_device())
>
> The 120ms delay was added to make sure we catch the second wake signal
> from a device in case host missed the first U3 exit LFPS wakeup signal.
>
> Even if no devices are connected this might be helpful if a device is
> connected while host is suspended.
Agree this may also help on this case.
> I haven't checked any timing for the link training during enumeration,
> but it also uses LFPS signalling, and connected device isn't visible to
> driver until link is successfully trained.
>
> So the original 120ms delay patch might as a positive side effect ensure
> driver doesn't suspend host mid device enumeration.
Is this unexpected suspend can be prevented by adding auto suspend
delay?
Thanks
Li Jun
>
> Could be looked into more, but I don't think we should this patch by
> Chunfeng
>
> Thanks
> Mathias
On Tue, 2021-12-14 at 12:05 +0200, Mathias Nyman wrote:
> On 14.12.2021 10.00, Jun Li wrote:
> > Mathias Nyman <[email protected]> 于2021年12月11日周六
> > 01:56写道：
> > >
> > > On 9.12.2021 9.22, Chunfeng Yun wrote:
> > > > When xHCI controller hibernated, the root hub lost power, if
> > > > controller
> > > > support Port Power Control (PPC), PP is not set at
> > > > xhci_resume() and
> > > > set by hub_reset_resume() later, so no need check pending port
> > > > event.
> > > > If PPC is not supported, device is disconneced, seems do not
> > > > send out
> > > > U3 LFPS wake signal, no need re-check again and drop 120ms
> > > > delay to
> > > > save resume time.
> > > >
> > > > Reported-by: Yun-Chien Yu <[email protected]>
> > > > Signed-off-by: Chunfeng Yun <[email protected]>
> > > > ---
> > >
> > > Thanks, adding
> >
> > Hi Mathias, Chunfeng
> >
> > I have a question on this, if there is no any usb devices connected
> > before suspend, do we need this 120ms delay to check again?
> > So do we need one more condition to limit this like?
> > if (!pending_portevent && !hibernated && xhci_has_child_device())
>
> The 120ms delay was added to make sure we catch the second wake
> signal
> from a device in case host missed the first U3 exit LFPS wakeup
> signal.
>
> Even if no devices are connected this might be helpful if a device is
> connected while host is suspended.
> I haven't checked any timing for the link training during
> enumeration,
> but it also uses LFPS signalling, and connected device isn't visible
> to
> driver until link is successfully trained.
For xhci-mtk, roothub can't detect device until root hub set PortPower,
seems also the same for other xhci controllers that support Port Power
Control.
>
> So the original 120ms delay patch might as a positive side effect
> ensure
> driver doesn't suspend host mid device enumeration.
this is in resume?
>
> Could be looked into more, but I don't think we should this patch by
> Chunfeng
>
> Thanks
> Mathias