RE: [PATCH v5 1/5] soc: renesas: rz-sysc: add syscon/regmap support

[John Madieu]

Hi Claudiu,

Thanks for the feedback.

> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@xxxxxxxxx>
> Sent: Thursday, April 3, 2025 10:04 AM
> To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>; geert+renesas@xxxxxxxxx;
> conor+dt@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; robh@xxxxxxxxxx;
> rafael@xxxxxxxxxx; daniel.lezcano@xxxxxxxxxx
> Cc: magnus.damm@xxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> john.madieu@xxxxxxxxx; rui.zhang@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> linux-renesas-soc@xxxxxxxxxxxxxxx; sboyd@xxxxxxxxxx; Biju Das
> <biju.das.jz@xxxxxxxxxxxxxx>; linux-pm@xxxxxxxxxxxxxxx;
> lukasz.luba@xxxxxxx
> Subject: Re: [PATCH v5 1/5] soc: renesas: rz-sysc: add syscon/regmap
> support
> 
> Hi, John,
> 
> On 31.03.2025 00:49, John Madieu 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, let's register a custom regmap configuration.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> > ---
> > v1 -> v2: no changes
> > v2 -> v3: no changes
> > v3 -> v4: no changes
> > v4 -> v5: no changes
> >
> > Note for Maintainers: There is a false positive warning reported by
> > checkpatch.pl on this patch patch stating that the regmap_config
> > struct should be const, despite the fact it's updated in probe().
> >
> >  drivers/soc/renesas/Kconfig         |  1 +
> >  drivers/soc/renesas/r9a09g047-sys.c |  1 +
> >  drivers/soc/renesas/rz-sysc.c       | 30 ++++++++++++++++++++++++++++-
> >  drivers/soc/renesas/rz-sysc.h       |  2 ++
> >  4 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> > index 49648cf28bd2..3ffd3a4ca18d 100644
> > --- a/drivers/soc/renesas/Kconfig
> > +++ b/drivers/soc/renesas/Kconfig
> > @@ -388,6 +388,7 @@ config RST_RCAR
> >
> >  config SYSC_RZ
> >  	bool "System controller for RZ SoCs" if COMPILE_TEST
> > +	select MFD_SYSCON
> >
> >  config SYSC_R9A08G045
> >  	bool "Renesas RZ/G3S System controller support" if COMPILE_TEST diff
> > --git a/drivers/soc/renesas/r9a09g047-sys.c
> > b/drivers/soc/renesas/r9a09g047-sys.c
> > index cd2eb7782cfe..5b010a519fab 100644
> > --- a/drivers/soc/renesas/r9a09g047-sys.c
> > +++ b/drivers/soc/renesas/r9a09g047-sys.c
> > @@ -64,4 +64,5 @@ static const struct rz_sysc_soc_id_init_data
> > rzg3e_sys_soc_id_init_data __initco
> >
> >  const struct rz_sysc_init_data rzg3e_sys_init_data = {
> >  	.soc_id_init_data = &rzg3e_sys_soc_id_init_data,
> > +	.max_register_offset = 0x170c,
> >  };
> > diff --git a/drivers/soc/renesas/rz-sysc.c
> > b/drivers/soc/renesas/rz-sysc.c index 1c98da37b7d1..bcbc23da954b
> > 100644
> > --- a/drivers/soc/renesas/rz-sysc.c
> > +++ b/drivers/soc/renesas/rz-sysc.c
> > @@ -6,8 +6,10 @@
> >   */
> >
> >  #include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/of.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >  #include <linux/sys_soc.h>
> >
> >  #include "rz-sysc.h"
> > @@ -81,6 +83,14 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc,
> const struct of_device_id *mat
> >  	return 0;
> >  }
> >
> > +static struct regmap_config rz_sysc_regmap = {
> > +	.name = "rz_sysc_regs",
> > +	.reg_bits = 32,
> > +	.reg_stride = 4,
> > +	.val_bits = 32,
> > +	.fast_io = true,
> > +};
> > +
> >  static const struct of_device_id rz_sysc_match[] = {  #ifdef
> > CONFIG_SYSC_R9A08G045
> >  	{ .compatible = "renesas,r9a08g045-sysc", .data =
> > &rzg3s_sysc_init_data }, @@ -97,14 +107,21 @@ MODULE_DEVICE_TABLE(of,
> > rz_sysc_match);
> >
> >  static int rz_sysc_probe(struct platform_device *pdev)  {
> > +	const struct rz_sysc_init_data *data;
> >  	const struct of_device_id *match;
> >  	struct device *dev = &pdev->dev;
> > +	struct regmap *regmap;
> >  	struct rz_sysc *sysc;
> > +	int ret;
> >
> >  	match = of_match_node(rz_sysc_match, dev->of_node);
> >  	if (!match)
> >  		return -ENODEV;
> >
> > +	data = match->data;
> > +	if (!data)
> > +		return -EINVAL;
> > +
> >  	sysc = devm_kzalloc(dev, sizeof(*sysc), GFP_KERNEL);
> >  	if (!sysc)
> >  		return -ENOMEM;
> > @@ -114,7 +131,18 @@ 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);
> 
> The return value of rz_sysc_soc_init() is lost in case
> data->max_register_offset != 0. Is there a reason for this?

There is no reason for this. Will fix it for v6.

> 
> > +
> > +	if (data->max_register_offset) {
> > +		rz_sysc_regmap.max_register = data->max_register_offset;
> > +		regmap = devm_regmap_init_mmio(dev, sysc->base,
> &rz_sysc_regmap);
> > +		if (IS_ERR(regmap))
> > +			return PTR_ERR(regmap);
> > +
> > +		ret = of_syscon_register_regmap(dev->of_node, regmap);
> > +	}
> > +
> > +	return ret;
> >  }
> >
> >  static struct platform_driver rz_sysc_driver = { diff --git
> > a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h index
> > aa83948c5117..37a3bb2c87f8 100644
> > --- a/drivers/soc/renesas/rz-sysc.h
> > +++ b/drivers/soc/renesas/rz-sysc.h
> > @@ -34,9 +34,11 @@ struct rz_sysc_soc_id_init_data {
> >  /**
> >   * struct rz_sysc_init_data - RZ SYSC initialization data
> >   * @soc_id_init_data: RZ SYSC SoC ID initialization data
> > + * @max_register_offset: Maximum SYSC register offset to be used by
> > + the regmap config
> >   */
> >  struct rz_sysc_init_data {
> >  	const struct rz_sysc_soc_id_init_data *soc_id_init_data;
> > +	u32 max_register_offset;
> >  };
> >
> >  extern const struct rz_sysc_init_data rzg3e_sys_init_data;

Regards,
John