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 <biju.das.jz@xxxxxxxxxxxxxx> > wrote: > > > > Hi Rob, > > > > Thanks for the feedback. > > > > > 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: > > > > Add device tree binding document for RZ/G2L USB PHY control driver. > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > > > Reviewed-by: Lad Prabhakar > > > > <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > > --- > > > > V1->V2: > > > > * Add clock properties > > > > --- > > > > .../phy/renesas,rzg2l-usbphyctrl.yaml | 65 > +++++++++++++++++++ > > > > 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&sdata=Jcf6Om4DehifCe1KO1rmt5 > > > > +LxTB > > > > +6jtGoQLD1MoqWGM%2F0%3D&reserved=0 > > > > +$schema: > > > > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2F > > > > +devi > > > > +cetree.org%2Fmeta- > schemas%2Fcore.yaml%23&data=04%7C01%7Cbiju.das. > > > > +jz%40bp.renesas.com%7Cc6bbf5f6ce334eaa722a08d9359f07ad%7C53d82571 > > > > +da19 > > > > +47e49cb4625a166a4a2a%7C0%7C0%7C637599779421910039%7CUnknown%7CTWF > > > > +pbGZ > > > > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > > > > +Mn0% > > > > +3D%7C1000&sdata=LlqPRLf9%2BGrEdSapxCFhwxVKcXTVh9ECr%2FXPN0SIz > > > > +i4%3 > > > > +D&reserved=0 > > > > + > > > > +title: Renesas RZ/G2L USB2.0 PHY Control > > > > + > > > > +maintainers: > > > > + - Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > > > + > > > > +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 > > > > + 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. > > > > > +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 > > > > 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. > > > > > + 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. > > > > > + #phy-cells = <1>; > > > > + }; > > > > -- > > > > 2.17.1 > > > > > > > >
