2023-01-18 19:51:00

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v2 2/2] dt-bindings: usb: rockchip,dwc3: Move RK3399 to its own schema

The rockchip,dwc3.yaml schema defines a single DWC3 node, but the RK3399
uses the discouraged parent wrapper node and child 'generic' DWC3 node.
The intent was to modify the RK3399 DTs to use a single node, but the DT
changes were rejected for ABI reasons. However, the schema was accepted
as-is.

To fix this, we need to move the RK3399 binding to its own schema file.
The RK3328 and RK3568 bindings are correct and use a single node.

Cc: Johan Jonker <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
---
.../bindings/usb/rockchip,dwc3.yaml | 10 +-
.../bindings/usb/rockchip,rk3399-dwc3.yaml | 115 ++++++++++++++++++
2 files changed, 119 insertions(+), 6 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml

diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
index b3798d94d2fd..edb130c780e4 100644
--- a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
@@ -29,7 +29,6 @@ select:
contains:
enum:
- rockchip,rk3328-dwc3
- - rockchip,rk3399-dwc3
- rockchip,rk3568-dwc3
required:
- compatible
@@ -39,7 +38,6 @@ properties:
items:
- enum:
- rockchip,rk3328-dwc3
- - rockchip,rk3399-dwc3
- rockchip,rk3568-dwc3
- const: snps,dwc3

@@ -90,7 +88,7 @@ required:

examples:
- |
- #include <dt-bindings/clock/rk3399-cru.h>
+ #include <dt-bindings/clock/rk3328-cru.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>

bus {
@@ -98,11 +96,11 @@ examples:
#size-cells = <2>;

usbdrd3_0: usb@fe800000 {
- compatible = "rockchip,rk3399-dwc3", "snps,dwc3";
+ compatible = "rockchip,rk3328-dwc3", "snps,dwc3";
reg = <0x0 0xfe800000 0x0 0x100000>;
interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
- <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_GRF>;
+ clocks = <&cru SCLK_USB3OTG_REF>, <&cru SCLK_USB3OTG_SUSPEND>,
+ <&cru ACLK_USB3OTG>;
clock-names = "ref_clk", "suspend_clk",
"bus_clk", "grf_clk";
dr_mode = "otg";
diff --git a/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
new file mode 100644
index 000000000000..e39a8a3a7ab3
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/rockchip,rk3399-dwc3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip RK3399 SuperSpeed DWC3 USB SoC controller
+
+maintainers:
+ - Heiko Stuebner <[email protected]>
+
+properties:
+ compatible:
+ const: rockchip,rk3399-dwc3
+
+ '#address-cells':
+ const: 2
+
+ '#size-cells':
+ const: 2
+
+ ranges: true
+
+ clocks:
+ items:
+ - description:
+ Controller reference clock, must to be 24 MHz
+ - description:
+ Controller suspend clock, must to be 24 MHz or 32 KHz
+ - description:
+ Master/Core clock, must to be >= 62.5 MHz for SS
+ operation and >= 30MHz for HS operation
+ - description:
+ USB3 aclk peri
+ - description:
+ USB3 aclk
+ - description:
+ Controller grf clock
+
+ clock-names:
+ items:
+ - const: ref_clk
+ - const: suspend_clk
+ - const: bus_clk
+ - const: aclk_usb3_rksoc_axi_perf
+ - const: aclk_usb3
+ - const: grf_clk
+
+ resets:
+ maxItems: 1
+
+ reset-names:
+ const: usb3-otg
+
+patternProperties:
+ '^usb@':
+ $ref: snps,dwc3.yaml#
+
+additionalProperties: false
+
+required:
+ - compatible
+ - '#address-cells'
+ - '#size-cells'
+ - ranges
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+
+examples:
+ - |
+ #include <dt-bindings/clock/rk3399-cru.h>
+ #include <dt-bindings/power/rk3399-power.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ bus {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ usb {
+ compatible = "rockchip,rk3399-dwc3";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+ clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
+ <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_RKSOC_AXI_PERF>,
+ <&cru ACLK_USB3>, <&cru ACLK_USB3_GRF>;
+ clock-names = "ref_clk", "suspend_clk",
+ "bus_clk", "aclk_usb3_rksoc_axi_perf",
+ "aclk_usb3", "grf_clk";
+ resets = <&cru SRST_A_USB3_OTG0>;
+ reset-names = "usb3-otg";
+
+ usb@fe800000 {
+ compatible = "snps,dwc3";
+ reg = <0x0 0xfe800000 0x0 0x100000>;
+ interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
+ <&cru SCLK_USB3OTG0_SUSPEND>;
+ clock-names = "ref", "bus_early", "suspend";
+ dr_mode = "otg";
+ phys = <&u2phy0_otg>, <&tcphy0_usb3>;
+ phy-names = "usb2-phy", "usb3-phy";
+ phy_type = "utmi_wide";
+ snps,dis_enblslpm_quirk;
+ snps,dis-u2-freeclk-exists-quirk;
+ snps,dis_u2_susphy_quirk;
+ snps,dis-del-phy-power-chg-quirk;
+ snps,dis-tx-ipgap-linecheck-quirk;
+ power-domains = <&power RK3399_PD_USB3>;
+ };
+ };
+ };
+...
--
2.39.0


2023-01-18 21:38:36

by Johan Jonker

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: usb: rockchip,dwc3: Move RK3399 to its own schema

Hi,

Some alignment at the examples and the unknown extcon property.

usb@fe800000: 'extcon' does not match any of the regexes

Johan

On 1/18/23 20:30, Rob Herring wrote:
> The rockchip,dwc3.yaml schema defines a single DWC3 node, but the RK3399
> uses the discouraged parent wrapper node and child 'generic' DWC3 node.
> The intent was to modify the RK3399 DTs to use a single node, but the DT
> changes were rejected for ABI reasons. However, the schema was accepted
> as-is.
>
> To fix this, we need to move the RK3399 binding to its own schema file.
> The RK3328 and RK3568 bindings are correct and use a single node.
>
> Cc: Johan Jonker <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
> ---
> .../bindings/usb/rockchip,dwc3.yaml | 10 +-
> .../bindings/usb/rockchip,rk3399-dwc3.yaml | 115 ++++++++++++++++++
> 2 files changed, 119 insertions(+), 6 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
>
> diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> index b3798d94d2fd..edb130c780e4 100644
> --- a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> @@ -29,7 +29,6 @@ select:
> contains:
> enum:
> - rockchip,rk3328-dwc3
> - - rockchip,rk3399-dwc3
> - rockchip,rk3568-dwc3
> required:
> - compatible
> @@ -39,7 +38,6 @@ properties:
> items:
> - enum:
> - rockchip,rk3328-dwc3
> - - rockchip,rk3399-dwc3
> - rockchip,rk3568-dwc3
> - const: snps,dwc3
>
> @@ -90,7 +88,7 @@ required:
>
> examples:
> - |
> - #include <dt-bindings/clock/rk3399-cru.h>
> + #include <dt-bindings/clock/rk3328-cru.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
>
> bus {
> @@ -98,11 +96,11 @@ examples:
> #size-cells = <2>;
>
> usbdrd3_0: usb@fe800000 {
> - compatible = "rockchip,rk3399-dwc3", "snps,dwc3";
> + compatible = "rockchip,rk3328-dwc3", "snps,dwc3";
> reg = <0x0 0xfe800000 0x0 0x100000>;
> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
> - <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_GRF>;
> + clocks = <&cru SCLK_USB3OTG_REF>, <&cru SCLK_USB3OTG_SUSPEND>,

> + <&cru ACLK_USB3OTG>;

align

> clock-names = "ref_clk", "suspend_clk",
> "bus_clk", "grf_clk";
> dr_mode = "otg";
> diff --git a/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
> new file mode 100644
> index 000000000000..e39a8a3a7ab3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/rockchip,rk3399-dwc3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip RK3399 SuperSpeed DWC3 USB SoC controller
> +
> +maintainers:
> + - Heiko Stuebner <[email protected]>
> +
> +properties:
> + compatible:
> + const: rockchip,rk3399-dwc3
> +
> + '#address-cells':
> + const: 2
> +
> + '#size-cells':
> + const: 2
> +
> + ranges: true
> +
> + clocks:
> + items:
> + - description:
> + Controller reference clock, must to be 24 MHz
> + - description:
> + Controller suspend clock, must to be 24 MHz or 32 KHz
> + - description:
> + Master/Core clock, must to be >= 62.5 MHz for SS
> + operation and >= 30MHz for HS operation
> + - description:
> + USB3 aclk peri
> + - description:
> + USB3 aclk
> + - description:
> + Controller grf clock
> +
> + clock-names:
> + items:
> + - const: ref_clk
> + - const: suspend_clk
> + - const: bus_clk
> + - const: aclk_usb3_rksoc_axi_perf
> + - const: aclk_usb3
> + - const: grf_clk
> +
> + resets:
> + maxItems: 1
> +
> + reset-names:
> + const: usb3-otg
> +
> +patternProperties:
> + '^usb@':
> + $ref: snps,dwc3.yaml#
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - '#address-cells'
> + - '#size-cells'
> + - ranges
> + - clocks
> + - clock-names
> + - resets
> + - reset-names
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/rk3399-cru.h>
> + #include <dt-bindings/power/rk3399-power.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + bus {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + usb {
> + compatible = "rockchip,rk3399-dwc3";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,

> + <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_RKSOC_AXI_PERF>,
> + <&cru ACLK_USB3>, <&cru ACLK_USB3_GRF>;

align

> + clock-names = "ref_clk", "suspend_clk",

> + "bus_clk", "aclk_usb3_rksoc_axi_perf",
> + "aclk_usb3", "grf_clk";

align

> + resets = <&cru SRST_A_USB3_OTG0>;
> + reset-names = "usb3-otg";
> +
> + usb@fe800000 {
> + compatible = "snps,dwc3";
> + reg = <0x0 0xfe800000 0x0 0x100000>;
> + interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,

> + <&cru SCLK_USB3OTG0_SUSPEND>;

align

> + clock-names = "ref", "bus_early", "suspend";
> + dr_mode = "otg";
> + phys = <&u2phy0_otg>, <&tcphy0_usb3>;
> + phy-names = "usb2-phy", "usb3-phy";
> + phy_type = "utmi_wide";
> + snps,dis_enblslpm_quirk;
> + snps,dis-u2-freeclk-exists-quirk;
> + snps,dis_u2_susphy_quirk;
> + snps,dis-del-phy-power-chg-quirk;
> + snps,dis-tx-ipgap-linecheck-quirk;
> + power-domains = <&power RK3399_PD_USB3>;
> + };
> + };
> + };
> +...

2023-01-20 21:01:19

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: usb: rockchip,dwc3: Move RK3399 to its own schema

On Wed, Jan 18, 2023 at 3:05 PM Johan Jonker <[email protected]> wrote:
>
> Hi,
>
> Some alignment at the examples and the unknown extcon property.
>
> usb@fe800000: 'extcon' does not match any of the regexes

Does that go in the wrapper or dwc3 node?:

arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dtb: usb@fe800000:
usb@fe800000: Unevaluated properties are not allowed ('extcon' was
unexpected)
From schema:
/home/rob/proj/linux-dt/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml

That's the dwc3 node, but the majority are in the wrapper node, so I'm
going with the majority and leaving this one.

Rob

2023-01-21 00:08:45

by Johan Jonker

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: usb: rockchip,dwc3: Move RK3399 to its own schema



On 1/20/23 21:30, Rob Herring wrote:
> On Wed, Jan 18, 2023 at 3:05 PM Johan Jonker <[email protected]> wrote:
>>
>> Hi,
>>
>> Some alignment at the examples and the unknown extcon property.
>>
>> usb@fe800000: 'extcon' does not match any of the regexes
>
> Does that go in the wrapper or dwc3 node?:
>
> arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dtb: usb@fe800000:
> usb@fe800000: Unevaluated properties are not allowed ('extcon' was
> unexpected)
> From schema:
> /home/rob/proj/linux-dt/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
>


> That's the dwc3 node, but the majority are in the wrapper node, so I'm
> going with the majority and leaving this one.

In wrapper code for rk33899 in dwc3-of-simple.c I don't see no extcon activity I think.

In core there's recently made some changes:
https://github.com/torvalds/linux/blame/master/drivers/usb/dwc3/core.c#L1710

usb: dwc3: Don't switch OTG -> peripheral if extcon is present
https://lore.kernel.org/all/[email protected]/

Binding status update for that is unknown for me.
Do whatever suites you best.

Johan
> Rob

2023-01-23 21:07:14

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: usb: rockchip,dwc3: Move RK3399 to its own schema

On Sat, Jan 21, 2023 at 12:40:12AM +0100, Johan Jonker wrote:
>
>
> On 1/20/23 21:30, Rob Herring wrote:
> > On Wed, Jan 18, 2023 at 3:05 PM Johan Jonker <[email protected]> wrote:
> >>
> >> Hi,
> >>
> >> Some alignment at the examples and the unknown extcon property.
> >>
> >> usb@fe800000: 'extcon' does not match any of the regexes
> >
> > Does that go in the wrapper or dwc3 node?:
> >
> > arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dtb: usb@fe800000:
> > usb@fe800000: Unevaluated properties are not allowed ('extcon' was
> > unexpected)
> > From schema:
> > /home/rob/proj/linux-dt/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
> >
>
>
> > That's the dwc3 node, but the majority are in the wrapper node, so I'm
> > going with the majority and leaving this one.
>
> In wrapper code for rk33899 in dwc3-of-simple.c I don't see no extcon activity I think.

So all the other cases are just a stray property and the above one is
correct.

'extcon' should be replaced in favor of a connector node, so if not
used, it should be able to be removed. Though maybe BSD or something
else cares.

> In core there's recently made some changes:
> https://github.com/torvalds/linux/blame/master/drivers/usb/dwc3/core.c#L1710
>
> usb: dwc3: Don't switch OTG -> peripheral if extcon is present
> https://lore.kernel.org/all/[email protected]/
>
> Binding status update for that is unknown for me.
> Do whatever suites you best.

I'm going to leave it as-is for now.

Rob