linux-kernel - Re: [PATCH v4] usb: host: xhci: Avoid XHCI resume delay if SSUSB device is not present

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b5d81f1c-c33e-99f0-a05b-c00d43ff5c28@linux.intel.com>
Date:   Tue, 17 Oct 2023 15:50:48 +0300
From:   Mathias Nyman <mathias.nyman@...ux.intel.com>
To:     Wesley Cheng <quic_wcheng@...cinc.com>, mathias.nyman@...el.com,
        gregkh@...uxfoundation.org
Cc:     linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH v4] usb: host: xhci: Avoid XHCI resume delay if SSUSB
 device is not present

On 12.10.2023 1.07, Wesley Cheng wrote:
> There is a 120ms delay implemented for allowing the XHCI host controller to
> detect a U3 wakeup pulse.  The intention is to wait for the device to retry
> the wakeup event if the USB3 PORTSC doesn't reflect the RESUME link status
> by the time it is checked.  As per the USB3 specification:
> 
>    tU3WakeupRetryDelay ("Table 7-12. LTSSM State Transition Timeouts")
> 
> This would allow the XHCI resume sequence to determine if the root hub
> needs to be also resumed.  However, in case there is no device connected,
> or if there is only a HSUSB device connected, this delay would still affect
> the overall resume timing.
> 
> Since this delay is solely for detecting U3 wake events (USB3 specific)
> then ignore this delay for the disconnected case and the HSUSB connected
> only case.
> 
> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
> ---
> Depends-on:
> https://lore.kernel.org/linux-usb/20230915143108.1532163-3-mathias.nyman@linux.intel.com/
> 
> changes in v4:
> - Added change log between versions
> 
> changes in v3:
> - Modified logic to determine if a USB3.0 device is connected.  Using the suspended_ports
> and bus_suspended bitmasks.  suspended_port is non-zero as ports are runtime suspended,
> while bus_suspended is non-zero if system suspend occurs while ports are active.
> 
> changes in v2:
> - Looping across portsc to determine if there is a valid USB3 connection.
> 
>   drivers/usb/host/xhci.c | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index e1b1b64a0723..1855cab1be56 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -805,6 +805,18 @@ static void xhci_disable_hub_port_wake(struct xhci_hcd *xhci,
>   	spin_unlock_irqrestore(&xhci->lock, flags);
>   }
>   
> +/*
> + * Utilize suspended_ports and bus_suspended to determine if USB3 device is
> + * connected.  The bus state bits are set by XHCI hub when root hub udev is
> + * suspended.  Used to determine if USB3 remote wakeup considerations need to
> + * be accounted for during XHCI resume.
> + */
> +static bool xhci_usb3_dev_connected(struct xhci_hcd *xhci)
> +{
> +	return !!xhci->usb3_rhub.bus_state.suspended_ports ||
> +		!!xhci->usb3_rhub.bus_state.bus_suspended;
> +}
> +

The function name xhci_usb3_dev_connected() is a bit misleading as it only returns
true if there are _suspended_ USB 3 devices connected.

I got rid of this helper and did some other minor modifications as well.
Modified version is in my for-usb-next branch.

https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=for-usb-next

Let me know if these changes are ok.
If yes, then I'll send it forward

Thanks
-Mathias