| Message ID | 20250110123923.270626-11-szemzo.andras@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support for Allwinner V853 SoC | expand |
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.
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
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?
> 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
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
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>; + };
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