2023-05-29 04:21:24

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V4 1/2] soc: imx: Correct i.MX8MP soc device

From: Peng Fan <[email protected]>

i.MX8MP UID is actually 128bits and partitioned into two parts, with 1st
64bits at 0x410 and 0x420, with 2nd 64bits at 0xE00 and 0xE10.

So introduce soc_uid_h for the higher 64bits

Fixes: 18f662a73862 ("soc: imx: Add i.MX8MP SoC driver support")
Reported-by: Rasmus Villemoes <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Peng Fan <[email protected]>
---

V4:
New patch

drivers/soc/imx/soc-imx8m.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
index 1dcd243df567..be26bbdac9fa 100644
--- a/drivers/soc/imx/soc-imx8m.c
+++ b/drivers/soc/imx/soc-imx8m.c
@@ -24,6 +24,7 @@
#define OCOTP_UID_HIGH 0x420

#define IMX8MP_OCOTP_UID_OFFSET 0x10
+#define IMX8MP_OCOTP_UID_HIGH 0xE00

/* Same as ANADIG_DIGPROG_IMX7D */
#define ANADIG_DIGPROG_IMX8MM 0x800
@@ -34,6 +35,7 @@ struct imx8_soc_data {
};

static u64 soc_uid;
+static u64 soc_uid_h;

#ifdef CONFIG_HAVE_ARM_SMCCC
static u32 imx8mq_soc_revision_from_atf(void)
@@ -114,6 +116,12 @@ static void __init imx8mm_soc_uid(void)
soc_uid <<= 32;
soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW + offset);

+ if (offset) {
+ soc_uid_h = readl_relaxed(ocotp_base + IMX8MP_OCOTP_UID_HIGH + 0x10);
+ soc_uid_h <<= 32;
+ soc_uid_h |= readl_relaxed(ocotp_base + IMX8MP_OCOTP_UID_HIGH);
+ }
+
iounmap(ocotp_base);
of_node_put(np);
}
@@ -212,7 +220,12 @@ static int __init imx8_soc_init(void)
goto free_soc;
}

- soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX", soc_uid);
+ if (soc_uid_h)
+ soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX%016llX",
+ soc_uid_h, soc_uid);
+ else
+ soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX", soc_uid);
+
if (!soc_dev_attr->serial_number) {
ret = -ENOMEM;
goto free_rev;
--
2.37.1



2023-05-29 04:21:30

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V4 2/2] soc: imx: support i.MX93 soc device

From: Peng Fan <[email protected]>

i.MX93 Device Unique ID(UID) is in eFuse that could be read through
OCOTP Fuse Shadow Block. i.MX93 UID is 128 bits long.

The overall logic is similar as i.MX8M, so reuse soc-imx8m driver
for i.MX93.

Signed-off-by: Peng Fan <[email protected]>
---

V4:
With patch 1 included, the soc_uid_h moved to patch 1

V3:
Update commit log
Drop uneeded {}

V2:
The ocotp yaml has got R-b from DT maintainer

drivers/soc/imx/Makefile | 2 +-
drivers/soc/imx/soc-imx8m.c | 57 ++++++++++++++++++++++++++++++++++++-
2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
index a28c44a1f16a..83aff181ae51 100644
--- a/drivers/soc/imx/Makefile
+++ b/drivers/soc/imx/Makefile
@@ -7,5 +7,5 @@ obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
obj-$(CONFIG_IMX8M_BLK_CTRL) += imx8m-blk-ctrl.o
obj-$(CONFIG_IMX8M_BLK_CTRL) += imx8mp-blk-ctrl.o
-obj-$(CONFIG_SOC_IMX9) += imx93-src.o imx93-pd.o
+obj-$(CONFIG_SOC_IMX9) += soc-imx8m.o imx93-src.o imx93-pd.o
obj-$(CONFIG_IMX9_BLK_CTRL) += imx93-blk-ctrl.o
diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
index be26bbdac9fa..cca207b90110 100644
--- a/drivers/soc/imx/soc-imx8m.c
+++ b/drivers/soc/imx/soc-imx8m.c
@@ -26,8 +26,11 @@
#define IMX8MP_OCOTP_UID_OFFSET 0x10
#define IMX8MP_OCOTP_UID_HIGH 0xE00

+#define IMX93_OCOTP_UID_OFFSET 0x80c0
+
/* Same as ANADIG_DIGPROG_IMX7D */
#define ANADIG_DIGPROG_IMX8MM 0x800
+#define ANADIG_DIGPROG_IMX93 0x800

struct imx8_soc_data {
char *name;
@@ -149,6 +152,53 @@ static u32 __init imx8mm_soc_revision(void)
return rev;
}

+static void __init imx93_soc_uid(void)
+{
+ void __iomem *ocotp_base;
+ struct device_node *np;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,imx93-ocotp");
+ if (!np)
+ return;
+
+ ocotp_base = of_iomap(np, 0);
+ WARN_ON(!ocotp_base);
+
+ soc_uid = readl_relaxed(ocotp_base + IMX93_OCOTP_UID_OFFSET + 0x8);
+ soc_uid <<= 32;
+ soc_uid |= readl_relaxed(ocotp_base + IMX93_OCOTP_UID_OFFSET + 0xC);
+
+ soc_uid_h = readl_relaxed(ocotp_base + IMX93_OCOTP_UID_OFFSET + 0x0);
+ soc_uid_h <<= 32;
+ soc_uid_h |= readl_relaxed(ocotp_base + IMX93_OCOTP_UID_OFFSET + 0x4);
+
+ iounmap(ocotp_base);
+ of_node_put(np);
+}
+
+static u32 __init imx93_soc_revision(void)
+{
+ struct device_node *np;
+ void __iomem *anatop_base;
+ u32 rev;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,imx93-anatop");
+ if (!np)
+ return 0;
+
+ anatop_base = of_iomap(np, 0);
+ WARN_ON(!anatop_base);
+
+ rev = readl_relaxed(anatop_base + ANADIG_DIGPROG_IMX93);
+
+ iounmap(anatop_base);
+ of_node_put(np);
+
+ imx93_soc_uid();
+
+ return rev;
+}
+
static const struct imx8_soc_data imx8mq_soc_data = {
.name = "i.MX8MQ",
.soc_revision = imx8mq_soc_revision,
@@ -169,11 +219,17 @@ static const struct imx8_soc_data imx8mp_soc_data = {
.soc_revision = imx8mm_soc_revision,
};

+static const struct imx8_soc_data imx93_soc_data = {
+ .name = "i.MX93",
+ .soc_revision = imx93_soc_revision,
+};
+
static __maybe_unused const struct of_device_id imx8_soc_match[] = {
{ .compatible = "fsl,imx8mq", .data = &imx8mq_soc_data, },
{ .compatible = "fsl,imx8mm", .data = &imx8mm_soc_data, },
{ .compatible = "fsl,imx8mn", .data = &imx8mn_soc_data, },
{ .compatible = "fsl,imx8mp", .data = &imx8mp_soc_data, },
+ { .compatible = "fsl,imx93", .data = &imx93_soc_data, },
{ }
};

@@ -225,7 +281,6 @@ static int __init imx8_soc_init(void)
soc_uid_h, soc_uid);
else
soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX", soc_uid);
-
if (!soc_dev_attr->serial_number) {
ret = -ENOMEM;
goto free_rev;
--
2.37.1


2023-05-30 08:21:07

by Rasmus Villemoes

[permalink] [raw]
Subject: Re: [PATCH V4 1/2] soc: imx: Correct i.MX8MP soc device

On 29/05/2023 05.37, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> i.MX8MP UID is actually 128bits and partitioned into two parts, with 1st
> 64bits at 0x410 and 0x420, with 2nd 64bits at 0xE00 and 0xE10.
>
> So introduce soc_uid_h for the higher 64bits

Interestingly, reaching out to our NXP sales rep asking for
clarification resulted in:

On i.MX 8MP Unique ID is 64 bits. Please see here:


https://github.com/nxp-imx/uboot-imx/blob/lf_v2022.04/arch/arm/mach-imx/imx8m/soc.c#L752

So could you guys (and here I'm referring to everybody with an @nxp.com
email) internally _please_ talk to each other and figure out what's what.

And, again assuming that the UID is really 128 bits, nobody has yet
answered why the upper 64 bits are not locked down, nor what would/could
happen if the end user/customer modifies those bits.

> Fixes: 18f662a73862 ("soc: imx: Add i.MX8MP SoC driver support")
> Reported-by: Rasmus Villemoes <[email protected]>

That's true.

> Closes: https://lore.kernel.org/all/[email protected]/

Not at all. We (anybody outside nxp.com, and from what I can tell,
probably quite a few people inside) still lack a complete explanation
for this whole mess - why does the RM say one thing, which gets repeated
by NXP TechSupport in a community forum, while a sales representative
asserts that the current code (in both mainline and downstream linux and
u-boot) is correct, and now you claim that in fact the current code is
not correct.

Rasmus


2023-05-30 09:26:01

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH V4 1/2] soc: imx: Correct i.MX8MP soc device

> Subject: Re: [PATCH V4 1/2] soc: imx: Correct i.MX8MP soc device
>
> On 29/05/2023 05.37, Peng Fan (OSS) wrote:
> > From: Peng Fan <[email protected]>
> >
> > i.MX8MP UID is actually 128bits and partitioned into two parts, with
> > 1st 64bits at 0x410 and 0x420, with 2nd 64bits at 0xE00 and 0xE10.
> >
> > So introduce soc_uid_h for the higher 64bits
>
> Interestingly, reaching out to our NXP sales rep asking for clarification
> resulted in:
>
> On i.MX 8MP Unique ID is 64 bits. Please see here:
>
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
> b.com%2Fnxp-imx%2Fuboot-
> imx%2Fblob%2Flf_v2022.04%2Farch%2Farm%2Fmach-
> imx%2Fimx8m%2Fsoc.c%23L752&data=05%7C01%7Cpeng.fan%40nxp.com
> %7Cc6ca2761f8144f3f78d008db60e4bfe0%7C686ea1d3bc2b4c6fa92cd99c5c
> 301635%7C0%7C0%7C638210307846757073%7CUnknown%7CTWFpbGZsb3
> d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%7C3000%7C%7C%7C&sdata=LTdrBwhNesEEfrcvmw0zV0hA08SbX6ZMI
> qPppwWQ5nk%3D&reserved=0

U-Boot only supports 64bits for now.
struct tag_serialnr {
u32 low;
u32 high;
};

>
> So could you guys (and here I'm referring to everybody with an @nxp.com
> email) internally _please_ talk to each other and figure out what's what.
>
> And, again assuming that the UID is really 128 bits, nobody has yet
> answered why the upper 64 bits are not locked down, nor what
> would/could happen if the end user/customer modifies those bits.
>
> > Fixes: 18f662a73862 ("soc: imx: Add i.MX8MP SoC driver support")
> > Reported-by: Rasmus Villemoes <[email protected]>
>
> That's true.
>
> > Closes:
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .kernel.org%2Fall%2Ffe5e2b36-6a8e-656c-a4a6-
> 13a47f4871af%40prevas.dk%2
> >
> F&data=05%7C01%7Cpeng.fan%40nxp.com%7Cc6ca2761f8144f3f78d008db
> 60e4bfe0
> > %7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63821030784675
> 7073%7CUnk
> >
> nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
> k1haWw
> >
> iLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FqgRvctzHuImX8tFkF3zasB
> mnyG0Uc12
> > fA38i0Pxwus%3D&reserved=0
>
> Not at all. We (anybody outside nxp.com, and from what I can tell, probably
> quite a few people inside) still lack a complete explanation for this whole
> mess - why does the RM say one thing, which gets repeated by NXP
> TechSupport in a community forum, while a sales representative asserts
> that the current code (in both mainline and downstream linux and
> u-boot) is correct, and now you claim that in fact the current code is not
> correct.

What I checked was just RM and fuse map.

Let me try to reach to the lead of the i.MX8MP project and back if any
information.

Regards,
Peng.

>
> Rasmus