Hi, Lubomir,

Thank you for your review!

On Wed, Jan 27, 2021 at 01:27:17PM +0200, kostap@marvell.com wrote:

quoted

From: Konstantin Porotchkin <redacted>

Add DTS binding for Marvell CP110 UTMI driver

Signed-off-by: Konstantin Porotchkin <redacted>

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.

quoted

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

...

quoted

 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.

quoted

+			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".

quoted

+- #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

quoted

+				  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.

quoted

 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=