Re: [RESEND v7 15/37] clk: renesas: Add SH7750/7751 CPG Driver

[Geert Uytterhoeven]

Hi Sato-san,

On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
<ysato@xxxxxxxxxxxxxxxxxxxx> wrote:
> Renesas SH7750 and SH7751 series CPG driver.
> This driver supported frequency control and clock gating.
>
> Signed-off-by: Yoshinori Sato <ysato@xxxxxxxxxxxxxxxxxxxx>

Thanks for the update!

As you plan to send a v8 soon, I'm sending you a comment from the
(incomplete) review I started a while ago...

> --- /dev/null
> +++ b/drivers/clk/renesas/clk-sh7750.c

> +static int register_pll(struct device_node *node, struct cpg_priv *cpg)
> +{
> +       const char *clk_name = node->name;
> +       const char *parent_name;
> +       struct clk_init_data init = {
> +               .name = PLLOUT,
> +               .ops = &pll_ops,
> +               .flags = 0,
> +               .num_parents = 1,
> +       };
> +       int ret;
> +
> +       parent_name = of_clk_get_parent_name(node, 0);
> +       init.parent_names = &parent_name;
> +       cpg->hw.init = &init;
> +
> +       ret = of_clk_hw_register(node, &cpg->hw);
> +       if (ret < 0)
> +               pr_err("%pOF: failed to add provider %s (%d)\n",

I think you retained the wrong error message?
"%s: failed to register %s pll clock (%d)\n" sounds more suitable to me.

> +                      node, clk_name, ret);
> +       return ret;
> +}

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

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