Re: [PATCH] usb: xhci: skip re-check pending port event if hibernated

From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Jun Li <lijun.kernel@gmail.com>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>,
	Mathias Nyman <mathias.nyman@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Linux USB List <linux-usb@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linux-mediatek@lists.infradead.org,
	lkml <linux-kernel@vger.kernel.org>,
	Eddie Hung <eddie.hung@mediatek.com>,
	Yun-Chien Yu <yun-chien.yu@mediatek.com>
Subject: Re: [PATCH] usb: xhci: skip re-check pending port event if hibernated
Date: Tue, 14 Dec 2021 12:05:28 +0200	[thread overview]
Message-ID: <e7a6e45e-68ce-54a5-9632-80244dd1e4c7@linux.intel.com> (raw)
In-Reply-To: <CAKgpwJXxtLwOjxjg3vFHiqS92j6rx_b1+C-bRwDnp+PBvXCMTg@mail.gmail.com>

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