> On Jun 10, 2020, at 23:58, Mathias Nyman <mathias.ny...@linux.intel.com>
> wrote:
>
> On 10.6.2020 18.43, Kai-Heng Feng wrote:
>>
>>
>>> On Jun 10, 2020, at 22:32, Alan Stern <st...@rowland.harvard.edu> wrote:
>>>
>>> On Wed, Jun 10, 2020 at 02:42:30PM +0800, Kai-Heng Feng wrote:
>>>> xHCI spec "4.15.1 Port Suspend" states that port can be put to U3 as long
>>>> as Enabled bit is set and from U0, U1 or U2 state.
>>>>
>>>> Currently only USB_PORT_FEAT_LINK_STATE puts port to U3 directly, let's
>>>> do the same for USB_PORT_FEAT_SUSPEND and bus suspend case.
>>>>
>>>> This is particularly useful for USB2 devices, which may take a very long
>>>> time to switch USB2 LPM on and off.
>>>
>>> Have these two patches been tested with a variety of USB-2.0 and USB-2.1
>>> devices?
>>
>> I tested some laptops around and they work fine.
>> Only internally connected USB devices like USB Bluetooth and USB Camera have
>> USB2 LPM enabled, so this patch won't affect external connected devices.
>>
>
> Took a fresh look at the USB2 side and it's not as clear as the USB3 case,
> where
> we know the hub must support transition to U3 from any other state. [1]
>
> Supporting link state transition to U3 (USB2 L2) from any other U state for
> USB2 seems
> to be xHCI specific feature. xHC hardware will make sure it goes via the U0
> state.
>
> I have no clue about other hosts (or hubs), USB2 LPM ECN just shows that link
> state transitions to L1 or L2 should always goes via L0.
> It's possible this has to be done in software by disabling USB2 LPM before
> suspending the device.
Is there any host or hub other than xHCI support USB2 LPM? Seem like only xHCI
implements it.
>
> So I guess the original suggestion to wait for link state to reach U0 before
> is a better solution. Sorry about my hasty suggestion.
>
> Kai-Heng, does it help if you fist manually set the link to U0 before
> disabling
> USB2 LPM (set PLS to 0 before clearing the HLE bit). Does it transition to U0
> any faster, or get rid of the extra port event with PLC:U0?
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 03b64b73eb99..0b8db13e79e4 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4385,7 +4385,7 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct xhci_port **ports;
__le32 __iomem *pm_addr, *hlpm_addr;
- u32 pm_val, hlpm_val, field;
+ u32 portsc, pm_val, hlpm_val, field;
unsigned int port_num;
unsigned long flags;
int hird, exit_latency;
@@ -4463,6 +4463,15 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd
*hcd,
/* flush write */
readl(pm_addr);
} else {
+ portsc = readl(ports[port_num]->addr);
+ if ((portsc & PORT_PE) && ((portsc & PORT_PLS_MASK) == XDEV_U1
||
+ (portsc & PORT_PLS_MASK) == XDEV_U2)) {
+ xhci_set_link_state(xhci, ports[port_num], XDEV_U0);
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ dev_info(&udev->dev, "DEBUG: SLEEP\n");
+ msleep(100);
+ spin_lock_irqsave(&xhci->lock, flags);
+ }
pm_val &= ~(PORT_HLE | PORT_RWE | PORT_HIRD_MASK |
PORT_L1DS_MASK);
writel(pm_val, pm_addr);
/* flush write */
If there's a long enough delay for U0 before clearing HLE, then the PLC can be
avoided.
It's not any faster though.
Kai-Heng
>
> -Mathias
>
>
> [1]
> USB3.1 spec (10.16.2.10) Set Port feature:
> "If the value is 3, then host software wants to selectively suspend the
> device connected to this port. The hub shall transition the link to U3
> from any of the other U states using allowed link state transitions.
> If the port is not already in the U0 state, then it shall transition the
> port to the U0 state and then initiate the transition to U3.
