[PATCH v2] usb: host: xhci: Avoid XHCI resume delay if SSUSB device is not present

[PATCH v2] usb: host: xhci: Avoid XHCI resume delay if SSUSB device is not present

[Wesley Cheng]

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@quicinc.com>
---
 drivers/usb/host/xhci.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e1b1b64a0723..640db6a4e686 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -805,6 +805,24 @@ static void xhci_disable_hub_port_wake(struct xhci_hcd *xhci,
 	spin_unlock_irqrestore(&xhci->lock, flags);
 }
 
+static bool xhci_usb3_dev_connected(struct xhci_hcd *xhci)
+{
+	struct xhci_port	**ports;
+	int			port_index;
+	u32			portsc;
+
+	port_index = xhci->usb3_rhub.num_ports;
+	ports = xhci->usb3_rhub.ports;
+
+	while (port_index--) {
+		portsc = readl(ports[port_index]->addr);
+		if (portsc & (portsc & PORT_CONNECT))
+			return true;
+	}
+
+	return false;
+}
+
 static bool xhci_pending_portevent(struct xhci_hcd *xhci)
 {
 	struct xhci_port	**ports;
@@ -968,6 +986,7 @@ int xhci_resume(struct xhci_hcd *xhci, pm_message_t msg)
 	int			retval = 0;
 	bool			comp_timer_running = false;
 	bool			pending_portevent = false;
+	bool			usb3_connected = false;
 	bool			reinit_xhc = false;
 
 	if (!hcd->state)
@@ -1116,9 +1135,14 @@ int xhci_resume(struct xhci_hcd *xhci, pm_message_t msg)
 		 * Resume roothubs only if there are pending events.
 		 * USB 3 devices resend U3 LFPS wake after a 100ms delay if
 		 * the first wake signalling failed, give it that chance.
+		 * Avoid this check if there are no devices connected to
+		 * the SS root hub (i.e. HS device connected or no device
+		 * connected)
 		 */
 		pending_portevent = xhci_pending_portevent(xhci);
-		if (!pending_portevent && msg.event == PM_EVENT_AUTO_RESUME) {
+		usb3_connected = xhci_usb3_dev_connected(xhci);
+		if (!pending_portevent && usb3_connected &&
+		     msg.event == PM_EVENT_AUTO_RESUME) {
 			msleep(120);
 			pending_portevent = xhci_pending_portevent(xhci);
 		}

Re: [PATCH v2] usb: host: xhci: Avoid XHCI resume delay if SSUSB device is not present

[Mathias Nyman]

On 1.9.2023 3.15, 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.

Thanks, makes sense

> 
> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> ---
>   drivers/usb/host/xhci.c | 26 +++++++++++++++++++++++++-
>   1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index e1b1b64a0723..640db6a4e686 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -805,6 +805,24 @@ static void xhci_disable_hub_port_wake(struct xhci_hcd *xhci,
>   	spin_unlock_irqrestore(&xhci->lock, flags);
>   }
>   
> +static bool xhci_usb3_dev_connected(struct xhci_hcd *xhci)
> +{
> +	struct xhci_port	**ports;
> +	int			port_index;
> +	u32			portsc;
> +
> +	port_index = xhci->usb3_rhub.num_ports;
> +	ports = xhci->usb3_rhub.ports;
> +
> +	while (port_index--) {
> +		portsc = readl(ports[port_index]->addr);
> +		if (portsc & (portsc & PORT_CONNECT))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +

This is one way, but we can probably avoid re-reading all the usb3 portsc registers
by checking if any bit is set in either:

  // bitfield, set if xhci usb3 port neatly set to U3 with a hub request
xhci->usb3_rhub.bus_state.suspended_ports

// bitfield, set if xhci usb3 port is forced to U3 during xhci suspend.
xhci->usb3_rhub.bus_state.bus_suspended

But haven't checked this works in all corner cases.

Thanks
Mathias

Re: [PATCH v2] usb: host: xhci: Avoid XHCI resume delay if SSUSB device is not present

[Wesley Cheng]

Hi Mathias,

On 9/11/2023 6:50 AM, Mathias Nyman wrote:
> On 1.9.2023 3.15, 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.
> 
> Thanks, makes sense
> 
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
>>   drivers/usb/host/xhci.c | 26 +++++++++++++++++++++++++-
>>   1 file changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index e1b1b64a0723..640db6a4e686 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -805,6 +805,24 @@ static void xhci_disable_hub_port_wake(struct 
>> xhci_hcd *xhci,
>>       spin_unlock_irqrestore(&xhci->lock, flags);
>>   }
>> +static bool xhci_usb3_dev_connected(struct xhci_hcd *xhci)
>> +{
>> +    struct xhci_port    **ports;
>> +    int            port_index;
>> +    u32            portsc;
>> +
>> +    port_index = xhci->usb3_rhub.num_ports;
>> +    ports = xhci->usb3_rhub.ports;
>> +
>> +    while (port_index--) {
>> +        portsc = readl(ports[port_index]->addr);
>> +        if (portsc & (portsc & PORT_CONNECT))
>> +            return true;
>> +    }
>> +
>> +    return false;
>> +}
>> +
> 
> This is one way, but we can probably avoid re-reading all the usb3 
> portsc registers
> by checking if any bit is set in either:
> 
>   // bitfield, set if xhci usb3 port neatly set to U3 with a hub request
> xhci->usb3_rhub.bus_state.suspended_ports
> 
> // bitfield, set if xhci usb3 port is forced to U3 during xhci suspend.
> xhci->usb3_rhub.bus_state.bus_suspended
> 
> But haven't checked this works in all corner cases.
> 
Thanks for the suggestion.  I think I also looked at seeing if we could 
use the suspended_ports param, and it was missing one of the use cases 
we had.  I haven't thought on combining it with the bus_suspend param 
also to see if it could work.  Let me give it a try, and I'll get back 
to you.

Thanks
Wesley Cheng

Re: [PATCH v2] usb: host: xhci: Avoid XHCI resume delay if SSUSB device is not present

[Wesley Cheng]

Hi Mathias,

On 9/11/2023 5:15 PM, Wesley Cheng wrote:
> Hi Mathias,
> 
> On 9/11/2023 6:50 AM, Mathias Nyman wrote:
>> On 1.9.2023 3.15, 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.
>>
>> Thanks, makes sense
>>
>>>
>>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>>> ---
>>>   drivers/usb/host/xhci.c | 26 +++++++++++++++++++++++++-
>>>   1 file changed, 25 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>>> index e1b1b64a0723..640db6a4e686 100644
>>> --- a/drivers/usb/host/xhci.c
>>> +++ b/drivers/usb/host/xhci.c
>>> @@ -805,6 +805,24 @@ static void xhci_disable_hub_port_wake(struct 
>>> xhci_hcd *xhci,
>>>       spin_unlock_irqrestore(&xhci->lock, flags);
>>>   }
>>> +static bool xhci_usb3_dev_connected(struct xhci_hcd *xhci)
>>> +{
>>> +    struct xhci_port    **ports;
>>> +    int            port_index;
>>> +    u32            portsc;
>>> +
>>> +    port_index = xhci->usb3_rhub.num_ports;
>>> +    ports = xhci->usb3_rhub.ports;
>>> +
>>> +    while (port_index--) {
>>> +        portsc = readl(ports[port_index]->addr);
>>> +        if (portsc & (portsc & PORT_CONNECT))
>>> +            return true;
>>> +    }
>>> +
>>> +    return false;
>>> +}
>>> +
>>
>> This is one way, but we can probably avoid re-reading all the usb3 
>> portsc registers
>> by checking if any bit is set in either:
>>
>>   // bitfield, set if xhci usb3 port neatly set to U3 with a hub request
>> xhci->usb3_rhub.bus_state.suspended_ports
>>
>> // bitfield, set if xhci usb3 port is forced to U3 during xhci suspend.
>> xhci->usb3_rhub.bus_state.bus_suspended
>>
>> But haven't checked this works in all corner cases.
>>
> Thanks for the suggestion.  I think I also looked at seeing if we could 
> use the suspended_ports param, and it was missing one of the use cases 
> we had.  I haven't thought on combining it with the bus_suspend param 
> also to see if it could work.  Let me give it a try, and I'll get back 
> to you.
> 

So in one of our normal use cases, which is to use an USB OTG adapter 
with our devices, we can have this connected with no device.  In this 
situation, the XHCI HCD and root hub are enumerated, and is in a state 
where nothing is connected to the port.  I added a print to the 
xhci_resume() path to check the status of "suspended_ports" and 
"bus_suspended" and they seem to reflect the same status as when there 
is something connected (to a device that supports autosuspend).  Here's 
some pointers I've found on why these parameters may not work:

1.  bus_suspended is only set (for the bus) if we reach the 
bus_suspend() callback from USB HCD if the link is still in U0.  If USB 
autosuspend is enabled, then the suspending of the root hub udev, would 
have caused a call to suspend the port (usb_port_suspend()), and that 
would set "suspended_ports" and placed the link in U3 already.

2. "suspended_ports" can't differentiate if a device is connected or not 
after plugging in a USB3 device that has autosuspend enabled.  It looks 
like on device disconnection, the suspended_ports isn't cleared for that 
port number.  It is only cleared during the resume path where a get port 
status is queried:

static void xhci_get_usb3_port_status(struct xhci_port *port, u32 *status,
				      u32 portsc)
{
...
  	/* USB3 specific wPortStatus bits */
  	if (portsc & PORT_POWER) {
  		*status |= USB_SS_PORT_STAT_POWER;
  		/* link state handling */
  		if (link_state == XDEV_U0)
  			bus_state->suspended_ports &= ~(1 << portnum);
  	}

IMO, this seems kind of weird, because the PLS shows that the port is in 
the RxDetect state, so it technically isn't suspended.  If you think we 
should clear suspended_ports on disconnect, then I think we can also 
change the logic to rely on it for avoiding the unnecessary delay in 
xhci_resume().

Thanks
Wesley Cheng

Re: [PATCH v2] usb: host: xhci: Avoid XHCI resume delay if SSUSB device is not present

[Mathias Nyman]

Hi

On 13.9.2023 0.51, Wesley Cheng wrote:
> Hi Mathias,
> 
>>> This is one way, but we can probably avoid re-reading all the usb3 portsc registers
>>> by checking if any bit is set in either:
>>>
>>>   // bitfield, set if xhci usb3 port neatly set to U3 with a hub request
>>> xhci->usb3_rhub.bus_state.suspended_ports
>>>
>>> // bitfield, set if xhci usb3 port is forced to U3 during xhci suspend.
>>> xhci->usb3_rhub.bus_state.bus_suspended
>>>
>>> But haven't checked this works in all corner cases.
>>>
>> Thanks for the suggestion.  I think I also looked at seeing if we could use the suspended_ports param, and it was missing one of the use cases we had.  I haven't thought on combining it with the bus_suspend param also to see if it could work.  Let me give it a try, and I'll get back to you.
>>
> 
> So in one of our normal use cases, which is to use an USB OTG adapter with our devices, we can have this connected with no device.  In this situation, the XHCI HCD and root hub are enumerated, and is in a state where nothing is connected to the port.  I added a print to the xhci_resume() path to check the status of "suspended_ports" and "bus_suspended" and they seem to reflect the same status as when there is something connected (to a device that supports autosuspend).  Here's some pointers I've found on why these parameters may not work:
> 
> 1.  bus_suspended is only set (for the bus) if we reach the bus_suspend() callback from USB HCD if the link is still in U0.  If USB autosuspend is enabled, then the suspending of the root hub udev, would have caused a call to suspend the port (usb_port_suspend()), and that would set "suspended_ports" and placed the link in U3 already.
> 
> 2. "suspended_ports" can't differentiate if a device is connected or not after plugging in a USB3 device that has autosuspend enabled.  It looks like on device disconnection, the suspended_ports isn't cleared for that port number.  It is only cleared during the resume path where a get port status is queried:
> 
> static void xhci_get_usb3_port_status(struct xhci_port *port, u32 *status,
>                        u32 portsc)
> {
> ...
>       /* USB3 specific wPortStatus bits */
>       if (portsc & PORT_POWER) {
>           *status |= USB_SS_PORT_STAT_POWER;
>           /* link state handling */
>           if (link_state == XDEV_U0)
>               bus_state->suspended_ports &= ~(1 << portnum);
>       }
> 
> IMO, this seems kind of weird, because the PLS shows that the port is in the RxDetect state, so it technically isn't suspended.  If you think we should clear suspended_ports on disconnect, then I think we can also change the logic to rely on it for avoiding the unnecessary delay in xhci_resume().

I think you found a bug.

We should clear suspended_ports bit if link state in portsc is anything other than U3, Resume or Recovery.

Not doing so might cause USB_PORT_STAT_C_SUSPEND bit to be set incorrectly in a USB2 get port status request.

So we want something like:
if (suspended_ports bit is set) {
	if (U3 || Resume || Recovery) {
		don't touch anything
	} else {
		clear suspended_port bit
		if ((U2 || U0) && USB2)
			set bus_state->port_c_suspend bit
}

I'll look into it

-Mathias

Re: [PATCH v2] usb: host: xhci: Avoid XHCI resume delay if SSUSB device is not present

[Wesley Cheng]

Hi Mathias,

On 9/13/2023 7:21 AM, Mathias Nyman wrote:
> Hi
> 
> On 13.9.2023 0.51, Wesley Cheng wrote:
>> Hi Mathias,
>>
>>>> This is one way, but we can probably avoid re-reading all the usb3 
>>>> portsc registers
>>>> by checking if any bit is set in either:
>>>>
>>>>   // bitfield, set if xhci usb3 port neatly set to U3 with a hub 
>>>> request
>>>> xhci->usb3_rhub.bus_state.suspended_ports
>>>>
>>>> // bitfield, set if xhci usb3 port is forced to U3 during xhci suspend.
>>>> xhci->usb3_rhub.bus_state.bus_suspended
>>>>
>>>> But haven't checked this works in all corner cases.
>>>>
>>> Thanks for the suggestion.  I think I also looked at seeing if we 
>>> could use the suspended_ports param, and it was missing one of the 
>>> use cases we had.  I haven't thought on combining it with the 
>>> bus_suspend param also to see if it could work.  Let me give it a 
>>> try, and I'll get back to you.
>>>
>>
>> So in one of our normal use cases, which is to use an USB OTG adapter 
>> with our devices, we can have this connected with no device.  In this 
>> situation, the XHCI HCD and root hub are enumerated, and is in a state 
>> where nothing is connected to the port.  I added a print to the 
>> xhci_resume() path to check the status of "suspended_ports" and 
>> "bus_suspended" and they seem to reflect the same status as when there 
>> is something connected (to a device that supports autosuspend).  
>> Here's some pointers I've found on why these parameters may not work:
>>
>> 1.  bus_suspended is only set (for the bus) if we reach the 
>> bus_suspend() callback from USB HCD if the link is still in U0.  If 
>> USB autosuspend is enabled, then the suspending of the root hub udev, 
>> would have caused a call to suspend the port (usb_port_suspend()), and 
>> that would set "suspended_ports" and placed the link in U3 already.
>>
>> 2. "suspended_ports" can't differentiate if a device is connected or 
>> not after plugging in a USB3 device that has autosuspend enabled.  It 
>> looks like on device disconnection, the suspended_ports isn't cleared 
>> for that port number.  It is only cleared during the resume path where 
>> a get port status is queried:
>>
>> static void xhci_get_usb3_port_status(struct xhci_port *port, u32 
>> *status,
>>                        u32 portsc)
>> {
>> ...
>>       /* USB3 specific wPortStatus bits */
>>       if (portsc & PORT_POWER) {
>>           *status |= USB_SS_PORT_STAT_POWER;
>>           /* link state handling */
>>           if (link_state == XDEV_U0)
>>               bus_state->suspended_ports &= ~(1 << portnum);
>>       }
>>
>> IMO, this seems kind of weird, because the PLS shows that the port is 
>> in the RxDetect state, so it technically isn't suspended.  If you 
>> think we should clear suspended_ports on disconnect, then I think we 
>> can also change the logic to rely on it for avoiding the unnecessary 
>> delay in xhci_resume().
> 
> I think you found a bug.
> 
> We should clear suspended_ports bit if link state in portsc is anything 
> other than U3, Resume or Recovery.
> 
> Not doing so might cause USB_PORT_STAT_C_SUSPEND bit to be set 
> incorrectly in a USB2 get port status request.
> 
> So we want something like:
> if (suspended_ports bit is set) {
>      if (U3 || Resume || Recovery) {
>          don't touch anything
>      } else {
>          clear suspended_port bit
>          if ((U2 || U0) && USB2)
>              set bus_state->port_c_suspend bit
> }
> 
> I'll look into it
> 

Thanks, Mathias.  Will take some time to take a look as well since I 
have a reliable set up that observes this issue.  If you have any test 
code you might want to try, let me know!

Thanks
Wesley Cheng

[RFT PATCH] xhci: track port suspend state correctly in unsuccessful resume cases

[Mathias Nyman]

xhci-hub.c tracks suspended ports in a suspended_port bitfield.
This is checked when responding to a Get_Status(PORT) request to see if a
port in running U0 state was recently resumed, and adds the required
USB_PORT_STAT_C_SUSPEND change bit in those cases.

The suspended_port bit was left uncleared if a device is disconnected
during suspend. The bit remained set even when a new device was connected
and enumerated. The set bit resulted in a incorrect Get_Status(PORT)
response with a bogus USB_PORT_STAT_C_SUSPEND change
bit set once the new device reached U0 link state.

USB_PORT_STAT_C_SUSPEND change bit is only used for USB2 ports, but
xhci-hub keeps track of both USB2 and USB3 suspended ports.

Reported-by: Wesley Cheng <quic_wcheng@quicinc.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0054d02239e2..0df5d807a77e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1062,19 +1062,19 @@ static void xhci_get_usb3_port_status(struct xhci_port *port, u32 *status,
 		*status |= USB_PORT_STAT_C_CONFIG_ERROR << 16;
 
 	/* USB3 specific wPortStatus bits */
-	if (portsc & PORT_POWER) {
+	if (portsc & PORT_POWER)
 		*status |= USB_SS_PORT_STAT_POWER;
-		/* link state handling */
-		if (link_state == XDEV_U0)
-			bus_state->suspended_ports &= ~(1 << portnum);
-	}
 
-	/* remote wake resume signaling complete */
-	if (bus_state->port_remote_wakeup & (1 << portnum) &&
+	/* no longer suspended or resuming */
+	if (link_state != XDEV_U3 &&
 	    link_state != XDEV_RESUME &&
 	    link_state != XDEV_RECOVERY) {
-		bus_state->port_remote_wakeup &= ~(1 << portnum);
-		usb_hcd_end_port_resume(&hcd->self, portnum);
+		/* remote wake resume signaling complete */
+		if (bus_state->port_remote_wakeup & (1 << portnum)) {
+			bus_state->port_remote_wakeup &= ~(1 << portnum);
+			usb_hcd_end_port_resume(&hcd->self, portnum);
+		}
+		bus_state->suspended_ports &= ~(1 << portnum);
 	}
 
 	xhci_hub_report_usb3_link_state(xhci, status, portsc);
@@ -1131,6 +1131,7 @@ static void xhci_get_usb2_port_status(struct xhci_port *port, u32 *status,
 			usb_hcd_end_port_resume(&port->rhub->hcd->self, portnum);
 		}
 		port->rexit_active = 0;
+		bus_state->suspended_ports &= ~(1 << portnum);
 	}
 }
 
-- 
2.25.1

Re: [RFT PATCH] xhci: track port suspend state correctly in unsuccessful resume cases

[Wesley Cheng]

Hi Mathias,

On 9/14/2023 2:27 AM, Mathias Nyman wrote:
> xhci-hub.c tracks suspended ports in a suspended_port bitfield.
> This is checked when responding to a Get_Status(PORT) request to see if a
> port in running U0 state was recently resumed, and adds the required
> USB_PORT_STAT_C_SUSPEND change bit in those cases.
> 
> The suspended_port bit was left uncleared if a device is disconnected
> during suspend. The bit remained set even when a new device was connected
> and enumerated. The set bit resulted in a incorrect Get_Status(PORT)
> response with a bogus USB_PORT_STAT_C_SUSPEND change
> bit set once the new device reached U0 link state.
> 
> USB_PORT_STAT_C_SUSPEND change bit is only used for USB2 ports, but
> xhci-hub keeps track of both USB2 and USB3 suspended ports.
> 

Thanks for looking at this change.  Tested it on my environment and it 
looks good to me:

//Disconnect while bus is suspended (before get status)
msm-dwc3 a600000.ssusb: DWC3 exited from low power mode
xhci_resume: usb3 suspended_ports = 0, bus_suspended = 0
xhci_resume: usb2 suspended_ports = 1, bus_suspended = 0
usb 1-1: USB disconnect, device number 2
msm-dwc3 a600000.ssusb: could not transition HS PHY to L2
msm-dwc3 a600000.ssusb: DWC3 in low power mode

//Plug in after disconnect
msm-dwc3 a600000.ssusb: DWC3 exited from low power mode
xhci_resume: usb3 suspended_ports = 0, bus_suspended = 0
xhci_resume: usb2 suspended_ports = 0, bus_suspended = 0
usb 1-1: new full-speed USB device number 3 using xhci-hcd
...
msm-dwc3 a600000.ssusb: could not transition HS PHY to L2
msm-dwc3 a600000.ssusb: DWC3 in low power mode

//Remote wakeup
msm-dwc3 a600000.ssusb: DWC3 exited from low power mode
xhci_resume: usb3 suspended_ports = 0, bus_suspended = 0
xhci_resume: usb2 suspended_ports = 1, bus_suspended = 0
plantronics 0003:047F:C025.0002: intr, xhci-hcd.2.auto-1/input3, status -2
msm-dwc3 a600000.ssusb: could not transition HS PHY to L2
msm-dwc3 a600000.ssusb: DWC3 in low power mode

Will update my change ([1]) to include testing for the suspended_ports 
for if a device is connected (and suspended) while running xhci_resume()

Thanks
Wesley Cheng

[1] 
https://lore.kernel.org/linux-usb/20230901001518.25403-1-quic_wcheng@quicinc.com/