2020-01-16 10:21:37

by Peng Fan

[permalink] [raw]
Subject: [RFC 0/4] ARM: imx: move cpu code to drivers/soc/imx

From: Peng Fan <[email protected]>

Follow i.MX8, move the soc device register code to drivers/soc/imx
to simplify arch/arm/mach-imx/cpu.c

I planned to use similar logic as soc-imx8.c to restructure soc-imx.c
and merged the two files into one. But not sure, so still keep
the logic in cpu.c.

There is one change is the platform devices are not under
/sys/devices/soc0 after patch 1/4. Actually ARM64 platform
devices are not under /sys/devices/soc0, such as i.MX8/8M.
So it should not hurt to let the platform devices under platform dir.

Peng Fan (4):
ARM: imx: use device_initcall for imx_soc_device_init
ARM: imx: cpu: drop dead code
ARM: imx: move cpu definitions into a header
soc: imx: move cpu code to drivers/soc/imx

arch/arm/mach-imx/common.h | 1 -
arch/arm/mach-imx/cpu.c | 159 ---------------------------------------
arch/arm/mach-imx/mach-imx6q.c | 8 +-
arch/arm/mach-imx/mach-imx6sl.c | 8 +-
arch/arm/mach-imx/mach-imx6sx.c | 8 +-
arch/arm/mach-imx/mach-imx6ul.c | 8 +-
arch/arm/mach-imx/mach-imx7d.c | 6 --
arch/arm/mach-imx/mach-imx7ulp.c | 2 +-
arch/arm/mach-imx/mxc.h | 22 +-----
drivers/soc/imx/Makefile | 3 +
drivers/soc/imx/soc-imx.c | 146 +++++++++++++++++++++++++++++++++++
include/soc/imx/cpu.h | 30 ++++++++
12 files changed, 185 insertions(+), 216 deletions(-)
create mode 100644 drivers/soc/imx/soc-imx.c
create mode 100644 include/soc/imx/cpu.h

--
2.16.4


2020-01-16 10:21:51

by Peng Fan

[permalink] [raw]
Subject: [RFC 1/4] ARM: imx: use device_initcall for imx_soc_device_init

From: Peng Fan <[email protected]>

This is preparation to move imx_soc_device_init to drivers/soc/imx/

There is no reason to must put dt devices under /sys/devices/soc0,
they could also be under /sys/devices/platform, so we could
pass NULL as parent when calling of_platform_default_populate.

Following soc-imx8.c soc-imx-scu.c using device_initcall, need
to change return type to int type for imx_soc_device_init.

Signed-off-by: Peng Fan <[email protected]>
---
arch/arm/mach-imx/common.h | 1 -
arch/arm/mach-imx/cpu.c | 9 +++++----
arch/arm/mach-imx/mach-imx6q.c | 8 +-------
arch/arm/mach-imx/mach-imx6sl.c | 8 +-------
arch/arm/mach-imx/mach-imx6sx.c | 8 +-------
arch/arm/mach-imx/mach-imx6ul.c | 8 +-------
arch/arm/mach-imx/mach-imx7d.c | 6 ------
arch/arm/mach-imx/mach-imx7ulp.c | 2 +-
8 files changed, 10 insertions(+), 40 deletions(-)

diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index 912aeceb4ff8..09e89aa7be50 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -49,7 +49,6 @@ 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);
-struct device *imx_soc_device_init(void);
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/cpu.c b/arch/arm/mach-imx/cpu.c
index 06f8d64b65af..2df649a84697 100644
--- a/arch/arm/mach-imx/cpu.c
+++ b/arch/arm/mach-imx/cpu.c
@@ -83,7 +83,7 @@ void __init imx_aips_allow_unprivileged_access(
}
}

-struct device * __init imx_soc_device_init(void)
+static int __init imx_soc_device_init(void)
{
struct soc_device_attribute *soc_dev_attr;
const char *ocotp_compat = NULL;
@@ -97,7 +97,7 @@ struct device * __init imx_soc_device_init(void)

soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
if (!soc_dev_attr)
- return NULL;
+ return PTR_ERR(soc_dev_attr);

soc_dev_attr->family = "Freescale i.MX";

@@ -219,7 +219,7 @@ struct device * __init imx_soc_device_init(void)
if (IS_ERR(soc_dev))
goto free_serial_number;

- return soc_device_to_device(soc_dev);
+ return 0;

free_serial_number:
kfree(soc_dev_attr->serial_number);
@@ -227,5 +227,6 @@ struct device * __init imx_soc_device_init(void)
kfree(soc_dev_attr->revision);
free_soc:
kfree(soc_dev_attr);
- return NULL;
+ return -ENOMEM;
}
+device_initcall(imx_soc_device_init);
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index edd26e0ffeec..735da3311320 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -258,21 +258,15 @@ static void __init imx6q_axi_init(void)

static void __init imx6q_init_machine(void)
{
- struct device *parent;
-
if (cpu_is_imx6q() && imx_get_soc_revision() == IMX_CHIP_REVISION_2_0)
imx_print_silicon_rev("i.MX6QP", IMX_CHIP_REVISION_1_0);
else
imx_print_silicon_rev(cpu_is_imx6dl() ? "i.MX6DL" : "i.MX6Q",
imx_get_soc_revision());

- parent = imx_soc_device_init();
- if (parent == NULL)
- pr_warn("failed to initialize soc device\n");
-
imx6q_enet_phy_init();

- of_platform_default_populate(NULL, NULL, parent);
+ of_platform_default_populate(NULL, NULL, NULL);

imx_anatop_init();
cpu_is_imx6q() ? imx6q_pm_init() : imx6dl_pm_init();
diff --git a/arch/arm/mach-imx/mach-imx6sl.c b/arch/arm/mach-imx/mach-imx6sl.c
index e00818abe54d..0f046a37dc73 100644
--- a/arch/arm/mach-imx/mach-imx6sl.c
+++ b/arch/arm/mach-imx/mach-imx6sl.c
@@ -46,13 +46,7 @@ static void __init imx6sl_init_late(void)

static void __init imx6sl_init_machine(void)
{
- struct device *parent;
-
- parent = imx_soc_device_init();
- if (parent == NULL)
- pr_warn("failed to initialize soc device\n");
-
- of_platform_default_populate(NULL, NULL, parent);
+ of_platform_default_populate(NULL, NULL, NULL);

if (cpu_is_imx6sl())
imx6sl_fec_init();
diff --git a/arch/arm/mach-imx/mach-imx6sx.c b/arch/arm/mach-imx/mach-imx6sx.c
index d5310bf307ff..781e2a94fdd7 100644
--- a/arch/arm/mach-imx/mach-imx6sx.c
+++ b/arch/arm/mach-imx/mach-imx6sx.c
@@ -63,13 +63,7 @@ static inline void imx6sx_enet_init(void)

static void __init imx6sx_init_machine(void)
{
- struct device *parent;
-
- parent = imx_soc_device_init();
- if (parent == NULL)
- pr_warn("failed to initialize soc device\n");
-
- of_platform_default_populate(NULL, NULL, parent);
+ of_platform_default_populate(NULL, NULL, NULL);

imx6sx_enet_init();
imx_anatop_init();
diff --git a/arch/arm/mach-imx/mach-imx6ul.c b/arch/arm/mach-imx/mach-imx6ul.c
index 311f5e4ff723..9db8e567c6b5 100644
--- a/arch/arm/mach-imx/mach-imx6ul.c
+++ b/arch/arm/mach-imx/mach-imx6ul.c
@@ -56,13 +56,7 @@ static inline void imx6ul_enet_init(void)

static void __init imx6ul_init_machine(void)
{
- struct device *parent;
-
- parent = imx_soc_device_init();
- if (parent == NULL)
- pr_warn("failed to initialize soc device\n");
-
- of_platform_default_populate(NULL, NULL, parent);
+ of_platform_default_populate(NULL, NULL, NULL);
imx6ul_enet_init();
imx_anatop_init();
imx6ul_pm_init();
diff --git a/arch/arm/mach-imx/mach-imx7d.c b/arch/arm/mach-imx/mach-imx7d.c
index ebb27592a9f7..879c35929a13 100644
--- a/arch/arm/mach-imx/mach-imx7d.c
+++ b/arch/arm/mach-imx/mach-imx7d.c
@@ -78,12 +78,6 @@ static inline void imx7d_enet_init(void)

static void __init imx7d_init_machine(void)
{
- struct device *parent;
-
- parent = imx_soc_device_init();
- if (parent == NULL)
- pr_warn("failed to initialize soc device\n");
-
imx_anatop_init();
imx7d_enet_init();
}
diff --git a/arch/arm/mach-imx/mach-imx7ulp.c b/arch/arm/mach-imx/mach-imx7ulp.c
index 11ac71aaf965..128cf4c92aab 100644
--- a/arch/arm/mach-imx/mach-imx7ulp.c
+++ b/arch/arm/mach-imx/mach-imx7ulp.c
@@ -57,7 +57,7 @@ static void __init imx7ulp_init_machine(void)

mxc_set_cpu_type(MXC_CPU_IMX7ULP);
imx7ulp_set_revision();
- of_platform_default_populate(NULL, NULL, imx_soc_device_init());
+ of_platform_default_populate(NULL, NULL, NULL);
}

static const char *const imx7ulp_dt_compat[] __initconst = {
--
2.16.4

2020-01-16 10:21:56

by Peng Fan

[permalink] [raw]
Subject: [RFC 4/4] soc: imx: move cpu code to drivers/soc/imx

From: Peng Fan <[email protected]>

Move the soc device register code to drivers/soc/imx to align with
i.MX8.

Signed-off-by: Peng Fan <[email protected]>
---
arch/arm/mach-imx/cpu.c | 136 ------------------------------------------
drivers/soc/imx/Makefile | 3 +
drivers/soc/imx/soc-imx.c | 146 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 149 insertions(+), 136 deletions(-)
create mode 100644 drivers/soc/imx/soc-imx.c

diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c
index 0302cb66134b..65c7224f5250 100644
--- a/arch/arm/mach-imx/cpu.c
+++ b/arch/arm/mach-imx/cpu.c
@@ -1,25 +1,13 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/err.h>
-#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/io.h>
#include <linux/of.h>
#include <linux/of_address.h>
-#include <linux/regmap.h>
-#include <linux/slab.h>
-#include <linux/sys_soc.h>

#include "hardware.h"
#include "common.h"

-#define OCOTP_UID_H 0x420
-#define OCOTP_UID_L 0x410
-
-#define OCOTP_ULP_UID_1 0x4b0
-#define OCOTP_ULP_UID_2 0x4c0
-#define OCOTP_ULP_UID_3 0x4d0
-#define OCOTP_ULP_UID_4 0x4e0
-
unsigned int __mxc_cpu_type;
static unsigned int imx_soc_revision;

@@ -82,127 +70,3 @@ void __init imx_aips_allow_unprivileged_access(
imx_set_aips(aips_base_addr);
}
}
-
-static int __init imx_soc_device_init(void)
-{
- struct soc_device_attribute *soc_dev_attr;
- const char *ocotp_compat = NULL;
- struct soc_device *soc_dev;
- struct device_node *root;
- struct regmap *ocotp = NULL;
- const char *soc_id;
- u64 soc_uid = 0;
- u32 val;
- int ret;
-
- soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
- if (!soc_dev_attr)
- return PTR_ERR(soc_dev_attr);
-
- soc_dev_attr->family = "Freescale i.MX";
-
- root = of_find_node_by_path("/");
- ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
- of_node_put(root);
- if (ret)
- goto free_soc;
-
- switch (__mxc_cpu_type) {
- case MXC_CPU_IMX6SL:
- ocotp_compat = "fsl,imx6sl-ocotp";
- soc_id = "i.MX6SL";
- break;
- case MXC_CPU_IMX6DL:
- ocotp_compat = "fsl,imx6q-ocotp";
- soc_id = "i.MX6DL";
- break;
- case MXC_CPU_IMX6SX:
- ocotp_compat = "fsl,imx6sx-ocotp";
- soc_id = "i.MX6SX";
- break;
- case MXC_CPU_IMX6Q:
- ocotp_compat = "fsl,imx6q-ocotp";
- soc_id = "i.MX6Q";
- break;
- case MXC_CPU_IMX6UL:
- ocotp_compat = "fsl,imx6ul-ocotp";
- soc_id = "i.MX6UL";
- break;
- case MXC_CPU_IMX6ULL:
- ocotp_compat = "fsl,imx6ull-ocotp";
- soc_id = "i.MX6ULL";
- break;
- case MXC_CPU_IMX6ULZ:
- ocotp_compat = "fsl,imx6ull-ocotp";
- soc_id = "i.MX6ULZ";
- break;
- case MXC_CPU_IMX6SLL:
- ocotp_compat = "fsl,imx6sll-ocotp";
- soc_id = "i.MX6SLL";
- break;
- case MXC_CPU_IMX7D:
- ocotp_compat = "fsl,imx7d-ocotp";
- soc_id = "i.MX7D";
- break;
- case MXC_CPU_IMX7ULP:
- ocotp_compat = "fsl,imx7ulp-ocotp";
- soc_id = "i.MX7ULP";
- break;
- default:
- soc_id = "Unknown";
- }
- soc_dev_attr->soc_id = soc_id;
-
- if (ocotp_compat) {
- ocotp = syscon_regmap_lookup_by_compatible(ocotp_compat);
- if (IS_ERR(ocotp))
- pr_err("%s: failed to find %s regmap!\n", __func__, ocotp_compat);
- }
-
- if (!IS_ERR_OR_NULL(ocotp)) {
- if (__mxc_cpu_type == MXC_CPU_IMX7ULP) {
- regmap_read(ocotp, OCOTP_ULP_UID_4, &val);
- soc_uid = val & 0xffff;
- regmap_read(ocotp, OCOTP_ULP_UID_3, &val);
- soc_uid <<= 16;
- soc_uid |= val & 0xffff;
- regmap_read(ocotp, OCOTP_ULP_UID_2, &val);
- soc_uid <<= 16;
- soc_uid |= val & 0xffff;
- regmap_read(ocotp, OCOTP_ULP_UID_1, &val);
- soc_uid <<= 16;
- soc_uid |= val & 0xffff;
- } else {
- regmap_read(ocotp, OCOTP_UID_H, &val);
- soc_uid = val;
- regmap_read(ocotp, OCOTP_UID_L, &val);
- soc_uid <<= 32;
- soc_uid |= val;
- }
- }
-
- soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d.%d",
- (imx_soc_revision >> 4) & 0xf,
- imx_soc_revision & 0xf);
- if (!soc_dev_attr->revision)
- goto free_soc;
-
- soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX", soc_uid);
- if (!soc_dev_attr->serial_number)
- goto free_rev;
-
- soc_dev = soc_device_register(soc_dev_attr);
- if (IS_ERR(soc_dev))
- goto free_serial_number;
-
- return 0;
-
-free_serial_number:
- kfree(soc_dev_attr->serial_number);
-free_rev:
- kfree(soc_dev_attr->revision);
-free_soc:
- kfree(soc_dev_attr);
- return -ENOMEM;
-}
-device_initcall(imx_soc_device_init);
diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
index 103e2c93c342..45dd49df3044 100644
--- a/drivers/soc/imx/Makefile
+++ b/drivers/soc/imx/Makefile
@@ -1,5 +1,8 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
+obj-$(CONFIG_SOC_IMX6)+= soc-imx.o
+obj-$(CONFIG_SOC_IMX7D)+= soc-imx.o
+obj-$(CONFIG_SOC_IMX7ULP)+= soc-imx.o
obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
obj-$(CONFIG_IMX_SCU_SOC) += soc-imx-scu.o
diff --git a/drivers/soc/imx/soc-imx.c b/drivers/soc/imx/soc-imx.c
new file mode 100644
index 000000000000..e01c7ccf1f4b
--- /dev/null
+++ b/drivers/soc/imx/soc-imx.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2020 NXP.
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/sys_soc.h>
+
+#include <soc/imx/cpu.h>
+#include <soc/imx/revision.h>
+
+#define OCOTP_UID_H 0x420
+#define OCOTP_UID_L 0x410
+
+#define OCOTP_ULP_UID_1 0x4b0
+#define OCOTP_ULP_UID_2 0x4c0
+#define OCOTP_ULP_UID_3 0x4d0
+#define OCOTP_ULP_UID_4 0x4e0
+
+static int __init imx_soc_device_init(void)
+{
+ struct soc_device_attribute *soc_dev_attr;
+ const char *ocotp_compat = NULL;
+ struct soc_device *soc_dev;
+ struct device_node *root;
+ struct regmap *ocotp = NULL;
+ const char *soc_id;
+ u64 soc_uid = 0;
+ u32 val;
+ int ret;
+
+ soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
+ if (!soc_dev_attr)
+ return PTR_ERR(soc_dev_attr);
+
+ soc_dev_attr->family = "Freescale i.MX";
+
+ root = of_find_node_by_path("/");
+ ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
+ of_node_put(root);
+ if (ret)
+ goto free_soc;
+
+ switch (__mxc_cpu_type) {
+ case MXC_CPU_IMX6SL:
+ ocotp_compat = "fsl,imx6sl-ocotp";
+ soc_id = "i.MX6SL";
+ break;
+ case MXC_CPU_IMX6DL:
+ ocotp_compat = "fsl,imx6q-ocotp";
+ soc_id = "i.MX6DL";
+ break;
+ case MXC_CPU_IMX6SX:
+ ocotp_compat = "fsl,imx6sx-ocotp";
+ soc_id = "i.MX6SX";
+ break;
+ case MXC_CPU_IMX6Q:
+ ocotp_compat = "fsl,imx6q-ocotp";
+ soc_id = "i.MX6Q";
+ break;
+ case MXC_CPU_IMX6UL:
+ ocotp_compat = "fsl,imx6ul-ocotp";
+ soc_id = "i.MX6UL";
+ break;
+ case MXC_CPU_IMX6ULL:
+ ocotp_compat = "fsl,imx6ull-ocotp";
+ soc_id = "i.MX6ULL";
+ break;
+ case MXC_CPU_IMX6ULZ:
+ ocotp_compat = "fsl,imx6ull-ocotp";
+ soc_id = "i.MX6ULZ";
+ break;
+ case MXC_CPU_IMX6SLL:
+ ocotp_compat = "fsl,imx6sll-ocotp";
+ soc_id = "i.MX6SLL";
+ break;
+ case MXC_CPU_IMX7D:
+ ocotp_compat = "fsl,imx7d-ocotp";
+ soc_id = "i.MX7D";
+ break;
+ case MXC_CPU_IMX7ULP:
+ ocotp_compat = "fsl,imx7ulp-ocotp";
+ soc_id = "i.MX7ULP";
+ break;
+ default:
+ soc_id = "Unknown";
+ }
+ soc_dev_attr->soc_id = soc_id;
+
+ if (ocotp_compat) {
+ ocotp = syscon_regmap_lookup_by_compatible(ocotp_compat);
+ if (IS_ERR(ocotp))
+ pr_err("%s: failed to find %s regmap!\n", __func__, ocotp_compat);
+ }
+
+ if (!IS_ERR_OR_NULL(ocotp)) {
+ if (__mxc_cpu_type == MXC_CPU_IMX7ULP) {
+ regmap_read(ocotp, OCOTP_ULP_UID_4, &val);
+ soc_uid = val & 0xffff;
+ regmap_read(ocotp, OCOTP_ULP_UID_3, &val);
+ soc_uid <<= 16;
+ soc_uid |= val & 0xffff;
+ regmap_read(ocotp, OCOTP_ULP_UID_2, &val);
+ soc_uid <<= 16;
+ soc_uid |= val & 0xffff;
+ regmap_read(ocotp, OCOTP_ULP_UID_1, &val);
+ soc_uid <<= 16;
+ soc_uid |= val & 0xffff;
+ } else {
+ regmap_read(ocotp, OCOTP_UID_H, &val);
+ soc_uid = val;
+ regmap_read(ocotp, OCOTP_UID_L, &val);
+ soc_uid <<= 32;
+ soc_uid |= val;
+ }
+ }
+
+ soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d.%d",
+ (imx_get_soc_revision() >> 4) & 0xf,
+ imx_get_soc_revision() & 0xf);
+ if (!soc_dev_attr->revision)
+ goto free_soc;
+
+ soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX", soc_uid);
+ if (!soc_dev_attr->serial_number)
+ goto free_rev;
+
+ soc_dev = soc_device_register(soc_dev_attr);
+ if (IS_ERR(soc_dev))
+ goto free_serial_number;
+
+ return 0;
+
+free_serial_number:
+ kfree(soc_dev_attr->serial_number);
+free_rev:
+ kfree(soc_dev_attr->revision);
+free_soc:
+ kfree(soc_dev_attr);
+ return -ENOMEM;
+}
+device_initcall(imx_soc_device_init);
--
2.16.4

2020-01-17 03:57:54

by Jacky Bai

[permalink] [raw]
Subject: RE: [RFC 1/4] ARM: imx: use device_initcall for imx_soc_device_init

> -----Original Message-----
> From: Peng Fan <[email protected]>
> Sent: Thursday, January 16, 2020 5:37 PM
> To: [email protected]; [email protected]
> Cc: [email protected]; [email protected]; dl-linux-imx
> <[email protected]>; [email protected]; [email protected]; Anson Huang
> <[email protected]>; Leonard Crestez <[email protected]>;
> [email protected]; Abel Vesa <[email protected]>; [email protected];
> [email protected]; [email protected]; Peng Fan
> <[email protected]>
> Subject: [RFC 1/4] ARM: imx: use device_initcall for imx_soc_device_init
>
> From: Peng Fan <[email protected]>
>
> This is preparation to move imx_soc_device_init to drivers/soc/imx/
>
> There is no reason to must put dt devices under /sys/devices/soc0, they could
> also be under /sys/devices/platform, so we could pass NULL as parent when
> calling of_platform_default_populate.
>

This change will impact various internal test case & userspace lib, I think.
Need to ask test team & other developer to double check the impact.

BR
Jacky Bai
> Following soc-imx8.c soc-imx-scu.c using device_initcall, need to change
> return type to int type for imx_soc_device_init.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> arch/arm/mach-imx/common.h | 1 -
> arch/arm/mach-imx/cpu.c | 9 +++++----
> arch/arm/mach-imx/mach-imx6q.c | 8 +-------
> arch/arm/mach-imx/mach-imx6sl.c | 8 +-------
> arch/arm/mach-imx/mach-imx6sx.c | 8 +-------
> arch/arm/mach-imx/mach-imx6ul.c | 8 +-------
> arch/arm/mach-imx/mach-imx7d.c | 6 ------
> arch/arm/mach-imx/mach-imx7ulp.c | 2 +-
> 8 files changed, 10 insertions(+), 40 deletions(-)
>
> diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
> index 912aeceb4ff8..09e89aa7be50 100644
> --- a/arch/arm/mach-imx/common.h
> +++ b/arch/arm/mach-imx/common.h
> @@ -49,7 +49,6 @@ 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);
> -struct device *imx_soc_device_init(void); 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/cpu.c b/arch/arm/mach-imx/cpu.c index
> 06f8d64b65af..2df649a84697 100644
> --- a/arch/arm/mach-imx/cpu.c
> +++ b/arch/arm/mach-imx/cpu.c
> @@ -83,7 +83,7 @@ void __init imx_aips_allow_unprivileged_access(
> }
> }
>
> -struct device * __init imx_soc_device_init(void)
> +static int __init imx_soc_device_init(void)
> {
> struct soc_device_attribute *soc_dev_attr;
> const char *ocotp_compat = NULL;
> @@ -97,7 +97,7 @@ struct device * __init imx_soc_device_init(void)
>
> soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> if (!soc_dev_attr)
> - return NULL;
> + return PTR_ERR(soc_dev_attr);
>
> soc_dev_attr->family = "Freescale i.MX";
>
> @@ -219,7 +219,7 @@ struct device * __init imx_soc_device_init(void)
> if (IS_ERR(soc_dev))
> goto free_serial_number;
>
> - return soc_device_to_device(soc_dev);
> + return 0;
>
> free_serial_number:
> kfree(soc_dev_attr->serial_number);
> @@ -227,5 +227,6 @@ struct device * __init imx_soc_device_init(void)
> kfree(soc_dev_attr->revision);
> free_soc:
> kfree(soc_dev_attr);
> - return NULL;
> + return -ENOMEM;
> }
> +device_initcall(imx_soc_device_init);
> diff --git a/arch/arm/mach-imx/mach-imx6q.c
> b/arch/arm/mach-imx/mach-imx6q.c index edd26e0ffeec..735da3311320
> 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -258,21 +258,15 @@ static void __init imx6q_axi_init(void)
>
> static void __init imx6q_init_machine(void) {
> - struct device *parent;
> -
> if (cpu_is_imx6q() && imx_get_soc_revision() ==
> IMX_CHIP_REVISION_2_0)
> imx_print_silicon_rev("i.MX6QP", IMX_CHIP_REVISION_1_0);
> else
> imx_print_silicon_rev(cpu_is_imx6dl() ? "i.MX6DL" : "i.MX6Q",
> imx_get_soc_revision());
>
> - parent = imx_soc_device_init();
> - if (parent == NULL)
> - pr_warn("failed to initialize soc device\n");
> -
> imx6q_enet_phy_init();
>
> - of_platform_default_populate(NULL, NULL, parent);
> + of_platform_default_populate(NULL, NULL, NULL);
>
> imx_anatop_init();
> cpu_is_imx6q() ? imx6q_pm_init() : imx6dl_pm_init(); diff --git
> a/arch/arm/mach-imx/mach-imx6sl.c b/arch/arm/mach-imx/mach-imx6sl.c
> index e00818abe54d..0f046a37dc73 100644
> --- a/arch/arm/mach-imx/mach-imx6sl.c
> +++ b/arch/arm/mach-imx/mach-imx6sl.c
> @@ -46,13 +46,7 @@ static void __init imx6sl_init_late(void)
>
> static void __init imx6sl_init_machine(void) {
> - struct device *parent;
> -
> - parent = imx_soc_device_init();
> - if (parent == NULL)
> - pr_warn("failed to initialize soc device\n");
> -
> - of_platform_default_populate(NULL, NULL, parent);
> + of_platform_default_populate(NULL, NULL, NULL);
>
> if (cpu_is_imx6sl())
> imx6sl_fec_init();
> diff --git a/arch/arm/mach-imx/mach-imx6sx.c
> b/arch/arm/mach-imx/mach-imx6sx.c index d5310bf307ff..781e2a94fdd7
> 100644
> --- a/arch/arm/mach-imx/mach-imx6sx.c
> +++ b/arch/arm/mach-imx/mach-imx6sx.c
> @@ -63,13 +63,7 @@ static inline void imx6sx_enet_init(void)
>
> static void __init imx6sx_init_machine(void) {
> - struct device *parent;
> -
> - parent = imx_soc_device_init();
> - if (parent == NULL)
> - pr_warn("failed to initialize soc device\n");
> -
> - of_platform_default_populate(NULL, NULL, parent);
> + of_platform_default_populate(NULL, NULL, NULL);
>
> imx6sx_enet_init();
> imx_anatop_init();
> diff --git a/arch/arm/mach-imx/mach-imx6ul.c
> b/arch/arm/mach-imx/mach-imx6ul.c index 311f5e4ff723..9db8e567c6b5
> 100644
> --- a/arch/arm/mach-imx/mach-imx6ul.c
> +++ b/arch/arm/mach-imx/mach-imx6ul.c
> @@ -56,13 +56,7 @@ static inline void imx6ul_enet_init(void)
>
> static void __init imx6ul_init_machine(void) {
> - struct device *parent;
> -
> - parent = imx_soc_device_init();
> - if (parent == NULL)
> - pr_warn("failed to initialize soc device\n");
> -
> - of_platform_default_populate(NULL, NULL, parent);
> + of_platform_default_populate(NULL, NULL, NULL);
> imx6ul_enet_init();
> imx_anatop_init();
> imx6ul_pm_init();
> diff --git a/arch/arm/mach-imx/mach-imx7d.c
> b/arch/arm/mach-imx/mach-imx7d.c index ebb27592a9f7..879c35929a13
> 100644
> --- a/arch/arm/mach-imx/mach-imx7d.c
> +++ b/arch/arm/mach-imx/mach-imx7d.c
> @@ -78,12 +78,6 @@ static inline void imx7d_enet_init(void)
>
> static void __init imx7d_init_machine(void) {
> - struct device *parent;
> -
> - parent = imx_soc_device_init();
> - if (parent == NULL)
> - pr_warn("failed to initialize soc device\n");
> -
> imx_anatop_init();
> imx7d_enet_init();
> }
> diff --git a/arch/arm/mach-imx/mach-imx7ulp.c
> b/arch/arm/mach-imx/mach-imx7ulp.c
> index 11ac71aaf965..128cf4c92aab 100644
> --- a/arch/arm/mach-imx/mach-imx7ulp.c
> +++ b/arch/arm/mach-imx/mach-imx7ulp.c
> @@ -57,7 +57,7 @@ static void __init imx7ulp_init_machine(void)
>
> mxc_set_cpu_type(MXC_CPU_IMX7ULP);
> imx7ulp_set_revision();
> - of_platform_default_populate(NULL, NULL, imx_soc_device_init());
> + of_platform_default_populate(NULL, NULL, NULL);
> }
>
> static const char *const imx7ulp_dt_compat[] __initconst = {
> --
> 2.16.4

2020-01-17 08:17:23

by Peng Fan

[permalink] [raw]
Subject: RE: [RFC 1/4] ARM: imx: use device_initcall for imx_soc_device_init

> > Subject: [RFC 1/4] ARM: imx: use device_initcall for
> > imx_soc_device_init
> >
> > From: Peng Fan <[email protected]>
> >
> > This is preparation to move imx_soc_device_init to drivers/soc/imx/
> >
> > There is no reason to must put dt devices under /sys/devices/soc0,
> > they could also be under /sys/devices/platform, so we could pass NULL
> > as parent when calling of_platform_default_populate.
> >
>
> This change will impact various internal test case & userspace lib, I think.
> Need to ask test team & other developer to double check the impact.

/sys/devices/soc0 is still there, the patchset only moves
the platform devices which under /sys/devices/soc0 to /sys/devices/platform

In this way, we aligned with ARM64. And simplify arch code by moving
the code to drivers/soc/imx. In future, considering more cleanup,
we could merge the code to soc-imx8.c, since they share similar
silicon rev ocotp logic.

Thanks,
Peng.

>
> BR
> Jacky Bai
> > Following soc-imx8.c soc-imx-scu.c using device_initcall, need to
> > change return type to int type for imx_soc_device_init.
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> > arch/arm/mach-imx/common.h | 1 -
> > arch/arm/mach-imx/cpu.c | 9 +++++----
> > arch/arm/mach-imx/mach-imx6q.c | 8 +-------
> > arch/arm/mach-imx/mach-imx6sl.c | 8 +-------
> > arch/arm/mach-imx/mach-imx6sx.c | 8 +-------
> > arch/arm/mach-imx/mach-imx6ul.c | 8 +-------
> > arch/arm/mach-imx/mach-imx7d.c | 6 ------
> > arch/arm/mach-imx/mach-imx7ulp.c | 2 +-
> > 8 files changed, 10 insertions(+), 40 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/common.h
> b/arch/arm/mach-imx/common.h
> > index 912aeceb4ff8..09e89aa7be50 100644
> > --- a/arch/arm/mach-imx/common.h
> > +++ b/arch/arm/mach-imx/common.h
> > @@ -49,7 +49,6 @@ 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);
> > -struct device *imx_soc_device_init(void); 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/cpu.c b/arch/arm/mach-imx/cpu.c index
> > 06f8d64b65af..2df649a84697 100644
> > --- a/arch/arm/mach-imx/cpu.c
> > +++ b/arch/arm/mach-imx/cpu.c
> > @@ -83,7 +83,7 @@ void __init imx_aips_allow_unprivileged_access(
> > }
> > }
> >
> > -struct device * __init imx_soc_device_init(void)
> > +static int __init imx_soc_device_init(void)
> > {
> > struct soc_device_attribute *soc_dev_attr;
> > const char *ocotp_compat = NULL;
> > @@ -97,7 +97,7 @@ struct device * __init imx_soc_device_init(void)
> >
> > soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> > if (!soc_dev_attr)
> > - return NULL;
> > + return PTR_ERR(soc_dev_attr);
> >
> > soc_dev_attr->family = "Freescale i.MX";
> >
> > @@ -219,7 +219,7 @@ struct device * __init imx_soc_device_init(void)
> > if (IS_ERR(soc_dev))
> > goto free_serial_number;
> >
> > - return soc_device_to_device(soc_dev);
> > + return 0;
> >
> > free_serial_number:
> > kfree(soc_dev_attr->serial_number);
> > @@ -227,5 +227,6 @@ struct device * __init imx_soc_device_init(void)
> > kfree(soc_dev_attr->revision);
> > free_soc:
> > kfree(soc_dev_attr);
> > - return NULL;
> > + return -ENOMEM;
> > }
> > +device_initcall(imx_soc_device_init);
> > diff --git a/arch/arm/mach-imx/mach-imx6q.c
> > b/arch/arm/mach-imx/mach-imx6q.c index edd26e0ffeec..735da3311320
> > 100644
> > --- a/arch/arm/mach-imx/mach-imx6q.c
> > +++ b/arch/arm/mach-imx/mach-imx6q.c
> > @@ -258,21 +258,15 @@ static void __init imx6q_axi_init(void)
> >
> > static void __init imx6q_init_machine(void) {
> > - struct device *parent;
> > -
> > if (cpu_is_imx6q() && imx_get_soc_revision() ==
> > IMX_CHIP_REVISION_2_0)
> > imx_print_silicon_rev("i.MX6QP", IMX_CHIP_REVISION_1_0);
> > else
> > imx_print_silicon_rev(cpu_is_imx6dl() ? "i.MX6DL" : "i.MX6Q",
> > imx_get_soc_revision());
> >
> > - parent = imx_soc_device_init();
> > - if (parent == NULL)
> > - pr_warn("failed to initialize soc device\n");
> > -
> > imx6q_enet_phy_init();
> >
> > - of_platform_default_populate(NULL, NULL, parent);
> > + of_platform_default_populate(NULL, NULL, NULL);
> >
> > imx_anatop_init();
> > cpu_is_imx6q() ? imx6q_pm_init() : imx6dl_pm_init(); diff --git
> > a/arch/arm/mach-imx/mach-imx6sl.c
> b/arch/arm/mach-imx/mach-imx6sl.c
> > index e00818abe54d..0f046a37dc73 100644
> > --- a/arch/arm/mach-imx/mach-imx6sl.c
> > +++ b/arch/arm/mach-imx/mach-imx6sl.c
> > @@ -46,13 +46,7 @@ static void __init imx6sl_init_late(void)
> >
> > static void __init imx6sl_init_machine(void) {
> > - struct device *parent;
> > -
> > - parent = imx_soc_device_init();
> > - if (parent == NULL)
> > - pr_warn("failed to initialize soc device\n");
> > -
> > - of_platform_default_populate(NULL, NULL, parent);
> > + of_platform_default_populate(NULL, NULL, NULL);
> >
> > if (cpu_is_imx6sl())
> > imx6sl_fec_init();
> > diff --git a/arch/arm/mach-imx/mach-imx6sx.c
> > b/arch/arm/mach-imx/mach-imx6sx.c index d5310bf307ff..781e2a94fdd7
> > 100644
> > --- a/arch/arm/mach-imx/mach-imx6sx.c
> > +++ b/arch/arm/mach-imx/mach-imx6sx.c
> > @@ -63,13 +63,7 @@ static inline void imx6sx_enet_init(void)
> >
> > static void __init imx6sx_init_machine(void) {
> > - struct device *parent;
> > -
> > - parent = imx_soc_device_init();
> > - if (parent == NULL)
> > - pr_warn("failed to initialize soc device\n");
> > -
> > - of_platform_default_populate(NULL, NULL, parent);
> > + of_platform_default_populate(NULL, NULL, NULL);
> >
> > imx6sx_enet_init();
> > imx_anatop_init();
> > diff --git a/arch/arm/mach-imx/mach-imx6ul.c
> > b/arch/arm/mach-imx/mach-imx6ul.c index 311f5e4ff723..9db8e567c6b5
> > 100644
> > --- a/arch/arm/mach-imx/mach-imx6ul.c
> > +++ b/arch/arm/mach-imx/mach-imx6ul.c
> > @@ -56,13 +56,7 @@ static inline void imx6ul_enet_init(void)
> >
> > static void __init imx6ul_init_machine(void) {
> > - struct device *parent;
> > -
> > - parent = imx_soc_device_init();
> > - if (parent == NULL)
> > - pr_warn("failed to initialize soc device\n");
> > -
> > - of_platform_default_populate(NULL, NULL, parent);
> > + of_platform_default_populate(NULL, NULL, NULL);
> > imx6ul_enet_init();
> > imx_anatop_init();
> > imx6ul_pm_init();
> > diff --git a/arch/arm/mach-imx/mach-imx7d.c
> > b/arch/arm/mach-imx/mach-imx7d.c index ebb27592a9f7..879c35929a13
> > 100644
> > --- a/arch/arm/mach-imx/mach-imx7d.c
> > +++ b/arch/arm/mach-imx/mach-imx7d.c
> > @@ -78,12 +78,6 @@ static inline void imx7d_enet_init(void)
> >
> > static void __init imx7d_init_machine(void) {
> > - struct device *parent;
> > -
> > - parent = imx_soc_device_init();
> > - if (parent == NULL)
> > - pr_warn("failed to initialize soc device\n");
> > -
> > imx_anatop_init();
> > imx7d_enet_init();
> > }
> > diff --git a/arch/arm/mach-imx/mach-imx7ulp.c
> > b/arch/arm/mach-imx/mach-imx7ulp.c
> > index 11ac71aaf965..128cf4c92aab 100644
> > --- a/arch/arm/mach-imx/mach-imx7ulp.c
> > +++ b/arch/arm/mach-imx/mach-imx7ulp.c
> > @@ -57,7 +57,7 @@ static void __init imx7ulp_init_machine(void)
> >
> > mxc_set_cpu_type(MXC_CPU_IMX7ULP);
> > imx7ulp_set_revision();
> > - of_platform_default_populate(NULL, NULL, imx_soc_device_init());
> > + of_platform_default_populate(NULL, NULL, NULL);
> > }
> >
> > static const char *const imx7ulp_dt_compat[] __initconst = {
> > --
> > 2.16.4

2020-02-13 05:45:45

by Shawn Guo

[permalink] [raw]
Subject: Re: [RFC 1/4] ARM: imx: use device_initcall for imx_soc_device_init

On Fri, Jan 17, 2020 at 08:15:54AM +0000, Peng Fan wrote:
> > > Subject: [RFC 1/4] ARM: imx: use device_initcall for
> > > imx_soc_device_init
> > >
> > > From: Peng Fan <[email protected]>
> > >
> > > This is preparation to move imx_soc_device_init to drivers/soc/imx/
> > >
> > > There is no reason to must put dt devices under /sys/devices/soc0,
> > > they could also be under /sys/devices/platform, so we could pass NULL
> > > as parent when calling of_platform_default_populate.
> > >
> >
> > This change will impact various internal test case & userspace lib, I think.
> > Need to ask test team & other developer to double check the impact.
>
> /sys/devices/soc0 is still there, the patchset only moves
> the platform devices which under /sys/devices/soc0 to /sys/devices/platform

Jacky's concern still stands, as there are many user spaces which will be
broken and need update.

> In this way, we aligned with ARM64. And simplify arch code by moving
> the code to drivers/soc/imx. In future, considering more cleanup,
> we could merge the code to soc-imx8.c, since they share similar
> silicon rev ocotp logic.

Though this is a good thing from maintenance point of view, we do not
want to break user spaces.

Shawn

2020-02-13 05:48:08

by Peng Fan

[permalink] [raw]
Subject: RE: [RFC 1/4] ARM: imx: use device_initcall for imx_soc_device_init

> Subject: Re: [RFC 1/4] ARM: imx: use device_initcall for imx_soc_device_init
>
> On Fri, Jan 17, 2020 at 08:15:54AM +0000, Peng Fan wrote:
> > > > Subject: [RFC 1/4] ARM: imx: use device_initcall for
> > > > imx_soc_device_init
> > > >
> > > > From: Peng Fan <[email protected]>
> > > >
> > > > This is preparation to move imx_soc_device_init to
> > > > drivers/soc/imx/
> > > >
> > > > There is no reason to must put dt devices under /sys/devices/soc0,
> > > > they could also be under /sys/devices/platform, so we could pass
> > > > NULL as parent when calling of_platform_default_populate.
> > > >
> > >
> > > This change will impact various internal test case & userspace lib, I think.
> > > Need to ask test team & other developer to double check the impact.
> >
> > /sys/devices/soc0 is still there, the patchset only moves the platform
> > devices which under /sys/devices/soc0 to /sys/devices/platform
>
> Jacky's concern still stands, as there are many user spaces which will be
> broken and need update.

The soc device itself still under /sys/devices/soc0, the soc_id/revision still there.
It is just the platform devices moved to /sys/devices/platform.

When I confirm with Jacky before, his concern is soc_id/revision will be
moved. But this is not true, they are still there as before.

>
> > In this way, we aligned with ARM64. And simplify arch code by moving
> > the code to drivers/soc/imx. In future, considering more cleanup, we
> > could merge the code to soc-imx8.c, since they share similar silicon
> > rev ocotp logic.
>
> Though this is a good thing from maintenance point of view, we do not want
> to break user spaces.

Actually not break user spaces, since this is RFC, I not expect this be merged.
If you agree, I could post normal V1 patchset.

Thanks,
Peng.

>
> Shawn

2020-02-13 05:57:16

by Shawn Guo

[permalink] [raw]
Subject: Re: [RFC 1/4] ARM: imx: use device_initcall for imx_soc_device_init

On Thu, Feb 13, 2020 at 05:47:41AM +0000, Peng Fan wrote:
> > Subject: Re: [RFC 1/4] ARM: imx: use device_initcall for imx_soc_device_init
> >
> > On Fri, Jan 17, 2020 at 08:15:54AM +0000, Peng Fan wrote:
> > > > > Subject: [RFC 1/4] ARM: imx: use device_initcall for
> > > > > imx_soc_device_init
> > > > >
> > > > > From: Peng Fan <[email protected]>
> > > > >
> > > > > This is preparation to move imx_soc_device_init to
> > > > > drivers/soc/imx/
> > > > >
> > > > > There is no reason to must put dt devices under /sys/devices/soc0,
> > > > > they could also be under /sys/devices/platform, so we could pass
> > > > > NULL as parent when calling of_platform_default_populate.
> > > > >
> > > >
> > > > This change will impact various internal test case & userspace lib, I think.
> > > > Need to ask test team & other developer to double check the impact.
> > >
> > > /sys/devices/soc0 is still there, the patchset only moves the platform
> > > devices which under /sys/devices/soc0 to /sys/devices/platform
> >
> > Jacky's concern still stands, as there are many user spaces which will be
> > broken and need update.
>
> The soc device itself still under /sys/devices/soc0, the soc_id/revision still there.
> It is just the platform devices moved to /sys/devices/platform.
>
> When I confirm with Jacky before, his concern is soc_id/revision will be
> moved. But this is not true, they are still there as before.
>
> >
> > > In this way, we aligned with ARM64. And simplify arch code by moving
> > > the code to drivers/soc/imx. In future, considering more cleanup, we
> > > could merge the code to soc-imx8.c, since they share similar silicon
> > > rev ocotp logic.
> >
> > Though this is a good thing from maintenance point of view, we do not want
> > to break user spaces.
>
> Actually not break user spaces, since this is RFC, I not expect this be merged.
> If you agree, I could post normal V1 patchset.

Okay. You send formal patches, and we get them into linux-next to see
if people will complain any breakage.

Shawn