diff mbox series

[2/4] devicetree/bindings: add support for CP110 UTMI driver

Message ID 20210127112719.30632-3-kostap@marvell.com (mailing list archive)
State New, archived
Headers show
Series Add support for CP110 UTMI PHY | expand

Commit Message

Kostya Porotchkin Jan. 27, 2021, 11:27 a.m. UTC 
From: Konstantin Porotchkin <kostap@marvell.com>

Add DTS binding for Marvell CP110 UTMI driver

Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
---
 Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt | 69 ++++++++++++++++++--
 1 file changed, 63 insertions(+), 6 deletions(-)

Comments

Lubomir Rintel Jan. 29, 2021, 9:38 a.m. UTC | #1 
Hi,

On Wed, Jan 27, 2021 at 01:27:17PM +0200, kostap@marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
> 
> Add DTS binding for Marvell CP110 UTMI driver
> 
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>

Any chance you could convert the document to YAML so that it could be
used for automatic validation?

> ---
>  Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt | 69 ++++++++++++++++++--
>  1 file changed, 63 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> index aa99ceec73b0..109888ba9d2d 100644
> --- a/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> @@ -1,30 +1,61 @@
> -MVEBU A3700 UTMI PHY
> ---------------------
> +MVEBU UTMI PHY
> +---------------
>  
>  USB2 UTMI+ PHY controllers can be found on the following Marvell MVEBU SoCs:
>  * Armada 3700
> +* Armada 7k/8k (on the CP110)
> +* Armada CN913x (on the CP115)
>  
>  On Armada 3700, there are two USB controllers, one is compatible with the USB2
>  and USB3 specifications and supports OTG. The other one is USB2 compliant and
>  only supports host mode. Both of these controllers come with a slightly
>  different UTMI PHY.
>  
> +On Armada 7k/8k and CN913x, there are two host and one device USB controllers.
> +Each of two exiting UTMI PHYs could be connected to either USB host or USB device
> +controller.
> +The USB device controller can only be connected to a single UTMI PHY port:
> +                    0.H----- USB HOST0
> +UTMI PHY0  --------/
> +                    0.D-----0
> +                             \------ USB DEVICE
> +                    1.D-----1
> +UTMI PHY1  --------\
> +                    1.H----- USB HOST1
> +
> +
>  Required Properties:
>  
>  - compatible: Should be one of:
>  	      * "marvell,a3700-utmi-host-phy" for the PHY connected to
> -	        the USB2 host-only controller.
> +	        the USB2 host-only controller (for Armada3700 only).
>  	      * "marvell,a3700-utmi-otg-phy" for the PHY connected to
> -	        the USB3 and USB2 OTG capable controller.
> +	        the USB3 and USB2 OTG capable controller (for Armada3700 only.
> +	      * "marvell,cp110-utmi-phy" (for Armada 7k/8k or CN913x only).
>  - reg: PHY IP register range.
>  - marvell,usb-misc-reg: handle on the "USB miscellaneous registers" shared
>  			region covering registers related to both the host
> -			controller and the PHY.
> -- #phy-cells: Standard property (Documentation: phy-bindings.txt) Should be 0.
> +			controller and the PHY (for Armada3700 only).
> +- marvell,system-controller: should contain a phandle to the system
> +			     controller node (for Armada 7k/8k or CN913x only)

I guess this is okay, but referring to a syscon is done in a multitude
ways across various other bindings; with the most popular being just:

  syscon = <&syscon>;

Perhaps consider doing that?

> +- #phy-cells: Standard property (Documentation: phy-bindings.txt.
> +		Should be 0 (for Armada3700 only).
> +
> +
> +Required properties (child nodes, for Armada 7k/8k/CN913x only):
> +
> +- reg: UTMI PHY port ID (0 or 1).
> +- #phy-cells : Should be 0.
> +
> +
> +Optional Properties (child nodes, for Armada 7k/8k/CN913x only)::
>  
> +- marvell,cp110-utmi-device-mode: request the driver to connect the UTMI PHY
> +				  port to USB device controller.

Do you need a separate property for this? Could the driver look at
"dr_mode" property of the USB controller node to see if it's supposed
to be in device/peripheral mode?

>  Example:
>  
> +Armada3700
>  	usb2_utmi_host_phy: phy@5f000 {
>  		compatible = "marvell,armada-3700-utmi-host-phy";
>  		reg = <0x5f000 0x800>;
> @@ -36,3 +67,29 @@ Example:
>  		compatible = "marvell,armada-3700-usb2-host-misc", "syscon";
>  		reg = <0x5f800 0x800>;
>  	};
> +
> +Armada 7k/8k/CN913x
> +
> +	CP11X_LABEL(utmi): utmi@580000 {
> +		compatible = "marvell,cp110-utmi-phy";
> +		reg = <0x580000 0x2000>;
> +		marvell,system-controller = <&CP11X_LABEL(syscon0)>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		status = "disabled";
> +
> +		CP11X_LABEL(utmi0): phy@0 {
> +			/* UTMI PHY port-0 is connected to USB Host controller-0 */
> +			reg = <0>;
> +			#phy-cells = <0>;
> +		};
> +
> +		CP11X_LABEL(utmi1): phy@1 {
> +			/* UTMI PHY port-1 is connected to USB device controller */
> +			reg = <1>;
> +			#phy-cells = <0>;
> +			marvell,cp110-utmi-device-mode;
> +		};
> +	};
> +
> +
> -- 
> 2.17.1
>
Kostya Porotchkin Jan. 31, 2021, 3:05 p.m. UTC | #2 
Hi, Lubomir,

Thank you for your review!

> On Wed, Jan 27, 2021 at 01:27:17PM +0200, kostap@marvell.com wrote:
> > From: Konstantin Porotchkin <kostap@marvell.com>
> >
> > Add DTS binding for Marvell CP110 UTMI driver
> >
> > Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> 
> Any chance you could convert the document to YAML so that it could be used
> for automatic validation?
> 
[KP] I believe it is possible, but probably should be done by a separate patch.
Here I am extending the existing documentation.

> > ---
> >  Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt | 69
> > ++++++++++++++++++--
> >  1 file changed, 63 insertions(+), 6 deletions(-)

...
> >  Required Properties:
> >
> >  - compatible: Should be one of:
> >  	      * "marvell,a3700-utmi-host-phy" for the PHY connected to
> > -	        the USB2 host-only controller.
> > +	        the USB2 host-only controller (for Armada3700 only).
> >  	      * "marvell,a3700-utmi-otg-phy" for the PHY connected to
> > -	        the USB3 and USB2 OTG capable controller.
> > +	        the USB3 and USB2 OTG capable controller (for Armada3700 only.
> > +	      * "marvell,cp110-utmi-phy" (for Armada 7k/8k or CN913x only).
> >  - reg: PHY IP register range.
> >  - marvell,usb-misc-reg: handle on the "USB miscellaneous registers" shared
> >  			region covering registers related to both the host
> > -			controller and the PHY.
> > -- #phy-cells: Standard property (Documentation: phy-bindings.txt) Should be
> 0.
> > +			controller and the PHY (for Armada3700 only).
> > +- marvell,system-controller: should contain a phandle to the system
> > +			     controller node (for Armada 7k/8k or CN913x only)
> 
> I guess this is okay, but referring to a syscon is done in a multitude ways across
> various other bindings; with the most popular being just:
> 
>   syscon = <&syscon>;
> 
> Perhaps consider doing that?
[KP] I was not sure that I can use a generic name inside the PHY entry 
if it is not defined as part of the generic PHY properties. 
I just did not see a good example of such in PHY bindings documentation.
If it is legal, I can change this entry name to just "syscon".
> 
> > +- #phy-cells: Standard property (Documentation: phy-bindings.txt.
> > +		Should be 0 (for Armada3700 only).
> > +
> > +
> > +Required properties (child nodes, for Armada 7k/8k/CN913x only):
> > +
> > +- reg: UTMI PHY port ID (0 or 1).
> > +- #phy-cells : Should be 0.
> > +
> > +
> > +Optional Properties (child nodes, for Armada 7k/8k/CN913x only)::
> >
> > +- marvell,cp110-utmi-device-mode: request the driver to connect the UTMI
> PHY
> > +				  port to USB device controller.
> 
> Do you need a separate property for this? Could the driver look at "dr_mode"
> property of the USB controller node to see if it's supposed to be in
> device/peripheral mode?
[KP] Yes, it seems I missed this option. I will try to change the code to support it in version 2.

> 
> >  Example:
> >
> > +Armada3700
> >  	usb2_utmi_host_phy: phy@5f000 {
> >  		compatible = "marvell,armada-3700-utmi-host-phy";
> >  		reg = <0x5f000 0x800>;
> > @@ -36,3 +67,29 @@ Example:
> > --
> > 2.17.1
> >
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__lists.infradead.org_mailman_listinfo_linux-2Darm-
> 2Dkernel&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=-
> N9sN4p5NSr0JGQoQ_2UCOgAqajG99W1EbSOww0WU8o&m=_ZOAKZShBT3Qj
> uT3RZIld2HoLnlvv6gkbHW9gSvEfI4&s=ggCBpvhDLJ8M6-
> Q41qbt8GRxryUo_mHxLMkUl8Ao5mA&e=
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
index aa99ceec73b0..109888ba9d2d 100644
--- a/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
+++ b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
@@ -1,30 +1,61 @@ 
-MVEBU A3700 UTMI PHY
---------------------
+MVEBU UTMI PHY
+---------------
 
 USB2 UTMI+ PHY controllers can be found on the following Marvell MVEBU SoCs:
 * Armada 3700
+* Armada 7k/8k (on the CP110)
+* Armada CN913x (on the CP115)
 
 On Armada 3700, there are two USB controllers, one is compatible with the USB2
 and USB3 specifications and supports OTG. The other one is USB2 compliant and
 only supports host mode. Both of these controllers come with a slightly
 different UTMI PHY.
 
+On Armada 7k/8k and CN913x, there are two host and one device USB controllers.
+Each of two exiting UTMI PHYs could be connected to either USB host or USB device
+controller.
+The USB device controller can only be connected to a single UTMI PHY port:
+                    0.H----- USB HOST0
+UTMI PHY0  --------/
+                    0.D-----0
+                             \------ USB DEVICE
+                    1.D-----1
+UTMI PHY1  --------\
+                    1.H----- USB HOST1
+
+
 Required Properties:
 
 - compatible: Should be one of:
 	      * "marvell,a3700-utmi-host-phy" for the PHY connected to
-	        the USB2 host-only controller.
+	        the USB2 host-only controller (for Armada3700 only).
 	      * "marvell,a3700-utmi-otg-phy" for the PHY connected to
-	        the USB3 and USB2 OTG capable controller.
+	        the USB3 and USB2 OTG capable controller (for Armada3700 only.
+	      * "marvell,cp110-utmi-phy" (for Armada 7k/8k or CN913x only).
 - reg: PHY IP register range.
 - marvell,usb-misc-reg: handle on the "USB miscellaneous registers" shared
 			region covering registers related to both the host
-			controller and the PHY.
-- #phy-cells: Standard property (Documentation: phy-bindings.txt) Should be 0.
+			controller and the PHY (for Armada3700 only).
+- marvell,system-controller: should contain a phandle to the system
+			     controller node (for Armada 7k/8k or CN913x only)
+- #phy-cells: Standard property (Documentation: phy-bindings.txt.
+		Should be 0 (for Armada3700 only).
+
+
+Required properties (child nodes, for Armada 7k/8k/CN913x only):
+
+- reg: UTMI PHY port ID (0 or 1).
+- #phy-cells : Should be 0.
+
+
+Optional Properties (child nodes, for Armada 7k/8k/CN913x only)::
 
+- marvell,cp110-utmi-device-mode: request the driver to connect the UTMI PHY
+				  port to USB device controller.
 
 Example:
 
+Armada3700
 	usb2_utmi_host_phy: phy@5f000 {
 		compatible = "marvell,armada-3700-utmi-host-phy";
 		reg = <0x5f000 0x800>;
@@ -36,3 +67,29 @@  Example:
 		compatible = "marvell,armada-3700-usb2-host-misc", "syscon";
 		reg = <0x5f800 0x800>;
 	};
+
+Armada 7k/8k/CN913x
+
+	CP11X_LABEL(utmi): utmi@580000 {
+		compatible = "marvell,cp110-utmi-phy";
+		reg = <0x580000 0x2000>;
+		marvell,system-controller = <&CP11X_LABEL(syscon0)>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+
+		CP11X_LABEL(utmi0): phy@0 {
+			/* UTMI PHY port-0 is connected to USB Host controller-0 */
+			reg = <0>;
+			#phy-cells = <0>;
+		};
+
+		CP11X_LABEL(utmi1): phy@1 {
+			/* UTMI PHY port-1 is connected to USB device controller */
+			reg = <1>;
+			#phy-cells = <0>;
+			marvell,cp110-utmi-device-mode;
+		};
+	};
+
+