diff mbox series

[1/6] dt-bindings: clock: qcom: Add CMN PLL support for IPQ5018 SoC

Message ID 20250502-ipq5018-cmn-pll-v1-1-27902c1c4071@outlook.com
State New
Headers show
Series Add CMN PLL clock controller support for IPQ5018 | expand

Commit Message

George Moussalem via B4 Relay May 2, 2025, 10:15 a.m. UTC 
From: George Moussalem <george.moussalem@outlook.com>

The CMN PLL block in the IPQ5018 SoC takes 96 MHZ as the reference
input clock. Its output clocks are the XO (24Mhz), sleep (32Khz), and
ethernet (50Mhz) clocks.

Unlike IPQ9574, the CMN PLL to the ethernet block needs to be enabled
first in IPQ5018. Hence, add optional phandle to TCSR register space
and offset to do so.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml  | 11 ++++++++---
 include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h         | 16 ++++++++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)

Comments

Rob Herring May 2, 2025, 11:35 a.m. UTC | #1 
On Fri, 02 May 2025 14:15:43 +0400, George Moussalem wrote:
> The CMN PLL block in the IPQ5018 SoC takes 96 MHZ as the reference
> input clock. Its output clocks are the XO (24Mhz), sleep (32Khz), and
> ethernet (50Mhz) clocks.
> 
> Unlike IPQ9574, the CMN PLL to the ethernet block needs to be enabled
> first in IPQ5018. Hence, add optional phandle to TCSR register space
> and offset to do so.
> 
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> ---
>  .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml  | 11 ++++++++---
>  include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h         | 16 ++++++++++++++++
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml:55:9: [warning] wrong indentation: expected 6 but found 8 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.example.dtb: clock-controller@9b000 (qcom,ipq9574-cmn-pll): 'reg' does not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/clock/qcom,ipq9574-cmn-pll.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250502-ipq5018-cmn-pll-v1-1-27902c1c4071@outlook.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Jie Luo May 4, 2025, 1:49 a.m. UTC | #2 
On 5/2/2025 6:15 PM, George Moussalem via B4 Relay wrote:
> From: George Moussalem <george.moussalem@outlook.com>
> 
> The CMN PLL block in the IPQ5018 SoC takes 96 MHZ as the reference
> input clock. Its output clocks are the XO (24Mhz), sleep (32Khz), and
> ethernet (50Mhz) clocks.
> 
> Unlike IPQ9574, the CMN PLL to the ethernet block needs to be enabled
> first in IPQ5018. Hence, add optional phandle to TCSR register space
> and offset to do so.
> 
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> ---
>   .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml  | 11 ++++++++---
>   include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h         | 16 ++++++++++++++++
>   2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> index cb6e09f4247f4b25105b25f4ae746c0b3ef47616..25006d65d30e20ef8e1f43537bcf3dca65bae73d 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> @@ -24,12 +24,10 @@ description:
>   properties:
>     compatible:
>       enum:
> +      - qcom,ipq5018-cmn-pll
>         - qcom,ipq5424-cmn-pll
>         - qcom,ipq9574-cmn-pll
>   
> -  reg:
> -    maxItems: 1
> -

The property 'reg' should not be removed.

>     clocks:
>       items:
>         - description: The reference clock. The supported clock rates include
> @@ -50,6 +48,13 @@ properties:
>     "#clock-cells":
>       const: 1
>   
> +  qcom,cmn-pll-eth-enable:
> +    description: Register in TCSR to enable CMN PLL to ethernet
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +        - description: phandle of TCSR syscon
> +        - description: offset of TCSR register to enable CMN PLL to ethernet
> +

This TCSR should not be a part of CMN PLL, it is the LDO controller for
the internal GEPHY in IPQ5018 SoC, which can be moved to a part of GEPHY
device.

>   required:
>     - compatible
>     - reg
> diff --git a/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..586d1c9b33b374331bef413f543c526212c18494
> --- /dev/null
> +++ b/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_QCOM_IPQ5018_CMN_PLL_H
> +#define _DT_BINDINGS_CLK_QCOM_IPQ5018_CMN_PLL_H
> +
> +/* CMN PLL core clock. */
> +#define IPQ5018_CMN_PLL_CLK			0
> +
> +/* The output clocks from CMN PLL of IPQ5018. */
> +#define IPQ5018_XO_24MHZ_CLK			1
> +#define IPQ5018_SLEEP_32KHZ_CLK			2
> +#define IPQ5018_ETH_50MHZ_CLK			3
> +#endif
>
George Moussalem May 4, 2025, 7:03 a.m. UTC | #3 
On 5/4/25 05:49, Jie Luo wrote:
> 
> 
> On 5/2/2025 6:15 PM, George Moussalem via B4 Relay wrote:
>> From: George Moussalem <george.moussalem@outlook.com>
>>
>> The CMN PLL block in the IPQ5018 SoC takes 96 MHZ as the reference
>> input clock. Its output clocks are the XO (24Mhz), sleep (32Khz), and
>> ethernet (50Mhz) clocks.
>>
>> Unlike IPQ9574, the CMN PLL to the ethernet block needs to be enabled
>> first in IPQ5018. Hence, add optional phandle to TCSR register space
>> and offset to do so.
>>
>> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
>> ---
>>    .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml  | 11 ++++++++---
>>    include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h         | 16 ++++++++++++++++
>>    2 files changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> index cb6e09f4247f4b25105b25f4ae746c0b3ef47616..25006d65d30e20ef8e1f43537bcf3dca65bae73d 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> @@ -24,12 +24,10 @@ description:
>>    properties:
>>      compatible:
>>        enum:
>> +      - qcom,ipq5018-cmn-pll
>>          - qcom,ipq5424-cmn-pll
>>          - qcom,ipq9574-cmn-pll
>>    
>> -  reg:
>> -    maxItems: 1
>> -
> 
> The property 'reg' should not be removed.

Moved it back, I unintentionally removed it. Thanks!

> 
>>      clocks:
>>        items:
>>          - description: The reference clock. The supported clock rates include
>> @@ -50,6 +48,13 @@ properties:
>>      "#clock-cells":
>>        const: 1
>>    
>> +  qcom,cmn-pll-eth-enable:
>> +    description: Register in TCSR to enable CMN PLL to ethernet
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    items:
>> +        - description: phandle of TCSR syscon
>> +        - description: offset of TCSR register to enable CMN PLL to ethernet
>> +
> 
> This TCSR should not be a part of CMN PLL, it is the LDO controller for
> the internal GEPHY in IPQ5018 SoC, which can be moved to a part of GEPHY
> device.

I'm preparing a patch for the ipq5018 GE PHY too so can move it there. 
Can you confirm the uniphy isn't dependent on this too? There are boards 
out there which don't use the GE PHY at all, so if the uniphy depends on 
it for SGMII/SGMII+, this approach wouldn't work.

> 
>>    required:
>>      - compatible
>>      - reg
>> diff --git a/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..586d1c9b33b374331bef413f543c526212c18494
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>> +/*
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#ifndef _DT_BINDINGS_CLK_QCOM_IPQ5018_CMN_PLL_H
>> +#define _DT_BINDINGS_CLK_QCOM_IPQ5018_CMN_PLL_H
>> +
>> +/* CMN PLL core clock. */
>> +#define IPQ5018_CMN_PLL_CLK			0
>> +
>> +/* The output clocks from CMN PLL of IPQ5018. */
>> +#define IPQ5018_XO_24MHZ_CLK			1
>> +#define IPQ5018_SLEEP_32KHZ_CLK			2
>> +#define IPQ5018_ETH_50MHZ_CLK			3
>> +#endif
>>
> 

Best regards,
George
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
index cb6e09f4247f4b25105b25f4ae746c0b3ef47616..25006d65d30e20ef8e1f43537bcf3dca65bae73d 100644
--- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
@@ -24,12 +24,10 @@  description:
 properties:
   compatible:
     enum:
+      - qcom,ipq5018-cmn-pll
       - qcom,ipq5424-cmn-pll
       - qcom,ipq9574-cmn-pll
 
-  reg:
-    maxItems: 1
-
   clocks:
     items:
       - description: The reference clock. The supported clock rates include
@@ -50,6 +48,13 @@  properties:
   "#clock-cells":
     const: 1
 
+  qcom,cmn-pll-eth-enable:
+    description: Register in TCSR to enable CMN PLL to ethernet
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+        - description: phandle of TCSR syscon
+        - description: offset of TCSR register to enable CMN PLL to ethernet
+
 required:
   - compatible
   - reg
diff --git a/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h
new file mode 100644
index 0000000000000000000000000000000000000000..586d1c9b33b374331bef413f543c526212c18494
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h
@@ -0,0 +1,16 @@ 
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_QCOM_IPQ5018_CMN_PLL_H
+#define _DT_BINDINGS_CLK_QCOM_IPQ5018_CMN_PLL_H
+
+/* CMN PLL core clock. */
+#define IPQ5018_CMN_PLL_CLK			0
+
+/* The output clocks from CMN PLL of IPQ5018. */
+#define IPQ5018_XO_24MHZ_CLK			1
+#define IPQ5018_SLEEP_32KHZ_CLK			2
+#define IPQ5018_ETH_50MHZ_CLK			3
+#endif