Re: [PATCH v7 1/5] dt-bindings: clock: qcom: Add CMN PLL clock controller for IPQ SoC

From: Jie Luo
Date: Wed Dec 25 2024 - 00:54:02 EST



On 12/22/2024 4:19 PM, Krzysztof Kozlowski wrote:
On Fri, Dec 20, 2024 at 09:22:42PM +0800, Luo Jie wrote:
The CMN PLL controller provides clocks to networking hardware blocks
and to GCC on Qualcomm IPQ9574 SoC. It receives input clock from the
on-chip Wi-Fi, and produces output clocks at fixed rates. These output
rates are predetermined, and are unrelated to the input clock rate.
The primary purpose of CMN PLL is to supply clocks to the networking
hardware such as PPE (packet process engine), PCS and the externally
connected switch or PHY device. The CMN PLL block also outputs fixed
rate clocks to GCC, such as 24 MHZ as XO clock and 32 KHZ as sleep
clock supplied to GCC.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
Signed-off-by: Luo Jie <quic_luoj@xxxxxxxxxxx>
---
.../bindings/clock/qcom,ipq9574-cmn-pll.yaml | 85 ++++++++++++++++++++++
include/dt-bindings/clock/qcom,ipq-cmn-pll.h | 22 ++++++
2 files changed, 107 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
new file mode 100644
index 000000000000..db8a3ee56067
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,ipq9574-cmn-pll.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm CMN PLL Clock Controller on IPQ SoC
+
+maintainers:
+ - Bjorn Andersson <andersson@xxxxxxxxxx>
+ - Luo Jie <quic_luoj@xxxxxxxxxxx>
+
+description:
+ The CMN (or common) PLL clock controller expects a reference
+ input clock. This reference clock is from the on-board Wi-Fi.
+ The CMN PLL supplies a number of fixed rate output clocks to
+ the devices providing networking functions and to GCC. These
+ networking hardware include PPE (packet process engine), PCS
+ and the externally connected switch or PHY devices. The CMN
+ PLL block also outputs fixed rate clocks to GCC. The PLL's
+ primary function is to enable fixed rate output clocks for
+ networking hardware functions used with the IPQ SoC.
+
+properties:
+ compatible:
+ enum:
+ - qcom,ipq9574-cmn-pll
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: The reference clock. The supported clock rates include
+ 25000000, 31250000, 40000000, 48000000, 50000000 and 96000000 HZ.
+ - description: The AHB clock
+ - description: The SYS clock
+ description:
+ The reference clock is the source clock of CMN PLL, which is from the
+ Wi-Fi. The AHB and SYS clocks must be enabled to access CMN PLL
+ clock registers.
+
+ clock-names:
+ items:
+ - const: ref
+ - const: ahb
+ - const: sys
+
+ "#clock-cells":
+ const: 1
+
+ assigned-clocks:

Drop

OK.


+ maxItems: 1
+
+ assigned-clock-rates-u64:
+ maxItems: 1

These wasn't here when you received review. Adding new properties always
invalidates review.

No, drop them.

OK.

I will remove the reviewed-by tag next time when such modifications are
made.



+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - "#clock-cells"
+ - assigned-clocks

Drop

OK.


+ - assigned-clock-rates-u64

Drop... or explain

I will drop it as the core DTS schema allows it for the clock
provider.

The DT property 'assigned-clock-rates-u64' is used for configuring the
clock rate of CMN PLL to 12 GHZ, which is the working clock rate of CMN
PLL on IPQ9574.


Drop all review tags after making significant changes like that.

Sure I will.


Best regards,
Krzysztof