* [PATCH 0/3] Add SMP support for i.MX7D
@ 2016-08-26 11:12 Anson Huang
2016-08-26 11:12 ` [PATCH 1/3] ARM: dts: imx7: support SMP boot up Anson Huang
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Anson Huang @ 2016-08-26 11:12 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-kernel
Cc: shawnguo, kernel, fabio.estevam, robh+dt, mark.rutland, linux
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
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/3] ARM: dts: imx7: support SMP boot up 2016-08-26 11:12 [PATCH 0/3] Add SMP support for i.MX7D Anson Huang @ 2016-08-26 11:12 ` Anson Huang 2016-08-26 11:12 ` [PATCH 2/3] ARM: imx: add gpcv2 support Anson Huang ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ messages in thread From: Anson Huang @ 2016-08-26 11:12 UTC (permalink / raw) To: linux-arm-kernel, devicetree, linux-kernel Cc: shawnguo, kernel, fabio.estevam, robh+dt, mark.rutland, linux 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] ARM: imx: add gpcv2 support 2016-08-26 11:12 [PATCH 0/3] Add SMP support for i.MX7D Anson Huang 2016-08-26 11:12 ` [PATCH 1/3] ARM: dts: imx7: support SMP boot up Anson Huang @ 2016-08-26 11:12 ` Anson Huang 2016-08-26 11:17 ` Russell King - ARM Linux 2016-08-26 11:12 ` [PATCH 3/3] ARM: imx: add SMP support for i.MX7D Anson Huang 2016-08-26 12:43 ` [PATCH 0/3] Add " Fabio Estevam 3 siblings, 1 reply; 15+ messages in thread From: Anson Huang @ 2016-08-26 11:12 UTC (permalink / raw) To: linux-arm-kernel, devicetree, linux-kernel Cc: shawnguo, kernel, fabio.estevam, robh+dt, mark.rutland, linux 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] ARM: imx: add gpcv2 support 2016-08-26 11:12 ` [PATCH 2/3] ARM: imx: add gpcv2 support Anson Huang @ 2016-08-26 11:17 ` Russell King - ARM Linux 2016-08-26 12:25 ` Yongcai Huang 0 siblings, 1 reply; 15+ messages in thread From: Russell King - ARM Linux @ 2016-08-26 11:17 UTC (permalink / raw) To: Anson Huang Cc: linux-arm-kernel, devicetree, linux-kernel, shawnguo, kernel, fabio.estevam, robh+dt, mark.rutland 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 2/3] ARM: imx: add gpcv2 support 2016-08-26 11:17 ` Russell King - ARM Linux @ 2016-08-26 12:25 ` Yongcai Huang 0 siblings, 0 replies; 15+ messages in thread From: Yongcai Huang @ 2016-08-26 12:25 UTC (permalink / raw) To: Russell King - ARM Linux Cc: linux-arm-kernel, devicetree, linux-kernel, shawnguo, kernel, Fabio Estevam, robh+dt, mark.rutland 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] ARM: imx: add SMP support for i.MX7D 2016-08-26 11:12 [PATCH 0/3] Add SMP support for i.MX7D Anson Huang 2016-08-26 11:12 ` [PATCH 1/3] ARM: dts: imx7: support SMP boot up Anson Huang 2016-08-26 11:12 ` [PATCH 2/3] ARM: imx: add gpcv2 support Anson Huang @ 2016-08-26 11:12 ` Anson Huang 2016-08-26 8:59 ` Arnd Bergmann ` (2 more replies) 2016-08-26 12:43 ` [PATCH 0/3] Add " Fabio Estevam 3 siblings, 3 replies; 15+ messages in thread From: Anson Huang @ 2016-08-26 11:12 UTC (permalink / raw) To: linux-arm-kernel, devicetree, linux-kernel Cc: shawnguo, kernel, fabio.estevam, robh+dt, mark.rutland, linux 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D 2016-08-26 11:12 ` [PATCH 3/3] ARM: imx: add SMP support for i.MX7D Anson Huang @ 2016-08-26 8:59 ` Arnd Bergmann 2016-08-26 10:28 ` Yongcai Huang 2016-08-26 11:13 ` Russell King - ARM Linux 2016-08-26 13:00 ` kbuild test robot 2 siblings, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2016-08-26 8:59 UTC (permalink / raw) To: Anson Huang Cc: linux-arm-kernel, devicetree, linux-kernel, shawnguo, kernel, fabio.estevam, robh+dt, mark.rutland, linux 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D 2016-08-26 8:59 ` Arnd Bergmann @ 2016-08-26 10:28 ` Yongcai Huang 0 siblings, 0 replies; 15+ messages in thread From: Yongcai Huang @ 2016-08-26 10:28 UTC (permalink / raw) To: Arnd Bergmann Cc: linux-arm-kernel, devicetree, linux-kernel, shawnguo, kernel, Fabio Estevam, robh+dt, mark.rutland, linux 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D 2016-08-26 11:12 ` [PATCH 3/3] ARM: imx: add SMP support for i.MX7D Anson Huang 2016-08-26 8:59 ` Arnd Bergmann @ 2016-08-26 11:13 ` Russell King - ARM Linux 2016-08-26 12:20 ` Yongcai Huang 2016-08-26 13:00 ` kbuild test robot 2 siblings, 1 reply; 15+ messages in thread From: Russell King - ARM Linux @ 2016-08-26 11:13 UTC (permalink / raw) To: Anson Huang Cc: linux-arm-kernel, devicetree, linux-kernel, shawnguo, kernel, fabio.estevam, robh+dt, mark.rutland 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D 2016-08-26 11:13 ` Russell King - ARM Linux @ 2016-08-26 12:20 ` Yongcai Huang 2016-08-26 12:35 ` Russell King - ARM Linux 2016-08-26 13:48 ` Arnd Bergmann 0 siblings, 2 replies; 15+ messages in thread From: Yongcai Huang @ 2016-08-26 12:20 UTC (permalink / raw) To: Russell King - ARM Linux Cc: linux-arm-kernel, devicetree, linux-kernel, shawnguo, kernel, Fabio Estevam, robh+dt, mark.rutland 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D 2016-08-26 12:20 ` Yongcai Huang @ 2016-08-26 12:35 ` Russell King - ARM Linux 2016-08-26 13:48 ` Arnd Bergmann 1 sibling, 0 replies; 15+ messages in thread From: Russell King - ARM Linux @ 2016-08-26 12:35 UTC (permalink / raw) To: Yongcai Huang Cc: linux-arm-kernel, devicetree, linux-kernel, shawnguo, kernel, Fabio Estevam, robh+dt, mark.rutland 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D 2016-08-26 12:20 ` Yongcai Huang 2016-08-26 12:35 ` Russell King - ARM Linux @ 2016-08-26 13:48 ` Arnd Bergmann 1 sibling, 0 replies; 15+ messages in thread From: Arnd Bergmann @ 2016-08-26 13:48 UTC (permalink / raw) To: linux-arm-kernel Cc: Yongcai Huang, Russell King - ARM Linux, mark.rutland, devicetree, linux-kernel, robh+dt, kernel, Fabio Estevam, shawnguo 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] ARM: imx: add SMP support for i.MX7D 2016-08-26 11:12 ` [PATCH 3/3] ARM: imx: add SMP support for i.MX7D Anson Huang 2016-08-26 8:59 ` Arnd Bergmann 2016-08-26 11:13 ` Russell King - ARM Linux @ 2016-08-26 13:00 ` kbuild test robot 2 siblings, 0 replies; 15+ messages in thread From: kbuild test robot @ 2016-08-26 13:00 UTC (permalink / raw) To: Anson Huang Cc: kbuild-all, linux-arm-kernel, devicetree, linux-kernel, shawnguo, kernel, fabio.estevam, robh+dt, mark.rutland, linux [-- 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 --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Add SMP support for i.MX7D 2016-08-26 11:12 [PATCH 0/3] Add SMP support for i.MX7D Anson Huang ` (2 preceding siblings ...) 2016-08-26 11:12 ` [PATCH 3/3] ARM: imx: add SMP support for i.MX7D Anson Huang @ 2016-08-26 12:43 ` Fabio Estevam 2016-08-26 13:02 ` Yongcai Huang 3 siblings, 1 reply; 15+ messages in thread From: Fabio Estevam @ 2016-08-26 12:43 UTC (permalink / raw) To: Anson Huang Cc: linux-arm-kernel, devicetree, linux-kernel, Mark Rutland, Russell King - ARM Linux, robh+dt, Sascha Hauer, Fabio Estevam, Shawn Guo 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 0/3] Add SMP support for i.MX7D 2016-08-26 12:43 ` [PATCH 0/3] Add " Fabio Estevam @ 2016-08-26 13:02 ` Yongcai Huang 0 siblings, 0 replies; 15+ messages in thread From: Yongcai Huang @ 2016-08-26 13:02 UTC (permalink / raw) To: Fabio Estevam Cc: linux-arm-kernel, devicetree, linux-kernel, Mark Rutland, Russell King - ARM Linux, robh+dt, Sascha Hauer, Fabio Estevam, Shawn Guo 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-08-26 19:59 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-26 11:12 [PATCH 0/3] Add SMP support for i.MX7D Anson Huang 2016-08-26 11:12 ` [PATCH 1/3] ARM: dts: imx7: support SMP boot up Anson Huang 2016-08-26 11:12 ` [PATCH 2/3] ARM: imx: add gpcv2 support Anson Huang 2016-08-26 11:17 ` Russell King - ARM Linux 2016-08-26 12:25 ` Yongcai Huang 2016-08-26 11:12 ` [PATCH 3/3] ARM: imx: add SMP support for i.MX7D Anson Huang 2016-08-26 8:59 ` Arnd Bergmann 2016-08-26 10:28 ` Yongcai Huang 2016-08-26 11:13 ` Russell King - ARM Linux 2016-08-26 12:20 ` Yongcai Huang 2016-08-26 12:35 ` Russell King - ARM Linux 2016-08-26 13:48 ` Arnd Bergmann 2016-08-26 13:00 ` kbuild test robot 2016-08-26 12:43 ` [PATCH 0/3] Add " Fabio Estevam 2016-08-26 13:02 ` Yongcai Huang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).