linux-kernel - Re: [PATCH v2 6/9] clk: renesas: rzv2h-cpg: Ignore monitoring CLK

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdXuqYHAv+yyOJxC3kre1vaspuXmTMev0ZBixEiEo+4saQ@mail.gmail.com>
Date: Tue, 15 Apr 2025 16:36:16 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Prabhakar <prabhakar.csengg@...il.com>
Cc: Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Magnus Damm <magnus.damm@...il.com>, linux-renesas-soc@...r.kernel.org, 
	linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, Biju Das <biju.das.jz@...renesas.com>, 
	Fabrizio Castro <fabrizio.castro.jz@...esas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v2 6/9] clk: renesas: rzv2h-cpg: Ignore monitoring CLK_MON
 bits for external clocks

Hi Prabhakar,

Thanks for your patch!

On Mon, 7 Apr 2025 at 18:52, Prabhakar <prabhakar.csengg@...il.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
>
> Ignore CLK_MON bits when turning on/off module clocks that use an external
> clock source.
>
> Introduce the `DEF_MOD_EXTERNAL()` macro for defining module clocks that
> may have an external clock source. Update `rzv2h_cpg_register_mod_clk()`
> to update mon_index.

So I guess you implemented this because the external clock was not
running, and you got into an infinite loop?

This looks rather fragile to me. How do you know when the clock
is actually running, and thus usable?

> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>

> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c
> @@ -569,6 +569,25 @@ static void rzv2h_mod_clock_mstop_disable(struct rzv2h_cpg_priv *priv,
>         spin_unlock_irqrestore(&priv->rmw_lock, flags);
>  }
>
> +static bool rzv2h_mod_clock_is_external(struct rzv2h_cpg_priv *priv,
> +                                       u16 ext_clk_offset,

unsigned int

> +                                       u8 ext_clk_bit,

unsigned int

> +                                       u8 ext_cond)

bool

> +{
> +       u32 value;
> +
> +       if (!ext_clk_offset)
> +               return false;
> +
> +       value = readl(priv->base + ext_clk_offset) & BIT(ext_clk_bit);
> +       value >>= ext_clk_bit;

No need to shift:

    return !!value == ext_cond;

> +
> +       if (value == ext_cond)
> +               return true;
> +
> +       return false;
> +}
> +
>  static int rzv2h_mod_clock_is_enabled(struct clk_hw *hw)
>  {
>         struct mod_clock *clock = to_mod_clock(hw);
> @@ -691,6 +710,11 @@ rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
>         clock->on_index = mod->on_index;
>         clock->on_bit = mod->on_bit;
>         clock->mon_index = mod->mon_index;
> +       /* If clock is coming from external source ignore the monitor bit for it */
> +       if (rzv2h_mod_clock_is_external(priv, mod->external_clk_offset,
> +                                       mod->external_clk_bit,
> +                                       mod->external_cond))

Perhaps just pass "mod" instead of three of its members, to fully
hide the logic inside the helper function?

> +               clock->mon_index = -1;
>         clock->mon_bit = mod->mon_bit;
>         clock->no_pm = mod->no_pm;
>         clock->priv = priv;

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