2016-12-04 12:42:35

by Thang Q. Nguyen

[permalink] [raw]
Subject: usb:xhci: support disable usb2 LPM Remote Wakeup

From: Thang Nguyen <[email protected]>

As per USB 2.0 link power management addendum ECN, table 1-2, page 4,
device or host initiated via resume signaling; device-initiated resumes
can be optionally enabled/disabled by software. This patch adds support
to control enabling the USB2 RWE feature via DT/ACPI attribute.

Signed-off-by: Vu Nguyen <[email protected]>
Signed-off-by: Thang Nguyen <[email protected]>
---
Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
drivers/usb/host/xhci-plat.c | 3 +++
drivers/usb/host/xhci.c | 5 ++++-
drivers/usb/host/xhci.h | 1 +
4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index 966885c..9b4cd14 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -25,6 +25,7 @@ Required properties:

Optional properties:
- clocks: reference to a clock
+ - usb2-rwe-disable: disable USB2 LPM Remote Wakeup capable
- usb3-lpm-capable: determines if platform is USB3 LPM capable

Example:
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ed56bf9..15c540d 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -220,6 +220,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
goto disable_clk;
}

+ if (device_property_read_bool(&pdev->dev, "usb2-rwe-disable"))
+ xhci->quirks |= XHCI_RWE_DISABLE;
+
if (device_property_read_bool(&pdev->dev, "usb3-lpm-capable"))
xhci->quirks |= XHCI_LPM_SUPPORT;

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 1a4ca02..f804868 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4183,7 +4183,10 @@ int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
}

pm_val &= ~PORT_HIRD_MASK;
- pm_val |= PORT_HIRD(hird) | PORT_RWE | PORT_L1DS(udev->slot_id);
+ if ((xhci->quirks & XHCI_RWE_DISABLE) && (xhci->hci_version <= 0x100))
+ pm_val |= PORT_HIRD(hird) | PORT_L1DS(udev->slot_id);
+ else
+ pm_val |= PORT_HIRD(hird) | PORT_RWE | PORT_L1DS(udev->slot_id);
writel(pm_val, pm_addr);
pm_val = readl(pm_addr);
pm_val |= PORT_HLE;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index f945380..2b9bc33 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1656,6 +1656,7 @@ struct xhci_hcd {
#define XHCI_SSIC_PORT_UNUSED (1 << 22)
#define XHCI_NO_64BIT_SUPPORT (1 << 23)
#define XHCI_MISSING_CAS (1 << 24)
+#define XHCI_RWE_DISABLE (1 << 25)
unsigned int num_active_eps;
unsigned int limit_active_eps;
/* There are two roothubs to keep track of bus suspend info for */
--
2.7.4


2016-12-06 06:59:05

by Thang Q. Nguyen

[permalink] [raw]
Subject: Re: usb:xhci: support disable usb2 LPM Remote Wakeup

Hi,
Do you have any feedback on this?

Thanks,
Thang Q. Nguyen

On Sun, Dec 4, 2016 at 7:42 PM, Thang Q. Nguyen <[email protected]> wrote:
> From: Thang Nguyen <[email protected]>
>
> As per USB 2.0 link power management addendum ECN, table 1-2, page 4,
> device or host initiated via resume signaling; device-initiated resumes
> can be optionally enabled/disabled by software. This patch adds support
> to control enabling the USB2 RWE feature via DT/ACPI attribute.
>
> Signed-off-by: Vu Nguyen <[email protected]>
> Signed-off-by: Thang Nguyen <[email protected]>
> ---
> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
> drivers/usb/host/xhci-plat.c | 3 +++
> drivers/usb/host/xhci.c | 5 ++++-
> drivers/usb/host/xhci.h | 1 +
> 4 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index 966885c..9b4cd14 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -25,6 +25,7 @@ Required properties:
>
> Optional properties:
> - clocks: reference to a clock
> + - usb2-rwe-disable: disable USB2 LPM Remote Wakeup capable
> - usb3-lpm-capable: determines if platform is USB3 LPM capable
>
> Example:
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index ed56bf9..15c540d 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -220,6 +220,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
> goto disable_clk;
> }
>
> + if (device_property_read_bool(&pdev->dev, "usb2-rwe-disable"))
> + xhci->quirks |= XHCI_RWE_DISABLE;
> +
> if (device_property_read_bool(&pdev->dev, "usb3-lpm-capable"))
> xhci->quirks |= XHCI_LPM_SUPPORT;
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 1a4ca02..f804868 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -4183,7 +4183,10 @@ int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
> }
>
> pm_val &= ~PORT_HIRD_MASK;
> - pm_val |= PORT_HIRD(hird) | PORT_RWE | PORT_L1DS(udev->slot_id);
> + if ((xhci->quirks & XHCI_RWE_DISABLE) && (xhci->hci_version <= 0x100))
> + pm_val |= PORT_HIRD(hird) | PORT_L1DS(udev->slot_id);
> + else
> + pm_val |= PORT_HIRD(hird) | PORT_RWE | PORT_L1DS(udev->slot_id);
> writel(pm_val, pm_addr);
> pm_val = readl(pm_addr);
> pm_val |= PORT_HLE;
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index f945380..2b9bc33 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1656,6 +1656,7 @@ struct xhci_hcd {
> #define XHCI_SSIC_PORT_UNUSED (1 << 22)
> #define XHCI_NO_64BIT_SUPPORT (1 << 23)
> #define XHCI_MISSING_CAS (1 << 24)
> +#define XHCI_RWE_DISABLE (1 << 25)
> unsigned int num_active_eps;
> unsigned int limit_active_eps;
> /* There are two roothubs to keep track of bus suspend info for */
> --
> 2.7.4
>



--

Thang Q. Nguyen | Staff SW Eng.

C: +849.7684.7606 | O: +848.3770.0640

F: +848.3770.0641 | [email protected]

2016-12-06 07:06:16

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: usb:xhci: support disable usb2 LPM Remote Wakeup

On Tue, Dec 06, 2016 at 01:59:00PM +0700, Thang Q. Nguyen wrote:
> Hi,
> Do you have any feedback on this?
>
> Thanks,
> Thang Q. Nguyen
>
> On Sun, Dec 4, 2016 at 7:42 PM, Thang Q. Nguyen <[email protected]> wrote:

It has been 1 day, please relax, wait, and be patient. If after 2 weeks
there has not been a response, then feel free to resend, but not after
just 1 day...

greg k-h

2016-12-09 21:36:33

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: usb:xhci: support disable usb2 LPM Remote Wakeup

On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
> From: Thang Nguyen <[email protected]>
>
> As per USB 2.0 link power management addendum ECN, table 1-2, page 4,
> device or host initiated via resume signaling; device-initiated resumes
> can be optionally enabled/disabled by software. This patch adds support
> to control enabling the USB2 RWE feature via DT/ACPI attribute.
>
> Signed-off-by: Vu Nguyen <[email protected]>
> Signed-off-by: Thang Nguyen <[email protected]>
> ---
> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
> drivers/usb/host/xhci-plat.c | 3 +++
> drivers/usb/host/xhci.c | 5 ++++-
> drivers/usb/host/xhci.h | 1 +
> 4 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index 966885c..9b4cd14 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -25,6 +25,7 @@ Required properties:
>
> Optional properties:
> - clocks: reference to a clock
> + - usb2-rwe-disable: disable USB2 LPM Remote Wakeup capable

Remote wakeup has been around since USB 1.0 days. Does this need to be
USB2 or XHCI specific?

> - usb3-lpm-capable: determines if platform is USB3 LPM capable
>
> Example:

2016-12-12 04:00:11

by Thang Q. Nguyen

[permalink] [raw]
Subject: Re: usb:xhci: support disable usb2 LPM Remote Wakeup

On Sat, Dec 10, 2016 at 4:36 AM, Rob Herring <[email protected]> wrote:
> On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
>> From: Thang Nguyen <[email protected]>
>>
>> As per USB 2.0 link power management addendum ECN, table 1-2, page 4,
>> device or host initiated via resume signaling; device-initiated resumes
>> can be optionally enabled/disabled by software. This patch adds support
>> to control enabling the USB2 RWE feature via DT/ACPI attribute.
>>
>> Signed-off-by: Vu Nguyen <[email protected]>
>> Signed-off-by: Thang Nguyen <[email protected]>
>> ---
>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
>> drivers/usb/host/xhci-plat.c | 3 +++
>> drivers/usb/host/xhci.c | 5 ++++-
>> drivers/usb/host/xhci.h | 1 +
>> 4 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> index 966885c..9b4cd14 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> @@ -25,6 +25,7 @@ Required properties:
>>
>> Optional properties:
>> - clocks: reference to a clock
>> + - usb2-rwe-disable: disable USB2 LPM Remote Wakeup capable
>
> Remote wakeup has been around since USB 1.0 days. Does this need to be
> USB2 or XHCI specific?
This is XHCI specific. Per XHCI specification 1.1, remote wakeup is
optional for XHCI 1.0 and required for XHCI 1.1. This patch provides
ability for software to disable RWE for USB2 in XHCI1.0 controller.
>
>> - usb3-lpm-capable: determines if platform is USB3 LPM capable
>>
>> Example:



--

Thang Q. Nguyen | Staff SW Eng.

C: +849.7684.7606 | O: +848.3770.0640

F: +848.3770.0641 | [email protected]

2016-12-12 08:39:17

by Felipe Balbi

[permalink] [raw]
Subject: Re: usb:xhci: support disable usb2 LPM Remote Wakeup


Hi,

"Thang Q. Nguyen" <[email protected]> writes:
> On Sat, Dec 10, 2016 at 4:36 AM, Rob Herring <[email protected]> wrote:
>> On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
>>> From: Thang Nguyen <[email protected]>
>>>
>>> As per USB 2.0 link power management addendum ECN, table 1-2, page 4,
>>> device or host initiated via resume signaling; device-initiated resumes
>>> can be optionally enabled/disabled by software. This patch adds support
>>> to control enabling the USB2 RWE feature via DT/ACPI attribute.
>>>
>>> Signed-off-by: Vu Nguyen <[email protected]>
>>> Signed-off-by: Thang Nguyen <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
>>> drivers/usb/host/xhci-plat.c | 3 +++
>>> drivers/usb/host/xhci.c | 5 ++++-
>>> drivers/usb/host/xhci.h | 1 +
>>> 4 files changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> index 966885c..9b4cd14 100644
>>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> @@ -25,6 +25,7 @@ Required properties:
>>>
>>> Optional properties:
>>> - clocks: reference to a clock
>>> + - usb2-rwe-disable: disable USB2 LPM Remote Wakeup capable
>>
>> Remote wakeup has been around since USB 1.0 days. Does this need to be
>> USB2 or XHCI specific?
> This is XHCI specific. Per XHCI specification 1.1, remote wakeup is
> optional for XHCI 1.0 and required for XHCI 1.1. This patch provides
> ability for software to disable RWE for USB2 in XHCI1.0 controller.

is there no way of detecting this in runtime?

--
balbi


Attachments:
signature.asc (832.00 B)

2016-12-12 08:55:06

by Thang Q. Nguyen

[permalink] [raw]
Subject: Re: usb:xhci: support disable usb2 LPM Remote Wakeup

Hi,

On Mon, Dec 12, 2016 at 3:37 PM, Felipe Balbi
<[email protected]> wrote:
>
> Hi,
>
> "Thang Q. Nguyen" <[email protected]> writes:
>> On Sat, Dec 10, 2016 at 4:36 AM, Rob Herring <[email protected]> wrote:
>>> On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
>>>> From: Thang Nguyen <[email protected]>
>>>>
>>>> As per USB 2.0 link power management addendum ECN, table 1-2, page 4,
>>>> device or host initiated via resume signaling; device-initiated resumes
>>>> can be optionally enabled/disabled by software. This patch adds support
>>>> to control enabling the USB2 RWE feature via DT/ACPI attribute.
>>>>
>>>> Signed-off-by: Vu Nguyen <[email protected]>
>>>> Signed-off-by: Thang Nguyen <[email protected]>
>>>> ---
>>>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
>>>> drivers/usb/host/xhci-plat.c | 3 +++
>>>> drivers/usb/host/xhci.c | 5 ++++-
>>>> drivers/usb/host/xhci.h | 1 +
>>>> 4 files changed, 9 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>>> index 966885c..9b4cd14 100644
>>>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>>> @@ -25,6 +25,7 @@ Required properties:
>>>>
>>>> Optional properties:
>>>> - clocks: reference to a clock
>>>> + - usb2-rwe-disable: disable USB2 LPM Remote Wakeup capable
>>>
>>> Remote wakeup has been around since USB 1.0 days. Does this need to be
>>> USB2 or XHCI specific?
>> This is XHCI specific. Per XHCI specification 1.1, remote wakeup is
>> optional for XHCI 1.0 and required for XHCI 1.1. This patch provides
>> ability for software to disable RWE for USB2 in XHCI1.0 controller.
>
> is there no way of detecting this in runtime?
Current driver always enable remote wakeup when the controller and USB
support this feature. However, per the Errata, software can optionally
enable/disable this.
Some XHCI-1.0-compatible controller does not implement features such
as HIRD, BESL which are optional for XHCI 1.1 will be fail to work
with USB2.0 device that require long time to resume from suspend.
Disabling remote wakeup by software will make this case not happen.
>
> --
> balbi



--

Thang Q. Nguyen | Staff SW Eng.

C: +849.7684.7606 | O: +848.3770.0640

F: +848.3770.0641 | [email protected]

2016-12-12 12:59:21

by Mathias Nyman

[permalink] [raw]
Subject: Re: usb:xhci: support disable usb2 LPM Remote Wakeup

On 12.12.2016 06:00, Thang Q. Nguyen wrote:
> On Sat, Dec 10, 2016 at 4:36 AM, Rob Herring <[email protected]> wrote:
>> On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
>>> From: Thang Nguyen <[email protected]>
>>>
>>> As per USB 2.0 link power management addendum ECN, table 1-2, page 4,
>>> device or host initiated via resume signaling; device-initiated resumes
>>> can be optionally enabled/disabled by software. This patch adds support
>>> to control enabling the USB2 RWE feature via DT/ACPI attribute.
>>>
>>> Signed-off-by: Vu Nguyen <[email protected]>
>>> Signed-off-by: Thang Nguyen <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
>>> drivers/usb/host/xhci-plat.c | 3 +++
>>> drivers/usb/host/xhci.c | 5 ++++-
>>> drivers/usb/host/xhci.h | 1 +
>>> 4 files changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> index 966885c..9b4cd14 100644
>>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> @@ -25,6 +25,7 @@ Required properties:
>>>
>>> Optional properties:
>>> - clocks: reference to a clock
>>> + - usb2-rwe-disable: disable USB2 LPM Remote Wakeup capable
>>
>> Remote wakeup has been around since USB 1.0 days. Does this need to be
>> USB2 or XHCI specific?
> This is XHCI specific. Per XHCI specification 1.1, remote wakeup is
> optional for XHCI 1.0 and required for XHCI 1.1. This patch provides
> ability for software to disable RWE for USB2 in XHCI1.0 controller.

I think I understand what's going on.

USB:

The good old USB2 suspend is called L2. Device enters it after 3ms if there is no link activity.
If a device can remote wakeup (RWE) it's stated in the descriptor. RWE can be turned on
of off using standard SET/CLEAR Fature requests

The LPM L1 USB2 state again is entered with a LPM extended transaction to avoid the
3ms wait before powersaving. L1 state is exit can be done with a simialr RWE as L2 resume.
The RWE from L1 can turned on/off using a bit in the LPM extended transaction.

XHCI:

Specs say that if the device supports RWE we should enable it for LPM L1 exit as well.
This is done by setting the RWE (LPM L1) bit in PORTPMSC register. This bit only affect LPM L1 remote
wake. see 4.23.5.1.1.1

The issue might be that xhci driver never check if the device actually supports RWE, we always
set the PORTPMSC RWE (for LPM L1) bit.

How about checking something like udev->do_remote_wakeup and setting and setting the bit
based on that.

The function that you are changing, xhci_set_usb2_hardware_lpm() should only be used if
host has Hardware LPM Cabaility bit (HLC) set for that USB2 port in the
USB 2.0 xHCI Supported Protocol Capability.
Host that don't supprt LPM won't have that set. See xhci 7.2.2.1.3.2

-Mathias