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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMuHMdXQMtTuvi+VzEN-OkGNneoxngrcyxffG80gk73GMN8Fpg@mail.gmail.com>
Date: Thu, 8 May 2025 15:55:30 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Claudiu Beznea <claudiu.beznea@...on.dev>
Cc: mturquette@...libre.com, sboyd@...nel.org, robh@...nel.org, 
	krzk+dt@...nel.org, conor+dt@...nel.org, magnus.damm@...il.com, 
	linux-renesas-soc@...r.kernel.org, linux-clk@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH 1/7] clk: renesas: rzg2l-cpg: Skip lookup of clock when
 searching for a sibling

Hi Claudiu,

On Wed, 7 May 2025 at 14:12, Claudiu Beznea <claudiu.beznea@...on.dev> wrote:
> On 05.05.2025 18:52, Geert Uytterhoeven wrote:
> > On Thu, 10 Apr 2025 at 16:06, Claudiu <claudiu.beznea@...on.dev> wrote:
> >> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> >>
> >> 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@...renesas.com>
> >
> > 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.

Given you have a later patch that contains a similar check, postponing
setting priv->clks[id] looks like the better solution to me.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

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