2021-04-30 03:20:34

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 1/2] dt-bindings: usb: Document the Allwinner H6 DWC3 glue

The RST_BUS_XHCI reset line in the H6 affects both the DWC3 core and the
USB3 PHY. This suggests the reset line controls the USB3 IP as a whole.
Represent this by attaching the reset line to a glue layer device.

Signed-off-by: Samuel Holland <[email protected]>
---
.../usb/allwinner,sun50i-h6-dwc3.yaml | 75 +++++++++++++++++++
1 file changed, 75 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml

diff --git a/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml b/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml
new file mode 100644
index 000000000000..936b5c74043f
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/allwinner,sun50i-h6-dwc3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner H6 DWC3 USB controller
+
+maintainers:
+ - Chen-Yu Tsai <[email protected]>
+ - Maxime Ripard <[email protected]>
+
+properties:
+ compatible:
+ const: allwinner,sun50i-h6-dwc3
+
+ "#address-cells": true
+
+ "#size-cells": true
+
+ ranges: true
+
+ resets:
+ maxItems: 1
+
+# Required child node:
+
+patternProperties:
+ "^phy@[0-9a-f]+$":
+ $ref: ../phy/allwinner,sun50i-h6-usb3-phy.yaml#
+
+ "^usb@[0-9a-f]+$":
+ $ref: snps,dwc3.yaml#
+
+required:
+ - compatible
+ - ranges
+ - resets
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/sun50i-h6-ccu.h>
+ #include <dt-bindings/reset/sun50i-h6-ccu.h>
+
+ usb3: usb@5200000 {
+ compatible = "allwinner,sun50i-h6-dwc3";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ resets = <&ccu RST_BUS_XHCI>;
+
+ dwc3: usb@5200000 {
+ compatible = "snps,dwc3";
+ reg = <0x05200000 0x10000>;
+ interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_XHCI>,
+ <&ccu CLK_BUS_XHCI>,
+ <&rtc 0>;
+ clock-names = "ref", "bus_early", "suspend";
+ dr_mode = "host";
+ phys = <&usb3phy>;
+ phy-names = "usb3-phy";
+ };
+
+ usb3phy: phy@5210000 {
+ compatible = "allwinner,sun50i-h6-usb3-phy";
+ reg = <0x5210000 0x10000>;
+ clocks = <&ccu CLK_USB_PHY1>;
+ resets = <&ccu RST_USB_PHY1>;
+ #phy-cells = <0>;
+ };
+ };
--
2.26.3


2021-04-30 15:28:45

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: usb: Document the Allwinner H6 DWC3 glue

On Thu, 29 Apr 2021 22:19:11 -0500, Samuel Holland wrote:
> The RST_BUS_XHCI reset line in the H6 affects both the DWC3 core and the
> USB3 PHY. This suggests the reset line controls the USB3 IP as a whole.
> Represent this by attaching the reset line to a glue layer device.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
> .../usb/allwinner,sun50i-h6-dwc3.yaml | 75 +++++++++++++++++++
> 1 file changed, 75 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml
>

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/allwinner,sun50i-h6-dwc3.example.dts:23.27-50.11: Warning (unit_address_vs_reg): /example-0/usb@5200000: node has a unit name, but no reg or ranges property
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.example.dt.yaml: usb@5200000: usb@5200000:phy-names:0: 'usb2-phy' was expected
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.example.dt.yaml: usb@5200000: phy-names:0: 'usb2-phy' was expected
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/snps,dwc3.yaml

See https://patchwork.ozlabs.org/patch/1471948

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

2021-04-30 19:48:36

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: usb: Document the Allwinner H6 DWC3 glue

On Thu, Apr 29, 2021 at 10:19:11PM -0500, Samuel Holland wrote:
> The RST_BUS_XHCI reset line in the H6 affects both the DWC3 core and the
> USB3 PHY. This suggests the reset line controls the USB3 IP as a whole.
> Represent this by attaching the reset line to a glue layer device.

Does that really mean anything more than a shared reset? Doesn't the
reset code support shared resets?

>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
> .../usb/allwinner,sun50i-h6-dwc3.yaml | 75 +++++++++++++++++++
> 1 file changed, 75 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml
>
> diff --git a/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml b/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml
> new file mode 100644
> index 000000000000..936b5c74043f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/allwinner,sun50i-h6-dwc3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner H6 DWC3 USB controller
> +
> +maintainers:
> + - Chen-Yu Tsai <[email protected]>
> + - Maxime Ripard <[email protected]>
> +
> +properties:
> + compatible:
> + const: allwinner,sun50i-h6-dwc3
> +
> + "#address-cells": true
> +
> + "#size-cells": true
> +
> + ranges: true
> +
> + resets:
> + maxItems: 1
> +
> +# Required child node:
> +
> +patternProperties:
> + "^phy@[0-9a-f]+$":
> + $ref: ../phy/allwinner,sun50i-h6-usb3-phy.yaml#
> +
> + "^usb@[0-9a-f]+$":
> + $ref: snps,dwc3.yaml#
> +
> +required:
> + - compatible
> + - ranges
> + - resets
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/sun50i-h6-ccu.h>
> + #include <dt-bindings/reset/sun50i-h6-ccu.h>
> +
> + usb3: usb@5200000 {
> + compatible = "allwinner,sun50i-h6-dwc3";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + resets = <&ccu RST_BUS_XHCI>;
> +
> + dwc3: usb@5200000 {
> + compatible = "snps,dwc3";
> + reg = <0x05200000 0x10000>;
> + interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&ccu CLK_BUS_XHCI>,
> + <&ccu CLK_BUS_XHCI>,
> + <&rtc 0>;
> + clock-names = "ref", "bus_early", "suspend";
> + dr_mode = "host";
> + phys = <&usb3phy>;
> + phy-names = "usb3-phy";
> + };
> +
> + usb3phy: phy@5210000 {
> + compatible = "allwinner,sun50i-h6-usb3-phy";
> + reg = <0x5210000 0x10000>;
> + clocks = <&ccu CLK_USB_PHY1>;
> + resets = <&ccu RST_USB_PHY1>;
> + #phy-cells = <0>;
> + };
> + };
> --
> 2.26.3
>

2021-05-10 07:32:52

by Samuel Holland

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: usb: Document the Allwinner H6 DWC3 glue

On 4/30/21 2:44 PM, Rob Herring wrote:
> On Thu, Apr 29, 2021 at 10:19:11PM -0500, Samuel Holland wrote:
>> The RST_BUS_XHCI reset line in the H6 affects both the DWC3 core and the
>> USB3 PHY. This suggests the reset line controls the USB3 IP as a whole.
>> Represent this by attaching the reset line to a glue layer device.
>
> Does that really mean anything more than a shared reset? Doesn't the
> reset code support shared resets?

Yes, shared resets are supported.

I don't know exactly what it means -- the hardware's platform
integration is not well documented. The vendor BSP groups the controller
+ PHY together as a single node/device, and enables resources all at
once, so it is not clear which resources belong to which part.

The hardware layout appeared to be similar to other platforms, so it
seemed reasonable that the binding should be similar as well. Only
unipher and hi3660 put the resets property in the snps,dwc3 node itself;
the vast majority of platforms put it one level higher. I don't know
which is more "correct".

The benefit of following the more common binding is that it allows
sharing the glue code. The downside is that it is a bit more
complicated, and the warning for "ranges;", which several other
platforms appear to have as well.

>>
>> Signed-off-by: Samuel Holland <[email protected]>
>> ---
>> .../usb/allwinner,sun50i-h6-dwc3.yaml | 75 +++++++++++++++++++
>> 1 file changed, 75 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml b/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml
>> new file mode 100644
>> index 000000000000..936b5c74043f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/allwinner,sun50i-h6-dwc3.yaml
>> @@ -0,0 +1,75 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/usb/allwinner,sun50i-h6-dwc3.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Allwinner H6 DWC3 USB controller
>> +
>> +maintainers:
>> + - Chen-Yu Tsai <[email protected]>
>> + - Maxime Ripard <[email protected]>
>> +
>> +properties:
>> + compatible:
>> + const: allwinner,sun50i-h6-dwc3
>> +
>> + "#address-cells": true
>> +
>> + "#size-cells": true
>> +
>> + ranges: true
>> +
>> + resets:
>> + maxItems: 1
>> +
>> +# Required child node:
>> +
>> +patternProperties:
>> + "^phy@[0-9a-f]+$":
>> + $ref: ../phy/allwinner,sun50i-h6-usb3-phy.yaml#
>> +
>> + "^usb@[0-9a-f]+$":
>> + $ref: snps,dwc3.yaml#
>> +
>> +required:
>> + - compatible
>> + - ranges
>> + - resets
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/clock/sun50i-h6-ccu.h>
>> + #include <dt-bindings/reset/sun50i-h6-ccu.h>
>> +
>> + usb3: usb@5200000 {
>> + compatible = "allwinner,sun50i-h6-dwc3";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> + resets = <&ccu RST_BUS_XHCI>;
>> +
>> + dwc3: usb@5200000 {
>> + compatible = "snps,dwc3";
>> + reg = <0x05200000 0x10000>;
>> + interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&ccu CLK_BUS_XHCI>,
>> + <&ccu CLK_BUS_XHCI>,
>> + <&rtc 0>;
>> + clock-names = "ref", "bus_early", "suspend";
>> + dr_mode = "host";
>> + phys = <&usb3phy>;
>> + phy-names = "usb3-phy";
>> + };
>> +
>> + usb3phy: phy@5210000 {
>> + compatible = "allwinner,sun50i-h6-usb3-phy";
>> + reg = <0x5210000 0x10000>;
>> + clocks = <&ccu CLK_USB_PHY1>;
>> + resets = <&ccu RST_USB_PHY1>;
>> + #phy-cells = <0>;
>> + };
>> + };
>> --
>> 2.26.3
>>