diff mbox series

[V3,1/3] ARM: imx: use device_initcall for imx_soc_device_init

Message ID 1589953889-30955-2-git-send-email-peng.fan@nxp.com (mailing list archive)
State Mainlined
Commit d2199b34871b859d33cd08398af5f1530241cb4e
Headers show
Series ARM: imx: move cpu code to drivers/soc/imx | expand

Commit Message

Peng Fan May 20, 2020, 5:51 a.m. UTC 
From: Peng Fan <peng.fan@nxp.com>

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 <peng.fan@nxp.com>
---
 arch/arm/mach-imx/common.h       |  1 -
 arch/arm/mach-imx/cpu.c          | 21 ++++++++++++++-------
 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/mach-vf610.c   |  8 +-------
 9 files changed, 20 insertions(+), 50 deletions(-)

Comments

Rob Herring (Arm) Oct. 28, 2020, 1:28 p.m. UTC | #1 
On Wed, May 20, 2020 at 1:01 AM <peng.fan@nxp.com> wrote:
>
> From: Peng Fan <peng.fan@nxp.com>
>
> 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 <peng.fan@nxp.com>
> ---

> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index 284bce1112d2..85c084a716ab 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -245,21 +245,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);

I know this is already applied, but really we want to get rid of
of_platform_default_populate calls and let the core code call it. i.MX
is one of the last remaining platforms to fix. This one remained
mainly because of 'parent'. Looks like there's some initcall ordering
issues to fix though. Maybe moving things to init_late works? If not,
moving the setup to drivers would be better if there's any ordering
issues as with deferred probe, initcalls alone don't guarantee the
order.

Rob
Peng Fan Oct. 28, 2020, 1:57 p.m. UTC | #2 
Hi Rob,

> Subject: Re: [PATCH V3 1/3] ARM: imx: use device_initcall for
> imx_soc_device_init
> 
> On Wed, May 20, 2020 at 1:01 AM <peng.fan@nxp.com> wrote:
> >
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > 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 <peng.fan@nxp.com>
> > ---
> 
> > diff --git a/arch/arm/mach-imx/mach-imx6q.c
> > b/arch/arm/mach-imx/mach-imx6q.c index 284bce1112d2..85c084a716ab
> > 100644
> > --- a/arch/arm/mach-imx/mach-imx6q.c
> > +++ b/arch/arm/mach-imx/mach-imx6q.c
> > @@ -245,21 +245,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);
> 
> I know this is already applied, but really we want to get rid of
> of_platform_default_populate calls and let the core code call it. i.MX is one of
> the last remaining platforms to fix.

Understand.

 This one remained mainly because of
> 'parent'. Looks like there's some initcall ordering issues to fix though. Maybe
> moving things to init_late works? If not, moving the setup to drivers would be
> better if there's any ordering issues as with deferred probe, initcalls alone
> don't guarantee the order.

I did one patch before, but breaks i.MX6Q pm code,
https://patchwork.ozlabs.org/project/linux-imx/patch/
20190313091757.16410-1-peng.fan@nxp.com/

Indeed there are some order issue, I will try again to clean up the stuff.

Thanks,
Peng.

> 
> Rob
diff mbox series

Patch

diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index 5aa5796cff0e..72c3fcc32910 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 e3d12b21d6f6..75ffcba9f878 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 -ENOMEM;
 
 	soc_dev_attr->family = "Freescale i.MX";
 
@@ -224,18 +224,24 @@  struct device * __init imx_soc_device_init(void)
 	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d.%d",
 					   (imx_soc_revision >> 4) & 0xf,
 					   imx_soc_revision & 0xf);
-	if (!soc_dev_attr->revision)
+	if (!soc_dev_attr->revision) {
+		ret = -ENOMEM;
 		goto free_soc;
+	}
 
 	soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX", soc_uid);
-	if (!soc_dev_attr->serial_number)
+	if (!soc_dev_attr->serial_number) {
+		ret = -ENOMEM;
 		goto free_rev;
+	}
 
 	soc_dev = soc_device_register(soc_dev_attr);
-	if (IS_ERR(soc_dev))
+	if (IS_ERR(soc_dev)) {
+		ret = PTR_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);
@@ -243,5 +249,6 @@  struct device * __init imx_soc_device_init(void)
 	kfree(soc_dev_attr->revision);
 free_soc:
 	kfree(soc_dev_attr);
-	return NULL;
+	return ret;
 }
+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 284bce1112d2..85c084a716ab 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -245,21 +245,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 e27a6889cc56..f6e87363d605 100644
--- a/arch/arm/mach-imx/mach-imx6sl.c
+++ b/arch/arm/mach-imx/mach-imx6sl.c
@@ -45,13 +45,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 3b0e16ccd59d..e018e716735f 100644
--- a/arch/arm/mach-imx/mach-imx6ul.c
+++ b/arch/arm/mach-imx/mach-imx6ul.c
@@ -55,13 +55,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 = {
diff --git a/arch/arm/mach-imx/mach-vf610.c b/arch/arm/mach-imx/mach-vf610.c
index 565dc08412a2..208ff640698d 100644
--- a/arch/arm/mach-imx/mach-vf610.c
+++ b/arch/arm/mach-imx/mach-vf610.c
@@ -49,15 +49,9 @@  static void __init vf610_detect_cpu(void)
 
 static void __init vf610_init_machine(void)
 {
-	struct device *parent;
-
 	vf610_detect_cpu();
 
-	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);
 }
 
 static const char * const vf610_dt_compat[] __initconst = {