diff mbox series

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

Message ID 20210721140424.725744-36-maxime@cerno.tech (mailing list archive)
State New, archived
Headers show
Series ARM: dts: Last round of DT schema fixes | expand

Commit Message

Maxime Ripard July 21, 2021, 2:04 p.m. UTC
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(-)

Comments

Rob Herring July 23, 2021, 10:15 p.m. UTC | #1
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
> 
>
Maxime Ripard July 29, 2021, 12:29 p.m. UTC | #2
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
diff mbox series

Patch

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