Re: [PATCH 08/10] clk: amlogic: Add A9 PLL clock controller driver

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

On 5/11/2026 11:36 PM, Brian Masney wrote:
> [ EXTERNAL EMAIL ]
>
> Hi Jian,
>
> On Mon, May 11, 2026 at 08:47:30PM +0800, Jian Hu via B4 Relay wrote:
>> From: Jian Hu <jian.hu@amlogic.com>
>>
>> Add the PLL 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-pll.c | 831 +++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 845 insertions(+)
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index cf8cf3f9e4ee..3549e67d6988 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_PLL
>> +     tristate "Amlogic A9 SoC PLL controller support"
>> +     depends on ARM64
> depends on ARM64 || COMPILE_TEST


Ok, I will add COMPILE_TEST in the next version.

>> +     default ARCH_MESON
>> +     select COMMON_CLK_MESON_REGMAP
>> +     select COMMON_CLK_MESON_CLKC_UTILS
>> +     select COMMON_CLK_MESON_PLL
>> +     imply COMMON_CLK_SCMI
>> +     help
>> +       Support for the PLL clock controller on Amlogic A311Y3 based
>> +       device, AKA A9. PLLs are required by most peripheral to operate.
>> +       Say Y if you want A9 PLL 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..77636033061f 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_PLL) += a9-pll.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-pll.c b/drivers/clk/meson/a9-pll.c
>> new file mode 100644
>> index 000000000000..84b591c3afff
>> --- /dev/null
>> +++ b/drivers/clk/meson/a9-pll.c
>> @@ -0,0 +1,831 @@
>> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
>> +/*
>> + * Copyright (C) 2026 Amlogic, Inc. All rights reserved
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
>> +#include <dt-bindings/clock/amlogic,a9-pll-clkc.h>
>> +#include "clk-regmap.h"
>> +#include "clk-pll.h"
>> +#include "meson-clkc-utils.h"
> Sort the headers


Ok , I will place dt-bindings header at the top.

After updated:

#include <dt-bindings/clock/amlogic,a9-pll-clkc.h>
#include <linux/clk-provider.h>
#include <linux/platform_device.h>
#include "clk-regmap.h"
#include "clk-pll.h"
#include "meson-clkc-utils.h"


If I have misunderstood, please correct me.

>> +
>> +#define GP0PLL_CTRL0                 0x00
>> +#define GP0PLL_CTRL1                 0x04
>> +#define GP0PLL_CTRL2                 0x08
>> +#define GP0PLL_CTRL3                 0x0c
>> +#define GP0PLL_CTRL4                 0x10
>> +
>> +/* HIFI0 and HIFI1 share the same IP and register offset layout. */
>> +#define HIFIPLL_CTRL0                        0x00
>> +#define HIFIPLL_CTRL1                        0x04
>> +#define HIFIPLL_CTRL2                        0x08
>> +#define HIFIPLL_CTRL3                        0x0c
>> +#define HIFIPLL_CTRL4                        0x10
>> +
>> +/* MCLK0 and MCLK1 share the same IP and register offset layout. */
>> +#define MCLKPLL_CTRL0                        0x00
>> +#define MCLKPLL_CTRL1                        0x04
>> +#define MCLKPLL_CTRL2                        0x08
>> +#define MCLKPLL_CTRL3                        0x0c
>> +#define MCLKPLL_CTRL4                        0x10
>> +
>> +#define A9_COMP_SEL(_name, _reg, _shift, _mask, _pdata) \
>> +     MESON_COMP_SEL(a9_, _name, _reg, _shift, _mask, _pdata, NULL, 0, 0)
>> +
>> +#define A9_COMP_DIV(_name, _reg, _shift, _width) \
>> +     MESON_COMP_DIV(a9_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
>> +
>> +#define A9_COMP_GATE(_name, _reg, _bit) \
>> +     MESON_COMP_GATE(a9_, _name, _reg, _bit, CLK_SET_RATE_PARENT)
>> +
>> +/*
>> + * Compared with previous SoC PLLs, the A9 PLL input path has an inherent
>> + * 2-divider. The N pre-divider follows the same calculation rule as OD,
>> + * where the pre-divider ratio equals 2^N.
>> + *
>> + * A9 PLL is composed as follows:
>> + *
>> + *                      PLL
>> + *         +---------------------------------+
>> + *         |                                 |
>> + *         |             +--+                |
>> + *  in/2 >>---[ /2^N ]-->|  |      +-----+   |
>> + *         |             |  |------| DCO |----->> out
>> + *         |  +--------->|  |      +--v--+   |
>> + *         |  |          +--+         |      |
>> + *         |  |                       |      |
>> + *         |  +--[ *(M + (F/Fmax) ]<--+      |
>> + *         |                                 |
>> + *         +---------------------------------+
>> + *
>> + * out = in / 2  * (m + frac / frac_max) / 2^n
>> + */
>> +
>> +static struct clk_fixed_factor a9_gp0_in_div2_div = {
>> +     .mult = 1,
>> +     .div = 2,
>> +     .hw.init = &(struct clk_init_data){
>> +             .name = "gp0_in_div2_div",
>> +             .ops = &clk_fixed_factor_ops,
>> +             .parent_data = &(const struct clk_parent_data) {
>> +                     .fw_name = "in0",
>> +             },
>> +             .num_parents = 1,
>> +     },
> You can use CLK_HW_INIT_FW_NAME() for the hw.init here and other places
> below.


Ok, I will use CLK_HW_INIT_FW_NAME instead in the next version.

>> +};
>> +
>> +static struct clk_regmap a9_gp0_in_div2 = {
>> +     .data = &(struct clk_regmap_gate_data) {
>> +             .offset = GP0PLL_CTRL0,
>> +             .bit_idx = 27,
>> +     },
>> +     .hw.init = &(struct clk_init_data) {
>> +             .name = "gp0_in_div2",
>> +             .ops = &clk_regmap_gate_ops,
>> +             .parent_hws = (const struct clk_hw *[]) {
>> +                     &a9_gp0_in_div2_div.hw
>> +             },
>> +             .num_parents = 1,
>> +     },
>> +};
>> +
>> +/* The output frequency range of the A9 PLL_DCO is 1.4 GHz to 2.8 GHz. */
>> +static const struct pll_mult_range a9_pll_mult_range = {
>> +     .min = 117,
>> +     .max = 233,
>> +};
>> +
>> +static const struct reg_sequence a9_gp0_pll_init_regs[] = {
>> +     { .reg = GP0PLL_CTRL0, .def = 0x00010000 },
>> +     { .reg = GP0PLL_CTRL1, .def = 0x11480000 },
>> +     { .reg = GP0PLL_CTRL2, .def = 0x1219b010 },
>> +     { .reg = GP0PLL_CTRL3, .def = 0x00008010 }
>> +};
>> +
>> +static struct clk_regmap a9_gp0_pll_dco = {
>> +     .data = &(struct meson_clk_pll_data) {
>> +             .en = {
>> +                     .reg_off = GP0PLL_CTRL0,
>> +                     .shift   = 28,
>> +                     .width   = 1,
>> +             },
>> +             .m = {
>> +                     .reg_off = GP0PLL_CTRL0,
>> +                     .shift   = 0,
>> +                     .width   = 9,
>> +             },
>> +             .n = {
>> +                     .reg_off = GP0PLL_CTRL0,
>> +                     .shift   = 12,
>> +                     .width   = 3,
>> +             },
>> +             .frac = {
>> +                     .reg_off = GP0PLL_CTRL1,
>> +                     .shift   = 0,
>> +                     .width   = 17,
>> +             },
>> +             .l = {
>> +                     .reg_off = GP0PLL_CTRL0,
>> +                     .shift   = 31,
>> +                     .width   = 1,
>> +             },
>> +             .rst = {
>> +                     .reg_off = GP0PLL_CTRL0,
>> +                     .shift   = 29,
>> +                     .width   = 1,
>> +             },
>> +             .l_detect = {
>> +                     .reg_off = GP0PLL_CTRL0,
>> +                     .shift   = 30,
>> +                     .width   = 1,
>> +             },
>> +             .range = &a9_pll_mult_range,
>> +             .init_regs = a9_gp0_pll_init_regs,
>> +             .init_count = ARRAY_SIZE(a9_gp0_pll_init_regs),
>> +             .flags = CLK_MESON_PLL_RST_ACTIVE_LOW |
>> +                      CLK_MESON_PLL_N_POWER_OF_TWO |
>> +                      CLK_MESON_PLL_L_DETECT_ACTIVE_HIGH,
>> +     },
>> +     .hw.init = &(struct clk_init_data) {
>> +             .name = "gp0_pll_dco",
>> +             .ops = &meson_clk_pll_ops,
>> +             .parent_hws = (const struct clk_hw *[]) {
>> +                     &a9_gp0_in_div2.hw
>> +             },
>> +             .num_parents = 1,
>> +     },
> You can use CLK_HW_INIT_HWS() here and other places below.
>
> Brian
>

Ok, I will use CLK_HW_INIT_HW instead for single parent case.


Best regards,

Jian

[......]

>> 2.47.1
>>
>>