2022-01-22 00:29:28

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v2] soc: renesas: Add support for reading product revision for RZ/G2L family

From: Biju Das <[email protected]>

As per RZ/G2L HW manual (Rev.1.00 Sep, 2021) DEV_ID [31:28] indicates
product revision. Use this information to populate the revision info
for RZ/G2L family.

Signed-off-by: Biju Das <[email protected]>
Signed-off-by: Lad Prabhakar <[email protected]>
---
v1->v2
* Fixed freeing up soc_dev_attr in error path.

Output from SMARC RZ/G2L:
root@smarc-rzg2l:~# for i in machine family soc_id revision; do echo -n "$i: ";cat /sys/devices/soc0/$i; done
machine: Renesas SMARC EVK based on r9a07g044l2
family: RZ/G2L
soc_id: r9a07g044
revision: Rev 1
root@smarc-rzg2l:~#
root@smarc-rzg2l:~#
---
drivers/soc/renesas/renesas-soc.c | 49 ++++++++++++++++++-------------
1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c
index 7da0ea3587c4..44e365b36b26 100644
--- a/drivers/soc/renesas/renesas-soc.c
+++ b/drivers/soc/renesas/renesas-soc.c
@@ -371,6 +371,7 @@ static int __init renesas_soc_init(void)
struct soc_device *soc_dev;
struct device_node *np;
const char *soc_id;
+ int ret;

match = of_match_node(renesas_socs, of_root);
if (!match)
@@ -391,6 +392,17 @@ static int __init renesas_soc_init(void)
chipid = ioremap(family->reg, 4);
}

+ 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(soc_id, GFP_KERNEL);
+
if (chipid) {
product = readl(chipid + id->offset);
iounmap(chipid);
@@ -405,41 +417,38 @@ static int __init renesas_soc_init(void)

eshi = ((product >> 4) & 0x0f) + 1;
eslo = product & 0xf;
+ soc_dev_attr->revision = kasprintf(GFP_KERNEL, "ES%u.%u",
+ eshi, eslo);
+ } else if (id == &id_rzg2l) {
+ eshi = ((product >> 28) & 0x0f);
+ soc_dev_attr->revision = kasprintf(GFP_KERNEL, "Rev %u",
+ eshi);
}

if (soc->id &&
((product & id->mask) >> __ffs(id->mask)) != soc->id) {
pr_warn("SoC mismatch (product = 0x%x)\n", product);
- return -ENODEV;
+ ret = -ENODEV;
+ goto free_soc_dev_attr;
}
}

- 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(soc_id, GFP_KERNEL);
- if (eshi)
- soc_dev_attr->revision = kasprintf(GFP_KERNEL, "ES%u.%u", eshi,
- eslo);
-
pr_info("Detected Renesas %s %s %s\n", soc_dev_attr->family,
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);
+ ret = PTR_ERR(soc_dev);
+ goto free_soc_dev_attr;
}

return 0;
+
+free_soc_dev_attr:
+ kfree(soc_dev_attr->revision);
+ kfree_const(soc_dev_attr->soc_id);
+ kfree_const(soc_dev_attr->family);
+ kfree(soc_dev_attr);
+ return ret;
}
early_initcall(renesas_soc_init);
--
2.17.1


2022-01-27 17:56:22

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2] soc: renesas: Add support for reading product revision for RZ/G2L family

Hi Prabhakar,

On Fri, Jan 21, 2022 at 2:41 AM Lad Prabhakar
<[email protected]> wrote:
> From: Biju Das <[email protected]>
>
> As per RZ/G2L HW manual (Rev.1.00 Sep, 2021) DEV_ID [31:28] indicates
> product revision. Use this information to populate the revision info
> for RZ/G2L family.
>
> Signed-off-by: Biju Das <[email protected]>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> v1->v2
> * Fixed freeing up soc_dev_attr in error path.

Thanks for the update!

Reviewed-by: Geert Uytterhoeven <[email protected]>
i.e. will queue in renesas-devel for v5.18.

> --- a/drivers/soc/renesas/renesas-soc.c
> +++ b/drivers/soc/renesas/renesas-soc.c
> @@ -405,41 +417,38 @@ static int __init renesas_soc_init(void)
>
> eshi = ((product >> 4) & 0x0f) + 1;
> eslo = product & 0xf;
> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "ES%u.%u",
> + eshi, eslo);
> + } else if (id == &id_rzg2l) {
> + eshi = ((product >> 28) & 0x0f);
> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "Rev %u",
> + eshi);

Would you mind if I would drop the "Rev " while applying?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-01-27 20:02:12

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v2] soc: renesas: Add support for reading product revision for RZ/G2L family

Hi Geert,

Thank you for the review.

On Thu, Jan 27, 2022 at 10:12 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Fri, Jan 21, 2022 at 2:41 AM Lad Prabhakar
> <[email protected]> wrote:
> > From: Biju Das <[email protected]>
> >
> > As per RZ/G2L HW manual (Rev.1.00 Sep, 2021) DEV_ID [31:28] indicates
> > product revision. Use this information to populate the revision info
> > for RZ/G2L family.
> >
> > Signed-off-by: Biju Das <[email protected]>
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > v1->v2
> > * Fixed freeing up soc_dev_attr in error path.
>
> Thanks for the update!
>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
> i.e. will queue in renesas-devel for v5.18.
>
> > --- a/drivers/soc/renesas/renesas-soc.c
> > +++ b/drivers/soc/renesas/renesas-soc.c
> > @@ -405,41 +417,38 @@ static int __init renesas_soc_init(void)
> >
> > eshi = ((product >> 4) & 0x0f) + 1;
> > eslo = product & 0xf;
> > + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "ES%u.%u",
> > + eshi, eslo);
> > + } else if (id == &id_rzg2l) {
> > + eshi = ((product >> 28) & 0x0f);
> > + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "Rev %u",
> > + eshi);
>
> Would you mind if I would drop the "Rev " while applying?
>
Fine by me as it's already assigned to revision so there isn't any
point having "Rev" prepended to it.

Cheers,
Prabhakar

2022-02-02 11:26:30

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2] soc: renesas: Add support for reading product revision for RZ/G2L family

Hi Biju,

On Wed, Feb 2, 2022 at 10:51 AM Biju Das <[email protected]> wrote:
> > Subject: Re: [PATCH v2] soc: renesas: Add support for reading product
> > revision for RZ/G2L family
> > On Fri, Jan 21, 2022 at 2:41 AM Lad Prabhakar <prabhakar.mahadev-
> > [email protected]> wrote:
> > > From: Biju Das <[email protected]>
> > > As per RZ/G2L HW manual (Rev.1.00 Sep, 2021) DEV_ID [31:28] indicates
> > > product revision. Use this information to populate the revision info
> > > for RZ/G2L family.
> > >
> > > Signed-off-by: Biju Das <[email protected]>
> > > Signed-off-by: Lad Prabhakar <[email protected]>

> > > --- a/drivers/soc/renesas/renesas-soc.c
> > > +++ b/drivers/soc/renesas/renesas-soc.c
> > > @@ -405,41 +417,38 @@ static int __init renesas_soc_init(void)
> > >
> > > eshi = ((product >> 4) & 0x0f) + 1;
> > > eslo = product & 0xf;
> > > + soc_dev_attr->revision = kasprintf(GFP_KERNEL,
> > "ES%u.%u",
> > > + eshi, eslo);
> > > + } else if (id == &id_rzg2l) {
> > > + eshi = ((product >> 28) & 0x0f);
> > > + soc_dev_attr->revision = kasprintf(GFP_KERNEL,
> > "Rev %u",
> > > + eshi);
> >
> > Would you mind if I would drop the "Rev " while applying?
>
> Kernel reports the below message after dropping Rev. Is it OK?
>
> [ 0.018297] Detected Renesas RZ/G2L r9a07g044 1

That's indeed not so nice...

Either we have to add it back, or do something like:

- pr_info("Detected Renesas %s %s %s\n", soc_dev_attr->family,
- soc_dev_attr->soc_id, soc_dev_attr->revision ?: "");
+ pr_info("Detected Renesas %s %s%s%s\n", soc_dev_attr->family,
+ soc_dev_attr->soc_id, soc_dev_attr->revision ? " Rev " : "",
+ soc_dev_attr->revision ?: "");

Any other options?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-02-02 18:04:55

by Biju Das

[permalink] [raw]
Subject: RE: [PATCH v2] soc: renesas: Add support for reading product revision for RZ/G2L family

Hi Geert,

> Subject: Re: [PATCH v2] soc: renesas: Add support for reading product
> revision for RZ/G2L family
>
> Hi Prabhakar,
>
> On Fri, Jan 21, 2022 at 2:41 AM Lad Prabhakar <prabhakar.mahadev-
> [email protected]> wrote:
> > From: Biju Das <[email protected]>
> >
> > As per RZ/G2L HW manual (Rev.1.00 Sep, 2021) DEV_ID [31:28] indicates
> > product revision. Use this information to populate the revision info
> > for RZ/G2L family.
> >
> > Signed-off-by: Biju Das <[email protected]>
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > v1->v2
> > * Fixed freeing up soc_dev_attr in error path.
>
> Thanks for the update!
>
> Reviewed-by: Geert Uytterhoeven <[email protected]> i.e. will queue
> in renesas-devel for v5.18.
>
> > --- a/drivers/soc/renesas/renesas-soc.c
> > +++ b/drivers/soc/renesas/renesas-soc.c
> > @@ -405,41 +417,38 @@ static int __init renesas_soc_init(void)
> >
> > eshi = ((product >> 4) & 0x0f) + 1;
> > eslo = product & 0xf;
> > + soc_dev_attr->revision = kasprintf(GFP_KERNEL,
> "ES%u.%u",
> > + eshi, eslo);
> > + } else if (id == &id_rzg2l) {
> > + eshi = ((product >> 28) & 0x0f);
> > + soc_dev_attr->revision = kasprintf(GFP_KERNEL,
> "Rev %u",
> > + eshi);
>
> Would you mind if I would drop the "Rev " while applying?

Kernel reports the below message after dropping Rev. Is it OK?

[ 0.018297] Detected Renesas RZ/G2L r9a07g044 1

Cheers,
Biju

2022-02-03 07:51:31

by Biju Das

[permalink] [raw]
Subject: RE: [PATCH v2] soc: renesas: Add support for reading product revision for RZ/G2L family

Hi Geert,


> Subject: Re: [PATCH v2] soc: renesas: Add support for reading product
> revision for RZ/G2L family
>
> Hi Biju,
>
> On Wed, Feb 2, 2022 at 10:51 AM Biju Das <[email protected]>
> wrote:
> > > Subject: Re: [PATCH v2] soc: renesas: Add support for reading
> > > product revision for RZ/G2L family On Fri, Jan 21, 2022 at 2:41 AM
> > > Lad Prabhakar <prabhakar.mahadev- [email protected]> wrote:
> > > > From: Biju Das <[email protected]> As per RZ/G2L HW
> > > > manual (Rev.1.00 Sep, 2021) DEV_ID [31:28] indicates product
> > > > revision. Use this information to populate the revision info for
> > > > RZ/G2L family.
> > > >
> > > > Signed-off-by: Biju Das <[email protected]>
> > > > Signed-off-by: Lad Prabhakar
> > > > <[email protected]>
>
> > > > --- a/drivers/soc/renesas/renesas-soc.c
> > > > +++ b/drivers/soc/renesas/renesas-soc.c
> > > > @@ -405,41 +417,38 @@ static int __init renesas_soc_init(void)
> > > >
> > > > eshi = ((product >> 4) & 0x0f) + 1;
> > > > eslo = product & 0xf;
> > > > + soc_dev_attr->revision =
> > > > + kasprintf(GFP_KERNEL,
> > > "ES%u.%u",
> > > > + eshi,
> eslo);
> > > > + } else if (id == &id_rzg2l) {
> > > > + eshi = ((product >> 28) & 0x0f);
> > > > + soc_dev_attr->revision =
> > > > + kasprintf(GFP_KERNEL,
> > > "Rev %u",
> > > > + eshi);
> > >
> > > Would you mind if I would drop the "Rev " while applying?
> >
> > Kernel reports the below message after dropping Rev. Is it OK?
> >
> > [ 0.018297] Detected Renesas RZ/G2L r9a07g044 1
>
> That's indeed not so nice...
>
> Either we have to add it back, or do something like:

This is much better.

[ 0.003427] Detected Renesas RZ/G2L r9a07g044 Rev 1
root@smarc-rzg2l:~# for i in machine family soc_id revision; do echo -n "$i: ";cat /sys/devices/soc0/$i; done
machine: Renesas SMARC EVK based on r9a07g044l2
family: RZ/G2L
soc_id: r9a07g044
revision: 1
root@smarc-rzg2l:~#

>
> - pr_info("Detected Renesas %s %s %s\n", soc_dev_attr->family,
> - soc_dev_attr->soc_id, soc_dev_attr->revision ?: "");
> + pr_info("Detected Renesas %s %s%s%s\n", soc_dev_attr->family,
> + soc_dev_attr->soc_id, soc_dev_attr->revision ? " Rev " :
> "",
> + soc_dev_attr->revision ?: "");
>

Will you post this change or Do you want me to send the patch?

Please let me know.

Cheers,
Biju

2022-02-03 15:53:25

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2] soc: renesas: Add support for reading product revision for RZ/G2L family

Hi Biju,

On Wed, Feb 2, 2022 at 12:20 PM Biju Das <[email protected]> wrote:
> > Subject: Re: [PATCH v2] soc: renesas: Add support for reading product
> > revision for RZ/G2L family
> > On Wed, Feb 2, 2022 at 10:51 AM Biju Das <[email protected]>
> > wrote:
> > > > Subject: Re: [PATCH v2] soc: renesas: Add support for reading
> > > > product revision for RZ/G2L family On Fri, Jan 21, 2022 at 2:41 AM
> > > > Lad Prabhakar <prabhakar.mahadev- [email protected]> wrote:
> > > > > From: Biju Das <[email protected]> As per RZ/G2L HW
> > > > > manual (Rev.1.00 Sep, 2021) DEV_ID [31:28] indicates product
> > > > > revision. Use this information to populate the revision info for
> > > > > RZ/G2L family.
> > > > >
> > > > > Signed-off-by: Biju Das <[email protected]>
> > > > > Signed-off-by: Lad Prabhakar
> > > > > <[email protected]>
> >
> > > > > --- a/drivers/soc/renesas/renesas-soc.c
> > > > > +++ b/drivers/soc/renesas/renesas-soc.c
> > > > > @@ -405,41 +417,38 @@ static int __init renesas_soc_init(void)
> > > > >
> > > > > eshi = ((product >> 4) & 0x0f) + 1;
> > > > > eslo = product & 0xf;
> > > > > + soc_dev_attr->revision =
> > > > > + kasprintf(GFP_KERNEL,
> > > > "ES%u.%u",
> > > > > + eshi,
> > eslo);
> > > > > + } else if (id == &id_rzg2l) {
> > > > > + eshi = ((product >> 28) & 0x0f);
> > > > > + soc_dev_attr->revision =
> > > > > + kasprintf(GFP_KERNEL,
> > > > "Rev %u",
> > > > > + eshi);
> > > >
> > > > Would you mind if I would drop the "Rev " while applying?
> > >
> > > Kernel reports the below message after dropping Rev. Is it OK?
> > >
> > > [ 0.018297] Detected Renesas RZ/G2L r9a07g044 1
> >
> > That's indeed not so nice...
> >
> > Either we have to add it back, or do something like:
>
> This is much better.
>
> [ 0.003427] Detected Renesas RZ/G2L r9a07g044 Rev 1
> root@smarc-rzg2l:~# for i in machine family soc_id revision; do echo -n "$i: ";cat /sys/devices/soc0/$i; done
> machine: Renesas SMARC EVK based on r9a07g044l2
> family: RZ/G2L
> soc_id: r9a07g044
> revision: 1
> root@smarc-rzg2l:~#
>
> >
> > - pr_info("Detected Renesas %s %s %s\n", soc_dev_attr->family,
> > - soc_dev_attr->soc_id, soc_dev_attr->revision ?: "");
> > + pr_info("Detected Renesas %s %s%s%s\n", soc_dev_attr->family,
> > + soc_dev_attr->soc_id, soc_dev_attr->revision ? " Rev " :
> > "",
> > + soc_dev_attr->revision ?: "");
> >
>
> Will you post this change or Do you want me to send the patch?
>
> Please let me know.

I'll send a patch, to be folded in to the original.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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