diff mbox series

[V4,1/2] soc: imx: Correct i.MX8MP soc device

Message ID 20230529033710.4098568-1-peng.fan@oss.nxp.com
State New
Headers
Series [V4,1/2] soc: imx: Correct i.MX8MP soc device |

Commit Message

Peng Fan (OSS) May 29, 2023, 3:37 a.m. UTC 
  From: Peng Fan <peng.fan@nxp.com>

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 <rasmus.villemoes@prevas.dk>
Closes: https://lore.kernel.org/all/fe5e2b36-6a8e-656c-a4a6-13a47f4871af@prevas.dk/
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V4:
 New patch

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

Comments

Rasmus Villemoes May 30, 2023, 8:06 a.m. UTC | #1 
On 29/05/2023 05.37, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> 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 <rasmus.villemoes@prevas.dk>

That's true.

> Closes: https://lore.kernel.org/all/fe5e2b36-6a8e-656c-a4a6-13a47f4871af@prevas.dk/

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
Peng Fan May 30, 2023, 9:05 a.m. UTC | #2 
> 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 <peng.fan@nxp.com>
> >
> > 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 <rasmus.villemoes@prevas.dk>
> 
> 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
diff mbox series

Patch

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;