[PATCH v3 2/3] dt-bindings: usb: snps,dwc3: Add allow-rtsusp-on-u3 property

Elson Roy Serrao posted 3 patches 2 years, 7 months ago
There is a newer version of this series
[PATCH v3 2/3] dt-bindings: usb: snps,dwc3: Add allow-rtsusp-on-u3 property
Posted by Elson Roy Serrao 2 years, 7 months ago
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@quicinc.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..18ad99a26dd9 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,allow-rtsusp-on-u3:
+    description:
+      If True then dwc3 runtime suspend is allowed during bus suspend
+      case even with the USB cable connected.
+
 unevaluatedProperties: false
 
 required:
-- 
2.17.1
Re: [PATCH v3 2/3] dt-bindings: usb: snps,dwc3: Add allow-rtsusp-on-u3 property
Posted by Thinh Nguyen 2 years, 7 months ago
On Tue, Jul 11, 2023, 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@quicinc.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..18ad99a26dd9 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,allow-rtsusp-on-u3:

Please spell out the whole thing as "rtsusp" isn't clear. Also, it's not
just for U3 right? For highspeed, it's L2.

How about the name that Roger use: "snps,gadget-keep-connect-sys-sleep"

> +    description:
> +      If True then dwc3 runtime suspend is allowed during bus suspend
> +      case even with the USB cable connected.
> +
>  unevaluatedProperties: false
>  
>  required:
> -- 
> 2.17.1
> 

Did you Cc Rob, the devicetree maintainer?

Thanks,
Thinh
Re: [PATCH v3 2/3] dt-bindings: usb: snps,dwc3: Add allow-rtsusp-on-u3 property
Posted by Elson Serrao 2 years, 7 months ago

On 7/11/2023 2:56 PM, Thinh Nguyen wrote:
> On Tue, Jul 11, 2023, 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@quicinc.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..18ad99a26dd9 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,allow-rtsusp-on-u3:
> 
> Please spell out the whole thing as "rtsusp" isn't clear. Also, it's not
> just for U3 right? For highspeed, it's L2.
> 
> How about the name that Roger use: "snps,gadget-keep-connect-sys-sleep"
> 
Done. Will make that modification and upload v4

>> +    description:
>> +      If True then dwc3 runtime suspend is allowed during bus suspend
>> +      case even with the USB cable connected.
>> +
>>   unevaluatedProperties: false
>>   
>>   required:
>> -- 
>> 2.17.1
>>
> 
> Did you Cc Rob, the devicetree maintainer?
> 

My bad. Thanks for pointing this out
Re: [PATCH v3 2/3] dt-bindings: usb: snps,dwc3: Add allow-rtsusp-on-u3 property
Posted by Roger Quadros 2 years, 7 months ago
+Rob

On 12/07/2023 00:56, Thinh Nguyen wrote:
> On Tue, Jul 11, 2023, 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@quicinc.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..18ad99a26dd9 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,allow-rtsusp-on-u3:
> 
> Please spell out the whole thing as "rtsusp" isn't clear. Also, it's not
> just for U3 right? For highspeed, it's L2.
> 
> How about the name that Roger use: "snps,gadget-keep-connect-sys-sleep"

That property was meant to keep the USB device controller connected
during system sleep. So that name may not be appropriate here as this
is about allowing controller runtime suspend during USB suspend.

> 
>> +    description:
>> +      If True then dwc3 runtime suspend is allowed during bus suspend
>> +      case even with the USB cable connected.
>> +
>>  unevaluatedProperties: false
>>  
>>  required:
>> -- 
>> 2.17.1
>>
> 
> Did you Cc Rob, the devicetree maintainer?
> 
> Thanks,
> Thinh

-- 
cheers,
-roger
Re: [PATCH v3 2/3] dt-bindings: usb: snps,dwc3: Add allow-rtsusp-on-u3 property
Posted by Thinh Nguyen 2 years, 7 months ago
On Wed, Jul 12, 2023, Roger Quadros wrote:
> +Rob
> 
> On 12/07/2023 00:56, Thinh Nguyen wrote:
> > On Tue, Jul 11, 2023, 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@quicinc.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..18ad99a26dd9 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,allow-rtsusp-on-u3:
> > 
> > Please spell out the whole thing as "rtsusp" isn't clear. Also, it's not
> > just for U3 right? For highspeed, it's L2.
> > 
> > How about the name that Roger use: "snps,gadget-keep-connect-sys-sleep"
> 
> That property was meant to keep the USB device controller connected
> during system sleep. So that name may not be appropriate here as this
> is about allowing controller runtime suspend during USB suspend.
> 

Ah... you're right. Don't know what I was thinking. They're not the
same, and they may need to be separated. I'm not great with the naming,
but it can be something like this: "snps,runtime-suspend-on-usb-suspend"

BTW, I assume that your platform (both from Roger and Elson) does not
turn off the bus that handle DMA of events on suspend? (e.g. AXI bus)
Looking at the change, it looks like the events are still written in the
event buffer.

Thanks,
Thinh
Re: [PATCH v3 2/3] dt-bindings: usb: snps,dwc3: Add allow-rtsusp-on-u3 property
Posted by Krzysztof Kozlowski 2 years, 7 months ago
On 11/07/2023 19:43, 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.

v3 and still ignoring maintainers?

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

You missed at least DT list (maybe more), so this won't be tested by
automated tooling. Performing review on untested code might be a waste
of time, thus I will skip this patch entirely till you follow the
process allowing the patch to be tested.

Please kindly resend and include all necessary To/Cc entries.


Best regards,
Krzysztof