Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Thu, 10 Jun 2021 15:03:48 +0200 | Subject | Re: [PATCH v3 08/11] clk: renesas: Add CPG core wrapper for RZ/G2L SoC |
| |
Hi Prabhakar,
On Wed, Jun 9, 2021 at 5:33 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Add CPG core wrapper for RZ/G2L family. > > Based on a patch in the BSP by Binh Nguyen > <binh.nguyen.jz@renesas.com>. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Thanks for your patch!
> --- a/drivers/clk/renesas/Kconfig > +++ b/drivers/clk/renesas/Kconfig > @@ -182,6 +182,11 @@ config CLK_RCAR_USB2_CLOCK_SEL > help > This is a driver for R-Car USB2 clock selector > > +config CLK_RZG2L > + bool "Renesas RZ/G2L SoC clock support" if COMPILE_TEST && !ARCH_RENESAS
s/SoC/family/?
I think "if COMPILE_TEST", as all other entries are using, is sufficient.
> + depends on ARCH_RENESAS || COMPILE_TEST
I think this can be dropped.
> + select RESET_CONTROLLER > + > # Generic
> --- /dev/null > +++ b/drivers/clk/renesas/renesas-rzg2l-cpg.c
> +static struct clk > +*rzg2l_cpg_clk_src_twocell_get(struct of_phandle_args *clkspec, > + void *data) > +{ > + unsigned int clkidx = clkspec->args[1]; > + struct rzg2l_cpg_priv *priv = data; > + struct device *dev = priv->dev; > + const char *type; > + struct clk *clk; > + > + switch (clkspec->args[0]) { > + case CPG_CORE: > + type = "core"; > + if (clkidx > priv->last_dt_core_clk) { > + dev_err(dev, "Invalid %s clock index %u\n", type, clkidx); > + return ERR_PTR(-EINVAL); > + } > + clk = priv->clks[clkidx]; > + break; > + > + case CPG_MOD: > + type = "module"; > + if (clkidx > priv->num_core_clks + priv->num_mod_clks) {
The range of module clocks in DT specifiers starts at zero, so
if (clkidx > priv->num_mod_clks) {
> + dev_err(dev, "Invalid %s clock index %u\n", type, > + clkidx); > + return ERR_PTR(-EINVAL); > + } > + clk = priv->clks[priv->num_core_clks + clkidx]; > + break; > + > + default: > + dev_err(dev, "Invalid CPG clock type %u\n", clkspec->args[0]); > + return ERR_PTR(-EINVAL); > + } > + > + if (IS_ERR(clk)) > + dev_err(dev, "Cannot get %s clock %u: %ld", type, clkidx, > + PTR_ERR(clk)); > + else > + dev_dbg(dev, "clock (%u, %u) is %pC at %lu Hz\n", > + clkspec->args[0], clkspec->args[1], clk, > + clk_get_rate(clk)); > + return clk; > +}
With the above fixed: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
I can fix these while applying.
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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
| |