linux-kernel - Re: [PATCH v6 1/5] soc: renesas: rz-sysc: Add syscon/regmap support

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: <CAMuHMdVyf3Xtpw=LWHrnD2CVQX4xYm=FBHvY_dx9OesHDz5zNg@mail.gmail.com>
Date: Mon, 4 Aug 2025 11:19:05 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: John Madieu <john.madieu.xa@...renesas.com>
Cc: conor+dt@...nel.org, daniel.lezcano@...aro.org, krzk+dt@...nel.org, 
	rafael@...nel.org, biju.das.jz@...renesas.com, devicetree@...r.kernel.org, 
	john.madieu@...il.com, linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, 
	linux-renesas-soc@...r.kernel.org, lukasz.luba@....com, magnus.damm@...il.com, 
	robh@...nel.org, rui.zhang@...el.com, sboyd@...nel.org, 
	niklas.soderlund+renesas@...natech.se, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH v6 1/5] soc: renesas: rz-sysc: Add syscon/regmap support

Hi John,

On Thu, 22 May 2025 at 20:23, John Madieu <john.madieu.xa@...renesas.com> wrote:
> The RZ/G3E system controller has various registers that control or report
> some properties specific to individual IPs. The regmap is registered as a
> syscon device to allow these IP drivers to access the registers through the
> regmap API.
>
> As other RZ SoCs might have custom read/write callbacks or max-offsets,
> add register a custom regmap configuration.
>
> Signed-off-by: John Madieu <john.madieu.xa@...renesas.com>
> [claudiu.beznea:
>  - s/rzg3e_sysc_regmap/rzv2h_sysc_regmap in RZ/V2H sysc
>    file
>  - do not check the match->data validity in rz_sysc_probe() as it is
>    always valid
>  - register the regmap if data->regmap_cfg is valid]
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>

Thanks for your patch!

As you probably noticed, I am not a big fan of syscon, so I try
to avoid getting involved with syscon patches as much as possible.
But this patch has appeared in multiple series, so I am afraid I cannot
avoid it anymore ;-)

> --- a/drivers/soc/renesas/r9a08g045-sysc.c
> +++ b/drivers/soc/renesas/r9a08g045-sysc.c
> @@ -18,6 +18,16 @@ static const struct rz_sysc_soc_id_init_data rzg3s_sysc_soc_id_init_data __initc
>         .specific_id_mask = GENMASK(27, 0),
>  };
>
> +static const struct regmap_config rzg3s_sysc_regmap __initconst = {
> +       .name = "rzg3s_sysc_regs",
> +       .reg_bits = 32,
> +       .reg_stride = 4,
> +       .val_bits = 32,
> +       .fast_io = true,
> +       .max_register = 0xe20,
> +};

Struct regmap_config is a rather large structure, and the only
SoC-specific members are the .name (which doesn't really matter) and
.max_register members.  Hence please move .max_register back to struct
rz_sysc_init_data (like in v5), and allocate struct regmap_config
dynamically (by embedding it into struct rz_sysc).

> +
>  const struct rz_sysc_init_data rzg3s_sysc_init_data __initconst = {
>         .soc_id_init_data = &rzg3s_sysc_soc_id_init_data,
> +       .regmap_cfg = &rzg3s_sysc_regmap,
>  };

> --- a/drivers/soc/renesas/rz-sysc.c
> +++ b/drivers/soc/renesas/rz-sysc.c
> @@ -117,7 +124,15 @@ static int rz_sysc_probe(struct platform_device *pdev)
>                 return PTR_ERR(sysc->base);
>
>         sysc->dev = dev;
> -       return rz_sysc_soc_init(sysc, match);
> +       ret = rz_sysc_soc_init(sysc, match);
> +       if (ret || !data->regmap_cfg)

data->regmap_cfg should never be NULL (but this doesn't matter anymore,
in the context of my request above).

> +               return ret;
> +
> +       regmap = devm_regmap_init_mmio(dev, sysc->base, data->regmap_cfg);
> +       if (IS_ERR(regmap))
> +               return PTR_ERR(regmap);
> +
> +       return of_syscon_register_regmap(dev->of_node, regmap);
>  }

The rest LGTM.

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