diff mbox series

ARM: mach-imx: set system_serial from ocotp registers

Message ID 1606404505-133200-1-git-send-email-r.karszniewicz@phytec.de
State New
Headers show
Series ARM: mach-imx: set system_serial from ocotp registers | expand

Commit Message

Robert Karszniewicz Nov. 26, 2020, 3:28 p.m. UTC 
From: Stefan Christ <s.christ@phytec.de>

The i.MX6 fuses have two 32 bit registers (OCOTP_CFG0 and OCOTP_CFG1)
that contain an 64 bit unique id. Use them to setup the system serial,
which can be seen in '/proc/cpuinfo'.

Signed-off-by: Stefan Christ <s.christ@phytec.de>
Signed-off-by: Yunus Bas <y.bas@phytec.de>
Cc: Stefan Lengfeld <contact@stefanchrist.eu>
Signed-off-by: Robert Karszniewicz <r.karszniewicz@phytec.de>
---
Hello,

I've noticed that arch/arm/mach-imx/ocotp.c duplicates functionality of
drivers/soc/imx/soc-imx.c . Should this be resolved?

Thanks.

 arch/arm/mach-imx/Makefile      |  1 +
 arch/arm/mach-imx/common.h      |  1 +
 arch/arm/mach-imx/mach-imx6q.c  |  1 +
 arch/arm/mach-imx/mach-imx6sl.c |  1 +
 arch/arm/mach-imx/mach-imx6sx.c |  1 +
 arch/arm/mach-imx/mach-imx6ul.c |  3 +++
 arch/arm/mach-imx/ocotp.c       | 40 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 48 insertions(+)
 create mode 100644 arch/arm/mach-imx/ocotp.c

Comments

Arnd Bergmann Nov. 26, 2020, 7:35 p.m. UTC | #1 
On Thu, Nov 26, 2020 at 4:28 PM Robert Karszniewicz
<r.karszniewicz@phytec.de> wrote:
>
> From: Stefan Christ <s.christ@phytec.de>
>
> The i.MX6 fuses have two 32 bit registers (OCOTP_CFG0 and OCOTP_CFG1)
> that contain an 64 bit unique id. Use them to setup the system serial,
> which can be seen in '/proc/cpuinfo'.
>
> Signed-off-by: Stefan Christ <s.christ@phytec.de>
> Signed-off-by: Yunus Bas <y.bas@phytec.de>
> Cc: Stefan Lengfeld <contact@stefanchrist.eu>
> Signed-off-by: Robert Karszniewicz <r.karszniewicz@phytec.de>
> ---
> Hello,
>
> I've noticed that arch/arm/mach-imx/ocotp.c duplicates functionality of
> drivers/soc/imx/soc-imx.c . Should this be resolved?

I think it would be best to just have drivers/soc/imx/soc-imx.c
and not add another one. In particular, the serial number is
not normally exposed through system_serial_low/system_serial_high
these days, but rather the architecture-independent soc device.

Looking at that driver though, I see it still has a bug in that
it creates a bogus soc_device on most (non-imx) machines.
I think there was a fix a while ago, but it never got merged.

        Arnd
Stefan Lengfeld Nov. 27, 2020, 10:22 a.m. UTC | #2 
Hi Arnd and Robert,

On Thu, Nov 26, 2020 at 08:35:24PM +0100, Arnd Bergmann wrote:
> On Thu, Nov 26, 2020 at 4:28 PM Robert Karszniewicz
> <r.karszniewicz@phytec.de> wrote:
> >
> > From: Stefan Christ <s.christ@phytec.de>
> >
> > The i.MX6 fuses have two 32 bit registers (OCOTP_CFG0 and OCOTP_CFG1)
> > that contain an 64 bit unique id. Use them to setup the system serial,
> > which can be seen in '/proc/cpuinfo'.
> >
> > Signed-off-by: Stefan Christ <s.christ@phytec.de>
> > Signed-off-by: Yunus Bas <y.bas@phytec.de>
> > Cc: Stefan Lengfeld <contact@stefanchrist.eu>
> > Signed-off-by: Robert Karszniewicz <r.karszniewicz@phytec.de>
> > ---
> > Hello,
> >
> > I've noticed that arch/arm/mach-imx/ocotp.c duplicates functionality of
> > drivers/soc/imx/soc-imx.c . Should this be resolved?
> 
> I think it would be best to just have drivers/soc/imx/soc-imx.c
> and not add another one. In particular, the serial number is
> not normally exposed through system_serial_low/system_serial_high
> these days, but rather the architecture-independent soc device.
  
I would also suggest to just drop the patch.

Reading the serial number was added in 2019/v5.5 to the mainline kernel.
See commit 8267ff89b7131 ("ARM: imx: Add serial number support for
i.MX6/7 SoCs"):

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8267ff89b7131

It's just exposed at "/sys/devices/soc0/serial_number" instead of in
"/proc/cpuinfo" to userspace.

Kind regards,
Stefan
Robert Karszniewicz Nov. 27, 2020, 5:12 p.m. UTC | #3 
Hello Arnd and Stefan,

On 11/26/20 8:35 PM, Arnd Bergmann wrote:
> On Thu, Nov 26, 2020 at 4:28 PM Robert Karszniewicz
> <r.karszniewicz@phytec.de> wrote:
>>
>> From: Stefan Christ <s.christ@phytec.de>
>>
>> The i.MX6 fuses have two 32 bit registers (OCOTP_CFG0 and OCOTP_CFG1)
>> that contain an 64 bit unique id. Use them to setup the system serial,
>> which can be seen in '/proc/cpuinfo'.
>>
>> Signed-off-by: Stefan Christ <s.christ@phytec.de>
>> Signed-off-by: Yunus Bas <y.bas@phytec.de>
>> Cc: Stefan Lengfeld <contact@stefanchrist.eu>
>> Signed-off-by: Robert Karszniewicz <r.karszniewicz@phytec.de>
>> ---
>> Hello,
>>
>> I've noticed that arch/arm/mach-imx/ocotp.c duplicates functionality of
>> drivers/soc/imx/soc-imx.c . Should this be resolved?
> 
> I think it would be best to just have drivers/soc/imx/soc-imx.c
> and not add another one. In particular, the serial number is
> not normally exposed through system_serial_low/system_serial_high
> these days, but rather the architecture-independent soc device.

I've done a bit more research and I think I understand better now.

In the same vein, our patch
"[PATCH 1/2] ARM: mach-imx: imx6ul: Print SOC revision on boot and set
system_rev"[1]
should drop the line where system_rev is set, and only 
imx_print_silicon_rev()
should be kept? And the second patch in that series should be dropped, too.

Thank you for your replies, the both of you.

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2020-November/620524.html

> Looking at that driver though, I see it still has a bug in that
> it creates a bogus soc_device on most (non-imx) machines.
> I think there was a fix a while ago, but it never got merged.
Just interested; Is there somewhere I can read more about that bug/fix?

Regards,
Robert
diff mbox series

Patch

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 9cebd360d58e..6e0d51b55615 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -48,6 +48,7 @@  obj-$(CONFIG_SOC_IMX6UL) += mach-imx6ul.o
 obj-$(CONFIG_SOC_IMX7D_CA7) += mach-imx7d.o
 obj-$(CONFIG_SOC_IMX7D_CM4) += mach-imx7d-cm4.o
 obj-$(CONFIG_SOC_IMX7ULP) += mach-imx7ulp.o pm-imx7ulp.o
+obj-$(CONFIG_SOC_IMX6) += ocotp.o
 
 ifeq ($(CONFIG_SUSPEND),y)
 AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index 2d76e2c6c99e..4a41fdc1defd 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -34,6 +34,7 @@  void imx_aips_allow_unprivileged_access(const char *compat);
 int mxc_device_init(void);
 void imx_set_soc_revision(unsigned int rev);
 void imx_init_revision_from_anatop(void);
+void imx_init_serial_from_ocotp(const char *ocotp_compat);
 void imx6_enable_rbc(bool enable);
 void imx_gpc_check_dt(void);
 void imx_gpc_set_arm_power_in_lpm(bool power_off);
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 703998ebb52e..2922c6128fbd 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -292,6 +292,7 @@  static void __init imx6q_init_irq(void)
 {
 	imx_gpc_check_dt();
 	imx_init_revision_from_anatop();
+	imx_init_serial_from_ocotp("fsl,imx6q-ocotp");
 	imx_init_l2cache();
 	imx_src_init();
 	irqchip_init();
diff --git a/arch/arm/mach-imx/mach-imx6sl.c b/arch/arm/mach-imx/mach-imx6sl.c
index f6e87363d605..d5765ba37517 100644
--- a/arch/arm/mach-imx/mach-imx6sl.c
+++ b/arch/arm/mach-imx/mach-imx6sl.c
@@ -57,6 +57,7 @@  static void __init imx6sl_init_irq(void)
 {
 	imx_gpc_check_dt();
 	imx_init_revision_from_anatop();
+	imx_init_serial_from_ocotp("fsl,imx6sl-ocotp");
 	imx_init_l2cache();
 	imx_src_init();
 	irqchip_init();
diff --git a/arch/arm/mach-imx/mach-imx6sx.c b/arch/arm/mach-imx/mach-imx6sx.c
index 781e2a94fdd7..5a772b4e06ee 100644
--- a/arch/arm/mach-imx/mach-imx6sx.c
+++ b/arch/arm/mach-imx/mach-imx6sx.c
@@ -74,6 +74,7 @@  static void __init imx6sx_init_irq(void)
 {
 	imx_gpc_check_dt();
 	imx_init_revision_from_anatop();
+	imx_init_serial_from_ocotp("fsl,imx6sx-ocotp");
 	imx_init_l2cache();
 	imx_src_init();
 	irqchip_init();
diff --git a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
index e018e716735f..1c3c33b8b55c 100644
--- a/arch/arm/mach-imx/mach-imx6ul.c
+++ b/arch/arm/mach-imx/mach-imx6ul.c
@@ -14,6 +14,7 @@ 
 
 #include "common.h"
 #include "cpuidle.h"
+#include "hardware.h"
 
 static void __init imx6ul_enet_clk_init(void)
 {
@@ -64,6 +65,8 @@  static void __init imx6ul_init_machine(void)
 static void __init imx6ul_init_irq(void)
 {
 	imx_init_revision_from_anatop();
+	imx_init_serial_from_ocotp(cpu_is_imx6ull() ? "fsl,imx6ull-ocotp" :
+		"fsl,imx6ul-ocotp");
 	imx_src_init();
 	irqchip_init();
 	imx6_pm_ccm_init("fsl,imx6ul-ccm");
diff --git a/arch/arm/mach-imx/ocotp.c b/arch/arm/mach-imx/ocotp.c
new file mode 100644
index 000000000000..acc0cbe08e2b
--- /dev/null
+++ b/arch/arm/mach-imx/ocotp.c
@@ -0,0 +1,40 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2018 PHYTEC Messtechnik GmbH,
+ * Author: Stefan Christ <s.christ@phytec.de>
+ */
+
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <asm/system_info.h>
+
+#include "common.h"
+
+#define OCOTP_CFG0		0x410
+#define OCOTP_CFG1		0x420
+
+void __init imx_init_serial_from_ocotp(const char *ocotp_compat)
+{
+	struct device_node *np;
+	void __iomem *base;
+
+	np = of_find_compatible_node(NULL, NULL, ocotp_compat);
+	if (!np) {
+		pr_warn("failed to find ocotp node in dtb!\n");
+		return;
+	}
+
+	base = of_iomap(np, 0);
+	if (!base) {
+		pr_warn("failed to map ocotp\n");
+		goto put_node;
+	}
+
+	system_serial_low  = readl_relaxed(base + OCOTP_CFG0);
+	system_serial_high = readl_relaxed(base + OCOTP_CFG1);
+
+	iounmap(base);
+put_node:
+	of_node_put(np);
+}