[RFC PATCH 1/2] dt-bindings: usb: snps,dwc3: Add 'snps,gadget-keep-connect-sys-sleep'

[RFC PATCH 1/2] dt-bindings: usb: snps,dwc3: Add 'snps,gadget-keep-connect-sys-sleep'

[Roger Quadros]

The current USB gadget driver behaviour is to stop the controller
and disconnect from the bus during System sleep.

The 'snps,gadget-keep-connect-sys-sleep' property can be used to
change this behaviour and keep the controller active and connected
to the bus during System sleep. This is useful for applications
that want to enter a low power state when USB is suspended but
remain connected so they can resume activity on USB resume.

This feature introduces a new constraint if Gadget driver is connected
to USB host: i.e.  the gadget must be in USB suspend state to allow
a System sleep as we cannot process any USB transactions
when in System sleep.

The system hardware is responsible to detect the end of USB suspend
and wake up the system so we can begin processing the USB transactions
as soon as possible.

Cc: devicetree@vger.kernel.org
Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 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 be36956af53b..1ce8008e7fef 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -262,6 +262,11 @@ properties:
       asserts utmi_sleep_n.
     type: boolean
 
+  snps,gadget-keep-connect-sys-sleep:
+    description:
+      If True then gadget driver will not disconnect during system sleep.
+      System sleep will not be allowed if gadget is not already in USB suspend.
+
   snps,hird-threshold:
     description: HIRD threshold
     $ref: /schemas/types.yaml#/definitions/uint8
-- 
2.34.1

Re: [RFC PATCH 1/2] dt-bindings: usb: snps,dwc3: Add 'snps,gadget-keep-connect-sys-sleep'

[Rob Herring]

On Mon, 20 Mar 2023 11:34:46 +0200, Roger Quadros wrote:
> The current USB gadget driver behaviour is to stop the controller
> and disconnect from the bus during System sleep.
> 
> The 'snps,gadget-keep-connect-sys-sleep' property can be used to
> change this behaviour and keep the controller active and connected
> to the bus during System sleep. This is useful for applications
> that want to enter a low power state when USB is suspended but
> remain connected so they can resume activity on USB resume.
> 
> This feature introduces a new constraint if Gadget driver is connected
> to USB host: i.e.  the gadget must be in USB suspend state to allow
> a System sleep as we cannot process any USB transactions
> when in System sleep.
> 
> The system hardware is responsible to detect the end of USB suspend
> and wake up the system so we can begin processing the USB transactions
> as soon as possible.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/usb/samsung,exynos-dwc3.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/usb/snps,dwc3.yaml
./Documentation/devicetree/bindings/usb/intel,keembay-dwc3.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/usb/snps,dwc3.yaml
./Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/usb/snps,dwc3.yaml
./Documentation/devicetree/bindings/usb/qcom,dwc3.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/usb/snps,dwc3.yaml
./Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/usb/snps,dwc3.yaml
./Documentation/devicetree/bindings/usb/fsl,imx8mp-dwc3.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/usb/snps,dwc3.yaml
./Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/usb/snps,dwc3.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/snps,dwc3.yaml: properties:snps,gadget-keep-connect-sys-sleep: 'oneOf' conditional failed, one must be fixed:
	'type' is a required property
		hint: A vendor boolean property can use "type: boolean"
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/snps,dwc3.yaml: properties:snps,gadget-keep-connect-sys-sleep: 'oneOf' conditional failed, one must be fixed:
		'enum' is a required property
		'const' is a required property
		hint: A vendor string property with exact values has an implicit type
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/snps,dwc3.yaml: properties:snps,gadget-keep-connect-sys-sleep: 'oneOf' conditional failed, one must be fixed:
		'$ref' is a required property
		'allOf' is a required property
		hint: A vendor property needs a $ref to types.yaml
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
./Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/usb/snps,dwc3.yaml
./Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/usb/snps,dwc3.yaml
./Documentation/devicetree/bindings/usb/ti,am62-usb.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/usb/snps,dwc3.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230320093447.32105-2-rogerq@kernel.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [RFC PATCH 1/2] dt-bindings: usb: snps,dwc3: Add 'snps,gadget-keep-connect-sys-sleep'

[Rob Herring]

On Mon, Mar 20, 2023 at 11:34:46AM +0200, Roger Quadros wrote:
> The current USB gadget driver behaviour is to stop the controller
> and disconnect from the bus during System sleep.

What's USB gadget? ;)

> The 'snps,gadget-keep-connect-sys-sleep' property can be used to
> change this behaviour and keep the controller active and connected
> to the bus during System sleep. This is useful for applications
> that want to enter a low power state when USB is suspended but
> remain connected so they can resume activity on USB resume.
> 
> This feature introduces a new constraint if Gadget driver is connected
> to USB host: i.e.  the gadget must be in USB suspend state to allow
> a System sleep as we cannot process any USB transactions
> when in System sleep.
> 
> The system hardware is responsible to detect the end of USB suspend
> and wake up the system so we can begin processing the USB transactions
> as soon as possible.

Sounds like something the user/OS would want to choose rather than fixed 
by your board's firmware.

Is this somehow DWC3 specific? If not, why a DWC3 specific property?

> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  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 be36956af53b..1ce8008e7fef 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -262,6 +262,11 @@ properties:
>        asserts utmi_sleep_n.
>      type: boolean
>  
> +  snps,gadget-keep-connect-sys-sleep:
> +    description:
> +      If True then gadget driver will not disconnect during system sleep.
> +      System sleep will not be allowed if gadget is not already in USB suspend.

'gadget' is a Linuxism.

> +
>    snps,hird-threshold:
>      description: HIRD threshold
>      $ref: /schemas/types.yaml#/definitions/uint8
> -- 
> 2.34.1
> 

Re: [RFC PATCH 1/2] dt-bindings: usb: snps,dwc3: Add 'snps,gadget-keep-connect-sys-sleep'

[Roger Quadros]

On 20/03/2023 15:22, Rob Herring wrote:
> On Mon, Mar 20, 2023 at 11:34:46AM +0200, Roger Quadros wrote:
>> The current USB gadget driver behaviour is to stop the controller
>> and disconnect from the bus during System sleep.
> 
> What's USB gadget? ;)

:)

> 
>> The 'snps,gadget-keep-connect-sys-sleep' property can be used to
>> change this behaviour and keep the controller active and connected
>> to the bus during System sleep. This is useful for applications
>> that want to enter a low power state when USB is suspended but
>> remain connected so they can resume activity on USB resume.
>>
>> This feature introduces a new constraint if Gadget driver is connected
>> to USB host: i.e.  the gadget must be in USB suspend state to allow
>> a System sleep as we cannot process any USB transactions
>> when in System sleep.
>>
>> The system hardware is responsible to detect the end of USB suspend
>> and wake up the system so we can begin processing the USB transactions
>> as soon as possible.
> 
> Sounds like something the user/OS would want to choose rather than fixed 
> by your board's firmware.

Yes.

> 
> Is this somehow DWC3 specific? If not, why a DWC3 specific property?

This is not DWC3 specific. 

Should we make this a UDC class device's sysfs attribute instead?
Only concern is that in dual-role case, if a role switch from
device mode to host mode and back to device mode happens, we loose
the UDC device's attributes as we re-init the UDC device.

Or should we make it a udc_core module parameter? This should be
persistent between role switches.

> 
>>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  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 be36956af53b..1ce8008e7fef 100644
>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> @@ -262,6 +262,11 @@ properties:
>>        asserts utmi_sleep_n.
>>      type: boolean
>>  
>> +  snps,gadget-keep-connect-sys-sleep:
>> +    description:
>> +      If True then gadget driver will not disconnect during system sleep.
>> +      System sleep will not be allowed if gadget is not already in USB suspend.
> 
> 'gadget' is a Linuxism.

Got it. Will avoid using it ;)

> 
>> +
>>    snps,hird-threshold:
>>      description: HIRD threshold
>>      $ref: /schemas/types.yaml#/definitions/uint8
>> -- 
>> 2.34.1
>>

cheers,
-roger