[PATCH 34/54] dt-bindings: usb: Convert SMSC USB3503 binding to a schema

[PATCH 34/54] dt-bindings: usb: Convert SMSC USB3503 binding to a schema

[Maxime Ripard]

The SMSC USB3503 USB Hub Controller is supported by Linux thanks to
its device tree binding.

Now that we have the DT validation in place, let's convert the device
tree bindings for that driver over to a YAML schema.

Cc: Dongjin Kim <tobetter@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 .../devicetree/bindings/usb/smsc,usb3503.yaml | 104 ++++++++++++++++++
 .../devicetree/bindings/usb/usb3503.txt       |  39 -------
 2 files changed, 104 insertions(+), 39 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/smsc,usb3503.yaml
 delete mode 100644 Documentation/devicetree/bindings/usb/usb3503.txt

diff --git a/Documentation/devicetree/bindings/usb/smsc,usb3503.yaml b/Documentation/devicetree/bindings/usb/smsc,usb3503.yaml
new file mode 100644
index 000000000000..0e5622e7df87
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/smsc,usb3503.yaml
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/smsc,usb3503.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SMSC USB3503 High-Speed Hub Controller Device Tree Bindings
+
+maintainers:
+  - Dongjin Kim <tobetter@gmail.com>
+
+properties:
+  compatible:
+    enum:
+      - smsc,usb3503
+      - smsc,usb3503a
+
+  reg:
+    maxItems: 1
+
+  connect-gpios:
+    description: >
+      GPIO for connect
+
+  intn-gpios:
+    description: >
+      GPIO for interrupt
+
+  reset-gpios:
+    description: >
+      GPIO for reset
+
+  disabled-ports:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 3
+    items:
+      minimum: 1
+      maximum: 3
+    description: >
+      Specifies the ports unused using their port number. Do not describe this
+      property if all ports have to be enabled.
+
+  initial-mode:
+    enum: [1, 2]
+    description: >
+      Specifies initial mode. 1 for Hub mode, 2 for standby mode.
+
+  clocks:
+    description: >
+      Clock used for driving REFCLK signal. If not provided the driver assumes
+      that clock signal is always available, its rate is specified by REF_SEL
+      pins and a value from the primary reference clock frequencies table is
+      used.
+
+  clock-names:
+    const: refclk
+
+  refclk-frequency:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: >
+      Frequency of the REFCLK signal as defined by REF_SEL pins. If not
+      provided, driver will not set rate of the REFCLK signal and assume that a
+      value from the primary reference clock frequencies table is used.
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+      i2c {
+          #address-cells = <1>;
+          #size-cells = <0>;
+        
+          usb3503@8 {
+              compatible = "smsc,usb3503";
+              reg = <0x08>;
+              connect-gpios = <&gpx3 0 1>;
+              disabled-ports = <2 3>;
+              intn-gpios = <&gpx3 4 1>;
+              reset-gpios = <&gpx3 5 1>;
+              initial-mode = <1>;
+              clocks = <&clks 80>;
+              clock-names = "refclk";
+          };
+      };
+
+  - |
+      #include <dt-bindings/gpio/gpio.h>
+
+      usb-hub {
+          /* I2C is not connected */
+          compatible = "smsc,usb3503";
+          initial-mode = <1>; /* initialize in HUB mode */
+          disabled-ports = <1>;
+          intn-gpios = <&pio 7 5 GPIO_ACTIVE_HIGH>; /* PH5 */
+          reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
+          connect-gpios = <&pio 4 17 GPIO_ACTIVE_HIGH>; /* PE17 */
+          refclk-frequency = <19200000>;
+      };
+
+...
diff --git a/Documentation/devicetree/bindings/usb/usb3503.txt b/Documentation/devicetree/bindings/usb/usb3503.txt
deleted file mode 100644
index 057dd384d473..000000000000
--- a/Documentation/devicetree/bindings/usb/usb3503.txt
+++ /dev/null
@@ -1,39 +0,0 @@
-SMSC USB3503 High-Speed Hub Controller
-
-Required properties:
-- compatible: Should be "smsc,usb3503" or "smsc,usb3503a".
-
-Optional properties:
-- reg: Specifies the i2c slave address, it is required and should be 0x08
-       if I2C is used.
-- connect-gpios: Should specify GPIO for connect.
-- disabled-ports: Should specify the ports unused.
-	'1' or '2' or '3' are available for this property to describe the port
-	number. 1~3 property values are possible to be described.
-	Do not describe this property if all ports have to be enabled.
-- intn-gpios: Should specify GPIO for interrupt.
-- reset-gpios: Should specify GPIO for reset.
-- initial-mode: Should specify initial mode.
-                (1 for HUB mode, 2 for STANDBY mode)
-- refclk: Clock used for driving REFCLK signal (optional, if not provided
-	the driver assumes that clock signal is always available, its
-	rate is specified by REF_SEL pins and a value from the primary
-	reference clock frequencies table is used). Use clocks and
-	clock-names in order to assign it
-- refclk-frequency: Frequency of the REFCLK signal as defined by REF_SEL
-	pins (optional, if not provided, driver will not set rate of the
-	REFCLK signal and assume that a value from the primary reference
-	clock frequencies table is used)
-
-Examples:
-	usb3503@8 {
-		compatible = "smsc,usb3503";
-		reg = <0x08>;
-		connect-gpios = <&gpx3 0 1>;
-		disabled-ports = <2 3>;
-		intn-gpios = <&gpx3 4 1>;
-		reset-gpios = <&gpx3 5 1>;
-		initial-mode = <1>;
-		clocks = <&clks 80>;
-		clock-names = "refclk";
-	};
-- 
2.31.1

Re: [PATCH 34/54] dt-bindings: usb: Convert SMSC USB3503 binding to a schema

[Rob Herring]

On Wed, Jul 21, 2021 at 04:04:04PM +0200, Maxime Ripard wrote:
> The SMSC USB3503 USB Hub Controller is supported by Linux thanks to
> its device tree binding.
> 
> Now that we have the DT validation in place, let's convert the device
> tree bindings for that driver over to a YAML schema.
> 
> Cc: Dongjin Kim <tobetter@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  .../devicetree/bindings/usb/smsc,usb3503.yaml | 104 ++++++++++++++++++
>  .../devicetree/bindings/usb/usb3503.txt       |  39 -------
>  2 files changed, 104 insertions(+), 39 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/smsc,usb3503.yaml
>  delete mode 100644 Documentation/devicetree/bindings/usb/usb3503.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/smsc,usb3503.yaml b/Documentation/devicetree/bindings/usb/smsc,usb3503.yaml
> new file mode 100644
> index 000000000000..0e5622e7df87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/smsc,usb3503.yaml
> @@ -0,0 +1,104 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/smsc,usb3503.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SMSC USB3503 High-Speed Hub Controller Device Tree Bindings
> +
> +maintainers:
> +  - Dongjin Kim <tobetter@gmail.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - smsc,usb3503
> +      - smsc,usb3503a
> +
> +  reg:
> +    maxItems: 1
> +
> +  connect-gpios:
> +    description: >
> +      GPIO for connect

maxItems: 1
> +
> +  intn-gpios:
> +    description: >
> +      GPIO for interrupt

maxItems: 1

> +
> +  reset-gpios:
> +    description: >
> +      GPIO for reset

maxItems: 1

> +
> +  disabled-ports:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 3
> +    items:
> +      minimum: 1
> +      maximum: 3
> +    description: >
> +      Specifies the ports unused using their port number. Do not describe this
> +      property if all ports have to be enabled.
> +
> +  initial-mode:
> +    enum: [1, 2]
> +    description: >
> +      Specifies initial mode. 1 for Hub mode, 2 for standby mode.
> +
> +  clocks:
> +    description: >
> +      Clock used for driving REFCLK signal. If not provided the driver assumes
> +      that clock signal is always available, its rate is specified by REF_SEL
> +      pins and a value from the primary reference clock frequencies table is
> +      used.

maxItems: 1

> +
> +  clock-names:
> +    const: refclk
> +
> +  refclk-frequency:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: >
> +      Frequency of the REFCLK signal as defined by REF_SEL pins. If not
> +      provided, driver will not set rate of the REFCLK signal and assume that a
> +      value from the primary reference clock frequencies table is used.
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +      i2c {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +        
> +          usb3503@8 {

usb-hub@8


> +              compatible = "smsc,usb3503";
> +              reg = <0x08>;
> +              connect-gpios = <&gpx3 0 1>;
> +              disabled-ports = <2 3>;
> +              intn-gpios = <&gpx3 4 1>;
> +              reset-gpios = <&gpx3 5 1>;
> +              initial-mode = <1>;
> +              clocks = <&clks 80>;
> +              clock-names = "refclk";
> +          };
> +      };
> +
> +  - |
> +      #include <dt-bindings/gpio/gpio.h>
> +
> +      usb-hub {
> +          /* I2C is not connected */

We should probably require this is a USB bus child device in this case, 
but that's a separate change.

> +          compatible = "smsc,usb3503";
> +          initial-mode = <1>; /* initialize in HUB mode */
> +          disabled-ports = <1>;
> +          intn-gpios = <&pio 7 5 GPIO_ACTIVE_HIGH>; /* PH5 */
> +          reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
> +          connect-gpios = <&pio 4 17 GPIO_ACTIVE_HIGH>; /* PE17 */
> +          refclk-frequency = <19200000>;
> +      };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/usb/usb3503.txt b/Documentation/devicetree/bindings/usb/usb3503.txt
> deleted file mode 100644
> index 057dd384d473..000000000000
> --- a/Documentation/devicetree/bindings/usb/usb3503.txt
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -SMSC USB3503 High-Speed Hub Controller
> -
> -Required properties:
> -- compatible: Should be "smsc,usb3503" or "smsc,usb3503a".
> -
> -Optional properties:
> -- reg: Specifies the i2c slave address, it is required and should be 0x08
> -       if I2C is used.
> -- connect-gpios: Should specify GPIO for connect.
> -- disabled-ports: Should specify the ports unused.
> -	'1' or '2' or '3' are available for this property to describe the port
> -	number. 1~3 property values are possible to be described.
> -	Do not describe this property if all ports have to be enabled.
> -- intn-gpios: Should specify GPIO for interrupt.
> -- reset-gpios: Should specify GPIO for reset.
> -- initial-mode: Should specify initial mode.
> -                (1 for HUB mode, 2 for STANDBY mode)
> -- refclk: Clock used for driving REFCLK signal (optional, if not provided
> -	the driver assumes that clock signal is always available, its
> -	rate is specified by REF_SEL pins and a value from the primary
> -	reference clock frequencies table is used). Use clocks and
> -	clock-names in order to assign it
> -- refclk-frequency: Frequency of the REFCLK signal as defined by REF_SEL
> -	pins (optional, if not provided, driver will not set rate of the
> -	REFCLK signal and assume that a value from the primary reference
> -	clock frequencies table is used)
> -
> -Examples:
> -	usb3503@8 {
> -		compatible = "smsc,usb3503";
> -		reg = <0x08>;
> -		connect-gpios = <&gpx3 0 1>;
> -		disabled-ports = <2 3>;
> -		intn-gpios = <&gpx3 4 1>;
> -		reset-gpios = <&gpx3 5 1>;
> -		initial-mode = <1>;
> -		clocks = <&clks 80>;
> -		clock-names = "refclk";
> -	};
> -- 
> 2.31.1
> 
> 

[PATCH 35/54] dt-bindings: usb: dwc3: Fix usb-phy check

[Maxime Ripard]

The original binding was allowing any combination of usb2-phy and
usb3-phy in the phys and phy-names properties.

However, the current binding enforces that those properties must be a
list of usb2-phy and usb3-phy, with exactly one element, effectively
making usb2-phy the only value being valid.

Let's rework the properties description to allow either one or two
element picked with values either usb2-phy or usb3-phy. The rest of the
tooling makes sure that we don't get any duplicate value, so this should
be what we want.

Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 41416fbd92aa..6c3f7c9a76c0 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -73,15 +73,15 @@ properties:
 
   phys:
     minItems: 1
-    items:
-      - description: USB2/HS PHY
-      - description: USB3/SS PHY
+    maxItems: 2
 
   phy-names:
     minItems: 1
+    maxItems: 2
     items:
-      - const: usb2-phy
-      - const: usb3-phy
+      enum:
+        - usb2-phy
+        - usb3-phy
 
   resets:
     minItems: 1
-- 
2.31.1

Re: [PATCH 35/54] dt-bindings: usb: dwc3: Fix usb-phy check

[Rob Herring]

On Wed, Jul 21, 2021 at 04:04:05PM +0200, Maxime Ripard wrote:
> The original binding was allowing any combination of usb2-phy and
> usb3-phy in the phys and phy-names properties.
> 
> However, the current binding enforces that those properties must be a
> list of usb2-phy and usb3-phy, with exactly one element, effectively
> making usb2-phy the only value being valid.

Huh? If 'maxItems' is not specified, then it's the length of 'items' 
list.

> Let's rework the properties description to allow either one or two
> element picked with values either usb2-phy or usb3-phy. The rest of the
> tooling makes sure that we don't get any duplicate value, so this should
> be what we want.

Is it really valid to have only a USB3 PHY and what you want here? That 
would mean the USB3 phy also handles USB2, right?

> 
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index 41416fbd92aa..6c3f7c9a76c0 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -73,15 +73,15 @@ properties:
>  
>    phys:
>      minItems: 1
> -    items:
> -      - description: USB2/HS PHY
> -      - description: USB3/SS PHY
> +    maxItems: 2
>  
>    phy-names:
>      minItems: 1
> +    maxItems: 2
>      items:
> -      - const: usb2-phy
> -      - const: usb3-phy
> +      enum:
> +        - usb2-phy
> +        - usb3-phy
>  
>    resets:
>      minItems: 1
> -- 
> 2.31.1
> 
> 

Re: [PATCH 35/54] dt-bindings: usb: dwc3: Fix usb-phy check

[Maxime Ripard]

Hi Rob,

On Fri, Jul 23, 2021 at 04:15:30PM -0600, Rob Herring wrote:
> On Wed, Jul 21, 2021 at 04:04:05PM +0200, Maxime Ripard wrote:
> > The original binding was allowing any combination of usb2-phy and
> > usb3-phy in the phys and phy-names properties.
> > 
> > However, the current binding enforces that those properties must be a
> > list of usb2-phy and usb3-phy, with exactly one element, effectively
> > making usb2-phy the only value being valid.
> 
> Huh? If 'maxItems' is not specified, then it's the length of 'items' 
> list.

Even if minItems is set?

This doesn't really change anything to my issue though.

"""
 - phy-names: from the *Generic PHY* bindings; supported names are "usb2-phy"
        or "usb3-phy".
"""

The YAML binding has

  phy-names:
    minItems: 1
    items:
      - const: usb2-phy
      - const: usb3-phy

which means that only usb2-phy is accepted (and possibly usb2-phy,
usb3-phy) but only usb3-phy isn't anymore, while it was valid according
to the original binding and used in multiple DT across multiple vendors
(arch/arm64/boot/dts/hisilicon/hi3660.dtsi,
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi)

> > Let's rework the properties description to allow either one or two
> > element picked with values either usb2-phy or usb3-phy. The rest of the
> > tooling makes sure that we don't get any duplicate value, so this should
> > be what we want.
> 
> Is it really valid to have only a USB3 PHY and what you want here? That 
> would mean the USB3 phy also handles USB2, right?

IIRC that's how it works for the H6 at least yes.

Maxime

[PATCH 36/54] dt-bindings: usb: ehci: Add Allwinner A83t compatible

[Maxime Ripard]

The A83t EHCI compatible was introduced in device trees, but it was
never documented.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 Documentation/devicetree/bindings/usb/generic-ehci.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
index 8089dc956ba3..19217a8fbe22 100644
--- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
@@ -34,6 +34,7 @@ properties:
               - allwinner,sun6i-a31-ehci
               - allwinner,sun7i-a20-ehci
               - allwinner,sun8i-a23-ehci
+              - allwinner,sun8i-a83t-ehci
               - allwinner,sun8i-h3-ehci
               - allwinner,sun8i-r40-ehci
               - allwinner,sun9i-a80-ehci
-- 
2.31.1

Re: [PATCH 36/54] dt-bindings: usb: ehci: Add Allwinner A83t compatible

[Rob Herring]

On Wed, 21 Jul 2021 16:04:06 +0200, Maxime Ripard wrote:
> The A83t EHCI compatible was introduced in device trees, but it was
> never documented.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  Documentation/devicetree/bindings/usb/generic-ehci.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 37/54] dt-bindings: usb: ohci: Add Allwinner A83t compatible

[Maxime Ripard]

The A83t OHCI compatible was introduced in device trees, but it was
never documented.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 Documentation/devicetree/bindings/usb/generic-ohci.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/generic-ohci.yaml b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
index 0f5f6ea702d0..700e95262a8e 100644
--- a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
@@ -24,6 +24,7 @@ properties:
               - allwinner,sun6i-a31-ohci
               - allwinner,sun7i-a20-ohci
               - allwinner,sun8i-a23-ohci
+              - allwinner,sun8i-a83t-ohci
               - allwinner,sun8i-h3-ohci
               - allwinner,sun8i-r40-ohci
               - allwinner,sun9i-a80-ohci
-- 
2.31.1

Re: [PATCH 37/54] dt-bindings: usb: ohci: Add Allwinner A83t compatible

[Rob Herring]

On Wed, 21 Jul 2021 16:04:07 +0200, Maxime Ripard wrote:
> The A83t OHCI compatible was introduced in device trees, but it was
> never documented.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  Documentation/devicetree/bindings/usb/generic-ohci.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>