Re: [PATCH 24/26] clk: amlogic: add composite clock helpers

[Chuan Liu <chuan.liu@amlogic.com>]

Hi Jerome:


On 7/2/2025 11:26 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> Device composite clocks tend to reproduce the usual sel/div/gate
> arrangement.
>
> Add macros to help define simple composite clocks in the system.
>
> The idea is _not_ to replace all instances of mux, div or gate with those
> macros. It is rather to use it for recurring and/or simple composite
> clocks, reducing controller verbosity where it makes sense. This should
> help reviews focus on the tricky parts.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>   drivers/clk/meson/meson-clkc-utils.h | 57 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 57 insertions(+)
>
> diff --git a/drivers/clk/meson/meson-clkc-utils.h b/drivers/clk/meson/meson-clkc-utils.h
> index 95d9f85f7ca22f63a16f8665d6f7a250b21bfdb8..ddadf14b4923781d8807546f35a1ba2e6a8a894a 100644
> --- a/drivers/clk/meson/meson-clkc-utils.h
> +++ b/drivers/clk/meson/meson-clkc-utils.h
> @@ -48,4 +48,61 @@ struct clk_regmap _name = {                                          \
>   #define MESON_PCLK_RO(_name, _reg, _bit, _pdata, _flags)               \
>          __MESON_PCLK(_name, _reg, _bit, &clk_regmap_gate_ro_ops, _pdata, _flags)
>
> +/* Helpers for the usual sel/div/gate composite clocks */
> +#define MESON_COMP_SEL(_prefix, _name, _reg, _shift, _mask, _pdata,    \
> +                      _table, _dflags, _iflags)                        \
> +struct clk_regmap _prefix##_name##_sel = {                             \


Can we apply the same naming style to the '**PCLK' clocks? The SoC prefix in
clock names looks inconsistent and awkward.


> +       .data = &(struct clk_regmap_mux_data) {                         \
> +               .offset = (_reg),                                       \
> +               .mask = (_mask),                                        \
> +               .shift = (_shift),                                      \
> +               .flags = (_dflags),                                     \
> +               .table = (_table),                                      \
> +       },                                                              \
> +       .hw.init = &(struct clk_init_data){                             \
> +               .name = #_name "_sel",                                  \
> +               .ops = &clk_regmap_mux_ops,                             \
> +               .parent_data = _pdata,                                  \
> +               .num_parents = ARRAY_SIZE(_pdata),                      \
> +               .flags = (_iflags),                                     \
> +       },                                                              \
> +}
> +
> +#define MESON_COMP_DIV(_prefix, _name, _reg, _shift, _width,           \
> +                      _dflags, _iflags)                                \
> +struct clk_regmap _prefix##_name##_div = {                             \
> +       .data = &(struct clk_regmap_div_data) {                         \
> +               .offset = (_reg),                                       \
> +               .shift = (_shift),                                      \
> +               .width = (_width),                                      \
> +               .flags = (_dflags),                                     \
> +       },                                                              \
> +       .hw.init = &(struct clk_init_data) {                            \
> +               .name = #_name "_div",                                  \
> +               .ops = &clk_regmap_divider_ops,                         \
> +               .parent_hws = (const struct clk_hw *[]) {               \
> +                       &_prefix##_name##_sel.hw                        \
> +               },                                                      \
> +               .num_parents = 1,                                       \
> +               .flags = (_iflags),                                     \
> +       },                                                              \
> +}
> +
> +#define MESON_COMP_GATE(_prefix, _name, _reg, _bit, _iflags)           \
> +struct clk_regmap _prefix##_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_hws = (const struct clk_hw *[]) {               \
> +                       &_prefix##_name##_div.hw                        \
> +               },                                                      \
> +               .num_parents = 1,                                       \
> +               .flags = (_iflags),                                     \
> +       },                                                              \
> +}
> +
>   #endif
>
> --
> 2.47.2
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic