| Message ID | 1475572167-29581-5-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Hi Geert, I've been offline some weeks, so sorry if I'm not completely up to date, yet, or miss anything. Overall, having a quick look, the proposal in this patch series and your second series "arm64: renesas: r8a7795: R-Car H3 ES2.0 Prototype" looks nice to me. At least much better than encoding the ESx.x in the device tree as discussed some month ago ;) Two minor comments below: On 04.10.2016 11:09, Geert Uytterhoeven wrote: > 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 <geert+renesas@glider.be> > --- > 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 > > diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig > index b5a3cbe81dd1d1f0..e41d2cbb2c825981 100644 > --- a/arch/arm/mach-shmobile/Kconfig > +++ b/arch/arm/mach-shmobile/Kconfig > @@ -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 > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > index be5d824ebdba2dab..a2675afc61baba8d 100644 > --- a/arch/arm64/Kconfig.platforms > +++ b/arch/arm64/Kconfig.platforms > @@ -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. > > diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile > index 623039c3514cdc34..ae6ae8a11f98aba1 100644 > --- a/drivers/soc/renesas/Makefile > +++ b/drivers/soc/renesas/Makefile > @@ -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 > diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c > new file mode 100644 > index 0000000000000000..74b72e4112b8889e > --- /dev/null > +++ b/drivers/soc/renesas/renesas-soc.c > @@ -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 */ I'm wondering if we want to encode this information in the device tree? From the structs below it looks like this is information would be typically given in the device tree, and not hard coded in this C code? On the other hand, above you mention "catch accidental use of a DTB for a different SoC" which is a nice feature, too. So I just want to talk about the pros & cons, most probably both ways are fine. > +}; > + > +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; > + > + product = readl(mapped); > + iounmap(mapped); > + > + if (((product >> 8) & 0xff) != soc->id) { > + pr_crit("SoC mismatch (product = 0x%x)\n", product); > + return -ENODEV; > + } > + > + esi = ((product >> 4) & 0x0f) + 1; > + esf = product & 0xf; I'm somehow surprised to see that all SoCs covered here use the same way to encode esi and esf? I would have expected that we need different decoding for different SoCs. But if this isn't the case, even better :) Best regards Dirk
On Tuesday, October 4, 2016 11:09:27 AM CEST Geert Uytterhoeven wrote: > 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 > Seems all reasonable. > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > 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. Right, we have moved away from that a while ago, and now just use the device for identification, not to model the device hierarchy. > diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c > new file mode 100644 > index 0000000000000000..74b72e4112b8889e > --- /dev/null > +++ b/drivers/soc/renesas/renesas-soc.c > @@ -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 */ > +}; > + > +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. > +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? > +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) */ > +}; > + > +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? > +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. Arnd
Hi Arnd, On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday, October 4, 2016 11:09:27 AM CEST Geert Uytterhoeven wrote: >> 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 >> > > Seems all reasonable. > >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >> --- >> 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. > > Right, we have moved away from that a while ago, and now just > use the device for identification, not to model the device > hierarchy. OK. >> diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c >> new file mode 100644 >> index 0000000000000000..74b72e4112b8889e >> --- /dev/null >> +++ b/drivers/soc/renesas/renesas-soc.c >> @@ -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 */ >> +}; >> + >> +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. OK. Emma Mobile is special, as it doesn't share any drivers with the other SoCs (NEC vs. Hitachi origin). >> +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. >> +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) */ >> +}; >> + >> +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. > 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. >> +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? Thanks for your comments! 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
On Wednesday, October 19, 2016 10:02:57 AM CEST Geert Uytterhoeven wrote: > On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Tuesday, October 4, 2016 11:09:27 AM CEST Geert Uytterhoeven wrote: > >> +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. I'd prefer seeing a separate soc driver for that one. > >> +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) */ > >> +}; > >> + > >> +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 ;-) I see. Hopefully this is just the same register block at a different location though. > 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. How about having a table to look up the family name by the value of the PRR or CCCR then? > > 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. 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. > >> +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? 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. Arnd
Hi Arnd, On Wed, Oct 19, 2016 at 12:59 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Wednesday, October 19, 2016 10:02:57 AM CEST Geert Uytterhoeven wrote: >> On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> > On Tuesday, October 4, 2016 11:09:27 AM CEST Geert Uytterhoeven wrote: >> >> +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. > > I'd prefer seeing a separate soc driver for that one. OK, that can be done. >> >> +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) */ >> >> +}; >> >> + >> >> +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 ;-) > > I see. Hopefully this is just the same register block at a different > location though. More or less. >> 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. > > 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). >> > 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. > > 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? For future SoCs, we can easily make it mandatory. > 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. >> >> +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? > > 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). OK. > 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. It's been a while I've tried multi_v7_defconfig, but I'm afraid there's no Renesas SoC left that can boot it, due to sheer size. We're facing the same issue with arm64_defconfig soon (it already fails now if you add a small (1.5 MiB) initrd). Not to mention RZ/A1H, which is used on some boards running with its 10 MiB of internal SRAM only. Kernel size does matter, despite no progress on linux-tiny. 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
On Friday, October 21, 2016 8:16:00 PM CEST Geert Uytterhoeven wrote: > On Wed, Oct 19, 2016 at 12:59 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Wednesday, October 19, 2016 10:02:57 AM CEST Geert Uytterhoeven wrote: > >> On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > I'd prefer seeing a separate soc driver for that one. > >> 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. > > > > 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). Is this something we expect to see more of in the future, or can we expect future chips to handle this more consistently? > > 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. 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. > > 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. Arnd
Hi Arnd, On Fri, Oct 21, 2016 at 11:16 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday, October 21, 2016 8:16:00 PM CEST Geert Uytterhoeven wrote: >> On Wed, Oct 19, 2016 at 12:59 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> > On Wednesday, October 19, 2016 10:02:57 AM CEST Geert Uytterhoeven wrote: >> >> On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> > I'd prefer seeing a separate soc driver for that one. >> >> 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. >> > >> > 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). > > 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. >> > 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. > > 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. >> > 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. Yep, ca. 200 bytes per ID. > Please just add the __maybe_unused then, to save us a patch in case > we make -Wunused-const the default in the future. Sure. 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
On Saturday, October 22, 2016 9:44:11 AM CEST Geert Uytterhoeven wrote: > Hi Arnd, > > On Fri, Oct 21, 2016 at 11:16 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Friday, October 21, 2016 8:16:00 PM CEST Geert Uytterhoeven wrote: > >> On Wed, Oct 19, 2016 at 12:59 PM, Arnd Bergmann <arnd@arndb.de> wrote: > >> > On Wednesday, October 19, 2016 10:02:57 AM CEST Geert Uytterhoeven wrote: > >> >> On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <arnd@arndb.de> wrote: > >> > I'd prefer seeing a separate soc driver for that one. > >> >> 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. > >> > > >> > 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). > > > > 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. 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. > >> > 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. > > > > 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. 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. Arnd
Hi Arnd, On Sat, Oct 29, 2016 at 11:27 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Saturday, October 22, 2016 9:44:11 AM CEST Geert Uytterhoeven wrote: >> On Fri, Oct 21, 2016 at 11:16 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> > On Friday, October 21, 2016 8:16:00 PM CEST Geert Uytterhoeven wrote: >> >> On Wed, Oct 19, 2016 at 12:59 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> >> > On Wednesday, October 19, 2016 10:02:57 AM CEST Geert Uytterhoeven wrote: >> >> >> On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> >> > I'd prefer seeing a separate soc driver for that one. >> >> >> 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. >> >> > >> >> > 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). >> > >> > 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. > > 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 removing the marketing names. We don't match them anyway (and probably shouldn't, as we don't control them anyway, cfr. https://www.renesas.com/en-us/solutions/automotive/products.html). > I'm still hoping to be able to limit the need for specifying the > register addresses in the driver instead. Adding DT binding... >> >> > 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. >> > >> > 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. > > 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. H3 also needs DT changes for some features (e.g. different number of USB channels, different topology for graphics). soc_device_match() would mostly (only?) be used to handle limitations and quirks in early revisions. These are intended to be removed once production has been ramped up, and there's no longer a need to support them. However, that also means soc_device_match() would be used to match against early revisions, not against late revisions. I.e. the early SoCs need the chip ID registers declared, not the new ones. Stay tuned for v2... 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
diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig index b5a3cbe81dd1d1f0..e41d2cbb2c825981 100644 --- a/arch/arm/mach-shmobile/Kconfig +++ b/arch/arm/mach-shmobile/Kconfig @@ -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 diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms index be5d824ebdba2dab..a2675afc61baba8d 100644 --- a/arch/arm64/Kconfig.platforms +++ b/arch/arm64/Kconfig.platforms @@ -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. diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile index 623039c3514cdc34..ae6ae8a11f98aba1 100644 --- a/drivers/soc/renesas/Makefile +++ b/drivers/soc/renesas/Makefile @@ -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 diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c new file mode 100644 index 0000000000000000..74b72e4112b8889e --- /dev/null +++ b/drivers/soc/renesas/renesas-soc.c @@ -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 */ +}; + +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; + + product = readl(mapped); + iounmap(mapped); + + if (((product >> 8) & 0xff) != soc->id) { + pr_crit("SoC mismatch (product = 0x%x)\n", product); + return -ENODEV; + } + + esi = ((product >> 4) & 0x0f) + 1; + esf = product & 0xf; + } + + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL); + if (!soc_dev_attr) + return -ENOMEM; + + np = of_find_node_by_path("/"); + of_property_read_string(np, "model", &soc_dev_attr->machine); + of_node_put(np); + + soc_dev_attr->family = kstrdup_const(family->name, GFP_KERNEL); + soc_dev_attr->soc_id = kstrdup_const(strchr(match->compatible, ',') + 1, + GFP_KERNEL); + if (esi > 0) + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "ES%u.%u", esi, + esf); + + pr_info("Detected Renesas %s %s\n", soc_dev_attr->soc_id, + soc_dev_attr->revision ?: ""); + + soc_dev = soc_device_register(soc_dev_attr); + if (IS_ERR(soc_dev)) { + kfree(soc_dev_attr->revision); + kfree_const(soc_dev_attr->soc_id); + kfree_const(soc_dev_attr->family); + kfree(soc_dev_attr); + return PTR_ERR(soc_dev); + } + + return 0; +} +core_initcall(renesas_soc_init);
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 <geert+renesas@glider.be> --- 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