| Message ID | 20210604180933.16754-3-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | SoC identification support for RZ/G2L | expand |
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
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
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
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) {
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(-)