Re: [PATCH 03/10] dt-bindings: clock: Add Amlogic A9 peripherals clock controller

[Jian Hu]

On 5/15/2026 4:10 PM, Krzysztof Kozlowski wrote:
> [ EXTERNAL EMAIL ]
>
> On Mon, May 11, 2026 at 08:47:25PM +0800, Jian Hu wrote:
> > Add the peripherals clock controller dt-bindings for the Amlogic A9
> > SoC family.
> >
> > Signed-off-by: Jian Hu <jian.hu@xxxxxxxxxxx>
> > ---
> >   .../clock/amlogic,a9-peripherals-clkc.yaml         | 150 +++++++++
> >   .../clock/amlogic,a9-peripherals-clkc.h            | 352 +++++++++++++++++++++
> >   2 files changed, 502 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/amlogic,a9-peripherals-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a9-peripherals-clkc.yaml
> > new file mode 100644
> > index 000000000000..97e2c44d8630
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/amlogic,a9-peripherals-clkc.yaml
> > @@ -0,0 +1,150 @@
> > +# 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-peripherals-clkc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Amlogic A9 Series Peripherals Clock Controller
> > +
> > +maintainers:
> > +  - Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> > +  - Jerome Brunet <jbrunet@xxxxxxxxxxxx>
> > +  - Jian Hu <jian.hu@xxxxxxxxxxx>
> > +  - Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
> > +
> > +properties:
> > +  compatible:
> > +    const: amlogic,a9-peripherals-clkc
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  '#clock-cells':
> > +    const: 1
> > +
> > +  clocks:
> > +    minItems: 20
> I don't think so. How they could be optional in silicon? How does
> exactly work from silicon point of view?


These are internal clocks from unimplemented analog blocks, and these 
clocks will be added

in the future. Marking them as optional is indeed incorrect.

Only the last external clock is actually optional.

I will fix it in the next version.

> > +    items:
> > +      - description: input oscillator
> > +      - description: input fclk div 2
> > +      - description: input fclk div 3
> > +      - description: input fclk div 4
> > +      - description: input fclk div 5
> > +      - description: input fclk div 7
> > +      - description: input fclk div 2p5
> > +      - description: input sys clk
> > +      - description: input gp1 pll
> > +      - description: input gp2 pll
> > +      - description: input sys pll div 16
> > +      - description: input cpu clk div 16
> > +      - description: input a78 clk div 16
> > +      - description: input dsu clk div 16
> > +      - description: input rtc clk
> > +      - description: input gp0 pll
> > +      - description: input hifi0 pll
> > +      - description: input hifi1 pll
> > +      - description: input mclk0 pll
> > +      - description: input mclk1 pll
> > +      - description: input video1 pll (optional)
> > +      - description: input video2 pll (optional)
> > +      - description: input hdmi out2 clk (optional)
> > +      - description: input hdmi pixel clk (optional)
> > +      - description: input pixel0 pll (optional)
> > +      - description: input pixel1 pll (optional)
> > +      - description: input usb2 drd clk (optional)
> > +      - description: external input rmii oscillator (optional)
> > +
> > +  clock-names:
> > +    minItems: 20
> > +    items:
> > +      - const: xtal
> > +      - const: fdiv2
> > +      - const: fdiv3
> > +      - const: fdiv4
> > +      - const: fdiv5
> > +      - const: fdiv7
> > +      - const: fdiv2p5
> > +      - const: sys
> > +      - const: gp1
> > +      - const: gp2
> > +      - const: sysplldiv16
> > +      - const: cpudiv16
> > +      - const: a78div16
> > +      - const: dsudiv16
> > +      - const: rtc
> > +      - const: gp0
> > +      - const: hifi0
> > +      - const: hifi1
> > +      - const: mclk0
> > +      - const: mclk1
> > +      - const: vid1
> > +      - const: vid2
> > +      - const: hdmiout2
> > +      - const: hdmipix
> > +      - const: pix0
> > +      - const: pix1
> > +      - const: u2drd
> > +      - const: ext_rmii
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - '#clock-cells'
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    apb4 {
> Same comments as other patches. Do not come with your own style, but
> adjust to mainline. Do you see this anywhere?
>
> git grep apb4 -- Documentation/devicetree/bindings/clock/
>
> So why coming with something COMPLETELY different?
>
> Best regards,
> Krzysztof


Thanks for pointing this out. You are correct that there is no precedent 
for "apb4"

in the mainline clock bindings. I should not have invented a new naming 
scheme here.



I will rename this to use the standard "soc" naming that is consistent 
with all other

similar bindings in the kernel tree.

Furthermore, I will search the kernel to see if it exists when naming.



This will be fixed in the next revision.