Re: [PATCH 26/26] clk: amlogic: c3-peripherals: use helper for basic composite clocks

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

Hi Jerome:

     Nise, The code looks much cleaner now. Thanks!!!


Reviewed-by: Chuan Liu <chuan.liu@amlogic.com>


On 7/2/2025 11:26 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> Use the composite clock helpers to define simple composite clocks of
> the c3-peripherals clock controller.
>
> This reduces the verbosity of the controller code on these very simple
> parts, making maintenance simpler.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>   drivers/clk/meson/c3-peripherals.c | 1029 +++---------------------------------
>   1 file changed, 63 insertions(+), 966 deletions(-)
>
> diff --git a/drivers/clk/meson/c3-peripherals.c b/drivers/clk/meson/c3-peripherals.c
> index fd35f9b7994720d069c5f72142d6064790d40b60..b158756cfee4dd4bad5c0c9576da02d2cb8ee515 100644
> --- a/drivers/clk/meson/c3-peripherals.c
> +++ b/drivers/clk/meson/c3-peripherals.c


[...]


>   static const struct clk_parent_data c3_hcodec_pre_parents[] = {
>          { .fw_name = "fdiv2p5" },
> @@ -1228,99 +755,13 @@ static const struct clk_parent_data c3_hcodec_pre_parents[] = {
>          { .fw_name = "oscin" }
>   };
>
> -static struct clk_regmap c3_hcodec_0_sel = {
> -       .data = &(struct clk_regmap_mux_data) {
> -               .offset = VDEC_CLK_CTRL,
> -               .mask = 0x7,
> -               .shift = 9,
> -       },
> -       .hw.init = &(struct clk_init_data) {
> -               .name = "hcodec_0_sel",
> -               .ops = &clk_regmap_mux_ops,
> -               .parent_data = c3_hcodec_pre_parents,
> -               .num_parents = ARRAY_SIZE(c3_hcodec_pre_parents),
> -       },
> -};
> +static C3_COMP_SEL(hcodec_0,  VDEC_CLK_CTRL, 9, 0x7, c3_hcodec_pre_parents);
> +static C3_COMP_DIV(hcodec_0,  VDEC_CLK_CTRL, 0, 7);
> +static C3_COMP_GATE(hcodec_0,  VDEC_CLK_CTRL, 8);
>
> -static struct clk_regmap c3_hcodec_0_div = {
> -       .data = &(struct clk_regmap_div_data) {
> -               .offset = VDEC_CLK_CTRL,
> -               .shift = 0,
> -               .width = 7,
> -       },
> -       .hw.init = &(struct clk_init_data) {
> -               .name = "hcodec_0_div",
> -               .ops = &clk_regmap_divider_ops,
> -               .parent_hws = (const struct clk_hw *[]) {
> -                       &c3_hcodec_0_sel.hw
> -               },
> -               .num_parents = 1,
> -               .flags = CLK_SET_RATE_PARENT,
> -       },
> -};
> -
> -static struct clk_regmap c3_hcodec_0 = {
> -       .data = &(struct clk_regmap_gate_data) {
> -               .offset = VDEC_CLK_CTRL,
> -               .bit_idx = 8,
> -       },
> -       .hw.init = &(struct clk_init_data) {
> -               .name = "hcodec_0",
> -               .ops = &clk_regmap_gate_ops,
> -               .parent_hws = (const struct clk_hw *[]) {
> -                       &c3_hcodec_0_div.hw
> -               },
> -               .num_parents = 1,
> -               .flags = CLK_SET_RATE_PARENT,
> -       },
> -};
> -
> -static struct clk_regmap c3_hcodec_1_sel = {
> -       .data = &(struct clk_regmap_mux_data) {
> -               .offset = VDEC3_CLK_CTRL,
> -               .mask = 0x7,
> -               .shift = 9,
> -       },
> -       .hw.init = &(struct clk_init_data) {
> -               .name = "hcodec_1_sel",
> -               .ops = &clk_regmap_mux_ops,
> -               .parent_data = c3_hcodec_pre_parents,
> -               .num_parents = ARRAY_SIZE(c3_hcodec_pre_parents),
> -       },
> -};
> -
> -static struct clk_regmap c3_hcodec_1_div = {
> -       .data = &(struct clk_regmap_div_data) {
> -               .offset = VDEC3_CLK_CTRL,
> -               .shift = 0,
> -               .width = 7,
> -       },
> -       .hw.init = &(struct clk_init_data) {
> -               .name = "hcodec_1_div",
> -               .ops = &clk_regmap_divider_ops,
> -               .parent_hws = (const struct clk_hw *[]) {
> -                       &c3_hcodec_1_sel.hw
> -               },
> -               .num_parents = 1,
> -               .flags = CLK_SET_RATE_PARENT,
> -       },
> -};
> -
> -static struct clk_regmap c3_hcodec_1 = {
> -       .data = &(struct clk_regmap_gate_data) {
> -               .offset = VDEC3_CLK_CTRL,
> -               .bit_idx = 8,
> -       },
> -       .hw.init = &(struct clk_init_data) {
> -               .name = "hcodec_1",
> -               .ops = &clk_regmap_gate_ops,
> -               .parent_hws = (const struct clk_hw *[]) {
> -                       &c3_hcodec_1_div.hw
> -               },
> -               .num_parents = 1,
> -               .flags = CLK_SET_RATE_PARENT,
> -       },
> -};
> +static C3_COMP_SEL(hcodec_1, VDEC3_CLK_CTRL, 9, 0x7, c3_hcodec_pre_parents);
> +static C3_COMP_DIV(hcodec_1, VDEC3_CLK_CTRL, 0, 7);
> +static C3_COMP_GATE(hcodec_1, VDEC3_CLK_CTRL, 8);


Note: hcodec_clk is a no-glitch clock. The current driver may fail to set
hcodec_clk properly. A previous patch attempted to fix this by adding a 
flag,
but was abandoned as it introduced other issues.
(https://lore.kernel.org/all/f8c3b6e7-2f5d-493e-8254-2a27623f0d2e@amlogic.com/)

This macro won't be suitable if we revisit the flag approach later.


>
>   static const struct clk_parent_data c3_hcodec_parents[] = {
>          { .hw = &c3_hcodec_0.hw },


[...]


> --
> 2.47.2
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic