linux-kernel - Re: [PATCH] clk: renesas: r8a08g045: Check the source of the CPU PLL settings

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: <CAMuHMdUHCBy+i+P=URcg1U=qHfgHAkAhPQgxWfJQ8AUiNQYqtA@mail.gmail.com>
Date: Fri, 10 Jan 2025 15:54:46 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Claudiu <claudiu.beznea@...on.dev>
Cc: mturquette@...libre.com, sboyd@...nel.org, 
	linux-renesas-soc@...r.kernel.org, linux-clk@...r.kernel.org, 
	linux-kernel@...r.kernel.org, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH] clk: renesas: r8a08g045: Check the source of the CPU PLL settings

Hi Claudiu,

On Fri, Jan 3, 2025 at 5:49 PM Claudiu <claudiu.beznea@...on.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
>
> On the RZ/G3S SoC, the CPU PLL settings can be set and retrieved through
> the CPG_PLL1_CLK1 and CPG_PLL1_CLK2 registers. However, these settings are
> applied only when CPG_PLL1_SETTING.SEL_PLL1 is set to 0. Otherwise, the
> CPU PLL operates at the default frequency of 1.1 GHz. This patch adds
> support to the PLL driver to return the 1.1 GHz frequency when the CPU PLL
> is configured with the default frequency.
>
> Fixes: 01eabef547e6 ("clk: renesas: rzg2l: Add support for RZ/G3S PLL")
> Fixes: de60a3ebe410 ("clk: renesas: Add minimal boot support for RZ/G3S SoC")
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/r9a08g045-cpg.c
> +++ b/drivers/clk/renesas/r9a08g045-cpg.c
> @@ -51,7 +51,7 @@
>  #define G3S_SEL_SDHI2          SEL_PLL_PACK(G3S_CPG_SDHI_DSEL, 8, 2)
>
>  /* PLL 1/4/6 configuration registers macro. */
> -#define G3S_PLL146_CONF(clk1, clk2)    ((clk1) << 22 | (clk2) << 12)
> +#define G3S_PLL146_CONF(clk1, clk2, settings)  ((clk1) << 22 | (clk2) << 12 | (settings))

s/settings/setting/, to better match the actual register name.

>
>  #define DEF_G3S_MUX(_name, _id, _conf, _parent_names, _mux_flags, _clk_flags) \
>         DEF_TYPE(_name, _id, CLK_TYPE_MUX, .conf = (_conf), \

> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c

> @@ -60,6 +61,7 @@
>  #define GET_REG_OFFSET(val)            ((val >> 20) & 0xfff)
>  #define GET_REG_SAMPLL_CLK1(val)       ((val >> 22) & 0xfff)
>  #define GET_REG_SAMPLL_CLK2(val)       ((val >> 12) & 0xfff)
> +#define GET_REG_SAMPLL_SETTINGS(val)   ((val) & 0xfff)

Likewise, s/SETTINGS/SETTING/

>
>  #define CPG_WEN_BIT            BIT(16)
>

> @@ -986,6 +989,10 @@ static unsigned long rzg3s_cpg_pll_clk_recalc_rate(struct clk_hw *hw,
>         if (pll_clk->type != CLK_TYPE_G3S_PLL)
>                 return parent_rate;
>
> +       val = readl(priv->base + GET_REG_SAMPLL_SETTINGS(pll_clk->conf));
> +       if (val & RZG3S_SEL_PLL)
> +               return pll_clk->default_rate;

This code and the G3S_PLL146_CONF() macro are intended for PLL1,
PLL4, and PLL6 (currently the driver instantiates only PLL1), but the
SETTING register exists only for PLL1.  Hence you should check first
if the register offset is valid, e.g. by checking for a non-zero value.

According to the documentation for the CPG_PLL1_SETTING.SEL_PLL1 bit,
the clock config also depends on the OTP_OTPPLL[0-2] pins.

> +
>         val = readl(priv->base + GET_REG_SAMPLL_CLK1(pll_clk->conf));
>
>         pr = 1 << FIELD_GET(RZG3S_DIV_P, val);

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