Hi, Geert,
On 05.05.2025 18:52, Geert Uytterhoeven wrote:
> 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;
This should work as well. I considered the proposed patch generates less
diff. Please let me know if you prefer it addressed as you proposed.
Thank you for your review,
Claudiu
>
> 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
