[PATCH] soc: renesas: rz-sysc: Populate readable_reg/writeable_reg in regmap config

Claudiu posted 1 patch 1 week, 2 days ago
drivers/soc/renesas/r9a08g045-sysc.c | 17 +++++++++++++++++
drivers/soc/renesas/r9a09g047-sys.c  | 21 +++++++++++++++++++++
drivers/soc/renesas/r9a09g056-sys.c  |  7 +++++++
drivers/soc/renesas/r9a09g057-sys.c  |  7 +++++++
drivers/soc/renesas/rz-sysc.c        |  2 ++
drivers/soc/renesas/rz-sysc.h        |  4 ++++
6 files changed, 58 insertions(+)
[PATCH] soc: renesas: rz-sysc: Populate readable_reg/writeable_reg in regmap config
Posted by Claudiu 1 week, 2 days ago
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Not all system controller registers are accessible from Linux. Accessing
such registers generates synchronous external abort. Populate the
readable_reg and writeable_reg members of the regmap config to inform the
regmap core which registers can be accessed. The list will need to be
updated whenever new system controller functionality is exported through
regmap.

Fixes: 2da2740fb9c8 ("soc: renesas: rz-sysc: Add syscon/regmap support")
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
 drivers/soc/renesas/r9a08g045-sysc.c | 17 +++++++++++++++++
 drivers/soc/renesas/r9a09g047-sys.c  | 21 +++++++++++++++++++++
 drivers/soc/renesas/r9a09g056-sys.c  |  7 +++++++
 drivers/soc/renesas/r9a09g057-sys.c  |  7 +++++++
 drivers/soc/renesas/rz-sysc.c        |  2 ++
 drivers/soc/renesas/rz-sysc.h        |  4 ++++
 6 files changed, 58 insertions(+)

diff --git a/drivers/soc/renesas/r9a08g045-sysc.c b/drivers/soc/renesas/r9a08g045-sysc.c
index 0504d4e68761..e4455ac37511 100644
--- a/drivers/soc/renesas/r9a08g045-sysc.c
+++ b/drivers/soc/renesas/r9a08g045-sysc.c
@@ -6,10 +6,14 @@
  */
 
 #include <linux/bits.h>
+#include <linux/device.h>
 #include <linux/init.h>
 
 #include "rz-sysc.h"
 
+#define SYS_USB_PWRRDY		0xd70
+#define SYS_PCIE_RST_RSM_B	0xd74
+
 static const struct rz_sysc_soc_id_init_data rzg3s_sysc_soc_id_init_data __initconst = {
 	.family = "RZ/G3S",
 	.id = 0x85e0447,
@@ -18,7 +22,20 @@ static const struct rz_sysc_soc_id_init_data rzg3s_sysc_soc_id_init_data __initc
 	.specific_id_mask = GENMASK(27, 0),
 };
 
+static bool rzg3s_regmap_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case SYS_USB_PWRRDY:
+	case SYS_PCIE_RST_RSM_B:
+		return true;
+	default:
+		return false;
+	}
+}
+
 const struct rz_sysc_init_data rzg3s_sysc_init_data __initconst = {
 	.soc_id_init_data = &rzg3s_sysc_soc_id_init_data,
+	.readable_reg = rzg3s_regmap_readable_reg,
+	.writeable_reg = rzg3s_regmap_readable_reg,
 	.max_register = 0xe20,
 };
diff --git a/drivers/soc/renesas/r9a09g047-sys.c b/drivers/soc/renesas/r9a09g047-sys.c
index 2e8426c03050..4200253638f8 100644
--- a/drivers/soc/renesas/r9a09g047-sys.c
+++ b/drivers/soc/renesas/r9a09g047-sys.c
@@ -29,6 +29,9 @@
 #define SYS_LSI_PRR_CA55_DIS		BIT(8)
 #define SYS_LSI_PRR_NPU_DIS		BIT(1)
 
+#define SYS_LSI_OTPTSU1TRMVAL0		0x330
+#define SYS_LSI_OTPTSU1TRMVAL1		0x334
+
 static void rzg3e_sys_print_id(struct device *dev,
 				void __iomem *sysc_base,
 				struct soc_device_attribute *soc_dev_attr)
@@ -62,7 +65,25 @@ static const struct rz_sysc_soc_id_init_data rzg3e_sys_soc_id_init_data __initco
 	.print_id = rzg3e_sys_print_id,
 };
 
+static bool rzg3e_regmap_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case SYS_LSI_OTPTSU1TRMVAL0:
+	case SYS_LSI_OTPTSU1TRMVAL1:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool rzg3e_regmap_writeable_reg(struct device *dev, unsigned int reg)
+{
+	return false;
+}
+
 const struct rz_sysc_init_data rzg3e_sys_init_data = {
 	.soc_id_init_data = &rzg3e_sys_soc_id_init_data,
+	.readable_reg = rzg3e_regmap_readable_reg,
+	.writeable_reg = rzg3e_regmap_writeable_reg,
 	.max_register = 0x170c,
 };
diff --git a/drivers/soc/renesas/r9a09g056-sys.c b/drivers/soc/renesas/r9a09g056-sys.c
index 3ad1422eba36..5ebe53042524 100644
--- a/drivers/soc/renesas/r9a09g056-sys.c
+++ b/drivers/soc/renesas/r9a09g056-sys.c
@@ -70,6 +70,13 @@ static const struct rz_sysc_soc_id_init_data rzv2n_sys_soc_id_init_data __initco
 	.print_id = rzv2n_sys_print_id,
 };
 
+static bool rzv2n_regmap_readable_reg(struct device *dev, unsigned int reg)
+{
+	return false;
+}
+
 const struct rz_sysc_init_data rzv2n_sys_init_data = {
 	.soc_id_init_data = &rzv2n_sys_soc_id_init_data,
+	.readable_reg = rzv2n_regmap_readable_reg,
+	.writeable_reg = rzv2n_regmap_readable_reg,
 };
diff --git a/drivers/soc/renesas/r9a09g057-sys.c b/drivers/soc/renesas/r9a09g057-sys.c
index e3390e7c7fe5..8336b8466bbf 100644
--- a/drivers/soc/renesas/r9a09g057-sys.c
+++ b/drivers/soc/renesas/r9a09g057-sys.c
@@ -62,7 +62,14 @@ static const struct rz_sysc_soc_id_init_data rzv2h_sys_soc_id_init_data __initco
 	.print_id = rzv2h_sys_print_id,
 };
 
+static bool rzv2h_regmap_readable_reg(struct device *dev, unsigned int reg)
+{
+	return false;
+}
+
 const struct rz_sysc_init_data rzv2h_sys_init_data = {
 	.soc_id_init_data = &rzv2h_sys_soc_id_init_data,
+	.readable_reg = rzv2h_regmap_readable_reg,
+	.writeable_reg = rzv2h_regmap_readable_reg,
 	.max_register = 0x170c,
 };
diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c
index 9f79e299e6f4..19c1e666279b 100644
--- a/drivers/soc/renesas/rz-sysc.c
+++ b/drivers/soc/renesas/rz-sysc.c
@@ -140,6 +140,8 @@ static int rz_sysc_probe(struct platform_device *pdev)
 	regmap_cfg->val_bits = 32;
 	regmap_cfg->fast_io = true;
 	regmap_cfg->max_register = data->max_register;
+	regmap_cfg->readable_reg = data->readable_reg;
+	regmap_cfg->writeable_reg = data->writeable_reg;
 
 	regmap = devm_regmap_init_mmio(dev, sysc->base, regmap_cfg);
 	if (IS_ERR(regmap))
diff --git a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h
index 8eec355d5d56..88929bf21cb1 100644
--- a/drivers/soc/renesas/rz-sysc.h
+++ b/drivers/soc/renesas/rz-sysc.h
@@ -34,10 +34,14 @@ 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
+ * @writeable_reg: Regmap writeable register check function
+ * @readable_reg: Regmap readable register check function
  * @max_register: 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;
+	bool (*writeable_reg)(struct device *dev, unsigned int reg);
+	bool (*readable_reg)(struct device *dev, unsigned int reg);
 	u32 max_register;
 };
 
-- 
2.43.0
Re: [PATCH] soc: renesas: rz-sysc: Populate readable_reg/writeable_reg in regmap config
Posted by Geert Uytterhoeven 6 days, 13 hours ago
Hi Claudiu,

On Mon, 22 Sept 2025 at 09:41, Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Not all system controller registers are accessible from Linux. Accessing
> such registers generates synchronous external abort. Populate the
> readable_reg and writeable_reg members of the regmap config to inform the
> regmap core which registers can be accessed. The list will need to be
> updated whenever new system controller functionality is exported through
> regmap.
>
> Fixes: 2da2740fb9c8 ("soc: renesas: rz-sysc: Add syscon/regmap support")
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/soc/renesas/r9a08g045-sysc.c
> +++ b/drivers/soc/renesas/r9a08g045-sysc.c
> @@ -6,10 +6,14 @@
>   */
>
>  #include <linux/bits.h>
> +#include <linux/device.h>
>  #include <linux/init.h>
>
>  #include "rz-sysc.h"
>
> +#define SYS_USB_PWRRDY         0xd70
> +#define SYS_PCIE_RST_RSM_B     0xd74
> +
>  static const struct rz_sysc_soc_id_init_data rzg3s_sysc_soc_id_init_data __initconst = {
>         .family = "RZ/G3S",
>         .id = 0x85e0447,
> @@ -18,7 +22,20 @@ static const struct rz_sysc_soc_id_init_data rzg3s_sysc_soc_id_init_data __initc
>         .specific_id_mask = GENMASK(27, 0),
>  };
>
> +static bool rzg3s_regmap_readable_reg(struct device *dev, unsigned int reg)
> +{
> +       switch (reg) {
> +       case SYS_USB_PWRRDY:
> +       case SYS_PCIE_RST_RSM_B:

Given upstream has already support for RZ/G3S Ethernet, it may be wise
to add the GEther0/1 Config Registers at 0x380/0x390, too.  That way
you avoid a possible future hard dependency and regression when adding
support for configuring that register from the Ethernet driver.
The same is true for RZ/G3E, RZ/V2H, and RZ/V2N.

> +               return true;
> +       default:
> +               return false;
> +       }
> +}
> +
>  const struct rz_sysc_init_data rzg3s_sysc_init_data __initconst = {
>         .soc_id_init_data = &rzg3s_sysc_soc_id_init_data,
> +       .readable_reg = rzg3s_regmap_readable_reg,
> +       .writeable_reg = rzg3s_regmap_readable_reg,
>         .max_register = 0xe20,
>  };

> --- a/drivers/soc/renesas/r9a09g056-sys.c
> +++ b/drivers/soc/renesas/r9a09g056-sys.c
> @@ -70,6 +70,13 @@ static const struct rz_sysc_soc_id_init_data rzv2n_sys_soc_id_init_data __initco
>         .print_id = rzv2n_sys_print_id,
>  };
>
> +static bool rzv2n_regmap_readable_reg(struct device *dev, unsigned int reg)
> +{
> +       return false;

I would already add the TRU trimming registers, also for RZ/V2H, as
they can probably just reuse the RZ/G3E TSU driver.

> +}
> +
>  const struct rz_sysc_init_data rzv2n_sys_init_data = {
>         .soc_id_init_data = &rzv2n_sys_soc_id_init_data,
> +       .readable_reg = rzv2n_regmap_readable_reg,
> +       .writeable_reg = rzv2n_regmap_readable_reg,

Oops, this one does not have ".max_register = 0x170c" yet.
Does this cause any ill effects that warrant an urgent fix?

>  };

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
Re: [PATCH] soc: renesas: rz-sysc: Populate readable_reg/writeable_reg in regmap config
Posted by Claudiu Beznea 5 days, 22 hours ago
Hi, Geert,

On 9/25/25 17:05, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Mon, 22 Sept 2025 at 09:41, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Not all system controller registers are accessible from Linux. Accessing
>> such registers generates synchronous external abort. Populate the
>> readable_reg and writeable_reg members of the regmap config to inform the
>> regmap core which registers can be accessed. The list will need to be
>> updated whenever new system controller functionality is exported through
>> regmap.
>>
>> Fixes: 2da2740fb9c8 ("soc: renesas: rz-sysc: Add syscon/regmap support")
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Thanks for your patch!
> 
>> --- a/drivers/soc/renesas/r9a08g045-sysc.c
>> +++ b/drivers/soc/renesas/r9a08g045-sysc.c
>> @@ -6,10 +6,14 @@
>>   */
>>
>>  #include <linux/bits.h>
>> +#include <linux/device.h>
>>  #include <linux/init.h>
>>
>>  #include "rz-sysc.h"
>>
>> +#define SYS_USB_PWRRDY         0xd70
>> +#define SYS_PCIE_RST_RSM_B     0xd74
>> +
>>  static const struct rz_sysc_soc_id_init_data rzg3s_sysc_soc_id_init_data __initconst = {
>>         .family = "RZ/G3S",
>>         .id = 0x85e0447,
>> @@ -18,7 +22,20 @@ static const struct rz_sysc_soc_id_init_data rzg3s_sysc_soc_id_init_data __initc
>>         .specific_id_mask = GENMASK(27, 0),
>>  };
>>
>> +static bool rzg3s_regmap_readable_reg(struct device *dev, unsigned int reg)
>> +{
>> +       switch (reg) {
>> +       case SYS_USB_PWRRDY:
>> +       case SYS_PCIE_RST_RSM_B:
> 
> Given upstream has already support for RZ/G3S Ethernet, it may be wise
> to add the GEther0/1 Config Registers at 0x380/0x390, too.  That way
> you avoid a possible future hard dependency and regression when adding
> support for configuring that register from the Ethernet driver.
> The same is true for RZ/G3E, RZ/V2H, and RZ/V2N.

Ok, I'll update.

> 
>> +               return true;
>> +       default:
>> +               return false;
>> +       }
>> +}
>> +
>>  const struct rz_sysc_init_data rzg3s_sysc_init_data __initconst = {
>>         .soc_id_init_data = &rzg3s_sysc_soc_id_init_data,
>> +       .readable_reg = rzg3s_regmap_readable_reg,
>> +       .writeable_reg = rzg3s_regmap_readable_reg,
>>         .max_register = 0xe20,
>>  };
> 
>> --- a/drivers/soc/renesas/r9a09g056-sys.c
>> +++ b/drivers/soc/renesas/r9a09g056-sys.c
>> @@ -70,6 +70,13 @@ static const struct rz_sysc_soc_id_init_data rzv2n_sys_soc_id_init_data __initco
>>         .print_id = rzv2n_sys_print_id,
>>  };
>>
>> +static bool rzv2n_regmap_readable_reg(struct device *dev, unsigned int reg)
>> +{
>> +       return false;
> 
> I would already add the TRU trimming registers, also for RZ/V2H, as
> they can probably just reuse the RZ/G3E TSU driver.

Wasn't aware of it. I'll check it.

> 
>> +}
>> +
>>  const struct rz_sysc_init_data rzv2n_sys_init_data = {
>>         .soc_id_init_data = &rzv2n_sys_soc_id_init_data,
>> +       .readable_reg = rzv2n_regmap_readable_reg,
>> +       .writeable_reg = rzv2n_regmap_readable_reg,
> 
> Oops, this one does not have ".max_register = 0x170c" yet.
> Does this cause any ill effects that warrant an urgent fix?

Yes, similar crash. I wrongly checked it initially. I'll update it.

Thank you for your review,
Claudiu

> 
>>  };
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
>
Re: [PATCH] soc: renesas: rz-sysc: Populate readable_reg/writeable_reg in regmap config
Posted by Geert Uytterhoeven 1 week ago
Hi Claudiu,

On Mon, 22 Sept 2025 at 09:41, Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Not all system controller registers are accessible from Linux. Accessing
> such registers generates synchronous external abort. Populate the
> readable_reg and writeable_reg members of the regmap config to inform the
> regmap core which registers can be accessed. The list will need to be
> updated whenever new system controller functionality is exported through
> regmap.
>
> Fixes: 2da2740fb9c8 ("soc: renesas: rz-sysc: Add syscon/regmap support")
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thanks for your patch!

How can this be triggered? AFAIU, registers are only accessed as
obtained from syscon_regmap_lookup_by_phandle_args(), i.e. based on the
register offset stored in the DTB.  If the offset in the DTB is wrong,
there is not much we can do ("garbage in, garbage out"), and the DTB
should be fixed instead.

Is there another way the user can access these non-existing registers?
Thanks!

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
Re: [PATCH] soc: renesas: rz-sysc: Populate readable_reg/writeable_reg in regmap config
Posted by Claudiu Beznea 1 week ago
Hi, Geert,

On 9/24/25 16:44, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Mon, 22 Sept 2025 at 09:41, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Not all system controller registers are accessible from Linux. Accessing
>> such registers generates synchronous external abort. Populate the
>> readable_reg and writeable_reg members of the regmap config to inform the
>> regmap core which registers can be accessed. The list will need to be
>> updated whenever new system controller functionality is exported through
>> regmap.
>>
>> Fixes: 2da2740fb9c8 ("soc: renesas: rz-sysc: Add syscon/regmap support")
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Thanks for your patch!
> 
> How can this be triggered? 

I found this issue by reading the exported regmap debug file:

cat /sys/kernel/debug/regmap/11020000.system-controller-rz_sysc_regs/registers

Thank you,
Claudiu

> AFAIU, registers are only accessed as
> obtained from syscon_regmap_lookup_by_phandle_args(), i.e. based on the
> register offset stored in the DTB.  If the offset in the DTB is wrong,
> there is not much we can do ("garbage in, garbage out"), and the DTB
> should be fixed instead.
> 
> Is there another way the user can access these non-existing registers?
> Thanks!
> 
> 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
Re: [PATCH] soc: renesas: rz-sysc: Populate readable_reg/writeable_reg in regmap config
Posted by Geert Uytterhoeven 6 days, 13 hours ago
Hi Claudiu,

On Wed, 24 Sept 2025 at 17:45, Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:
> On 9/24/25 16:44, Geert Uytterhoeven wrote:
> > On Mon, 22 Sept 2025 at 09:41, Claudiu <claudiu.beznea@tuxon.dev> wrote:
> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> Not all system controller registers are accessible from Linux. Accessing
> >> such registers generates synchronous external abort. Populate the
> >> readable_reg and writeable_reg members of the regmap config to inform the
> >> regmap core which registers can be accessed. The list will need to be
> >> updated whenever new system controller functionality is exported through
> >> regmap.
> >>
> >> Fixes: 2da2740fb9c8 ("soc: renesas: rz-sysc: Add syscon/regmap support")
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > How can this be triggered?
>
> I found this issue by reading the exported regmap debug file:
>
> cat /sys/kernel/debug/regmap/11020000.system-controller-rz_sysc_regs/registers

Thank you, that was exactly what I was looking for!

> > AFAIU, registers are only accessed as
> > obtained from syscon_regmap_lookup_by_phandle_args(), i.e. based on the
> > register offset stored in the DTB.  If the offset in the DTB is wrong,
> > there is not much we can do ("garbage in, garbage out"), and the DTB
> > should be fixed instead.
> >
> > Is there another way the user can access these non-existing registers?
> > Thanks!

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