[PATCH 10/12] dt-bindings: phy: allwinner: add v853 usb phy

Andras Szemzo posted 12 patches 8 months, 3 weeks ago
There is a newer version of this series
[PATCH 10/12] dt-bindings: phy: allwinner: add v853 usb phy
Posted by Andras Szemzo 8 months, 3 weeks ago
Document Allwinner v853 USB phy.

Signed-off-by: Andras Szemzo <szemzo.andras@gmail.com>
---
 .../phy/allwinner,sun8i-v853-usb-phy.yaml     | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
new file mode 100644
index 000000000000..773c3f476db8
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/allwinner,sun8i-v853-usb-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner V853 USB PHY
+
+maintainers:
+  - Chen-Yu Tsai <wens@csie.org>
+  - Maxime Ripard <mripard@kernel.org>
+
+properties:
+  "#phy-cells":
+    const: 1
+
+  compatible:
+    const:
+	- allwinner,sun8i-v853-usb-phy
+
+  reg:
+    items:
+      - description: PHY Control registers
+      - description: PHY PMU0 registers
+
+  reg-names:
+    items:
+      - const: phy_ctrl
+      - const: pmu0
+
+  clocks:
+    maxItems: 1
+    description: USB OHCI PHY bus clock
+
+  clock-names:
+    const: usb0_phy
+
+  resets:
+    maxItems: 1
+    description: USB OHCI reset
+
+  reset-names:
+    const: usb0_reset
+
+  usb0_id_det-gpios:
+    maxItems: 1
+    description: GPIO to the USB OTG ID pin
+
+  usb0_vbus_det-gpios:
+    maxItems: 1
+    description: GPIO to the USB OTG VBUS detect pin
+
+  usb0_vbus_power-supply:
+    description: Power supply to detect the USB OTG VBUS
+
+  usb0_vbus-supply:
+    description: Regulator controlling USB OTG VBUS
+
+required:
+  - "#phy-cells"
+  - compatible
+  - clocks
+  - clock-names
+  - reg
+  - reg-names
+  - resets
+  - reset-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/clock/sun8i-v853-ccu.h>
+    #include <dt-bindings/reset/sun8i-v853-ccu.h>
+
+    usbphy: phy@4100400 {
+        #phy-cells = <1>;
+        compatible = "allwinner,sun8i-v853-usb-phy";
+        reg = <0x4100400 0x100>,
+              <0x4101800 0x100>;
+        reg-names = "phy_ctrl",
+                    "pmu0";
+        clocks = <&ccu CLK_USB_OHCI>;
+        clock-names = "usb0_phy";
+        resets = <&ccu RST_USB_PHY>;
+        reset-names = "usb0_reset";
+        usb0_id_det-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>;
+    };
-- 
2.39.5
Re: [PATCH 10/12] dt-bindings: phy: allwinner: add v853 usb phy
Posted by Vinod Koul 8 months, 2 weeks ago
On 10-01-25, 13:39, Andras Szemzo wrote:
> Document Allwinner v853 USB phy.
> 
> Signed-off-by: Andras Szemzo <szemzo.andras@gmail.com>
> ---
>  .../phy/allwinner,sun8i-v853-usb-phy.yaml     | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
> new file mode 100644
> index 000000000000..773c3f476db8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/allwinner,sun8i-v853-usb-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner V853 USB PHY
> +
> +maintainers:
> +  - Chen-Yu Tsai <wens@csie.org>
> +  - Maxime Ripard <mripard@kernel.org>
> +
> +properties:
> +  "#phy-cells":
> +    const: 1
> +
> +  compatible:
> +    const:
> +	- allwinner,sun8i-v853-usb-phy

Does this really need a new binding document, if so why... Cant this be
added to one of the existing docs which driver uses?

-- 
~Vinod
Re: [PATCH 10/12] dt-bindings: phy: allwinner: add v853 usb phy
Posted by Andre Przywara 8 months, 2 weeks ago
On Wed, 15 Jan 2025 07:51:50 +0000
Vinod Koul <vkoul@kernel.org> wrote:

> On 10-01-25, 13:39, Andras Szemzo wrote:
> > Document Allwinner v853 USB phy.
> > 
> > Signed-off-by: Andras Szemzo <szemzo.andras@gmail.com>
> > ---
> >  .../phy/allwinner,sun8i-v853-usb-phy.yaml     | 89 +++++++++++++++++++
> >  1 file changed, 89 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
> > new file mode 100644
> > index 000000000000..773c3f476db8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
> > @@ -0,0 +1,89 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/allwinner,sun8i-v853-usb-phy.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Allwinner V853 USB PHY
> > +
> > +maintainers:
> > +  - Chen-Yu Tsai <wens@csie.org>
> > +  - Maxime Ripard <mripard@kernel.org>
> > +
> > +properties:
> > +  "#phy-cells":
> > +    const: 1
> > +
> > +  compatible:
> > +    const:
> > +	- allwinner,sun8i-v853-usb-phy  
> 
> Does this really need a new binding document, if so why... Cant this be
> added to one of the existing docs which driver uses?

The USB-PHY bindings don't differ too much on a first glance, but still
enough in nasty details (number of PHYs supported, number of clocks
required, etc.) to make a joint binding basically unreadable (we tried
that). That's why we opted to have separate bindings.
Now I believe it's worth to look for the closest existing binding, and
just put the compatible in there, in the hope we don't need much else, and
that it still stays readable.

Cheers,
Andre
Re: [PATCH 10/12] dt-bindings: phy: allwinner: add v853 usb phy
Posted by András Szemző 8 months, 2 weeks ago

> On 15 Jan 2025, at 08:51, Vinod Koul <vkoul@kernel.org> wrote:
> 
> On 10-01-25, 13:39, Andras Szemzo wrote:
>> Document Allwinner v853 USB phy.
>> 
>> Signed-off-by: Andras Szemzo <szemzo.andras@gmail.com>
>> ---
>> .../phy/allwinner,sun8i-v853-usb-phy.yaml     | 89 +++++++++++++++++++
>> 1 file changed, 89 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
>> new file mode 100644
>> index 000000000000..773c3f476db8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
>> @@ -0,0 +1,89 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/phy/allwinner,sun8i-v853-usb-phy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Allwinner V853 USB PHY
>> +
>> +maintainers:
>> +  - Chen-Yu Tsai <wens@csie.org>
>> +  - Maxime Ripard <mripard@kernel.org>
>> +
>> +properties:
>> +  "#phy-cells":
>> +    const: 1
>> +
>> +  compatible:
>> +    const:
>> +	- allwinner,sun8i-v853-usb-phy
> 
> Does this really need a new binding document, if so why... Cant this be
> added to one of the existing docs which driver uses?
> 

Yes, that was my first intention too, but didn’t know how to do it propery, but I’ll remove it
and add to one of the existing doc.

> -- 
> ~Vinod
Re: [PATCH 10/12] dt-bindings: phy: allwinner: add v853 usb phy
Posted by Krzysztof Kozlowski 8 months, 3 weeks ago
On 10/01/2025 13:39, Andras Szemzo wrote:
> +
> +properties:
> +  "#phy-cells":
> +    const: 1
> +
> +  compatible:
> +    const:
> +	- allwinner,sun8i-v853-usb-phy

Wrong indentation, never tested.

Compatible is always the first property.

> +
> +  reg:
> +    items:
> +      - description: PHY Control registers
> +      - description: PHY PMU0 registers
> +
> +  reg-names:
> +    items:
> +      - const: phy_ctrl
> +      - const: pmu0
> +
> +  clocks:
> +    maxItems: 1
> +    description: USB OHCI PHY bus clock

Redundant description really...

> +
> +  clock-names:
> +    const: usb0_phy

Drop clock-names, not really helpful.


> +
> +  resets:
> +    maxItems: 1
> +    description: USB OHCI reset
> +
> +  reset-names:
> +    const: usb0_reset

Drop names, also not really helping.

> +
> +  usb0_id_det-gpios:

There are no properties with underscores.

I don't get why you are using "usb0" prefix. Document all pins, so
"usb1" as well, assuming it exists.

It anyway looks questionable - aren't these properties of connector?


> +    maxItems: 1
> +    description: GPIO to the USB OTG ID pin
> +
> +  usb0_vbus_det-gpios:
> +    maxItems: 1
> +    description: GPIO to the USB OTG VBUS detect pin
> +
> +  usb0_vbus_power-supply:
> +    description: Power supply to detect the USB OTG VBUS
> +
> +  usb0_vbus-supply:
> +    description: Regulator controlling USB OTG VBUS

Regulator and power supply are here synonyms, so I don't understand
these two properties.

> +
> +required:
> +  - "#phy-cells"
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - reg
> +  - reg-names
> +  - resets
> +  - reset-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/clock/sun8i-v853-ccu.h>
> +    #include <dt-bindings/reset/sun8i-v853-ccu.h>
> +
> +    usbphy: phy@4100400 {
> +        #phy-cells = <1>;
Please follow DTS coding style.

Best regards,
Krzysztof
Re: [PATCH 10/12] dt-bindings: phy: allwinner: add v853 usb phy
Posted by Rob Herring (Arm) 8 months, 3 weeks ago
On Fri, 10 Jan 2025 13:39:21 +0100, Andras Szemzo wrote:
> Document Allwinner v853 USB phy.
> 
> Signed-off-by: Andras Szemzo <szemzo.andras@gmail.com>
> ---
>  .../phy/allwinner,sun8i-v853-usb-phy.yaml     | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml:19:1: [error] syntax error: found character '\t' that cannot start any token (syntax)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml: ignoring, error parsing file
make[2]: *** Deleting file 'Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.example.dts'
Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml:19:1: found character '\t' that cannot start any token
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/phy/allwinner,sun8i-v853-usb-phy.yaml:19:1: found character '\t' that cannot start any token
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1506: dt_binding_check] Error 2
make: *** [Makefile:251: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250110123923.270626-11-szemzo.andras@gmail.com

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.