| Message ID | 20211209072218.21651-1-chunfeng.yun@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: xhci: skip re-check pending port event if hibernated | expand |
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 <yun-chien.yu@mediatek.com> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- Thanks, adding -Mathias
Mathias Nyman <mathias.nyman@linux.intel.com> 于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 <yun-chien.yu@mediatek.com> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > --- > > 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 <mathias.nyman@linux.intel.com> 于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 <yun-chien.yu@mediatek.com> >>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> >>> --- >> >> 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 <mathias.nyman@linux.intel.com> 于2021年12月14日周二 18:03写道： > > On 14.12.2021 10.00, Jun Li wrote: > > Mathias Nyman <mathias.nyman@linux.intel.com> 于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 <yun-chien.yu@mediatek.com> > >>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > >>> --- > >> > >> 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 <mathias.nyman@linux.intel.com> 于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 <yun-chien.yu@mediatek.com> > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > > > --- > > > > > > 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
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); }
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 <yun-chien.yu@mediatek.com> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- drivers/usb/host/xhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)