diff mbox series

[v4,07/10] dt-bindings: phy: add binding for Allwinner USB3 PHY

Message ID 20181004122855.22981-8-icenowy@aosc.io (mailing list archive)
State New, archived
Headers show
Series Allwinner H6 USB support | expand

Commit Message

Icenowy Zheng Oct. 4, 2018, 12:28 p.m. UTC 
The new Allwinner H6 SoC contains a USB3 PHY that is wired to the
external USB3 pins of the SoC.

Add a device tree binding for the PHY.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
Changes in v4:
- Changed Vbus regulator property to vbus-supply.

Changes in v3:
- Added Chen-Yu's Review tag.

No changes in v2, v1.

 .../bindings/phy/sun50i-usb3-phy.txt          | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt

Comments

Rob Herring Oct. 5, 2018, 8:58 p.m. UTC | #1 
On Thu, Oct 04, 2018 at 08:28:52PM +0800, Icenowy Zheng wrote:
> The new Allwinner H6 SoC contains a USB3 PHY that is wired to the
> external USB3 pins of the SoC.
> 
> Add a device tree binding for the PHY.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> ---
> Changes in v4:
> - Changed Vbus regulator property to vbus-supply.
> 
> Changes in v3:
> - Added Chen-Yu's Review tag.
> 
> No changes in v2, v1.
> 
>  .../bindings/phy/sun50i-usb3-phy.txt          | 23 +++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
> new file mode 100644
> index 000000000000..9f49c6b8c7e7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
> @@ -0,0 +1,23 @@
> +Allwinner sun50i USB3 PHY
> +-----------------------
> +
> +Required properties:
> +- compatible : should be one of
> +  * allwinner,sun60i-h6-usb3-phy
> +- reg : a list of offset + length pairs
> +- #phy-cells : from the generic phy bindings, must be 0
> +- clocks : phandle + clock specifier for the phy clock
> +- resets : phandle + reset specifier for the phy reset
> +
> +Optional Properties:
> +- vbus-supply : a phandle to a regulator that provides power to VBUS.

This belongs in a connector node as it is part of the connector unless 
the phy physically needs Vbus for power.

But others have done this, so all the phys can just be wrong...

> +
> +Example:
> +	usb3phy: phy@5210000 {

usb-phy@

> +		compatible = "allwinner,sun50i-h6-usb3-phy";
> +		reg = <0x5210000 0x10000>;
> +		clocks = <&ccu CLK_USB_PHY1>;
> +		resets = <&ccu RST_USB_PHY1>;
> +		#phy-cells = <0>;
> +		status = "disabled";

Don't show status in examples.

> +	};
> -- 
> 2.18.0
>
Icenowy Zheng Oct. 14, 2018, 2:41 a.m. UTC | #2 
在 2018-10-05五的 15:58 -0500，Rob Herring写道：
> On Thu, Oct 04, 2018 at 08:28:52PM +0800, Icenowy Zheng wrote:
> > The new Allwinner H6 SoC contains a USB3 PHY that is wired to the
> > external USB3 pins of the SoC.
> > 
> > Add a device tree binding for the PHY.
> > 
> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> > ---
> > Changes in v4:
> > - Changed Vbus regulator property to vbus-supply.
> > 
> > Changes in v3:
> > - Added Chen-Yu's Review tag.
> > 
> > No changes in v2, v1.
> > 
> >  .../bindings/phy/sun50i-usb3-phy.txt          | 23
> > +++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/sun50i-
> > usb3-phy.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/sun50i-usb3-
> > phy.txt b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
> > new file mode 100644
> > index 000000000000..9f49c6b8c7e7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
> > @@ -0,0 +1,23 @@
> > +Allwinner sun50i USB3 PHY
> > +-----------------------
> > +
> > +Required properties:
> > +- compatible : should be one of
> > +  * allwinner,sun60i-h6-usb3-phy
> > +- reg : a list of offset + length pairs
> > +- #phy-cells : from the generic phy bindings, must be 0
> > +- clocks : phandle + clock specifier for the phy clock
> > +- resets : phandle + reset specifier for the phy reset
> > +
> > +Optional Properties:
> > +- vbus-supply : a phandle to a regulator that provides power to
> > VBUS.
> 
> This belongs in a connector node as it is part of the connector
> unless 
> the phy physically needs Vbus for power.
> 
> But others have done this, so all the phys can just be wrong...

How should we reference the connector?

Via OF graph or simply a property in PHY node?

> 
> > +
> > +Example:
> > +	usb3phy: phy@5210000 {
> 
> usb-phy@
> 
> > +		compatible = "allwinner,sun50i-h6-usb3-phy";
> > +		reg = <0x5210000 0x10000>;
> > +		clocks = <&ccu CLK_USB_PHY1>;
> > +		resets = <&ccu RST_USB_PHY1>;
> > +		#phy-cells = <0>;
> > +		status = "disabled";
> 
> Don't show status in examples.
> 
> > +	};
> > -- 
> > 2.18.0
> > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Rob Herring Oct. 18, 2018, 1:58 p.m. UTC | #3 
On Sat, Oct 13, 2018 at 9:42 PM Icenowy Zheng <icenowy@aosc.io> wrote:
>
> 在 2018-10-05五的 15:58 -0500，Rob Herring写道：
> > On Thu, Oct 04, 2018 at 08:28:52PM +0800, Icenowy Zheng wrote:
> > > The new Allwinner H6 SoC contains a USB3 PHY that is wired to the
> > > external USB3 pins of the SoC.
> > >
> > > Add a device tree binding for the PHY.
> > >
> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> > > ---
> > > Changes in v4:
> > > - Changed Vbus regulator property to vbus-supply.
> > >
> > > Changes in v3:
> > > - Added Chen-Yu's Review tag.
> > >
> > > No changes in v2, v1.
> > >
> > >  .../bindings/phy/sun50i-usb3-phy.txt          | 23
> > > +++++++++++++++++++
> > >  1 file changed, 23 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/phy/sun50i-
> > > usb3-phy.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/sun50i-usb3-
> > > phy.txt b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
> > > new file mode 100644
> > > index 000000000000..9f49c6b8c7e7
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
> > > @@ -0,0 +1,23 @@
> > > +Allwinner sun50i USB3 PHY
> > > +-----------------------
> > > +
> > > +Required properties:
> > > +- compatible : should be one of
> > > +  * allwinner,sun60i-h6-usb3-phy
> > > +- reg : a list of offset + length pairs
> > > +- #phy-cells : from the generic phy bindings, must be 0
> > > +- clocks : phandle + clock specifier for the phy clock
> > > +- resets : phandle + reset specifier for the phy reset
> > > +
> > > +Optional Properties:
> > > +- vbus-supply : a phandle to a regulator that provides power to
> > > VBUS.
> >
> > This belongs in a connector node as it is part of the connector
> > unless
> > the phy physically needs Vbus for power.
> >
> > But others have done this, so all the phys can just be wrong...
>
> How should we reference the connector?
>
> Via OF graph or simply a property in PHY node?

The connector is either a child of the controller or an OF graph from
the controller to the connector. The phy driver needs the controller
node and then it can walk the tree or graph to get the connector node.

Rob
Icenowy Zheng Oct. 19, 2018, 5:54 a.m. UTC | #4 
于 2018年10月18日 GMT+08:00 下午9:58:25, Rob Herring <robh@kernel.org> 写到:
>On Sat, Oct 13, 2018 at 9:42 PM Icenowy Zheng <icenowy@aosc.io> wrote:
>>
>> 在 2018-10-05五的 15:58 -0500，Rob Herring写道：
>> > On Thu, Oct 04, 2018 at 08:28:52PM +0800, Icenowy Zheng wrote:
>> > > The new Allwinner H6 SoC contains a USB3 PHY that is wired to the
>> > > external USB3 pins of the SoC.
>> > >
>> > > Add a device tree binding for the PHY.
>> > >
>> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> > > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>> > > ---
>> > > Changes in v4:
>> > > - Changed Vbus regulator property to vbus-supply.
>> > >
>> > > Changes in v3:
>> > > - Added Chen-Yu's Review tag.
>> > >
>> > > No changes in v2, v1.
>> > >
>> > >  .../bindings/phy/sun50i-usb3-phy.txt          | 23
>> > > +++++++++++++++++++
>> > >  1 file changed, 23 insertions(+)
>> > >  create mode 100644 Documentation/devicetree/bindings/phy/sun50i-
>> > > usb3-phy.txt
>> > >
>> > > diff --git a/Documentation/devicetree/bindings/phy/sun50i-usb3-
>> > > phy.txt
>b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
>> > > new file mode 100644
>> > > index 000000000000..9f49c6b8c7e7
>> > > --- /dev/null
>> > > +++ b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
>> > > @@ -0,0 +1,23 @@
>> > > +Allwinner sun50i USB3 PHY
>> > > +-----------------------
>> > > +
>> > > +Required properties:
>> > > +- compatible : should be one of
>> > > +  * allwinner,sun60i-h6-usb3-phy
>> > > +- reg : a list of offset + length pairs
>> > > +- #phy-cells : from the generic phy bindings, must be 0
>> > > +- clocks : phandle + clock specifier for the phy clock
>> > > +- resets : phandle + reset specifier for the phy reset
>> > > +
>> > > +Optional Properties:
>> > > +- vbus-supply : a phandle to a regulator that provides power to
>> > > VBUS.
>> >
>> > This belongs in a connector node as it is part of the connector
>> > unless
>> > the phy physically needs Vbus for power.
>> >
>> > But others have done this, so all the phys can just be wrong...
>>
>> How should we reference the connector?
>>
>> Via OF graph or simply a property in PHY node?
>
>The connector is either a child of the controller or an OF graph from
>the controller to the connector. The phy driver needs the controller
>node and then it can walk the tree or graph to get the connector node.

Why is it a child of the controller?

I think on hardware the connector is connected to the PHY via USB, and PHY is
connected to the controller via ULPI/UTMI and PIPE, so the connector node
should link to PHY in some way, not to controller.

For Allwinner USB3 PHY I prefer to use child node now, as it's a simple
single-port PHY, and there seems to be no reserved space for multi-port.

>
>Rob
Icenowy Zheng Nov. 14, 2018, 5:15 a.m. UTC | #5 
在 2018-10-18四的 08:58 -0500，Rob Herring写道：
> On Sat, Oct 13, 2018 at 9:42 PM Icenowy Zheng <icenowy@aosc.io>
> wrote:
> > 
> > 在 2018-10-05五的 15:58 -0500，Rob Herring写道：
> > > On Thu, Oct 04, 2018 at 08:28:52PM +0800, Icenowy Zheng wrote:
> > > > The new Allwinner H6 SoC contains a USB3 PHY that is wired to
> > > > the
> > > > external USB3 pins of the SoC.
> > > > 
> > > > Add a device tree binding for the PHY.
> > > > 
> > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> > > > ---
> > > > Changes in v4:
> > > > - Changed Vbus regulator property to vbus-supply.
> > > > 
> > > > Changes in v3:
> > > > - Added Chen-Yu's Review tag.
> > > > 
> > > > No changes in v2, v1.
> > > > 
> > > >  .../bindings/phy/sun50i-usb3-phy.txt          | 23
> > > > +++++++++++++++++++
> > > >  1 file changed, 23 insertions(+)
> > > >  create mode 100644
> > > > Documentation/devicetree/bindings/phy/sun50i-
> > > > usb3-phy.txt
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/phy/sun50i-usb3-
> > > > phy.txt b/Documentation/devicetree/bindings/phy/sun50i-usb3-
> > > > phy.txt
> > > > new file mode 100644
> > > > index 000000000000..9f49c6b8c7e7
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
> > > > @@ -0,0 +1,23 @@
> > > > +Allwinner sun50i USB3 PHY
> > > > +-----------------------
> > > > +
> > > > +Required properties:
> > > > +- compatible : should be one of
> > > > +  * allwinner,sun60i-h6-usb3-phy
> > > > +- reg : a list of offset + length pairs
> > > > +- #phy-cells : from the generic phy bindings, must be 0
> > > > +- clocks : phandle + clock specifier for the phy clock
> > > > +- resets : phandle + reset specifier for the phy reset
> > > > +
> > > > +Optional Properties:
> > > > +- vbus-supply : a phandle to a regulator that provides power
> > > > to
> > > > VBUS.
> > > 
> > > This belongs in a connector node as it is part of the connector
> > > unless
> > > the phy physically needs Vbus for power.
> > > 
> > > But others have done this, so all the phys can just be wrong...
> > 
> > How should we reference the connector?
> > 
> > Via OF graph or simply a property in PHY node?
> 
> The connector is either a child of the controller or an OF graph from
> the controller to the connector. The phy driver needs the controller
> node and then it can walk the tree or graph to get the connector
> node.

By reading the binding, it says the connector should be a child of the
"interface controller", not the USB controller. In this case I think
the interface controller is the PHY rather than the USB controller.

In addition, the connector is a passive component, and power management
should be done by the "interface controller", either directly by the
PHY or make a dummy interface controller driver that only controls the
Vbus.

> 
> Rob
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
new file mode 100644
index 000000000000..9f49c6b8c7e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt
@@ -0,0 +1,23 @@ 
+Allwinner sun50i USB3 PHY
+-----------------------
+
+Required properties:
+- compatible : should be one of
+  * allwinner,sun60i-h6-usb3-phy
+- reg : a list of offset + length pairs
+- #phy-cells : from the generic phy bindings, must be 0
+- clocks : phandle + clock specifier for the phy clock
+- resets : phandle + reset specifier for the phy reset
+
+Optional Properties:
+- vbus-supply : a phandle to a regulator that provides power to VBUS.
+
+Example:
+	usb3phy: phy@5210000 {
+		compatible = "allwinner,sun50i-h6-usb3-phy";
+		reg = <0x5210000 0x10000>;
+		clocks = <&ccu CLK_USB_PHY1>;
+		resets = <&ccu RST_USB_PHY1>;
+		#phy-cells = <0>;
+		status = "disabled";
+	};