Re: [PATCH RFC v2 2/9] clk: renesas: r9a09g077: Add SYSC regmap support

Hi Geert,

Thank you for the review.

On Mon, Jul 13, 2026 at 4:54 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Thu, 2 Jul 2026 at 14:31, Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Add System Controller (SYSC) regmap support for the RZ/T2H SoC.
> >
> > The RZ/T2H SoC features two System Controller (SYSC) blocks that handle
> > low-power management and access control. These blocks are defined as child
> > nodes within the Clock Pulse Generator (CPG) address space in the device
> > tree. Implement support for creating regmaps for these SYSC blocks.
> >
> > Register the regmap using of_syscon_register_regmap() against the CPG
> > device node so that consumers can access SYSC registers via the existing
> > syscon phandle.
> >
> > Add a sysc_init() callback to the CPG/MSSR core to allow SoC-specific
> > initialization of the SYSC regmap.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > v1->v2:
> > - Made use of for_each_child_of_node_scoped
> > - Moved sysc_init() to the end of the probe function
>
> Thanks for the update!
>
> > --- a/drivers/clk/renesas/r9a09g077-cpg.c
> > +++ b/drivers/clk/renesas/r9a09g077-cpg.c
>
> > +static bool rzt2h_writeable_readable_sysc0(struct device *dev, unsigned int reg)
> > +{
> > +       switch (reg) {
> > +       case 0x0000 ... 0x0008:
> > +       case 0x1000 ... 0x1164:
> > +       case 0x2000 ... 0x2024:
> > +       case 0x2030 ... 0x2054:
> > +       case 0x2060:
> > +       case 0x3000 ... 0x300C:
> > +       case 0x3100 ... 0x310C:
> > +       case 0x4100:
> > +       case 0x4200:
> > +               return true;
> > +
> > +       default:
> > +               return false;
> > +       }
> > +}
> > +
> > +static bool rzt2h_writeable_readable_sysc1(struct device *dev, unsigned int reg)
> > +{
> > +       switch (reg) {
> > +       case 0x000C:
> > +       case 0x0034 ... 0x0038:
> > +       case 0x0048 ... 0x007C:
> > +       case 0x0100 ... 0x017C:
> > +       case 0x0200 ... 0x027C:
> > +       case 0x0308 ... 0x030C:
> > +       case 0x0320 ... 0x037C:
> > +       case 0x0480 ... 0x0484:
> > +       case 0x0580 ... 0x0584:
> > +       case 0x0680 ... 0x0684:
> > +       case 0x0780 ... 0x0784:
> > +       case 0x0880:
> > +       case 0x0980 ... 0x098C:
> > +       case 0x1100 ... 0x1118:
> > +       case 0x1200 ... 0x1204:
> > +       case 0x1400 ... 0x1484:
> > +       case 0x1500 ... 0x1584:
> > +       case 0x1600 ... 0x1680:
> > +       case 0x1700 ... 0x1780:
> > +       case 0x1800 ... 0x1884:
> > +       case 0x1900 ... 0x1984:
> > +       case 0x1A00 ... 0x1A84:
> > +       case 0x1B00 ... 0x1B8C:
> > +       case 0x1C00 ... 0x1C8C:
> > +       case 0x1D00 ... 0x1D8C:
> > +       case 0x3000:
> > +       case 0x3010:
> > +       case 0x3020:
> > +       case 0x3030:
> > +       case 0x4000:
> > +       case 0x5008 ... 0x500C:
> > +       case 0x5020 ... 0x503C:
> > +       case 0x5100 ... 0x5114:
> > +       case 0x6000:
> > +               return true;
> > +
> > +       default:
> > +               return false;
> > +       }
> > +}
>
> This exposes a lot of dangerous registers.  Aren't they also exported
> to userspace through drivers/base/regmap/regmap-debugfs.c?
>
Yes they are exported to userspace. I'll drop it

> > --- a/drivers/clk/renesas/renesas-cpg-mssr.h
> > +++ b/drivers/clk/renesas/renesas-cpg-mssr.h
> > @@ -178,6 +178,7 @@ struct cpg_mssr_info {
> >
> >         /* Callbacks */
> >         int (*init)(struct device *dev);
> > +       int (*sysc_init)(struct device *dev);
>
> I would rather call it "late_init" or "post_init", as it might become
> useful for other purposes, too.
>
Ok, I will rename this to post_init.

Cheers,
Prabhakar

> >         struct clk *(*cpg_clk_register)(struct device *dev,
> >                                         const struct cpg_core_clk *core,
> >                                         const struct cpg_mssr_info *info,
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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