Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Arnd Bergmann]

On Friday, August 26, 2016 7:12:51 PM CEST Anson Huang wrote:
> i.MX7D has 2 cortex-a7 ARM core, add support for
> booting up SMP kernel with 2 CPUs.
> 
> The existing i.MX SMP code is designed for i.MX6
> series SoCs which have cortex-a9 ARM core, but i.MX7D
> has 2 cortex-a7 ARM core, so we need to add runtime
> check for those differences between cortex-a9 and
> cortex-a7.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> 

Can't you just point i.MX7D to start from secondary_startup
rather than v7_secondary_startup?

 ENTRY(v7_secondary_startup)
+       .word   0xc070                  @ 0xc07 is cortex-a7 id
+       .word   0xfff0                  @ mask for core type
+

This looks like you are trying to execute instructions that are
actually data. Does this work?

On a side note, could you rename v7_secondary_startup to
imx6_secondary_startup? The name sounds overly generic
on a multiplatform kernel.

	Arnd

Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Arnd Bergmann]

On Friday, August 26, 2016 7:12:51 PM CEST Anson Huang wrote:
> i.MX7D has 2 cortex-a7 ARM core, add support for
> booting up SMP kernel with 2 CPUs.
> 
> The existing i.MX SMP code is designed for i.MX6
> series SoCs which have cortex-a9 ARM core, but i.MX7D
> has 2 cortex-a7 ARM core, so we need to add runtime
> check for those differences between cortex-a9 and
> cortex-a7.
> 
> Signed-off-by: Anson Huang <Anson.Huang-3arQi8VN3Tc@public.gmane.org>
> 

Can't you just point i.MX7D to start from secondary_startup
rather than v7_secondary_startup?

 ENTRY(v7_secondary_startup)
+       .word   0xc070                  @ 0xc07 is cortex-a7 id
+       .word   0xfff0                  @ mask for core type
+

This looks like you are trying to execute instructions that are
actually data. Does this work?

On a side note, could you rename v7_secondary_startup to
imx6_secondary_startup? The name sounds overly generic
on a multiplatform kernel.

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Arnd Bergmann]

On Friday, August 26, 2016 7:12:51 PM CEST Anson Huang wrote:
> i.MX7D has 2 cortex-a7 ARM core, add support for
> booting up SMP kernel with 2 CPUs.
> 
> The existing i.MX SMP code is designed for i.MX6
> series SoCs which have cortex-a9 ARM core, but i.MX7D
> has 2 cortex-a7 ARM core, so we need to add runtime
> check for those differences between cortex-a9 and
> cortex-a7.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> 

Can't you just point i.MX7D to start from secondary_startup
rather than v7_secondary_startup?

 ENTRY(v7_secondary_startup)
+       .word   0xc070                  @ 0xc07 is cortex-a7 id
+       .word   0xfff0                  @ mask for core type
+

This looks like you are trying to execute instructions that are
actually data. Does this work?

On a side note, could you rename v7_secondary_startup to
imx6_secondary_startup? The name sounds overly generic
on a multiplatform kernel.

	Arnd

RE: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 2016-08-26 4:59 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; shawnguo@kernel.org; kernel@pengutronix.de;
> Fabio Estevam <fabio.estevam@nxp.com>; robh+dt@kernel.org;
> mark.rutland@arm.com; linux@armlinux.org.uk
> Subject: Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D
> 
> On Friday, August 26, 2016 7:12:51 PM CEST Anson Huang wrote:
> > i.MX7D has 2 cortex-a7 ARM core, add support for booting up SMP kernel
> > with 2 CPUs.
> >
> > The existing i.MX SMP code is designed for i.MX6 series SoCs which
> > have cortex-a9 ARM core, but i.MX7D has 2 cortex-a7 ARM core, so we
> > need to add runtime check for those differences between cortex-a9 and
> > cortex-a7.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> >
> 
> Can't you just point i.MX7D to start from secondary_startup rather than
> v7_secondary_startup?
> 
>  ENTRY(v7_secondary_startup)
> +       .word   0xc070                  @ 0xc07 is cortex-a7 id
> +       .word   0xfff0                  @ mask for core type
> +
> 
> This looks like you are trying to execute instructions that are actually data. Does
> this work?
> 
> On a side note, could you rename v7_secondary_startup to
> imx6_secondary_startup? The name sounds overly generic on a multiplatform
> kernel.
> 
> 	Arnd


Thanks for review, yes, I made a mistake here, the data should put in another place,
but it accidently can work and boot up secondary CPU (see below the objdump),
 and also, I can just use  secondary_startup for i.MX7D. So I will
leave this file untouched, will verify the function next Mon and send out a V2 patch.

10 00000004 <v7_secondary_startup>:
 11    4:   0000c070        andeq   ip, r0, r0, ror r0
 12    8:   0000fff0        strdeq  pc, [r0], -r0

Anson.

RE: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 2016-08-26 4:59 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; shawnguo@kernel.org; kernel@pengutronix.de;
> Fabio Estevam <fabio.estevam@nxp.com>; robh+dt@kernel.org;
> mark.rutland@arm.com; linux@armlinux.org.uk
> Subject: Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D
> 
> On Friday, August 26, 2016 7:12:51 PM CEST Anson Huang wrote:
> > i.MX7D has 2 cortex-a7 ARM core, add support for booting up SMP kernel
> > with 2 CPUs.
> >
> > The existing i.MX SMP code is designed for i.MX6 series SoCs which
> > have cortex-a9 ARM core, but i.MX7D has 2 cortex-a7 ARM core, so we
> > need to add runtime check for those differences between cortex-a9 and
> > cortex-a7.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> >
> 
> Can't you just point i.MX7D to start from secondary_startup rather than
> v7_secondary_startup?
> 
>  ENTRY(v7_secondary_startup)
> +       .word   0xc070                  @ 0xc07 is cortex-a7 id
> +       .word   0xfff0                  @ mask for core type
> +
> 
> This looks like you are trying to execute instructions that are actually data. Does
> this work?
> 
> On a side note, could you rename v7_secondary_startup to
> imx6_secondary_startup? The name sounds overly generic on a multiplatform
> kernel.
> 
> 	Arnd


Thanks for review, yes, I made a mistake here, the data should put in another place,
but it accidently can work and boot up secondary CPU (see below the objdump),
 and also, I can just use  secondary_startup for i.MX7D. So I will
leave this file untouched, will verify the function next Mon and send out a V2 patch.

10 00000004 <v7_secondary_startup>:
 11    4:   0000c070        andeq   ip, r0, r0, ror r0
 12    8:   0000fff0        strdeq  pc, [r0], -r0

Anson.

[PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 2016-08-26 4:59 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; devicetree at vger.kernel.org; linux-
> kernel at vger.kernel.org; shawnguo at kernel.org; kernel at pengutronix.de;
> Fabio Estevam <fabio.estevam@nxp.com>; robh+dt at kernel.org;
> mark.rutland at arm.com; linux at armlinux.org.uk
> Subject: Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D
> 
> On Friday, August 26, 2016 7:12:51 PM CEST Anson Huang wrote:
> > i.MX7D has 2 cortex-a7 ARM core, add support for booting up SMP kernel
> > with 2 CPUs.
> >
> > The existing i.MX SMP code is designed for i.MX6 series SoCs which
> > have cortex-a9 ARM core, but i.MX7D has 2 cortex-a7 ARM core, so we
> > need to add runtime check for those differences between cortex-a9 and
> > cortex-a7.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> >
> 
> Can't you just point i.MX7D to start from secondary_startup rather than
> v7_secondary_startup?
> 
>  ENTRY(v7_secondary_startup)
> +       .word   0xc070                  @ 0xc07 is cortex-a7 id
> +       .word   0xfff0                  @ mask for core type
> +
> 
> This looks like you are trying to execute instructions that are actually data. Does
> this work?
> 
> On a side note, could you rename v7_secondary_startup to
> imx6_secondary_startup? The name sounds overly generic on a multiplatform
> kernel.
> 
> 	Arnd


Thanks for review, yes, I made a mistake here, the data should put in another place,
but it accidently can work and boot up secondary CPU (see below the objdump),
 and also, I can just use  secondary_startup for i.MX7D. So I will
leave this file untouched, will verify the function next Mon and send out a V2 patch.

10 00000004 <v7_secondary_startup>:
 11    4:   0000c070        andeq   ip, r0, r0, ror r0
 12    8:   0000fff0        strdeq  pc, [r0], -r0

Anson.

[PATCH 0/3] Add SMP support for i.MX7D

[Anson Huang]

i.MX7D has 2 Cortex-A7 ARM cores, and it has a different GPC design
than i.MX6, so this patch set adds a new GPCV2 driver for i.MX7D,
and also adds runtime check in SMP code to support both Cortex-A9
and Cortex-A7 ARM cores.

With this patch set, i.MX7D can boot up SMP kernel with 2 CPUs.

Anson Huang (3):
  ARM: dts: imx7: support SMP boot up
  ARM: imx: add gpcv2 support
  ARM: imx: add SMP support for i.MX7D

 arch/arm/boot/dts/imx7s.dtsi   |  8 +++++
 arch/arm/mach-imx/Kconfig      |  4 +++
 arch/arm/mach-imx/Makefile     |  1 +
 arch/arm/mach-imx/common.h     |  2 ++
 arch/arm/mach-imx/gpcv2.c      | 66 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-imx/headsmp.S    | 11 +++++++
 arch/arm/mach-imx/mach-imx7d.c |  2 ++
 arch/arm/mach-imx/platsmp.c    | 23 ++++++++++++++-
 arch/arm/mach-imx/src.c        | 38 +++++++++++++++++++-----
 9 files changed, 146 insertions(+), 9 deletions(-)
 create mode 100644 arch/arm/mach-imx/gpcv2.c

-- 
1.9.1

[PATCH 0/3] Add SMP support for i.MX7D

[Anson Huang]

i.MX7D has 2 Cortex-A7 ARM cores, and it has a different GPC design
than i.MX6, so this patch set adds a new GPCV2 driver for i.MX7D,
and also adds runtime check in SMP code to support both Cortex-A9
and Cortex-A7 ARM cores.

With this patch set, i.MX7D can boot up SMP kernel with 2 CPUs.

Anson Huang (3):
  ARM: dts: imx7: support SMP boot up
  ARM: imx: add gpcv2 support
  ARM: imx: add SMP support for i.MX7D

 arch/arm/boot/dts/imx7s.dtsi   |  8 +++++
 arch/arm/mach-imx/Kconfig      |  4 +++
 arch/arm/mach-imx/Makefile     |  1 +
 arch/arm/mach-imx/common.h     |  2 ++
 arch/arm/mach-imx/gpcv2.c      | 66 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-imx/headsmp.S    | 11 +++++++
 arch/arm/mach-imx/mach-imx7d.c |  2 ++
 arch/arm/mach-imx/platsmp.c    | 23 ++++++++++++++-
 arch/arm/mach-imx/src.c        | 38 +++++++++++++++++++-----
 9 files changed, 146 insertions(+), 9 deletions(-)
 create mode 100644 arch/arm/mach-imx/gpcv2.c

-- 
1.9.1

[PATCH 0/3] Add SMP support for i.MX7D

[Anson Huang]

i.MX7D has 2 Cortex-A7 ARM cores, and it has a different GPC design
than i.MX6, so this patch set adds a new GPCV2 driver for i.MX7D,
and also adds runtime check in SMP code to support both Cortex-A9
and Cortex-A7 ARM cores.

With this patch set, i.MX7D can boot up SMP kernel with 2 CPUs.

Anson Huang (3):
  ARM: dts: imx7: support SMP boot up
  ARM: imx: add gpcv2 support
  ARM: imx: add SMP support for i.MX7D

 arch/arm/boot/dts/imx7s.dtsi   |  8 +++++
 arch/arm/mach-imx/Kconfig      |  4 +++
 arch/arm/mach-imx/Makefile     |  1 +
 arch/arm/mach-imx/common.h     |  2 ++
 arch/arm/mach-imx/gpcv2.c      | 66 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-imx/headsmp.S    | 11 +++++++
 arch/arm/mach-imx/mach-imx7d.c |  2 ++
 arch/arm/mach-imx/platsmp.c    | 23 ++++++++++++++-
 arch/arm/mach-imx/src.c        | 38 +++++++++++++++++++-----
 9 files changed, 146 insertions(+), 9 deletions(-)
 create mode 100644 arch/arm/mach-imx/gpcv2.c

-- 
1.9.1

[PATCH 1/3] ARM: dts: imx7: support SMP boot up

[Anson Huang]

This patch adds GPC module, i.MX7 has a different
design of GPC module from i.MX6, so we call it GPCV2,
booting up secondary CPUs needs to access GPCV2
to enable secondary CPUs' power.

Also, adds "arm,cpu-registers-not-fw-configured"
property, interrupt parent and clock rate to
arch timer.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/boot/dts/imx7s.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index bb7102c..e920436 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -299,10 +299,13 @@
 
 		timer {
 			compatible = "arm,armv7-timer";
+			arm,cpu-registers-not-fw-configured;
 			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+			interrupt-parent = <&intc>;
+			clock-frequency = <8000000>;
 		};
 
 		aips1: aips-bus@30000000 {
@@ -312,6 +315,11 @@
 			reg = <0x30000000 0x400000>;
 			ranges;
 
+			gpc: gpc@303a0000 {
+				compatible = "fsl,imx7d-gpc";
+				reg = <0x303a0000 0x10000>;
+			};
+
 			gpio1: gpio@30200000 {
 				compatible = "fsl,imx7d-gpio", "fsl,imx35-gpio";
 				reg = <0x30200000 0x10000>;
-- 
1.9.1

[PATCH 1/3] ARM: dts: imx7: support SMP boot up

[Anson Huang]

This patch adds GPC module, i.MX7 has a different
design of GPC module from i.MX6, so we call it GPCV2,
booting up secondary CPUs needs to access GPCV2
to enable secondary CPUs' power.

Also, adds "arm,cpu-registers-not-fw-configured"
property, interrupt parent and clock rate to
arch timer.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/boot/dts/imx7s.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index bb7102c..e920436 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -299,10 +299,13 @@
 
 		timer {
 			compatible = "arm,armv7-timer";
+			arm,cpu-registers-not-fw-configured;
 			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+			interrupt-parent = <&intc>;
+			clock-frequency = <8000000>;
 		};
 
 		aips1: aips-bus@30000000 {
@@ -312,6 +315,11 @@
 			reg = <0x30000000 0x400000>;
 			ranges;
 
+			gpc: gpc@303a0000 {
+				compatible = "fsl,imx7d-gpc";
+				reg = <0x303a0000 0x10000>;
+			};
+
 			gpio1: gpio@30200000 {
 				compatible = "fsl,imx7d-gpio", "fsl,imx35-gpio";
 				reg = <0x30200000 0x10000>;
-- 
1.9.1

[PATCH 1/3] ARM: dts: imx7: support SMP boot up

[Anson Huang]

This patch adds GPC module, i.MX7 has a different
design of GPC module from i.MX6, so we call it GPCV2,
booting up secondary CPUs needs to access GPCV2
to enable secondary CPUs' power.

Also, adds "arm,cpu-registers-not-fw-configured"
property, interrupt parent and clock rate to
arch timer.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/boot/dts/imx7s.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index bb7102c..e920436 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -299,10 +299,13 @@
 
 		timer {
 			compatible = "arm,armv7-timer";
+			arm,cpu-registers-not-fw-configured;
 			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+			interrupt-parent = <&intc>;
+			clock-frequency = <8000000>;
 		};
 
 		aips1: aips-bus at 30000000 {
@@ -312,6 +315,11 @@
 			reg = <0x30000000 0x400000>;
 			ranges;
 
+			gpc: gpc at 303a0000 {
+				compatible = "fsl,imx7d-gpc";
+				reg = <0x303a0000 0x10000>;
+			};
+
 			gpio1: gpio at 30200000 {
 				compatible = "fsl,imx7d-gpio", "fsl,imx35-gpio";
 				reg = <0x30200000 0x10000>;
-- 
1.9.1

[PATCH 2/3] ARM: imx: add gpcv2 support

[Anson Huang]

i.MX7's GPC(general power controller) module is
different from i.MX6, name it as GPCV2 and add
its driver for SMP support, as secondary CPUs
boot up will need GPC to enable power.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/mach-imx/Kconfig  |  4 +++
 arch/arm/mach-imx/Makefile |  1 +
 arch/arm/mach-imx/common.h |  2 ++
 arch/arm/mach-imx/gpcv2.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+)
 create mode 100644 arch/arm/mach-imx/gpcv2.c

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 0ac05a0..13d5952 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -51,6 +51,9 @@ config HAVE_IMX_GPC
 	bool
 	select PM_GENERIC_DOMAINS if PM
 
+config HAVE_IMX_GPCV2
+	bool
+
 config HAVE_IMX_MMDC
 	bool
 
@@ -537,6 +540,7 @@ config SOC_IMX7D
 	select HAVE_IMX_ANATOP
 	select HAVE_IMX_MMDC
 	select HAVE_IMX_SRC
+	select HAVE_IMX_GPCV2
 	help
 		This enables support for Freescale i.MX7 Dual processor.
 
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index edce8df..6d812f6 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_MACH_IMX35_DT) += imx35-dt.o
 
 obj-$(CONFIG_HAVE_IMX_ANATOP) += anatop.o
 obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
+obj-$(CONFIG_HAVE_IMX_GPCV2) += gpcv2.o
 obj-$(CONFIG_HAVE_IMX_MMDC) += mmdc.o
 obj-$(CONFIG_HAVE_IMX_SRC) += src.o
 ifneq ($(CONFIG_SOC_IMX6)$(CONFIG_SOC_LS1021A),)
diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index b757811..732a19a 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -58,9 +58,11 @@ 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_gpcv2_check_dt(void);
 void imx_gpc_set_arm_power_in_lpm(bool power_off);
 void imx_gpc_set_arm_power_up_timing(u32 sw2iso, u32 sw);
 void imx_gpc_set_arm_power_down_timing(u32 sw2iso, u32 sw);
+void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn);
 void imx25_pm_init(void);
 void imx27_pm_init(void);
 
diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c
new file mode 100644
index 0000000..98577c4
--- /dev/null
+++ b/arch/arm/mach-imx/gpcv2.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include "common.h"
+
+#define GPC_CPU_PGC_SW_PUP_REQ	0xf0
+#define GPC_CPU_PGC_SW_PDN_REQ	0xfc
+#define GPC_PGC_C1		0x840
+
+#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7	0x2
+#define BM_GPC_PGC_PCG				0x1
+
+static void __iomem *gpcv2_base;
+
+static void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset)
+{
+	u32 val = readl_relaxed(gpcv2_base + offset) & (~BM_GPC_PGC_PCG);
+
+	if (enable)
+		val |= BM_GPC_PGC_PCG;
+
+	writel_relaxed(val, gpcv2_base + offset);
+}
+
+void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn)
+{
+	u32 val = readl_relaxed(gpcv2_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
+
+	imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1);
+	val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
+	writel_relaxed(val, gpcv2_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
+
+	while ((readl_relaxed(gpcv2_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ)) &
+		BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
+		;
+	imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1);
+}
+
+void __init imx_gpcv2_check_dt(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-gpc");
+	if (WARN_ON(!np))
+		return;
+
+	gpcv2_base = of_iomap(np, 0);
+	WARN_ON(!gpcv2_base);
+}
-- 
1.9.1

[PATCH 2/3] ARM: imx: add gpcv2 support

[Anson Huang]

i.MX7's GPC(general power controller) module is
different from i.MX6, name it as GPCV2 and add
its driver for SMP support, as secondary CPUs
boot up will need GPC to enable power.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/mach-imx/Kconfig  |  4 +++
 arch/arm/mach-imx/Makefile |  1 +
 arch/arm/mach-imx/common.h |  2 ++
 arch/arm/mach-imx/gpcv2.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+)
 create mode 100644 arch/arm/mach-imx/gpcv2.c

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 0ac05a0..13d5952 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -51,6 +51,9 @@ config HAVE_IMX_GPC
 	bool
 	select PM_GENERIC_DOMAINS if PM
 
+config HAVE_IMX_GPCV2
+	bool
+
 config HAVE_IMX_MMDC
 	bool
 
@@ -537,6 +540,7 @@ config SOC_IMX7D
 	select HAVE_IMX_ANATOP
 	select HAVE_IMX_MMDC
 	select HAVE_IMX_SRC
+	select HAVE_IMX_GPCV2
 	help
 		This enables support for Freescale i.MX7 Dual processor.
 
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index edce8df..6d812f6 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_MACH_IMX35_DT) += imx35-dt.o
 
 obj-$(CONFIG_HAVE_IMX_ANATOP) += anatop.o
 obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
+obj-$(CONFIG_HAVE_IMX_GPCV2) += gpcv2.o
 obj-$(CONFIG_HAVE_IMX_MMDC) += mmdc.o
 obj-$(CONFIG_HAVE_IMX_SRC) += src.o
 ifneq ($(CONFIG_SOC_IMX6)$(CONFIG_SOC_LS1021A),)
diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index b757811..732a19a 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -58,9 +58,11 @@ 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_gpcv2_check_dt(void);
 void imx_gpc_set_arm_power_in_lpm(bool power_off);
 void imx_gpc_set_arm_power_up_timing(u32 sw2iso, u32 sw);
 void imx_gpc_set_arm_power_down_timing(u32 sw2iso, u32 sw);
+void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn);
 void imx25_pm_init(void);
 void imx27_pm_init(void);
 
diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c
new file mode 100644
index 0000000..98577c4
--- /dev/null
+++ b/arch/arm/mach-imx/gpcv2.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include "common.h"
+
+#define GPC_CPU_PGC_SW_PUP_REQ	0xf0
+#define GPC_CPU_PGC_SW_PDN_REQ	0xfc
+#define GPC_PGC_C1		0x840
+
+#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7	0x2
+#define BM_GPC_PGC_PCG				0x1
+
+static void __iomem *gpcv2_base;
+
+static void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset)
+{
+	u32 val = readl_relaxed(gpcv2_base + offset) & (~BM_GPC_PGC_PCG);
+
+	if (enable)
+		val |= BM_GPC_PGC_PCG;
+
+	writel_relaxed(val, gpcv2_base + offset);
+}
+
+void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn)
+{
+	u32 val = readl_relaxed(gpcv2_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
+
+	imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1);
+	val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
+	writel_relaxed(val, gpcv2_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
+
+	while ((readl_relaxed(gpcv2_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ)) &
+		BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
+		;
+	imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1);
+}
+
+void __init imx_gpcv2_check_dt(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-gpc");
+	if (WARN_ON(!np))
+		return;
+
+	gpcv2_base = of_iomap(np, 0);
+	WARN_ON(!gpcv2_base);
+}
-- 
1.9.1

[PATCH 2/3] ARM: imx: add gpcv2 support

[Anson Huang]

i.MX7's GPC(general power controller) module is
different from i.MX6, name it as GPCV2 and add
its driver for SMP support, as secondary CPUs
boot up will need GPC to enable power.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/mach-imx/Kconfig  |  4 +++
 arch/arm/mach-imx/Makefile |  1 +
 arch/arm/mach-imx/common.h |  2 ++
 arch/arm/mach-imx/gpcv2.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+)
 create mode 100644 arch/arm/mach-imx/gpcv2.c

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 0ac05a0..13d5952 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -51,6 +51,9 @@ config HAVE_IMX_GPC
 	bool
 	select PM_GENERIC_DOMAINS if PM
 
+config HAVE_IMX_GPCV2
+	bool
+
 config HAVE_IMX_MMDC
 	bool
 
@@ -537,6 +540,7 @@ config SOC_IMX7D
 	select HAVE_IMX_ANATOP
 	select HAVE_IMX_MMDC
 	select HAVE_IMX_SRC
+	select HAVE_IMX_GPCV2
 	help
 		This enables support for Freescale i.MX7 Dual processor.
 
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index edce8df..6d812f6 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_MACH_IMX35_DT) += imx35-dt.o
 
 obj-$(CONFIG_HAVE_IMX_ANATOP) += anatop.o
 obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
+obj-$(CONFIG_HAVE_IMX_GPCV2) += gpcv2.o
 obj-$(CONFIG_HAVE_IMX_MMDC) += mmdc.o
 obj-$(CONFIG_HAVE_IMX_SRC) += src.o
 ifneq ($(CONFIG_SOC_IMX6)$(CONFIG_SOC_LS1021A),)
diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index b757811..732a19a 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -58,9 +58,11 @@ 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_gpcv2_check_dt(void);
 void imx_gpc_set_arm_power_in_lpm(bool power_off);
 void imx_gpc_set_arm_power_up_timing(u32 sw2iso, u32 sw);
 void imx_gpc_set_arm_power_down_timing(u32 sw2iso, u32 sw);
+void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn);
 void imx25_pm_init(void);
 void imx27_pm_init(void);
 
diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c
new file mode 100644
index 0000000..98577c4
--- /dev/null
+++ b/arch/arm/mach-imx/gpcv2.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include "common.h"
+
+#define GPC_CPU_PGC_SW_PUP_REQ	0xf0
+#define GPC_CPU_PGC_SW_PDN_REQ	0xfc
+#define GPC_PGC_C1		0x840
+
+#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7	0x2
+#define BM_GPC_PGC_PCG				0x1
+
+static void __iomem *gpcv2_base;
+
+static void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset)
+{
+	u32 val = readl_relaxed(gpcv2_base + offset) & (~BM_GPC_PGC_PCG);
+
+	if (enable)
+		val |= BM_GPC_PGC_PCG;
+
+	writel_relaxed(val, gpcv2_base + offset);
+}
+
+void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn)
+{
+	u32 val = readl_relaxed(gpcv2_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
+
+	imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1);
+	val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
+	writel_relaxed(val, gpcv2_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
+
+	while ((readl_relaxed(gpcv2_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ)) &
+		BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
+		;
+	imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1);
+}
+
+void __init imx_gpcv2_check_dt(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-gpc");
+	if (WARN_ON(!np))
+		return;
+
+	gpcv2_base = of_iomap(np, 0);
+	WARN_ON(!gpcv2_base);
+}
-- 
1.9.1

[PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Anson Huang]

i.MX7D has 2 cortex-a7 ARM core, add support for
booting up SMP kernel with 2 CPUs.

The existing i.MX SMP code is designed for i.MX6
series SoCs which have cortex-a9 ARM core, but i.MX7D
has 2 cortex-a7 ARM core, so we need to add runtime
check for those differences between cortex-a9 and
cortex-a7.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/mach-imx/headsmp.S    | 11 +++++++++++
 arch/arm/mach-imx/mach-imx7d.c |  2 ++
 arch/arm/mach-imx/platsmp.c    | 19 ++++++++++++++++++-
 arch/arm/mach-imx/src.c        | 38 ++++++++++++++++++++++++++++++--------
 4 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-imx/headsmp.S
index 6c28d28..a26e459 100644
--- a/arch/arm/mach-imx/headsmp.S
+++ b/arch/arm/mach-imx/headsmp.S
@@ -26,7 +26,18 @@ diag_reg_offset:
 	.endm
 
 ENTRY(v7_secondary_startup)
+	.word	0xc070			@ 0xc07 is cortex-a7 id
+	.word	0xfff0			@ mask for core type
+
 ARM_BE8(setend be)			@ go BE8 if entered LE
+	mrc     p15, 0, r0, c0, c0, 0
+	adr	r1, v7_secondary_startup
+	ldr	r2, [r1]
+	ldr	r3, [r1, #0x4]
+	and     r0, r0, r3
+	cmp     r0, r2
+	beq     secondary_startup
+
 	set_diag_reg
 	b	secondary_startup
 ENDPROC(v7_secondary_startup)
diff --git a/arch/arm/mach-imx/mach-imx7d.c b/arch/arm/mach-imx/mach-imx7d.c
index 26ca744..ef3dce6 100644
--- a/arch/arm/mach-imx/mach-imx7d.c
+++ b/arch/arm/mach-imx/mach-imx7d.c
@@ -99,6 +99,7 @@ static void __init imx7d_init_machine(void)
 
 static void __init imx7d_init_irq(void)
 {
+	imx_gpcv2_check_dt();
 	imx_init_revision_from_anatop();
 	imx_src_init();
 	irqchip_init();
@@ -111,6 +112,7 @@ static const char *const imx7d_dt_compat[] __initconst = {
 };
 
 DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
+	.smp		= smp_ops(imx_smp_ops),
 	.init_irq	= imx7d_init_irq,
 	.init_machine	= imx7d_init_machine,
 	.dt_compat	= imx7d_dt_compat,
diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
index 711dbbd..63af911 100644
--- a/arch/arm/mach-imx/platsmp.c
+++ b/arch/arm/mach-imx/platsmp.c
@@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu, struct task_struct *idle)
 static void __init imx_smp_init_cpus(void)
 {
 	int i, ncores;
+	unsigned long val, arch_type;
 
-	ncores = scu_get_core_count(scu_base);
+	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
+
+	if (((arch_type >> 4) & 0xfff) == 0xc07) {
+		/* cortex-a7 core number is in bit[25:24] of CP15 L2CTLR */
+		asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
+		ncores = ((val >> 24) & 0x3) + 1;
+	} else {
+		ncores = scu_get_core_count(scu_base);
+	}
 
 	for (i = ncores; i < NR_CPUS; i++)
 		set_cpu_possible(i, false);
@@ -74,6 +83,14 @@ void imx_smp_prepare(void)
 
 static void __init imx_smp_prepare_cpus(unsigned int max_cpus)
 {
+	unsigned long arch_type;
+
+	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
+
+	/* no need for cortex-a7 */
+	if (((arch_type >> 4) & 0xfff) == 0xc07)
+		return;
+
 	imx_smp_prepare();
 
 	/*
diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
index 70b083f..1fda72a 100644
--- a/arch/arm/mach-imx/src.c
+++ b/arch/arm/mach-imx/src.c
@@ -18,6 +18,7 @@
 #include <linux/smp.h>
 #include <asm/smp_plat.h>
 #include "common.h"
+#include "hardware.h"
 
 #define SRC_SCR				0x000
 #define SRC_GPR1			0x020
@@ -30,6 +31,15 @@
 #define BP_SRC_SCR_CORE1_RST		14
 #define BP_SRC_SCR_CORE1_ENABLE		22
 
+/* below are for i.MX7D */
+#define SRC_GPR1_V2                     0x074
+#define SRC_A7RCR0                      0x004
+#define SRC_A7RCR1                      0x008
+#define SRC_M4RCR                       0x00C
+
+#define BP_SRC_A7RCR0_A7_CORE_RESET0   0
+#define BP_SRC_A7RCR1_A7_CORE1_ENABLE  1
+
 static void __iomem *src_base;
 static DEFINE_SPINLOCK(scr_lock);
 
@@ -87,12 +97,21 @@ void imx_enable_cpu(int cpu, bool enable)
 	u32 mask, val;
 
 	cpu = cpu_logical_map(cpu);
-	mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
 	spin_lock(&scr_lock);
-	val = readl_relaxed(src_base + SRC_SCR);
-	val = enable ? val | mask : val & ~mask;
-	val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
-	writel_relaxed(val, src_base + SRC_SCR);
+	if (cpu_is_imx7d()) {
+		if (enable)
+			imx_gpcv2_set_core1_pdn_pup_by_software(false);
+		mask = 1 << (BP_SRC_A7RCR1_A7_CORE1_ENABLE + cpu - 1);
+		val = readl_relaxed(src_base + SRC_A7RCR1);
+		val = enable ? val | mask : val & ~mask;
+		writel_relaxed(val, src_base + SRC_A7RCR1);
+	} else {
+		mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
+		val = readl_relaxed(src_base + SRC_SCR);
+		val = enable ? val | mask : val & ~mask;
+		val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
+		writel_relaxed(val, src_base + SRC_SCR);
+	}
 	spin_unlock(&scr_lock);
 }
 
@@ -100,19 +119,22 @@ void imx_set_cpu_jump(int cpu, void *jump_addr)
 {
 	cpu = cpu_logical_map(cpu);
 	writel_relaxed(virt_to_phys(jump_addr),
-		       src_base + SRC_GPR1 + cpu * 8);
+		       src_base + (cpu_is_imx7d() ?
+		       SRC_GPR1_V2 : SRC_GPR1) + cpu * 8);
 }
 
 u32 imx_get_cpu_arg(int cpu)
 {
 	cpu = cpu_logical_map(cpu);
-	return readl_relaxed(src_base + SRC_GPR1 + cpu * 8 + 4);
+	return readl_relaxed(src_base + (cpu_is_imx7d() ?
+		SRC_GPR1_V2 : SRC_GPR1) + cpu * 8 + 4);
 }
 
 void imx_set_cpu_arg(int cpu, u32 arg)
 {
 	cpu = cpu_logical_map(cpu);
-	writel_relaxed(arg, src_base + SRC_GPR1 + cpu * 8 + 4);
+	writel_relaxed(arg, src_base + (cpu_is_imx7d() ?
+		SRC_GPR1_V2 : SRC_GPR1) + cpu * 8 + 4);
 }
 
 void __init imx_src_init(void)
-- 
1.9.1

[PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Anson Huang]

i.MX7D has 2 cortex-a7 ARM core, add support for
booting up SMP kernel with 2 CPUs.

The existing i.MX SMP code is designed for i.MX6
series SoCs which have cortex-a9 ARM core, but i.MX7D
has 2 cortex-a7 ARM core, so we need to add runtime
check for those differences between cortex-a9 and
cortex-a7.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/mach-imx/headsmp.S    | 11 +++++++++++
 arch/arm/mach-imx/mach-imx7d.c |  2 ++
 arch/arm/mach-imx/platsmp.c    | 19 ++++++++++++++++++-
 arch/arm/mach-imx/src.c        | 38 ++++++++++++++++++++++++++++++--------
 4 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-imx/headsmp.S
index 6c28d28..a26e459 100644
--- a/arch/arm/mach-imx/headsmp.S
+++ b/arch/arm/mach-imx/headsmp.S
@@ -26,7 +26,18 @@ diag_reg_offset:
 	.endm
 
 ENTRY(v7_secondary_startup)
+	.word	0xc070			@ 0xc07 is cortex-a7 id
+	.word	0xfff0			@ mask for core type
+
 ARM_BE8(setend be)			@ go BE8 if entered LE
+	mrc     p15, 0, r0, c0, c0, 0
+	adr	r1, v7_secondary_startup
+	ldr	r2, [r1]
+	ldr	r3, [r1, #0x4]
+	and     r0, r0, r3
+	cmp     r0, r2
+	beq     secondary_startup
+
 	set_diag_reg
 	b	secondary_startup
 ENDPROC(v7_secondary_startup)
diff --git a/arch/arm/mach-imx/mach-imx7d.c b/arch/arm/mach-imx/mach-imx7d.c
index 26ca744..ef3dce6 100644
--- a/arch/arm/mach-imx/mach-imx7d.c
+++ b/arch/arm/mach-imx/mach-imx7d.c
@@ -99,6 +99,7 @@ static void __init imx7d_init_machine(void)
 
 static void __init imx7d_init_irq(void)
 {
+	imx_gpcv2_check_dt();
 	imx_init_revision_from_anatop();
 	imx_src_init();
 	irqchip_init();
@@ -111,6 +112,7 @@ static const char *const imx7d_dt_compat[] __initconst = {
 };
 
 DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
+	.smp		= smp_ops(imx_smp_ops),
 	.init_irq	= imx7d_init_irq,
 	.init_machine	= imx7d_init_machine,
 	.dt_compat	= imx7d_dt_compat,
diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
index 711dbbd..63af911 100644
--- a/arch/arm/mach-imx/platsmp.c
+++ b/arch/arm/mach-imx/platsmp.c
@@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu, struct task_struct *idle)
 static void __init imx_smp_init_cpus(void)
 {
 	int i, ncores;
+	unsigned long val, arch_type;
 
-	ncores = scu_get_core_count(scu_base);
+	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
+
+	if (((arch_type >> 4) & 0xfff) == 0xc07) {
+		/* cortex-a7 core number is in bit[25:24] of CP15 L2CTLR */
+		asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
+		ncores = ((val >> 24) & 0x3) + 1;
+	} else {
+		ncores = scu_get_core_count(scu_base);
+	}
 
 	for (i = ncores; i < NR_CPUS; i++)
 		set_cpu_possible(i, false);
@@ -74,6 +83,14 @@ void imx_smp_prepare(void)
 
 static void __init imx_smp_prepare_cpus(unsigned int max_cpus)
 {
+	unsigned long arch_type;
+
+	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
+
+	/* no need for cortex-a7 */
+	if (((arch_type >> 4) & 0xfff) == 0xc07)
+		return;
+
 	imx_smp_prepare();
 
 	/*
diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
index 70b083f..1fda72a 100644
--- a/arch/arm/mach-imx/src.c
+++ b/arch/arm/mach-imx/src.c
@@ -18,6 +18,7 @@
 #include <linux/smp.h>
 #include <asm/smp_plat.h>
 #include "common.h"
+#include "hardware.h"
 
 #define SRC_SCR				0x000
 #define SRC_GPR1			0x020
@@ -30,6 +31,15 @@
 #define BP_SRC_SCR_CORE1_RST		14
 #define BP_SRC_SCR_CORE1_ENABLE		22
 
+/* below are for i.MX7D */
+#define SRC_GPR1_V2                     0x074
+#define SRC_A7RCR0                      0x004
+#define SRC_A7RCR1                      0x008
+#define SRC_M4RCR                       0x00C
+
+#define BP_SRC_A7RCR0_A7_CORE_RESET0   0
+#define BP_SRC_A7RCR1_A7_CORE1_ENABLE  1
+
 static void __iomem *src_base;
 static DEFINE_SPINLOCK(scr_lock);
 
@@ -87,12 +97,21 @@ void imx_enable_cpu(int cpu, bool enable)
 	u32 mask, val;
 
 	cpu = cpu_logical_map(cpu);
-	mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
 	spin_lock(&scr_lock);
-	val = readl_relaxed(src_base + SRC_SCR);
-	val = enable ? val | mask : val & ~mask;
-	val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
-	writel_relaxed(val, src_base + SRC_SCR);
+	if (cpu_is_imx7d()) {
+		if (enable)
+			imx_gpcv2_set_core1_pdn_pup_by_software(false);
+		mask = 1 << (BP_SRC_A7RCR1_A7_CORE1_ENABLE + cpu - 1);
+		val = readl_relaxed(src_base + SRC_A7RCR1);
+		val = enable ? val | mask : val & ~mask;
+		writel_relaxed(val, src_base + SRC_A7RCR1);
+	} else {
+		mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
+		val = readl_relaxed(src_base + SRC_SCR);
+		val = enable ? val | mask : val & ~mask;
+		val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
+		writel_relaxed(val, src_base + SRC_SCR);
+	}
 	spin_unlock(&scr_lock);
 }
 
@@ -100,19 +119,22 @@ void imx_set_cpu_jump(int cpu, void *jump_addr)
 {
 	cpu = cpu_logical_map(cpu);
 	writel_relaxed(virt_to_phys(jump_addr),
-		       src_base + SRC_GPR1 + cpu * 8);
+		       src_base + (cpu_is_imx7d() ?
+		       SRC_GPR1_V2 : SRC_GPR1) + cpu * 8);
 }
 
 u32 imx_get_cpu_arg(int cpu)
 {
 	cpu = cpu_logical_map(cpu);
-	return readl_relaxed(src_base + SRC_GPR1 + cpu * 8 + 4);
+	return readl_relaxed(src_base + (cpu_is_imx7d() ?
+		SRC_GPR1_V2 : SRC_GPR1) + cpu * 8 + 4);
 }
 
 void imx_set_cpu_arg(int cpu, u32 arg)
 {
 	cpu = cpu_logical_map(cpu);
-	writel_relaxed(arg, src_base + SRC_GPR1 + cpu * 8 + 4);
+	writel_relaxed(arg, src_base + (cpu_is_imx7d() ?
+		SRC_GPR1_V2 : SRC_GPR1) + cpu * 8 + 4);
 }
 
 void __init imx_src_init(void)
-- 
1.9.1

[PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Anson Huang]

i.MX7D has 2 cortex-a7 ARM core, add support for
booting up SMP kernel with 2 CPUs.

The existing i.MX SMP code is designed for i.MX6
series SoCs which have cortex-a9 ARM core, but i.MX7D
has 2 cortex-a7 ARM core, so we need to add runtime
check for those differences between cortex-a9 and
cortex-a7.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/mach-imx/headsmp.S    | 11 +++++++++++
 arch/arm/mach-imx/mach-imx7d.c |  2 ++
 arch/arm/mach-imx/platsmp.c    | 19 ++++++++++++++++++-
 arch/arm/mach-imx/src.c        | 38 ++++++++++++++++++++++++++++++--------
 4 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-imx/headsmp.S
index 6c28d28..a26e459 100644
--- a/arch/arm/mach-imx/headsmp.S
+++ b/arch/arm/mach-imx/headsmp.S
@@ -26,7 +26,18 @@ diag_reg_offset:
 	.endm
 
 ENTRY(v7_secondary_startup)
+	.word	0xc070			@ 0xc07 is cortex-a7 id
+	.word	0xfff0			@ mask for core type
+
 ARM_BE8(setend be)			@ go BE8 if entered LE
+	mrc     p15, 0, r0, c0, c0, 0
+	adr	r1, v7_secondary_startup
+	ldr	r2, [r1]
+	ldr	r3, [r1, #0x4]
+	and     r0, r0, r3
+	cmp     r0, r2
+	beq     secondary_startup
+
 	set_diag_reg
 	b	secondary_startup
 ENDPROC(v7_secondary_startup)
diff --git a/arch/arm/mach-imx/mach-imx7d.c b/arch/arm/mach-imx/mach-imx7d.c
index 26ca744..ef3dce6 100644
--- a/arch/arm/mach-imx/mach-imx7d.c
+++ b/arch/arm/mach-imx/mach-imx7d.c
@@ -99,6 +99,7 @@ static void __init imx7d_init_machine(void)
 
 static void __init imx7d_init_irq(void)
 {
+	imx_gpcv2_check_dt();
 	imx_init_revision_from_anatop();
 	imx_src_init();
 	irqchip_init();
@@ -111,6 +112,7 @@ static const char *const imx7d_dt_compat[] __initconst = {
 };
 
 DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
+	.smp		= smp_ops(imx_smp_ops),
 	.init_irq	= imx7d_init_irq,
 	.init_machine	= imx7d_init_machine,
 	.dt_compat	= imx7d_dt_compat,
diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
index 711dbbd..63af911 100644
--- a/arch/arm/mach-imx/platsmp.c
+++ b/arch/arm/mach-imx/platsmp.c
@@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu, struct task_struct *idle)
 static void __init imx_smp_init_cpus(void)
 {
 	int i, ncores;
+	unsigned long val, arch_type;
 
-	ncores = scu_get_core_count(scu_base);
+	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
+
+	if (((arch_type >> 4) & 0xfff) == 0xc07) {
+		/* cortex-a7 core number is in bit[25:24] of CP15 L2CTLR */
+		asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
+		ncores = ((val >> 24) & 0x3) + 1;
+	} else {
+		ncores = scu_get_core_count(scu_base);
+	}
 
 	for (i = ncores; i < NR_CPUS; i++)
 		set_cpu_possible(i, false);
@@ -74,6 +83,14 @@ void imx_smp_prepare(void)
 
 static void __init imx_smp_prepare_cpus(unsigned int max_cpus)
 {
+	unsigned long arch_type;
+
+	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
+
+	/* no need for cortex-a7 */
+	if (((arch_type >> 4) & 0xfff) == 0xc07)
+		return;
+
 	imx_smp_prepare();
 
 	/*
diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
index 70b083f..1fda72a 100644
--- a/arch/arm/mach-imx/src.c
+++ b/arch/arm/mach-imx/src.c
@@ -18,6 +18,7 @@
 #include <linux/smp.h>
 #include <asm/smp_plat.h>
 #include "common.h"
+#include "hardware.h"
 
 #define SRC_SCR				0x000
 #define SRC_GPR1			0x020
@@ -30,6 +31,15 @@
 #define BP_SRC_SCR_CORE1_RST		14
 #define BP_SRC_SCR_CORE1_ENABLE		22
 
+/* below are for i.MX7D */
+#define SRC_GPR1_V2                     0x074
+#define SRC_A7RCR0                      0x004
+#define SRC_A7RCR1                      0x008
+#define SRC_M4RCR                       0x00C
+
+#define BP_SRC_A7RCR0_A7_CORE_RESET0   0
+#define BP_SRC_A7RCR1_A7_CORE1_ENABLE  1
+
 static void __iomem *src_base;
 static DEFINE_SPINLOCK(scr_lock);
 
@@ -87,12 +97,21 @@ void imx_enable_cpu(int cpu, bool enable)
 	u32 mask, val;
 
 	cpu = cpu_logical_map(cpu);
-	mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
 	spin_lock(&scr_lock);
-	val = readl_relaxed(src_base + SRC_SCR);
-	val = enable ? val | mask : val & ~mask;
-	val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
-	writel_relaxed(val, src_base + SRC_SCR);
+	if (cpu_is_imx7d()) {
+		if (enable)
+			imx_gpcv2_set_core1_pdn_pup_by_software(false);
+		mask = 1 << (BP_SRC_A7RCR1_A7_CORE1_ENABLE + cpu - 1);
+		val = readl_relaxed(src_base + SRC_A7RCR1);
+		val = enable ? val | mask : val & ~mask;
+		writel_relaxed(val, src_base + SRC_A7RCR1);
+	} else {
+		mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
+		val = readl_relaxed(src_base + SRC_SCR);
+		val = enable ? val | mask : val & ~mask;
+		val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
+		writel_relaxed(val, src_base + SRC_SCR);
+	}
 	spin_unlock(&scr_lock);
 }
 
@@ -100,19 +119,22 @@ void imx_set_cpu_jump(int cpu, void *jump_addr)
 {
 	cpu = cpu_logical_map(cpu);
 	writel_relaxed(virt_to_phys(jump_addr),
-		       src_base + SRC_GPR1 + cpu * 8);
+		       src_base + (cpu_is_imx7d() ?
+		       SRC_GPR1_V2 : SRC_GPR1) + cpu * 8);
 }
 
 u32 imx_get_cpu_arg(int cpu)
 {
 	cpu = cpu_logical_map(cpu);
-	return readl_relaxed(src_base + SRC_GPR1 + cpu * 8 + 4);
+	return readl_relaxed(src_base + (cpu_is_imx7d() ?
+		SRC_GPR1_V2 : SRC_GPR1) + cpu * 8 + 4);
 }
 
 void imx_set_cpu_arg(int cpu, u32 arg)
 {
 	cpu = cpu_logical_map(cpu);
-	writel_relaxed(arg, src_base + SRC_GPR1 + cpu * 8 + 4);
+	writel_relaxed(arg, src_base + (cpu_is_imx7d() ?
+		SRC_GPR1_V2 : SRC_GPR1) + cpu * 8 + 4);
 }
 
 void __init imx_src_init(void)
-- 
1.9.1

Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 07:12:51PM +0800, Anson Huang wrote:
> i.MX7D has 2 cortex-a7 ARM core, add support for
> booting up SMP kernel with 2 CPUs.
> 
> The existing i.MX SMP code is designed for i.MX6
> series SoCs which have cortex-a9 ARM core, but i.MX7D
> has 2 cortex-a7 ARM core, so we need to add runtime
> check for those differences between cortex-a9 and
> cortex-a7.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  arch/arm/mach-imx/headsmp.S    | 11 +++++++++++
>  arch/arm/mach-imx/mach-imx7d.c |  2 ++
>  arch/arm/mach-imx/platsmp.c    | 19 ++++++++++++++++++-
>  arch/arm/mach-imx/src.c        | 38 ++++++++++++++++++++++++++++++--------
>  4 files changed, 61 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-imx/headsmp.S
> index 6c28d28..a26e459 100644
> --- a/arch/arm/mach-imx/headsmp.S
> +++ b/arch/arm/mach-imx/headsmp.S
> @@ -26,7 +26,18 @@ diag_reg_offset:
>  	.endm
>  
>  ENTRY(v7_secondary_startup)
> +	.word	0xc070			@ 0xc07 is cortex-a7 id
> +	.word	0xfff0			@ mask for core type
> +
>  ARM_BE8(setend be)			@ go BE8 if entered LE
> +	mrc     p15, 0, r0, c0, c0, 0
> +	adr	r1, v7_secondary_startup
> +	ldr	r2, [r1]
> +	ldr	r3, [r1, #0x4]
> +	and     r0, r0, r3
> +	cmp     r0, r2
> +	beq     secondary_startup
> +

Total NAK on the above.  There's no way that we want to even try
executing those two .word values (and I don't care if "it works
when tested", we just don't try and execute data.)

>  	set_diag_reg
>  	b	secondary_startup
>  ENDPROC(v7_secondary_startup)
> diff --git a/arch/arm/mach-imx/mach-imx7d.c b/arch/arm/mach-imx/mach-imx7d.c
> index 26ca744..ef3dce6 100644
> --- a/arch/arm/mach-imx/mach-imx7d.c
> +++ b/arch/arm/mach-imx/mach-imx7d.c
> @@ -99,6 +99,7 @@ static void __init imx7d_init_machine(void)
>  
>  static void __init imx7d_init_irq(void)
>  {
> +	imx_gpcv2_check_dt();
>  	imx_init_revision_from_anatop();
>  	imx_src_init();
>  	irqchip_init();
> @@ -111,6 +112,7 @@ static const char *const imx7d_dt_compat[] __initconst = {
>  };
>  
>  DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
> +	.smp		= smp_ops(imx_smp_ops),
>  	.init_irq	= imx7d_init_irq,
>  	.init_machine	= imx7d_init_machine,
>  	.dt_compat	= imx7d_dt_compat,
> diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
> index 711dbbd..63af911 100644
> --- a/arch/arm/mach-imx/platsmp.c
> +++ b/arch/arm/mach-imx/platsmp.c
> @@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu, struct task_struct *idle)
>  static void __init imx_smp_init_cpus(void)
>  {
>  	int i, ncores;
> +	unsigned long val, arch_type;
>  
> -	ncores = scu_get_core_count(scu_base);
> +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> +
> +	if (((arch_type >> 4) & 0xfff) == 0xc07) {

This is buggy.  Plus, we have macros for this.  Please use the
macros in asm/cputype.h to achieve these tests.

> +		/* cortex-a7 core number is in bit[25:24] of CP15 L2CTLR */
> +		asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
> +		ncores = ((val >> 24) & 0x3) + 1;
> +	} else {
> +		ncores = scu_get_core_count(scu_base);
> +	}
>  
>  	for (i = ncores; i < NR_CPUS; i++)
>  		set_cpu_possible(i, false);
> @@ -74,6 +83,14 @@ void imx_smp_prepare(void)
>  
>  static void __init imx_smp_prepare_cpus(unsigned int max_cpus)
>  {
> +	unsigned long arch_type;
> +
> +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> +
> +	/* no need for cortex-a7 */
> +	if (((arch_type >> 4) & 0xfff) == 0xc07)

Ditto.

> +		return;
> +
>  	imx_smp_prepare();
>  
>  	/*
> diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
> index 70b083f..1fda72a 100644
> --- a/arch/arm/mach-imx/src.c
> +++ b/arch/arm/mach-imx/src.c
> @@ -18,6 +18,7 @@
>  #include <linux/smp.h>
>  #include <asm/smp_plat.h>
>  #include "common.h"
> +#include "hardware.h"
>  
>  #define SRC_SCR				0x000
>  #define SRC_GPR1			0x020
> @@ -30,6 +31,15 @@
>  #define BP_SRC_SCR_CORE1_RST		14
>  #define BP_SRC_SCR_CORE1_ENABLE		22
>  
> +/* below are for i.MX7D */
> +#define SRC_GPR1_V2                     0x074
> +#define SRC_A7RCR0                      0x004
> +#define SRC_A7RCR1                      0x008
> +#define SRC_M4RCR                       0x00C
> +
> +#define BP_SRC_A7RCR0_A7_CORE_RESET0   0
> +#define BP_SRC_A7RCR1_A7_CORE1_ENABLE  1
> +
>  static void __iomem *src_base;
>  static DEFINE_SPINLOCK(scr_lock);
>  
> @@ -87,12 +97,21 @@ void imx_enable_cpu(int cpu, bool enable)
>  	u32 mask, val;
>  
>  	cpu = cpu_logical_map(cpu);
> -	mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
>  	spin_lock(&scr_lock);
> -	val = readl_relaxed(src_base + SRC_SCR);
> -	val = enable ? val | mask : val & ~mask;
> -	val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
> -	writel_relaxed(val, src_base + SRC_SCR);
> +	if (cpu_is_imx7d()) {

It's about time iMX folk learned that "imx*" is a SoC and _not_ a CPU.
It should be "soc_is_imx7d()" because we're wanting to know whether
the _SoC_ is an iMX7D.  The _CPU_ is a _Cortex-A7_.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 07:12:51PM +0800, Anson Huang wrote:
> i.MX7D has 2 cortex-a7 ARM core, add support for
> booting up SMP kernel with 2 CPUs.
> 
> The existing i.MX SMP code is designed for i.MX6
> series SoCs which have cortex-a9 ARM core, but i.MX7D
> has 2 cortex-a7 ARM core, so we need to add runtime
> check for those differences between cortex-a9 and
> cortex-a7.
> 
> Signed-off-by: Anson Huang <Anson.Huang-3arQi8VN3Tc@public.gmane.org>
> ---
>  arch/arm/mach-imx/headsmp.S    | 11 +++++++++++
>  arch/arm/mach-imx/mach-imx7d.c |  2 ++
>  arch/arm/mach-imx/platsmp.c    | 19 ++++++++++++++++++-
>  arch/arm/mach-imx/src.c        | 38 ++++++++++++++++++++++++++++++--------
>  4 files changed, 61 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-imx/headsmp.S
> index 6c28d28..a26e459 100644
> --- a/arch/arm/mach-imx/headsmp.S
> +++ b/arch/arm/mach-imx/headsmp.S
> @@ -26,7 +26,18 @@ diag_reg_offset:
>  	.endm
>  
>  ENTRY(v7_secondary_startup)
> +	.word	0xc070			@ 0xc07 is cortex-a7 id
> +	.word	0xfff0			@ mask for core type
> +
>  ARM_BE8(setend be)			@ go BE8 if entered LE
> +	mrc     p15, 0, r0, c0, c0, 0
> +	adr	r1, v7_secondary_startup
> +	ldr	r2, [r1]
> +	ldr	r3, [r1, #0x4]
> +	and     r0, r0, r3
> +	cmp     r0, r2
> +	beq     secondary_startup
> +

Total NAK on the above.  There's no way that we want to even try
executing those two .word values (and I don't care if "it works
when tested", we just don't try and execute data.)

>  	set_diag_reg
>  	b	secondary_startup
>  ENDPROC(v7_secondary_startup)
> diff --git a/arch/arm/mach-imx/mach-imx7d.c b/arch/arm/mach-imx/mach-imx7d.c
> index 26ca744..ef3dce6 100644
> --- a/arch/arm/mach-imx/mach-imx7d.c
> +++ b/arch/arm/mach-imx/mach-imx7d.c
> @@ -99,6 +99,7 @@ static void __init imx7d_init_machine(void)
>  
>  static void __init imx7d_init_irq(void)
>  {
> +	imx_gpcv2_check_dt();
>  	imx_init_revision_from_anatop();
>  	imx_src_init();
>  	irqchip_init();
> @@ -111,6 +112,7 @@ static const char *const imx7d_dt_compat[] __initconst = {
>  };
>  
>  DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
> +	.smp		= smp_ops(imx_smp_ops),
>  	.init_irq	= imx7d_init_irq,
>  	.init_machine	= imx7d_init_machine,
>  	.dt_compat	= imx7d_dt_compat,
> diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
> index 711dbbd..63af911 100644
> --- a/arch/arm/mach-imx/platsmp.c
> +++ b/arch/arm/mach-imx/platsmp.c
> @@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu, struct task_struct *idle)
>  static void __init imx_smp_init_cpus(void)
>  {
>  	int i, ncores;
> +	unsigned long val, arch_type;
>  
> -	ncores = scu_get_core_count(scu_base);
> +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> +
> +	if (((arch_type >> 4) & 0xfff) == 0xc07) {

This is buggy.  Plus, we have macros for this.  Please use the
macros in asm/cputype.h to achieve these tests.

> +		/* cortex-a7 core number is in bit[25:24] of CP15 L2CTLR */
> +		asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
> +		ncores = ((val >> 24) & 0x3) + 1;
> +	} else {
> +		ncores = scu_get_core_count(scu_base);
> +	}
>  
>  	for (i = ncores; i < NR_CPUS; i++)
>  		set_cpu_possible(i, false);
> @@ -74,6 +83,14 @@ void imx_smp_prepare(void)
>  
>  static void __init imx_smp_prepare_cpus(unsigned int max_cpus)
>  {
> +	unsigned long arch_type;
> +
> +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> +
> +	/* no need for cortex-a7 */
> +	if (((arch_type >> 4) & 0xfff) == 0xc07)

Ditto.

> +		return;
> +
>  	imx_smp_prepare();
>  
>  	/*
> diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
> index 70b083f..1fda72a 100644
> --- a/arch/arm/mach-imx/src.c
> +++ b/arch/arm/mach-imx/src.c
> @@ -18,6 +18,7 @@
>  #include <linux/smp.h>
>  #include <asm/smp_plat.h>
>  #include "common.h"
> +#include "hardware.h"
>  
>  #define SRC_SCR				0x000
>  #define SRC_GPR1			0x020
> @@ -30,6 +31,15 @@
>  #define BP_SRC_SCR_CORE1_RST		14
>  #define BP_SRC_SCR_CORE1_ENABLE		22
>  
> +/* below are for i.MX7D */
> +#define SRC_GPR1_V2                     0x074
> +#define SRC_A7RCR0                      0x004
> +#define SRC_A7RCR1                      0x008
> +#define SRC_M4RCR                       0x00C
> +
> +#define BP_SRC_A7RCR0_A7_CORE_RESET0   0
> +#define BP_SRC_A7RCR1_A7_CORE1_ENABLE  1
> +
>  static void __iomem *src_base;
>  static DEFINE_SPINLOCK(scr_lock);
>  
> @@ -87,12 +97,21 @@ void imx_enable_cpu(int cpu, bool enable)
>  	u32 mask, val;
>  
>  	cpu = cpu_logical_map(cpu);
> -	mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
>  	spin_lock(&scr_lock);
> -	val = readl_relaxed(src_base + SRC_SCR);
> -	val = enable ? val | mask : val & ~mask;
> -	val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
> -	writel_relaxed(val, src_base + SRC_SCR);
> +	if (cpu_is_imx7d()) {

It's about time iMX folk learned that "imx*" is a SoC and _not_ a CPU.
It should be "soc_is_imx7d()" because we're wanting to know whether
the _SoC_ is an iMX7D.  The _CPU_ is a _Cortex-A7_.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 07:12:51PM +0800, Anson Huang wrote:
> i.MX7D has 2 cortex-a7 ARM core, add support for
> booting up SMP kernel with 2 CPUs.
> 
> The existing i.MX SMP code is designed for i.MX6
> series SoCs which have cortex-a9 ARM core, but i.MX7D
> has 2 cortex-a7 ARM core, so we need to add runtime
> check for those differences between cortex-a9 and
> cortex-a7.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  arch/arm/mach-imx/headsmp.S    | 11 +++++++++++
>  arch/arm/mach-imx/mach-imx7d.c |  2 ++
>  arch/arm/mach-imx/platsmp.c    | 19 ++++++++++++++++++-
>  arch/arm/mach-imx/src.c        | 38 ++++++++++++++++++++++++++++++--------
>  4 files changed, 61 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-imx/headsmp.S
> index 6c28d28..a26e459 100644
> --- a/arch/arm/mach-imx/headsmp.S
> +++ b/arch/arm/mach-imx/headsmp.S
> @@ -26,7 +26,18 @@ diag_reg_offset:
>  	.endm
>  
>  ENTRY(v7_secondary_startup)
> +	.word	0xc070			@ 0xc07 is cortex-a7 id
> +	.word	0xfff0			@ mask for core type
> +
>  ARM_BE8(setend be)			@ go BE8 if entered LE
> +	mrc     p15, 0, r0, c0, c0, 0
> +	adr	r1, v7_secondary_startup
> +	ldr	r2, [r1]
> +	ldr	r3, [r1, #0x4]
> +	and     r0, r0, r3
> +	cmp     r0, r2
> +	beq     secondary_startup
> +

Total NAK on the above.  There's no way that we want to even try
executing those two .word values (and I don't care if "it works
when tested", we just don't try and execute data.)

>  	set_diag_reg
>  	b	secondary_startup
>  ENDPROC(v7_secondary_startup)
> diff --git a/arch/arm/mach-imx/mach-imx7d.c b/arch/arm/mach-imx/mach-imx7d.c
> index 26ca744..ef3dce6 100644
> --- a/arch/arm/mach-imx/mach-imx7d.c
> +++ b/arch/arm/mach-imx/mach-imx7d.c
> @@ -99,6 +99,7 @@ static void __init imx7d_init_machine(void)
>  
>  static void __init imx7d_init_irq(void)
>  {
> +	imx_gpcv2_check_dt();
>  	imx_init_revision_from_anatop();
>  	imx_src_init();
>  	irqchip_init();
> @@ -111,6 +112,7 @@ static const char *const imx7d_dt_compat[] __initconst = {
>  };
>  
>  DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
> +	.smp		= smp_ops(imx_smp_ops),
>  	.init_irq	= imx7d_init_irq,
>  	.init_machine	= imx7d_init_machine,
>  	.dt_compat	= imx7d_dt_compat,
> diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
> index 711dbbd..63af911 100644
> --- a/arch/arm/mach-imx/platsmp.c
> +++ b/arch/arm/mach-imx/platsmp.c
> @@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu, struct task_struct *idle)
>  static void __init imx_smp_init_cpus(void)
>  {
>  	int i, ncores;
> +	unsigned long val, arch_type;
>  
> -	ncores = scu_get_core_count(scu_base);
> +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> +
> +	if (((arch_type >> 4) & 0xfff) == 0xc07) {

This is buggy.  Plus, we have macros for this.  Please use the
macros in asm/cputype.h to achieve these tests.

> +		/* cortex-a7 core number is in bit[25:24] of CP15 L2CTLR */
> +		asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
> +		ncores = ((val >> 24) & 0x3) + 1;
> +	} else {
> +		ncores = scu_get_core_count(scu_base);
> +	}
>  
>  	for (i = ncores; i < NR_CPUS; i++)
>  		set_cpu_possible(i, false);
> @@ -74,6 +83,14 @@ void imx_smp_prepare(void)
>  
>  static void __init imx_smp_prepare_cpus(unsigned int max_cpus)
>  {
> +	unsigned long arch_type;
> +
> +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> +
> +	/* no need for cortex-a7 */
> +	if (((arch_type >> 4) & 0xfff) == 0xc07)

Ditto.

> +		return;
> +
>  	imx_smp_prepare();
>  
>  	/*
> diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
> index 70b083f..1fda72a 100644
> --- a/arch/arm/mach-imx/src.c
> +++ b/arch/arm/mach-imx/src.c
> @@ -18,6 +18,7 @@
>  #include <linux/smp.h>
>  #include <asm/smp_plat.h>
>  #include "common.h"
> +#include "hardware.h"
>  
>  #define SRC_SCR				0x000
>  #define SRC_GPR1			0x020
> @@ -30,6 +31,15 @@
>  #define BP_SRC_SCR_CORE1_RST		14
>  #define BP_SRC_SCR_CORE1_ENABLE		22
>  
> +/* below are for i.MX7D */
> +#define SRC_GPR1_V2                     0x074
> +#define SRC_A7RCR0                      0x004
> +#define SRC_A7RCR1                      0x008
> +#define SRC_M4RCR                       0x00C
> +
> +#define BP_SRC_A7RCR0_A7_CORE_RESET0   0
> +#define BP_SRC_A7RCR1_A7_CORE1_ENABLE  1
> +
>  static void __iomem *src_base;
>  static DEFINE_SPINLOCK(scr_lock);
>  
> @@ -87,12 +97,21 @@ void imx_enable_cpu(int cpu, bool enable)
>  	u32 mask, val;
>  
>  	cpu = cpu_logical_map(cpu);
> -	mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
>  	spin_lock(&scr_lock);
> -	val = readl_relaxed(src_base + SRC_SCR);
> -	val = enable ? val | mask : val & ~mask;
> -	val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
> -	writel_relaxed(val, src_base + SRC_SCR);
> +	if (cpu_is_imx7d()) {

It's about time iMX folk learned that "imx*" is a SoC and _not_ a CPU.
It should be "soc_is_imx7d()" because we're wanting to know whether
the _SoC_ is an iMX7D.  The _CPU_ is a _Cortex-A7_.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/3] ARM: imx: add gpcv2 support

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 07:12:50PM +0800, Anson Huang wrote:
> diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c
> new file mode 100644
> index 0000000..98577c4
> --- /dev/null
> +++ b/arch/arm/mach-imx/gpcv2.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +
> +#include "common.h"
> +
> +#define GPC_CPU_PGC_SW_PUP_REQ	0xf0
> +#define GPC_CPU_PGC_SW_PDN_REQ	0xfc
> +#define GPC_PGC_C1		0x840
> +
> +#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7	0x2
> +#define BM_GPC_PGC_PCG				0x1
> +
> +static void __iomem *gpcv2_base;
> +
> +static void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset)
> +{
> +	u32 val = readl_relaxed(gpcv2_base + offset) & (~BM_GPC_PGC_PCG);

Unnecessary parens, and the code doesn't flow with the bit clearance
here...

> +
> +	if (enable)
> +		val |= BM_GPC_PGC_PCG;

My first read of this lead me to think "okay, so what's the point of
enable=false".  It would be clearer with:

	u32 val = readl_relaxed(gpcv2_base + offset);

	if (enable)
		val |= BM_GPC_PGC_PCG;
	else
		val &= ~BM_GPC_PGC_PCG;

here.

> +
> +	writel_relaxed(val, gpcv2_base + offset);
> +}
> +
> +void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn)
> +{
> +	u32 val = readl_relaxed(gpcv2_base + (pdn ?
> +		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
> +
> +	imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1);
> +	val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
> +	writel_relaxed(val, gpcv2_base + (pdn ?
> +		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
> +
> +	while ((readl_relaxed(gpcv2_base + (pdn ?
> +		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ)) &
> +		BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
> +		;
> +	imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1);
> +}
> +
> +void __init imx_gpcv2_check_dt(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-gpc");
> +	if (WARN_ON(!np))
> +		return;
> +
> +	gpcv2_base = of_iomap(np, 0);
> +	WARN_ON(!gpcv2_base);

What happens if gpcv2_base is NULL (apart from the obvious warning
from the above line)?  I guess a bit later in the boot,
imx_gpcv2_set_core1_pdn_pup_by_software() oopses the kernel,
probably before the console has been initialised.  Probably not
nice behaviour.

> +}
> -- 
> 1.9.1
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/3] ARM: imx: add gpcv2 support

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 07:12:50PM +0800, Anson Huang wrote:
> diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c
> new file mode 100644
> index 0000000..98577c4
> --- /dev/null
> +++ b/arch/arm/mach-imx/gpcv2.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +
> +#include "common.h"
> +
> +#define GPC_CPU_PGC_SW_PUP_REQ	0xf0
> +#define GPC_CPU_PGC_SW_PDN_REQ	0xfc
> +#define GPC_PGC_C1		0x840
> +
> +#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7	0x2
> +#define BM_GPC_PGC_PCG				0x1
> +
> +static void __iomem *gpcv2_base;
> +
> +static void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset)
> +{
> +	u32 val = readl_relaxed(gpcv2_base + offset) & (~BM_GPC_PGC_PCG);

Unnecessary parens, and the code doesn't flow with the bit clearance
here...

> +
> +	if (enable)
> +		val |= BM_GPC_PGC_PCG;

My first read of this lead me to think "okay, so what's the point of
enable=false".  It would be clearer with:

	u32 val = readl_relaxed(gpcv2_base + offset);

	if (enable)
		val |= BM_GPC_PGC_PCG;
	else
		val &= ~BM_GPC_PGC_PCG;

here.

> +
> +	writel_relaxed(val, gpcv2_base + offset);
> +}
> +
> +void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn)
> +{
> +	u32 val = readl_relaxed(gpcv2_base + (pdn ?
> +		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
> +
> +	imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1);
> +	val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
> +	writel_relaxed(val, gpcv2_base + (pdn ?
> +		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
> +
> +	while ((readl_relaxed(gpcv2_base + (pdn ?
> +		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ)) &
> +		BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
> +		;
> +	imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1);
> +}
> +
> +void __init imx_gpcv2_check_dt(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-gpc");
> +	if (WARN_ON(!np))
> +		return;
> +
> +	gpcv2_base = of_iomap(np, 0);
> +	WARN_ON(!gpcv2_base);

What happens if gpcv2_base is NULL (apart from the obvious warning
from the above line)?  I guess a bit later in the boot,
imx_gpcv2_set_core1_pdn_pup_by_software() oopses the kernel,
probably before the console has been initialised.  Probably not
nice behaviour.

> +}
> -- 
> 1.9.1
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 2/3] ARM: imx: add gpcv2 support

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 07:12:50PM +0800, Anson Huang wrote:
> diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c
> new file mode 100644
> index 0000000..98577c4
> --- /dev/null
> +++ b/arch/arm/mach-imx/gpcv2.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +
> +#include "common.h"
> +
> +#define GPC_CPU_PGC_SW_PUP_REQ	0xf0
> +#define GPC_CPU_PGC_SW_PDN_REQ	0xfc
> +#define GPC_PGC_C1		0x840
> +
> +#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7	0x2
> +#define BM_GPC_PGC_PCG				0x1
> +
> +static void __iomem *gpcv2_base;
> +
> +static void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset)
> +{
> +	u32 val = readl_relaxed(gpcv2_base + offset) & (~BM_GPC_PGC_PCG);

Unnecessary parens, and the code doesn't flow with the bit clearance
here...

> +
> +	if (enable)
> +		val |= BM_GPC_PGC_PCG;

My first read of this lead me to think "okay, so what's the point of
enable=false".  It would be clearer with:

	u32 val = readl_relaxed(gpcv2_base + offset);

	if (enable)
		val |= BM_GPC_PGC_PCG;
	else
		val &= ~BM_GPC_PGC_PCG;

here.

> +
> +	writel_relaxed(val, gpcv2_base + offset);
> +}
> +
> +void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn)
> +{
> +	u32 val = readl_relaxed(gpcv2_base + (pdn ?
> +		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
> +
> +	imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1);
> +	val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
> +	writel_relaxed(val, gpcv2_base + (pdn ?
> +		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
> +
> +	while ((readl_relaxed(gpcv2_base + (pdn ?
> +		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ)) &
> +		BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
> +		;
> +	imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1);
> +}
> +
> +void __init imx_gpcv2_check_dt(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-gpc");
> +	if (WARN_ON(!np))
> +		return;
> +
> +	gpcv2_base = of_iomap(np, 0);
> +	WARN_ON(!gpcv2_base);

What happens if gpcv2_base is NULL (apart from the obvious warning
from the above line)?  I guess a bit later in the boot,
imx_gpcv2_set_core1_pdn_pup_by_software() oopses the kernel,
probably before the console has been initialised.  Probably not
nice behaviour.

> +}
> -- 
> 1.9.1
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

RE: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@armlinux.org.uk]
> Sent: 2016-08-26 7:14 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; shawnguo@kernel.org; kernel@pengutronix.de;
> Fabio Estevam <fabio.estevam@nxp.com>; robh+dt@kernel.org;
> mark.rutland@arm.com
> Subject: Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D
> 
> On Fri, Aug 26, 2016 at 07:12:51PM +0800, Anson Huang wrote:
> > i.MX7D has 2 cortex-a7 ARM core, add support for booting up SMP kernel
> > with 2 CPUs.
> >
> > The existing i.MX SMP code is designed for i.MX6 series SoCs which
> > have cortex-a9 ARM core, but i.MX7D has 2 cortex-a7 ARM core, so we
> > need to add runtime check for those differences between cortex-a9 and
> > cortex-a7.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  arch/arm/mach-imx/headsmp.S    | 11 +++++++++++
> >  arch/arm/mach-imx/mach-imx7d.c |  2 ++
> >  arch/arm/mach-imx/platsmp.c    | 19 ++++++++++++++++++-
> >  arch/arm/mach-imx/src.c        | 38 ++++++++++++++++++++++++++++++----
> ----
> >  4 files changed, 61 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-
> imx/headsmp.S
> > index 6c28d28..a26e459 100644
> > --- a/arch/arm/mach-imx/headsmp.S
> > +++ b/arch/arm/mach-imx/headsmp.S
> > @@ -26,7 +26,18 @@ diag_reg_offset:
> >  	.endm
> >
> >  ENTRY(v7_secondary_startup)
> > +	.word	0xc070			@ 0xc07 is cortex-a7 id
> > +	.word	0xfff0			@ mask for core type
> > +
> >  ARM_BE8(setend be)			@ go BE8 if entered LE
> > +	mrc     p15, 0, r0, c0, c0, 0
> > +	adr	r1, v7_secondary_startup
> > +	ldr	r2, [r1]
> > +	ldr	r3, [r1, #0x4]
> > +	and     r0, r0, r3
> > +	cmp     r0, r2
> > +	beq     secondary_startup
> > +
> 
> Total NAK on the above.  There's no way that we want to even try executing
> those two .word values (and I don't care if "it works when tested", we just
> don't try and execute data.)

Sure, I misunderstood the way of putting .word, actually, for i.MX7D, I can
just call common secondary_startup instead of v7_seondary_startup, so I can
remove all changes in this file.

> 
> >  	set_diag_reg
> >  	b	secondary_startup
> >  ENDPROC(v7_secondary_startup)
> > diff --git a/arch/arm/mach-imx/mach-imx7d.c
> > b/arch/arm/mach-imx/mach-imx7d.c index 26ca744..ef3dce6 100644
> > --- a/arch/arm/mach-imx/mach-imx7d.c
> > +++ b/arch/arm/mach-imx/mach-imx7d.c
> > @@ -99,6 +99,7 @@ static void __init imx7d_init_machine(void)
> >
> >  static void __init imx7d_init_irq(void)  {
> > +	imx_gpcv2_check_dt();
> >  	imx_init_revision_from_anatop();
> >  	imx_src_init();
> >  	irqchip_init();
> > @@ -111,6 +112,7 @@ static const char *const imx7d_dt_compat[]
> > __initconst = {  };
> >
> >  DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
> > +	.smp		= smp_ops(imx_smp_ops),
> >  	.init_irq	= imx7d_init_irq,
> >  	.init_machine	= imx7d_init_machine,
> >  	.dt_compat	= imx7d_dt_compat,
> > diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
> > index 711dbbd..63af911 100644
> > --- a/arch/arm/mach-imx/platsmp.c
> > +++ b/arch/arm/mach-imx/platsmp.c
> > @@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu,
> > struct task_struct *idle)  static void __init imx_smp_init_cpus(void)
> > {
> >  	int i, ncores;
> > +	unsigned long val, arch_type;
> >
> > -	ncores = scu_get_core_count(scu_base);
> > +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> > +
> > +	if (((arch_type >> 4) & 0xfff) == 0xc07) {
> 
> This is buggy.  Plus, we have macros for this.  Please use the macros in
> asm/cputype.h to achieve these tests.

Thanks, I will use read_cpuid_id() API intead of putting asm code here.

> 
> > +		/* cortex-a7 core number is in bit[25:24] of CP15 L2CTLR */
> > +		asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
> > +		ncores = ((val >> 24) & 0x3) + 1;
> > +	} else {
> > +		ncores = scu_get_core_count(scu_base);
> > +	}
> >
> >  	for (i = ncores; i < NR_CPUS; i++)
> >  		set_cpu_possible(i, false);
> > @@ -74,6 +83,14 @@ void imx_smp_prepare(void)
> >
> >  static void __init imx_smp_prepare_cpus(unsigned int max_cpus)  {
> > +	unsigned long arch_type;
> > +
> > +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> > +
> > +	/* no need for cortex-a7 */
> > +	if (((arch_type >> 4) & 0xfff) == 0xc07)
> 
> Ditto.

Ditto.

> 
> > +		return;
> > +
> >  	imx_smp_prepare();
> >
> >  	/*
> > diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c index
> > 70b083f..1fda72a 100644
> > --- a/arch/arm/mach-imx/src.c
> > +++ b/arch/arm/mach-imx/src.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/smp.h>
> >  #include <asm/smp_plat.h>
> >  #include "common.h"
> > +#include "hardware.h"
> >
> >  #define SRC_SCR				0x000
> >  #define SRC_GPR1			0x020
> > @@ -30,6 +31,15 @@
> >  #define BP_SRC_SCR_CORE1_RST		14
> >  #define BP_SRC_SCR_CORE1_ENABLE		22
> >
> > +/* below are for i.MX7D */
> > +#define SRC_GPR1_V2                     0x074
> > +#define SRC_A7RCR0                      0x004
> > +#define SRC_A7RCR1                      0x008
> > +#define SRC_M4RCR                       0x00C
> > +
> > +#define BP_SRC_A7RCR0_A7_CORE_RESET0   0
> > +#define BP_SRC_A7RCR1_A7_CORE1_ENABLE  1
> > +
> >  static void __iomem *src_base;
> >  static DEFINE_SPINLOCK(scr_lock);
> >
> > @@ -87,12 +97,21 @@ void imx_enable_cpu(int cpu, bool enable)
> >  	u32 mask, val;
> >
> >  	cpu = cpu_logical_map(cpu);
> > -	mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
> >  	spin_lock(&scr_lock);
> > -	val = readl_relaxed(src_base + SRC_SCR);
> > -	val = enable ? val | mask : val & ~mask;
> > -	val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
> > -	writel_relaxed(val, src_base + SRC_SCR);
> > +	if (cpu_is_imx7d()) {
> 
> It's about time iMX folk learned that "imx*" is a SoC and _not_ a CPU.
> It should be "soc_is_imx7d()" because we're wanting to know whether the
> _SoC_ is an iMX7D.  The _CPU_ is a _Cortex-A7_.

Agree, I will add a new patch in this patch set to replace all the cpu_is_xxx with
Soc_is_xxx.

Thanks for review.
Anson.

> 
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

RE: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@armlinux.org.uk]
> Sent: 2016-08-26 7:14 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; shawnguo@kernel.org; kernel@pengutronix.de;
> Fabio Estevam <fabio.estevam@nxp.com>; robh+dt@kernel.org;
> mark.rutland@arm.com
> Subject: Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D
> 
> On Fri, Aug 26, 2016 at 07:12:51PM +0800, Anson Huang wrote:
> > i.MX7D has 2 cortex-a7 ARM core, add support for booting up SMP kernel
> > with 2 CPUs.
> >
> > The existing i.MX SMP code is designed for i.MX6 series SoCs which
> > have cortex-a9 ARM core, but i.MX7D has 2 cortex-a7 ARM core, so we
> > need to add runtime check for those differences between cortex-a9 and
> > cortex-a7.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  arch/arm/mach-imx/headsmp.S    | 11 +++++++++++
> >  arch/arm/mach-imx/mach-imx7d.c |  2 ++
> >  arch/arm/mach-imx/platsmp.c    | 19 ++++++++++++++++++-
> >  arch/arm/mach-imx/src.c        | 38 ++++++++++++++++++++++++++++++----
> ----
> >  4 files changed, 61 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-
> imx/headsmp.S
> > index 6c28d28..a26e459 100644
> > --- a/arch/arm/mach-imx/headsmp.S
> > +++ b/arch/arm/mach-imx/headsmp.S
> > @@ -26,7 +26,18 @@ diag_reg_offset:
> >  	.endm
> >
> >  ENTRY(v7_secondary_startup)
> > +	.word	0xc070			@ 0xc07 is cortex-a7 id
> > +	.word	0xfff0			@ mask for core type
> > +
> >  ARM_BE8(setend be)			@ go BE8 if entered LE
> > +	mrc     p15, 0, r0, c0, c0, 0
> > +	adr	r1, v7_secondary_startup
> > +	ldr	r2, [r1]
> > +	ldr	r3, [r1, #0x4]
> > +	and     r0, r0, r3
> > +	cmp     r0, r2
> > +	beq     secondary_startup
> > +
> 
> Total NAK on the above.  There's no way that we want to even try executing
> those two .word values (and I don't care if "it works when tested", we just
> don't try and execute data.)

Sure, I misunderstood the way of putting .word, actually, for i.MX7D, I can
just call common secondary_startup instead of v7_seondary_startup, so I can
remove all changes in this file.

> 
> >  	set_diag_reg
> >  	b	secondary_startup
> >  ENDPROC(v7_secondary_startup)
> > diff --git a/arch/arm/mach-imx/mach-imx7d.c
> > b/arch/arm/mach-imx/mach-imx7d.c index 26ca744..ef3dce6 100644
> > --- a/arch/arm/mach-imx/mach-imx7d.c
> > +++ b/arch/arm/mach-imx/mach-imx7d.c
> > @@ -99,6 +99,7 @@ static void __init imx7d_init_machine(void)
> >
> >  static void __init imx7d_init_irq(void)  {
> > +	imx_gpcv2_check_dt();
> >  	imx_init_revision_from_anatop();
> >  	imx_src_init();
> >  	irqchip_init();
> > @@ -111,6 +112,7 @@ static const char *const imx7d_dt_compat[]
> > __initconst = {  };
> >
> >  DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
> > +	.smp		= smp_ops(imx_smp_ops),
> >  	.init_irq	= imx7d_init_irq,
> >  	.init_machine	= imx7d_init_machine,
> >  	.dt_compat	= imx7d_dt_compat,
> > diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
> > index 711dbbd..63af911 100644
> > --- a/arch/arm/mach-imx/platsmp.c
> > +++ b/arch/arm/mach-imx/platsmp.c
> > @@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu,
> > struct task_struct *idle)  static void __init imx_smp_init_cpus(void)
> > {
> >  	int i, ncores;
> > +	unsigned long val, arch_type;
> >
> > -	ncores = scu_get_core_count(scu_base);
> > +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> > +
> > +	if (((arch_type >> 4) & 0xfff) == 0xc07) {
> 
> This is buggy.  Plus, we have macros for this.  Please use the macros in
> asm/cputype.h to achieve these tests.

Thanks, I will use read_cpuid_id() API intead of putting asm code here.

> 
> > +		/* cortex-a7 core number is in bit[25:24] of CP15 L2CTLR */
> > +		asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
> > +		ncores = ((val >> 24) & 0x3) + 1;
> > +	} else {
> > +		ncores = scu_get_core_count(scu_base);
> > +	}
> >
> >  	for (i = ncores; i < NR_CPUS; i++)
> >  		set_cpu_possible(i, false);
> > @@ -74,6 +83,14 @@ void imx_smp_prepare(void)
> >
> >  static void __init imx_smp_prepare_cpus(unsigned int max_cpus)  {
> > +	unsigned long arch_type;
> > +
> > +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> > +
> > +	/* no need for cortex-a7 */
> > +	if (((arch_type >> 4) & 0xfff) == 0xc07)
> 
> Ditto.

Ditto.

> 
> > +		return;
> > +
> >  	imx_smp_prepare();
> >
> >  	/*
> > diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c index
> > 70b083f..1fda72a 100644
> > --- a/arch/arm/mach-imx/src.c
> > +++ b/arch/arm/mach-imx/src.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/smp.h>
> >  #include <asm/smp_plat.h>
> >  #include "common.h"
> > +#include "hardware.h"
> >
> >  #define SRC_SCR				0x000
> >  #define SRC_GPR1			0x020
> > @@ -30,6 +31,15 @@
> >  #define BP_SRC_SCR_CORE1_RST		14
> >  #define BP_SRC_SCR_CORE1_ENABLE		22
> >
> > +/* below are for i.MX7D */
> > +#define SRC_GPR1_V2                     0x074
> > +#define SRC_A7RCR0                      0x004
> > +#define SRC_A7RCR1                      0x008
> > +#define SRC_M4RCR                       0x00C
> > +
> > +#define BP_SRC_A7RCR0_A7_CORE_RESET0   0
> > +#define BP_SRC_A7RCR1_A7_CORE1_ENABLE  1
> > +
> >  static void __iomem *src_base;
> >  static DEFINE_SPINLOCK(scr_lock);
> >
> > @@ -87,12 +97,21 @@ void imx_enable_cpu(int cpu, bool enable)
> >  	u32 mask, val;
> >
> >  	cpu = cpu_logical_map(cpu);
> > -	mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
> >  	spin_lock(&scr_lock);
> > -	val = readl_relaxed(src_base + SRC_SCR);
> > -	val = enable ? val | mask : val & ~mask;
> > -	val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
> > -	writel_relaxed(val, src_base + SRC_SCR);
> > +	if (cpu_is_imx7d()) {
> 
> It's about time iMX folk learned that "imx*" is a SoC and _not_ a CPU.
> It should be "soc_is_imx7d()" because we're wanting to know whether the
> _SoC_ is an iMX7D.  The _CPU_ is a _Cortex-A7_.

Agree, I will add a new patch in this patch set to replace all the cpu_is_xxx with
Soc_is_xxx.

Thanks for review.
Anson.

> 
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

[PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at armlinux.org.uk]
> Sent: 2016-08-26 7:14 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; devicetree at vger.kernel.org; linux-
> kernel at vger.kernel.org; shawnguo at kernel.org; kernel at pengutronix.de;
> Fabio Estevam <fabio.estevam@nxp.com>; robh+dt at kernel.org;
> mark.rutland at arm.com
> Subject: Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D
> 
> On Fri, Aug 26, 2016 at 07:12:51PM +0800, Anson Huang wrote:
> > i.MX7D has 2 cortex-a7 ARM core, add support for booting up SMP kernel
> > with 2 CPUs.
> >
> > The existing i.MX SMP code is designed for i.MX6 series SoCs which
> > have cortex-a9 ARM core, but i.MX7D has 2 cortex-a7 ARM core, so we
> > need to add runtime check for those differences between cortex-a9 and
> > cortex-a7.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  arch/arm/mach-imx/headsmp.S    | 11 +++++++++++
> >  arch/arm/mach-imx/mach-imx7d.c |  2 ++
> >  arch/arm/mach-imx/platsmp.c    | 19 ++++++++++++++++++-
> >  arch/arm/mach-imx/src.c        | 38 ++++++++++++++++++++++++++++++----
> ----
> >  4 files changed, 61 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-
> imx/headsmp.S
> > index 6c28d28..a26e459 100644
> > --- a/arch/arm/mach-imx/headsmp.S
> > +++ b/arch/arm/mach-imx/headsmp.S
> > @@ -26,7 +26,18 @@ diag_reg_offset:
> >  	.endm
> >
> >  ENTRY(v7_secondary_startup)
> > +	.word	0xc070			@ 0xc07 is cortex-a7 id
> > +	.word	0xfff0			@ mask for core type
> > +
> >  ARM_BE8(setend be)			@ go BE8 if entered LE
> > +	mrc     p15, 0, r0, c0, c0, 0
> > +	adr	r1, v7_secondary_startup
> > +	ldr	r2, [r1]
> > +	ldr	r3, [r1, #0x4]
> > +	and     r0, r0, r3
> > +	cmp     r0, r2
> > +	beq     secondary_startup
> > +
> 
> Total NAK on the above.  There's no way that we want to even try executing
> those two .word values (and I don't care if "it works when tested", we just
> don't try and execute data.)

Sure, I misunderstood the way of putting .word, actually, for i.MX7D, I can
just call common secondary_startup instead of v7_seondary_startup, so I can
remove all changes in this file.

> 
> >  	set_diag_reg
> >  	b	secondary_startup
> >  ENDPROC(v7_secondary_startup)
> > diff --git a/arch/arm/mach-imx/mach-imx7d.c
> > b/arch/arm/mach-imx/mach-imx7d.c index 26ca744..ef3dce6 100644
> > --- a/arch/arm/mach-imx/mach-imx7d.c
> > +++ b/arch/arm/mach-imx/mach-imx7d.c
> > @@ -99,6 +99,7 @@ static void __init imx7d_init_machine(void)
> >
> >  static void __init imx7d_init_irq(void)  {
> > +	imx_gpcv2_check_dt();
> >  	imx_init_revision_from_anatop();
> >  	imx_src_init();
> >  	irqchip_init();
> > @@ -111,6 +112,7 @@ static const char *const imx7d_dt_compat[]
> > __initconst = {  };
> >
> >  DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
> > +	.smp		= smp_ops(imx_smp_ops),
> >  	.init_irq	= imx7d_init_irq,
> >  	.init_machine	= imx7d_init_machine,
> >  	.dt_compat	= imx7d_dt_compat,
> > diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
> > index 711dbbd..63af911 100644
> > --- a/arch/arm/mach-imx/platsmp.c
> > +++ b/arch/arm/mach-imx/platsmp.c
> > @@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu,
> > struct task_struct *idle)  static void __init imx_smp_init_cpus(void)
> > {
> >  	int i, ncores;
> > +	unsigned long val, arch_type;
> >
> > -	ncores = scu_get_core_count(scu_base);
> > +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> > +
> > +	if (((arch_type >> 4) & 0xfff) == 0xc07) {
> 
> This is buggy.  Plus, we have macros for this.  Please use the macros in
> asm/cputype.h to achieve these tests.

Thanks, I will use read_cpuid_id() API intead of putting asm code here.

> 
> > +		/* cortex-a7 core number is in bit[25:24] of CP15 L2CTLR */
> > +		asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
> > +		ncores = ((val >> 24) & 0x3) + 1;
> > +	} else {
> > +		ncores = scu_get_core_count(scu_base);
> > +	}
> >
> >  	for (i = ncores; i < NR_CPUS; i++)
> >  		set_cpu_possible(i, false);
> > @@ -74,6 +83,14 @@ void imx_smp_prepare(void)
> >
> >  static void __init imx_smp_prepare_cpus(unsigned int max_cpus)  {
> > +	unsigned long arch_type;
> > +
> > +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> > +
> > +	/* no need for cortex-a7 */
> > +	if (((arch_type >> 4) & 0xfff) == 0xc07)
> 
> Ditto.

Ditto.

> 
> > +		return;
> > +
> >  	imx_smp_prepare();
> >
> >  	/*
> > diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c index
> > 70b083f..1fda72a 100644
> > --- a/arch/arm/mach-imx/src.c
> > +++ b/arch/arm/mach-imx/src.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/smp.h>
> >  #include <asm/smp_plat.h>
> >  #include "common.h"
> > +#include "hardware.h"
> >
> >  #define SRC_SCR				0x000
> >  #define SRC_GPR1			0x020
> > @@ -30,6 +31,15 @@
> >  #define BP_SRC_SCR_CORE1_RST		14
> >  #define BP_SRC_SCR_CORE1_ENABLE		22
> >
> > +/* below are for i.MX7D */
> > +#define SRC_GPR1_V2                     0x074
> > +#define SRC_A7RCR0                      0x004
> > +#define SRC_A7RCR1                      0x008
> > +#define SRC_M4RCR                       0x00C
> > +
> > +#define BP_SRC_A7RCR0_A7_CORE_RESET0   0
> > +#define BP_SRC_A7RCR1_A7_CORE1_ENABLE  1
> > +
> >  static void __iomem *src_base;
> >  static DEFINE_SPINLOCK(scr_lock);
> >
> > @@ -87,12 +97,21 @@ void imx_enable_cpu(int cpu, bool enable)
> >  	u32 mask, val;
> >
> >  	cpu = cpu_logical_map(cpu);
> > -	mask = 1 << (BP_SRC_SCR_CORE1_ENABLE + cpu - 1);
> >  	spin_lock(&scr_lock);
> > -	val = readl_relaxed(src_base + SRC_SCR);
> > -	val = enable ? val | mask : val & ~mask;
> > -	val |= 1 << (BP_SRC_SCR_CORE1_RST + cpu - 1);
> > -	writel_relaxed(val, src_base + SRC_SCR);
> > +	if (cpu_is_imx7d()) {
> 
> It's about time iMX folk learned that "imx*" is a SoC and _not_ a CPU.
> It should be "soc_is_imx7d()" because we're wanting to know whether the
> _SoC_ is an iMX7D.  The _CPU_ is a _Cortex-A7_.

Agree, I will add a new patch in this patch set to replace all the cpu_is_xxx with
Soc_is_xxx.

Thanks for review.
Anson.

> 
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

RE: [PATCH 2/3] ARM: imx: add gpcv2 support

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@armlinux.org.uk]
> Sent: 2016-08-26 7:18 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; shawnguo@kernel.org; kernel@pengutronix.de;
> Fabio Estevam <fabio.estevam@nxp.com>; robh+dt@kernel.org;
> mark.rutland@arm.com
> Subject: Re: [PATCH 2/3] ARM: imx: add gpcv2 support
> 
> On Fri, Aug 26, 2016 at 07:12:50PM +0800, Anson Huang wrote:
> > diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c new
> > file mode 100644 index 0000000..98577c4
> > --- /dev/null
> > +++ b/arch/arm/mach-imx/gpcv2.c
> > @@ -0,0 +1,66 @@
> > +/*
> > + * Copyright 2016 Freescale Semiconductor, Inc.
> > + *
> > + * The code contained herein is licensed under the GNU General Public
> > + * License. You may obtain a copy of the GNU General Public License
> > + * Version 2 or later at the following locations:
> > + *
> > + * http://www.opensource.org/licenses/gpl-license.html
> > + * http://www.gnu.org/copyleft/gpl.html
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +
> > +#include "common.h"
> > +
> > +#define GPC_CPU_PGC_SW_PUP_REQ	0xf0
> > +#define GPC_CPU_PGC_SW_PDN_REQ	0xfc
> > +#define GPC_PGC_C1		0x840
> > +
> > +#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7	0x2
> > +#define BM_GPC_PGC_PCG				0x1
> > +
> > +static void __iomem *gpcv2_base;
> > +
> > +static void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset) {
> > +	u32 val = readl_relaxed(gpcv2_base + offset) & (~BM_GPC_PGC_PCG);
> 
> Unnecessary parens, and the code doesn't flow with the bit clearance here...
> 
> > +
> > +	if (enable)
> > +		val |= BM_GPC_PGC_PCG;
> 
> My first read of this lead me to think "okay, so what's the point of
> enable=false".  It would be clearer with:
> 
> 	u32 val = readl_relaxed(gpcv2_base + offset);
> 
> 	if (enable)
> 		val |= BM_GPC_PGC_PCG;
> 	else
> 		val &= ~BM_GPC_PGC_PCG;
> 
> here.

Agree, will improve it in V2 patch.

> 
> > +
> > +	writel_relaxed(val, gpcv2_base + offset); }
> > +
> > +void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn) {
> > +	u32 val = readl_relaxed(gpcv2_base + (pdn ?
> > +		GPC_CPU_PGC_SW_PDN_REQ :
> GPC_CPU_PGC_SW_PUP_REQ));
> > +
> > +	imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1);
> > +	val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
> > +	writel_relaxed(val, gpcv2_base + (pdn ?
> > +		GPC_CPU_PGC_SW_PDN_REQ :
> GPC_CPU_PGC_SW_PUP_REQ));
> > +
> > +	while ((readl_relaxed(gpcv2_base + (pdn ?
> > +		GPC_CPU_PGC_SW_PDN_REQ :
> GPC_CPU_PGC_SW_PUP_REQ)) &
> > +		BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
> > +		;
> > +	imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1); }
> > +
> > +void __init imx_gpcv2_check_dt(void)
> > +{
> > +	struct device_node *np;
> > +
> > +	np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-gpc");
> > +	if (WARN_ON(!np))
> > +		return;
> > +
> > +	gpcv2_base = of_iomap(np, 0);
> > +	WARN_ON(!gpcv2_base);
> 
> What happens if gpcv2_base is NULL (apart from the obvious warning from the
> above line)?  I guess a bit later in the boot,
> imx_gpcv2_set_core1_pdn_pup_by_software() oopses the kernel, probably
> before the console has been initialised.  Probably not nice behaviour.
>

Yes, normal console is NOT ready at this stage, unless early console is enabled.

Could you please advise how to handle such case if gpcv2_base is NULL and console is NOT
ready? Checking gpcv2_base everywhere before using it? Normally gpcv2_base
should NOT be NULL. 

Thanks.
Anson
 
> > +}
> > --
> > 1.9.1
> >
> 
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

RE: [PATCH 2/3] ARM: imx: add gpcv2 support

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org]
> Sent: 2016-08-26 7:18 PM
> To: Yongcai Huang <anson.huang-3arQi8VN3Tc@public.gmane.org>
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org;
> Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>; robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org;
> mark.rutland-5wv7dgnIgG8@public.gmane.org
> Subject: Re: [PATCH 2/3] ARM: imx: add gpcv2 support
> 
> On Fri, Aug 26, 2016 at 07:12:50PM +0800, Anson Huang wrote:
> > diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c new
> > file mode 100644 index 0000000..98577c4
> > --- /dev/null
> > +++ b/arch/arm/mach-imx/gpcv2.c
> > @@ -0,0 +1,66 @@
> > +/*
> > + * Copyright 2016 Freescale Semiconductor, Inc.
> > + *
> > + * The code contained herein is licensed under the GNU General Public
> > + * License. You may obtain a copy of the GNU General Public License
> > + * Version 2 or later at the following locations:
> > + *
> > + * http://www.opensource.org/licenses/gpl-license.html
> > + * http://www.gnu.org/copyleft/gpl.html
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +
> > +#include "common.h"
> > +
> > +#define GPC_CPU_PGC_SW_PUP_REQ	0xf0
> > +#define GPC_CPU_PGC_SW_PDN_REQ	0xfc
> > +#define GPC_PGC_C1		0x840
> > +
> > +#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7	0x2
> > +#define BM_GPC_PGC_PCG				0x1
> > +
> > +static void __iomem *gpcv2_base;
> > +
> > +static void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset) {
> > +	u32 val = readl_relaxed(gpcv2_base + offset) & (~BM_GPC_PGC_PCG);
> 
> Unnecessary parens, and the code doesn't flow with the bit clearance here...
> 
> > +
> > +	if (enable)
> > +		val |= BM_GPC_PGC_PCG;
> 
> My first read of this lead me to think "okay, so what's the point of
> enable=false".  It would be clearer with:
> 
> 	u32 val = readl_relaxed(gpcv2_base + offset);
> 
> 	if (enable)
> 		val |= BM_GPC_PGC_PCG;
> 	else
> 		val &= ~BM_GPC_PGC_PCG;
> 
> here.

Agree, will improve it in V2 patch.

> 
> > +
> > +	writel_relaxed(val, gpcv2_base + offset); }
> > +
> > +void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn) {
> > +	u32 val = readl_relaxed(gpcv2_base + (pdn ?
> > +		GPC_CPU_PGC_SW_PDN_REQ :
> GPC_CPU_PGC_SW_PUP_REQ));
> > +
> > +	imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1);
> > +	val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
> > +	writel_relaxed(val, gpcv2_base + (pdn ?
> > +		GPC_CPU_PGC_SW_PDN_REQ :
> GPC_CPU_PGC_SW_PUP_REQ));
> > +
> > +	while ((readl_relaxed(gpcv2_base + (pdn ?
> > +		GPC_CPU_PGC_SW_PDN_REQ :
> GPC_CPU_PGC_SW_PUP_REQ)) &
> > +		BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
> > +		;
> > +	imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1); }
> > +
> > +void __init imx_gpcv2_check_dt(void)
> > +{
> > +	struct device_node *np;
> > +
> > +	np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-gpc");
> > +	if (WARN_ON(!np))
> > +		return;
> > +
> > +	gpcv2_base = of_iomap(np, 0);
> > +	WARN_ON(!gpcv2_base);
> 
> What happens if gpcv2_base is NULL (apart from the obvious warning from the
> above line)?  I guess a bit later in the boot,
> imx_gpcv2_set_core1_pdn_pup_by_software() oopses the kernel, probably
> before the console has been initialised.  Probably not nice behaviour.
>

Yes, normal console is NOT ready at this stage, unless early console is enabled.

Could you please advise how to handle such case if gpcv2_base is NULL and console is NOT
ready? Checking gpcv2_base everywhere before using it? Normally gpcv2_base
should NOT be NULL. 

Thanks.
Anson
 
> > +}
> > --
> > 1.9.1
> >
> 
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] ARM: imx: add gpcv2 support

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at armlinux.org.uk]
> Sent: 2016-08-26 7:18 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; devicetree at vger.kernel.org; linux-
> kernel at vger.kernel.org; shawnguo at kernel.org; kernel at pengutronix.de;
> Fabio Estevam <fabio.estevam@nxp.com>; robh+dt at kernel.org;
> mark.rutland at arm.com
> Subject: Re: [PATCH 2/3] ARM: imx: add gpcv2 support
> 
> On Fri, Aug 26, 2016 at 07:12:50PM +0800, Anson Huang wrote:
> > diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c new
> > file mode 100644 index 0000000..98577c4
> > --- /dev/null
> > +++ b/arch/arm/mach-imx/gpcv2.c
> > @@ -0,0 +1,66 @@
> > +/*
> > + * Copyright 2016 Freescale Semiconductor, Inc.
> > + *
> > + * The code contained herein is licensed under the GNU General Public
> > + * License. You may obtain a copy of the GNU General Public License
> > + * Version 2 or later at the following locations:
> > + *
> > + * http://www.opensource.org/licenses/gpl-license.html
> > + * http://www.gnu.org/copyleft/gpl.html
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +
> > +#include "common.h"
> > +
> > +#define GPC_CPU_PGC_SW_PUP_REQ	0xf0
> > +#define GPC_CPU_PGC_SW_PDN_REQ	0xfc
> > +#define GPC_PGC_C1		0x840
> > +
> > +#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7	0x2
> > +#define BM_GPC_PGC_PCG				0x1
> > +
> > +static void __iomem *gpcv2_base;
> > +
> > +static void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset) {
> > +	u32 val = readl_relaxed(gpcv2_base + offset) & (~BM_GPC_PGC_PCG);
> 
> Unnecessary parens, and the code doesn't flow with the bit clearance here...
> 
> > +
> > +	if (enable)
> > +		val |= BM_GPC_PGC_PCG;
> 
> My first read of this lead me to think "okay, so what's the point of
> enable=false".  It would be clearer with:
> 
> 	u32 val = readl_relaxed(gpcv2_base + offset);
> 
> 	if (enable)
> 		val |= BM_GPC_PGC_PCG;
> 	else
> 		val &= ~BM_GPC_PGC_PCG;
> 
> here.

Agree, will improve it in V2 patch.

> 
> > +
> > +	writel_relaxed(val, gpcv2_base + offset); }
> > +
> > +void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn) {
> > +	u32 val = readl_relaxed(gpcv2_base + (pdn ?
> > +		GPC_CPU_PGC_SW_PDN_REQ :
> GPC_CPU_PGC_SW_PUP_REQ));
> > +
> > +	imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1);
> > +	val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
> > +	writel_relaxed(val, gpcv2_base + (pdn ?
> > +		GPC_CPU_PGC_SW_PDN_REQ :
> GPC_CPU_PGC_SW_PUP_REQ));
> > +
> > +	while ((readl_relaxed(gpcv2_base + (pdn ?
> > +		GPC_CPU_PGC_SW_PDN_REQ :
> GPC_CPU_PGC_SW_PUP_REQ)) &
> > +		BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
> > +		;
> > +	imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1); }
> > +
> > +void __init imx_gpcv2_check_dt(void)
> > +{
> > +	struct device_node *np;
> > +
> > +	np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-gpc");
> > +	if (WARN_ON(!np))
> > +		return;
> > +
> > +	gpcv2_base = of_iomap(np, 0);
> > +	WARN_ON(!gpcv2_base);
> 
> What happens if gpcv2_base is NULL (apart from the obvious warning from the
> above line)?  I guess a bit later in the boot,
> imx_gpcv2_set_core1_pdn_pup_by_software() oopses the kernel, probably
> before the console has been initialised.  Probably not nice behaviour.
>

Yes, normal console is NOT ready at this stage, unless early console is enabled.

Could you please advise how to handle such case if gpcv2_base is NULL and console is NOT
ready? Checking gpcv2_base everywhere before using it? Normally gpcv2_base
should NOT be NULL. 

Thanks.
Anson
 
> > +}
> > --
> > 1.9.1
> >
> 
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 12:20:15PM +0000, Yongcai Huang wrote:
> > On Fri, Aug 26, 2016 at 07:12:51PM +0800, Anson Huang wrote:
> > > diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
> > > index 711dbbd..63af911 100644
> > > --- a/arch/arm/mach-imx/platsmp.c
> > > +++ b/arch/arm/mach-imx/platsmp.c
> > > @@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu,
> > > struct task_struct *idle)  static void __init imx_smp_init_cpus(void)
> > > {
> > >  	int i, ncores;
> > > +	unsigned long val, arch_type;
> > >
> > > -	ncores = scu_get_core_count(scu_base);
> > > +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> > > +
> > > +	if (((arch_type >> 4) & 0xfff) == 0xc07) {
> > 
> > This is buggy.  Plus, we have macros for this.  Please use the macros in
> > asm/cputype.h to achieve these tests.
> 
> Thanks, I will use read_cpuid_id() API intead of putting asm code here.

Nope.  Do this:

	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A7) {

Because the part number field is defined by the implementer, so before
you can interpret the part number field, you have to first check who
has implemented the device.

That's exactly why these macros exist - to help you get these tests
correct, so when someone else releases an ARM core with the same value
in the part number field, we don't end up with a load of tests which
incorrectly matching.

While it may be unlikely that an iMX7 would end up with a non-ARM CPU,
that's no real excuse for avoiding using what's been provided for you,
especially when avoiding it causes issues for static analysis.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 12:20:15PM +0000, Yongcai Huang wrote:
> > On Fri, Aug 26, 2016 at 07:12:51PM +0800, Anson Huang wrote:
> > > diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
> > > index 711dbbd..63af911 100644
> > > --- a/arch/arm/mach-imx/platsmp.c
> > > +++ b/arch/arm/mach-imx/platsmp.c
> > > @@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu,
> > > struct task_struct *idle)  static void __init imx_smp_init_cpus(void)
> > > {
> > >  	int i, ncores;
> > > +	unsigned long val, arch_type;
> > >
> > > -	ncores = scu_get_core_count(scu_base);
> > > +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> > > +
> > > +	if (((arch_type >> 4) & 0xfff) == 0xc07) {
> > 
> > This is buggy.  Plus, we have macros for this.  Please use the macros in
> > asm/cputype.h to achieve these tests.
> 
> Thanks, I will use read_cpuid_id() API intead of putting asm code here.

Nope.  Do this:

	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A7) {

Because the part number field is defined by the implementer, so before
you can interpret the part number field, you have to first check who
has implemented the device.

That's exactly why these macros exist - to help you get these tests
correct, so when someone else releases an ARM core with the same value
in the part number field, we don't end up with a load of tests which
incorrectly matching.

While it may be unlikely that an iMX7 would end up with a non-ARM CPU,
that's no real excuse for avoiding using what's been provided for you,
especially when avoiding it causes issues for static analysis.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Russell King - ARM Linux]

On Fri, Aug 26, 2016 at 12:20:15PM +0000, Yongcai Huang wrote:
> > On Fri, Aug 26, 2016 at 07:12:51PM +0800, Anson Huang wrote:
> > > diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
> > > index 711dbbd..63af911 100644
> > > --- a/arch/arm/mach-imx/platsmp.c
> > > +++ b/arch/arm/mach-imx/platsmp.c
> > > @@ -60,8 +60,17 @@ static int imx_boot_secondary(unsigned int cpu,
> > > struct task_struct *idle)  static void __init imx_smp_init_cpus(void)
> > > {
> > >  	int i, ncores;
> > > +	unsigned long val, arch_type;
> > >
> > > -	ncores = scu_get_core_count(scu_base);
> > > +	asm volatile("mrc p15, 0, %0, c0, c0, 0" : "=r" (arch_type));
> > > +
> > > +	if (((arch_type >> 4) & 0xfff) == 0xc07) {
> > 
> > This is buggy.  Plus, we have macros for this.  Please use the macros in
> > asm/cputype.h to achieve these tests.
> 
> Thanks, I will use read_cpuid_id() API intead of putting asm code here.

Nope.  Do this:

	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A7) {

Because the part number field is defined by the implementer, so before
you can interpret the part number field, you have to first check who
has implemented the device.

That's exactly why these macros exist - to help you get these tests
correct, so when someone else releases an ARM core with the same value
in the part number field, we don't end up with a load of tests which
incorrectly matching.

While it may be unlikely that an iMX7 would end up with a non-ARM CPU,
that's no real excuse for avoiding using what's been provided for you,
especially when avoiding it causes issues for static analysis.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 0/3] Add SMP support for i.MX7D

[Fabio Estevam]

Hi Anson,

On Fri, Aug 26, 2016 at 8:12 AM, Anson Huang <Anson.Huang@nxp.com> wrote:
> i.MX7D has 2 Cortex-A7 ARM cores, and it has a different GPC design
> than i.MX6, so this patch set adds a new GPCV2 driver for i.MX7D,
> and also adds runtime check in SMP code to support both Cortex-A9
> and Cortex-A7 ARM cores.
>
> With this patch set, i.MX7D can boot up SMP kernel with 2 CPUs.

Currently mx7d can boot the 2 CPUs if you use PSCI in the bootloader.

Give mainline U-Boot a try and it will boot the 2 CPUs via PSCI.

There was an attempt to add gpcv2 support at:
https://patchwork.kernel.org/patch/6919831/

, and Shawn suggested to go to the PSCI approach.

PSCI for mx7 is still missing suspend support though.

Regards,

Fabio Estevam

Re: [PATCH 0/3] Add SMP support for i.MX7D

[Fabio Estevam]

Hi Anson,

On Fri, Aug 26, 2016 at 8:12 AM, Anson Huang <Anson.Huang-3arQi8VN3Tc@public.gmane.org> wrote:
> i.MX7D has 2 Cortex-A7 ARM cores, and it has a different GPC design
> than i.MX6, so this patch set adds a new GPCV2 driver for i.MX7D,
> and also adds runtime check in SMP code to support both Cortex-A9
> and Cortex-A7 ARM cores.
>
> With this patch set, i.MX7D can boot up SMP kernel with 2 CPUs.

Currently mx7d can boot the 2 CPUs if you use PSCI in the bootloader.

Give mainline U-Boot a try and it will boot the 2 CPUs via PSCI.

There was an attempt to add gpcv2 support at:
https://patchwork.kernel.org/patch/6919831/

, and Shawn suggested to go to the PSCI approach.

PSCI for mx7 is still missing suspend support though.

Regards,

Fabio Estevam
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] Add SMP support for i.MX7D

[Fabio Estevam]

Hi Anson,

On Fri, Aug 26, 2016 at 8:12 AM, Anson Huang <Anson.Huang@nxp.com> wrote:
> i.MX7D has 2 Cortex-A7 ARM cores, and it has a different GPC design
> than i.MX6, so this patch set adds a new GPCV2 driver for i.MX7D,
> and also adds runtime check in SMP code to support both Cortex-A9
> and Cortex-A7 ARM cores.
>
> With this patch set, i.MX7D can boot up SMP kernel with 2 CPUs.

Currently mx7d can boot the 2 CPUs if you use PSCI in the bootloader.

Give mainline U-Boot a try and it will boot the 2 CPUs via PSCI.

There was an attempt to add gpcv2 support at:
https://patchwork.kernel.org/patch/6919831/

, and Shawn suggested to go to the PSCI approach.

PSCI for mx7 is still missing suspend support though.

Regards,

Fabio Estevam

Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]

Hi Anson,

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on next-20160825]
[cannot apply to v4.8-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Anson-Huang/Add-SMP-support-for-i-MX7D/20160826-140342
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: arm-arm67 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/mach-imx/built-in.o: In function `imx_enable_cpu':
>> arch/arm/mach-imx/src.c:103: undefined reference to `imx_gpcv2_set_core1_pdn_pup_by_software'

vim +103 arch/arm/mach-imx/src.c

    97		u32 mask, val;
    98	
    99		cpu = cpu_logical_map(cpu);
   100		spin_lock(&scr_lock);
   101		if (cpu_is_imx7d()) {
   102			if (enable)
 > 103				imx_gpcv2_set_core1_pdn_pup_by_software(false);
   104			mask = 1 << (BP_SRC_A7RCR1_A7_CORE1_ENABLE + cpu - 1);
   105			val = readl_relaxed(src_base + SRC_A7RCR1);
   106			val = enable ? val | mask : val & ~mask;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 29845 bytes --]

Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]

Hi Anson,

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on next-20160825]
[cannot apply to v4.8-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Anson-Huang/Add-SMP-support-for-i-MX7D/20160826-140342
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: arm-arm67 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/mach-imx/built-in.o: In function `imx_enable_cpu':
>> arch/arm/mach-imx/src.c:103: undefined reference to `imx_gpcv2_set_core1_pdn_pup_by_software'

vim +103 arch/arm/mach-imx/src.c

    97		u32 mask, val;
    98	
    99		cpu = cpu_logical_map(cpu);
   100		spin_lock(&scr_lock);
   101		if (cpu_is_imx7d()) {
   102			if (enable)
 > 103				imx_gpcv2_set_core1_pdn_pup_by_software(false);
   104			mask = 1 << (BP_SRC_A7RCR1_A7_CORE1_ENABLE + cpu - 1);
   105			val = readl_relaxed(src_base + SRC_A7RCR1);
   106			val = enable ? val | mask : val & ~mask;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 29845 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[kbuild test robot]

Hi Anson,

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on next-20160825]
[cannot apply to v4.8-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Anson-Huang/Add-SMP-support-for-i-MX7D/20160826-140342
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: arm-arm67 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/mach-imx/built-in.o: In function `imx_enable_cpu':
>> arch/arm/mach-imx/src.c:103: undefined reference to `imx_gpcv2_set_core1_pdn_pup_by_software'

vim +103 arch/arm/mach-imx/src.c

    97		u32 mask, val;
    98	
    99		cpu = cpu_logical_map(cpu);
   100		spin_lock(&scr_lock);
   101		if (cpu_is_imx7d()) {
   102			if (enable)
 > 103				imx_gpcv2_set_core1_pdn_pup_by_software(false);
   104			mask = 1 << (BP_SRC_A7RCR1_A7_CORE1_ENABLE + cpu - 1);
   105			val = readl_relaxed(src_base + SRC_A7RCR1);
   106			val = enable ? val | mask : val & ~mask;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 29845 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160826/bb7e3530/attachment-0001.obj>

RE: [PATCH 0/3] Add SMP support for i.MX7D

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Fabio Estevam [mailto:festevam@gmail.com]
> Sent: 2016-08-26 8:43 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel <linux-kernel@vger.kernel.org>; Mark Rutland
> <mark.rutland@arm.com>; Russell King - ARM Linux <linux@armlinux.org.uk>;
> robh+dt@kernel.org; Sascha Hauer <kernel@pengutronix.de>; Fabio Estevam
> <fabio.estevam@nxp.com>; Shawn Guo <shawnguo@kernel.org>
> Subject: Re: [PATCH 0/3] Add SMP support for i.MX7D
> 
> Hi Anson,
> 
> On Fri, Aug 26, 2016 at 8:12 AM, Anson Huang <Anson.Huang@nxp.com> wrote:
> > i.MX7D has 2 Cortex-A7 ARM cores, and it has a different GPC design
> > than i.MX6, so this patch set adds a new GPCV2 driver for i.MX7D, and
> > also adds runtime check in SMP code to support both Cortex-A9 and
> > Cortex-A7 ARM cores.
> >
> > With this patch set, i.MX7D can boot up SMP kernel with 2 CPUs.
> 
> Currently mx7d can boot the 2 CPUs if you use PSCI in the bootloader.
> 
> Give mainline U-Boot a try and it will boot the 2 CPUs via PSCI.
> 
> There was an attempt to add gpcv2 support at:
> https://patchwork.kernel.org/patch/6919831/
> 
> , and Shawn suggested to go to the PSCI approach.
> 
> PSCI for mx7 is still missing suspend support though.
> 
> Regards,
> 
> Fabio Estevam

Thanks Fabio for the info, OK, now that i.MX7D goes with PSCI, then please
Ignore this patch set, and I will add suspend support later. Please ignore this
Patch set.

For the suggestion of replace cpu_is_xxx with soc_is_xxx by Russell King, I will
Send another patch to improve that.

Anson.

RE: [PATCH 0/3] Add SMP support for i.MX7D

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Fabio Estevam [mailto:festevam@gmail.com]
> Sent: 2016-08-26 8:43 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel <linux-kernel@vger.kernel.org>; Mark Rutland
> <mark.rutland@arm.com>; Russell King - ARM Linux <linux@armlinux.org.uk>;
> robh+dt@kernel.org; Sascha Hauer <kernel@pengutronix.de>; Fabio Estevam
> <fabio.estevam@nxp.com>; Shawn Guo <shawnguo@kernel.org>
> Subject: Re: [PATCH 0/3] Add SMP support for i.MX7D
> 
> Hi Anson,
> 
> On Fri, Aug 26, 2016 at 8:12 AM, Anson Huang <Anson.Huang@nxp.com> wrote:
> > i.MX7D has 2 Cortex-A7 ARM cores, and it has a different GPC design
> > than i.MX6, so this patch set adds a new GPCV2 driver for i.MX7D, and
> > also adds runtime check in SMP code to support both Cortex-A9 and
> > Cortex-A7 ARM cores.
> >
> > With this patch set, i.MX7D can boot up SMP kernel with 2 CPUs.
> 
> Currently mx7d can boot the 2 CPUs if you use PSCI in the bootloader.
> 
> Give mainline U-Boot a try and it will boot the 2 CPUs via PSCI.
> 
> There was an attempt to add gpcv2 support at:
> https://patchwork.kernel.org/patch/6919831/
> 
> , and Shawn suggested to go to the PSCI approach.
> 
> PSCI for mx7 is still missing suspend support though.
> 
> Regards,
> 
> Fabio Estevam

Thanks Fabio for the info, OK, now that i.MX7D goes with PSCI, then please
Ignore this patch set, and I will add suspend support later. Please ignore this
Patch set.

For the suggestion of replace cpu_is_xxx with soc_is_xxx by Russell King, I will
Send another patch to improve that.

Anson.

[PATCH 0/3] Add SMP support for i.MX7D

[Yongcai Huang]

Best Regards!
Anson Huang



> -----Original Message-----
> From: Fabio Estevam [mailto:festevam at gmail.com]
> Sent: 2016-08-26 8:43 PM
> To: Yongcai Huang <anson.huang@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; devicetree at vger.kernel.org; linux-
> kernel <linux-kernel@vger.kernel.org>; Mark Rutland
> <mark.rutland@arm.com>; Russell King - ARM Linux <linux@armlinux.org.uk>;
> robh+dt at kernel.org; Sascha Hauer <kernel@pengutronix.de>; Fabio Estevam
> <fabio.estevam@nxp.com>; Shawn Guo <shawnguo@kernel.org>
> Subject: Re: [PATCH 0/3] Add SMP support for i.MX7D
> 
> Hi Anson,
> 
> On Fri, Aug 26, 2016 at 8:12 AM, Anson Huang <Anson.Huang@nxp.com> wrote:
> > i.MX7D has 2 Cortex-A7 ARM cores, and it has a different GPC design
> > than i.MX6, so this patch set adds a new GPCV2 driver for i.MX7D, and
> > also adds runtime check in SMP code to support both Cortex-A9 and
> > Cortex-A7 ARM cores.
> >
> > With this patch set, i.MX7D can boot up SMP kernel with 2 CPUs.
> 
> Currently mx7d can boot the 2 CPUs if you use PSCI in the bootloader.
> 
> Give mainline U-Boot a try and it will boot the 2 CPUs via PSCI.
> 
> There was an attempt to add gpcv2 support at:
> https://patchwork.kernel.org/patch/6919831/
> 
> , and Shawn suggested to go to the PSCI approach.
> 
> PSCI for mx7 is still missing suspend support though.
> 
> Regards,
> 
> Fabio Estevam

Thanks Fabio for the info, OK, now that i.MX7D goes with PSCI, then please
Ignore this patch set, and I will add suspend support later. Please ignore this
Patch set.

For the suggestion of replace cpu_is_xxx with soc_is_xxx by Russell King, I will
Send another patch to improve that.

Anson.

Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Arnd Bergmann]

On Friday 26 August 2016, Yongcai Huang wrote:
> > 
> > It's about time iMX folk learned that "imx*" is a SoC and not a CPU.
> > It should be "soc_is_imx7d()" because we're wanting to know whether the
> > SoC is an iMX7D.  The CPU is a _Cortex-A7_.
> 
> Agree, I will add a new patch in this patch set to replace all the cpu_is_xxx with
> Soc_is_xxx.

I recently did a patch to remove all the cpu_is_mx{1,2,3,5}* macros, which ended up
making the code simpler and more readable in most places. I think we can
do the same for the cpu_is_imx{6,7}* macros.

In this instance, the easy alternative is to have a separate smp_operation
structure for imx7d rather than using the same as imx6 and then figuring
out at runtime which one you have.

	Arnd

[PATCH 3/3] ARM: imx: add SMP support for i.MX7D

[Arnd Bergmann]

On Friday 26 August 2016, Yongcai Huang wrote:
> > 
> > It's about time iMX folk learned that "imx*" is a SoC and not a CPU.
> > It should be "soc_is_imx7d()" because we're wanting to know whether the
> > SoC is an iMX7D.  The CPU is a _Cortex-A7_.
> 
> Agree, I will add a new patch in this patch set to replace all the cpu_is_xxx with
> Soc_is_xxx.

I recently did a patch to remove all the cpu_is_mx{1,2,3,5}* macros, which ended up
making the code simpler and more readable in most places. I think we can
do the same for the cpu_is_imx{6,7}* macros.

In this instance, the easy alternative is to have a separate smp_operation
structure for imx7d rather than using the same as imx6 and then figuring
out at runtime which one you have.

	Arnd