2024-05-15 18:53:06

by Dmitry Rokosov

[permalink] [raw]
Subject: [PATCH v3 6/7] dt-bindings: clock: meson: add A1 CPU clock controller bindings

Add the documentation and dt bindings for Amlogic A1 CPU clock
controller.

This controller consists of the general 'cpu_clk' and two main parents:
'cpu fixed clock' and 'syspll'. The 'cpu fixed clock' is an internal
fixed clock, while the 'syspll' serves as an external input from the A1
PLL clock controller.

Signed-off-by: Dmitry Rokosov <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
.../bindings/clock/amlogic,a1-cpu-clkc.yaml | 64 +++++++++++++++++++
.../dt-bindings/clock/amlogic,a1-cpu-clkc.h | 19 ++++++
2 files changed, 83 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
create mode 100644 include/dt-bindings/clock/amlogic,a1-cpu-clkc.h

diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
new file mode 100644
index 000000000000..f4958b315ed4
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/amlogic,a1-cpu-clkc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic A1 CPU Clock Control Unit
+
+maintainers:
+ - Neil Armstrong <[email protected]>
+ - Jerome Brunet <[email protected]>
+ - Dmitry Rokosov <[email protected]>
+
+properties:
+ compatible:
+ const: amlogic,a1-cpu-clkc
+
+ '#clock-cells':
+ const: 1
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: input fixed pll div2
+ - description: input fixed pll div3
+ - description: input sys pll
+ - description: input oscillator (usually at 24MHz)
+
+ clock-names:
+ items:
+ - const: fclk_div2
+ - const: fclk_div3
+ - const: sys_pll
+ - const: xtal
+
+required:
+ - compatible
+ - '#clock-cells'
+ - reg
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/amlogic,a1-pll-clkc.h>
+ apb {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ clock-controller@fd000000 {
+ compatible = "amlogic,a1-cpu-clkc";
+ reg = <0 0xfd000080 0 0x8>;
+ #clock-cells = <1>;
+ clocks = <&clkc_pll CLKID_FCLK_DIV2>,
+ <&clkc_pll CLKID_FCLK_DIV3>,
+ <&clkc_pll CLKID_SYS_PLL>,
+ <&xtal>;
+ clock-names = "fclk_div2", "fclk_div3", "sys_pll", "xtal";
+ };
+ };
diff --git a/include/dt-bindings/clock/amlogic,a1-cpu-clkc.h b/include/dt-bindings/clock/amlogic,a1-cpu-clkc.h
new file mode 100644
index 000000000000..1d321c6eddb7
--- /dev/null
+++ b/include/dt-bindings/clock/amlogic,a1-cpu-clkc.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
+ * Author: Dmitry Rokosov <[email protected]>
+ */
+
+#ifndef __A1_CPU_CLKC_H
+#define __A1_CPU_CLKC_H
+
+#define CLKID_CPU_FSOURCE_SEL0 0
+#define CLKID_CPU_FSOURCE_DIV0 1
+#define CLKID_CPU_FSEL0 2
+#define CLKID_CPU_FSOURCE_SEL1 3
+#define CLKID_CPU_FSOURCE_DIV1 4
+#define CLKID_CPU_FSEL1 5
+#define CLKID_CPU_FCLK 6
+#define CLKID_CPU_CLK 7
+
+#endif /* __A1_CPU_CLKC_H */
--
2.43.0



2024-06-10 10:04:22

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] dt-bindings: clock: meson: add A1 CPU clock controller bindings

On Wed 15 May 2024 at 21:47, Dmitry Rokosov <[email protected]> wrote:

> Add the documentation and dt bindings for Amlogic A1 CPU clock
> controller.
>
> This controller consists of the general 'cpu_clk' and two main parents:
> 'cpu fixed clock' and 'syspll'. The 'cpu fixed clock' is an internal
> fixed clock, while the 'syspll' serves as an external input from the A1
> PLL clock controller.
>
> Signed-off-by: Dmitry Rokosov <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> ---
> .../bindings/clock/amlogic,a1-cpu-clkc.yaml | 64 +++++++++++++++++++
> .../dt-bindings/clock/amlogic,a1-cpu-clkc.h | 19 ++++++
> 2 files changed, 83 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> create mode 100644 include/dt-bindings/clock/amlogic,a1-cpu-clkc.h
>
> diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> new file mode 100644
> index 000000000000..f4958b315ed4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/amlogic,a1-cpu-clkc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Amlogic A1 CPU Clock Control Unit
> +
> +maintainers:
> + - Neil Armstrong <[email protected]>
> + - Jerome Brunet <[email protected]>
> + - Dmitry Rokosov <[email protected]>
> +
> +properties:
> + compatible:
> + const: amlogic,a1-cpu-clkc
> +
> + '#clock-cells':
> + const: 1
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: input fixed pll div2
> + - description: input fixed pll div3
> + - description: input sys pll
> + - description: input oscillator (usually at 24MHz)

According to the documentation, fdiv5 is also an input of the CPU clock
tree.

That is typically the kind of things we'd prefer to get right from the
beginning to avoid modifying the bindings later.

> +
> + clock-names:
> + items:
> + - const: fclk_div2
> + - const: fclk_div3
> + - const: sys_pll
> + - const: xtal
> +
> +required:
> + - compatible
> + - '#clock-cells'
> + - reg
> + - clocks
> + - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/amlogic,a1-pll-clkc.h>
> + apb {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + clock-controller@fd000000 {
> + compatible = "amlogic,a1-cpu-clkc";
> + reg = <0 0xfd000080 0 0x8>;

If reg is <0 0xfd000080 0 0x8> then node name should be clock-controller@fd000080

> + #clock-cells = <1>;
> + clocks = <&clkc_pll CLKID_FCLK_DIV2>,
> + <&clkc_pll CLKID_FCLK_DIV3>,
> + <&clkc_pll CLKID_SYS_PLL>,
> + <&xtal>;
> + clock-names = "fclk_div2", "fclk_div3", "sys_pll", "xtal";
> + };
> + };
> diff --git a/include/dt-bindings/clock/amlogic,a1-cpu-clkc.h b/include/dt-bindings/clock/amlogic,a1-cpu-clkc.h
> new file mode 100644
> index 000000000000..1d321c6eddb7
> --- /dev/null
> +++ b/include/dt-bindings/clock/amlogic,a1-cpu-clkc.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
> + * Author: Dmitry Rokosov <[email protected]>
> + */
> +
> +#ifndef __A1_CPU_CLKC_H
> +#define __A1_CPU_CLKC_H
> +
> +#define CLKID_CPU_FSOURCE_SEL0 0
> +#define CLKID_CPU_FSOURCE_DIV0 1
> +#define CLKID_CPU_FSEL0 2
> +#define CLKID_CPU_FSOURCE_SEL1 3
> +#define CLKID_CPU_FSOURCE_DIV1 4
> +#define CLKID_CPU_FSEL1 5
> +#define CLKID_CPU_FCLK 6
> +#define CLKID_CPU_CLK 7
> +
> +#endif /* __A1_CPU_CLKC_H */

--
Jerome

2024-06-10 11:18:49

by Dmitry Rokosov

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] dt-bindings: clock: meson: add A1 CPU clock controller bindings

Hello Jerome,

Thank you for the review!

On Mon, Jun 10, 2024 at 12:04:09PM +0200, Jerome Brunet wrote:
> On Wed 15 May 2024 at 21:47, Dmitry Rokosov <[email protected]> wrote:
>
> > Add the documentation and dt bindings for Amlogic A1 CPU clock
> > controller.
> >
> > This controller consists of the general 'cpu_clk' and two main parents:
> > 'cpu fixed clock' and 'syspll'. The 'cpu fixed clock' is an internal
> > fixed clock, while the 'syspll' serves as an external input from the A1
> > PLL clock controller.
> >
> > Signed-off-by: Dmitry Rokosov <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
> > ---
> > .../bindings/clock/amlogic,a1-cpu-clkc.yaml | 64 +++++++++++++++++++
> > .../dt-bindings/clock/amlogic,a1-cpu-clkc.h | 19 ++++++
> > 2 files changed, 83 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> > create mode 100644 include/dt-bindings/clock/amlogic,a1-cpu-clkc.h
> >
> > diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> > new file mode 100644
> > index 000000000000..f4958b315ed4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> > @@ -0,0 +1,64 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/amlogic,a1-cpu-clkc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Amlogic A1 CPU Clock Control Unit
> > +
> > +maintainers:
> > + - Neil Armstrong <[email protected]>
> > + - Jerome Brunet <[email protected]>
> > + - Dmitry Rokosov <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + const: amlogic,a1-cpu-clkc
> > +
> > + '#clock-cells':
> > + const: 1
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: input fixed pll div2
> > + - description: input fixed pll div3
> > + - description: input sys pll
> > + - description: input oscillator (usually at 24MHz)
>
> According to the documentation, fdiv5 is also an input of the CPU clock
> tree.
>
> That is typically the kind of things we'd prefer to get right from the
> beginning to avoid modifying the bindings later.
>

Could you please share which documentation you are referencing? I have
the A113L documentation, and there is no mention of the CPU clock IP.
I retrieved below register map from the vendor's custom driver:

===
CPUCTRL_CLK_CTRL0

bits 1:0 - cpu_fsource_sel0
0 - xtal
1 - fclk_div2
2 - fclk_div3

bit 2 - cpu_fsel0
0 - cpu_fsource_sel0
1 - cpu_fsource_div0

bit 3 - UNKNONWN

bits 9:4 - cpu_fsource_div0
Divider value

bit 10 - cpu_fclk
0 - cpu_fsel0
1 - cpu_fsel1

bit 11 - cpu_clk
0 - cpu_fclk
1 - sys_pll

bits 15:12 - UNKNONWN

bits 17:16 - cpu_fsource_sel1
0 - xtal
1 - fclk_div2
2 - fclk_div3

bit 18 - cpu_fsel1
0 - cpu_fsource_sel1
1 - cpu_fsource_div1

bit 19 - UNKNONWN

bits 25:20 - cpu_fsource_div1
Divider value

bits 31:26 - UNKNONWN
===

As you can see it doesn't have any other inputs except fclk_div2,
fclk_div3, sys_pll and xtal.

> > +
> > + clock-names:
> > + items:
> > + - const: fclk_div2
> > + - const: fclk_div3
> > + - const: sys_pll
> > + - const: xtal
> > +
> > +required:
> > + - compatible
> > + - '#clock-cells'
> > + - reg
> > + - clocks
> > + - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/amlogic,a1-pll-clkc.h>
> > + apb {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + clock-controller@fd000000 {
> > + compatible = "amlogic,a1-cpu-clkc";
> > + reg = <0 0xfd000080 0 0x8>;
>
> If reg is <0 0xfd000080 0 0x8> then node name should be clock-controller@fd000080
>

Okay, I will fix that example in the next version.

[...]

--
Thank you,
Dmitry

2024-06-10 11:47:36

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] dt-bindings: clock: meson: add A1 CPU clock controller bindings

On Mon 10 Jun 2024 at 14:18, Dmitry Rokosov <[email protected]> wrote:

> Hello Jerome,
>
> Thank you for the review!
>
> On Mon, Jun 10, 2024 at 12:04:09PM +0200, Jerome Brunet wrote:
>> On Wed 15 May 2024 at 21:47, Dmitry Rokosov <[email protected]> wrote:
>>
>> > Add the documentation and dt bindings for Amlogic A1 CPU clock
>> > controller.
>> >
>> > This controller consists of the general 'cpu_clk' and two main parents:
>> > 'cpu fixed clock' and 'syspll'. The 'cpu fixed clock' is an internal
>> > fixed clock, while the 'syspll' serves as an external input from the A1
>> > PLL clock controller.
>> >
>> > Signed-off-by: Dmitry Rokosov <[email protected]>
>> > Reviewed-by: Rob Herring <[email protected]>
>> > ---
>> > .../bindings/clock/amlogic,a1-cpu-clkc.yaml | 64 +++++++++++++++++++
>> > .../dt-bindings/clock/amlogic,a1-cpu-clkc.h | 19 ++++++
>> > 2 files changed, 83 insertions(+)
>> > create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
>> > create mode 100644 include/dt-bindings/clock/amlogic,a1-cpu-clkc.h
>> >
>> > diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
>> > new file mode 100644
>> > index 000000000000..f4958b315ed4
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
>> > @@ -0,0 +1,64 @@
>> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> > +%YAML 1.2
>> > +---
>> > +$id: http://devicetree.org/schemas/clock/amlogic,a1-cpu-clkc.yaml#
>> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> > +
>> > +title: Amlogic A1 CPU Clock Control Unit
>> > +
>> > +maintainers:
>> > + - Neil Armstrong <[email protected]>
>> > + - Jerome Brunet <[email protected]>
>> > + - Dmitry Rokosov <[email protected]>
>> > +
>> > +properties:
>> > + compatible:
>> > + const: amlogic,a1-cpu-clkc
>> > +
>> > + '#clock-cells':
>> > + const: 1
>> > +
>> > + reg:
>> > + maxItems: 1
>> > +
>> > + clocks:
>> > + items:
>> > + - description: input fixed pll div2
>> > + - description: input fixed pll div3
>> > + - description: input sys pll
>> > + - description: input oscillator (usually at 24MHz)
>>
>> According to the documentation, fdiv5 is also an input of the CPU clock
>> tree.
>>
>> That is typically the kind of things we'd prefer to get right from the
>> beginning to avoid modifying the bindings later.
>>
>
> Could you please share which documentation you are referencing? I have
> the A113L documentation, and there is no mention of the CPU clock IP.

You should get in touch with Amlogic.

> I retrieved below register map from the vendor's custom driver:
>
> ===
> CPUCTRL_CLK_CTRL0
>
> bits 1:0 - cpu_fsource_sel0
> 0 - xtal
> 1 - fclk_div2
> 2 - fclk_div3
>
> bit 2 - cpu_fsel0
> 0 - cpu_fsource_sel0
> 1 - cpu_fsource_div0
>
> bit 3 - UNKNONWN
>
> bits 9:4 - cpu_fsource_div0
> Divider value
>
> bit 10 - cpu_fclk
> 0 - cpu_fsel0
> 1 - cpu_fsel1
>
> bit 11 - cpu_clk
> 0 - cpu_fclk
> 1 - sys_pll
>
> bits 15:12 - UNKNONWN
>
> bits 17:16 - cpu_fsource_sel1
> 0 - xtal
> 1 - fclk_div2
> 2 - fclk_div3
>
> bit 18 - cpu_fsel1
> 0 - cpu_fsource_sel1
> 1 - cpu_fsource_div1
>
> bit 19 - UNKNONWN
>
> bits 25:20 - cpu_fsource_div1
> Divider value
>
> bits 31:26 - UNKNONWN
> ===
>
> As you can see it doesn't have any other inputs except fclk_div2,
> fclk_div3, sys_pll and xtal.

You might not know what to do with it yet, still it is part of the
documentation and should be part of the bindings too

>
>> > +
>> > + clock-names:
>> > + items:
>> > + - const: fclk_div2
>> > + - const: fclk_div3
>> > + - const: sys_pll
>> > + - const: xtal
>> > +
>> > +required:
>> > + - compatible
>> > + - '#clock-cells'
>> > + - reg
>> > + - clocks
>> > + - clock-names
>> > +
>> > +additionalProperties: false
>> > +
>> > +examples:
>> > + - |
>> > + #include <dt-bindings/clock/amlogic,a1-pll-clkc.h>
>> > + apb {
>> > + #address-cells = <2>;
>> > + #size-cells = <2>;
>> > +
>> > + clock-controller@fd000000 {
>> > + compatible = "amlogic,a1-cpu-clkc";
>> > + reg = <0 0xfd000080 0 0x8>;
>>
>> If reg is <0 0xfd000080 0 0x8> then node name should be clock-controller@fd000080
>>
>
> Okay, I will fix that example in the next version.
>
> [...]

--
Jerome

2024-06-10 12:49:00

by Dmitry Rokosov

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] dt-bindings: clock: meson: add A1 CPU clock controller bindings

On Mon, Jun 10, 2024 at 01:47:06PM +0200, Jerome Brunet wrote:
> On Mon 10 Jun 2024 at 14:18, Dmitry Rokosov <[email protected]> wrote:
>
> > Hello Jerome,
> >
> > Thank you for the review!
> >
> > On Mon, Jun 10, 2024 at 12:04:09PM +0200, Jerome Brunet wrote:
> >> On Wed 15 May 2024 at 21:47, Dmitry Rokosov <[email protected]> wrote:
> >>
> >> > Add the documentation and dt bindings for Amlogic A1 CPU clock
> >> > controller.
> >> >
> >> > This controller consists of the general 'cpu_clk' and two main parents:
> >> > 'cpu fixed clock' and 'syspll'. The 'cpu fixed clock' is an internal
> >> > fixed clock, while the 'syspll' serves as an external input from the A1
> >> > PLL clock controller.
> >> >
> >> > Signed-off-by: Dmitry Rokosov <[email protected]>
> >> > Reviewed-by: Rob Herring <[email protected]>
> >> > ---
> >> > .../bindings/clock/amlogic,a1-cpu-clkc.yaml | 64 +++++++++++++++++++
> >> > .../dt-bindings/clock/amlogic,a1-cpu-clkc.h | 19 ++++++
> >> > 2 files changed, 83 insertions(+)
> >> > create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> >> > create mode 100644 include/dt-bindings/clock/amlogic,a1-cpu-clkc.h
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> >> > new file mode 100644
> >> > index 000000000000..f4958b315ed4
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> >> > @@ -0,0 +1,64 @@
> >> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> >> > +%YAML 1.2
> >> > +---
> >> > +$id: http://devicetree.org/schemas/clock/amlogic,a1-cpu-clkc.yaml#
> >> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> > +
> >> > +title: Amlogic A1 CPU Clock Control Unit
> >> > +
> >> > +maintainers:
> >> > + - Neil Armstrong <[email protected]>
> >> > + - Jerome Brunet <[email protected]>
> >> > + - Dmitry Rokosov <[email protected]>
> >> > +
> >> > +properties:
> >> > + compatible:
> >> > + const: amlogic,a1-cpu-clkc
> >> > +
> >> > + '#clock-cells':
> >> > + const: 1
> >> > +
> >> > + reg:
> >> > + maxItems: 1
> >> > +
> >> > + clocks:
> >> > + items:
> >> > + - description: input fixed pll div2
> >> > + - description: input fixed pll div3
> >> > + - description: input sys pll
> >> > + - description: input oscillator (usually at 24MHz)
> >>
> >> According to the documentation, fdiv5 is also an input of the CPU clock
> >> tree.
> >>
> >> That is typically the kind of things we'd prefer to get right from the
> >> beginning to avoid modifying the bindings later.
> >>
> >
> > Could you please share which documentation you are referencing? I have
> > the A113L documentation, and there is no mention of the CPU clock IP.
>
> You should get in touch with Amlogic.
>

Okay, I will double check with Amlogic and back with accurate
information.

> > I retrieved below register map from the vendor's custom driver:
> >
> > ===
> > CPUCTRL_CLK_CTRL0
> >
> > bits 1:0 - cpu_fsource_sel0
> > 0 - xtal
> > 1 - fclk_div2
> > 2 - fclk_div3
> >
> > bit 2 - cpu_fsel0
> > 0 - cpu_fsource_sel0
> > 1 - cpu_fsource_div0
> >
> > bit 3 - UNKNONWN
> >
> > bits 9:4 - cpu_fsource_div0
> > Divider value
> >
> > bit 10 - cpu_fclk
> > 0 - cpu_fsel0
> > 1 - cpu_fsel1
> >
> > bit 11 - cpu_clk
> > 0 - cpu_fclk
> > 1 - sys_pll
> >
> > bits 15:12 - UNKNONWN
> >
> > bits 17:16 - cpu_fsource_sel1
> > 0 - xtal
> > 1 - fclk_div2
> > 2 - fclk_div3
> >
> > bit 18 - cpu_fsel1
> > 0 - cpu_fsource_sel1
> > 1 - cpu_fsource_div1
> >
> > bit 19 - UNKNONWN
> >
> > bits 25:20 - cpu_fsource_div1
> > Divider value
> >
> > bits 31:26 - UNKNONWN
> > ===
> >
> > As you can see it doesn't have any other inputs except fclk_div2,
> > fclk_div3, sys_pll and xtal.
>
> You might not know what to do with it yet, still it is part of the
> documentation and should be part of the bindings too
>
> >
> >> > +
> >> > + clock-names:
> >> > + items:
> >> > + - const: fclk_div2
> >> > + - const: fclk_div3
> >> > + - const: sys_pll
> >> > + - const: xtal
> >> > +
> >> > +required:
> >> > + - compatible
> >> > + - '#clock-cells'
> >> > + - reg
> >> > + - clocks
> >> > + - clock-names
> >> > +
> >> > +additionalProperties: false
> >> > +
> >> > +examples:
> >> > + - |
> >> > + #include <dt-bindings/clock/amlogic,a1-pll-clkc.h>
> >> > + apb {
> >> > + #address-cells = <2>;
> >> > + #size-cells = <2>;
> >> > +
> >> > + clock-controller@fd000000 {
> >> > + compatible = "amlogic,a1-cpu-clkc";
> >> > + reg = <0 0xfd000080 0 0x8>;
> >>
> >> If reg is <0 0xfd000080 0 0x8> then node name should be clock-controller@fd000080
> >>
> >
> > Okay, I will fix that example in the next version.
> >
> > [...]
>
> --
> Jerome

--
Thank you,
Dmitry

2024-06-13 09:04:45

by Dmitry Rokosov

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] dt-bindings: clock: meson: add A1 CPU clock controller bindings

On Mon, Jun 10, 2024 at 03:48:42PM +0300, Dmitry Rokosov wrote:
> On Mon, Jun 10, 2024 at 01:47:06PM +0200, Jerome Brunet wrote:
> > On Mon 10 Jun 2024 at 14:18, Dmitry Rokosov <[email protected]> wrote:
> >
> > > Hello Jerome,
> > >
> > > Thank you for the review!
> > >
> > > On Mon, Jun 10, 2024 at 12:04:09PM +0200, Jerome Brunet wrote:
> > >> On Wed 15 May 2024 at 21:47, Dmitry Rokosov <[email protected]> wrote:
> > >>
> > >> > Add the documentation and dt bindings for Amlogic A1 CPU clock
> > >> > controller.
> > >> >
> > >> > This controller consists of the general 'cpu_clk' and two main parents:
> > >> > 'cpu fixed clock' and 'syspll'. The 'cpu fixed clock' is an internal
> > >> > fixed clock, while the 'syspll' serves as an external input from the A1
> > >> > PLL clock controller.
> > >> >
> > >> > Signed-off-by: Dmitry Rokosov <[email protected]>
> > >> > Reviewed-by: Rob Herring <[email protected]>
> > >> > ---
> > >> > .../bindings/clock/amlogic,a1-cpu-clkc.yaml | 64 +++++++++++++++++++
> > >> > .../dt-bindings/clock/amlogic,a1-cpu-clkc.h | 19 ++++++
> > >> > 2 files changed, 83 insertions(+)
> > >> > create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> > >> > create mode 100644 include/dt-bindings/clock/amlogic,a1-cpu-clkc.h
> > >> >
> > >> > diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> > >> > new file mode 100644
> > >> > index 000000000000..f4958b315ed4
> > >> > --- /dev/null
> > >> > +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
> > >> > @@ -0,0 +1,64 @@
> > >> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > >> > +%YAML 1.2
> > >> > +---
> > >> > +$id: http://devicetree.org/schemas/clock/amlogic,a1-cpu-clkc.yaml#
> > >> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > >> > +
> > >> > +title: Amlogic A1 CPU Clock Control Unit
> > >> > +
> > >> > +maintainers:
> > >> > + - Neil Armstrong <[email protected]>
> > >> > + - Jerome Brunet <[email protected]>
> > >> > + - Dmitry Rokosov <[email protected]>
> > >> > +
> > >> > +properties:
> > >> > + compatible:
> > >> > + const: amlogic,a1-cpu-clkc
> > >> > +
> > >> > + '#clock-cells':
> > >> > + const: 1
> > >> > +
> > >> > + reg:
> > >> > + maxItems: 1
> > >> > +
> > >> > + clocks:
> > >> > + items:
> > >> > + - description: input fixed pll div2
> > >> > + - description: input fixed pll div3
> > >> > + - description: input sys pll
> > >> > + - description: input oscillator (usually at 24MHz)
> > >>
> > >> According to the documentation, fdiv5 is also an input of the CPU clock
> > >> tree.
> > >>
> > >> That is typically the kind of things we'd prefer to get right from the
> > >> beginning to avoid modifying the bindings later.
> > >>
> > >
> > > Could you please share which documentation you are referencing? I have
> > > the A113L documentation, and there is no mention of the CPU clock IP.
> >
> > You should get in touch with Amlogic.
> >
>
> Okay, I will double check with Amlogic and back with accurate
> information.
>

According to a statement from an Amlogic FAE, there is an error in the
datasheet's CPU clock controller figure. The FAE clarified the
following:

"""
The d5/d7 clock source inside the A1 chip is not supplied to CPU_CLK and
is instead used by other peripherals. Therefore, you can connect a fixed
frequency divider (div5/div7) to peripherals in A1. The CPU control only
supports fclk_div2/div3.
"""

[...]

--
Thank you,
Dmitry