Re: [PATCH v3 5/6] clk: meson: add MESON_PCLK_V2 for sys gate clock

[Jian Hu]

Resend it because it refused to receive html messages.


On 2025/5/14 15:39, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Fri 09 May 2025 at 07:48, Jian Hu <jian.hu@xxxxxxxxxxx> wrote:
>
> > A new MESON_PCLK_V2 macro is introduced for the sys gate clock. Its parent
> > is an SCMI clock. It belongs to another clock controller, and the parent
> > configuration is different from  that of MESON_PCLK. This avoids new macro
> > definition in the peripheral clock driver.
> >
> > Signed-off-by: Jian Hu <jian.hu@xxxxxxxxxxx>
> > ---
> >   drivers/clk/meson/clk-regmap.h | 24 ++++++++++++++++++++++++
> >   1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/clk/meson/clk-regmap.h b/drivers/clk/meson/clk-regmap.h
> > index e365312da54e..61b8fc2d875f 100644
> > --- a/drivers/clk/meson/clk-regmap.h
> > +++ b/drivers/clk/meson/clk-regmap.h
> This file is not meant for amlogic specific stuff. I know some found
> their way in regardless but that's being fixed


OK,  I found macro MESON_PCLK is used in G12A/MESON8B/GXBB/S4.

New macro is needed for T7 .

> > @@ -134,4 +134,28 @@ struct clk_regmap _name = {                                              \
> >
> >   #define MESON_PCLK_RO(_name, _reg, _bit, _pname)     \
> >        __MESON_PCLK(_name, _reg, _bit, &clk_regmap_gate_ro_ops, _pname)
> > +
> > +#define __MESON_PCLK_V2(_name, _reg, _bit, _ops, _pname)             \
> > +struct clk_regmap _name = {                                          \
> > +     .data = &(struct clk_regmap_gate_data){                         \
> > +             .offset = (_reg),                                       \
> > +             .bit_idx = (_bit),                                      \
> > +     },                                                              \
> > +     .hw.init = &(struct clk_init_data) {                            \
> > +             .name = #_name,                                         \
> > +             .ops = _ops,                                            \
> > +             .parent_data = &(const struct clk_parent_data) {        \
> > +                     .fw_name = #_pname,                             \
> > +             },                                                      \
> > +             .num_parents = 1,                                       \
> > +             .flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED),     \
> > +     },                                                              \
> > +}
> The proliferation of those macros has been going on for far too long,
> add using CLK_IGNORE_UNUSED inside is certainly a mistake I won't
> repeat.
>
> This will be part of more general clean-up that currently depends on a
> this [1] patch to go further. You'll have to be patient.
>
> [1]: https://lore.kernel.org/r/20250417-clk-hw-get-helpers-v1-0-7743e509612a@xxxxxxxxxxxx


OK, CLK_IGNORE_UNUSED is added for all gate clocks.

And  quote your comments in v2.  the T7_CLK_GATE is newly added in T7 
Peripheral driver in v2.

If I understand you correctly.  A common macro should be added for it 
and remove T7_CLK_GATE.


So I add a common macro in clk-regmap.h, and it can be used for 
different SoCs.

MESON_PCLK and MESON_PCLK_V2 difference is a clocks's parent defination.

the sys gate clock parent is in another clock controller. So parent_data 
member is needed for it.

and parent_hw member is for previous SoCs which clock and parent clock 
are in one clock controller.


How should we deal with it.

1.Add the macro in T7 Perpheral driver.

2.Or add a common macro for parent_data,   and where the macro should be 
placed? maybe put it in meson-clkc-utils.h?

> > +
> > +#define MESON_PCLK_V2(_name, _reg, _bit, _pname)     \
> > +     __MESON_PCLK_V2(_name, _reg, _bit, &clk_regmap_gate_ops, _pname)
> > +
> > +#define MESON_PCLK_RO_V2(_name, _reg, _bit, _pname)  \
> > +     __MESON_PCLK_V2(_name, _reg, _bit, &clk_regmap_gate_ro_ops, _pname)
> > +
> >   #endif /* __CLK_REGMAP_H */
> --
> Jerome