Re: [PATCH 1/7] clk: renesas: rzg2l-cpg: Skip lookup of clock when searching for a sibling

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Claudiu,

On Thu, 10 Apr 2025 at 16:06, Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> Since the sibling data is filled after the priv->clks[] array entry is
> populated, the first clock that is probed and has a sibling will
> temporarily behave as its own sibling until its actual sibling is
> populated. To avoid any issues, skip this clock when searching for a
> sibling.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>

Thanks for your patch!

> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -1324,6 +1324,9 @@ static struct mstp_clock
>
>                 hw = __clk_get_hw(priv->clks[priv->num_core_clks + i]);
>                 clk = to_mod_clock(hw);
> +               if (clk == clock)
> +                       continue;
> +
>                 if (clock->off == clk->off && clock->bit == clk->bit)
>                         return clk;
>         }

Why not move the whole block around the call to
rzg2l_mod_clock_get_sibling() up instead?

            ret = devm_clk_hw_register(dev, &clock->hw);
            if (ret) {
                    clk = ERR_PTR(ret);
                    goto fail;
            }

    -       clk = clock->hw.clk;
    -       dev_dbg(dev, "Module clock %pC at %lu Hz\n", clk,
clk_get_rate(clk));
    -       priv->clks[id] = clk;
    -
            if (mod->is_coupled) {
                    struct mstp_clock *sibling;

                    clock->enabled = rzg2l_mod_clock_is_enabled(&clock->hw);
                    sibling = rzg2l_mod_clock_get_sibling(clock, priv);
                    if (sibling) {
                            clock->sibling = sibling;
                            sibling->sibling = clock;
                    }
            }

    +       clk = clock->hw.clk;
    +       dev_dbg(dev, "Module clock %pC at %lu Hz\n", clk,
clk_get_rate(clk));
    +       priv->clks[id] = clk;
    +
            return;

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds