Re: [PATCH v5 5/8] clk: amlogic: Add A5 clock peripherals controller driver

[Chuan Liu]

Hi Jerome,

On 1/14/2026 5:25 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On jeu. 08 janv. 2026 at 14:08, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@xxxxxxxxxx> wrote:
>
> > +static struct clk_regmap a5_rtc_clk = {
> > +     .data = &(struct clk_regmap_mux_data) {
> > +             .offset = RTC_CTRL,
> > +             .mask = 0x3,
> > +             .shift = 0,
> > +     },
> > +     .hw.init = &(struct clk_init_data) {
> > +             .name = "rtc_clk",
> > +             .ops = &clk_regmap_mux_ops,
> > +             .parent_data = a5_rtc_clk_parents,
> > +             .num_parents = ARRAY_SIZE(a5_rtc_clk_parents),
> > +             .flags = CLK_SET_RATE_NO_REPARENT,
> > +     },
> > +};
> > +
> > +#define A5_PCLK(_name, _reg, _bit, _pdata, _flags)                   \
> > +struct clk_regmap a5_##_name = {                                     \
> > +     .data = &(struct clk_regmap_gate_data) {                        \
> > +             .offset = (_reg),                                       \
> > +             .bit_idx = (_bit),                                      \
> > +     },                                                              \
> > +     .hw.init = &(struct clk_init_data) {                            \
> > +             .name = #_name,                                         \
> > +             .ops = &clk_regmap_gate_ops,                            \
> > +             .parent_data = (_pdata),                                \
> > +             .num_parents = 1,                                       \
> > +             .flags = (_flags),                                      \
> > +     },                                                              \
> > +}
>
> I wonder why I bothered reviewing v4 ...

Regarding the comment you made on V4, my understanding is that you were 
just teasing ... In the next revision, I will change this part to use a 
unified macro.

We may also consider adjusting the "MESON_PCLK" macro later by removing 
the SoC prefix from the clock name, so that it is consistent with the 
naming style used by "MESON_COMP_SEL" / "MESON_COMP_DIV".

> > +
> > +static const struct clk_parent_data a5_sys_pclk_parents = { .fw_name = "sysclk" };
> > +
> > +#define A5_SYS_PCLK(_name, _reg, _bit, _flags) \
> > +     A5_PCLK(_name, _reg, _bit, &a5_sys_pclk_parents, _flags)
> > +
> > +static A5_SYS_PCLK(sys_reset_ctrl,   SYS_CLK_EN0_REG0, 1, 0);
> > +static A5_SYS_PCLK(sys_pwr_ctrl,     SYS_CLK_EN0_REG0, 3, 0);
> > +static A5_SYS_PCLK(sys_pad_ctrl,     SYS_CLK_EN0_REG0, 4, 0);
> > +static A5_SYS_PCLK(sys_ctrl,         SYS_CLK_EN0_REG0, 5, 0);
> > +static A5_SYS_PCLK(sys_ts_pll,               SYS_CLK_EN0_REG0, 6, 0);
> > +
>
> [...]
>
> > +
> > +static struct clk_regmap a5_gen = {
> > +     .data = &(struct clk_regmap_gate_data) {
> > +             .offset = GEN_CLK_CTRL,
> > +             .bit_idx = 11,
> > +     },
> > +     .hw.init = &(struct clk_init_data) {
> > +             .name = "gen",
> > +             .ops = &clk_regmap_gate_ops,
> > +             .parent_hws = (const struct clk_hw *[]) {
> > +                     &a5_gen_div.hw
> > +             },
> > +             .num_parents = 1,
> > +             .flags = CLK_SET_RATE_PARENT,
> > +     },
> > +};
> > +
> > +#define A5_COMP_SEL(_name, _reg, _shift, _mask, _pdata, _table) \
> > +     MESON_COMP_SEL(a5_, _name, _reg, _shift, _mask, _pdata, _table, 0, 0)
> > +
> > +#define A5_COMP_DIV(_name, _reg, _shift, _width) \
> > +     MESON_COMP_DIV(a5_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
> > +
> > +#define A5_COMP_GATE(_name, _reg, _bit, _iflags) \
> > +     MESON_COMP_GATE(a5_, _name, _reg, _bit, CLK_SET_RATE_PARENT | (_iflags))
> > +
>
> At the top. like C3 and T7

Except for A5_COMP_SEL, which differs slightly from T7 due to the 
additional "_table" parameter, the other macros are consistent with T7.

I also asked for your feedback on this in V4 and received your 
confirmation. Is there anything here that still needs to be updated?

[...]