diff mbox series

[v2,2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

Message ID 4f4136a91b24d3ad35fa12bd19fe14b83da9affe.1687414716.git.quic_varada@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Enable IPQ5332 USB2 | expand

Commit Message

Varadarajan Narayanan June 22, 2023, 6:22 a.m. UTC 
Document the M31 USB2 phy present in IPQ5332.

Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v1:
	Rename qcom,m31.yaml -> qcom,ipq5332-usb-hsphy.yaml
	Drop default binding "m31,usb-hsphy"
	Add clock
	Remove 'oneOf' from compatible
	Remove 'qscratch' region from register space as it is not needed
	Remove reset-names
	Fix the example definition
---
 .../bindings/phy/qcom,ipq5332-usb-hsphy.yaml       | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml

Comments

Rob Herring (Arm) June 22, 2023, 2:46 p.m. UTC | #1 
On Thu, Jun 22, 2023 at 11:52:09AM +0530, Varadarajan Narayanan wrote:
> Document the M31 USB2 phy present in IPQ5332.
> 
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v1:
> 	Rename qcom,m31.yaml -> qcom,ipq5332-usb-hsphy.yaml
> 	Drop default binding "m31,usb-hsphy"
> 	Add clock
> 	Remove 'oneOf' from compatible
> 	Remove 'qscratch' region from register space as it is not needed
> 	Remove reset-names
> 	Fix the example definition
> ---
>  .../bindings/phy/qcom,ipq5332-usb-hsphy.yaml       | 51 ++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> new file mode 100644
> index 0000000..ab2e945
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,ipq5332-usb-hsphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: M31 (https://www.m31tech.com) USB PHY

Put the URL in 'description'.

> +
> +maintainers:
> +  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
> +  - Varadarajan Narayanan <quic_varada@quicinc.org>

.org? It's .com everywhere else.

> +
> +description:
> +  USB M31 PHY found in Qualcomm IPQ5018, IPQ5332 SoCs.

Where's the IPQ5018 compatible?

> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - qcom,ipq5332-usb-hsphy
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    maxItems: 1
> +    contains:

'contains' is not appropriate here. Drop.
> +      items:
> +        - const: cfg_ahb

Don't need both items list and maxItems. Really, you don't need 
'clock-names' at all because there is only 1 clock.

> +
> +  resets:
> +    maxItems: 1
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> +    usbphy0: ipq5332-hsphy@7b000 {

Drop unused labels.

> +    	compatible = "qcom,ipq5332-usb-hsphy";
> +    	reg = <0x0007b000 0x12c>;
> +
> +    	clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
> +    	clock-names = "cfg_ahb";
> +
> +    	resets = <&gcc GCC_QUSB2_0_PHY_BCR>;

Whitespace errors in here.

> +    };
> +
> -- 
> 2.7.4
>
Varadarajan Narayanan June 23, 2023, 5:45 a.m. UTC | #2 
On Thu, Jun 22, 2023 at 08:46:27AM -0600, Rob Herring wrote:
> On Thu, Jun 22, 2023 at 11:52:09AM +0530, Varadarajan Narayanan wrote:
> > Document the M31 USB2 phy present in IPQ5332.
> >
> > Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> > v1:
> > 	Rename qcom,m31.yaml -> qcom,ipq5332-usb-hsphy.yaml
> > 	Drop default binding "m31,usb-hsphy"
> > 	Add clock
> > 	Remove 'oneOf' from compatible
> > 	Remove 'qscratch' region from register space as it is not needed
> > 	Remove reset-names
> > 	Fix the example definition
> > ---
> >  .../bindings/phy/qcom,ipq5332-usb-hsphy.yaml       | 51 ++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> > new file mode 100644
> > index 0000000..ab2e945
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> > @@ -0,0 +1,51 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/qcom,ipq5332-usb-hsphy.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: M31 (https://www.m31tech.com) USB PHY
>
> Put the URL in 'description'.

Ok.

> > +
> > +maintainers:
> > +  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
> > +  - Varadarajan Narayanan <quic_varada@quicinc.org>
>
> .org? It's .com everywhere else.

Ok.

> > +
> > +description:
> > +  USB M31 PHY found in Qualcomm IPQ5018, IPQ5332 SoCs.
>
> Where's the IPQ5018 compatible?

In the previous version had a default and IPQ5332 specific
compatible. IPQ5018 would have used the default compatible.
However, in the review was asked to drop the default compatible.
Hence planned to include ipq5018 compatible and post it in
separate patchset while enabling IPQ5018 USB. IPQ5018 init is
also diffferent from the init used here.

> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - qcom,ipq5332-usb-hsphy
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    maxItems: 1
> > +    contains:
>
> 'contains' is not appropriate here. Drop.
>
> > +      items:
> > +        - const: cfg_ahb
>
> Don't need both items list and maxItems. Really, you don't need
> 'clock-names' at all because there is only 1 clock.

Will drop 'clock-names'.

> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> > +    usbphy0: ipq5332-hsphy@7b000 {
>
> Drop unused labels.

Ok.

> > +    	compatible = "qcom,ipq5332-usb-hsphy";
> > +    	reg = <0x0007b000 0x12c>;
> > +
> > +    	clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
> > +    	clock-names = "cfg_ahb";
> > +
> > +    	resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
>
> Whitespace errors in here.

Ok.

Thanks
Varada

> > +    };
> > +
> > --
> > 2.7.4
> >
Krzysztof Kozlowski June 23, 2023, 8:08 a.m. UTC | #3 
On 23/06/2023 07:45, Varadarajan Narayanan wrote:
> 
>>> +
>>> +description:
>>> +  USB M31 PHY found in Qualcomm IPQ5018, IPQ5332 SoCs.
>>
>> Where's the IPQ5018 compatible?
> 
> In the previous version had a default and IPQ5332 specific
> compatible. IPQ5018 would have used the default compatible.
> However, in the review was asked to drop the default compatible.
> Hence planned to include ipq5018 compatible and post it in
> separate patchset while enabling IPQ5018 USB. IPQ5018 init is
> also diffferent from the init used here.

Your previous patch did not have ipq5018 compatible. There was nothing
indicating that some default means ipq5018...

Best regards,
Krzysztof
Krzysztof Kozlowski June 23, 2023, 8:08 a.m. UTC | #4 
On 22/06/2023 08:22, Varadarajan Narayanan wrote:
> Document the M31 USB2 phy present in IPQ5332.
> 

> +description:
> +  USB M31 PHY found in Qualcomm IPQ5018, IPQ5332 SoCs.
> +
> +properties:
> +  compatible:
> +    items:

Also drop items.

> +      - enum:
> +          - qcom,ipq5332-usb-hsphy
> +


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
new file mode 100644
index 0000000..ab2e945
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
@@ -0,0 +1,51 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,ipq5332-usb-hsphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: M31 (https://www.m31tech.com) USB PHY
+
+maintainers:
+  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
+  - Varadarajan Narayanan <quic_varada@quicinc.org>
+
+description:
+  USB M31 PHY found in Qualcomm IPQ5018, IPQ5332 SoCs.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,ipq5332-usb-hsphy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    maxItems: 1
+    contains:
+      items:
+        - const: cfg_ahb
+
+  resets:
+    maxItems: 1
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
+    usbphy0: ipq5332-hsphy@7b000 {
+    	compatible = "qcom,ipq5332-usb-hsphy";
+    	reg = <0x0007b000 0x12c>;
+
+    	clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
+    	clock-names = "cfg_ahb";
+
+    	resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
+    };
+