linux-kernel - Re: [PATCH v3 02/25] clk: versaclock3: Prepare for the addition of 5L35023 device

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: <CAMuHMdVv+2tEQ5hf+sbihbJMo3+=8kJaWy0YNU_spxQnmWF-bA@mail.gmail.com>
Date: Mon, 9 Dec 2024 11:57:39 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Claudiu <claudiu.beznea@...on.dev>
Cc: mturquette@...libre.com, sboyd@...nel.org, robh@...nel.org, 
	krzk+dt@...nel.org, conor+dt@...nel.org, biju.das.jz@...renesas.com, 
	prabhakar.mahadev-lad.rj@...renesas.com, lgirdwood@...il.com, 
	broonie@...nel.org, magnus.damm@...il.com, linus.walleij@...aro.org, 
	perex@...ex.cz, tiwai@...e.com, p.zabel@...gutronix.de, 
	linux-renesas-soc@...r.kernel.org, linux-clk@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-sound@...r.kernel.org, linux-gpio@...r.kernel.org, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH v3 02/25] clk: versaclock3: Prepare for the addition of
 5L35023 device

Hi Claudiu,

On Wed, Nov 13, 2024 at 2:35 PM Claudiu <claudiu.beznea@...on.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
>
> The 5P35023 and 5L35035 Versa 3 clock generator variants are different but
> the versaclock3 driver could be used with small adjustments. The features
> that are implemented in driver and differs b/w variants are the PLL2 Fvco
> and clock sel bit for SE2 clock. Adjust the driver to prepare for the
> addition of 5L35023 device.
>
> Reviewed-by: Biju Das <biju.das.jz@...renesas.com>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>

Thanks for your patch!

> --- a/drivers/clk/clk-versaclock3.c
> +++ b/drivers/clk/clk-versaclock3.c
> @@ -166,12 +167,17 @@ struct vc3_div_data {
>  struct vc3_hw_data {
>         struct clk_hw hw;
>         struct regmap *regmap;
> -       const void *data;
> +       void *data;
>
>         u32 div_int;
>         u32 div_frc;
>  };

> @@ -698,8 +706,6 @@ static struct vc3_hw_data clk_pll[] = {
>                         .num = VC3_PLL2,
>                         .int_div_msb_offs = VC3_PLL2_FB_INT_DIV_MSB,
>                         .int_div_lsb_offs = VC3_PLL2_FB_INT_DIV_LSB,
> -                       .vco_min = VC3_PLL2_VCO_MIN,
> -                       .vco_max = VC3_PLL2_VCO_MAX
>                 },
>                 .hw.init = &(struct clk_init_data) {
>                         .name = "pll2",

> @@ -1029,9 +1037,16 @@ static int vc3_probe(struct i2c_client *client)
>                                              clk_pfd[i].hw.init->name);
>         }
>
> +       data = i2c_get_match_data(client);
> +
>         /* Register pll's */
>         for (i = 0; i < ARRAY_SIZE(clk_pll); i++) {
>                 clk_pll[i].regmap = regmap;
> +               if (i == VC3_PLL2) {
> +                       struct vc3_pll_data *pll_data = clk_pll[i].data;
> +
> +                       pll_data->vco = data->pll2_vco;

You cannot modify the global clk_pll[] data, as it is shared when
there are multiple instances.
Oops, the driver already did this before, as the .regmap, .div_int,
and .div_frc fields are modified during operation.

> +               }
>                 ret = devm_clk_hw_register(dev, &clk_pll[i].hw);
>                 if (ret)
>                         return dev_err_probe(dev, ret, "%s failed\n",
> @@ -1059,6 +1074,11 @@ static int vc3_probe(struct i2c_client *client)
>         /* Register clk muxes */
>         for (i = 0; i < ARRAY_SIZE(clk_mux); i++) {
>                 clk_mux[i].regmap = regmap;
> +               if (i == VC3_SE2_MUX) {
> +                       struct vc3_clk_data *clk_data = clk_mux[i].data;
> +
> +                       clk_data->bitmsk = data->se2_clk_sel_msk;
> +               }

Likewise.

>                 ret = devm_clk_hw_register(dev, &clk_mux[i].hw);
>                 if (ret)
>                         return dev_err_probe(dev, ret, "%s failed\n",

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