diff mbox series

[RESEND,12/19] dt-bindings: Add bindings for USB phy on Allwinner A100

Message ID 1ce71bac2732620f8fe77b23ca84e062385e7e8a.1604988979.git.frank@allwinnertech.com (mailing list archive)
State New, archived
Headers show
Series Second step support for A100 | expand

Commit Message

Frank Lee Nov. 10, 2020, 6:39 a.m. UTC 
From: Yangtao Li <frank@allwinnertech.com>

Add a device tree binding for the A100's USB PHY.

Signed-off-by: Yangtao Li <frank@allwinnertech.com>
---
 .../phy/allwinner,sun50i-a100-usb-phy.yaml    | 105 ++++++++++++++++++
 1 file changed, 105 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml

Comments

Rob Herring (Arm) Nov. 11, 2020, 10:50 p.m. UTC | #1 
On Tue, Nov 10, 2020 at 02:39:42PM +0800, Frank Lee wrote:
> From: Yangtao Li <frank@allwinnertech.com>
> 
> Add a device tree binding for the A100's USB PHY.
> 
> Signed-off-by: Yangtao Li <frank@allwinnertech.com>
> ---
>  .../phy/allwinner,sun50i-a100-usb-phy.yaml    | 105 ++++++++++++++++++
>  1 file changed, 105 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml
> new file mode 100644
> index 000000000000..cc9bbebe2bd7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings. checkpatch.pl will tell you which ones.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/allwinner,sun50i-a100-usb-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner A100 USB PHY Device Tree Bindings
> +
> +maintainers:
> +  - Yangtao Li <tiny.windzz@gmail.com>
> +
> +properties:
> +  "#phy-cells":
> +    const: 1
> +
> +  compatible:
> +    const: allwinner,sun50i-a100-usb-phy
> +
> +  reg:
> +    items:
> +      - description: PHY Control registers
> +      - description: PHY PMU0 registers
> +      - description: PHY PMU1 registers
> +
> +  reg-names:
> +    items:
> +      - const: phy_ctrl
> +      - const: pmu0
> +      - const: pmu1
> +
> +  clocks:
> +    items:
> +      - description: USB OTG PHY bus clock
> +      - description: USB Host 0 PHY bus clock
> +
> +  clock-names:
> +    items:
> +      - const: usb0_phy
> +      - const: usb1_phy
> +
> +  resets:
> +    items:
> +      - description: USB OTG reset
> +      - description: USB Host 1 Controller reset
> +
> +  reset-names:
> +    items:
> +      - const: usb0_reset
> +      - const: usb1_reset
> +
> +  usb0_id_det-gpios:
> +    description: GPIO to the USB OTG ID pin

Needs 'maxItems: 1'

> +
> +  usb0_vbus_det-gpios:
> +    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
> +
> +  usb1_vbus-supply:
> +    description: Regulator controlling USB1 Host controller

Are ID and VBus actually connected to the phy h/w? Really, all this 
should be in a USB connector node for which we have bindings.

> +
> +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/sun50i-a100-ccu.h>
> +    #include <dt-bindings/reset/sun50i-a100-ccu.h>
> +
> +    phy@5100400 {
> +        #phy-cells = <1>;
> +        compatible = "allwinner,sun50i-a100-usb-phy";
> +        reg = <0x05100400 0x14>,
> +              <0x05101800 0x4>,
> +              <0x05200800 0x4>;
> +        reg-names = "phy_ctrl",
> +                    "pmu0",
> +                    "pmu1";
> +        clocks = <&ccu CLK_USB_PHY0>,
> +                 <&ccu CLK_USB_PHY1>;
> +        clock-names = "usb0_phy",
> +                      "usb1_phy";
> +        resets = <&ccu RST_USB_PHY0>,
> +                 <&ccu RST_USB_PHY1>;
> +        reset-names = "usb0_reset",
> +                      "usb1_reset";
> +        usb0_id_det-gpios = <&pio 7 10 GPIO_ACTIVE_HIGH>; /* PH10 */
> +        usb0_vbus_power-supply = <&usb_power_supply>;
> +        usb0_vbus-supply = <&reg_drivevbus>;
> +        usb1_vbus-supply = <&reg_usb1_vbus>;
> +    };
> -- 
> 2.28.0
>
Andre Przywara Nov. 28, 2020, 8:18 p.m. UTC | #2 
On 11/11/2020 22:50, Rob Herring wrote:

Hi,

> On Tue, Nov 10, 2020 at 02:39:42PM +0800, Frank Lee wrote:
>> From: Yangtao Li <frank@allwinnertech.com>
>>
>> Add a device tree binding for the A100's USB PHY.

Not your fault, Yangto, but why do we actually have a separate binding
document per SoC, when the differences between the PHYs are so minimal
that we get away with some flags in the compatible match, in one driver
file?

For a start this file is basically identical to the A64 one (apart from
the example), so can you just add the A100 compatible string to that
one, instead?

Cheers,
Andre

>>
>> Signed-off-by: Yangtao Li <frank@allwinnertech.com>
>> ---
>>  .../phy/allwinner,sun50i-a100-usb-phy.yaml    | 105 ++++++++++++++++++
>>  1 file changed, 105 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml
>> new file mode 100644
>> index 000000000000..cc9bbebe2bd7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml
>> @@ -0,0 +1,105 @@
>> +# SPDX-License-Identifier: GPL-2.0
> 
> Dual license new bindings. checkpatch.pl will tell you which ones.
> 
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/phy/allwinner,sun50i-a100-usb-phy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Allwinner A100 USB PHY Device Tree Bindings
>> +
>> +maintainers:
>> +  - Yangtao Li <tiny.windzz@gmail.com>
>> +
>> +properties:
>> +  "#phy-cells":
>> +    const: 1
>> +
>> +  compatible:
>> +    const: allwinner,sun50i-a100-usb-phy
>> +
>> +  reg:
>> +    items:
>> +      - description: PHY Control registers
>> +      - description: PHY PMU0 registers
>> +      - description: PHY PMU1 registers
>> +
>> +  reg-names:
>> +    items:
>> +      - const: phy_ctrl
>> +      - const: pmu0
>> +      - const: pmu1
>> +
>> +  clocks:
>> +    items:
>> +      - description: USB OTG PHY bus clock
>> +      - description: USB Host 0 PHY bus clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: usb0_phy
>> +      - const: usb1_phy
>> +
>> +  resets:
>> +    items:
>> +      - description: USB OTG reset
>> +      - description: USB Host 1 Controller reset
>> +
>> +  reset-names:
>> +    items:
>> +      - const: usb0_reset
>> +      - const: usb1_reset
>> +
>> +  usb0_id_det-gpios:
>> +    description: GPIO to the USB OTG ID pin
> 
> Needs 'maxItems: 1'
> 
>> +
>> +  usb0_vbus_det-gpios:
>> +    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
>> +
>> +  usb1_vbus-supply:
>> +    description: Regulator controlling USB1 Host controller
> 
> Are ID and VBus actually connected to the phy h/w? Really, all this 
> should be in a USB connector node for which we have bindings.
> 
>> +
>> +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/sun50i-a100-ccu.h>
>> +    #include <dt-bindings/reset/sun50i-a100-ccu.h>
>> +
>> +    phy@5100400 {
>> +        #phy-cells = <1>;
>> +        compatible = "allwinner,sun50i-a100-usb-phy";
>> +        reg = <0x05100400 0x14>,
>> +              <0x05101800 0x4>,
>> +              <0x05200800 0x4>;
>> +        reg-names = "phy_ctrl",
>> +                    "pmu0",
>> +                    "pmu1";
>> +        clocks = <&ccu CLK_USB_PHY0>,
>> +                 <&ccu CLK_USB_PHY1>;
>> +        clock-names = "usb0_phy",
>> +                      "usb1_phy";
>> +        resets = <&ccu RST_USB_PHY0>,
>> +                 <&ccu RST_USB_PHY1>;
>> +        reset-names = "usb0_reset",
>> +                      "usb1_reset";
>> +        usb0_id_det-gpios = <&pio 7 10 GPIO_ACTIVE_HIGH>; /* PH10 */
>> +        usb0_vbus_power-supply = <&usb_power_supply>;
>> +        usb0_vbus-supply = <&reg_drivevbus>;
>> +        usb1_vbus-supply = <&reg_usb1_vbus>;
>> +    };
>> -- 
>> 2.28.0
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Maxime Ripard Dec. 1, 2020, 6:44 p.m. UTC | #3 
On Sat, Nov 28, 2020 at 08:18:16PM +0000, André Przywara wrote:
> On 11/11/2020 22:50, Rob Herring wrote:
> 
> Hi,
> 
> > On Tue, Nov 10, 2020 at 02:39:42PM +0800, Frank Lee wrote:
> >> From: Yangtao Li <frank@allwinnertech.com>
> >>
> >> Add a device tree binding for the A100's USB PHY.
> 
> Not your fault, Yangto, but why do we actually have a separate binding
> document per SoC, when the differences between the PHYs are so minimal
> that we get away with some flags in the compatible match, in one driver
> file?

They are similar, but there's massive differences between them still
(like which regulators are supposed to be there, or the register names).
So putting them all in the same file just ended up in an unmaintainable
mess.

> For a start this file is basically identical to the A64 one (apart from
> the example), so can you just add the A100 compatible string to that
> one, instead?

But yeah, if two are identical they should be merged

Maxime
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml
new file mode 100644
index 000000000000..cc9bbebe2bd7
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/allwinner,sun50i-a100-usb-phy.yaml
@@ -0,0 +1,105 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/allwinner,sun50i-a100-usb-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner A100 USB PHY Device Tree Bindings
+
+maintainers:
+  - Yangtao Li <tiny.windzz@gmail.com>
+
+properties:
+  "#phy-cells":
+    const: 1
+
+  compatible:
+    const: allwinner,sun50i-a100-usb-phy
+
+  reg:
+    items:
+      - description: PHY Control registers
+      - description: PHY PMU0 registers
+      - description: PHY PMU1 registers
+
+  reg-names:
+    items:
+      - const: phy_ctrl
+      - const: pmu0
+      - const: pmu1
+
+  clocks:
+    items:
+      - description: USB OTG PHY bus clock
+      - description: USB Host 0 PHY bus clock
+
+  clock-names:
+    items:
+      - const: usb0_phy
+      - const: usb1_phy
+
+  resets:
+    items:
+      - description: USB OTG reset
+      - description: USB Host 1 Controller reset
+
+  reset-names:
+    items:
+      - const: usb0_reset
+      - const: usb1_reset
+
+  usb0_id_det-gpios:
+    description: GPIO to the USB OTG ID pin
+
+  usb0_vbus_det-gpios:
+    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
+
+  usb1_vbus-supply:
+    description: Regulator controlling USB1 Host controller
+
+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/sun50i-a100-ccu.h>
+    #include <dt-bindings/reset/sun50i-a100-ccu.h>
+
+    phy@5100400 {
+        #phy-cells = <1>;
+        compatible = "allwinner,sun50i-a100-usb-phy";
+        reg = <0x05100400 0x14>,
+              <0x05101800 0x4>,
+              <0x05200800 0x4>;
+        reg-names = "phy_ctrl",
+                    "pmu0",
+                    "pmu1";
+        clocks = <&ccu CLK_USB_PHY0>,
+                 <&ccu CLK_USB_PHY1>;
+        clock-names = "usb0_phy",
+                      "usb1_phy";
+        resets = <&ccu RST_USB_PHY0>,
+                 <&ccu RST_USB_PHY1>;
+        reset-names = "usb0_reset",
+                      "usb1_reset";
+        usb0_id_det-gpios = <&pio 7 10 GPIO_ACTIVE_HIGH>; /* PH10 */
+        usb0_vbus_power-supply = <&usb_power_supply>;
+        usb0_vbus-supply = <&reg_drivevbus>;
+        usb1_vbus-supply = <&reg_usb1_vbus>;
+    };