diff mbox series

[2/3] soc: renesas: Add support to read LSI DEVID register of RZ/G2{L,LC} SoC's

Message ID 20210604180933.16754-3-prabhakar.mahadev-lad.rj@bp.renesas.com
State Superseded
Headers show
Series SoC identification support for RZ/G2L | expand

Commit Message

Prabhakar Mahadev Lad June 4, 2021, 6:09 p.m. UTC
Add support for reading the LSI DEVID register which is present in
SYSC block of RZ/G2{L,LC} SoC's.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/soc/renesas/renesas-soc.c | 34 ++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

Comments

Geert Uytterhoeven June 9, 2021, 7:27 a.m. UTC | #1
Hi Prabhakar,

On Fri, Jun 4, 2021 at 8:09 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add support for reading the LSI DEVID register which is present in

> SYSC block of RZ/G2{L,LC} SoC's.

>

> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>


Thanks for your patch!

> --- a/drivers/soc/renesas/renesas-soc.c

> +++ b/drivers/soc/renesas/renesas-soc.c

> @@ -56,6 +56,11 @@ static const struct renesas_family fam_rzg2 __initconst __maybe_unused = {

>         .reg    = 0xfff00044,           /* PRR (Product Register) */

>  };

>

> +static const struct renesas_family fam_rzg2l __initconst __maybe_unused = {

> +       .name   = "RZ/G2L",

> +       .reg    = 0x11020a04,


Please don't add hardcoded register addresses for new SoCs (i.e. drop
".reg").  The "renesas,r9a07g044-sysc" is always present.
And if it were missing, the hardcoded fallback would lead into the
classic CCCR/PRR scheme, which is not correct for RZ/G2L...

> @@ -348,6 +361,25 @@ static int __init renesas_soc_init(void)

>                 goto done;

>         }

>

> +       np = of_find_compatible_node(NULL, NULL, "renesas,r9a07g044-sysc");

> +       if (np) {

> +               of_node_put(np);

> +               chipid = ioremap(family->reg, 4);


Just use of_iomap(np, 0)...

> +

> +               if (chipid) {

> +                       product = readl(chipid);


... and add the DEVID offset within the SYSC block here.

> +                       iounmap(chipid);

> +

> +                       if (soc->id && (product & 0xfffffff) != soc->id) {

> +                               pr_warn("SoC mismatch (product = 0x%x)\n",

> +                                       product);

> +                               return -ENODEV;

> +                       }

> +               }

> +

> +               goto done;

> +       }

> +

>         /* Try PRR first, then hardcoded fallback */

>         np = of_find_compatible_node(NULL, NULL, "renesas,prr");

>         if (np) {


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
Lad, Prabhakar June 9, 2021, 3:49 p.m. UTC | #2
Hi Geert,

Thank you for the review.

On Wed, Jun 9, 2021 at 8:27 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>

> Hi Prabhakar,

>

> On Fri, Jun 4, 2021 at 8:09 PM Lad Prabhakar

> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:

> > Add support for reading the LSI DEVID register which is present in

> > SYSC block of RZ/G2{L,LC} SoC's.

> >

> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

>

> Thanks for your patch!

>

> > --- a/drivers/soc/renesas/renesas-soc.c

> > +++ b/drivers/soc/renesas/renesas-soc.c

> > @@ -56,6 +56,11 @@ static const struct renesas_family fam_rzg2 __initconst __maybe_unused = {

> >         .reg    = 0xfff00044,           /* PRR (Product Register) */

> >  };

> >

> > +static const struct renesas_family fam_rzg2l __initconst __maybe_unused = {

> > +       .name   = "RZ/G2L",

> > +       .reg    = 0x11020a04,

>

> Please don't add hardcoded register addresses for new SoCs (i.e. drop

> ".reg").  The "renesas,r9a07g044-sysc" is always present.

> And if it were missing, the hardcoded fallback would lead into the

> classic CCCR/PRR scheme, which is not correct for RZ/G2L...

>

I wanted to avoid iomap for the entire sysc block for just a single register.

> > @@ -348,6 +361,25 @@ static int __init renesas_soc_init(void)

> >                 goto done;

> >         }

> >

> > +       np = of_find_compatible_node(NULL, NULL, "renesas,r9a07g044-sysc");

> > +       if (np) {

> > +               of_node_put(np);

> > +               chipid = ioremap(family->reg, 4);

>

> Just use of_iomap(np, 0)...

>

will do.

> > +

> > +               if (chipid) {

> > +                       product = readl(chipid);

>

> ... and add the DEVID offset within the SYSC block here.

>

will do.

Cheers,
Prabhakar

> > +                       iounmap(chipid);

> > +

> > +                       if (soc->id && (product & 0xfffffff) != soc->id) {

> > +                               pr_warn("SoC mismatch (product = 0x%x)\n",

> > +                                       product);

> > +                               return -ENODEV;

> > +                       }

> > +               }

> > +

> > +               goto done;

> > +       }

> > +

> >         /* Try PRR first, then hardcoded fallback */

> >         np = of_find_compatible_node(NULL, NULL, "renesas,prr");

> >         if (np) {

>

> 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
Geert Uytterhoeven June 9, 2021, 3:57 p.m. UTC | #3
Hi Prabhakar,

On Wed, Jun 9, 2021 at 5:50 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Wed, Jun 9, 2021 at 8:27 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> > On Fri, Jun 4, 2021 at 8:09 PM Lad Prabhakar

> > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:

> > > Add support for reading the LSI DEVID register which is present in

> > > SYSC block of RZ/G2{L,LC} SoC's.

> > >

> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

> >

> > Thanks for your patch!

> >

> > > --- a/drivers/soc/renesas/renesas-soc.c

> > > +++ b/drivers/soc/renesas/renesas-soc.c

> > > @@ -56,6 +56,11 @@ static const struct renesas_family fam_rzg2 __initconst __maybe_unused = {

> > >         .reg    = 0xfff00044,           /* PRR (Product Register) */

> > >  };

> > >

> > > +static const struct renesas_family fam_rzg2l __initconst __maybe_unused = {

> > > +       .name   = "RZ/G2L",

> > > +       .reg    = 0x11020a04,

> >

> > Please don't add hardcoded register addresses for new SoCs (i.e. drop

> > ".reg").  The "renesas,r9a07g044-sysc" is always present.

> > And if it were missing, the hardcoded fallback would lead into the

> > classic CCCR/PRR scheme, which is not correct for RZ/G2L...

> >

> I wanted to avoid iomap for the entire sysc block for just a single register.


The mapping will be rounded up to PAGE_SIZE anyway
(I know, SYSC is 64 KiB, hence larger than the typical page size).

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 mbox series

Patch

diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c
index 0f8eff4a641a..e6b21da281ef 100644
--- a/drivers/soc/renesas/renesas-soc.c
+++ b/drivers/soc/renesas/renesas-soc.c
@@ -56,6 +56,11 @@  static const struct renesas_family fam_rzg2 __initconst __maybe_unused = {
 	.reg	= 0xfff00044,		/* PRR (Product Register) */
 };
 
+static const struct renesas_family fam_rzg2l __initconst __maybe_unused = {
+	.name	= "RZ/G2L",
+	.reg	= 0x11020a04,
+};
+
 static const struct renesas_family fam_shmobile __initconst __maybe_unused = {
 	.name	= "SH-Mobile",
 	.reg	= 0xe600101c,		/* CCCR (Common Chip Code Register) */
@@ -64,7 +69,7 @@  static const struct renesas_family fam_shmobile __initconst __maybe_unused = {
 
 struct renesas_soc {
 	const struct renesas_family *family;
-	u8 id;
+	u32 id;
 };
 
 static const struct renesas_soc soc_rz_a1h __initconst __maybe_unused = {
@@ -131,6 +136,11 @@  static const struct renesas_soc soc_rz_g2h __initconst __maybe_unused = {
 	.id	= 0x4f,
 };
 
+static const struct renesas_soc soc_rz_g2l __initconst __maybe_unused = {
+	.family = &fam_rzg2l,
+	.id     = 0x841c447,
+};
+
 static const struct renesas_soc soc_rcar_m1a __initconst __maybe_unused = {
 	.family	= &fam_rcar_gen1,
 };
@@ -299,6 +309,9 @@  static const struct of_device_id renesas_socs[] __initconst = {
 #ifdef CONFIG_ARCH_R8A779A0
 	{ .compatible = "renesas,r8a779a0",	.data = &soc_rcar_v3u },
 #endif
+#if defined(CONFIG_ARCH_R9A07G044)
+	{ .compatible = "renesas,r9a07g044",	.data = &soc_rz_g2l },
+#endif
 #ifdef CONFIG_ARCH_SH73A0
 	{ .compatible = "renesas,sh73a0",	.data = &soc_shmobile_ag5 },
 #endif
@@ -348,6 +361,25 @@  static int __init renesas_soc_init(void)
 		goto done;
 	}
 
+	np = of_find_compatible_node(NULL, NULL, "renesas,r9a07g044-sysc");
+	if (np) {
+		of_node_put(np);
+		chipid = ioremap(family->reg, 4);
+
+		if (chipid) {
+			product = readl(chipid);
+			iounmap(chipid);
+
+			if (soc->id && (product & 0xfffffff) != soc->id) {
+				pr_warn("SoC mismatch (product = 0x%x)\n",
+					product);
+				return -ENODEV;
+			}
+		}
+
+		goto done;
+	}
+
 	/* Try PRR first, then hardcoded fallback */
 	np = of_find_compatible_node(NULL, NULL, "renesas,prr");
 	if (np) {