usb:xhci: support disable usb2 LPM Remote Wakeup

From: Thang Nguyen <tqnguyen@apm.com>

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 <vnguyen@apm.com>
Signed-off-by: Thang Nguyen <tqnguyen@apm.com>
---
 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(-)

Hi,
Do you have any feedback on this?

Thanks,
Thang Q. Nguyen

On Sun, Dec 4, 2016 at 7:42 PM, Thang Q. Nguyen <tqnguyen@apm.com> wrote:
> From: Thang Nguyen <tqnguyen@apm.com>
>
> 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 <vnguyen@apm.com>
> Signed-off-by: Thang Nguyen <tqnguyen@apm.com>
> ---
>  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
>

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 <tqnguyen@apm.com> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
> From: Thang Nguyen <tqnguyen@apm.com>
> 
> 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 <vnguyen@apm.com>
> Signed-off-by: Thang Nguyen <tqnguyen@apm.com>
> ---
>  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:
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sat, Dec 10, 2016 at 4:36 AM, Rob Herring <robh@kernel.org> wrote:
> On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
>> From: Thang Nguyen <tqnguyen@apm.com>
>>
>> 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 <vnguyen@apm.com>
>> Signed-off-by: Thang Nguyen <tqnguyen@apm.com>
>> ---
>>  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:

Hi,

"Thang Q. Nguyen" <tqnguyen@apm.com> writes:
> On Sat, Dec 10, 2016 at 4:36 AM, Rob Herring <robh@kernel.org> wrote:
>> On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
>>> From: Thang Nguyen <tqnguyen@apm.com>
>>>
>>> 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 <vnguyen@apm.com>
>>> Signed-off-by: Thang Nguyen <tqnguyen@apm.com>
>>> ---
>>>  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?

Hi,

On Mon, Dec 12, 2016 at 3:37 PM, Felipe Balbi
<felipe.balbi@linux.intel.com> wrote:
>
> Hi,
>
> "Thang Q. Nguyen" <tqnguyen@apm.com> writes:
>> On Sat, Dec 10, 2016 at 4:36 AM, Rob Herring <robh@kernel.org> wrote:
>>> On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
>>>> From: Thang Nguyen <tqnguyen@apm.com>
>>>>
>>>> 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 <vnguyen@apm.com>
>>>> Signed-off-by: Thang Nguyen <tqnguyen@apm.com>
>>>> ---
>>>>  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

On 12.12.2016 06:00, Thang Q. Nguyen wrote:
> On Sat, Dec 10, 2016 at 4:36 AM, Rob Herring <robh@kernel.org> wrote:
>> On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
>>> From: Thang Nguyen <tqnguyen@apm.com>
>>>
>>> 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 <vnguyen@apm.com>
>>> Signed-off-by: Thang Nguyen <tqnguyen@apm.com>
>>> ---
>>>   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







--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Mon, Dec 12, 2016 at 8:00 PM, Mathias Nyman
<mathias.nyman@linux.intel.com> wrote:
> On 12.12.2016 06:00, Thang Q. Nguyen wrote:
>>
>> On Sat, Dec 10, 2016 at 4:36 AM, Rob Herring <robh@kernel.org> wrote:
>>>
>>> On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
>>>>
>>>> From: Thang Nguyen <tqnguyen@apm.com>
>>>>
>>>> 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 <vnguyen@apm.com>
>>>> Signed-off-by: Thang Nguyen <tqnguyen@apm.com>
>>>> ---
>>>>   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.
Yes, we should check if device support Remote Wakeup to enable or
disable RWE as noted in cases 1 (page 265) and 2 (page 266) from
4.23.5.1.1.1.
>
> 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
When hosts support Hardware LPM (HLC), any problem if we add a DT/ACPI
attribute to support disable it (HLE=0)?
>  -Mathias
>
>
>
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html