Thread (18 messages) 18 messages, 4 authors, 2021-06-25

RE: [PATCH v2 01/11] dt-bindings: phy: renesas: Document RZ/G2L USB PHY Control bindings

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-06-23 15:46:01
Also in: linux-devicetree, linux-renesas-soc

Hi Rob,

Thanks for the feedback.
Subject: Re: [PATCH v2 01/11] dt-bindings: phy: renesas: Document RZ/G2L
USB PHY Control bindings

On Wed, Jun 23, 2021 at 7:38 AM Biju Das [off-list ref]
wrote:
quoted
Hi Rob,

Thanks for the feedback.
quoted
Subject: Re: [PATCH v2 01/11] dt-bindings: phy: renesas: Document
RZ/G2L USB PHY Control bindings

On Mon, Jun 21, 2021 at 10:39:33AM +0100, Biju Das wrote:
quoted
Add device tree binding document for RZ/G2L USB PHY control driver.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar
[off-list ref]
---
V1->V2:
 * Add clock properties
---
 .../phy/renesas,rzg2l-usbphyctrl.yaml         | 65
+++++++++++++++++++
quoted
quoted
quoted
 1 file changed, 65 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/phy/renesas,rzg2l-usbphyctrl.yam
l

diff --git
a/Documentation/devicetree/bindings/phy/renesas,rzg2l-usbphyctrl.y
aml
b/Documentation/devicetree/bindings/phy/renesas,rzg2l-usbphyctrl.y
aml
new file mode 100644
index 000000000000..8e8ba43f595d
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/renesas,rzg2l-usbphyct
+++ rl.y
+++ aml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
+1.2
+---
+$id:
+https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
+devi
+cetree.org%2Fschemas%2Fphy%2Frenesas%2Crzg2l-usbphyctrl.yaml%23&a
+mp;d
+ata=04%7C01%7Cbiju.das.jz%40bp.renesas.com%7Cc6bbf5f6ce334eaa722a
+08d9
+359f07ad%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63759977942
+1910
+039%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL
+CJBT
+iI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Jcf6Om4DehifCe1KO1rmt5
+LxTB
+6jtGoQLD1MoqWGM%2F0%3D&amp;reserved=0
+$schema:
+https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2F
+devi
+cetree.org%2Fmeta-
schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cbiju.das.
quoted
quoted
quoted
+jz%40bp.renesas.com%7Cc6bbf5f6ce334eaa722a08d9359f07ad%7C53d82571
+da19
+47e49cb4625a166a4a2a%7C0%7C0%7C637599779421910039%7CUnknown%7CTWF
+pbGZ
+sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
+Mn0%
+3D%7C1000&amp;sdata=LlqPRLf9%2BGrEdSapxCFhwxVKcXTVh9ECr%2FXPN0SIz
+i4%3
+D&amp;reserved=0
+
+title: Renesas RZ/G2L USB2.0 PHY Control
+
+maintainers:
+  - Biju Das [off-list ref]
+
+description:
+  The RZ/G2L USB2.0 PHY Control mainly controls reset and power
+down of the
+  USB/PHY.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a07g044-usbphyctrl # RZ/G2{L,LC}
+      - const: renesas,rzg2l-usbphyctrl
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  '#phy-cells':
+    # see phy-bindings.txt in the same directory
+    const: 1
+    description: |
+      The phandle's argument in the PHY specifier is the phy
+ reset
control bit
quoted
+      of usb phy control.
+      0 = Port 1 Phy reset
+      1 = Port 2 Phy reset
+    enum: [ 0, 1 ]
You already have the const, so this doesn't do anything.
OK, will take out const.
No, 'const' is correct. This is the value of '#phy-cells', not the
contents (we don't have a way to express schema for that).
OK.
quoted
quoted
quoted
+required:
+  - compatible
+  - reg
+  - clocks
+  - '#phy-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r9a07g044-cpg.h>
+
+    usbphyctrl@11c40000 {
usb-phy@...
The IP is called USBPHY control. It mainly controls reset and power down
of the USB2.0/PHY.

Sounds like it should be using the reset binding...
This IP has reset, clock control , connection control , clock status and power down setting registers.
Currenty we are using reset registers for turning ON USB/PHY block.

Since it has extra registers I thought of modelling it as a phy device. But we could model as reset device as well.
But it has extra functionalities apart from reset.

So what do you propose here? Model as a reset device or phy device since it is related to phy?
Please share your opinion on this.

Regards,
Biju
quoted
So not sure usb-phy is right one here ? I prefer usb-phy-ctrl instead.
Is it ok? Please let me know.

A node with #phy-cells should use the standard phy node names unless it
has other controls. 
Apart from reset, it has other controls like  clock control , connection control , clock status and powerdown setting registers.

Cheers,
Biju

As I said, this doesn't seem to be a phy, so using
#phy-cells here is what seems wrong.
quoted
quoted
quoted
+        compatible = "renesas,r9a07g044-usbphyctrl",
+                     "renesas,rzg2l-usbphyctrl";
+        reg = <0x11c40000 0x10000>;
+        clocks = <&cpg CPG_MOD R9A07G044_USB_PCLK>;
+        resets = <&cpg R9A07G044_USB_PCLK>;
+        power-domains = <&cpg>;
Also, are these all resources of the usbphyctrl block and not just
resources you happen to want in the driver? For example, the power-domain
should be the power island that this block resides in.
quoted
quoted
quoted
+        #phy-cells = <1>;
+    };
--
2.17.1
-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help