[PATCH 0/5] soc: renesas: Add system controller support for RZ/G3E SoC

[PATCH 0/5] soc: renesas: Add system controller support for RZ/G3E SoC

[John Madieu]

This patch series adds support for the RZ/G3E system controller and extends
the existing RZ/V2H(P) system controller to support syscon. The RZ/G3E
system controller allows detecting various SoC features like core count,
NPU availability, and CA55 PLL configuration.

Key features:
- Syscon support for both RZ/V2H and RZ/G3E system controllers
- Detection of quad/dual core configuration
- Detection of Ethos-U55 NPU presence
- Validation of CA55 PLL frequency setting
- SoC-specific extended identification through callbacks

This patch series depends upon [1] and [2].

Tested:
- Example of SoC detection:
[    0.065608] renesas-rz-sysc 10430000.system-controller: Detected Renesas Quad Core RZ/G3E r9a09g047 Rev 0  with Ethos-U55
- Example of PLL misconfiguration warning:
 [    0.065616] renesas-rz-sysc 10430000.system-controller: CA55 PLL is not set to 1.7GHz

[1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=914097
[2] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=912697

John Madieu (5):
  dt-bindings: arm: renesas: Add syscon compatibility to RZ/V2H(P) SYS
    Controller
  dt-bindings: soc: renesas: Document Renesas RZ/G3E SoC variants
  soc: renesas: rz-sysc: Add support for RZ/G3E family
  arm64: dts: renesas: r9a09g047: add sys node
  arm64: dts: renesas: r9a09g057: Add syscon compatibility to sys node

 .../soc/renesas/renesas,r9a09g057-sys.yaml    |  8 ++-
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi    |  7 ++
 arch/arm64/boot/dts/renesas/r9a09g057.dtsi    |  2 +-
 drivers/soc/renesas/Kconfig                   |  6 ++
 drivers/soc/renesas/Makefile                  |  1 +
 drivers/soc/renesas/r9a09g047-sysc.c          | 70 +++++++++++++++++++
 drivers/soc/renesas/rz-sysc.c                 | 44 ++++++++----
 drivers/soc/renesas/rz-sysc.h                 |  7 ++
 8 files changed, 128 insertions(+), 17 deletions(-)
 create mode 100644 drivers/soc/renesas/r9a09g047-sysc.c

-- 
2.25.1

[PATCH 1/5] dt-bindings: arm: renesas: Add syscon compatibility to RZ/V2H(P) SYS Controller

[John Madieu]

Update the RZ/V2H system controller binding to include syscon compatibility,
allowing it to be used as a generic system controller.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
 .../bindings/soc/renesas/renesas,r9a09g057-sys.yaml        | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml
index ebbf0c9109ce..8e6aa7e44857 100644
--- a/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml
+++ b/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml
@@ -22,7 +22,10 @@ description: |
 
 properties:
   compatible:
-    const: renesas,r9a09g057-sys
+    items:
+      - enum:
+          - renesas,r9a09g057-sys # RZ/V2H
+      - const: syscon
 
   reg:
     maxItems: 1
@@ -44,7 +47,7 @@ additionalProperties: false
 examples:
   - |
     sys: system-controller@10430000 {
-        compatible = "renesas,r9a09g057-sys";
+        compatible = "renesas,r9a09g057-sys", "syscon";
         reg = <0x10430000 0x10000>;
         clocks = <&cpg 1>;
         resets = <&cpg 1>;
-- 
2.25.1

[PATCH 2/5] dt-bindings: soc: renesas: Document Renesas RZ/G3E SoC variants

[John Madieu]

Document RZ/G3E (R9A09G047) SoC variants.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
 .../devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml   | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml
index 8e6aa7e44857..2760e3ea85fb 100644
--- a/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml
+++ b/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml
@@ -24,6 +24,7 @@ properties:
   compatible:
     items:
       - enum:
+          - renesas,r9a09g047-sys # RZ/G3E
           - renesas,r9a09g057-sys # RZ/V2H
       - const: syscon
 
-- 
2.25.1

[PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family

[John Madieu]

Add SoC detection support for RZ/G3E SoC. Also add support for detecting the
number of cores and ETHOS-U55 NPU and also detect PLL mismatch for SW settings
other than 1.7GHz.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
 drivers/soc/renesas/Kconfig          |  6 +++
 drivers/soc/renesas/Makefile         |  1 +
 drivers/soc/renesas/r9a09g047-sysc.c | 70 ++++++++++++++++++++++++++++
 drivers/soc/renesas/rz-sysc.c        | 44 +++++++++++------
 drivers/soc/renesas/rz-sysc.h        |  7 +++
 5 files changed, 114 insertions(+), 14 deletions(-)
 create mode 100644 drivers/soc/renesas/r9a09g047-sysc.c

diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index a792a3e915fe..9e46b0ee6e80 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -348,6 +348,7 @@ config ARCH_R9A09G011
 
 config ARCH_R9A09G047
 	bool "ARM64 Platform support for RZ/G3E"
+	select SYSC_R9A09G047
 	help
 	  This enables support for the Renesas RZ/G3E SoC variants.
 
@@ -386,9 +387,14 @@ config RST_RCAR
 
 config SYSC_RZ
 	bool "System controller for RZ SoCs" if COMPILE_TEST
+	depends on MFD_SYSCON
 
 config SYSC_R9A08G045
 	bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
 	select SYSC_RZ
 
+config SYSC_R9A09G047
+	bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
+	select SYSC_RZ
+
 endif # SOC_RENESAS
diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile
index 8cd139b3dd0a..3256706112d9 100644
--- a/drivers/soc/renesas/Makefile
+++ b/drivers/soc/renesas/Makefile
@@ -7,6 +7,7 @@ ifdef CONFIG_SMP
 obj-$(CONFIG_ARCH_R9A06G032)	+= r9a06g032-smp.o
 endif
 obj-$(CONFIG_SYSC_R9A08G045)	+= r9a08g045-sysc.o
+obj-$(CONFIG_SYSC_R9A09G047)	+= r9a09g047-sysc.o
 
 # Family
 obj-$(CONFIG_PWC_RZV2M)		+= pwc-rzv2m.o
diff --git a/drivers/soc/renesas/r9a09g047-sysc.c b/drivers/soc/renesas/r9a09g047-sysc.c
new file mode 100644
index 000000000000..32bdab9f1774
--- /dev/null
+++ b/drivers/soc/renesas/r9a09g047-sysc.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RZ/G3E System controller driver
+ *
+ * Copyright (C) 2024 Renesas Electronics Corp.
+ */
+
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/io.h>
+
+#include "rz-sysc.h"
+
+/* Register definitions */
+#define SYS_LSI_DEVID	0x304
+#define SYS_LSI_MODE	0x300
+#define SYS_LSI_PRR	0x308
+#define SYS_LSI_DEVID_REV	GENMASK(31, 28)
+#define SYS_LSI_DEVID_SPECIFIC	GENMASK(27, 0)
+#define SYS_LSI_PRR_CA55_DIS	BIT(8)
+#define SYS_LSI_PRR_NPU_DIS	BIT(1)
+/*
+ * BOOTPLLCA[1:0]
+ *	[0,0] => 1.1GHZ
+ *	[0,1] => 1.5GHZ
+ *	[1,0] => 1.6GHZ
+ *	[1,1] => 1.7GHZ
+ */
+#define SYS_LSI_MODE_STAT_BOOTPLLCA55	GENMASK(12, 11)
+#define SYS_LSI_MODE_CA55_1_7GHz	0x3
+
+static void rzg3e_extended_device_identification(struct device *dev,
+				void __iomem *sysc_base,
+				struct soc_device_attribute *soc_dev_attr)
+{
+	u32 prr_val, mode_val;
+	bool is_quad_core, npu_enabled;
+
+	prr_val = readl(sysc_base + SYS_LSI_PRR);
+	mode_val = readl(sysc_base + SYS_LSI_MODE);
+
+	/* Check CPU and NPU configuration */
+	is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
+	npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
+
+	dev_info(dev, "Detected Renesas %s Core %s %s Rev %s  %s\n",
+		 is_quad_core ? "Quad" : "Dual",
+		 soc_dev_attr->family,
+		 soc_dev_attr->soc_id,
+		 soc_dev_attr->revision,
+		 npu_enabled ? "with Ethos-U55" : "");
+
+	/* Check CA55 PLL configuration */
+	if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) != SYS_LSI_MODE_CA55_1_7GHz)
+		dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n");
+}
+
+static const struct rz_sysc_soc_id_init_data rzg3e_sysc_soc_id_init_data __initconst = {
+	.family = "RZ/G3E",
+	.id = 0x8679447,
+	.offset = SYS_LSI_DEVID,
+	.revision_mask = SYS_LSI_DEVID_REV,
+	.specific_id_mask = SYS_LSI_DEVID_SPECIFIC,
+	.extended_device_identification = rzg3e_extended_device_identification,
+};
+
+const struct rz_sysc_init_data rzg3e_sysc_init_data = {
+	.soc_id_init_data = &rzg3e_sysc_soc_id_init_data,
+};
diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c
index d34d295831b8..515eca249b6e 100644
--- a/drivers/soc/renesas/rz-sysc.c
+++ b/drivers/soc/renesas/rz-sysc.c
@@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
 
 	soc_id_start = strchr(match->compatible, ',') + 1;
 	soc_id_end = strchr(match->compatible, '-');
-	size = soc_id_end - soc_id_start;
+	size = soc_id_end - soc_id_start + 1;
 	if (size > 32)
 		size = 32;
 	strscpy(soc_id, soc_id_start, size);
@@ -257,8 +257,16 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
 		return -ENODEV;
 	}
 
-	dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n", soc_dev_attr->family,
-		 soc_dev_attr->soc_id, soc_dev_attr->revision);
+	/* Try to call SoC-specific device identification */
+	if (soc_data->extended_device_identification) {
+		soc_data->extended_device_identification(sysc->dev, sysc->base,
+							 soc_dev_attr);
+	} else {
+		dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n",
+			 soc_dev_attr->family,
+			 soc_dev_attr->soc_id,
+			 soc_dev_attr->revision);
+	}
 
 	soc_dev = soc_device_register(soc_dev_attr);
 	if (IS_ERR(soc_dev))
@@ -283,6 +291,9 @@ static struct regmap_config rz_sysc_regmap = {
 static const struct of_device_id rz_sysc_match[] = {
 #ifdef CONFIG_SYSC_R9A08G045
 	{ .compatible = "renesas,r9a08g045-sysc", .data = &rzg3s_sysc_init_data },
+#endif
+#ifdef CONFIG_SYSC_R9A09G047
+	{ .compatible = "renesas,r9a09g047-sys", .data = &rzg3e_sysc_init_data },
 #endif
 	{ }
 };
@@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device *pdev)
 		return ret;
 
 	data = match->data;
-	if (!data->max_register_offset)
-		return -EINVAL;
+	if (data->signals_init_data) {
+		if (!data->max_register_offset)
+			return -EINVAL;
 
-	ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
-	if (ret)
-		return ret;
+		ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
+		if (ret)
+			return ret;
+
+		rz_sysc_regmap.max_register = data->max_register_offset;
+		dev_set_drvdata(dev, sysc);
 
-	dev_set_drvdata(dev, sysc);
-	rz_sysc_regmap.max_register = data->max_register_offset;
-	regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
-	if (IS_ERR(regmap))
-		return PTR_ERR(regmap);
+		regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
+		if (IS_ERR(regmap))
+			return PTR_ERR(regmap);
 
-	return of_syscon_register_regmap(dev->of_node, regmap);
+		return of_syscon_register_regmap(dev->of_node, regmap);
+	}
+
+	return 0;
 }
 
 static struct platform_driver rz_sysc_driver = {
diff --git a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h
index babca9c743c7..2b5ad41cef9e 100644
--- a/drivers/soc/renesas/rz-sysc.h
+++ b/drivers/soc/renesas/rz-sysc.h
@@ -8,7 +8,9 @@
 #ifndef __SOC_RENESAS_RZ_SYSC_H__
 #define __SOC_RENESAS_RZ_SYSC_H__
 
+#include <linux/device.h>
 #include <linux/refcount.h>
+#include <linux/sys_soc.h>
 #include <linux/types.h>
 
 /**
@@ -42,6 +44,7 @@ struct rz_sysc_signal {
  * @offset: SYSC SoC ID register offset
  * @revision_mask: SYSC SoC ID revision mask
  * @specific_id_mask: SYSC SoC ID specific ID mask
+ * @extended_device_identification: SoC-specific extended device identification
  */
 struct rz_sysc_soc_id_init_data {
 	const char * const family;
@@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
 	u32 offset;
 	u32 revision_mask;
 	u32 specific_id_mask;
+	void (*extended_device_identification)(struct device *dev,
+		void __iomem *sysc_base,
+		struct soc_device_attribute *soc_dev_attr);
 };
 
 /**
@@ -65,6 +71,7 @@ struct rz_sysc_init_data {
 	u32 max_register_offset;
 };
 
+extern const struct rz_sysc_init_data rzg3e_sysc_init_data;
 extern const struct rz_sysc_init_data rzg3s_sysc_init_data;
 
 #endif /* __SOC_RENESAS_RZ_SYSC_H__ */
-- 
2.25.1

[PATCH 4/5] arm64: dts: renesas: r9a09g047: add sys node

[John Madieu]

Add system controller node to RZ/G3E (R9A09G047) SoC DTSI.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
index 17bc95fb111f..5f65e652a413 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
@@ -154,6 +154,13 @@ cpg: clock-controller@10420000 {
 			#power-domain-cells = <0>;
 		};
 
+		sys: system-controller@10430000 {
+			compatible = "renesas,r9a09g047-sys", "syscon";
+			reg = <0 0x10430000 0 0x10000>;
+			clocks = <&cpg CPG_CORE R9A09G047_SYS_0_PCLK>;
+			resets = <&cpg 0x30>;
+		};
+
 		scif0: serial@11c01400 {
 			compatible = "renesas,scif-r9a09g047", "renesas,scif-r9a09g057";
 			reg = <0 0x11c01400 0 0x400>;
-- 
2.25.1

[PATCH 5/5] arm64: dts: renesas: r9a09g057: Add syscon compatibility to sys node

[John Madieu]

Add syscon compatibility to the RZ/V2H(P) system controller node to enable
it to be used as a generic system controller.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r9a09g057.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
index 1c550b22b164..1288503ba444 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
@@ -245,7 +245,7 @@ cpg: clock-controller@10420000 {
 		};
 
 		sys: system-controller@10430000 {
-			compatible = "renesas,r9a09g057-sys";
+			compatible = "renesas,r9a09g057-sys", "syscon";
 			reg = <0 0x10430000 0 0x10000>;
 			clocks = <&cpg CPG_CORE R9A09G057_SYS_0_PCLK>;
 			resets = <&cpg 0x30>;
-- 
2.25.1

Re: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family

[Claudiu Beznea]

Hi, John,

On 06.12.2024 23:25, John Madieu wrote:
> Add SoC detection support for RZ/G3E SoC. Also add support for detecting the
> number of cores and ETHOS-U55 NPU and also detect PLL mismatch for SW settings
> other than 1.7GHz.
> 
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
>  drivers/soc/renesas/Kconfig          |  6 +++
>  drivers/soc/renesas/Makefile         |  1 +
>  drivers/soc/renesas/r9a09g047-sysc.c | 70 ++++++++++++++++++++++++++++
>  drivers/soc/renesas/rz-sysc.c        | 44 +++++++++++------
>  drivers/soc/renesas/rz-sysc.h        |  7 +++
>  5 files changed, 114 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/soc/renesas/r9a09g047-sysc.c
> 
> diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> index a792a3e915fe..9e46b0ee6e80 100644
> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -348,6 +348,7 @@ config ARCH_R9A09G011
>  
>  config ARCH_R9A09G047
>  	bool "ARM64 Platform support for RZ/G3E"
> +	select SYSC_R9A09G047
>  	help
>  	  This enables support for the Renesas RZ/G3E SoC variants.
>  
> @@ -386,9 +387,14 @@ config RST_RCAR
>  
>  config SYSC_RZ
>  	bool "System controller for RZ SoCs" if COMPILE_TEST
> +	depends on MFD_SYSCON
>  
>  config SYSC_R9A08G045
>  	bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
>  	select SYSC_RZ
>  
> +config SYSC_R9A09G047
> +	bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> +	select SYSC_RZ
> +
>  endif # SOC_RENESAS
> diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile
> index 8cd139b3dd0a..3256706112d9 100644
> --- a/drivers/soc/renesas/Makefile
> +++ b/drivers/soc/renesas/Makefile
> @@ -7,6 +7,7 @@ ifdef CONFIG_SMP
>  obj-$(CONFIG_ARCH_R9A06G032)	+= r9a06g032-smp.o
>  endif
>  obj-$(CONFIG_SYSC_R9A08G045)	+= r9a08g045-sysc.o
> +obj-$(CONFIG_SYSC_R9A09G047)	+= r9a09g047-sysc.o
>  
>  # Family
>  obj-$(CONFIG_PWC_RZV2M)		+= pwc-rzv2m.o
> diff --git a/drivers/soc/renesas/r9a09g047-sysc.c b/drivers/soc/renesas/r9a09g047-sysc.c
> new file mode 100644
> index 000000000000..32bdab9f1774
> --- /dev/null
> +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RZ/G3E System controller driver
> + *
> + * Copyright (C) 2024 Renesas Electronics Corp.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +
> +#include "rz-sysc.h"
> +
> +/* Register definitions */
> +#define SYS_LSI_DEVID	0x304
> +#define SYS_LSI_MODE	0x300
> +#define SYS_LSI_PRR	0x308
> +#define SYS_LSI_DEVID_REV	GENMASK(31, 28)
> +#define SYS_LSI_DEVID_SPECIFIC	GENMASK(27, 0)
> +#define SYS_LSI_PRR_CA55_DIS	BIT(8)
> +#define SYS_LSI_PRR_NPU_DIS	BIT(1)
> +/*
> + * BOOTPLLCA[1:0]
> + *	[0,0] => 1.1GHZ
> + *	[0,1] => 1.5GHZ
> + *	[1,0] => 1.6GHZ
> + *	[1,1] => 1.7GHZ
> + */
> +#define SYS_LSI_MODE_STAT_BOOTPLLCA55	GENMASK(12, 11)
> +#define SYS_LSI_MODE_CA55_1_7GHz	0x3
> +
> +static void rzg3e_extended_device_identification(struct device *dev,
> +				void __iomem *sysc_base,
> +				struct soc_device_attribute *soc_dev_attr)

Not strong preference here, I think it can still be aligned to (

> +{
> +	u32 prr_val, mode_val;
> +	bool is_quad_core, npu_enabled;

Reverse christmass tree order?

> +
> +	prr_val = readl(sysc_base + SYS_LSI_PRR);
> +	mode_val = readl(sysc_base + SYS_LSI_MODE);
> +
> +	/* Check CPU and NPU configuration */
> +	is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> +	npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> +
> +	dev_info(dev, "Detected Renesas %s Core %s %s Rev %s  %s\n",

I think you have an extra space towards the end: "%s  %s"

> +		 is_quad_core ? "Quad" : "Dual",
> +		 soc_dev_attr->family,
> +		 soc_dev_attr->soc_id,
> +		 soc_dev_attr->revision,
> +		 npu_enabled ? "with Ethos-U55" : "");
> +
> +	/* Check CA55 PLL configuration */
> +	if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) != SYS_LSI_MODE_CA55_1_7GHz)
> +		dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n");
> +}
> +
> +static const struct rz_sysc_soc_id_init_data rzg3e_sysc_soc_id_init_data __initconst = {
> +	.family = "RZ/G3E",
> +	.id = 0x8679447,
> +	.offset = SYS_LSI_DEVID,
> +	.revision_mask = SYS_LSI_DEVID_REV,
> +	.specific_id_mask = SYS_LSI_DEVID_SPECIFIC,
> +	.extended_device_identification = rzg3e_extended_device_identification,
> +};
> +
> +const struct rz_sysc_init_data rzg3e_sysc_init_data = {
> +	.soc_id_init_data = &rzg3e_sysc_soc_id_init_data,
> +};
> diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c
> index d34d295831b8..515eca249b6e 100644
> --- a/drivers/soc/renesas/rz-sysc.c
> +++ b/drivers/soc/renesas/rz-sysc.c
> @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
>  
>  	soc_id_start = strchr(match->compatible, ',') + 1;
>  	soc_id_end = strchr(match->compatible, '-');
> -	size = soc_id_end - soc_id_start;
> +	size = soc_id_end - soc_id_start + 1;

This may worth a different patch.

>  	if (size > 32)
>  		size = 32;
>  	strscpy(soc_id, soc_id_start, size);
> @@ -257,8 +257,16 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
>  		return -ENODEV;
>  	}
>  
> -	dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n", soc_dev_attr->family,
> -		 soc_dev_attr->soc_id, soc_dev_attr->revision);
> +	/* Try to call SoC-specific device identification */
> +	if (soc_data->extended_device_identification) {
> +		soc_data->extended_device_identification(sysc->dev, sysc->base,
> +							 soc_dev_attr);
> +	} else {
> +		dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n",
> +			 soc_dev_attr->family,
> +			 soc_dev_attr->soc_id,
> +			 soc_dev_attr->revision);
> +	}
>  
>  	soc_dev = soc_device_register(soc_dev_attr);
>  	if (IS_ERR(soc_dev))
> @@ -283,6 +291,9 @@ static struct regmap_config rz_sysc_regmap = {
>  static const struct of_device_id rz_sysc_match[] = {
>  #ifdef CONFIG_SYSC_R9A08G045
>  	{ .compatible = "renesas,r9a08g045-sysc", .data = &rzg3s_sysc_init_data },
> +#endif
> +#ifdef CONFIG_SYSC_R9A09G047
> +	{ .compatible = "renesas,r9a09g047-sys", .data = &rzg3e_sysc_init_data },
>  #endif
>  	{ }
>  };
> @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	data = match->data;
> -	if (!data->max_register_offset)
> -		return -EINVAL;

The idea with this was to still have the syscon regmap registered no matter
the signals are available or not. This may be needed for other use cases.

> +	if (data->signals_init_data) {

I'd prefer to have this check in rz_sysc_signals_init(). In this way you
have everything signal init specific in a single function.

> +		if (!data->max_register_offset)
> +			return -EINVAL;
>  
> -	ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> -	if (ret)
> -		return ret;
> +		ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> +		if (ret)
> +			return ret;
> +
> +		rz_sysc_regmap.max_register = data->max_register_offset;
> +		dev_set_drvdata(dev, sysc);

Why changed the initial order?

Thank you,
Claudiu

>  
> -	dev_set_drvdata(dev, sysc);
> -	rz_sysc_regmap.max_register = data->max_register_offset;
> -	regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> -	if (IS_ERR(regmap))
> -		return PTR_ERR(regmap);
> +		regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> +		if (IS_ERR(regmap))
> +			return PTR_ERR(regmap);
>  
> -	return of_syscon_register_regmap(dev->of_node, regmap);
> +		return of_syscon_register_regmap(dev->of_node, regmap);
> +	}
> +
> +	return 0;
>  }
>  
>  static struct platform_driver rz_sysc_driver = {
> diff --git a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h
> index babca9c743c7..2b5ad41cef9e 100644
> --- a/drivers/soc/renesas/rz-sysc.h
> +++ b/drivers/soc/renesas/rz-sysc.h
> @@ -8,7 +8,9 @@
>  #ifndef __SOC_RENESAS_RZ_SYSC_H__
>  #define __SOC_RENESAS_RZ_SYSC_H__
>  
> +#include <linux/device.h>
>  #include <linux/refcount.h>
> +#include <linux/sys_soc.h>
>  #include <linux/types.h>
>  
>  /**
> @@ -42,6 +44,7 @@ struct rz_sysc_signal {
>   * @offset: SYSC SoC ID register offset
>   * @revision_mask: SYSC SoC ID revision mask
>   * @specific_id_mask: SYSC SoC ID specific ID mask
> + * @extended_device_identification: SoC-specific extended device identification
>   */
>  struct rz_sysc_soc_id_init_data {
>  	const char * const family;
> @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
>  	u32 offset;
>  	u32 revision_mask;
>  	u32 specific_id_mask;
> +	void (*extended_device_identification)(struct device *dev,
> +		void __iomem *sysc_base,
> +		struct soc_device_attribute *soc_dev_attr);
>  };
>  
>  /**
> @@ -65,6 +71,7 @@ struct rz_sysc_init_data {
>  	u32 max_register_offset;
>  };
>  
> +extern const struct rz_sysc_init_data rzg3e_sysc_init_data;
>  extern const struct rz_sysc_init_data rzg3s_sysc_init_data;
>  
>  #endif /* __SOC_RENESAS_RZ_SYSC_H__ */

RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family

[Biju Das]

Hi John,

Thanks for the patch.

> -----Original Message-----
> From: John Madieu <john.madieu.xa@bp.renesas.com>
> Sent: 06 December 2024 21:26
> Subject: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
> 
> Add SoC detection support for RZ/G3E SoC. Also add support for detecting the number of cores and
> ETHOS-U55 NPU and also detect PLL mismatch for SW settings other than 1.7GHz.
> 
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
> 
>  	data = match->data;
> -	if (!data->max_register_offset)
> -		return -EINVAL;
> +	if (data->signals_init_data) {
> +		if (!data->max_register_offset)
> +			return -EINVAL;
> 
> -	ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> -	if (ret)
> -		return ret;
> +		ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> +		if (ret)
> +			return ret;
> +
> +		rz_sysc_regmap.max_register = data->max_register_offset;
> +		dev_set_drvdata(dev, sysc);
> 
> -	dev_set_drvdata(dev, sysc);
> -	rz_sysc_regmap.max_register = data->max_register_offset;
> -	regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> -	if (IS_ERR(regmap))
> -		return PTR_ERR(regmap);
> +		regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> +		if (IS_ERR(regmap))
> +			return PTR_ERR(regmap);
> 
> -	return of_syscon_register_regmap(dev->of_node, regmap);
> +		return of_syscon_register_regmap(dev->of_node, regmap);

Basically if I understand correctly, you are going to use normal
Syscon call for register access in PCIe and TSU drivers. Is it correct?

For example,

priv->syscon = syscon_regmap_lookup_by_phandle(np, "syscon");


regmap_read(priv->syscon,xxx)


Cheers,
Biju


> +	}
> +
> +	return 0;
>  }
> 
>  static struct platform_driver rz_sysc_driver = { diff --git a/drivers/soc/renesas/rz-sysc.h
> b/drivers/soc/renesas/rz-sysc.h index babca9c743c7..2b5ad41cef9e 100644
> --- a/drivers/soc/renesas/rz-sysc.h
> +++ b/drivers/soc/renesas/rz-sysc.h
> @@ -8,7 +8,9 @@
>  #ifndef __SOC_RENESAS_RZ_SYSC_H__
>  #define __SOC_RENESAS_RZ_SYSC_H__
> 
> +#include <linux/device.h>
>  #include <linux/refcount.h>
> +#include <linux/sys_soc.h>
>  #include <linux/types.h>
> 
>  /**
> @@ -42,6 +44,7 @@ struct rz_sysc_signal {
>   * @offset: SYSC SoC ID register offset
>   * @revision_mask: SYSC SoC ID revision mask
>   * @specific_id_mask: SYSC SoC ID specific ID mask
> + * @extended_device_identification: SoC-specific extended device
> + identification
>   */
>  struct rz_sysc_soc_id_init_data {
>  	const char * const family;
> @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
>  	u32 offset;
>  	u32 revision_mask;
>  	u32 specific_id_mask;
> +	void (*extended_device_identification)(struct device *dev,
> +		void __iomem *sysc_base,
> +		struct soc_device_attribute *soc_dev_attr);
>  };
> 
>  /**
> @@ -65,6 +71,7 @@ struct rz_sysc_init_data {
>  	u32 max_register_offset;
>  };
> 
> +extern const struct rz_sysc_init_data rzg3e_sysc_init_data;
>  extern const struct rz_sysc_init_data rzg3s_sysc_init_data;
> 
>  #endif /* __SOC_RENESAS_RZ_SYSC_H__ */
> --
> 2.25.1

RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family

[Biju Das]

Hi John,

Thanks for the patch.

> -----Original Message-----
> From: John Madieu <john.madieu.xa@bp.renesas.com>
> Sent: 06 December 2024 21:26
> Subject: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
> 
> Add SoC detection support for RZ/G3E SoC. Also add support for detecting the number of cores and
> ETHOS-U55 NPU and also detect PLL mismatch for SW settings other than 1.7GHz.
> 
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
>  drivers/soc/renesas/Kconfig          |  6 +++
>  drivers/soc/renesas/Makefile         |  1 +
>  drivers/soc/renesas/r9a09g047-sysc.c | 70 ++++++++++++++++++++++++++++
>  drivers/soc/renesas/rz-sysc.c        | 44 +++++++++++------
>  drivers/soc/renesas/rz-sysc.h        |  7 +++
>  5 files changed, 114 insertions(+), 14 deletions(-)  create mode 100644
> drivers/soc/renesas/r9a09g047-sysc.c
> 
> diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig index
> a792a3e915fe..9e46b0ee6e80 100644
> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -348,6 +348,7 @@ config ARCH_R9A09G011
> 
>  config ARCH_R9A09G047
>  	bool "ARM64 Platform support for RZ/G3E"
> +	select SYSC_R9A09G047
>  	help
>  	  This enables support for the Renesas RZ/G3E SoC variants.
> 
> @@ -386,9 +387,14 @@ config RST_RCAR
> 
>  config SYSC_RZ
>  	bool "System controller for RZ SoCs" if COMPILE_TEST
> +	depends on MFD_SYSCON
> 
>  config SYSC_R9A08G045
>  	bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
>  	select SYSC_RZ
> 
> +config SYSC_R9A09G047
> +	bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> +	select SYSC_RZ
> +
>  endif # SOC_RENESAS
> diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile index
> 8cd139b3dd0a..3256706112d9 100644
> --- a/drivers/soc/renesas/Makefile
> +++ b/drivers/soc/renesas/Makefile
> @@ -7,6 +7,7 @@ ifdef CONFIG_SMP
>  obj-$(CONFIG_ARCH_R9A06G032)	+= r9a06g032-smp.o
>  endif
>  obj-$(CONFIG_SYSC_R9A08G045)	+= r9a08g045-sysc.o
> +obj-$(CONFIG_SYSC_R9A09G047)	+= r9a09g047-sysc.o
> 
>  # Family
>  obj-$(CONFIG_PWC_RZV2M)		+= pwc-rzv2m.o
> diff --git a/drivers/soc/renesas/r9a09g047-sysc.c b/drivers/soc/renesas/r9a09g047-sysc.c
> new file mode 100644
> index 000000000000..32bdab9f1774
> --- /dev/null
> +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RZ/G3E System controller driver
> + *
> + * Copyright (C) 2024 Renesas Electronics Corp.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +
> +#include "rz-sysc.h"
> +
> +/* Register definitions */
> +#define SYS_LSI_DEVID	0x304
> +#define SYS_LSI_MODE	0x300
> +#define SYS_LSI_PRR	0x308
> +#define SYS_LSI_DEVID_REV	GENMASK(31, 28)
> +#define SYS_LSI_DEVID_SPECIFIC	GENMASK(27, 0)
> +#define SYS_LSI_PRR_CA55_DIS	BIT(8)
> +#define SYS_LSI_PRR_NPU_DIS	BIT(1)
> +/*
> + * BOOTPLLCA[1:0]
> + *	[0,0] => 1.1GHZ
> + *	[0,1] => 1.5GHZ
> + *	[1,0] => 1.6GHZ
> + *	[1,1] => 1.7GHZ
> + */
> +#define SYS_LSI_MODE_STAT_BOOTPLLCA55	GENMASK(12, 11)
> +#define SYS_LSI_MODE_CA55_1_7GHz	0x3
> +
> +static void rzg3e_extended_device_identification(struct device *dev,
> +				void __iomem *sysc_base,
> +				struct soc_device_attribute *soc_dev_attr) {
> +	u32 prr_val, mode_val;
> +	bool is_quad_core, npu_enabled;
> +
> +	prr_val = readl(sysc_base + SYS_LSI_PRR);
> +	mode_val = readl(sysc_base + SYS_LSI_MODE);
> +
> +	/* Check CPU and NPU configuration */
> +	is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> +	npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> +
> +	dev_info(dev, "Detected Renesas %s Core %s %s Rev %s  %s\n",
> +		 is_quad_core ? "Quad" : "Dual",
> +		 soc_dev_attr->family,
> +		 soc_dev_attr->soc_id,
> +		 soc_dev_attr->revision,
> +		 npu_enabled ? "with Ethos-U55" : "");
> +
> +	/* Check CA55 PLL configuration */
> +	if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) != SYS_LSI_MODE_CA55_1_7GHz)
> +		dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n"); }
> +
> +static const struct rz_sysc_soc_id_init_data rzg3e_sysc_soc_id_init_data __initconst = {
> +	.family = "RZ/G3E",
> +	.id = 0x8679447,
> +	.offset = SYS_LSI_DEVID,
> +	.revision_mask = SYS_LSI_DEVID_REV,
> +	.specific_id_mask = SYS_LSI_DEVID_SPECIFIC,
> +	.extended_device_identification =
> +rzg3e_extended_device_identification,
> +};
> +
> +const struct rz_sysc_init_data rzg3e_sysc_init_data = {
> +	.soc_id_init_data = &rzg3e_sysc_soc_id_init_data, };
> diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c index
> d34d295831b8..515eca249b6e 100644
> --- a/drivers/soc/renesas/rz-sysc.c
> +++ b/drivers/soc/renesas/rz-sysc.c
> @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
> 
>  	soc_id_start = strchr(match->compatible, ',') + 1;
>  	soc_id_end = strchr(match->compatible, '-');
> -	size = soc_id_end - soc_id_start;
> +	size = soc_id_end - soc_id_start + 1;
>  	if (size > 32)
>  		size = 32;
>  	strscpy(soc_id, soc_id_start, size);
> @@ -257,8 +257,16 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
>  		return -ENODEV;
>  	}
> 
> -	dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n", soc_dev_attr->family,
> -		 soc_dev_attr->soc_id, soc_dev_attr->revision);
> +	/* Try to call SoC-specific device identification */
> +	if (soc_data->extended_device_identification) {
> +		soc_data->extended_device_identification(sysc->dev, sysc->base,
> +							 soc_dev_attr);
> +	} else {
> +		dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n",
> +			 soc_dev_attr->family,
> +			 soc_dev_attr->soc_id,
> +			 soc_dev_attr->revision);
> +	}
> 
>  	soc_dev = soc_device_register(soc_dev_attr);
>  	if (IS_ERR(soc_dev))
> @@ -283,6 +291,9 @@ static struct regmap_config rz_sysc_regmap = {  static const struct of_device_id
> rz_sysc_match[] = {  #ifdef CONFIG_SYSC_R9A08G045
>  	{ .compatible = "renesas,r9a08g045-sysc", .data = &rzg3s_sysc_init_data },
> +#endif
> +#ifdef CONFIG_SYSC_R9A09G047
> +	{ .compatible = "renesas,r9a09g047-sys", .data = &rzg3e_sysc_init_data
> +},
>  #endif
>  	{ }
>  };
> @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device *pdev)
>  		return ret;
> 
>  	data = match->data;
> -	if (!data->max_register_offset)
> -		return -EINVAL;
> +	if (data->signals_init_data) {
> +		if (!data->max_register_offset)
> +			return -EINVAL;

I assume you don't have any signal use case like RZ/G3S.
Can you please confirm is it correct for RZ/G3E?

Multiple drivers/devices accessing same register like USB PWRRDY register in RZ/G3S?  

Cheers,
Biju

RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family

[John Madieu]

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Saturday, December 7, 2024 11:36 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Magnus Damm <magnus.damm@gmail.com>; Rob
> Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Claudiu Beznea
> <claudiu.beznea.uj@bp.renesas.com>
> Cc: john.madieu@gmail.com; linux-renesas-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; John Madieu
> <john.madieu.xa@bp.renesas.com>
> Subject: RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> family
> 
> Hi John,
Hi Biju,
> 
> Thanks for the patch.
> 
> > -----Original Message-----
> > From: John Madieu <john.madieu.xa@bp.renesas.com>
> > Sent: 06 December 2024 21:26
> > Subject: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> > family
> >
> > Add SoC detection support for RZ/G3E SoC. Also add support for
> > detecting the number of cores and
> > ETHOS-U55 NPU and also detect PLL mismatch for SW settings other than
> 1.7GHz.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> >  drivers/soc/renesas/Kconfig          |  6 +++
> >  drivers/soc/renesas/Makefile         |  1 +
> >  drivers/soc/renesas/r9a09g047-sysc.c | 70 ++++++++++++++++++++++++++++
> >  drivers/soc/renesas/rz-sysc.c        | 44 +++++++++++------
> >  drivers/soc/renesas/rz-sysc.h        |  7 +++
> >  5 files changed, 114 insertions(+), 14 deletions(-)  create mode
> > 100644 drivers/soc/renesas/r9a09g047-sysc.c
> >
> > diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> > index
> > a792a3e915fe..9e46b0ee6e80 100644
> > --- a/drivers/soc/renesas/Kconfig
> > +++ b/drivers/soc/renesas/Kconfig
> > @@ -348,6 +348,7 @@ config ARCH_R9A09G011
> >
> >  config ARCH_R9A09G047
> >  	bool "ARM64 Platform support for RZ/G3E"
> > +	select SYSC_R9A09G047
> >  	help
> >  	  This enables support for the Renesas RZ/G3E SoC variants.
> >
> > @@ -386,9 +387,14 @@ config RST_RCAR
> >
> >  config SYSC_RZ
> >  	bool "System controller for RZ SoCs" if COMPILE_TEST
> > +	depends on MFD_SYSCON
> >
> >  config SYSC_R9A08G045
> >  	bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
> >  	select SYSC_RZ
> >
> > +config SYSC_R9A09G047
> > +	bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> > +	select SYSC_RZ
> > +
> >  endif # SOC_RENESAS
> > diff --git a/drivers/soc/renesas/Makefile
> > b/drivers/soc/renesas/Makefile index
> > 8cd139b3dd0a..3256706112d9 100644
> > --- a/drivers/soc/renesas/Makefile
> > +++ b/drivers/soc/renesas/Makefile
> > @@ -7,6 +7,7 @@ ifdef CONFIG_SMP
> >  obj-$(CONFIG_ARCH_R9A06G032)	+= r9a06g032-smp.o
> >  endif
> >  obj-$(CONFIG_SYSC_R9A08G045)	+= r9a08g045-sysc.o
> > +obj-$(CONFIG_SYSC_R9A09G047)	+= r9a09g047-sysc.o
> >
> >  # Family
> >  obj-$(CONFIG_PWC_RZV2M)		+= pwc-rzv2m.o
> > diff --git a/drivers/soc/renesas/r9a09g047-sysc.c
> > b/drivers/soc/renesas/r9a09g047-sysc.c
> > new file mode 100644
> > index 000000000000..32bdab9f1774
> > --- /dev/null
> > +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> > @@ -0,0 +1,70 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RZ/G3E System controller driver
> > + *
> > + * Copyright (C) 2024 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/device.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +
> > +#include "rz-sysc.h"
> > +
> > +/* Register definitions */
> > +#define SYS_LSI_DEVID	0x304
> > +#define SYS_LSI_MODE	0x300
> > +#define SYS_LSI_PRR	0x308
> > +#define SYS_LSI_DEVID_REV	GENMASK(31, 28)
> > +#define SYS_LSI_DEVID_SPECIFIC	GENMASK(27, 0)
> > +#define SYS_LSI_PRR_CA55_DIS	BIT(8)
> > +#define SYS_LSI_PRR_NPU_DIS	BIT(1)
> > +/*
> > + * BOOTPLLCA[1:0]
> > + *	[0,0] => 1.1GHZ
> > + *	[0,1] => 1.5GHZ
> > + *	[1,0] => 1.6GHZ
> > + *	[1,1] => 1.7GHZ
> > + */
> > +#define SYS_LSI_MODE_STAT_BOOTPLLCA55	GENMASK(12, 11)
> > +#define SYS_LSI_MODE_CA55_1_7GHz	0x3
> > +
> > +static void rzg3e_extended_device_identification(struct device *dev,
> > +				void __iomem *sysc_base,
> > +				struct soc_device_attribute *soc_dev_attr) {
> > +	u32 prr_val, mode_val;
> > +	bool is_quad_core, npu_enabled;
> > +
> > +	prr_val = readl(sysc_base + SYS_LSI_PRR);
> > +	mode_val = readl(sysc_base + SYS_LSI_MODE);
> > +
> > +	/* Check CPU and NPU configuration */
> > +	is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> > +	npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> > +
> > +	dev_info(dev, "Detected Renesas %s Core %s %s Rev %s  %s\n",
> > +		 is_quad_core ? "Quad" : "Dual",
> > +		 soc_dev_attr->family,
> > +		 soc_dev_attr->soc_id,
> > +		 soc_dev_attr->revision,
> > +		 npu_enabled ? "with Ethos-U55" : "");
> > +
> > +	/* Check CA55 PLL configuration */
> > +	if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) !=
> SYS_LSI_MODE_CA55_1_7GHz)
> > +		dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n"); }
> > +
> > +static const struct rz_sysc_soc_id_init_data
> rzg3e_sysc_soc_id_init_data __initconst = {
> > +	.family = "RZ/G3E",
> > +	.id = 0x8679447,
> > +	.offset = SYS_LSI_DEVID,
> > +	.revision_mask = SYS_LSI_DEVID_REV,
> > +	.specific_id_mask = SYS_LSI_DEVID_SPECIFIC,
> > +	.extended_device_identification =
> > +rzg3e_extended_device_identification,
> > +};
> > +
> > +const struct rz_sysc_init_data rzg3e_sysc_init_data = {
> > +	.soc_id_init_data = &rzg3e_sysc_soc_id_init_data, };
> > diff --git a/drivers/soc/renesas/rz-sysc.c
> > b/drivers/soc/renesas/rz-sysc.c index d34d295831b8..515eca249b6e
> > 100644
> > --- a/drivers/soc/renesas/rz-sysc.c
> > +++ b/drivers/soc/renesas/rz-sysc.c
> > @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc,
> > const struct of_device_id *mat
> >
> >  	soc_id_start = strchr(match->compatible, ',') + 1;
> >  	soc_id_end = strchr(match->compatible, '-');
> > -	size = soc_id_end - soc_id_start;
> > +	size = soc_id_end - soc_id_start + 1;
> >  	if (size > 32)
> >  		size = 32;
> >  	strscpy(soc_id, soc_id_start, size); @@ -257,8 +257,16 @@ static int
> > rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
> >  		return -ENODEV;
> >  	}
> >
> > -	dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n", soc_dev_attr-
> >family,
> > -		 soc_dev_attr->soc_id, soc_dev_attr->revision);
> > +	/* Try to call SoC-specific device identification */
> > +	if (soc_data->extended_device_identification) {
> > +		soc_data->extended_device_identification(sysc->dev, sysc-
> >base,
> > +							 soc_dev_attr);
> > +	} else {
> > +		dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n",
> > +			 soc_dev_attr->family,
> > +			 soc_dev_attr->soc_id,
> > +			 soc_dev_attr->revision);
> > +	}
> >
> >  	soc_dev = soc_device_register(soc_dev_attr);
> >  	if (IS_ERR(soc_dev))
> > @@ -283,6 +291,9 @@ static struct regmap_config rz_sysc_regmap = {
> > static const struct of_device_id rz_sysc_match[] = {  #ifdef
> CONFIG_SYSC_R9A08G045
> >  	{ .compatible = "renesas,r9a08g045-sysc", .data =
> > &rzg3s_sysc_init_data },
> > +#endif
> > +#ifdef CONFIG_SYSC_R9A09G047
> > +	{ .compatible = "renesas,r9a09g047-sys", .data =
> > +&rzg3e_sysc_init_data },
> >  #endif
> >  	{ }
> >  };
> > @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device
> *pdev)
> >  		return ret;
> >
> >  	data = match->data;
> > -	if (!data->max_register_offset)
> > -		return -EINVAL;
> > +	if (data->signals_init_data) {
> > +		if (!data->max_register_offset)
> > +			return -EINVAL;
> 
> I assume you don't have any signal use case like RZ/G3S.
> Can you please confirm is it correct for RZ/G3E?
There is no such concept of signal.
> 
> Multiple drivers/devices accessing same register like USB PWRRDY register
> in RZ/G3S?
There is no register shared between drivers/devices. They each have
their own respective registers.
> 
> Cheers,
> Biju

RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family

[John Madieu]

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Saturday, December 7, 2024 11:30 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Magnus Damm <magnus.damm@gmail.com>; Rob
> Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Claudiu Beznea
> <claudiu.beznea.uj@bp.renesas.com>
> Cc: john.madieu@gmail.com; linux-renesas-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; John Madieu
> <john.madieu.xa@bp.renesas.com>
> Subject: RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> family
> 
> Hi John,
> 
> Thanks for the patch.
> 
> > -----Original Message-----
> > From: John Madieu <john.madieu.xa@bp.renesas.com>
> > Sent: 06 December 2024 21:26
> > Subject: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> > family
> >
> > Add SoC detection support for RZ/G3E SoC. Also add support for
> > detecting the number of cores and
> > ETHOS-U55 NPU and also detect PLL mismatch for SW settings other than
> 1.7GHz.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> >
> >  	data = match->data;
> > -	if (!data->max_register_offset)
> > -		return -EINVAL;
> > +	if (data->signals_init_data) {
> > +		if (!data->max_register_offset)
> > +			return -EINVAL;
> >
> > -	ret = rz_sysc_signals_init(sysc, data->signals_init_data, data-
> >num_signals);
> > -	if (ret)
> > -		return ret;
> > +		ret = rz_sysc_signals_init(sysc, data->signals_init_data,
> data->num_signals);
> > +		if (ret)
> > +			return ret;
> > +
> > +		rz_sysc_regmap.max_register = data->max_register_offset;
> > +		dev_set_drvdata(dev, sysc);
> >
> > -	dev_set_drvdata(dev, sysc);
> > -	rz_sysc_regmap.max_register = data->max_register_offset;
> > -	regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> > -	if (IS_ERR(regmap))
> > -		return PTR_ERR(regmap);
> > +		regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> > +		if (IS_ERR(regmap))
> > +			return PTR_ERR(regmap);
> >
> > -	return of_syscon_register_regmap(dev->of_node, regmap);
> > +		return of_syscon_register_regmap(dev->of_node, regmap);
> 
> Basically if I understand correctly, you are going to use normal Syscon
> call for register access in PCIe and TSU drivers. Is it correct?
> 
> For example,
> 
> priv->syscon = syscon_regmap_lookup_by_phandle(np, "syscon");
Yes, that's the idea.
> 
> 
> regmap_read(priv->syscon,xxx)
> 
> 
> Cheers,
> Biju
Cheers.
> 
> 
> > +	}
> > +
> > +	return 0;
> >  }
> >
> >  static struct platform_driver rz_sysc_driver = { diff --git
> > a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h index
> > babca9c743c7..2b5ad41cef9e 100644
> > --- a/drivers/soc/renesas/rz-sysc.h
> > +++ b/drivers/soc/renesas/rz-sysc.h
> > @@ -8,7 +8,9 @@
> >  #ifndef __SOC_RENESAS_RZ_SYSC_H__
> >  #define __SOC_RENESAS_RZ_SYSC_H__
> >
> > +#include <linux/device.h>
> >  #include <linux/refcount.h>
> > +#include <linux/sys_soc.h>
> >  #include <linux/types.h>
> >
> >  /**
> > @@ -42,6 +44,7 @@ struct rz_sysc_signal {
> >   * @offset: SYSC SoC ID register offset
> >   * @revision_mask: SYSC SoC ID revision mask
> >   * @specific_id_mask: SYSC SoC ID specific ID mask
> > + * @extended_device_identification: SoC-specific extended device
> > + identification
> >   */
> >  struct rz_sysc_soc_id_init_data {
> >  	const char * const family;
> > @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
> >  	u32 offset;
> >  	u32 revision_mask;
> >  	u32 specific_id_mask;
> > +	void (*extended_device_identification)(struct device *dev,
> > +		void __iomem *sysc_base,
> > +		struct soc_device_attribute *soc_dev_attr);
> >  };
> >
> >  /**
> > @@ -65,6 +71,7 @@ struct rz_sysc_init_data {
> >  	u32 max_register_offset;
> >  };
> >
> > +extern const struct rz_sysc_init_data rzg3e_sysc_init_data;
> >  extern const struct rz_sysc_init_data rzg3s_sysc_init_data;
> >
> >  #endif /* __SOC_RENESAS_RZ_SYSC_H__ */
> > --
> > 2.25.1

RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family

[John Madieu]

> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> Sent: Saturday, December 7, 2024 11:22 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Magnus Damm <magnus.damm@gmail.com>; Rob
> Herring <robh@kernel.org>; Biju Das <biju.das.jz@bp.renesas.com>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Cc: john.madieu@gmail.com; linux-renesas-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> family
> 
> Hi, John,
Hello Claudiu,
> 
> On 06.12.2024 23:25, John Madieu wrote:
> > Add SoC detection support for RZ/G3E SoC. Also add support for
> > detecting the number of cores and ETHOS-U55 NPU and also detect PLL
> > mismatch for SW settings other than 1.7GHz.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> >  drivers/soc/renesas/Kconfig          |  6 +++
> >  drivers/soc/renesas/Makefile         |  1 +
> >  drivers/soc/renesas/r9a09g047-sysc.c | 70 ++++++++++++++++++++++++++++
> >  drivers/soc/renesas/rz-sysc.c        | 44 +++++++++++------
> >  drivers/soc/renesas/rz-sysc.h        |  7 +++
> >  5 files changed, 114 insertions(+), 14 deletions(-)  create mode
> > 100644 drivers/soc/renesas/r9a09g047-sysc.c
> >
> > diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> > index a792a3e915fe..9e46b0ee6e80 100644
> > --- a/drivers/soc/renesas/Kconfig
> > +++ b/drivers/soc/renesas/Kconfig
> > @@ -348,6 +348,7 @@ config ARCH_R9A09G011
> >
> >  config ARCH_R9A09G047
> >  	bool "ARM64 Platform support for RZ/G3E"
> > +	select SYSC_R9A09G047
> >  	help
> >  	  This enables support for the Renesas RZ/G3E SoC variants.
> >
> > @@ -386,9 +387,14 @@ config RST_RCAR
> >
> >  config SYSC_RZ
> >  	bool "System controller for RZ SoCs" if COMPILE_TEST
> > +	depends on MFD_SYSCON
> >
> >  config SYSC_R9A08G045
> >  	bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
> >  	select SYSC_RZ
> >
> > +config SYSC_R9A09G047
> > +	bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> > +	select SYSC_RZ
> > +
> >  endif # SOC_RENESAS
> > diff --git a/drivers/soc/renesas/Makefile
> > b/drivers/soc/renesas/Makefile index 8cd139b3dd0a..3256706112d9 100644
> > --- a/drivers/soc/renesas/Makefile
> > +++ b/drivers/soc/renesas/Makefile
> > @@ -7,6 +7,7 @@ ifdef CONFIG_SMP
> >  obj-$(CONFIG_ARCH_R9A06G032)	+= r9a06g032-smp.o
> >  endif
> >  obj-$(CONFIG_SYSC_R9A08G045)	+= r9a08g045-sysc.o
> > +obj-$(CONFIG_SYSC_R9A09G047)	+= r9a09g047-sysc.o
> >
> >  # Family
> >  obj-$(CONFIG_PWC_RZV2M)		+= pwc-rzv2m.o
> > diff --git a/drivers/soc/renesas/r9a09g047-sysc.c
> > b/drivers/soc/renesas/r9a09g047-sysc.c
> > new file mode 100644
> > index 000000000000..32bdab9f1774
> > --- /dev/null
> > +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> > @@ -0,0 +1,70 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RZ/G3E System controller driver
> > + *
> > + * Copyright (C) 2024 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/device.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +
> > +#include "rz-sysc.h"
> > +
> > +/* Register definitions */
> > +#define SYS_LSI_DEVID	0x304
> > +#define SYS_LSI_MODE	0x300
> > +#define SYS_LSI_PRR	0x308
> > +#define SYS_LSI_DEVID_REV	GENMASK(31, 28)
> > +#define SYS_LSI_DEVID_SPECIFIC	GENMASK(27, 0)
> > +#define SYS_LSI_PRR_CA55_DIS	BIT(8)
> > +#define SYS_LSI_PRR_NPU_DIS	BIT(1)
> > +/*
> > + * BOOTPLLCA[1:0]
> > + *	[0,0] => 1.1GHZ
> > + *	[0,1] => 1.5GHZ
> > + *	[1,0] => 1.6GHZ
> > + *	[1,1] => 1.7GHZ
> > + */
> > +#define SYS_LSI_MODE_STAT_BOOTPLLCA55	GENMASK(12, 11)
> > +#define SYS_LSI_MODE_CA55_1_7GHz	0x3
> > +
> > +static void rzg3e_extended_device_identification(struct device *dev,
> > +				void __iomem *sysc_base,
> > +				struct soc_device_attribute *soc_dev_attr)
> 
> Not strong preference here, I think it can still be aligned to (
Noted. Thanks!
> 
> > +{
> > +	u32 prr_val, mode_val;
> > +	bool is_quad_core, npu_enabled;
> 
> Reverse christmass tree order?
Will swap it in v2.
> 
> > +
> > +	prr_val = readl(sysc_base + SYS_LSI_PRR);
> > +	mode_val = readl(sysc_base + SYS_LSI_MODE);
> > +
> > +	/* Check CPU and NPU configuration */
> > +	is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> > +	npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> > +
> > +	dev_info(dev, "Detected Renesas %s Core %s %s Rev %s  %s\n",
> 
> I think you have an extra space towards the end: "%s  %s"
Thanks for pointing it out.
> 
> > +		 is_quad_core ? "Quad" : "Dual",
> > +		 soc_dev_attr->family,
> > +		 soc_dev_attr->soc_id,
> > +		 soc_dev_attr->revision,
> > +		 npu_enabled ? "with Ethos-U55" : "");
> > +
> > +	/* Check CA55 PLL configuration */
> > +	if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) !=
> SYS_LSI_MODE_CA55_1_7GHz)
> > +		dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n"); }
> > +
> > +static const struct rz_sysc_soc_id_init_data
> rzg3e_sysc_soc_id_init_data __initconst = {
> > +	.family = "RZ/G3E",
> > +	.id = 0x8679447,
> > +	.offset = SYS_LSI_DEVID,
> > +	.revision_mask = SYS_LSI_DEVID_REV,
> > +	.specific_id_mask = SYS_LSI_DEVID_SPECIFIC,
> > +	.extended_device_identification =
> > +rzg3e_extended_device_identification,
> > +};
> > +
> > +const struct rz_sysc_init_data rzg3e_sysc_init_data = {
> > +	.soc_id_init_data = &rzg3e_sysc_soc_id_init_data, };
> > diff --git a/drivers/soc/renesas/rz-sysc.c
> > b/drivers/soc/renesas/rz-sysc.c index d34d295831b8..515eca249b6e
> > 100644
> > --- a/drivers/soc/renesas/rz-sysc.c
> > +++ b/drivers/soc/renesas/rz-sysc.c
> > @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc,
> > const struct of_device_id *mat
> >
> >  	soc_id_start = strchr(match->compatible, ',') + 1;
> >  	soc_id_end = strchr(match->compatible, '-');
> > -	size = soc_id_end - soc_id_start;
> > +	size = soc_id_end - soc_id_start + 1;
> 
> This may worth a different patch.
Got it.
> 
> >  	if (size > 32)
> >  		size = 32;
> >  	strscpy(soc_id, soc_id_start, size); @@ -257,8 +257,16 @@ static int
> > rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
> >  		return -ENODEV;
> >  	}
> >
> > -	dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n", soc_dev_attr-
> >family,
> > -		 soc_dev_attr->soc_id, soc_dev_attr->revision);
> > +	/* Try to call SoC-specific device identification */
> > +	if (soc_data->extended_device_identification) {
> > +		soc_data->extended_device_identification(sysc->dev, sysc-
> >base,
> > +							 soc_dev_attr);
> > +	} else {
> > +		dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n",
> > +			 soc_dev_attr->family,
> > +			 soc_dev_attr->soc_id,
> > +			 soc_dev_attr->revision);
> > +	}
> >
> >  	soc_dev = soc_device_register(soc_dev_attr);
> >  	if (IS_ERR(soc_dev))
> > @@ -283,6 +291,9 @@ static struct regmap_config rz_sysc_regmap = {
> > static const struct of_device_id rz_sysc_match[] = {  #ifdef
> > CONFIG_SYSC_R9A08G045
> >  	{ .compatible = "renesas,r9a08g045-sysc", .data =
> > &rzg3s_sysc_init_data },
> > +#endif
> > +#ifdef CONFIG_SYSC_R9A09G047
> > +	{ .compatible = "renesas,r9a09g047-sys", .data =
> > +&rzg3e_sysc_init_data },
> >  #endif
> >  	{ }
> >  };
> > @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device
> *pdev)
> >  		return ret;
> >
> >  	data = match->data;
> > -	if (!data->max_register_offset)
> > -		return -EINVAL;
> 
> The idea with this was to still have the syscon regmap registered no
> matter the signals are available or not. This may be needed for other use
> cases.
> 
> > +	if (data->signals_init_data) {
> 
> I'd prefer to have this check in rz_sysc_signals_init(). In this way you
> have everything signal init specific in a single function.
The idea here is to avoid calling rz_sysc_signals_init() for drivers that
don't need it instead of always calling it, and potentially dealing with
errors due to drivers not supporting it, instead of the function actually
failing.
> 
> > +		if (!data->max_register_offset)
> > +			return -EINVAL;
> >
> > -	ret = rz_sysc_signals_init(sysc, data->signals_init_data, data-
> >num_signals);
> > -	if (ret)
> > -		return ret;
> > +		ret = rz_sysc_signals_init(sysc, data->signals_init_data,
> data->num_signals);
> > +		if (ret)
> > +			return ret;
> > +
> > +		rz_sysc_regmap.max_register = data->max_register_offset;
> > +		dev_set_drvdata(dev, sysc);
> 
> Why changed the initial order?
No special reason. Will be reverted.
> 
> Thank you,
> Claudiu
Thanks!
> 
> >
> > -	dev_set_drvdata(dev, sysc);
> > -	rz_sysc_regmap.max_register = data->max_register_offset;
> > -	regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> > -	if (IS_ERR(regmap))
> > -		return PTR_ERR(regmap);
> > +		regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> > +		if (IS_ERR(regmap))
> > +			return PTR_ERR(regmap);
> >
> > -	return of_syscon_register_regmap(dev->of_node, regmap);
> > +		return of_syscon_register_regmap(dev->of_node, regmap);
> > +	}
> > +
> > +	return 0;
> >  }
> >
> >  static struct platform_driver rz_sysc_driver = { diff --git
> > a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h index
> > babca9c743c7..2b5ad41cef9e 100644
> > --- a/drivers/soc/renesas/rz-sysc.h
> > +++ b/drivers/soc/renesas/rz-sysc.h
> > @@ -8,7 +8,9 @@
> >  #ifndef __SOC_RENESAS_RZ_SYSC_H__
> >  #define __SOC_RENESAS_RZ_SYSC_H__
> >
> > +#include <linux/device.h>
> >  #include <linux/refcount.h>
> > +#include <linux/sys_soc.h>
> >  #include <linux/types.h>
> >
> >  /**
> > @@ -42,6 +44,7 @@ struct rz_sysc_signal {
> >   * @offset: SYSC SoC ID register offset
> >   * @revision_mask: SYSC SoC ID revision mask
> >   * @specific_id_mask: SYSC SoC ID specific ID mask
> > + * @extended_device_identification: SoC-specific extended device
> > + identification
> >   */
> >  struct rz_sysc_soc_id_init_data {
> >  	const char * const family;
> > @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
> >  	u32 offset;
> >  	u32 revision_mask;
> >  	u32 specific_id_mask;
> > +	void (*extended_device_identification)(struct device *dev,
> > +		void __iomem *sysc_base,
> > +		struct soc_device_attribute *soc_dev_attr);
> >  };
> >
> >  /**
> > @@ -65,6 +71,7 @@ struct rz_sysc_init_data {
> >  	u32 max_register_offset;
> >  };
> >
> > +extern const struct rz_sysc_init_data rzg3e_sysc_init_data;
> >  extern const struct rz_sysc_init_data rzg3s_sysc_init_data;
> >
> >  #endif /* __SOC_RENESAS_RZ_SYSC_H__ */

Re: [PATCH 2/5] dt-bindings: soc: renesas: Document Renesas RZ/G3E SoC variants

[Rob Herring]

On Fri, Dec 06, 2024 at 10:25:56PM +0100, John Madieu wrote:
> Document RZ/G3E (R9A09G047) SoC variants.

Your subject could be improved so that it doesn't match this one:

https://lore.kernel.org/all/20241203105005.103927-3-biju.das.jz@bp.renesas.com/

Which threw off my tools...

> 
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
>  .../devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml   | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 0/5] soc: renesas: Add system controller support for RZ/G3E SoC

[Rob Herring]

On Fri, Dec 06, 2024 at 10:25:54PM +0100, John Madieu wrote:
> This patch series adds support for the RZ/G3E system controller and extends
> the existing RZ/V2H(P) system controller to support syscon. The RZ/G3E
> system controller allows detecting various SoC features like core count,
> NPU availability, and CA55 PLL configuration.
> 
> Key features:
> - Syscon support for both RZ/V2H and RZ/G3E system controllers
> - Detection of quad/dual core configuration
> - Detection of Ethos-U55 NPU presence
> - Validation of CA55 PLL frequency setting
> - SoC-specific extended identification through callbacks

This series and some other questions about syscon prompted me to look 
into syscon driver a bit. Consider this resulting series[1] in context 
with your changes.

Rob

[1] https://lore.kernel.org/all/20241211-syscon-fixes-v1-3-b5ac8c219e96@kernel.org/

RE: [PATCH 2/5] dt-bindings: soc: renesas: Document Renesas RZ/G3E SoC variants

[John Madieu]

Hi Rob,

Thanks for the review.

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, December 10, 2024 10:56 PM
> Subject: Re: [PATCH 2/5] dt-bindings: soc: renesas: Document Renesas
> RZ/G3E SoC variants
> 
> On Fri, Dec 06, 2024 at 10:25:56PM +0100, John Madieu wrote:
> > Document RZ/G3E (R9A09G047) SoC variants.
> 
> Your subject could be improved so that it doesn't match this one:


Sorry for that, will fix it in the next version.
Thanks,

John 
> 
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> >  .../devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml   | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 0/5] soc: renesas: Add system controller support for RZ/G3E SoC

[Geert Uytterhoeven]

Hi Rob,

On Wed, Dec 11, 2024 at 10:08 PM Rob Herring <robh@kernel.org> wrote:
> On Fri, Dec 06, 2024 at 10:25:54PM +0100, John Madieu wrote:
> > This patch series adds support for the RZ/G3E system controller and extends
> > the existing RZ/V2H(P) system controller to support syscon. The RZ/G3E
> > system controller allows detecting various SoC features like core count,
> > NPU availability, and CA55 PLL configuration.
> >
> > Key features:
> > - Syscon support for both RZ/V2H and RZ/G3E system controllers
> > - Detection of quad/dual core configuration
> > - Detection of Ethos-U55 NPU presence
> > - Validation of CA55 PLL frequency setting
> > - SoC-specific extended identification through callbacks
>
> This series and some other questions about syscon prompted me to look
> into syscon driver a bit. Consider this resulting series[1] in context
> with your changes.
>
> Rob
>
> [1] https://lore.kernel.org/all/20241211-syscon-fixes-v1-3-b5ac8c219e96@kernel.org/

Thank you, not having to add a "syscon" compatible value makes
perfect sense!

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 2/5] dt-bindings: soc: renesas: Document Renesas RZ/G3E SoC variants

[Geert Uytterhoeven]

On Fri, Dec 6, 2024 at 10:26 PM John Madieu
<john.madieu.xa@bp.renesas.com> wrote:
> Document RZ/G3E (R9A09G047) SoC variants.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>

With the subject and description fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family

[Geert Uytterhoeven]

Hi John,

On Fri, Dec 6, 2024 at 10:26 PM John Madieu
<john.madieu.xa@bp.renesas.com> wrote:
> Add SoC detection support for RZ/G3E SoC. Also add support for detecting the
> number of cores and ETHOS-U55 NPU and also detect PLL mismatch for SW settings
> other than 1.7GHz.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -348,6 +348,7 @@ config ARCH_R9A09G011
>
>  config ARCH_R9A09G047
>         bool "ARM64 Platform support for RZ/G3E"
> +       select SYSC_R9A09G047
>         help
>           This enables support for the Renesas RZ/G3E SoC variants.
>
> @@ -386,9 +387,14 @@ config RST_RCAR
>
>  config SYSC_RZ
>         bool "System controller for RZ SoCs" if COMPILE_TEST
> +       depends on MFD_SYSCON

WARNING: unmet direct dependencies detected for SYSC_RZ
  Depends on [n]: SOC_RENESAS [=y] && MFD_SYSCON [=n]
  Selected by [y]:
  - SYSC_R9A08G045 [=y] && SOC_RENESAS [=y]
  - SYSC_R9A09G047 [=y] && SOC_RENESAS [=y]

So please select MFD_SYSCON instead.

>
>  config SYSC_R9A08G045
>         bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
>         select SYSC_RZ
>
> +config SYSC_R9A09G047
> +       bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> +       select SYSC_RZ
> +
>  endif # SOC_RENESAS

> --- /dev/null
> +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RZ/G3E System controller driver
> + *
> + * Copyright (C) 2024 Renesas Electronics Corp.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +
> +#include "rz-sysc.h"
> +
> +/* Register definitions */
> +#define SYS_LSI_DEVID  0x304
> +#define SYS_LSI_MODE   0x300
> +#define SYS_LSI_PRR    0x308
> +#define SYS_LSI_DEVID_REV      GENMASK(31, 28)
> +#define SYS_LSI_DEVID_SPECIFIC GENMASK(27, 0)
> +#define SYS_LSI_PRR_CA55_DIS   BIT(8)
> +#define SYS_LSI_PRR_NPU_DIS    BIT(1)
> +/*
> + * BOOTPLLCA[1:0]
> + *     [0,0] => 1.1GHZ
> + *     [0,1] => 1.5GHZ
> + *     [1,0] => 1.6GHZ
> + *     [1,1] => 1.7GHZ
> + */
> +#define SYS_LSI_MODE_STAT_BOOTPLLCA55  GENMASK(12, 11)
> +#define SYS_LSI_MODE_CA55_1_7GHz       0x3

Please align the second column, and please group register offsets
and bits together.

> +
> +static void rzg3e_extended_device_identification(struct device *dev,
> +                               void __iomem *sysc_base,
> +                               struct soc_device_attribute *soc_dev_attr)
> +{
> +       u32 prr_val, mode_val;
> +       bool is_quad_core, npu_enabled;
> +
> +       prr_val = readl(sysc_base + SYS_LSI_PRR);
> +       mode_val = readl(sysc_base + SYS_LSI_MODE);
> +
> +       /* Check CPU and NPU configuration */
> +       is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> +       npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> +
> +       dev_info(dev, "Detected Renesas %s Core %s %s Rev %s  %s\n",

There are two spaces before the last %s.
Please drop both spaces...

> +                is_quad_core ? "Quad" : "Dual",
> +                soc_dev_attr->family,
> +                soc_dev_attr->soc_id,
> +                soc_dev_attr->revision,
> +                npu_enabled ? "with Ethos-U55" : "");

... and add one space here, just before "with".

> +
> +       /* Check CA55 PLL configuration */
> +       if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) != SYS_LSI_MODE_CA55_1_7GHz)
> +               dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n");

Just wondering: is that a problem? Can't it be handled in the clock
driver?

> +}

> --- a/drivers/soc/renesas/rz-sysc.c
> +++ b/drivers/soc/renesas/rz-sysc.c
> @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
>
>         soc_id_start = strchr(match->compatible, ',') + 1;
>         soc_id_end = strchr(match->compatible, '-');
> -       size = soc_id_end - soc_id_start;
> +       size = soc_id_end - soc_id_start + 1;

Unrelated fix?

>         if (size > 32)
>                 size = 32;
>         strscpy(soc_id, soc_id_start, size);

> @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device *pdev)
>                 return ret;
>
>         data = match->data;
> -       if (!data->max_register_offset)
> -               return -EINVAL;
> +       if (data->signals_init_data) {

if (!data->signals_init_data)
        return 0;

> +               if (!data->max_register_offset)
> +                       return -EINVAL;
>
> -       ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> -       if (ret)
> -               return ret;
> +               ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> +               if (ret)
> +                       return ret;
> +
> +               rz_sysc_regmap.max_register = data->max_register_offset;
> +               dev_set_drvdata(dev, sysc);
>
> -       dev_set_drvdata(dev, sysc);
> -       rz_sysc_regmap.max_register = data->max_register_offset;
> -       regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> -       if (IS_ERR(regmap))
> -               return PTR_ERR(regmap);
> +               regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> +               if (IS_ERR(regmap))
> +                       return PTR_ERR(regmap);
>
> -       return of_syscon_register_regmap(dev->of_node, regmap);
> +               return of_syscon_register_regmap(dev->of_node, regmap);
> +       }
> +
> +       return 0;
>  }
>
>  static struct platform_driver rz_sysc_driver = {

> --- a/drivers/soc/renesas/rz-sysc.h
> +++ b/drivers/soc/renesas/rz-sysc.h
> @@ -42,6 +44,7 @@ struct rz_sysc_signal {
>   * @offset: SYSC SoC ID register offset
>   * @revision_mask: SYSC SoC ID revision mask
>   * @specific_id_mask: SYSC SoC ID specific ID mask
> + * @extended_device_identification: SoC-specific extended device identification
>   */
>  struct rz_sysc_soc_id_init_data {
>         const char * const family;
> @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
>         u32 offset;
>         u32 revision_mask;
>         u32 specific_id_mask;
> +       void (*extended_device_identification)(struct device *dev,
> +               void __iomem *sysc_base,
> +               struct soc_device_attribute *soc_dev_attr);

That's a rather long name...

>  };
>
>  /**

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 0/5] soc: renesas: Add system controller support for RZ/G3E SoC

[Geert Uytterhoeven]

Hi John,

On Fri, Dec 6, 2024 at 10:26 PM John Madieu
<john.madieu.xa@bp.renesas.com> wrote:
> This patch series adds support for the RZ/G3E system controller and extends
> the existing RZ/V2H(P) system controller to support syscon. The RZ/G3E
> system controller allows detecting various SoC features like core count,
> NPU availability, and CA55 PLL configuration.
>
> Key features:
> - Syscon support for both RZ/V2H and RZ/G3E system controllers
> - Detection of quad/dual core configuration
> - Detection of Ethos-U55 NPU presence
> - Validation of CA55 PLL frequency setting
> - SoC-specific extended identification through callbacks

Thanks for your series!

> This patch series depends upon [1] and [2].
>
> Tested:
> - Example of SoC detection:
> [    0.065608] renesas-rz-sysc 10430000.system-controller: Detected Renesas Quad Core RZ/G3E r9a09g047 Rev 0  with Ethos-U55
> - Example of PLL misconfiguration warning:
>  [    0.065616] renesas-rz-sysc 10430000.system-controller: CA55 PLL is not set to 1.7GHz
>
> [1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=914097
> [2] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=912697

The latter points to an already-applied unrelated series.  I assume you
meant "[PATCH v2 00/15] Add initial USB support for the Renesas RZ/G3S
SoC"[3]?

[3] https://lore.kernel.org/all/20241126092050.1825607-1-claudiu.beznea.uj@bp.renesas.com/


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 4/5] arm64: dts: renesas: r9a09g047: add sys node

[Geert Uytterhoeven]

Hi John,

On Fri, Dec 6, 2024 at 10:26 PM John Madieu
<john.madieu.xa@bp.renesas.com> wrote:
> Add system controller node to RZ/G3E (R9A09G047) SoC DTSI.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> @@ -154,6 +154,13 @@ cpg: clock-controller@10420000 {
>                         #power-domain-cells = <0>;
>                 };
>
> +               sys: system-controller@10430000 {
> +                       compatible = "renesas,r9a09g047-sys", "syscon";

The "syscon" may be dropped again depending on the outcome of
Rob's series, we'll see...

> +                       reg = <0 0x10430000 0 0x10000>;
> +                       clocks = <&cpg CPG_CORE R9A09G047_SYS_0_PCLK>;
> +                       resets = <&cpg 0x30>;
> +               };
> +
>                 scif0: serial@11c01400 {
>                         compatible = "renesas,scif-r9a09g047", "renesas,scif-r9a09g057";
>                         reg = <0 0x11c01400 0 0x400>;

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 2/5] dt-bindings: soc: renesas: Document Renesas RZ/G3E SoC variants

[John Madieu]

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Friday, December 13, 2024 5:06 PM
> Subject: Re: [PATCH 2/5] dt-bindings: soc: renesas: Document Renesas
> RZ/G3E SoC variants
> 
> On Fri, Dec 6, 2024 at 10:26 PM John Madieu
> <john.madieu.xa@bp.renesas.com> wrote:
> > Document RZ/G3E (R9A09G047) SoC variants.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> 
> With the subject and description fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Gr{oetje,eeting}s,
> 
>                         Geert

Hi Geert,

Will be fixed in v2.
Thanks for the review.

> 
> --
> 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 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family

[John Madieu]

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Friday, December 13, 2024 5:25 PM
> Subject: Re: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> family
> 
> Hi John,

Hello Geert,

> 
> On Fri, Dec 6, 2024 at 10:26 PM John Madieu
> <john.madieu.xa@bp.renesas.com> wrote:
> > Add SoC detection support for RZ/G3E SoC. Also add support for
> > detecting the number of cores and ETHOS-U55 NPU and also detect PLL
> > mismatch for SW settings other than 1.7GHz.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/soc/renesas/Kconfig
> > +++ b/drivers/soc/renesas/Kconfig
> > @@ -348,6 +348,7 @@ config ARCH_R9A09G011
> >
> >  config ARCH_R9A09G047
> >         bool "ARM64 Platform support for RZ/G3E"
> > +       select SYSC_R9A09G047
> >         help
> >           This enables support for the Renesas RZ/G3E SoC variants.
> >
> > @@ -386,9 +387,14 @@ config RST_RCAR
> >
> >  config SYSC_RZ
> >         bool "System controller for RZ SoCs" if COMPILE_TEST
> > +       depends on MFD_SYSCON
> 
> WARNING: unmet direct dependencies detected for SYSC_RZ
>   Depends on [n]: SOC_RENESAS [=y] && MFD_SYSCON [=n]
>   Selected by [y]:
>   - SYSC_R9A08G045 [=y] && SOC_RENESAS [=y]
>   - SYSC_R9A09G047 [=y] && SOC_RENESAS [=y]
> 
> So please select MFD_SYSCON instead.

Will be done in v2.

> 
> >
> >  config SYSC_R9A08G045
> >         bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
> >         select SYSC_RZ
> >
> > +config SYSC_R9A09G047
> > +       bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> > +       select SYSC_RZ
> > +
> >  endif # SOC_RENESAS
> 
> > --- /dev/null
> > +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> > @@ -0,0 +1,70 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RZ/G3E System controller driver
> > + *
> > + * Copyright (C) 2024 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/device.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +
> > +#include "rz-sysc.h"
> > +
> > +/* Register definitions */
> > +#define SYS_LSI_DEVID  0x304
> > +#define SYS_LSI_MODE   0x300
> > +#define SYS_LSI_PRR    0x308
> > +#define SYS_LSI_DEVID_REV      GENMASK(31, 28)
> > +#define SYS_LSI_DEVID_SPECIFIC GENMASK(27, 0)
> > +#define SYS_LSI_PRR_CA55_DIS   BIT(8)
> > +#define SYS_LSI_PRR_NPU_DIS    BIT(1)
> > +/*
> > + * BOOTPLLCA[1:0]
> > + *     [0,0] => 1.1GHZ
> > + *     [0,1] => 1.5GHZ
> > + *     [1,0] => 1.6GHZ
> > + *     [1,1] => 1.7GHZ
> > + */
> > +#define SYS_LSI_MODE_STAT_BOOTPLLCA55  GENMASK(12, 11)
> > +#define SYS_LSI_MODE_CA55_1_7GHz       0x3
> 
> Please align the second column, and please group register offsets and bits
> together.

Noted.

> 
> > +
> > +static void rzg3e_extended_device_identification(struct device *dev,
> > +                               void __iomem *sysc_base,
> > +                               struct soc_device_attribute
> > +*soc_dev_attr) {
> > +       u32 prr_val, mode_val;
> > +       bool is_quad_core, npu_enabled;
> > +
> > +       prr_val = readl(sysc_base + SYS_LSI_PRR);
> > +       mode_val = readl(sysc_base + SYS_LSI_MODE);
> > +
> > +       /* Check CPU and NPU configuration */
> > +       is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> > +       npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> > +
> > +       dev_info(dev, "Detected Renesas %s Core %s %s Rev %s  %s\n",
> 
> There are two spaces before the last %s.
> Please drop both spaces...
> 
> > +                is_quad_core ? "Quad" : "Dual",
> > +                soc_dev_attr->family,
> > +                soc_dev_attr->soc_id,
> > +                soc_dev_attr->revision,
> > +                npu_enabled ? "with Ethos-U55" : "");
> 
> ... and add one space here, just before "with".

Thanks for the hint.

> 
> > +
> > +       /* Check CA55 PLL configuration */
> > +       if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) !=
> SYS_LSI_MODE_CA55_1_7GHz)
> > +               dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n");
> 
> Just wondering: is that a problem? Can't it be handled in the clock
> driver?

The current implementation supports only 1.7GHz, /2, /4, /8 frequencies,
hence, the warning.

> 
> > +}
> 
> > --- a/drivers/soc/renesas/rz-sysc.c
> > +++ b/drivers/soc/renesas/rz-sysc.c
> > @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc,
> > const struct of_device_id *mat
> >
> >         soc_id_start = strchr(match->compatible, ',') + 1;
> >         soc_id_end = strchr(match->compatible, '-');
> > -       size = soc_id_end - soc_id_start;
> > +       size = soc_id_end - soc_id_start + 1;
> 
> Unrelated fix?

I guess so. Was thinking of moving it in a separate patch in the v2
as suggested by Claudiu.

> 
> >         if (size > 32)
> >                 size = 32;
> >         strscpy(soc_id, soc_id_start, size);
> 
> > @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device
> *pdev)
> >                 return ret;
> >
> >         data = match->data;
> > -       if (!data->max_register_offset)
> > -               return -EINVAL;
> > +       if (data->signals_init_data) {
> 
> if (!data->signals_init_data)
>         return 0;

Thanks for the hint.

> 
> > +               if (!data->max_register_offset)
> > +                       return -EINVAL;
> >
> > -       ret = rz_sysc_signals_init(sysc, data->signals_init_data, data-
> >num_signals);
> > -       if (ret)
> > -               return ret;
> > +               ret = rz_sysc_signals_init(sysc, data-
> >signals_init_data, data->num_signals);
> > +               if (ret)
> > +                       return ret;
> > +
> > +               rz_sysc_regmap.max_register = data->max_register_offset;
> > +               dev_set_drvdata(dev, sysc);
> >
> > -       dev_set_drvdata(dev, sysc);
> > -       rz_sysc_regmap.max_register = data->max_register_offset;
> > -       regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> > -       if (IS_ERR(regmap))
> > -               return PTR_ERR(regmap);
> > +               regmap = devm_regmap_init(dev, NULL, sysc,
> &rz_sysc_regmap);
> > +               if (IS_ERR(regmap))
> > +                       return PTR_ERR(regmap);
> >
> > -       return of_syscon_register_regmap(dev->of_node, regmap);
> > +               return of_syscon_register_regmap(dev->of_node, regmap);
> > +       }
> > +
> > +       return 0;
> >  }
> >
> >  static struct platform_driver rz_sysc_driver = {
> 
> > --- a/drivers/soc/renesas/rz-sysc.h
> > +++ b/drivers/soc/renesas/rz-sysc.h
> > @@ -42,6 +44,7 @@ struct rz_sysc_signal {
> >   * @offset: SYSC SoC ID register offset
> >   * @revision_mask: SYSC SoC ID revision mask
> >   * @specific_id_mask: SYSC SoC ID specific ID mask
> > + * @extended_device_identification: SoC-specific extended device
> > + identification
> >   */
> >  struct rz_sysc_soc_id_init_data {
> >         const char * const family;
> > @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
> >         u32 offset;
> >         u32 revision_mask;
> >         u32 specific_id_mask;
> > +       void (*extended_device_identification)(struct device *dev,
> > +               void __iomem *sysc_base,
> > +               struct soc_device_attribute *soc_dev_attr);
> 
> That's a rather long name...

Will be shortened in v2. I'm thinking of ext_dev_id().

> 
> >  };
> >
> >  /**
> 
> 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 0/5] soc: renesas: Add system controller support for RZ/G3E SoC

[John Madieu]

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Friday, December 13, 2024 5:25 PM
> Subject: Re: [PATCH 0/5] soc: renesas: Add system controller support for
> RZ/G3E SoC
> 
> Hi John,

Hello Geert,

> 
> On Fri, Dec 6, 2024 at 10:26 PM John Madieu
> <john.madieu.xa@bp.renesas.com> wrote:
> > This patch series adds support for the RZ/G3E system controller and
> > extends the existing RZ/V2H(P) system controller to support syscon.
> > The RZ/G3E system controller allows detecting various SoC features
> > like core count, NPU availability, and CA55 PLL configuration.
> >
> > Key features:
> > - Syscon support for both RZ/V2H and RZ/G3E system controllers
> > - Detection of quad/dual core configuration
> > - Detection of Ethos-U55 NPU presence
> > - Validation of CA55 PLL frequency setting
> > - SoC-specific extended identification through callbacks
> 
> Thanks for your series!
> 
> > This patch series depends upon [1] and [2].
> >
> > Tested:
> > - Example of SoC detection:
> > [    0.065608] renesas-rz-sysc 10430000.system-controller: Detected
> Renesas Quad Core RZ/G3E r9a09g047 Rev 0  with Ethos-U55
> > - Example of PLL misconfiguration warning:
> >  [    0.065616] renesas-rz-sysc 10430000.system-controller: CA55 PLL is
> not set to 1.7GHz
> >
> 
> The latter points to an already-applied unrelated series.  I assume you
> meant "[PATCH v2 00/15] Add initial USB support for the Renesas RZ/G3S
> SoC"[3]?

You are right about [3], as it's the series that adds initial support for
the SYSC driver.

> 
> 
> 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 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family

[Geert Uytterhoeven]

Hi John,

On Sat, Dec 14, 2024 at 5:36 AM John Madieu
<john.madieu.xa@bp.renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Fri, Dec 6, 2024 at 10:26 PM John Madieu
> > <john.madieu.xa@bp.renesas.com> wrote:
> > > Add SoC detection support for RZ/G3E SoC. Also add support for
> > > detecting the number of cores and ETHOS-U55 NPU and also detect PLL
> > > mismatch for SW settings other than 1.7GHz.
> > >
> > > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>

> > > --- a/drivers/soc/renesas/rz-sysc.h
> > > +++ b/drivers/soc/renesas/rz-sysc.h
> > > @@ -42,6 +44,7 @@ struct rz_sysc_signal {
> > >   * @offset: SYSC SoC ID register offset
> > >   * @revision_mask: SYSC SoC ID revision mask
> > >   * @specific_id_mask: SYSC SoC ID specific ID mask
> > > + * @extended_device_identification: SoC-specific extended device
> > > + identification
> > >   */
> > >  struct rz_sysc_soc_id_init_data {
> > >         const char * const family;
> > > @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
> > >         u32 offset;
> > >         u32 revision_mask;
> > >         u32 specific_id_mask;
> > > +       void (*extended_device_identification)(struct device *dev,
> > > +               void __iomem *sysc_base,
> > > +               struct soc_device_attribute *soc_dev_attr);
> >
> > That's a rather long name...
>
> Will be shortened in v2. I'm thinking of ext_dev_id().

What about print_id() or print_ext_id(), which is what the function really does?

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

[PATCH v2 0/4] soc: renesas: Add system controller support for RZ/G3E SoC

[John Madieu]

This patch series adds support for the RZ/G3E system controller and extends
the existing RZ/V2H(P) system controller to support syscon. The RZ/G3E
system controller allows detecting various SoC features like core count,
NPU availability, and CA55 PLL configuration.

Changes in v2:
- Fixed code style issues in rz-sysc.c and r9a09g047-sysc.c
- Fixed device tree documentation, getting rid of syscon compatible string
- Handled non signal-aware readable/writeable regmap callback
- Consolidated common code between RZ/V2H and RZ/G3E drivers
- Moved SoC ID detection from the compatible string fix into a new patch

Key features:
- Syscon support for both RZ/V2H and RZ/G3E system controllers
- Detection of quad/dual core configuration
- Detection of Ethos-U55 NPU presence
- Validation of CA55 PLL frequency setting
- SoC-specific extended identification through callbacks

This patch series depends upon [1], [2], and [3].

Tested:
- Example of SoC detection:
[    0.065608] renesas-rz-sysc 10430000.system-controller: Detected Renesas 
Quad Core RZ/G3E r9a09g047 Rev 0  with Ethos-U55

- Example of PLL misconfiguration warning:
[    0.065616] renesas-rz-sysc 10430000.system-controller: CA55 PLL is not 
set to 1.7GHz

[1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=914097
[2] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=912455
[3] https://lore.kernel.org/lkml/Z2HTAJmBeIUlWysh@google.com/T/

John Madieu (4):
  dt-bindings: soc: renesas: Add RZ/G3E variant SYS bindings
  soc: renesas: rz-sysc: Fix SoC ID string extraction
  soc: renesas: rz-sysc: Add support for RZ/G3E family
  arm64: dts: renesas: r9a09g047: add sys node

 .../soc/renesas/renesas,r9a09g057-sys.yaml    |  5 +-
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi    |  7 ++
 drivers/soc/renesas/Kconfig                   |  6 ++
 drivers/soc/renesas/Makefile                  |  1 +
 drivers/soc/renesas/r9a09g047-sysc.c          | 73 +++++++++++++++++++
 drivers/soc/renesas/rz-sysc.c                 | 24 +++++-
 drivers/soc/renesas/rz-sysc.h                 |  6 ++
 7 files changed, 118 insertions(+), 4 deletions(-)
 create mode 100644 drivers/soc/renesas/r9a09g047-sysc.c

-- 
2.25.1

[PATCH v2 1/4] dt-bindings: soc: renesas: Add RZ/G3E variant SYS bindings

[John Madieu]

Add RZ/G3E (R9A09G047) variant to the existing RZ/V2H System
Controller (SYS) binding as both IPs are compatible.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
Changes:
 - v1 -> v2: Do not rely on syscon compatible string anymore

Notes:
    v1: Acked-by: Rob Herring (Arm) <robh@kernel.org>
    v1: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
    v2: Tags dropped due to small changes in compatible property structure.

 .../bindings/soc/renesas/renesas,r9a09g057-sys.yaml          | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml
index ebbf0c9109ce..e0f7503a9f35 100644
--- a/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml
+++ b/Documentation/devicetree/bindings/soc/renesas/renesas,r9a09g057-sys.yaml
@@ -22,7 +22,10 @@ description: |
 
 properties:
   compatible:
-    const: renesas,r9a09g057-sys
+    items:
+      - enum:
+          - renesas,r9a09g047-sys # RZ/G3E
+          - renesas,r9a09g057-sys # RZ/V2H
 
   reg:
     maxItems: 1
-- 
2.25.1

[PATCH v2 2/4] soc: renesas: rz-sysc: Fix SoC ID string extraction

[John Madieu]

Fix string length calculation when extracting the SoC ID from the compatible
string. Add +1 to the size calculation to ensure proper string termination when
copying with strncpy().

This prevents potential string trunctation when processing the device tree
compatible string to identify the SoC.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
New patch introduced in v2, targetting specific fix.

 drivers/soc/renesas/rz-sysc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c
index d34d295831b8..e472fda3995b 100644
--- a/drivers/soc/renesas/rz-sysc.c
+++ b/drivers/soc/renesas/rz-sysc.c
@@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
 
 	soc_id_start = strchr(match->compatible, ',') + 1;
 	soc_id_end = strchr(match->compatible, '-');
-	size = soc_id_end - soc_id_start;
+	size = soc_id_end - soc_id_start + 1;
 	if (size > 32)
 		size = 32;
 	strscpy(soc_id, soc_id_start, size);
-- 
2.25.1

[PATCH v2 3/4] soc: renesas: rz-sysc: Add support for RZ/G3E family

[John Madieu]

Add SoC detection support for RZ/G3E SoC. Also add support for detecting the
number of cores and ETHOS-U55 NPU and also detect PLL mismatch for SW settings
other than 1.7GHz.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
Changes in v2:
 - Group bitfields ordered by registers
 - Rename SoC-specific callback field to 'print_id'
 - Explicitely select 'MFD_SYSCON' config option
 - Do not rely on 'syscon'-compatible probing anymore.


 drivers/soc/renesas/Kconfig          |  6 +++
 drivers/soc/renesas/Makefile         |  1 +
 drivers/soc/renesas/r9a09g047-sysc.c | 73 ++++++++++++++++++++++++++++
 drivers/soc/renesas/rz-sysc.c        | 22 ++++++++-
 drivers/soc/renesas/rz-sysc.h        |  6 +++
 5 files changed, 106 insertions(+), 2 deletions(-)
 create mode 100644 drivers/soc/renesas/r9a09g047-sysc.c

diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index a792a3e915fe..33759f69c37c 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -348,6 +348,7 @@ config ARCH_R9A09G011
 
 config ARCH_R9A09G047
 	bool "ARM64 Platform support for RZ/G3E"
+	select SYSC_R9A09G047
 	help
 	  This enables support for the Renesas RZ/G3E SoC variants.
 
@@ -386,9 +387,14 @@ 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
 	select SYSC_RZ
 
+config SYSC_R9A09G047
+	bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
+	select SYSC_RZ
+
 endif # SOC_RENESAS
diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile
index 8cd139b3dd0a..3256706112d9 100644
--- a/drivers/soc/renesas/Makefile
+++ b/drivers/soc/renesas/Makefile
@@ -7,6 +7,7 @@ ifdef CONFIG_SMP
 obj-$(CONFIG_ARCH_R9A06G032)	+= r9a06g032-smp.o
 endif
 obj-$(CONFIG_SYSC_R9A08G045)	+= r9a08g045-sysc.o
+obj-$(CONFIG_SYSC_R9A09G047)	+= r9a09g047-sysc.o
 
 # Family
 obj-$(CONFIG_PWC_RZV2M)		+= pwc-rzv2m.o
diff --git a/drivers/soc/renesas/r9a09g047-sysc.c b/drivers/soc/renesas/r9a09g047-sysc.c
new file mode 100644
index 000000000000..3ad6057f9196
--- /dev/null
+++ b/drivers/soc/renesas/r9a09g047-sysc.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RZ/G3E System controller driver
+ *
+ * Copyright (C) 2024 Renesas Electronics Corp.
+ */
+
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/io.h>
+
+#include "rz-sysc.h"
+
+/* Register Offsets */
+#define SYS_LSI_DEVID		0x304
+#define SYS_LSI_DEVID_REV	GENMASK(31, 28)
+#define SYS_LSI_DEVID_SPECIFIC	GENMASK(27, 0)
+#define SYS_LSI_MODE		0x300
+/*
+ * BOOTPLLCA[1:0]
+ *	    [0,0] => 1.1GHZ
+ *	    [0,1] => 1.5GHZ
+ *	    [1,0] => 1.6GHZ
+ *	    [1,1] => 1.7GHZ
+ */
+#define SYS_LSI_MODE_STAT_BOOTPLLCA55	GENMASK(12, 11)
+#define SYS_LSI_MODE_CA55_1_7GHZ	0x3
+#define SYS_LSI_PRR			0x308
+#define SYS_LSI_PRR_CA55_DIS		BIT(8)
+#define SYS_LSI_PRR_NPU_DIS		BIT(1)
+#define SYS_MAX_REG			0x170c
+
+
+static void rzg3e_sysc_print_id(struct device *dev,
+				void __iomem *sysc_base,
+				struct soc_device_attribute *soc_dev_attr)
+{
+	bool is_quad_core, npu_enabled;
+	u32 prr_val, mode_val;
+
+	prr_val = readl(sysc_base + SYS_LSI_PRR);
+	mode_val = readl(sysc_base + SYS_LSI_MODE);
+
+	/* Check CPU and NPU configuration */
+	is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
+	npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
+
+	dev_info(dev, "Detected Renesas %s Core %s %s Rev %s%s\n",
+		 is_quad_core ? "Quad" : "Dual",
+		 soc_dev_attr->family,
+		 soc_dev_attr->soc_id,
+		 soc_dev_attr->revision,
+		 npu_enabled ? " with Ethos-U55" : "");
+
+	/* Check CA55 PLL configuration */
+	if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) != SYS_LSI_MODE_CA55_1_7GHZ)
+		dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n");
+}
+
+static const struct rz_sysc_soc_id_init_data rzg3e_sysc_soc_id_init_data __initconst = {
+	.family = "RZ/G3E",
+	.id = 0x8679447,
+	.offset = SYS_LSI_DEVID,
+	.revision_mask = SYS_LSI_DEVID_REV,
+	.specific_id_mask = SYS_LSI_DEVID_SPECIFIC,
+	.print_id = rzg3e_sysc_print_id,
+};
+
+const struct rz_sysc_init_data rzg3e_sysc_init_data = {
+	.soc_id_init_data = &rzg3e_sysc_soc_id_init_data,
+	.max_register_offset = SYS_MAX_REG,
+};
diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c
index e472fda3995b..6a33807e925a 100644
--- a/drivers/soc/renesas/rz-sysc.c
+++ b/drivers/soc/renesas/rz-sysc.c
@@ -130,6 +130,10 @@ static bool rz_sysc_writeable_reg(struct device *dev, unsigned int off)
 	struct rz_sysc *sysc = dev_get_drvdata(dev);
 	struct rz_sysc_signal *signal;
 
+	/* Fast path if not signal-aware */
+	if (!sysc->num_signals)
+		return true;
+
 	/* Any register containing a signal is writeable. */
 	signal = rz_sysc_off_to_signal(sysc, off, 0);
 	if (signal)
@@ -143,6 +147,10 @@ static bool rz_sysc_readable_reg(struct device *dev, unsigned int off)
 	struct rz_sysc *sysc = dev_get_drvdata(dev);
 	struct rz_sysc_signal *signal;
 
+	/* Fast path if not signal-aware */
+	if (!sysc->num_signals)
+		return true;
+
 	/* Any register containing a signal is readable. */
 	signal = rz_sysc_off_to_signal(sysc, off, 0);
 	if (signal)
@@ -257,8 +265,15 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
 		return -ENODEV;
 	}
 
-	dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n", soc_dev_attr->family,
-		 soc_dev_attr->soc_id, soc_dev_attr->revision);
+	/* Try to call SoC-specific device identification */
+	if (soc_data->print_id) {
+		soc_data->print_id(sysc->dev, sysc->base, soc_dev_attr);
+	} else {
+		dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n",
+			 soc_dev_attr->family,
+			 soc_dev_attr->soc_id,
+			 soc_dev_attr->revision);
+	}
 
 	soc_dev = soc_device_register(soc_dev_attr);
 	if (IS_ERR(soc_dev))
@@ -283,6 +298,9 @@ static struct regmap_config rz_sysc_regmap = {
 static const struct of_device_id rz_sysc_match[] = {
 #ifdef CONFIG_SYSC_R9A08G045
 	{ .compatible = "renesas,r9a08g045-sysc", .data = &rzg3s_sysc_init_data },
+#endif
+#ifdef CONFIG_SYSC_R9A09G047
+	{ .compatible = "renesas,r9a09g047-sys", .data = &rzg3e_sysc_init_data },
 #endif
 	{ }
 };
diff --git a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h
index babca9c743c7..2c92b252b40c 100644
--- a/drivers/soc/renesas/rz-sysc.h
+++ b/drivers/soc/renesas/rz-sysc.h
@@ -8,7 +8,9 @@
 #ifndef __SOC_RENESAS_RZ_SYSC_H__
 #define __SOC_RENESAS_RZ_SYSC_H__
 
+#include <linux/device.h>
 #include <linux/refcount.h>
+#include <linux/sys_soc.h>
 #include <linux/types.h>
 
 /**
@@ -42,6 +44,7 @@ struct rz_sysc_signal {
  * @offset: SYSC SoC ID register offset
  * @revision_mask: SYSC SoC ID revision mask
  * @specific_id_mask: SYSC SoC ID specific ID mask
+ * @print_id: SoC-specific extended device identification
  */
 struct rz_sysc_soc_id_init_data {
 	const char * const family;
@@ -49,6 +52,8 @@ struct rz_sysc_soc_id_init_data {
 	u32 offset;
 	u32 revision_mask;
 	u32 specific_id_mask;
+	void (*print_id)(struct device *dev, void __iomem *sysc_base,
+			 struct soc_device_attribute *soc_dev_attr);
 };
 
 /**
@@ -65,6 +70,7 @@ struct rz_sysc_init_data {
 	u32 max_register_offset;
 };
 
+extern const struct rz_sysc_init_data rzg3e_sysc_init_data;
 extern const struct rz_sysc_init_data rzg3s_sysc_init_data;
 
 #endif /* __SOC_RENESAS_RZ_SYSC_H__ */
-- 
2.25.1

[PATCH v2 4/4] arm64: dts: renesas: r9a09g047: add sys node

[John Madieu]

Add system controller node to RZ/G3E (R9A09G047) SoC DTSI.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
index b73daf43683f..e87521cf9a0b 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
@@ -162,6 +162,13 @@ cpg: clock-controller@10420000 {
 			#power-domain-cells = <0>;
 		};
 
+		sys: system-controller@10430000 {
+			compatible = "renesas,r9a09g047-sys";
+			reg = <0 0x10430000 0 0x10000>;
+			clocks = <&cpg CPG_CORE R9A09G047_SYS_0_PCLK>;
+			resets = <&cpg 0x30>;
+		};
+
 		ostm0: timer@11800000 {
 			compatible = "renesas,r9a09g047-ostm", "renesas,ostm";
 			reg = <0x0 0x11800000 0x0 0x1000>;
-- 
2.25.1

Re: [PATCH v2 2/4] soc: renesas: rz-sysc: Fix SoC ID string extraction

[Geert Uytterhoeven]

Hi John,

On Wed, Jan 1, 2025 at 5:34 PM John Madieu
<john.madieu.xa@bp.renesas.com> wrote:
> Fix string length calculation when extracting the SoC ID from the compatible
> string. Add +1 to the size calculation to ensure proper string termination when
> copying with strncpy().
>
> This prevents potential string trunctation when processing the device tree
> compatible string to identify the SoC.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
> New patch introduced in v2, targetting specific fix.

Thanks for your patch!

> --- a/drivers/soc/renesas/rz-sysc.c
> +++ b/drivers/soc/renesas/rz-sysc.c
> @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
>
>         soc_id_start = strchr(match->compatible, ',') + 1;
>         soc_id_end = strchr(match->compatible, '-');
> -       size = soc_id_end - soc_id_start;
> +       size = soc_id_end - soc_id_start + 1;
>         if (size > 32)
>                 size = 32;
>         strscpy(soc_id, soc_id_start, size);

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

As the code fixed was introduced by a patch[1] that has not been
accepted yet, this fix should be incorporated into the original patch
(together with other fixes according to review comments).

[1] "[PATCH v2 04/15] soc: renesas: rz-sysc: Add SoC detection support"
https://lore.kernel.org/linux-renesas-soc/20241126092050.1825607-5-claudiu.beznea.uj@bp.renesas.com

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 v2 1/4] dt-bindings: soc: renesas: Add RZ/G3E variant SYS bindings

[Rob Herring (Arm)]

On Wed, 01 Jan 2025 17:33:41 +0100, John Madieu wrote:
> Add RZ/G3E (R9A09G047) variant to the existing RZ/V2H System
> Controller (SYS) binding as both IPs are compatible.
> 
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
> Changes:
>  - v1 -> v2: Do not rely on syscon compatible string anymore
> 
> Notes:
>     v1: Acked-by: Rob Herring (Arm) <robh@kernel.org>
>     v1: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>     v2: Tags dropped due to small changes in compatible property structure.
> 
>  .../bindings/soc/renesas/renesas,r9a09g057-sys.yaml          | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>