soc: renesas: Identify SoC and register with the SoC bus

If soc_device_register() is called before soc_bus_register(), it crashes with a NULL pointer dereference.

soc_bus_register() is already a core_initcall(), but drivers/base/ is entered later than e.g. drivers/pinctrl/ and drivers/soc/. Hence there are several subsystems that may need to know SoC revision information, while it's not so easy to initialize the SoC bus even earlier using an initcall.

To fix this, let soc_device_register() register the bus early if that hasn't happened yet.

Signed-off-by Geert Uytterhoeven <[email protected]>

From: Arnd Bergmann <[email protected]>

We keep running into cases where device drivers want to know the exact version of the a SoC they are currently running on. In the past, this has usually been done through a vendor specific API that can be called by a driver, or by directly accessing some kind of version register that is not part of the device itself but that belongs to a global register area of the chip.

Common reasons for doing this include:

- A machine is not using devicetree or similar for passing data about on-chip devices, but just announces their presence using boot-time platform devices, and the machine code itself does not care about the revision.

- There is existing firmware or boot loaders with existing DT binaries with generic compatible strings that do not identify the particular revision of each device, but the driver knows which SoC revisions include which part.

- A prerelease version of a chip has some quirks and we are using the same version of the bootloader and the DT blob on both the prerelease and the final version. An update of the DT binding seems inappropriate because that would involve maintaining multiple copies of the dts and/or bootloader.

This patch introduces the soc_device_match() interface that is meant to work like of_match_node() but instead of identifying the version of a device, it identifies the SoC itself using a vendor-agnostic interface.

Unlike of_match_node(), we do not do an exact string compare but instead use glob_match() to allow wildcards in strings.

Signed-off-by Arnd Bergmann <[email protected]>

Signed-off-by Yangbo Lu <[email protected]>

Signed-off-by Geert Uytterhoeven <[email protected]>

---
 drivers/base/Kconfig    |  1 +
 drivers/base/soc.c      | 66 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/sys_soc.h |  3 +++
 3 files changed, 70 insertions(+)

If soc_device_match() is used to check the value of a specific attribute that is not present for the current SoC, the kernel crashes with a NULL pointer dereference.

Fix this by explicitly checking for the absence of a needed property, and considering this a non-match.

Signed-off-by Geert Uytterhoeven <[email protected]>

If soc_device_match() is used to check the value of a specific attribute that is not present for the current SoC, the kernel crashes with a NULL pointer dereference.

Fix this by explicitly checking for the absence of a needed property, and considering this a non-match.

Signed-off-by Geert Uytterhoeven <[email protected]>

Identify the SoC type and revision, and register this information with the SoC bus, so it is available under /sys/devices/soc0/, and can be checked where needed using soc_device_match().

In addition, on SoCs that support it, the product ID is read from a hardware register and validated, to catch accidental use of a DTB for a different SoC.

Example:

    Detected Renesas r8a7791 ES1.0
    ...
    # cat /sys/devices/soc0/{family,machine,soc_id,revision}
    R-Car Gen2
    Koelsch
    r8a7791
    ES1.0

Signed-off-by Geert Uytterhoeven <[email protected]>

---
This patch does NOT add a call to

        of_platform_default_populate(NULL, NULL,
                                     soc_device_to_device(soc_dev));

Contrary to suggested by commit 74d1d82cdaaec727 ("drivers/base: add bus for System-on-Chip devices), doing so would not only move on-SoC devices from /sys/devices/platform/ to /sys/devices/soc0/, but also all other board (off-SoC) devices specified in the DTB.

---
 arch/arm/mach-shmobile/Kconfig    |   1 +
 arch/arm64/Kconfig.platforms      |   1 +
 drivers/soc/renesas/Makefile      |   2 +
 drivers/soc/renesas/renesas-soc.c | 266 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 270 insertions(+)
 create mode 100644 drivers/soc/renesas/renesas-soc.c

arch/arm/mach-shmobile/Kconfig +1 -0

index b5a3cbe81dd1d1f0..e41d2cbb2c825981 100644
@@ -42,6 +42,7 @@ menuconfig ARCH_RENESAS
 	select HAVE_ARM_TWD if SMP
 	select NO_IOPORT_MAP
 	select PINCTRL
+	select SOC_BUS
 	select ZONE_DMA if ARM_LPAE
 
 if ARCH_RENESAS

arch/arm64/Kconfig.platforms +1 -0

index be5d824ebdba2dab..a2675afc61baba8d 100644
@@ -131,6 +131,7 @@ config ARCH_RENESAS
 	select PM
 	select PM_GENERIC_DOMAINS
 	select RENESAS_IRQC
+	select SOC_BUS
 	help
 	  This enables support for the ARMv8 based Renesas SoCs.
 

drivers/soc/renesas/Makefile +2 -0

index 623039c3514cdc34..ae6ae8a11f98aba1 100644
@@ -1,3 +1,5 @@
+obj-y				+= renesas-soc.o
+
 obj-$(CONFIG_ARCH_R8A7779)	+= rcar-sysc.o r8a7779-sysc.o
 obj-$(CONFIG_ARCH_R8A7790)	+= rcar-sysc.o r8a7790-sysc.o
 obj-$(CONFIG_ARCH_R8A7791)	+= rcar-sysc.o r8a7791-sysc.o

drivers/soc/renesas/renesas-soc.c new file +15 -0

new file mode 100644
index 0000000000000000..74b72e4112b8889e
@@ -0,0 +1,266 @@
+/*
+ * Renesas SoC Identification
+ *
+ * Copyright (C) 2014-2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+

drivers/soc/renesas/renesas-soc.c new file +199 -0

+};
+
+static const struct renesas_family fam_emev2 __initconst = {
+	.name	= "Emma Mobile EV2",
+};
+
+static const struct renesas_family fam_rmobile __initconst = {
+	.name	= "R-Mobile",
+	.reg	= 0xe600101c,		/* CCCR (Common Chip Code Register) */
+};
+
+static const struct renesas_family fam_rcar_gen1 __initconst = {
+	.name	= "R-Car Gen1",
+	.reg	= 0xff000044,		/* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rcar_gen2 __initconst = {
+	.name	= "R-Car Gen2",
+	.reg	= 0xff000044,		/* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rcar_gen3 __initconst = {
+	.name	= "R-Car Gen3",
+	.reg	= 0xfff00044,		/* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rza __initconst = {
+	.name	= "RZ/A",
+};
+
+static const struct renesas_family fam_rzg __initconst = {
+	.name	= "RZ/G",
+	.reg	= 0xff000044,		/* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_shmobile __initconst = {
+	.name	= "SH-Mobile",
+	.reg	= 0xe600101c,		/* CCCR (Common Chip Code Register) */
+};
+
+
+struct renesas_soc {
+	const struct renesas_family *family;
+	u8 id;
+};
+
+static const struct renesas_soc soc_emev2 __initconst = {
+	.family	= &fam_emev2,
+};
+
+static const struct renesas_soc soc_rz_a1h __initconst = {
+	.family	= &fam_rza,
+};
+
+static const struct renesas_soc soc_rmobile_ape6 __initconst = {
+	.family	= &fam_rmobile,
+	.id	= 0x3f,
+};
+
+static const struct renesas_soc soc_rmobile_a1 __initconst = {
+	.family	= &fam_rmobile,
+	.id	= 0x40,
+};
+
+static const struct renesas_soc soc_rz_g1m __initconst = {
+	.family	= &fam_rzg,
+	.id	= 0x47,
+};
+
+static const struct renesas_soc soc_rz_g1e __initconst = {
+	.family	= &fam_rzg,
+	.id	= 0x4c,
+};
+
+static const struct renesas_soc soc_rcar_m1a __initconst = {
+	.family	= &fam_rcar_gen1,
+};
+
+static const struct renesas_soc soc_rcar_h1 __initconst = {
+	.family	= &fam_rcar_gen1,
+	.id	= 0x3b,
+};
+
+static const struct renesas_soc soc_rcar_h2 __initconst = {
+	.family	= &fam_rcar_gen2,
+	.id	= 0x45,
+};
+
+static const struct renesas_soc soc_rcar_m2_w __initconst = {
+	.family	= &fam_rcar_gen2,
+	.id	= 0x47,
+};
+
+static const struct renesas_soc soc_rcar_v2h __initconst = {
+	.family	= &fam_rcar_gen2,
+	.id	= 0x4a,
+};
+
+static const struct renesas_soc soc_rcar_m2_n __initconst = {
+	.family	= &fam_rcar_gen2,
+	.id	= 0x4b,
+};
+
+static const struct renesas_soc soc_rcar_e2 __initconst = {
+	.family	= &fam_rcar_gen2,
+	.id	= 0x4c,
+};
+
+static const struct renesas_soc soc_rcar_h3 __initconst = {
+	.family	= &fam_rcar_gen3,
+	.id	= 0x4f,
+};
+
+static const struct renesas_soc soc_rcar_m3_w __initconst = {
+	.family	= &fam_rcar_gen3,
+	.id	= 0x52,
+};
+
+static const struct renesas_soc soc_shmobile_ag5 __initconst = {
+	.family	= &fam_shmobile,
+	.id	= 0x37,
+};
+
+static const struct of_device_id renesas_socs[] __initconst = {
+#ifdef CONFIG_ARCH_EMEV2
+	{ .compatible = "renesas,emev2",	.data = &soc_emev2 },
+#endif
+#ifdef CONFIG_ARCH_R7S72100
+	{ .compatible = "renesas,r7s72100",	.data = &soc_rz_a1h },
+#endif
+#ifdef CONFIG_ARCH_R8A73A4
+	{ .compatible = "renesas,r8a73a4",	.data = &soc_rmobile_ape6 },
+#endif
+#ifdef CONFIG_ARCH_R8A7740
+	{ .compatible = "renesas,r8a7740",	.data = &soc_rmobile_a1 },
+#endif
+#ifdef CONFIG_ARCH_R8A7743
+	{ .compatible = "renesas,r8a7743",	.data = &soc_rz_g1m },
+#endif
+#ifdef CONFIG_ARCH_R8A7745
+	{ .compatible = "renesas,r8a7745",	.data = &soc_rz_g1e },
+#endif
+#ifdef CONFIG_ARCH_R8A7778
+	{ .compatible = "renesas,r8a7778",	.data = &soc_rcar_m1a },
+#endif
+#ifdef CONFIG_ARCH_R8A7779
+	{ .compatible = "renesas,r8a7779",	.data = &soc_rcar_h1 },
+#endif
+#ifdef CONFIG_ARCH_R8A7790
+	{ .compatible = "renesas,r8a7790",	.data = &soc_rcar_h2 },
+#endif
+#ifdef CONFIG_ARCH_R8A7791
+	{ .compatible = "renesas,r8a7791",	.data = &soc_rcar_m2_w },
+#endif
+#ifdef CONFIG_ARCH_R8A7792
+	{ .compatible = "renesas,r8a7792",	.data = &soc_rcar_v2h },
+#endif
+#ifdef CONFIG_ARCH_R8A7793
+	{ .compatible = "renesas,r8a7793",	.data = &soc_rcar_m2_n },
+#endif
+#ifdef CONFIG_ARCH_R8A7794
+	{ .compatible = "renesas,r8a7794",	.data = &soc_rcar_e2 },
+#endif
+#ifdef CONFIG_ARCH_R8A7795
+	{ .compatible = "renesas,r8a7795",	.data = &soc_rcar_h3 },
+#endif
+#ifdef CONFIG_ARCH_R8A7796
+	{ .compatible = "renesas,r8a7796",	.data = &soc_rcar_m3_w },
+#endif
+#ifdef CONFIG_ARCH_SH73A0
+	{ .compatible = "renesas,sh73a0",	.data = &soc_shmobile_ag5 },
+#endif
+	{ /* sentinel */ }
+};
+
+static int __init renesas_soc_init(void)
+{
+	struct soc_device_attribute *soc_dev_attr;
+	const struct renesas_family *family;
+	unsigned int product, esi = 0, esf;
+	const struct of_device_id *match;
+	const struct renesas_soc *soc;
+	struct soc_device *soc_dev;
+	struct device_node *np;
+	void __iomem *mapped;
+
+	np = of_find_matching_node_and_match(NULL, renesas_socs, &match);
+	if (!np)
+		return -ENODEV;
+
+	of_node_put(np);
+	soc = match->data;
+	family = soc->family;
+
+	if (soc->id) {
+		mapped = ioremap(family->reg, 4);
+		if (!mapped)
+			return -ENOMEM;
+

Identify the SoC type and revision, and register this information with the SoC bus, so it is available under /sys/devices/soc0/, and can be checked where needed using soc_device_match().

In addition, on SoCs that support it, the product ID is read from a hardware register and validated, to catch accidental use of a DTB for a different SoC.

Example:

    Detected Renesas r8a7791 ES1.0
    ...
    # cat /sys/devices/soc0/{family,machine,soc_id,revision}
    R-Car Gen2
    Koelsch
    r8a7791
    ES1.0

Signed-off-by Geert Uytterhoeven <[email protected]>

---
This patch does NOT add a call to

        of_platform_default_populate(NULL, NULL,
                                     soc_device_to_device(soc_dev));

Contrary to suggested by commit 74d1d82cdaaec727 ("drivers/base: add bus for System-on-Chip devices), doing so would not only move on-SoC devices from /sys/devices/platform/ to /sys/devices/soc0/, but also all other board (off-SoC) devices specified in the DTB.

drivers/soc/renesas/renesas-soc.c new file +20 -0

new file mode 100644
index 0000000000000000..74b72e4112b8889e
@@ -0,0 +1,266 @@
+/*
+ * Renesas SoC Identification
+ *
+ * Copyright (C) 2014-2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/sys_soc.h>

drivers/soc/renesas/renesas-soc.c new file +19 -0

+static const struct renesas_family fam_rmobile __initconst = {
+	.name	= "R-Mobile",
+	.reg	= 0xe600101c,		/* CCCR (Common Chip Code Register) */
+};
+
+static const struct renesas_family fam_rcar_gen1 __initconst = {
+	.name	= "R-Car Gen1",
+	.reg	= 0xff000044,		/* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rcar_gen2 __initconst = {
+	.name	= "R-Car Gen2",
+	.reg	= 0xff000044,		/* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rcar_gen3 __initconst = {
+	.name	= "R-Car Gen3",
+	.reg	= 0xfff00044,		/* PRR (Product Register) */
+};

Quoted 10 lines · CEST Geert Uytterhoeven

Identify the SoC type and revision, and register this information with the SoC bus, so it is available under /sys/devices/soc0/, and can be checked where needed using soc_device_match().

In addition, on SoCs that support it, the product ID is read from a hardware register and validated, to catch accidental use of a DTB for a different SoC.

Example:

Quoted 8 lines

    Detected Renesas r8a7791 ES1.0
    ...
    # cat /sys/devices/soc0/{family,machine,soc_id,revision}
    R-Car Gen2
    Koelsch
    r8a7791
    ES1.0

Seems all reasonable.

Quoted 7 lines

Signed-off-by Geert Uytterhoeven <[email protected]>

---
This patch does NOT add a call to

        of_platform_default_populate(NULL, NULL,
                                     soc_device_to_device(soc_dev));

Quoted 4 lines

Contrary to suggested by commit 74d1d82cdaaec727 ("drivers/base: add bus for System-on-Chip devices), doing so would not only move on-SoC devices from /sys/devices/platform/ to /sys/devices/soc0/, but also all other board (off-SoC) devices specified in the DTB.

Right, we have moved away from that a while ago, and now just use the device for identification, not to model the device hierarchy.

Quoted 32 lines

drivers/soc/renesas/renesas-soc.c new file +26 -0

new file mode 100644
index 0000000000000000..74b72e4112b8889e
@@ -0,0 +1,266 @@
+/*
+ * Renesas SoC Identification
+ *
+ * Copyright (C) 2014-2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/sys_soc.h>
+
+
+struct renesas_family {
+     const char name[16];
+     u32 reg;                        /* CCCR, PVR, or PRR */
+};

drivers/soc/renesas/renesas-soc.c line 30

Quoted 4 lines

drivers/soc/renesas/renesas-soc.c new file +4 -0

+
+static const struct renesas_family fam_emev2 __initconst = {
+     .name   = "Emma Mobile EV2",
+};

As this is not related to the others and doesn't have the respective register, I'd leave the platform out of this, and possibly have a separate driver for it.

drivers/soc/renesas/renesas-soc.c line 54

Quoted 3 lines

drivers/soc/renesas/renesas-soc.c new file +3 -0

+static const struct renesas_family fam_rza __initconst = {
+     .name   = "RZ/A",
+};

I'm not sure about the relationship between this one and the others, maybe it should be treated in the same way as emev2 and left out from this driver?

Quoted 25 lines

drivers/soc/renesas/renesas-soc.c new file +25 -0

+static const struct renesas_family fam_rmobile __initconst = {
+     .name   = "R-Mobile",
+     .reg    = 0xe600101c,           /* CCCR (Common Chip Code Register) */
+};
+
+static const struct renesas_family fam_rcar_gen1 __initconst = {
+     .name   = "R-Car Gen1",
+     .reg    = 0xff000044,           /* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rcar_gen2 __initconst = {
+     .name   = "R-Car Gen2",
+     .reg    = 0xff000044,           /* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rcar_gen3 __initconst = {
+     .name   = "R-Car Gen3",
+     .reg    = 0xfff00044,           /* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rzg __initconst = {
+     .name   = "RZ/G",
+     .reg    = 0xff000044,           /* PRR (Product Register) */
+};
+

drivers/soc/renesas/renesas-soc.c line 64

Quoted 4 lines

drivers/soc/renesas/renesas-soc.c new file +4 -0

+static const struct renesas_family fam_shmobile __initconst = {
+     .name   = "SH-Mobile",
+     .reg    = 0xe600101c,           /* CCCR (Common Chip Code Register) */
+};

These seem to fall into two distinct categories, maybe there is a better way to group them. What device contain the two kinds of registers (PRR, CCCR)?

Hardcoding the register address seems rather ugly here, so maybe there is a way to have two separate probe methods based on the surrounding register range, and then bind to that?

drivers/soc/renesas/renesas-soc.c line 156

Quoted 8 lines

drivers/soc/renesas/renesas-soc.c new file +8 -0

+static const struct of_device_id renesas_socs[] __initconst = {
+#ifdef CONFIG_ARCH_EMEV2
+     { .compatible = "renesas,emev2",        .data = &soc_emev2 },
+#endif
+#ifdef CONFIG_ARCH_R7S72100
+     { .compatible = "renesas,r7s72100",     .data = &soc_rz_a1h },
+#endif
+#ifdef CONFIG_ARCH_R8A73A4

I think the #ifdefs here will result in warnings for unused symbols when the Kconfig symbols are disabled.

Quoted 9 lines · Arnd Bergmann

drivers/soc/renesas/renesas-soc.c line 54

Quoted 3 lines · CEST Geert Uytterhoeven

drivers/soc/renesas/renesas-soc.c new file +3 -0

+static const struct renesas_family fam_rza __initconst = {
+     .name   = "RZ/A",
+};

I'm not sure about the relationship between this one and the others, maybe it should be treated in the same way as emev2 and left out from this driver?

While RZ/A doesn't have a version registers (AFAIK), it shares several drivers with the other SoCs (SH/R-Mobile, R-Car).
Hence I'd like to keep it, so we can match for it in these drivers when needed. It has e.g. a different variant of the serial port (SCIF), more closely to the one on SH2 rather than SH4.

Quoted 25 lines

Quoted 25 lines

drivers/soc/renesas/renesas-soc.c new file +25 -0

+static const struct renesas_family fam_rmobile __initconst = {
+     .name   = "R-Mobile",
+     .reg    = 0xe600101c,           /* CCCR (Common Chip Code Register) */
+};
+
+static const struct renesas_family fam_rcar_gen1 __initconst = {
+     .name   = "R-Car Gen1",
+     .reg    = 0xff000044,           /* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rcar_gen2 __initconst = {
+     .name   = "R-Car Gen2",
+     .reg    = 0xff000044,           /* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rcar_gen3 __initconst = {
+     .name   = "R-Car Gen3",
+     .reg    = 0xfff00044,           /* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rzg __initconst = {
+     .name   = "RZ/G",
+     .reg    = 0xff000044,           /* PRR (Product Register) */
+};
+

Quoted 8 lines

drivers/soc/renesas/renesas-soc.c line 64

Quoted 4 lines

drivers/soc/renesas/renesas-soc.c new file +4 -0

+static const struct renesas_family fam_shmobile __initconst = {
+     .name   = "SH-Mobile",
+     .reg    = 0xe600101c,           /* CCCR (Common Chip Code Register) */
+};

These seem to fall into two distinct categories, maybe there is a better way to group them. What device contain the two kinds of registers (PRR, CCCR)?

Actually there are three (notice the extra "f" on R-Car Gen3 ;-)

Some SoCs have only CCCR, others have only PRR, some have both. On some SoCs one of them can be accessed from the RealTime CPU core (SH) only.
On some SoCs the register is not documented, but present.
If the PRR exists, it's a better choice, as it contains additional information in the high order bits (representing the presence of each big (CA15/CA57), little (CA7/CA53), and RT (CR7) CPU core). Currently we don't use that information, though.

Grouping them in some other way means we would loose the family name, which is exposed through soc_dev_attr->family.
The usefulness of family names is debatable though, as this is more an issue of marketing business.

Quoted 3 lines

Hardcoding the register address seems rather ugly here, so maybe there is a way to have two separate probe methods based on the surrounding register range, and then bind to that?

There's no simple relation between CCCR/PRR and other register blocks. I prefer not to add these to DT, as that would add one more worm to the backwards compatibility can.

Quoted 11 lines

drivers/soc/renesas/renesas-soc.c line 156

Quoted 8 lines

drivers/soc/renesas/renesas-soc.c new file +8 -0

+static const struct of_device_id renesas_socs[] __initconst = {
+#ifdef CONFIG_ARCH_EMEV2
+     { .compatible = "renesas,emev2",        .data = &soc_emev2 },
+#endif
+#ifdef CONFIG_ARCH_R7S72100
+     { .compatible = "renesas,r7s72100",     .data = &soc_rz_a1h },
+#endif
+#ifdef CONFIG_ARCH_R8A73A4

I think the #ifdefs here will result in warnings for unused symbols when the Kconfig symbols are disabled.

Originally I had __maybe_unused, but it didn't seem to be needed. Do you know which compiler needs it, so I can check?

Quoted 10 lines · CEST Geert Uytterhoeven

Quoted 8 lines · Arnd Bergmann

drivers/soc/renesas/renesas-soc.c line 54

Quoted 3 lines · CEST Geert Uytterhoeven

drivers/soc/renesas/renesas-soc.c new file +3 -0

+static const struct renesas_family fam_rza __initconst = {
+     .name   = "RZ/A",
+};

I'm not sure about the relationship between this one and the others, maybe it should be treated in the same way as emev2 and left out from this driver?

Quoted 5 lines

While RZ/A doesn't have a version registers (AFAIK), it shares several drivers with the other SoCs (SH/R-Mobile, R-Car).
Hence I'd like to keep it, so we can match for it in these drivers when needed. It has e.g. a different variant of the serial port (SCIF), more closely to the one on SH2 rather than SH4.

I'd prefer seeing a separate soc driver for that one.

Quoted 34 lines

Quoted 33 lines

Quoted 19 lines

drivers/soc/renesas/renesas-soc.c new file +19 -0

+static const struct renesas_family fam_rmobile __initconst = {
+     .name   = "R-Mobile",
+     .reg    = 0xe600101c,           /* CCCR (Common Chip Code Register) */
+};
+
+static const struct renesas_family fam_rcar_gen1 __initconst = {
+     .name   = "R-Car Gen1",
+     .reg    = 0xff000044,           /* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rcar_gen2 __initconst = {
+     .name   = "R-Car Gen2",
+     .reg    = 0xff000044,           /* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_rcar_gen3 __initconst = {
+     .name   = "R-Car Gen3",
+     .reg    = 0xfff00044,           /* PRR (Product Register) */
+};

drivers/soc/renesas/renesas-soc.c line 64

Quoted 10 lines

drivers/soc/renesas/renesas-soc.c new file +10 -0

+
+static const struct renesas_family fam_rzg __initconst = {
+     .name   = "RZ/G",
+     .reg    = 0xff000044,           /* PRR (Product Register) */
+};
+
+static const struct renesas_family fam_shmobile __initconst = {
+     .name   = "SH-Mobile",
+     .reg    = 0xe600101c,           /* CCCR (Common Chip Code Register) */
+};

These seem to fall into two distinct categories, maybe there is a better way to group them. What device contain the two kinds of registers (PRR, CCCR)?

Quoted 1 line

Actually there are three (notice the extra "f" on R-Car Gen3 ;-)

I see. Hopefully this is just the same register block at a different location though.

Quoted 9 lines

Some SoCs have only CCCR, others have only PRR, some have both. On some SoCs one of them can be accessed from the RealTime CPU core (SH) only.
On some SoCs the register is not documented, but present.
If the PRR exists, it's a better choice, as it contains additional information in the high order bits (representing the presence of each big (CA15/CA57), little (CA7/CA53), and RT (CR7) CPU core). Currently we don't use that information, though.

Quoted 4 lines

Grouping them in some other way means we would loose the family name, which is exposed through soc_dev_attr->family.
The usefulness of family names is debatable though, as this is more an issue of marketing business.

How about having a table to look up the family name by the value of the PRR or CCCR then?

Quoted 4 lines

Quoted 3 lines

Hardcoding the register address seems rather ugly here, so maybe there is a way to have two separate probe methods based on the surrounding register range, and then bind to that?

Quoted 3 lines

There's no simple relation between CCCR/PRR and other register blocks. I prefer not to add these to DT, as that would add one more worm to the backwards compatibility can.

Hmm, I understand the concern about compatibility with existing DT files, but I also really hate to see hardcoded register addresses.

Any reason against requiring the DT node for future chips though?

How about this:

The driver could report the hardcoded strings for the SoCs it already knows about (you have the table anyway) and not report the revision unless there is a regmap containing the CCCR or the PRR, in which case you use that. Future SoCs will provide the PRR (I assume CCCR is only used on the older ones) through a syscon regmap that we can use to find out the exact revision as well.

The existing DT files can gain the syscon device so you can report the revision on those machines as well, unless you use an old DTB.

Quoted 12 lines

Quoted 11 lines

drivers/soc/renesas/renesas-soc.c line 156

Quoted 8 lines

drivers/soc/renesas/renesas-soc.c new file +8 -0

+static const struct of_device_id renesas_socs[] __initconst = {
+#ifdef CONFIG_ARCH_EMEV2
+     { .compatible = "renesas,emev2",        .data = &soc_emev2 },
+#endif
+#ifdef CONFIG_ARCH_R7S72100
+     { .compatible = "renesas,r7s72100",     .data = &soc_rz_a1h },
+#endif
+#ifdef CONFIG_ARCH_R8A73A4

I think the #ifdefs here will result in warnings for unused symbols when the Kconfig symbols are disabled.

Quoted 2 lines

Originally I had __maybe_unused, but it didn't seem to be needed. Do you know which compiler needs it, so I can check?

Ah, I remember now: gcc doesn't warn for 'static const' variables unless we pass -Wunused-const, which should be enabled with "make W=1", and we might make that the default in the future (after fixing the handful of drivers currently relying on this).

Why not just drop all the #ifdef here? There should be very little overhead in size, especially if all the data is __initconst.

Quoted 17 lines · Arnd Bergmann

Quoted 1 line · CEST Geert Uytterhoeven

On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <[email protected]> wrote:

I'd prefer seeing a separate soc driver for that one.

Quoted 13 lines

Some SoCs have only CCCR, others have only PRR, some have both. On some SoCs one of them can be accessed from the RealTime CPU core (SH) only.
On some SoCs the register is not documented, but present.
If the PRR exists, it's a better choice, as it contains additional information in the high order bits (representing the presence of each big (CA15/CA57), little (CA7/CA53), and RT (CR7) CPU core). Currently we don't use that information, though.

Grouping them in some other way means we would loose the family name, which is exposed through soc_dev_attr->family.
The usefulness of family names is debatable though, as this is more an issue of marketing business.

Quoted 2 lines

How about having a table to look up the family name by the value of the PRR or CCCR then?

Unfortunately there exist SoCs from different families using the same product ID.

And different SoCs from the same family may have a revision register or not (e.g. R-Car H1 has, M1A hasn't).

Quoted 11 lines

How about this:

The driver could report the hardcoded strings for the SoCs it already knows about (you have the table anyway) and not report the revision unless there is a regmap containing the CCCR or the PRR, in which case you use that. Future SoCs will provide the PRR (I assume CCCR is only used on the older ones) through a syscon regmap that we can use to find out the exact revision as well.

The existing DT files can gain the syscon device so you can report the revision on those machines as well, unless you use an old DTB.

Hmm... That means that if we have to add a driver quirk to distinguish between different revisions of the same SoC, we have to update the DTB anyway, to add the CCCR/PRR device node.
We might as well just change the compatible value in that DTB for the device that needs the quirk. Which is what we'd like to avoid in the first place.

Quoted 2 lines

Why not just drop all the #ifdef here? There should be very little overhead in size, especially if all the data is __initconst.

It still saves ca. 3 KiB for a kernel for a single SoC.

Quoted 21 lines · CEST Geert Uytterhoeven

Quoted 19 lines · Arnd Bergmann

Quoted 1 line · CEST Geert Uytterhoeven

On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <[email protected]> wrote:

I'd prefer seeing a separate soc driver for that one.

Quoted 9 lines

Some SoCs have only CCCR, others have only PRR, some have both. On some SoCs one of them can be accessed from the RealTime CPU core (SH) only.
On some SoCs the register is not documented, but present.
If the PRR exists, it's a better choice, as it contains additional information in the high order bits (representing the presence of each big (CA15/CA57), little (CA7/CA53), and RT (CR7) CPU core). Currently we don't use that information, though.

Quoted 4 lines

Grouping them in some other way means we would loose the family name, which is exposed through soc_dev_attr->family.
The usefulness of family names is debatable though, as this is more an issue of marketing business.

How about having a table to look up the family name by the value of the PRR or CCCR then?

Quoted 5 lines

Unfortunately there exist SoCs from different families using the same product ID.

And different SoCs from the same family may have a revision register or not (e.g. R-Car H1 has, M1A hasn't).

Is this something we expect to see more of in the future, or can we expect future chips to handle this more consistently?

Quoted 12 lines

Quoted 11 lines

How about this:

The driver could report the hardcoded strings for the SoCs it already knows about (you have the table anyway) and not report the revision unless there is a regmap containing the CCCR or the PRR, in which case you use that. Future SoCs will provide the PRR (I assume CCCR is only used on the older ones) through a syscon regmap that we can use to find out the exact revision as well.

The existing DT files can gain the syscon device so you can report the revision on those machines as well, unless you use an old DTB.

Quoted 6 lines

Hmm... That means that if we have to add a driver quirk to distinguish between different revisions of the same SoC, we have to update the DTB anyway, to add the CCCR/PRR device node.
We might as well just change the compatible value in that DTB for the device that needs the quirk. Which is what we'd like to avoid in the first place.

Do you have a specific example in mind? If this is only a theoretical problem, we can worry about it when we get there, and then decide if we add a hardcoded register after all.

Quoted 4 lines

Quoted 2 lines

Why not just drop all the #ifdef here? There should be very little overhead in size, especially if all the data is __initconst.

It still saves ca. 3 KiB for a kernel for a single SoC.

Fair enough, that is more than I was expecting from looking at the source. It's probably the of_device_id structures for the most part.

Please just add the __maybe_unused then, to save us a patch in case we make -Wunused-const the default in the future.

Hi Arnd,

Quoted 22 lines · Arnd Bergmann

Quoted 21 lines · CEST Geert Uytterhoeven

Quoted 19 lines · Arnd Bergmann

Quoted 1 line · CEST Geert Uytterhoeven

On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <[email protected]> wrote:

I'd prefer seeing a separate soc driver for that one.

Quoted 9 lines

Some SoCs have only CCCR, others have only PRR, some have both. On some SoCs one of them can be accessed from the RealTime CPU core (SH) only.
On some SoCs the register is not documented, but present.
If the PRR exists, it's a better choice, as it contains additional information in the high order bits (representing the presence of each big (CA15/CA57), little (CA7/CA53), and RT (CR7) CPU core). Currently we don't use that information, though.

Quoted 4 lines

Grouping them in some other way means we would loose the family name, which is exposed through soc_dev_attr->family.
The usefulness of family names is debatable though, as this is more an issue of marketing business.

How about having a table to look up the family name by the value of the PRR or CCCR then?

Quoted 8 lines

Quoted 5 lines

Unfortunately there exist SoCs from different families using the same product ID.

And different SoCs from the same family may have a revision register or not (e.g. R-Car H1 has, M1A hasn't).

Is this something we expect to see more of in the future, or can we expect future chips to handle this more consistently?

I expect to see more of these in the future.

Perhaps I just should forget about the product IDs and (marketing) families, and just stick the CCCR/PRR addresses in the of_device_ids?
Then we'll have SoC names (e.g. "r8a7791") and (optional) revisions (e.g. "ES1.0") to match on.

Quoted 19 lines

Quoted 12 lines

Quoted 11 lines

How about this:

The driver could report the hardcoded strings for the SoCs it already knows about (you have the table anyway) and not report the revision unless there is a regmap containing the CCCR or the PRR, in which case you use that. Future SoCs will provide the PRR (I assume CCCR is only used on the older ones) through a syscon regmap that we can use to find out the exact revision as well.

The existing DT files can gain the syscon device so you can report the revision on those machines as well, unless you use an old DTB.

Quoted 6 lines

Hmm... That means that if we have to add a driver quirk to distinguish between different revisions of the same SoC, we have to update the DTB anyway, to add the CCCR/PRR device node.
We might as well just change the compatible value in that DTB for the device that needs the quirk. Which is what we'd like to avoid in the first place.

Quoted 3 lines

Do you have a specific example in mind? If this is only a theoretical problem, we can worry about it when we get there, and then decide if we add a hardcoded register after all.

For R-Car H3, there are small differences between ES1.0 and ES1.1, and more and larger differences between ES1.x and ES2.0, which need different handling (patches already floating around).

For (old) R-Car H1, the SATA driver already handles "renesas,sata-r8a7790-es1", but so far there didn't exist an established process to specify how that compatible value would end up in the DTB (the in-kernel DTS doesn't have it).

There may be more differences I'm not aware of.

Quoted 34 lines · CEST Geert Uytterhoeven

Quoted 22 lines · Arnd Bergmann

Quoted 21 lines · CEST Geert Uytterhoeven

Quoted 19 lines · Arnd Bergmann

Quoted 1 line · CEST Geert Uytterhoeven

On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <[email protected]> wrote:

I'd prefer seeing a separate soc driver for that one.

Quoted 9 lines

Some SoCs have only CCCR, others have only PRR, some have both. On some SoCs one of them can be accessed from the RealTime CPU core (SH) only.
On some SoCs the register is not documented, but present.
If the PRR exists, it's a better choice, as it contains additional information in the high order bits (representing the presence of each big (CA15/CA57), little (CA7/CA53), and RT (CR7) CPU core). Currently we don't use that information, though.

Quoted 4 lines

Grouping them in some other way means we would loose the family name, which is exposed through soc_dev_attr->family.
The usefulness of family names is debatable though, as this is more an issue of marketing business.

How about having a table to look up the family name by the value of the PRR or CCCR then?

Quoted 8 lines

Quoted 5 lines

Unfortunately there exist SoCs from different families using the same product ID.

And different SoCs from the same family may have a revision register or not (e.g. R-Car H1 has, M1A hasn't).

Is this something we expect to see more of in the future, or can we expect future chips to handle this more consistently?

I expect to see more of these in the future.

Quoted 4 lines

Perhaps I just should forget about the product IDs and (marketing) families, and just stick the CCCR/PRR addresses in the of_device_ids?
Then we'll have SoC names (e.g. "r8a7791") and (optional) revisions (e.g. "ES1.0") to match on.

I don't think listing the marketing names is a problem if we need a full list of all chips in of_device_ids anyway.

I'm still hoping to be able to limit the need for specifying the register addresses in the driver instead.

Quoted 27 lines

Quoted 19 lines

Quoted 12 lines

Quoted 11 lines

How about this:

The driver could report the hardcoded strings for the SoCs it already knows about (you have the table anyway) and not report the revision unless there is a regmap containing the CCCR or the PRR, in which case you use that. Future SoCs will provide the PRR (I assume CCCR is only used on the older ones) through a syscon regmap that we can use to find out the exact revision as well.

The existing DT files can gain the syscon device so you can report the revision on those machines as well, unless you use an old DTB.

Quoted 6 lines

Hmm... That means that if we have to add a driver quirk to distinguish between different revisions of the same SoC, we have to update the DTB anyway, to add the CCCR/PRR device node.
We might as well just change the compatible value in that DTB for the device that needs the quirk. Which is what we'd like to avoid in the first place.

Quoted 3 lines

Do you have a specific example in mind? If this is only a theoretical problem, we can worry about it when we get there, and then decide if we add a hardcoded register after all.

For R-Car H3, there are small differences between ES1.0 and ES1.1, and more and larger differences between ES1.x and ES2.0, which need different handling (patches already floating around).

Quoted 5 lines

For (old) R-Car H1, the SATA driver already handles "renesas,sata-r8a7790-es1", but so far there didn't exist an established process to specify how that compatible value would end up in the DTB (the in-kernel DTS doesn't have it).

There may be more differences I'm not aware of.

Ok, so for R-Car H1, I assume we don't need the driver, it would just be a way to replace the current workaround with a different one, right?

For R-Car H3, do we just require driver changes to work with ES2.0, or also DT changes? If the new chip version already implies a new DT, we can require the presence of a device node that has the correct register number.

Hi all,

Some Renesas SoCs may exist in different revisions, providing slightly different functionalities (e.g. R-Car H3 ES1.x and ES2.0). This needs to be catered for by drivers and/or platform code. The recently proposed soc_device_match() API seems like a good fit to handle this.

This patch series implements the core infrastructure to provide SoC and revision information through the SoC bus for Renesas ARM SoCs. It consists of 4 patches:
- Patch 1 avoids a crash when SoC revision information is needed and provided early,
- Patch 2 (from Arnd) introduces the soc_device_match() API. I don't know if, when, and through which channel this patch is planned to go upstream,
- Patch 3 fixes a bug in soc_device_match(), causing a crash when trying to match on an SoC attribute that is not provided (seen on EMEV2, RZ/A, and R-Car M1A, which lack revision information),
- Patch 4 identifies Renesas SoCs and registers them with the SoC bus.

Tested on (family, machine, soc_id, optional revision):

    Emma Mobile EV2, EMEV2 KZM9D Board, emev2
    RZ/A, Genmai, r7s72100
    R-Mobile, APE6EVM, r8a73a4, ES1.0
    R-Mobile, armadillo 800 eva, r8a7740, ES2.0
    R-Car Gen1, bockw, r8a7778
    R-Car Gen1, marzen, r8a7779, ES1.0
    R-Car Gen2, Lager, r8a7790, ES1.0
    R-Car Gen2, Koelsch, r8a7791, ES1.0
    R-Car Gen2, Gose, r8a7793, ES1.0
    R-Car Gen2, Alt, r8a7794, ES1.0
    R-Car Gen3, Renesas Salvator-X board based on r8a7795, r8a7795, ES1.0
    R-Car Gen3, Renesas Salvator-X board based on r8a7796, r8a7796, ES1.0
    SH-Mobile, KZM-A9-GT, sh73a0, ES2.0

Quoted 20 lines · CEST Geert Uytterhoeven

Some Renesas SoCs may exist in different revisions, providing slightly different functionalities (e.g. R-Car H3 ES1.x and ES2.0). This needs to be catered for by drivers and/or platform code. The recently proposed soc_device_match() API seems like a good fit to handle this.

This patch series implements the core infrastructure to provide SoC and revision information through the SoC bus for Renesas ARM SoCs. It consists of 4 patches:
- Patch 1 avoids a crash when SoC revision information is needed and provided early,
- Patch 2 (from Arnd) introduces the soc_device_match() API. I don't know if, when, and through which channel this patch is planned to go upstream,
- Patch 3 fixes a bug in soc_device_match(), causing a crash when trying to match on an SoC attribute that is not provided (seen on EMEV2, RZ/A, and R-Car M1A, which lack revision information),
- Patch 4 identifies Renesas SoCs and registers them with the SoC bus.

Tested on (family, machine, soc_id, optional revision):

Quoted 13 lines

    Emma Mobile EV2, EMEV2 KZM9D Board, emev2
    RZ/A, Genmai, r7s72100
    R-Mobile, APE6EVM, r8a73a4, ES1.0
    R-Mobile, armadillo 800 eva, r8a7740, ES2.0
    R-Car Gen1, bockw, r8a7778
    R-Car Gen1, marzen, r8a7779, ES1.0
    R-Car Gen2, Lager, r8a7790, ES1.0
    R-Car Gen2, Koelsch, r8a7791, ES1.0
    R-Car Gen2, Gose, r8a7793, ES1.0
    R-Car Gen2, Alt, r8a7794, ES1.0
    R-Car Gen3, Renesas Salvator-X board based on r8a7795, r8a7795, ES1.0
    R-Car Gen3, Renesas Salvator-X board based on r8a7796, r8a7796, ES1.0
    SH-Mobile, KZM-A9-GT, sh73a0, ES2.0

As mentioned in the comment for the driver patch, I think this makes a lot of sense for the machines that have a revision register, in particular when the interpretation of that register is always done the same way, but I'm a bit skeptical about doing it in the same driver for machines that don't have the register.

Matching by a device rather than the SoC platform also has the advantage that there is no need to maintain a list of compatible numbers in the driver.

Hi Arnd,

Quoted 41 lines · Arnd Bergmann

Quoted 20 lines · CEST Geert Uytterhoeven

Some Renesas SoCs may exist in different revisions, providing slightly different functionalities (e.g. R-Car H3 ES1.x and ES2.0). This needs to be catered for by drivers and/or platform code. The recently proposed soc_device_match() API seems like a good fit to handle this.

This patch series implements the core infrastructure to provide SoC and revision information through the SoC bus for Renesas ARM SoCs. It consists of 4 patches:
- Patch 1 avoids a crash when SoC revision information is needed and provided early,
- Patch 2 (from Arnd) introduces the soc_device_match() API. I don't know if, when, and through which channel this patch is planned to go upstream,
- Patch 3 fixes a bug in soc_device_match(), causing a crash when trying to match on an SoC attribute that is not provided (seen on EMEV2, RZ/A, and R-Car M1A, which lack revision information),
- Patch 4 identifies Renesas SoCs and registers them with the SoC bus.

Tested on (family, machine, soc_id, optional revision):

Quoted 13 lines

    Emma Mobile EV2, EMEV2 KZM9D Board, emev2
    RZ/A, Genmai, r7s72100
    R-Mobile, APE6EVM, r8a73a4, ES1.0
    R-Mobile, armadillo 800 eva, r8a7740, ES2.0
    R-Car Gen1, bockw, r8a7778
    R-Car Gen1, marzen, r8a7779, ES1.0
    R-Car Gen2, Lager, r8a7790, ES1.0
    R-Car Gen2, Koelsch, r8a7791, ES1.0
    R-Car Gen2, Gose, r8a7793, ES1.0
    R-Car Gen2, Alt, r8a7794, ES1.0
    R-Car Gen3, Renesas Salvator-X board based on r8a7795, r8a7795, ES1.0
    R-Car Gen3, Renesas Salvator-X board based on r8a7796, r8a7796, ES1.0
    SH-Mobile, KZM-A9-GT, sh73a0, ES2.0

As mentioned in the comment for the driver patch, I think this makes a lot of sense for the machines that have a revision register, in particular when the interpretation of that register is always done the same way, but I'm a bit skeptical about doing it in the same driver for machines that don't have the register.

Quoted 3 lines

Matching by a device rather than the SoC platform also has the advantage that there is no need to maintain a list of compatible numbers in the driver.

Currently we (usually) use:
- SoC-specific compatible values, to handle known differences within the same family now, and handle future unknown differences,
- Family-specific compatible values, which we define ourselves.

Usually drivers match on the latter.

Every time a new SoC is introduced, we have to update lots of DT binding docs, to add the new SoC-specific compatible values.

Two-phase matching (driver code matches against "renesas,<foo>", driver matches against SoC using soc_device_match()) would allow to remove the burden of updating DT documentation all the time. The drivers would need updates, though.
Another advantage would be that we can reuse .dtsi snippets for SoCs in the same family, which we currently can't easily do due to the SoC-specific compatible values.