Re: [PATCH v3 2/2] clk: amlogic: Add A9 AO clock controller driver

[Jian Hu <jian.hu@amlogic.com>]

On 6/10/2026 8:30 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On mer. 10 juin 2026 at 16:23, Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@kernel.org> wrote:
>
>> From: Jian Hu <jian.hu@amlogic.com>
>>
>> Add the Always-on clock controller driver for the Amlogic A9 SoC family.
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>>   drivers/clk/meson/Kconfig    |  13 ++
>>   drivers/clk/meson/Makefile   |   1 +
>>   drivers/clk/meson/a9-aoclk.c | 431 +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 445 insertions(+)
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index cf8cf3f9e4ee..b71299898197 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -132,6 +132,19 @@ config COMMON_CLK_A1_PERIPHERALS
>>          device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>>          controller to work.
>>
>> +config COMMON_CLK_A9_AO
>> +     tristate "Amlogic A9 SoC AO clock controller support"
>> +     depends on ARM64 || COMPILE_TEST
>> +     default ARCH_MESON
>> +     select COMMON_CLK_MESON_REGMAP
>> +     select COMMON_CLK_MESON_CLKC_UTILS
>> +     select COMMON_CLK_MESON_DUALDIV
>> +     imply COMMON_CLK_SCMI
>> +     help
>> +       Support for the AO clock controller on Amlogic A311Y3 based
>> +       device, AKA A9.
>> +       Say Y if you want A9 AO clock controller to work.
>> +
>>   config COMMON_CLK_C3_PLL
>>        tristate "Amlogic C3 PLL clock controller"
>>        depends on ARM64
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index c6719694a242..f89d027c282c 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -19,6 +19,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>>   obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>   obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>>   obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>> +obj-$(CONFIG_COMMON_CLK_A9_AO) += a9-aoclk.o
>>   obj-$(CONFIG_COMMON_CLK_C3_PLL) += c3-pll.o
>>   obj-$(CONFIG_COMMON_CLK_C3_PERIPHERALS) += c3-peripherals.o
>>   obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>> diff --git a/drivers/clk/meson/a9-aoclk.c b/drivers/clk/meson/a9-aoclk.c
>> new file mode 100644
>> index 000000000000..dd9fd8d24702
>> --- /dev/null
>> +++ b/drivers/clk/meson/a9-aoclk.c
>> @@ -0,0 +1,431 @@
>> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
>> +/*
>> + * Copyright (C) 2026 Amlogic, Inc. All rights reserved
>> + */
>> +
>> +#include <dt-bindings/clock/amlogic,a9-aoclkc.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
>> +#include "clk-regmap.h"
>> +#include "clk-dualdiv.h"
>> +#include "meson-clkc-utils.h"
>> +
>> +#define AO_OSCIN_CTRL                        0x00
>> +#define AO_SYS_CLK0                  0x04
>> +#define AO_PWM_CLK_A_CTRL            0x1c
>> +#define AO_PWM_CLK_B_CTRL            0x20
>> +#define AO_PWM_CLK_C_CTRL            0x24
>> +#define AO_PWM_CLK_D_CTRL            0x28
>> +#define AO_PWM_CLK_E_CTRL            0x2c
>> +#define AO_PWM_CLK_F_CTRL            0x30
>> +#define AO_PWM_CLK_G_CTRL            0x34
>> +#define AO_CEC_CTRL0                 0x38
>> +#define AO_CEC_CTRL1                 0x3c
>> +#define AO_RTC_BY_OSCIN_CTRL0                0x50
>> +#define AO_RTC_BY_OSCIN_CTRL1                0x54
>> +
>> +#define A9_COMP_SEL(_name, _reg, _shift, _mask, _pdata) \
>> +     MESON_COMP_SEL(a9_ao_, _name, _reg, _shift, _mask, _pdata, NULL, 0, 0)
>> +
>> +#define A9_COMP_DIV(_name, _reg, _shift, _width) \
>> +     MESON_COMP_DIV(a9_ao_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
>> +
>> +#define A9_COMP_GATE(_name, _reg, _bit) \
>> +     MESON_COMP_GATE(a9_ao_, _name, _reg, _bit, CLK_SET_RATE_PARENT)
>> +
>> +static struct clk_regmap a9_ao_xtal_in = {
>> +     .data = &(struct clk_regmap_gate_data){
>> +             .offset = AO_OSCIN_CTRL,
>> +             .bit_idx = 3,
>> +     },
>> +     .hw.init = &(struct clk_init_data) {
>> +             .name = "ao_xtal_in",
>> +             .ops = &clk_regmap_gate_ops,
>> +             .parent_data = &(const struct clk_parent_data) {
>> +                     .fw_name = "xtal",
>> +             },
>> +             .num_parents = 1,
>> +             /*
>> +              * ao_sys can select different clock sources. One possible clock path is:
>> +              *   ao_xtal_in->ao_xtal->ao_sys-> ao sys gate clocks
>> +              *
>> +              * ao_xtal_in is in the parent chain of AO sys gate clocks.
>> +              * Since some downstream clocks are marked CLK_IS_CRITICAL,
>> +              * ao_xtal_in must remain enabled and is therefore marked
>> +              * CLK_IS_CRITICAL as well.
>> +              */
>> +             .flags = CLK_IS_CRITICAL,
> Please allow some time for me to reply before reposting.
> See my answer on v2.
>

Sorry for reposting too quickly.

I'll allow more time for review feedback before sending the next revision.

I've seen your reply on v2 and will drop this flag in the next revision.

>> +     },
>> +};
>> +
>> +static struct clk_regmap a9_ao_xtal = {
>> +     .data = &(struct clk_regmap_mux_data) {
>> +             .offset = AO_OSCIN_CTRL,
>> +             .mask = 0x1,
>> +             .shift = 0,
>> +     },
>> +     /* ext_32k is from external PAD, do not automatically reparent */
>> +     .hw.init = CLK_HW_INIT_PARENTS_DATA("ao_xtal",
>> +                     ((const struct clk_parent_data []) {
>> +                             { .hw = &a9_ao_xtal_in.hw },
>> +                             { .fw_name = "ext_32k" }
>> +                     }), &clk_regmap_mux_ops, CLK_SET_RATE_NO_REPARENT),
> I hope my view on this is clear as well.
> Let me know if it isn't
>

Understood. I will drop all CLK_HW_INIT* macros and revert to explicit
struct clk_init_data initializers for the A9 clock controllers.


[ ... ]

> --
> Jerome


--

Jian