* [PATCH] dt-bindings: usb: Convert Allwinner A80 USB PHY controller to a schema
@ 2019-12-19 8:43 Maxime Ripard
2019-12-19 15:24 ` Chen-Yu Tsai
0 siblings, 1 reply; 8+ messages in thread
From: Maxime Ripard @ 2019-12-19 8:43 UTC (permalink / raw)
To: kishon
Cc: Mark Rutland, Rob Herring, Frank Rowand, devicetree,
Chen-Yu Tsai, Maxime Ripard, linux-arm-kernel, Maxime Ripard
The Allwinner A80 SoCs have a USB PHY controller that is used by Linux,
with a matching Device Tree binding.
Now that we have the DT validation in place, let's convert the device tree
bindings for that controller over to a YAML schemas.
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
.../phy/allwinner,sun9i-a80-usb-phy.yaml | 135 ++++++++++++++++++
.../devicetree/bindings/phy/sun9i-usb-phy.txt | 37 -----
2 files changed, 135 insertions(+), 37 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
delete mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
new file mode 100644
index 000000000000..ded7d6f0a119
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
@@ -0,0 +1,135 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/allwinner,sun9i-a80-usb-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner A80 USB PHY Device Tree Bindings
+
+maintainers:
+ - Chen-Yu Tsai <wens@csie.org>
+ - Maxime Ripard <mripard@kernel.org>
+
+properties:
+ "#phy-cells":
+ const: 0
+
+ compatible:
+ const: allwinner,sun9i-a80-usb-phy
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ anyOf:
+ - description: Main PHY Clock
+
+ - items:
+ - description: Main PHY clock
+ - description: HSIC 12MHz clock
+ - description: HSIC 480MHz clock
+
+ clock-names:
+ oneOf:
+ - const: phy
+
+ - items:
+ - const: phy
+ - const: hsic_12M
+ - const: hsic_480M
+
+ resets:
+ anyOf:
+ - description: Normal USB PHY reset
+
+ - items:
+ - description: Normal USB PHY reset
+ - description: HSIC Reset
+
+ reset-names:
+ oneOf:
+ - const: phy
+
+ - items:
+ - const: phy
+ - const: hsic
+
+ phy_type:
+ const: hsic
+ description:
+ When absent, the PHY type will be assumed to be normal USB.
+
+ phy-supply:
+ description:
+ Regulator that powers VBUS
+
+required:
+ - "#phy-cells"
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+
+additionalProperties: false
+
+if:
+ properties:
+ phy_type:
+ const: hsic
+
+ required:
+ - phy_type
+
+then:
+ properties:
+ clocks:
+ maxItems: 3
+
+ clock-names:
+ maxItems: 3
+
+ resets:
+ maxItems: 2
+
+ reset-names:
+ maxItems: 2
+
+examples:
+ - |
+ #include <dt-bindings/clock/sun9i-a80-usb.h>
+ #include <dt-bindings/reset/sun9i-a80-usb.h>
+
+ usbphy1: phy@a00800 {
+ compatible = "allwinner,sun9i-a80-usb-phy";
+ reg = <0x00a00800 0x4>;
+ clocks = <&usb_clocks CLK_USB0_PHY>;
+ clock-names = "phy";
+ resets = <&usb_clocks RST_USB0_PHY>;
+ reset-names = "phy";
+ phy-supply = <®_usb1_vbus>;
+ #phy-cells = <0>;
+ };
+
+ - |
+ #include <dt-bindings/clock/sun9i-a80-usb.h>
+ #include <dt-bindings/reset/sun9i-a80-usb.h>
+
+ usbphy3: phy@a02800 {
+ compatible = "allwinner,sun9i-a80-usb-phy";
+ reg = <0x00a02800 0x4>;
+ clocks = <&usb_clocks CLK_USB2_PHY>,
+ <&usb_clocks CLK_USB_HSIC>,
+ <&usb_clocks CLK_USB2_HSIC>;
+ clock-names = "phy",
+ "hsic_12M",
+ "hsic_480M";
+ resets = <&usb_clocks RST_USB2_PHY>,
+ <&usb_clocks RST_USB2_HSIC>;
+ reset-names = "phy",
+ "hsic";
+ phy_type = "hsic";
+ phy-supply = <®_usb3_vbus>;
+ #phy-cells = <0>;
+ };
diff --git a/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
deleted file mode 100644
index 64f7109aea1f..000000000000
--- a/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
+++ /dev/null
@@ -1,37 +0,0 @@
-Allwinner sun9i USB PHY
------------------------
-
-Required properties:
-- compatible : should be one of
- * allwinner,sun9i-a80-usb-phy
-- reg : a list of offset + length pairs
-- #phy-cells : from the generic phy bindings, must be 0
-- phy_type : "hsic" for HSIC usage;
- other values or absence of this property indicates normal USB
-- clocks : phandle + clock specifier for the phy clocks
-- clock-names : depending on the "phy_type" property,
- * "phy" for normal USB
- * "hsic_480M", "hsic_12M" for HSIC
-- resets : a list of phandle + reset specifier pairs
-- reset-names : depending on the "phy_type" property,
- * "phy" for normal USB
- * "hsic" for HSIC
-
-Optional Properties:
-- phy-supply : from the generic phy bindings, a phandle to a regulator that
- provides power to VBUS.
-
-It is recommended to list all clocks and resets available.
-The driver will only use those matching the phy_type.
-
-Example:
- usbphy1: phy@a01800 {
- compatible = "allwinner,sun9i-a80-usb-phy";
- reg = <0x00a01800 0x4>;
- clocks = <&usb_phy_clk 2>, <&usb_phy_clk 10>,
- <&usb_phy_clk 3>;
- clock-names = "hsic_480M", "hsic_12M", "phy";
- resets = <&usb_phy_clk 18>, <&usb_phy_clk 19>;
- reset-names = "hsic", "phy";
- #phy-cells = <0>;
- };
--
2.23.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] dt-bindings: usb: Convert Allwinner A80 USB PHY controller to a schema
2019-12-19 8:43 [PATCH] dt-bindings: usb: Convert Allwinner A80 USB PHY controller to a schema Maxime Ripard
@ 2019-12-19 15:24 ` Chen-Yu Tsai
2019-12-20 8:03 ` Maxime Ripard
0 siblings, 1 reply; 8+ messages in thread
From: Chen-Yu Tsai @ 2019-12-19 15:24 UTC (permalink / raw)
To: Maxime Ripard
Cc: Kishon Vijay Abraham I, Mark Rutland, Rob Herring, Frank Rowand,
devicetree, Maxime Ripard, linux-arm-kernel
On Thu, Dec 19, 2019 at 4:43 PM Maxime Ripard <maxime@cerno.tech> wrote:
>
> The Allwinner A80 SoCs have a USB PHY controller that is used by Linux,
> with a matching Device Tree binding.
>
> Now that we have the DT validation in place, let's convert the device tree
> bindings for that controller over to a YAML schemas.
>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
> .../phy/allwinner,sun9i-a80-usb-phy.yaml | 135 ++++++++++++++++++
> .../devicetree/bindings/phy/sun9i-usb-phy.txt | 37 -----
> 2 files changed, 135 insertions(+), 37 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> delete mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> new file mode 100644
> index 000000000000..ded7d6f0a119
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> @@ -0,0 +1,135 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/allwinner,sun9i-a80-usb-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner A80 USB PHY Device Tree Bindings
> +
> +maintainers:
> + - Chen-Yu Tsai <wens@csie.org>
> + - Maxime Ripard <mripard@kernel.org>
> +
> +properties:
> + "#phy-cells":
> + const: 0
> +
> + compatible:
> + const: allwinner,sun9i-a80-usb-phy
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + anyOf:
> + - description: Main PHY Clock
> +
> + - items:
> + - description: Main PHY clock
> + - description: HSIC 12MHz clock
> + - description: HSIC 480MHz clock
> +
> + clock-names:
> + oneOf:
> + - const: phy
> +
> + - items:
> + - const: phy
> + - const: hsic_12M
> + - const: hsic_480M
> +
> + resets:
> + anyOf:
> + - description: Normal USB PHY reset
> +
> + - items:
> + - description: Normal USB PHY reset
> + - description: HSIC Reset
> +
> + reset-names:
> + oneOf:
> + - const: phy
> +
> + - items:
> + - const: phy
> + - const: hsic
> +
> + phy_type:
> + const: hsic
> + description:
> + When absent, the PHY type will be assumed to be normal USB.
> +
> + phy-supply:
> + description:
> + Regulator that powers VBUS
> +
> +required:
> + - "#phy-cells"
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - resets
> + - reset-names
> +
> +additionalProperties: false
> +
> +if:
> + properties:
> + phy_type:
> + const: hsic
> +
> + required:
> + - phy_type
> +
> +then:
> + properties:
> + clocks:
> + maxItems: 3
> +
> + clock-names:
> + maxItems: 3
> +
> + resets:
> + maxItems: 2
> +
> + reset-names:
> + maxItems: 2
So this is slightly incorrect. If phy_type == "hsic", then the
"phy" clock and reset should not be needed. I say should because
no boards actually came with HSIC implemented. The A80 Optimus
board had the HSIC lines on one of the GPIO headers, but I never
had any HSIC chips lol.
ChenYu
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/sun9i-a80-usb.h>
> + #include <dt-bindings/reset/sun9i-a80-usb.h>
> +
> + usbphy1: phy@a00800 {
> + compatible = "allwinner,sun9i-a80-usb-phy";
> + reg = <0x00a00800 0x4>;
> + clocks = <&usb_clocks CLK_USB0_PHY>;
> + clock-names = "phy";
> + resets = <&usb_clocks RST_USB0_PHY>;
> + reset-names = "phy";
> + phy-supply = <®_usb1_vbus>;
> + #phy-cells = <0>;
> + };
> +
> + - |
> + #include <dt-bindings/clock/sun9i-a80-usb.h>
> + #include <dt-bindings/reset/sun9i-a80-usb.h>
> +
> + usbphy3: phy@a02800 {
> + compatible = "allwinner,sun9i-a80-usb-phy";
> + reg = <0x00a02800 0x4>;
> + clocks = <&usb_clocks CLK_USB2_PHY>,
> + <&usb_clocks CLK_USB_HSIC>,
> + <&usb_clocks CLK_USB2_HSIC>;
> + clock-names = "phy",
> + "hsic_12M",
> + "hsic_480M";
> + resets = <&usb_clocks RST_USB2_PHY>,
> + <&usb_clocks RST_USB2_HSIC>;
> + reset-names = "phy",
> + "hsic";
> + phy_type = "hsic";
> + phy-supply = <®_usb3_vbus>;
> + #phy-cells = <0>;
> + };
> diff --git a/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> deleted file mode 100644
> index 64f7109aea1f..000000000000
> --- a/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> +++ /dev/null
> @@ -1,37 +0,0 @@
> -Allwinner sun9i USB PHY
> ------------------------
> -
> -Required properties:
> -- compatible : should be one of
> - * allwinner,sun9i-a80-usb-phy
> -- reg : a list of offset + length pairs
> -- #phy-cells : from the generic phy bindings, must be 0
> -- phy_type : "hsic" for HSIC usage;
> - other values or absence of this property indicates normal USB
> -- clocks : phandle + clock specifier for the phy clocks
> -- clock-names : depending on the "phy_type" property,
> - * "phy" for normal USB
> - * "hsic_480M", "hsic_12M" for HSIC
> -- resets : a list of phandle + reset specifier pairs
> -- reset-names : depending on the "phy_type" property,
> - * "phy" for normal USB
> - * "hsic" for HSIC
> -
> -Optional Properties:
> -- phy-supply : from the generic phy bindings, a phandle to a regulator that
> - provides power to VBUS.
> -
> -It is recommended to list all clocks and resets available.
> -The driver will only use those matching the phy_type.
> -
> -Example:
> - usbphy1: phy@a01800 {
> - compatible = "allwinner,sun9i-a80-usb-phy";
> - reg = <0x00a01800 0x4>;
> - clocks = <&usb_phy_clk 2>, <&usb_phy_clk 10>,
> - <&usb_phy_clk 3>;
> - clock-names = "hsic_480M", "hsic_12M", "phy";
> - resets = <&usb_phy_clk 18>, <&usb_phy_clk 19>;
> - reset-names = "hsic", "phy";
> - #phy-cells = <0>;
> - };
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] dt-bindings: usb: Convert Allwinner A80 USB PHY controller to a schema
2019-12-19 15:24 ` Chen-Yu Tsai
@ 2019-12-20 8:03 ` Maxime Ripard
2019-12-20 8:10 ` Chen-Yu Tsai
0 siblings, 1 reply; 8+ messages in thread
From: Maxime Ripard @ 2019-12-20 8:03 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Kishon Vijay Abraham I, Mark Rutland, Rob Herring, Frank Rowand,
devicetree, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 4330 bytes --]
Hi,
On Thu, Dec 19, 2019 at 11:24:52PM +0800, Chen-Yu Tsai wrote:
> On Thu, Dec 19, 2019 at 4:43 PM Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > The Allwinner A80 SoCs have a USB PHY controller that is used by Linux,
> > with a matching Device Tree binding.
> >
> > Now that we have the DT validation in place, let's convert the device tree
> > bindings for that controller over to a YAML schemas.
> >
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > ---
> > .../phy/allwinner,sun9i-a80-usb-phy.yaml | 135 ++++++++++++++++++
> > .../devicetree/bindings/phy/sun9i-usb-phy.txt | 37 -----
> > 2 files changed, 135 insertions(+), 37 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > delete mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> >
> > diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > new file mode 100644
> > index 000000000000..ded7d6f0a119
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > @@ -0,0 +1,135 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/allwinner,sun9i-a80-usb-phy.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Allwinner A80 USB PHY Device Tree Bindings
> > +
> > +maintainers:
> > + - Chen-Yu Tsai <wens@csie.org>
> > + - Maxime Ripard <mripard@kernel.org>
> > +
> > +properties:
> > + "#phy-cells":
> > + const: 0
> > +
> > + compatible:
> > + const: allwinner,sun9i-a80-usb-phy
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + anyOf:
> > + - description: Main PHY Clock
> > +
> > + - items:
> > + - description: Main PHY clock
> > + - description: HSIC 12MHz clock
> > + - description: HSIC 480MHz clock
> > +
> > + clock-names:
> > + oneOf:
> > + - const: phy
> > +
> > + - items:
> > + - const: phy
> > + - const: hsic_12M
> > + - const: hsic_480M
> > +
> > + resets:
> > + anyOf:
> > + - description: Normal USB PHY reset
> > +
> > + - items:
> > + - description: Normal USB PHY reset
> > + - description: HSIC Reset
> > +
> > + reset-names:
> > + oneOf:
> > + - const: phy
> > +
> > + - items:
> > + - const: phy
> > + - const: hsic
> > +
> > + phy_type:
> > + const: hsic
> > + description:
> > + When absent, the PHY type will be assumed to be normal USB.
> > +
> > + phy-supply:
> > + description:
> > + Regulator that powers VBUS
> > +
> > +required:
> > + - "#phy-cells"
> > + - compatible
> > + - reg
> > + - clocks
> > + - clock-names
> > + - resets
> > + - reset-names
> > +
> > +additionalProperties: false
> > +
> > +if:
> > + properties:
> > + phy_type:
> > + const: hsic
> > +
> > + required:
> > + - phy_type
> > +
> > +then:
> > + properties:
> > + clocks:
> > + maxItems: 3
> > +
> > + clock-names:
> > + maxItems: 3
> > +
> > + resets:
> > + maxItems: 2
> > +
> > + reset-names:
> > + maxItems: 2
>
> So this is slightly incorrect. If phy_type == "hsic", then the
> "phy" clock and reset should not be needed. I say should because
> no boards actually came with HSIC implemented. The A80 Optimus
> board had the HSIC lines on one of the GPIO headers, but I never
> had any HSIC chips lol.
This isn't what the previous binding was saying though :/
> > -- phy_type : "hsic" for HSIC usage;
> > - other values or absence of this property indicates normal USB
> > -- clocks : phandle + clock specifier for the phy clocks
> > -- clock-names : depending on the "phy_type" property,
> > - * "phy" for normal USB
> > - * "hsic_480M", "hsic_12M" for HSIC
> > -- resets : a list of phandle + reset specifier pairs
> > -- reset-names : depending on the "phy_type" property,
> > - * "phy" for normal USB
> > - * "hsic" for HSIC
It's speficied that the reset and clock is needed. If we want to
revise that, we can do it, but I guess it should be in a separate
patch than the one doing the conversion. Here we just want to express
the exact same thing.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: usb: Convert Allwinner A80 USB PHY controller to a schema
2019-12-20 8:03 ` Maxime Ripard
@ 2019-12-20 8:10 ` Chen-Yu Tsai
2020-01-02 12:02 ` Maxime Ripard
0 siblings, 1 reply; 8+ messages in thread
From: Chen-Yu Tsai @ 2019-12-20 8:10 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, Kishon Vijay Abraham I, Mark Rutland, Rob Herring,
Frank Rowand, devicetree, linux-arm-kernel
On Fri, Dec 20, 2019 at 4:03 PM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
> On Thu, Dec 19, 2019 at 11:24:52PM +0800, Chen-Yu Tsai wrote:
> > On Thu, Dec 19, 2019 at 4:43 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > >
> > > The Allwinner A80 SoCs have a USB PHY controller that is used by Linux,
> > > with a matching Device Tree binding.
> > >
> > > Now that we have the DT validation in place, let's convert the device tree
> > > bindings for that controller over to a YAML schemas.
> > >
> > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > ---
> > > .../phy/allwinner,sun9i-a80-usb-phy.yaml | 135 ++++++++++++++++++
> > > .../devicetree/bindings/phy/sun9i-usb-phy.txt | 37 -----
> > > 2 files changed, 135 insertions(+), 37 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > delete mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > new file mode 100644
> > > index 000000000000..ded7d6f0a119
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > @@ -0,0 +1,135 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/phy/allwinner,sun9i-a80-usb-phy.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Allwinner A80 USB PHY Device Tree Bindings
> > > +
> > > +maintainers:
> > > + - Chen-Yu Tsai <wens@csie.org>
> > > + - Maxime Ripard <mripard@kernel.org>
> > > +
> > > +properties:
> > > + "#phy-cells":
> > > + const: 0
> > > +
> > > + compatible:
> > > + const: allwinner,sun9i-a80-usb-phy
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + clocks:
> > > + anyOf:
> > > + - description: Main PHY Clock
> > > +
> > > + - items:
> > > + - description: Main PHY clock
> > > + - description: HSIC 12MHz clock
> > > + - description: HSIC 480MHz clock
> > > +
> > > + clock-names:
> > > + oneOf:
> > > + - const: phy
> > > +
> > > + - items:
> > > + - const: phy
> > > + - const: hsic_12M
> > > + - const: hsic_480M
> > > +
> > > + resets:
> > > + anyOf:
> > > + - description: Normal USB PHY reset
> > > +
> > > + - items:
> > > + - description: Normal USB PHY reset
> > > + - description: HSIC Reset
> > > +
> > > + reset-names:
> > > + oneOf:
> > > + - const: phy
> > > +
> > > + - items:
> > > + - const: phy
> > > + - const: hsic
> > > +
> > > + phy_type:
> > > + const: hsic
> > > + description:
> > > + When absent, the PHY type will be assumed to be normal USB.
> > > +
> > > + phy-supply:
> > > + description:
> > > + Regulator that powers VBUS
> > > +
> > > +required:
> > > + - "#phy-cells"
> > > + - compatible
> > > + - reg
> > > + - clocks
> > > + - clock-names
> > > + - resets
> > > + - reset-names
> > > +
> > > +additionalProperties: false
> > > +
> > > +if:
> > > + properties:
> > > + phy_type:
> > > + const: hsic
> > > +
> > > + required:
> > > + - phy_type
> > > +
> > > +then:
> > > + properties:
> > > + clocks:
> > > + maxItems: 3
> > > +
> > > + clock-names:
> > > + maxItems: 3
> > > +
> > > + resets:
> > > + maxItems: 2
> > > +
> > > + reset-names:
> > > + maxItems: 2
> >
> > So this is slightly incorrect. If phy_type == "hsic", then the
> > "phy" clock and reset should not be needed. I say should because
> > no boards actually came with HSIC implemented. The A80 Optimus
> > board had the HSIC lines on one of the GPIO headers, but I never
> > had any HSIC chips lol.
>
> This isn't what the previous binding was saying though :/
From the original binding:
- clock-names : depending on the "phy_type" property,
* "phy" for normal USB
* "hsic_480M", "hsic_12M" for HSIC
- resets : a list of phandle + reset specifier pairs
- reset-names : depending on the "phy_type" property,
* "phy" for normal USB
* "hsic" for HSIC
It is recommended to list all clocks and resets available.
The driver will only use those matching the phy_type.
> > > -- phy_type : "hsic" for HSIC usage;
> > > - other values or absence of this property indicates normal USB
> > > -- clocks : phandle + clock specifier for the phy clocks
> > > -- clock-names : depending on the "phy_type" property,
> > > - * "phy" for normal USB
> > > - * "hsic_480M", "hsic_12M" for HSIC
> > > -- resets : a list of phandle + reset specifier pairs
> > > -- reset-names : depending on the "phy_type" property,
> > > - * "phy" for normal USB
> > > - * "hsic" for HSIC
>
> It's speficied that the reset and clock is needed. If we want to
> revise that, we can do it, but I guess it should be in a separate
> patch than the one doing the conversion. Here we just want to express
> the exact same thing.
So the original binding only recommends having all clocks.
But given that these are internal to the SoC, having them
all is easier I suppose.
ChenYu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: usb: Convert Allwinner A80 USB PHY controller to a schema
2019-12-20 8:10 ` Chen-Yu Tsai
@ 2020-01-02 12:02 ` Maxime Ripard
2020-01-02 12:31 ` Chen-Yu Tsai
0 siblings, 1 reply; 8+ messages in thread
From: Maxime Ripard @ 2020-01-02 12:02 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Kishon Vijay Abraham I, Mark Rutland, Rob Herring, Frank Rowand,
devicetree, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 5305 bytes --]
Hi,
On Fri, Dec 20, 2019 at 04:10:03PM +0800, Chen-Yu Tsai wrote:
> On Fri, Dec 20, 2019 at 4:03 PM Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi,
> >
> > On Thu, Dec 19, 2019 at 11:24:52PM +0800, Chen-Yu Tsai wrote:
> > > On Thu, Dec 19, 2019 at 4:43 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > >
> > > > The Allwinner A80 SoCs have a USB PHY controller that is used by Linux,
> > > > with a matching Device Tree binding.
> > > >
> > > > Now that we have the DT validation in place, let's convert the device tree
> > > > bindings for that controller over to a YAML schemas.
> > > >
> > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > > ---
> > > > .../phy/allwinner,sun9i-a80-usb-phy.yaml | 135 ++++++++++++++++++
> > > > .../devicetree/bindings/phy/sun9i-usb-phy.txt | 37 -----
> > > > 2 files changed, 135 insertions(+), 37 deletions(-)
> > > > create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > delete mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > new file mode 100644
> > > > index 000000000000..ded7d6f0a119
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > @@ -0,0 +1,135 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/phy/allwinner,sun9i-a80-usb-phy.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Allwinner A80 USB PHY Device Tree Bindings
> > > > +
> > > > +maintainers:
> > > > + - Chen-Yu Tsai <wens@csie.org>
> > > > + - Maxime Ripard <mripard@kernel.org>
> > > > +
> > > > +properties:
> > > > + "#phy-cells":
> > > > + const: 0
> > > > +
> > > > + compatible:
> > > > + const: allwinner,sun9i-a80-usb-phy
> > > > +
> > > > + reg:
> > > > + maxItems: 1
> > > > +
> > > > + clocks:
> > > > + anyOf:
> > > > + - description: Main PHY Clock
> > > > +
> > > > + - items:
> > > > + - description: Main PHY clock
> > > > + - description: HSIC 12MHz clock
> > > > + - description: HSIC 480MHz clock
> > > > +
> > > > + clock-names:
> > > > + oneOf:
> > > > + - const: phy
> > > > +
> > > > + - items:
> > > > + - const: phy
> > > > + - const: hsic_12M
> > > > + - const: hsic_480M
> > > > +
> > > > + resets:
> > > > + anyOf:
> > > > + - description: Normal USB PHY reset
> > > > +
> > > > + - items:
> > > > + - description: Normal USB PHY reset
> > > > + - description: HSIC Reset
> > > > +
> > > > + reset-names:
> > > > + oneOf:
> > > > + - const: phy
> > > > +
> > > > + - items:
> > > > + - const: phy
> > > > + - const: hsic
> > > > +
> > > > + phy_type:
> > > > + const: hsic
> > > > + description:
> > > > + When absent, the PHY type will be assumed to be normal USB.
> > > > +
> > > > + phy-supply:
> > > > + description:
> > > > + Regulator that powers VBUS
> > > > +
> > > > +required:
> > > > + - "#phy-cells"
> > > > + - compatible
> > > > + - reg
> > > > + - clocks
> > > > + - clock-names
> > > > + - resets
> > > > + - reset-names
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +if:
> > > > + properties:
> > > > + phy_type:
> > > > + const: hsic
> > > > +
> > > > + required:
> > > > + - phy_type
> > > > +
> > > > +then:
> > > > + properties:
> > > > + clocks:
> > > > + maxItems: 3
> > > > +
> > > > + clock-names:
> > > > + maxItems: 3
> > > > +
> > > > + resets:
> > > > + maxItems: 2
> > > > +
> > > > + reset-names:
> > > > + maxItems: 2
> > >
> > > So this is slightly incorrect. If phy_type == "hsic", then the
> > > "phy" clock and reset should not be needed. I say should because
> > > no boards actually came with HSIC implemented. The A80 Optimus
> > > board had the HSIC lines on one of the GPIO headers, but I never
> > > had any HSIC chips lol.
> >
> > This isn't what the previous binding was saying though :/
>
> From the original binding:
>
> - clock-names : depending on the "phy_type" property,
> * "phy" for normal USB
> * "hsic_480M", "hsic_12M" for HSIC
> - resets : a list of phandle + reset specifier pairs
> - reset-names : depending on the "phy_type" property,
> * "phy" for normal USB
> * "hsic" for HSIC
>
> It is recommended to list all clocks and resets available.
> The driver will only use those matching the phy_type.
I'm not quite sure how we want to fix this then, or even what there's
to fix.
The previous binding is saying that we need either phy or hsic, and
that we should ideally set both. The DT are following that
recommendation, and we have either one item for the clocks (phy), or
three (phy + 2 HSIC clocks). resets is in a similar situation.
The binding allows to have either one or three, and enforce that in
HSIC we have three (but leaves the option open to have either 1 or 3
in the normal phy type).
As far as I can see, we cover what the binding was saying. Am I
missing something?
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: usb: Convert Allwinner A80 USB PHY controller to a schema
2020-01-02 12:02 ` Maxime Ripard
@ 2020-01-02 12:31 ` Chen-Yu Tsai
2020-01-02 15:26 ` Maxime Ripard
0 siblings, 1 reply; 8+ messages in thread
From: Chen-Yu Tsai @ 2020-01-02 12:31 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, Kishon Vijay Abraham I, Mark Rutland, Rob Herring,
Frank Rowand, devicetree, linux-arm-kernel
On Thu, Jan 2, 2020 at 8:02 PM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
> On Fri, Dec 20, 2019 at 04:10:03PM +0800, Chen-Yu Tsai wrote:
> > On Fri, Dec 20, 2019 at 4:03 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, Dec 19, 2019 at 11:24:52PM +0800, Chen-Yu Tsai wrote:
> > > > On Thu, Dec 19, 2019 at 4:43 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > >
> > > > > The Allwinner A80 SoCs have a USB PHY controller that is used by Linux,
> > > > > with a matching Device Tree binding.
> > > > >
> > > > > Now that we have the DT validation in place, let's convert the device tree
> > > > > bindings for that controller over to a YAML schemas.
> > > > >
> > > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > > > ---
> > > > > .../phy/allwinner,sun9i-a80-usb-phy.yaml | 135 ++++++++++++++++++
> > > > > .../devicetree/bindings/phy/sun9i-usb-phy.txt | 37 -----
> > > > > 2 files changed, 135 insertions(+), 37 deletions(-)
> > > > > create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > > delete mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..ded7d6f0a119
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > > @@ -0,0 +1,135 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/phy/allwinner,sun9i-a80-usb-phy.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Allwinner A80 USB PHY Device Tree Bindings
> > > > > +
> > > > > +maintainers:
> > > > > + - Chen-Yu Tsai <wens@csie.org>
> > > > > + - Maxime Ripard <mripard@kernel.org>
> > > > > +
> > > > > +properties:
> > > > > + "#phy-cells":
> > > > > + const: 0
> > > > > +
> > > > > + compatible:
> > > > > + const: allwinner,sun9i-a80-usb-phy
> > > > > +
> > > > > + reg:
> > > > > + maxItems: 1
> > > > > +
> > > > > + clocks:
> > > > > + anyOf:
> > > > > + - description: Main PHY Clock
> > > > > +
> > > > > + - items:
> > > > > + - description: Main PHY clock
> > > > > + - description: HSIC 12MHz clock
> > > > > + - description: HSIC 480MHz clock
> > > > > +
> > > > > + clock-names:
> > > > > + oneOf:
> > > > > + - const: phy
> > > > > +
> > > > > + - items:
> > > > > + - const: phy
> > > > > + - const: hsic_12M
> > > > > + - const: hsic_480M
> > > > > +
> > > > > + resets:
> > > > > + anyOf:
> > > > > + - description: Normal USB PHY reset
> > > > > +
> > > > > + - items:
> > > > > + - description: Normal USB PHY reset
> > > > > + - description: HSIC Reset
> > > > > +
> > > > > + reset-names:
> > > > > + oneOf:
> > > > > + - const: phy
> > > > > +
> > > > > + - items:
> > > > > + - const: phy
> > > > > + - const: hsic
> > > > > +
> > > > > + phy_type:
> > > > > + const: hsic
> > > > > + description:
> > > > > + When absent, the PHY type will be assumed to be normal USB.
> > > > > +
> > > > > + phy-supply:
> > > > > + description:
> > > > > + Regulator that powers VBUS
> > > > > +
> > > > > +required:
> > > > > + - "#phy-cells"
> > > > > + - compatible
> > > > > + - reg
> > > > > + - clocks
> > > > > + - clock-names
> > > > > + - resets
> > > > > + - reset-names
> > > > > +
> > > > > +additionalProperties: false
> > > > > +
> > > > > +if:
> > > > > + properties:
> > > > > + phy_type:
> > > > > + const: hsic
> > > > > +
> > > > > + required:
> > > > > + - phy_type
> > > > > +
> > > > > +then:
> > > > > + properties:
> > > > > + clocks:
> > > > > + maxItems: 3
> > > > > +
> > > > > + clock-names:
> > > > > + maxItems: 3
> > > > > +
> > > > > + resets:
> > > > > + maxItems: 2
> > > > > +
> > > > > + reset-names:
> > > > > + maxItems: 2
> > > >
> > > > So this is slightly incorrect. If phy_type == "hsic", then the
> > > > "phy" clock and reset should not be needed. I say should because
> > > > no boards actually came with HSIC implemented. The A80 Optimus
> > > > board had the HSIC lines on one of the GPIO headers, but I never
> > > > had any HSIC chips lol.
> > >
> > > This isn't what the previous binding was saying though :/
> >
> > From the original binding:
> >
> > - clock-names : depending on the "phy_type" property,
> > * "phy" for normal USB
> > * "hsic_480M", "hsic_12M" for HSIC
> > - resets : a list of phandle + reset specifier pairs
> > - reset-names : depending on the "phy_type" property,
> > * "phy" for normal USB
> > * "hsic" for HSIC
> >
> > It is recommended to list all clocks and resets available.
> > The driver will only use those matching the phy_type.
>
> I'm not quite sure how we want to fix this then, or even what there's
> to fix.
>
> The previous binding is saying that we need either phy or hsic, and
> that we should ideally set both. The DT are following that
> recommendation, and we have either one item for the clocks (phy), or
> three (phy + 2 HSIC clocks). resets is in a similar situation.
>
> The binding allows to have either one or three, and enforce that in
> HSIC we have three (but leaves the option open to have either 1 or 3
> in the normal phy type).
>
> As far as I can see, we cover what the binding was saying. Am I
> missing something?
I guess you're right. Lets just keep why you've done already.
Sorry for the noise.
ChenYu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: usb: Convert Allwinner A80 USB PHY controller to a schema
2020-01-02 12:31 ` Chen-Yu Tsai
@ 2020-01-02 15:26 ` Maxime Ripard
2020-01-03 10:51 ` Chen-Yu Tsai
0 siblings, 1 reply; 8+ messages in thread
From: Maxime Ripard @ 2020-01-02 15:26 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Kishon Vijay Abraham I, Mark Rutland, Rob Herring, Frank Rowand,
devicetree, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 6209 bytes --]
On Thu, Jan 02, 2020 at 08:31:40PM +0800, Chen-Yu Tsai wrote:
> On Thu, Jan 2, 2020 at 8:02 PM Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi,
> >
> > On Fri, Dec 20, 2019 at 04:10:03PM +0800, Chen-Yu Tsai wrote:
> > > On Fri, Dec 20, 2019 at 4:03 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Thu, Dec 19, 2019 at 11:24:52PM +0800, Chen-Yu Tsai wrote:
> > > > > On Thu, Dec 19, 2019 at 4:43 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > >
> > > > > > The Allwinner A80 SoCs have a USB PHY controller that is used by Linux,
> > > > > > with a matching Device Tree binding.
> > > > > >
> > > > > > Now that we have the DT validation in place, let's convert the device tree
> > > > > > bindings for that controller over to a YAML schemas.
> > > > > >
> > > > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > > > > ---
> > > > > > .../phy/allwinner,sun9i-a80-usb-phy.yaml | 135 ++++++++++++++++++
> > > > > > .../devicetree/bindings/phy/sun9i-usb-phy.txt | 37 -----
> > > > > > 2 files changed, 135 insertions(+), 37 deletions(-)
> > > > > > create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > > > delete mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..ded7d6f0a119
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > > > @@ -0,0 +1,135 @@
> > > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: http://devicetree.org/schemas/phy/allwinner,sun9i-a80-usb-phy.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: Allwinner A80 USB PHY Device Tree Bindings
> > > > > > +
> > > > > > +maintainers:
> > > > > > + - Chen-Yu Tsai <wens@csie.org>
> > > > > > + - Maxime Ripard <mripard@kernel.org>
> > > > > > +
> > > > > > +properties:
> > > > > > + "#phy-cells":
> > > > > > + const: 0
> > > > > > +
> > > > > > + compatible:
> > > > > > + const: allwinner,sun9i-a80-usb-phy
> > > > > > +
> > > > > > + reg:
> > > > > > + maxItems: 1
> > > > > > +
> > > > > > + clocks:
> > > > > > + anyOf:
> > > > > > + - description: Main PHY Clock
> > > > > > +
> > > > > > + - items:
> > > > > > + - description: Main PHY clock
> > > > > > + - description: HSIC 12MHz clock
> > > > > > + - description: HSIC 480MHz clock
> > > > > > +
> > > > > > + clock-names:
> > > > > > + oneOf:
> > > > > > + - const: phy
> > > > > > +
> > > > > > + - items:
> > > > > > + - const: phy
> > > > > > + - const: hsic_12M
> > > > > > + - const: hsic_480M
> > > > > > +
> > > > > > + resets:
> > > > > > + anyOf:
> > > > > > + - description: Normal USB PHY reset
> > > > > > +
> > > > > > + - items:
> > > > > > + - description: Normal USB PHY reset
> > > > > > + - description: HSIC Reset
> > > > > > +
> > > > > > + reset-names:
> > > > > > + oneOf:
> > > > > > + - const: phy
> > > > > > +
> > > > > > + - items:
> > > > > > + - const: phy
> > > > > > + - const: hsic
> > > > > > +
> > > > > > + phy_type:
> > > > > > + const: hsic
> > > > > > + description:
> > > > > > + When absent, the PHY type will be assumed to be normal USB.
> > > > > > +
> > > > > > + phy-supply:
> > > > > > + description:
> > > > > > + Regulator that powers VBUS
> > > > > > +
> > > > > > +required:
> > > > > > + - "#phy-cells"
> > > > > > + - compatible
> > > > > > + - reg
> > > > > > + - clocks
> > > > > > + - clock-names
> > > > > > + - resets
> > > > > > + - reset-names
> > > > > > +
> > > > > > +additionalProperties: false
> > > > > > +
> > > > > > +if:
> > > > > > + properties:
> > > > > > + phy_type:
> > > > > > + const: hsic
> > > > > > +
> > > > > > + required:
> > > > > > + - phy_type
> > > > > > +
> > > > > > +then:
> > > > > > + properties:
> > > > > > + clocks:
> > > > > > + maxItems: 3
> > > > > > +
> > > > > > + clock-names:
> > > > > > + maxItems: 3
> > > > > > +
> > > > > > + resets:
> > > > > > + maxItems: 2
> > > > > > +
> > > > > > + reset-names:
> > > > > > + maxItems: 2
> > > > >
> > > > > So this is slightly incorrect. If phy_type == "hsic", then the
> > > > > "phy" clock and reset should not be needed. I say should because
> > > > > no boards actually came with HSIC implemented. The A80 Optimus
> > > > > board had the HSIC lines on one of the GPIO headers, but I never
> > > > > had any HSIC chips lol.
> > > >
> > > > This isn't what the previous binding was saying though :/
> > >
> > > From the original binding:
> > >
> > > - clock-names : depending on the "phy_type" property,
> > > * "phy" for normal USB
> > > * "hsic_480M", "hsic_12M" for HSIC
> > > - resets : a list of phandle + reset specifier pairs
> > > - reset-names : depending on the "phy_type" property,
> > > * "phy" for normal USB
> > > * "hsic" for HSIC
> > >
> > > It is recommended to list all clocks and resets available.
> > > The driver will only use those matching the phy_type.
> >
> > I'm not quite sure how we want to fix this then, or even what there's
> > to fix.
> >
> > The previous binding is saying that we need either phy or hsic, and
> > that we should ideally set both. The DT are following that
> > recommendation, and we have either one item for the clocks (phy), or
> > three (phy + 2 HSIC clocks). resets is in a similar situation.
> >
> > The binding allows to have either one or three, and enforce that in
> > HSIC we have three (but leaves the option open to have either 1 or 3
> > in the normal phy type).
> >
> > As far as I can see, we cover what the binding was saying. Am I
> > missing something?
>
> I guess you're right. Lets just keep why you've done already.
> Sorry for the noise.
Is that a Reviewed-by ? :)
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: usb: Convert Allwinner A80 USB PHY controller to a schema
2020-01-02 15:26 ` Maxime Ripard
@ 2020-01-03 10:51 ` Chen-Yu Tsai
0 siblings, 0 replies; 8+ messages in thread
From: Chen-Yu Tsai @ 2020-01-03 10:51 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, Kishon Vijay Abraham I, Mark Rutland, Rob Herring,
Frank Rowand, devicetree, linux-arm-kernel
On Thu, Jan 2, 2020 at 11:26 PM Maxime Ripard <maxime@cerno.tech> wrote:
>
> On Thu, Jan 02, 2020 at 08:31:40PM +0800, Chen-Yu Tsai wrote:
> > On Thu, Jan 2, 2020 at 8:02 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > >
> > > Hi,
> > >
> > > On Fri, Dec 20, 2019 at 04:10:03PM +0800, Chen-Yu Tsai wrote:
> > > > On Fri, Dec 20, 2019 at 4:03 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Thu, Dec 19, 2019 at 11:24:52PM +0800, Chen-Yu Tsai wrote:
> > > > > > On Thu, Dec 19, 2019 at 4:43 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > > >
> > > > > > > The Allwinner A80 SoCs have a USB PHY controller that is used by Linux,
> > > > > > > with a matching Device Tree binding.
> > > > > > >
> > > > > > > Now that we have the DT validation in place, let's convert the device tree
> > > > > > > bindings for that controller over to a YAML schemas.
> > > > > > >
> > > > > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > > > > > ---
> > > > > > > .../phy/allwinner,sun9i-a80-usb-phy.yaml | 135 ++++++++++++++++++
> > > > > > > .../devicetree/bindings/phy/sun9i-usb-phy.txt | 37 -----
> > > > > > > 2 files changed, 135 insertions(+), 37 deletions(-)
> > > > > > > create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > > > > delete mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..ded7d6f0a119
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/devicetree/bindings/phy/allwinner,sun9i-a80-usb-phy.yaml
> > > > > > > @@ -0,0 +1,135 @@
> > > > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > > > +%YAML 1.2
> > > > > > > +---
> > > > > > > +$id: http://devicetree.org/schemas/phy/allwinner,sun9i-a80-usb-phy.yaml#
> > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > > +
> > > > > > > +title: Allwinner A80 USB PHY Device Tree Bindings
> > > > > > > +
> > > > > > > +maintainers:
> > > > > > > + - Chen-Yu Tsai <wens@csie.org>
> > > > > > > + - Maxime Ripard <mripard@kernel.org>
> > > > > > > +
> > > > > > > +properties:
> > > > > > > + "#phy-cells":
> > > > > > > + const: 0
> > > > > > > +
> > > > > > > + compatible:
> > > > > > > + const: allwinner,sun9i-a80-usb-phy
> > > > > > > +
> > > > > > > + reg:
> > > > > > > + maxItems: 1
> > > > > > > +
> > > > > > > + clocks:
> > > > > > > + anyOf:
> > > > > > > + - description: Main PHY Clock
> > > > > > > +
> > > > > > > + - items:
> > > > > > > + - description: Main PHY clock
> > > > > > > + - description: HSIC 12MHz clock
> > > > > > > + - description: HSIC 480MHz clock
> > > > > > > +
> > > > > > > + clock-names:
> > > > > > > + oneOf:
> > > > > > > + - const: phy
> > > > > > > +
> > > > > > > + - items:
> > > > > > > + - const: phy
> > > > > > > + - const: hsic_12M
> > > > > > > + - const: hsic_480M
> > > > > > > +
> > > > > > > + resets:
> > > > > > > + anyOf:
> > > > > > > + - description: Normal USB PHY reset
> > > > > > > +
> > > > > > > + - items:
> > > > > > > + - description: Normal USB PHY reset
> > > > > > > + - description: HSIC Reset
> > > > > > > +
> > > > > > > + reset-names:
> > > > > > > + oneOf:
> > > > > > > + - const: phy
> > > > > > > +
> > > > > > > + - items:
> > > > > > > + - const: phy
> > > > > > > + - const: hsic
> > > > > > > +
> > > > > > > + phy_type:
> > > > > > > + const: hsic
> > > > > > > + description:
> > > > > > > + When absent, the PHY type will be assumed to be normal USB.
> > > > > > > +
> > > > > > > + phy-supply:
> > > > > > > + description:
> > > > > > > + Regulator that powers VBUS
> > > > > > > +
> > > > > > > +required:
> > > > > > > + - "#phy-cells"
> > > > > > > + - compatible
> > > > > > > + - reg
> > > > > > > + - clocks
> > > > > > > + - clock-names
> > > > > > > + - resets
> > > > > > > + - reset-names
> > > > > > > +
> > > > > > > +additionalProperties: false
> > > > > > > +
> > > > > > > +if:
> > > > > > > + properties:
> > > > > > > + phy_type:
> > > > > > > + const: hsic
> > > > > > > +
> > > > > > > + required:
> > > > > > > + - phy_type
> > > > > > > +
> > > > > > > +then:
> > > > > > > + properties:
> > > > > > > + clocks:
> > > > > > > + maxItems: 3
> > > > > > > +
> > > > > > > + clock-names:
> > > > > > > + maxItems: 3
> > > > > > > +
> > > > > > > + resets:
> > > > > > > + maxItems: 2
> > > > > > > +
> > > > > > > + reset-names:
> > > > > > > + maxItems: 2
> > > > > >
> > > > > > So this is slightly incorrect. If phy_type == "hsic", then the
> > > > > > "phy" clock and reset should not be needed. I say should because
> > > > > > no boards actually came with HSIC implemented. The A80 Optimus
> > > > > > board had the HSIC lines on one of the GPIO headers, but I never
> > > > > > had any HSIC chips lol.
> > > > >
> > > > > This isn't what the previous binding was saying though :/
> > > >
> > > > From the original binding:
> > > >
> > > > - clock-names : depending on the "phy_type" property,
> > > > * "phy" for normal USB
> > > > * "hsic_480M", "hsic_12M" for HSIC
> > > > - resets : a list of phandle + reset specifier pairs
> > > > - reset-names : depending on the "phy_type" property,
> > > > * "phy" for normal USB
> > > > * "hsic" for HSIC
> > > >
> > > > It is recommended to list all clocks and resets available.
> > > > The driver will only use those matching the phy_type.
> > >
> > > I'm not quite sure how we want to fix this then, or even what there's
> > > to fix.
> > >
> > > The previous binding is saying that we need either phy or hsic, and
> > > that we should ideally set both. The DT are following that
> > > recommendation, and we have either one item for the clocks (phy), or
> > > three (phy + 2 HSIC clocks). resets is in a similar situation.
> > >
> > > The binding allows to have either one or three, and enforce that in
> > > HSIC we have three (but leaves the option open to have either 1 or 3
> > > in the normal phy type).
> > >
> > > As far as I can see, we cover what the binding was saying. Am I
> > > missing something?
> >
> > I guess you're right. Lets just keep why you've done already.
> > Sorry for the noise.
>
> Is that a Reviewed-by ? :)
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
:)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-01-03 10:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-19 8:43 [PATCH] dt-bindings: usb: Convert Allwinner A80 USB PHY controller to a schema Maxime Ripard
2019-12-19 15:24 ` Chen-Yu Tsai
2019-12-20 8:03 ` Maxime Ripard
2019-12-20 8:10 ` Chen-Yu Tsai
2020-01-02 12:02 ` Maxime Ripard
2020-01-02 12:31 ` Chen-Yu Tsai
2020-01-02 15:26 ` Maxime Ripard
2020-01-03 10:51 ` Chen-Yu Tsai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).