diff mbox series

[v2,04/12] soc: renesas: sysc: Add SoC identification for RZ/V2N SoC

Message ID 20250407191628.323613-5-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive)
State Under Review
Headers show
Series Add support for Renesas RZ/V2N SoC and EVK | expand

Commit Message

Lad, Prabhakar April 7, 2025, 7:16 p.m. UTC 
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add SoC identification for the RZ/V2N SoC using the System Controller
(SYS) block.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No changes in the code.
---
 drivers/soc/renesas/Kconfig         |   5 ++
 drivers/soc/renesas/Makefile        |   1 +
 drivers/soc/renesas/r9a09g056-sys.c | 107 ++++++++++++++++++++++++++++
 drivers/soc/renesas/rz-sysc.c       |   3 +
 drivers/soc/renesas/rz-sysc.h       |   1 +
 5 files changed, 117 insertions(+)
 create mode 100644 drivers/soc/renesas/r9a09g056-sys.c

Comments

Geert Uytterhoeven April 10, 2025, 9:28 a.m. UTC | #1 
Hi Prabhakar,

On Mon, 7 Apr 2025 at 21:16, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add SoC identification for the RZ/V2N SoC using the System Controller
> (SYS) block.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/soc/renesas/r9a09g056-sys.c
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RZ/V2N System controller (SYS) driver
> + *
> + * Copyright (C) 2025 Renesas Electronics Corp.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/string.h>
> +
> +#include "rz-sysc.h"
> +
> +/* Register Offsets */
> +#define SYS_LSI_MODE           0x300
> +#define SYS_LSI_MODE_SEC_EN    BIT(16)
> +/*
> + * 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_GPU_DIS            BIT(0)
> +#define SYS_LSI_PRR_ISP_DIS            BIT(4)
> +
> +#define SYS_RZV2N_FEATURE_G31          BIT(0)
> +#define SYS_RZV2N_FEATURE_C55          BIT(1)
> +#define SYS_RZV2N_FEATURE_SEC          BIT(2)
> +
> +static void rzv2n_sys_print_id(struct device *dev,
> +                              void __iomem *sysc_base,
> +                              struct soc_device_attribute *soc_dev_attr)
> +{
> +       unsigned int part_number;
> +       char features[75] = "";
> +       u32 prr_val, mode_val;
> +       u8 feature_flags;
> +
> +       prr_val = readl(sysc_base + SYS_LSI_PRR);
> +       mode_val = readl(sysc_base + SYS_LSI_MODE);
> +
> +       /* Check GPU, ISP and Cryptographic configuration */
> +       feature_flags = !(prr_val & SYS_LSI_PRR_GPU_DIS) ? SYS_RZV2N_FEATURE_G31 : 0;
> +       feature_flags |= !(prr_val & SYS_LSI_PRR_ISP_DIS) ? SYS_RZV2N_FEATURE_C55 : 0;
> +       feature_flags |= (mode_val & SYS_LSI_MODE_SEC_EN) ? SYS_RZV2N_FEATURE_SEC : 0;
> +
> +       part_number = 41;
> +       if (feature_flags & SYS_RZV2N_FEATURE_G31)
> +               part_number++;
> +       if (feature_flags & SYS_RZV2N_FEATURE_C55)
> +               part_number += 2;
> +       if (feature_flags & SYS_RZV2N_FEATURE_SEC)
> +               part_number += 4;

The above construct can be simplified to

    part_number = 41 + feature_flags;

> +       if (feature_flags) {
> +               unsigned int features_len = sizeof(features);
> +
> +               strscpy(features, "with ");
> +               if (feature_flags & SYS_RZV2N_FEATURE_G31)
> +                       strlcat(features, "GE3D (Mali-G31)", features_len);
> +
> +               if (feature_flags == (SYS_RZV2N_FEATURE_G31 |
> +                                     SYS_RZV2N_FEATURE_C55 |
> +                                     SYS_RZV2N_FEATURE_SEC))
> +                       strlcat(features, ", ", features_len);
> +               else if ((feature_flags & SYS_RZV2N_FEATURE_G31) &&
> +                        (feature_flags & (SYS_RZV2N_FEATURE_C55 | SYS_RZV2N_FEATURE_SEC)))
> +                       strlcat(features, " and ", features_len);
> +
> +               if (feature_flags & SYS_RZV2N_FEATURE_SEC)
> +                       strlcat(features, "Cryptographic engine", features_len);
> +
> +               if ((feature_flags & SYS_RZV2N_FEATURE_SEC) &&
> +                   (feature_flags & SYS_RZV2N_FEATURE_C55))
> +                       strlcat(features, " and ", features_len);
> +
> +               if (feature_flags & SYS_RZV2N_FEATURE_C55)
> +                       strlcat(features, "ISP (Mali-C55)", features_len);
> +       }

The above looks overly complicated to me.  What about handling it
like on RZ/V2H?  I agree having 3x "with" doesn't look nice, but you
could just drop all "with"s.

> +       dev_info(dev, "Detected Renesas %s %sn%d Rev %s %s\n", soc_dev_attr->family,
> +                soc_dev_attr->soc_id, part_number, soc_dev_attr->revision, features);

This prints a trailing space if features is empty.

Gr{oetje,eeting}s,

                        Geert
Lad, Prabhakar April 14, 2025, 10:21 a.m. UTC | #2 
Hi Geert,

Thank you for the review.

On Thu, Apr 10, 2025 at 10:28 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, 7 Apr 2025 at 21:16, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add SoC identification for the RZ/V2N SoC using the System Controller
> > (SYS) block.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/drivers/soc/renesas/r9a09g056-sys.c
> > @@ -0,0 +1,107 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RZ/V2N System controller (SYS) driver
> > + *
> > + * Copyright (C) 2025 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/bits.h>
> > +#include <linux/device.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +#include <linux/string.h>
> > +
> > +#include "rz-sysc.h"
> > +
> > +/* Register Offsets */
> > +#define SYS_LSI_MODE           0x300
> > +#define SYS_LSI_MODE_SEC_EN    BIT(16)
> > +/*
> > + * 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_GPU_DIS            BIT(0)
> > +#define SYS_LSI_PRR_ISP_DIS            BIT(4)
> > +
> > +#define SYS_RZV2N_FEATURE_G31          BIT(0)
> > +#define SYS_RZV2N_FEATURE_C55          BIT(1)
> > +#define SYS_RZV2N_FEATURE_SEC          BIT(2)
> > +
> > +static void rzv2n_sys_print_id(struct device *dev,
> > +                              void __iomem *sysc_base,
> > +                              struct soc_device_attribute *soc_dev_attr)
> > +{
> > +       unsigned int part_number;
> > +       char features[75] = "";
> > +       u32 prr_val, mode_val;
> > +       u8 feature_flags;
> > +
> > +       prr_val = readl(sysc_base + SYS_LSI_PRR);
> > +       mode_val = readl(sysc_base + SYS_LSI_MODE);
> > +
> > +       /* Check GPU, ISP and Cryptographic configuration */
> > +       feature_flags = !(prr_val & SYS_LSI_PRR_GPU_DIS) ? SYS_RZV2N_FEATURE_G31 : 0;
> > +       feature_flags |= !(prr_val & SYS_LSI_PRR_ISP_DIS) ? SYS_RZV2N_FEATURE_C55 : 0;
> > +       feature_flags |= (mode_val & SYS_LSI_MODE_SEC_EN) ? SYS_RZV2N_FEATURE_SEC : 0;
> > +
> > +       part_number = 41;
> > +       if (feature_flags & SYS_RZV2N_FEATURE_G31)
> > +               part_number++;
> > +       if (feature_flags & SYS_RZV2N_FEATURE_C55)
> > +               part_number += 2;
> > +       if (feature_flags & SYS_RZV2N_FEATURE_SEC)
> > +               part_number += 4;
>
> The above construct can be simplified to
>
>     part_number = 41 + feature_flags;
>
Agreed.

> > +       if (feature_flags) {
> > +               unsigned int features_len = sizeof(features);
> > +
> > +               strscpy(features, "with ");
> > +               if (feature_flags & SYS_RZV2N_FEATURE_G31)
> > +                       strlcat(features, "GE3D (Mali-G31)", features_len);
> > +
> > +               if (feature_flags == (SYS_RZV2N_FEATURE_G31 |
> > +                                     SYS_RZV2N_FEATURE_C55 |
> > +                                     SYS_RZV2N_FEATURE_SEC))
> > +                       strlcat(features, ", ", features_len);
> > +               else if ((feature_flags & SYS_RZV2N_FEATURE_G31) &&
> > +                        (feature_flags & (SYS_RZV2N_FEATURE_C55 | SYS_RZV2N_FEATURE_SEC)))
> > +                       strlcat(features, " and ", features_len);
> > +
> > +               if (feature_flags & SYS_RZV2N_FEATURE_SEC)
> > +                       strlcat(features, "Cryptographic engine", features_len);
> > +
> > +               if ((feature_flags & SYS_RZV2N_FEATURE_SEC) &&
> > +                   (feature_flags & SYS_RZV2N_FEATURE_C55))
> > +                       strlcat(features, " and ", features_len);
> > +
> > +               if (feature_flags & SYS_RZV2N_FEATURE_C55)
> > +                       strlcat(features, "ISP (Mali-C55)", features_len);
> > +       }
>
> The above looks overly complicated to me.  What about handling it
> like on RZ/V2H?  I agree having 3x "with" doesn't look nice, but you
> could just drop all "with"s.
>
Ok, I will switch like below:

part_number = 41 + feature_flags;

dev_info(dev, "Detected Renesas %s %sn%d Rev %s%s%s%s%s\n",
soc_dev_attr->family,
               soc_dev_attr->soc_id, part_number,
soc_dev_attr->revision, feature_flags ?
               " with" : "", feature_flags & SYS_RZV2N_FEATURE_G31 ? "
GE3D (Mali-G31)" : "",
               feature_flags & SYS_RZV2N_FEATURE_SEC ? " Cryptographic
engine" : "",
               feature_flags & SYS_RZV2N_FEATURE_C55 ? " ISP (Mali-C55)" : "");


> > +       dev_info(dev, "Detected Renesas %s %sn%d Rev %s %s\n", soc_dev_attr->family,
> > +                soc_dev_attr->soc_id, part_number, soc_dev_attr->revision, features);
>
> This prints a trailing space if features is empty.
>
Agreed.

Cheers,
Prabhakar
diff mbox series

Patch

diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index 3fa5ed36d20b..7f4b4088a14e 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -396,6 +396,7 @@  config ARCH_R9A09G047
 config ARCH_R9A09G056
 	bool "ARM64 Platform support for RZ/V2N"
 	default y if ARCH_RENESAS
+	select SYS_R9A09G056
 	help
 	  This enables support for the Renesas RZ/V2N SoC variants.
 
@@ -451,6 +452,10 @@  config SYS_R9A09G047
 	bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
 	select SYSC_RZ
 
+config SYS_R9A09G056
+	bool "Renesas RZ/V2N System controller support" if COMPILE_TEST
+	select SYSC_RZ
+
 config SYS_R9A09G057
 	bool "Renesas RZ/V2H System controller support" if COMPILE_TEST
 	select SYSC_RZ
diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile
index 81d4c5726e4c..3bdcc6a395d5 100644
--- a/drivers/soc/renesas/Makefile
+++ b/drivers/soc/renesas/Makefile
@@ -8,6 +8,7 @@  obj-$(CONFIG_ARCH_R9A06G032)	+= r9a06g032-smp.o
 endif
 obj-$(CONFIG_SYSC_R9A08G045)	+= r9a08g045-sysc.o
 obj-$(CONFIG_SYS_R9A09G047)	+= r9a09g047-sys.o
+obj-$(CONFIG_SYS_R9A09G056)	+= r9a09g056-sys.o
 obj-$(CONFIG_SYS_R9A09G057)	+= r9a09g057-sys.o
 
 # Family
diff --git a/drivers/soc/renesas/r9a09g056-sys.c b/drivers/soc/renesas/r9a09g056-sys.c
new file mode 100644
index 000000000000..3bea674c785e
--- /dev/null
+++ b/drivers/soc/renesas/r9a09g056-sys.c
@@ -0,0 +1,107 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RZ/V2N System controller (SYS) driver
+ *
+ * Copyright (C) 2025 Renesas Electronics Corp.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/string.h>
+
+#include "rz-sysc.h"
+
+/* Register Offsets */
+#define SYS_LSI_MODE		0x300
+#define SYS_LSI_MODE_SEC_EN	BIT(16)
+/*
+ * 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_GPU_DIS		BIT(0)
+#define SYS_LSI_PRR_ISP_DIS		BIT(4)
+
+#define SYS_RZV2N_FEATURE_G31		BIT(0)
+#define SYS_RZV2N_FEATURE_C55		BIT(1)
+#define SYS_RZV2N_FEATURE_SEC		BIT(2)
+
+static void rzv2n_sys_print_id(struct device *dev,
+			       void __iomem *sysc_base,
+			       struct soc_device_attribute *soc_dev_attr)
+{
+	unsigned int part_number;
+	char features[75] = "";
+	u32 prr_val, mode_val;
+	u8 feature_flags;
+
+	prr_val = readl(sysc_base + SYS_LSI_PRR);
+	mode_val = readl(sysc_base + SYS_LSI_MODE);
+
+	/* Check GPU, ISP and Cryptographic configuration */
+	feature_flags = !(prr_val & SYS_LSI_PRR_GPU_DIS) ? SYS_RZV2N_FEATURE_G31 : 0;
+	feature_flags |= !(prr_val & SYS_LSI_PRR_ISP_DIS) ? SYS_RZV2N_FEATURE_C55 : 0;
+	feature_flags |= (mode_val & SYS_LSI_MODE_SEC_EN) ? SYS_RZV2N_FEATURE_SEC : 0;
+
+	part_number = 41;
+	if (feature_flags & SYS_RZV2N_FEATURE_G31)
+		part_number++;
+	if (feature_flags & SYS_RZV2N_FEATURE_C55)
+		part_number += 2;
+	if (feature_flags & SYS_RZV2N_FEATURE_SEC)
+		part_number += 4;
+
+	if (feature_flags) {
+		unsigned int features_len = sizeof(features);
+
+		strscpy(features, "with ");
+		if (feature_flags & SYS_RZV2N_FEATURE_G31)
+			strlcat(features, "GE3D (Mali-G31)", features_len);
+
+		if (feature_flags == (SYS_RZV2N_FEATURE_G31 |
+				      SYS_RZV2N_FEATURE_C55 |
+				      SYS_RZV2N_FEATURE_SEC))
+			strlcat(features, ", ", features_len);
+		else if ((feature_flags & SYS_RZV2N_FEATURE_G31) &&
+			 (feature_flags & (SYS_RZV2N_FEATURE_C55 | SYS_RZV2N_FEATURE_SEC)))
+			strlcat(features, " and ", features_len);
+
+		if (feature_flags & SYS_RZV2N_FEATURE_SEC)
+			strlcat(features, "Cryptographic engine", features_len);
+
+		if ((feature_flags & SYS_RZV2N_FEATURE_SEC) &&
+		    (feature_flags & SYS_RZV2N_FEATURE_C55))
+			strlcat(features, " and ", features_len);
+
+		if (feature_flags & SYS_RZV2N_FEATURE_C55)
+			strlcat(features, "ISP (Mali-C55)", features_len);
+	}
+	dev_info(dev, "Detected Renesas %s %sn%d Rev %s %s\n", soc_dev_attr->family,
+		 soc_dev_attr->soc_id, part_number, soc_dev_attr->revision, features);
+
+	/* 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 rzv2n_sys_soc_id_init_data __initconst = {
+	.family = "RZ/V2N",
+	.id = 0x867d447,
+	.devid_offset = 0x304,
+	.revision_mask = GENMASK(31, 28),
+	.specific_id_mask = GENMASK(27, 0),
+	.print_id = rzv2n_sys_print_id,
+};
+
+const struct rz_sysc_init_data rzv2n_sys_init_data = {
+	.soc_id_init_data = &rzv2n_sys_soc_id_init_data,
+};
diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c
index 14db508f669f..ffa65fb4dade 100644
--- a/drivers/soc/renesas/rz-sysc.c
+++ b/drivers/soc/renesas/rz-sysc.c
@@ -88,6 +88,9 @@  static const struct of_device_id rz_sysc_match[] = {
 #ifdef CONFIG_SYS_R9A09G047
 	{ .compatible = "renesas,r9a09g047-sys", .data = &rzg3e_sys_init_data },
 #endif
+#ifdef CONFIG_SYS_R9A09G056
+	{ .compatible = "renesas,r9a09g056-sys", .data = &rzv2n_sys_init_data },
+#endif
 #ifdef CONFIG_SYS_R9A09G057
 	{ .compatible = "renesas,r9a09g057-sys", .data = &rzv2h_sys_init_data },
 #endif
diff --git a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h
index aa83948c5117..56bc047a1bff 100644
--- a/drivers/soc/renesas/rz-sysc.h
+++ b/drivers/soc/renesas/rz-sysc.h
@@ -42,5 +42,6 @@  struct rz_sysc_init_data {
 extern const struct rz_sysc_init_data rzg3e_sys_init_data;
 extern const struct rz_sysc_init_data rzg3s_sysc_init_data;
 extern const struct rz_sysc_init_data rzv2h_sys_init_data;
+extern const struct rz_sysc_init_data rzv2n_sys_init_data;
 
 #endif /* __SOC_RENESAS_RZ_SYSC_H__ */