Re: [PATCH 06/14] clk: renesas: Add support for RZ/T2H family clock

[Thierry Bultel <thierry.bultel.yh@bp.renesas.com>]

On Mon, Feb 10, 2025 at 03:06:29PM +0100, Geert Uytterhoeven wrote:
> Hi Thierry,
>

Hi Geert,

> On Wed, 29 Jan 2025 at 17:52, Thierry Bultel
> <thierry.bultel.yh@bp.renesas.com> wrote:
> > Add the CPG driver for T2H family.
> >
> > Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/drivers/clk/renesas/rzt2h-cpg.c
> > @@ -0,0 +1,549 @@
> 
> > +static void __init
> > +rzt2h_cpg_register_mod_clk(const struct rzt2h_mod_clk *mod,
> > +                          const struct rzt2h_cpg_info *info,
> > +                          struct rzt2h_cpg_priv *priv)
> > +{
> > +       struct mstp_clock *clock = NULL;
> > +       struct device *dev = priv->dev;
> > +       unsigned int id = mod->id;
> > +       struct clk_init_data init;
> > +       struct clk *parent, *clk;
> > +       const char *parent_name;
> > +       unsigned int i;
> > +
> > +       WARN_DEBUG(id < priv->num_core_clks);
> > +       WARN_DEBUG(id >= priv->num_core_clks + priv->num_mod_clks);
> > +       WARN_DEBUG(mod->parent >= priv->num_core_clks + priv->num_mod_clks);
> > +       WARN_DEBUG(PTR_ERR(priv->clks[id]) != -ENOENT);
> > +
> > +       /* Skip NULLified clock */
> > +       if (!mod->name)
> > +               return;
> 
> Copied from rzg2l-cpg.c (which copied from renesas-cpg-mssr.c)?
> Do you need this?
> 
> Given RZ/T2 does not use the "write bit 16 + n when touching bit
> n"-scheme (like RZ/G2L and RZ/V2H), I am wondering if it would be easier
> to use renesas-cpg-mssr.c instead, like R-Car and RZ/A2M are doing?
> 

Because this is the early support for RZ/T2H CPG, the patch
does not show the all differences that the driver will have with
renesas-cpg-mssr.c
Taking an example, renesas-cpg-mssr.c does not seem to 
handle all the clock types (eg 	case CLK_TYPE_MUX) that RZ/T2H
uses. I am just afraid that even if renesas-cpg-mssr.c would 
eventually fit now, significant changes would be required later.
But you have a wider sight on this than me.

> 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