Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1512507 > unrolled thread

[PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus

Started byGeert Uytterhoeven <geert+renesas@glider.be>
First post2016-10-31 12:40 +0100
Last post2016-11-10 12:40 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus Geert Uytterhoeven <geert+renesas@glider.be> - 2016-10-31 12:40 +0100
    Re: [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus Arnd Bergmann <arnd@arndb.de> - 2016-11-09 18:00 +0100
      Re: [PATCH v2 7/7] soc: renesas: Identify SoC and register with the  SoC bus Geert Uytterhoeven <geert@linux-m68k.org> - 2016-11-10 11:20 +0100
        Re: [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus Arnd Bergmann <arnd@arndb.de> - 2016-11-10 12:40 +0100

#1512507 — [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus

FromGeert Uytterhoeven <geert+renesas@glider.be>
Date2016-10-31 12:40 +0100
Subject[PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus
Message-ID<syj6y-1JI-15@gated-at.bofh.it>
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().

Identification is done using the Product Register or Common Chip Code
Register, as declared in DT, or using a hardcoded fallback if missing.

Example:

    Detected Renesas r8a7791 (0x47) ES1.0
    ...
    # cat /sys/devices/soc0/{family,machine,soc_id,revision}
    Renesas
    Koelsch
    r8a7791
    ES1.0

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Drop SoC families and family names; use fixed "Renesas" instead,
  - Drop EMEV2, which doesn't have a chip ID register, and doesn't share
    devices with other SoCs,
  - Drop RZ/A1H and R-CAR M1A, which don't have chip ID registers (for
    M1A: not accessible from the ARM core?),
  - On arm, move "select SOC_BUS" from ARCH_RENESAS to Kconfig symbols
    for SoCs that provide a chip ID register,
  - Build renesas-soc only if SOC_BUS is enabled,
  - Use "renesas,prr" and "renesas,cccr" device nodes in DT if
    available, else fall back to hardcoded addresses for compatibility
    with existing DTBs,
  - Remove verification of product IDs; just print the ID instead,
  - Don't register the SoC bus if the chip ID register is missing,
  - Change R-Mobile APE6 fallback to use PRR instead of CCCR (it has
    both).
---
 arch/arm/mach-shmobile/Kconfig    |   3 +
 arch/arm64/Kconfig.platforms      |   1 +
 drivers/soc/renesas/Makefile      |   2 +
 drivers/soc/renesas/renesas-soc.c | 130 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 136 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 6fbd9b7d2d67a18f..dd4d52dd71b4eab2 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -21,11 +21,13 @@ config ARCH_RCAR_GEN2
 	select PM
 	select PM_GENERIC_DOMAINS
 	select RENESAS_IRQC
+	select SOC_BUS
 	select SYS_SUPPORTS_SH_CMT
 
 config ARCH_RMOBILE
 	bool
 	select PM_RMOBILE
+	select SOC_BUS
 	select SYS_SUPPORTS_SH_CMT
 	select SYS_SUPPORTS_SH_TMU
 
@@ -80,6 +82,7 @@ config ARCH_R8A7778
 config ARCH_R8A7779
 	bool "R-Car H1 (R8A77790)"
 	select ARCH_RCAR_GEN1
+	select SOC_BUS
 
 config ARCH_R8A7790
 	bool "R-Car H2 (R8A77900)"
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 101794f5ce1008b7..b751c6891c6a51ed 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -143,6 +143,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 9e0bb329594c4fca..1652df037955e0e6 100644
--- a/drivers/soc/renesas/Makefile
+++ b/drivers/soc/renesas/Makefile
@@ -1,3 +1,5 @@
+obj-$(CONFIG_SOC_BUS)		+= renesas-soc.o
+
 obj-$(CONFIG_ARCH_R8A7743)	+= rcar-sysc.o r8a7743-sysc.o
 obj-$(CONFIG_ARCH_R8A7779)	+= rcar-sysc.o r8a7779-sysc.o
 obj-$(CONFIG_ARCH_R8A7790)	+= rcar-sysc.o r8a7790-sysc.o
diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c
new file mode 100644
index 0000000000000000..01104bfaefdb9f32
--- /dev/null
+++ b/drivers/soc/renesas/renesas-soc.c
@@ -0,0 +1,130 @@
+/*
+ * 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/of_address.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/sys_soc.h>
+
+
+#define CCCR	0xe600101c	/* Common Chip Code Register */
+#define PRR	0xff000044	/* Product Register */
+#define PRR3	0xfff00044	/* Product Register on R-Car Gen3 */
+
+static const struct of_device_id renesas_socs[] __initconst = {
+#ifdef CONFIG_ARCH_R8A73A4
+	{ .compatible = "renesas,r8a73a4",	.data = (void *)PRR, },
+#endif
+#ifdef CONFIG_ARCH_R8A7740
+	{ .compatible = "renesas,r8a7740",	.data = (void *)CCCR, },
+#endif
+#ifdef CONFIG_ARCH_R8A7743
+	{ .compatible = "renesas,r8a7743",	.data = (void *)PRR, },
+#endif
+#ifdef CONFIG_ARCH_R8A7745
+	{ .compatible = "renesas,r8a7745",	.data = (void *)PRR, },
+#endif
+#ifdef CONFIG_ARCH_R8A7779
+	{ .compatible = "renesas,r8a7779",	.data = (void *)PRR, },
+#endif
+#ifdef CONFIG_ARCH_R8A7790
+	{ .compatible = "renesas,r8a7790",	.data = (void *)PRR, },
+#endif
+#ifdef CONFIG_ARCH_R8A7791
+	{ .compatible = "renesas,r8a7791",	.data = (void *)PRR, },
+#endif
+#ifdef CONFIG_ARCH_R8A7792
+	{ .compatible = "renesas,r8a7792",	.data = (void *)PRR, },
+#endif
+#ifdef CONFIG_ARCH_R8A7793
+	{ .compatible = "renesas,r8a7793",	.data = (void *)PRR, },
+#endif
+#ifdef CONFIG_ARCH_R8A7794
+	{ .compatible = "renesas,r8a7794",	.data = (void *)PRR, },
+#endif
+#ifdef CONFIG_ARCH_R8A7795
+	{ .compatible = "renesas,r8a7795",	.data = (void *)PRR3, },
+#endif
+#ifdef CONFIG_ARCH_R8A7796
+	{ .compatible = "renesas,r8a7796",	.data = (void *)PRR3, },
+#endif
+#ifdef CONFIG_ARCH_SH73A0
+	{ .compatible = "renesas,sh73a0",	.data = (void *)CCCR, },
+#endif
+	{ /* sentinel */ }
+};
+
+static int __init renesas_soc_init(void)
+{
+	struct soc_device_attribute *soc_dev_attr;
+	const struct of_device_id *match;
+	void __iomem *chipid = NULL;
+	struct soc_device *soc_dev;
+	struct device_node *np;
+	unsigned int product;
+
+	np = of_find_matching_node_and_match(NULL, renesas_socs, &match);
+	if (!np)
+		return -ENODEV;
+
+	of_node_put(np);
+
+	/* Try PRR first, then CCCR, then hardcoded fallback */
+	np = of_find_compatible_node(NULL, NULL, "renesas,prr");
+	if (!np)
+		np = of_find_compatible_node(NULL, NULL, "renesas,cccr");
+	if (np) {
+		chipid = of_iomap(np, 0);
+		of_node_put(np);
+	} else if (match->data) {
+		chipid = ioremap((uintptr_t)match->data, 4);
+	}
+	if (!chipid)
+		return -ENODEV;
+
+	product = readl(chipid);
+	iounmap(chipid);
+
+	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 = "Renesas";
+	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "ES%u.%u",
+					   ((product >> 4) & 0xf) + 1,
+					   product & 0xf);
+	soc_dev_attr->soc_id = kstrdup_const(strchr(match->compatible, ',') + 1,
+					     GFP_KERNEL);
+	pr_info("Detected %s %s (0x%02x) %s\n", soc_dev_attr->family,
+		soc_dev_attr->soc_id, (product >> 8) & 0xff,
+		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(soc_dev_attr);
+		return PTR_ERR(soc_dev);
+	}
+
+	return 0;
+}
+core_initcall(renesas_soc_init);
-- 
1.9.1

[toc] | [next] | [standalone]


#1518308

FromArnd Bergmann <arnd@arndb.de>
Date2016-11-09 18:00 +0100
Message-ID<sBEoa-1rS-43@gated-at.bofh.it>
In reply to#1512507
On Monday, October 31, 2016 12:30:55 PM CET Geert Uytterhoeven wrote:

> v2:
>   - Drop SoC families and family names; use fixed "Renesas" instead,

I think I'd rather have seen the family names left in there, but it's
not important, so up to you.

>   - Use "renesas,prr" and "renesas,cccr" device nodes in DT if
>     available, else fall back to hardcoded addresses for compatibility
>     with existing DTBs,

I only see patches 2, 3, 5, and 7 in my inbox, so I'm lacking the DT
binding for these, among other things.

It does seem wrong to have a device node for a specific register though.
Shouldn't the node be for the block of registers that these are inside
of?

>   - Don't register the SoC bus if the chip ID register is missing,

Why? My objection was to hardcoding the register, not to registering
the device? I think I'd rather see the device registered with an
empty revision string.

> +#define CCCR   0xe600101c      /* Common Chip Code Register */
> +#define PRR    0xff000044      /* Product Register */
> +#define PRR3   0xfff00044      /* Product Register on R-Car Gen3 */
> +
> +static const struct of_device_id renesas_socs[] __initconst = {
> +#ifdef CONFIG_ARCH_R8A73A4
> +       { .compatible = "renesas,r8a73a4",      .data = (void *)PRR, },
> +#endif
> +#ifdef CONFIG_ARCH_R8A7740
> +       { .compatible = "renesas,r8a7740",      .data = (void *)CCCR, },
> +#endif

My preference here would be to list the register address only for
SoCs that are known to need them, while also having .dtb files that
don't have the nodes.

> +static int __init renesas_soc_init(void)
> +{
> +       struct soc_device_attribute *soc_dev_attr;
> +       const struct of_device_id *match;
> +       void __iomem *chipid = NULL;
> +       struct soc_device *soc_dev;
> +       struct device_node *np;
> +       unsigned int product;
> +
> +       np = of_find_matching_node_and_match(NULL, renesas_socs, &match);
> +       if (!np)
> +               return -ENODEV;
> +
> +       of_node_put(np);
> +
> +       /* Try PRR first, then CCCR, then hardcoded fallback */
> +       np = of_find_compatible_node(NULL, NULL, "renesas,prr");
> +       if (!np)
> +               np = of_find_compatible_node(NULL, NULL, "renesas,cccr");
> +       if (np) {
> +               chipid = of_iomap(np, 0);
> +               of_node_put(np);
> +       } else if (match->data) {
> +               chipid = ioremap((uintptr_t)match->data, 4);
> +       }
> +       if (!chipid)
> 

Here, I'd turn the order around and look for the DT nodes of the
devices first. Only if they are not found, look at the compatible
string of the root node. No need to search for a node though,
you know which one it is when you look for a compatible =
"renesas,r8a73a4".

	Arnd

[toc] | [prev] | [next] | [standalone]


#1518831 — Re: [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2016-11-10 11:20 +0100
SubjectRe: [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus
Message-ID<sBUCB-4lx-17@gated-at.bofh.it>
In reply to#1518308
Hi Arnd,

Thanks for your comments!

On Wed, Nov 9, 2016 at 5:55 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday, October 31, 2016 12:30:55 PM CET Geert Uytterhoeven wrote:
>> v2:
>>   - Drop SoC families and family names; use fixed "Renesas" instead,
>
> I think I'd rather have seen the family names left in there, but it's
> not important, so up to you.

They're not useful for matching, as family names may change anytime, and don't
always say much about the hardware capabilities.
E.g. SH-Mobile -> R-Mobile -> R-Car | RZ/A | RZ/G
Some SH-Mobile (even some R-Car) parts are SuperH only, others have ARM and
SuperH.

At least the SoC part numbers are stable (hmm, sh73a0 == r8a73a0).

>>   - Use "renesas,prr" and "renesas,cccr" device nodes in DT if
>>     available, else fall back to hardcoded addresses for compatibility
>>     with existing DTBs,
>
> I only see patches 2, 3, 5, and 7 in my inbox, so I'm lacking the DT
> binding for these, among other things.

I understand you've received them in the mean time?

> It does seem wrong to have a device node for a specific register though.
> Shouldn't the node be for the block of registers that these are inside
> of?

On R-Mobile APE6, R-Car Gen2 and Gen3, PRR is a lone register.
On R-Car Gen1, it's not even documented (and doesn't exist on all parts).

On SH-Mobile/R-Mobile, CCCR may be part of the HPB/APB register block, which
we further don't touch at all.
On R-Car Gen2, it's not documented, but does exist.

BTW, see why I'd prefer not to have it in DT at all, and go for KISS in code
we can change at any time? To avoid mistakes we have to keep on supporting
forever.

>>   - Don't register the SoC bus if the chip ID register is missing,
>
> Why? My objection was to hardcoding the register, not to registering
> the device? I think I'd rather see the device registered with an
> empty revision string.

If there's no chip ID register, there's no reason to use soc_device_match(),
as we can always look at a compatible value. All SoCs listed in this driver
have a chip ID register.

if you want me to register the soc_bus for  those SoCs regardless, I want to
re-add r7s72100 (RZ/A) and r8a7778 (R-Car M1A), who don't have chip ID
registers ;-)

>> +#define CCCR   0xe600101c      /* Common Chip Code Register */
>> +#define PRR    0xff000044      /* Product Register */
>> +#define PRR3   0xfff00044      /* Product Register on R-Car Gen3 */
>> +
>> +static const struct of_device_id renesas_socs[] __initconst = {
>> +#ifdef CONFIG_ARCH_R8A73A4
>> +       { .compatible = "renesas,r8a73a4",      .data = (void *)PRR, },
>> +#endif
>> +#ifdef CONFIG_ARCH_R8A7740
>> +       { .compatible = "renesas,r8a7740",      .data = (void *)CCCR, },
>> +#endif
>
> My preference here would be to list the register address only for
> SoCs that are known to need them, while also having .dtb files that
> don't have the nodes.

Even if drivers don't have to handle differences, there's been a long
outstanding request to print SoC revision information during bootup
(E.g. "Does it still work on ES1.0?"). Hence that covers all SoCs.

>> +static int __init renesas_soc_init(void)
>> +{
>> +       struct soc_device_attribute *soc_dev_attr;
>> +       const struct of_device_id *match;
>> +       void __iomem *chipid = NULL;
>> +       struct soc_device *soc_dev;
>> +       struct device_node *np;
>> +       unsigned int product;
>> +
>> +       np = of_find_matching_node_and_match(NULL, renesas_socs, &match);
>> +       if (!np)
>> +               return -ENODEV;
>> +
>> +       of_node_put(np);
>> +
>> +       /* Try PRR first, then CCCR, then hardcoded fallback */
>> +       np = of_find_compatible_node(NULL, NULL, "renesas,prr");
>> +       if (!np)
>> +               np = of_find_compatible_node(NULL, NULL, "renesas,cccr");
>> +       if (np) {
>> +               chipid = of_iomap(np, 0);
>> +               of_node_put(np);
>> +       } else if (match->data) {
>> +               chipid = ioremap((uintptr_t)match->data, 4);
>> +       }
>> +       if (!chipid)
>>
>
> Here, I'd turn the order around and look for the DT nodes of the
> devices first. Only if they are not found, look at the compatible
> string of the root node. No need to search for a node though,
> you know which one it is when you look for a compatible =
> "renesas,r8a73a4".

"renesas,r8a73a4" is the root node, not the device, so it does not have the
"reg" property for reading the chip ID?

There is no SoC part number in the "renesas,prr" and "renesas,cccr" nodes.
Hence I always need to look at the root nodes.

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

[toc] | [prev] | [next] | [standalone]


#1518885

FromArnd Bergmann <arnd@arndb.de>
Date2016-11-10 12:40 +0100
Message-ID<sBVS1-5xN-7@gated-at.bofh.it>
In reply to#1518831
On Thursday, November 10, 2016 11:19:20 AM CET Geert Uytterhoeven wrote:
> Hi Arnd,
> 
> Thanks for your comments!
> 
> On Wed, Nov 9, 2016 at 5:55 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday, October 31, 2016 12:30:55 PM CET Geert Uytterhoeven wrote:
> >> v2:
> >>   - Drop SoC families and family names; use fixed "Renesas" instead,
> >
> > I think I'd rather have seen the family names left in there, but it's
> > not important, so up to you.
> 
> They're not useful for matching, as family names may change anytime, and don't
> always say much about the hardware capabilities.
> E.g. SH-Mobile -> R-Mobile -> R-Car | RZ/A | RZ/G
> Some SH-Mobile (even some R-Car) parts are SuperH only, others have ARM and
> SuperH.
> 
> At least the SoC part numbers are stable (hmm, sh73a0 == r8a73a0).

I think the marketing names are much more useful for humans looking
at the sysfs files than the kernel doing matching on, but both use
cases are important.

> >>   - Use "renesas,prr" and "renesas,cccr" device nodes in DT if
> >>     available, else fall back to hardcoded addresses for compatibility
> >>     with existing DTBs,
> >
> > I only see patches 2, 3, 5, and 7 in my inbox, so I'm lacking the DT
> > binding for these, among other things.
> 
> I understand you've received them in the mean time?

Yes, I found them in my inbox later, not sure why I didn't see them
at first.

> > It does seem wrong to have a device node for a specific register though.
> > Shouldn't the node be for the block of registers that these are inside
> > of?
> 
> On R-Mobile APE6, R-Car Gen2 and Gen3, PRR is a lone register.
> On R-Car Gen1, it's not even documented (and doesn't exist on all parts).

It just seems odd to have it at address 0xff000044 when all the other
devices are at page-aligned addresses. Do you mean that accessing
0xff000040 or 0xff000048 will result in a bus-level exception for a
missing register and just 0xff000044 is actually valid for access,
or is it just the only thing that is documented?

> On SH-Mobile/R-Mobile, CCCR may be part of the HPB/APB register block, which
> we further don't touch at all.
> On R-Car Gen2, it's not documented, but does exist.

This is where the family names would come in handy ;-) I now have
no idea which chip(s) you are referring to.

If you know the name of the register block, just put it into DT with
that name. The driver can trivially add the right offset.

> >>   - Don't register the SoC bus if the chip ID register is missing,
> >
> > Why? My objection was to hardcoding the register, not to registering
> > the device? I think I'd rather see the device registered with an
> > empty revision string.
> 
> If there's no chip ID register, there's no reason to use soc_device_match(),
> as we can always look at a compatible value. All SoCs listed in this driver
> have a chip ID register.

But you may still have user space tools looking into sysfs, e.g. to
figure out how to install a kernel that the boot loader can find,
or which hardware specific distro packages to install.

> if you want me to register the soc_bus for  those SoCs regardless, I want to
> re-add r7s72100 (RZ/A) and r8a7778 (R-Car M1A), who don't have chip ID
> registers ;-)

Right. Just don't encode too much knowledge about the SoCs into the
driver, so we are prepared for adding new ones: We should still look
for the registers in DT on all chips.

> >> +#define CCCR   0xe600101c      /* Common Chip Code Register */
> >> +#define PRR    0xff000044      /* Product Register */
> >> +#define PRR3   0xfff00044      /* Product Register on R-Car Gen3 */
> >> +
> >> +static const struct of_device_id renesas_socs[] __initconst = {
> >> +#ifdef CONFIG_ARCH_R8A73A4
> >> +       { .compatible = "renesas,r8a73a4",      .data = (void *)PRR, },
> >> +#endif
> >> +#ifdef CONFIG_ARCH_R8A7740
> >> +       { .compatible = "renesas,r8a7740",      .data = (void *)CCCR, },
> >> +#endif
> >
> > My preference here would be to list the register address only for
> > SoCs that are known to need them, while also having .dtb files that
> > don't have the nodes.
> 
> Even if drivers don't have to handle differences, there's been a long
> outstanding request to print SoC revision information during bootup
> (E.g. "Does it still work on ES1.0?"). Hence that covers all SoCs.

Ok, fair enough.

> >> +static int __init renesas_soc_init(void)
> >> +{
> >> +       struct soc_device_attribute *soc_dev_attr;
> >> +       const struct of_device_id *match;
> >> +       void __iomem *chipid = NULL;
> >> +       struct soc_device *soc_dev;
> >> +       struct device_node *np;
> >> +       unsigned int product;
> >> +
> >> +       np = of_find_matching_node_and_match(NULL, renesas_socs, &match);
> >> +       if (!np)
> >> +               return -ENODEV;
> >> +
> >> +       of_node_put(np);
> >> +
> >> +       /* Try PRR first, then CCCR, then hardcoded fallback */
> >> +       np = of_find_compatible_node(NULL, NULL, "renesas,prr");
> >> +       if (!np)
> >> +               np = of_find_compatible_node(NULL, NULL, "renesas,cccr");
> >> +       if (np) {
> >> +               chipid = of_iomap(np, 0);
> >> +               of_node_put(np);
> >> +       } else if (match->data) {
> >> +               chipid = ioremap((uintptr_t)match->data, 4);
> >> +       }
> >> +       if (!chipid)
> >>
> >
> > Here, I'd turn the order around and look for the DT nodes of the
> > devices first. Only if they are not found, look at the compatible
> > string of the root node. No need to search for a node though,
> > you know which one it is when you look for a compatible =
> > "renesas,r8a73a4".
> 
> "renesas,r8a73a4" is the root node, not the device, so it does not have the
> "reg" property for reading the chip ID?

I mean replace of_find_matching_node_and_match() with
of_match_node(renesas_socs, of_root).

It does the same thing, just more efficiently.
 
> There is no SoC part number in the "renesas,prr" and "renesas,cccr" nodes.
> Hence I always need to look at the root nodes.

Not sure what that would protect you from. Could you have a renesas,cccr


	Arnd

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web