linux-kernel - Re: [PATCH v4 2/3] dt-bindings: usb: snps,dwc3: Add runtime-suspend-on-usb-suspend property

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 22 Aug 2023 16:58:32 -0700
From:   Elson Serrao <quic_eserrao@...cinc.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        <gregkh@...uxfoundation.org>, <Thinh.Nguyen@...opsys.com>,
        <rogerq@...nel.org>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
        <devicetree@...r.kernel.org>
CC:     <linux-kernel@...r.kernel.org>, <linux-usb@...r.kernel.org>
Subject: Re: [PATCH v4 2/3] dt-bindings: usb: snps,dwc3: Add
 runtime-suspend-on-usb-suspend property

On 8/19/2023 2:35 AM, Krzysztof Kozlowski wrote:
> On 18/08/2023 21:16, Elson Serrao wrote:
>>
>>
>> On 8/15/2023 10:44 PM, Krzysztof Kozlowski wrote:
>>> On 14/08/2023 20:50, Elson Roy Serrao wrote:
>>>> This property allows dwc3 runtime suspend when bus suspend interrupt
>>>> is received even with cable connected. This would allow the dwc3
>>>> controller to enter low power mode during bus suspend scenario.
>>>>
>>>> This property would particularly benefit dwc3 IPs where hibernation is
>>>> not enabled and the dwc3 low power mode entry/exit is handled by the
>>>> glue driver. The assumption here is that the platform using this dt
>>>> property is capable of detecting resume events to bring the controller
>>>> out of suspend.
>>>>
>>>> Signed-off-by: Elson Roy Serrao <quic_eserrao@...cinc.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>>> index a696f23730d3..e19a60d06d2b 100644
>>>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>>> @@ -403,6 +403,11 @@ properties:
>>>>        description:
>>>>          Enable USB remote wakeup.
>>>>    
>>>> +  snps,runtime-suspend-on-usb-suspend:
>>>> +    description:
>>>> +      If True then dwc3 runtime suspend is allowed during bus suspend
>>>> +      case even with the USB cable connected.
>>>
>>> This was no tested... but anyway, this is no a DT property but OS
>>> policy. There is no such thing as "runtime suspend" in the hardware,
>>> because you describe one particular OS.
>>>
>>> Sorry, no a DT property, drop the change entirely.
>>>
>>>
>> Hi Krzysztof
>>
>> Sorry my local dt checker had some issue and it did not catch these
>> errors. I have rectified it now.
>>
>> This dt property is mainly for skipping dwc3 controller halt when a USB
>> suspend interrupt is received with usb cable connected, so that we dont
>> trigger a DISCONNECT event. Perhaps a better name would reflect the true
>> usage of this?
>>
>> Something like snps,skip-dwc3-halt-on-usb-suspend. dwc3 cores where
>> hibernation feature is not enabled/supported can use this property
> 
> So this is specific to DWC3 core, thus should be just implied by compatible.
> 

Hi Krzysztof

Apologies for not being clear. Below is the reasoning behind this dt entry.

When bus suspend interrupt is received and if usb cable is connected, 
dwc3 driver does not suspend. The aim of this series is to handle this 
interrupt when USB cable is connected to achieve power savings. OEMs 
might have their own implementation in their glue driver to turn off 
clocks and other resources when USB is not in use, thus saving power. 
But since glue layer has dependency on dwc3 driver (parent-child 
relationship) we need to allow dwc3 driver to NOT ignore the bus suspend 
interrupt and let the dwc3 driver suspend (so that glue driver can 
suspend as well)

Now it is the responsibility of glue driver to detect USB wakeup signal 
from the host during resume (since dwc3 driver is suspended at this 
point and cannot handle interrupts). Every OEM may not have the 
capability to detect wakeup signal. The goal of this dt property is for 
the dwc3 driver to allow handling of the bus suspend interrupt when such 
a capability exists on a given HW platform. When this property is 
defined dwc3 driver knows that the low power mode entry/exit is 
controlled by the glue driver and thus it can allow the suspend 
operation when bus suspend interrupt is received.

For example on Qualcomm platforms there is a phy sideband signalling 
which detects the wakeup signal when resume is initiated by the host. 
Thus qcom platforms can benefit from this feature by defining this dt 
property. (in a parallel discussion with Thinh N to come up with a 
better name for this dt entry).

Thanks
Elson