Hi John,
On Thu, 22 May 2025 at 20:23, John Madieu <john.madieu.xa@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> [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@xxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxx
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
