Re: [PATCH 02/10] dt-bindings: clock: Add Amlogic A9 PLL clock controller

From: Jian Hu

Date: Fri May 22 2026 - 02:20:55 EST

Hi Krzysztof,

Thanks for your review.

On 5/15/2026 4:09 PM, Krzysztof Kozlowski wrote:

[ EXTERNAL EMAIL ]

On Mon, May 11, 2026 at 08:47:24PM +0800, Jian Hu wrote:

Add the PLL clock controller dt-bindings for the Amlogic A9 SoC family.

Signed-off-by: Jian Hu <jian.hu@xxxxxxxxxxx>
---
.../bindings/clock/amlogic,a9-pll-clkc.yaml | 110 +++++++++++++++++++++
include/dt-bindings/clock/amlogic,a9-pll-clkc.h | 55 +++++++++++
2 files changed, 165 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/amlogic,a9-pll-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a9-pll-clkc.yaml
new file mode 100644
index 000000000000..4ee6013ba1a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/amlogic,a9-pll-clkc.yaml
@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2026 Amlogic, Inc. All rights reserved
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/amlogic,a9-pll-clkc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic A9 Series PLL Clock Controller
+
+maintainers:
+ - Neil Armstrong <neil.armstrong@xxxxxxxxxx>
+ - Jerome Brunet <jbrunet@xxxxxxxxxxxx>
+ - Jian Hu <jian.hu@xxxxxxxxxxx>
+ - Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
+
+properties:
+ compatible:
+ enum:
+ - amlogic,a9-gp0-pll
+ - amlogic,a9-hifi0-pll
+ - amlogic,a9-hifi1-pll
+ - amlogic,a9-mclk0-pll
+ - amlogic,a9-mclk1-pll
+
+ reg:
+ maxItems: 1
+
+ '#clock-cells':
+ const: 1
+
+ clocks:
+ items:
+ - description: pll input oscillator gate
+ - description: fixed input clock source for mclk_sel_0
+ - description: u3p2pll input clock source for mclk_sel_0 (optional)

Second clock is also optional. Drop "(optional)" comment, just
confusing.

GP0 has only one parent clock, while MCLK has three.

The second and third parent entries of GP0 are vacant,

so they need to be marked optional.

I will add the optional property for the second clock in the next revision.

+ minItems: 1
+
+ clock-names:
+ items:
+ - const: in0
+ - const: in1
+ - const: in2

Pretty pointless names, drop property.

Ok, I will drop them.

clock-names:
- items:
- - const: in0
- - const: in1
- - const: in2
minItems: 1

+ minItems: 1
+
+required:
+ - compatible
+ - '#clock-cells'
+ - reg
+ - clocks
+ - clock-names
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - amlogic,a9-mclk0-pll
+ - amlogic,a9-mclk1-pll
+
+ then:
+ properties:
+ clocks:
+ maxItems: 3

No, minItems instead. maxItems is already 3, so what is the point of
redefining it?

Ok, I will use minItems instead.

+
+ clock-names:
+ maxItems: 3
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - amlogic,a9-gp0-pll
+ - amlogic,a9-hifi0-pll
+ - amlogic,a9-hifi1-pll
+
+ then:
+ properties:
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ maxItems: 1
+
+additionalProperties: false
+
+examples:
+ - |
+ apb4 {

soc

Ok, I will rename it to soc.

+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ clock-controller@8200 {
+ compatible = "amlogic,a9-gp0-pll";
+ reg = <0x0 0x8200 0x0 0x20>;
+ #clock-cells = <1>;
+ clocks = <&scmi_clk 0>;
+ clock-names = "in0";
+ };
+
+ clock-controller@8330 {
+ compatible = "amlogic,a9-mclk0-pll";
+ reg = <0x0 0x8330 0x0 0x14>;
+ #clock-cells = <1>;
+ clocks = <&scmi_clk 4>,
+ <&scmi_clk 8>;
+ clock-names = "in0", "in1";

One example is enough, you have exactly the same properties.

Ok, I will drop the second clock node.

Best regards,
Krzysztof

Best regards,

Jian