Skip to contentSkip to conversation

ARM: imx: enable smp for i.MX7D

PATCH Superseded Frank.Li at freescale.com 4 participants 8 messages May 11, 2015 – May 19, 2015

Subsystems (4)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 ARM: imx: enable smp for i.MX7D

From: Frank Li <[email protected]>

i.MX7D have two cores.
enable multicore support.
enable cpu hotplug support.

---
 arch/arm/mach-imx/Kconfig      |   1 +
 arch/arm/mach-imx/Makefile     |   2 +-
 arch/arm/mach-imx/common.h     |   6 ++
 arch/arm/mach-imx/gpcv2.c      | 137 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-imx/headsmp.S    |   5 ++
 arch/arm/mach-imx/hotplug.c    |   7 +++
 arch/arm/mach-imx/mach-imx7d.c |   2 +
 arch/arm/mach-imx/platsmp.c    |  14 +++++
 arch/arm/mach-imx/src.c        |  38 +++++++++++-
 9 files changed, 208 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/mach-imx/gpcv2.c
arch/arm/mach-imx/Kconfig +1 -0
index 1d87078..0e50ce7 100644
@@ -586,6 +586,7 @@ config SOC_IMX7D
 	bool "i.MX7 Dual support"
 	select PINCTRL_IMX7D
 	select ARM_GIC
+	select ARM_ARCH_TIMER
 	help
 		This enables support for Freescale i.MX7 Dual processor.
 
arch/arm/mach-imx/Makefile +1 -1
index 40df12a..8a83366 100644
@@ -74,7 +74,7 @@ obj-$(CONFIG_MACH_VPR200) += mach-vpr200.o
 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_GPC) += gpc.o gpcv2.o
 obj-$(CONFIG_HAVE_IMX_MMDC) += mmdc.o
 obj-$(CONFIG_HAVE_IMX_SRC) += src.o
 ifneq ($(CONFIG_SOC_IMX6)$(CONFIG_SOC_LS1021A),)
arch/arm/mach-imx/common.h +6 -0
index d1e2873..216031b 100644
@@ -67,6 +67,7 @@ void imx_gpc_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);
 
 enum mxc_cpu_pwr_mode {
 	WAIT_CLOCKED,		/* wfi only */
@@ -86,11 +87,13 @@ enum mx3_cpu_pwr_mode {
 void mx3_cpu_lp_set(enum mx3_cpu_pwr_mode mode);
 
 void imx_enable_cpu(int cpu, bool enable);
+void imx_enable_cpu_ca7(int cpu, bool enable);
 void imx_set_cpu_jump(int cpu, void *jump_addr);
 u32 imx_get_cpu_arg(int cpu);
 void imx_set_cpu_arg(int cpu, u32 arg);
 #ifdef CONFIG_SMP
 void v7_secondary_startup(void);
+void ca7_secondary_startup(void);
 void imx_scu_map_io(void);
 void imx_smp_prepare(void);
 #else
@@ -111,9 +114,11 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode);
 void imx6q_set_int_mem_clk_lpm(bool enable);
 void imx6sl_set_wait_clk(bool enter);
 int imx_mmdc_get_ddr_type(void);
+int imx_gpcv2_init(const char *gpc_compat);
 
 void imx_cpu_die(unsigned int cpu);
 int imx_cpu_kill(unsigned int cpu);
+int imx_cpu_ca7_kill(unsigned int cpu);
 
 #ifdef CONFIG_SUSPEND
 void v7_cpu_resume(void);
@@ -149,6 +154,7 @@ static inline void imx_init_l2cache(void) {}
 #endif
 
 extern struct smp_operations imx_smp_ops;
+extern struct smp_operations imx_smp_ca7_ops;
 extern struct smp_operations ls1021a_smp_ops;
 
 #endif
arch/arm/mach-imx/gpcv2.c new file +137 -0

137 lines not on this page · raw message

arch/arm/mach-imx/headsmp.S +5 -0
index de5047c..95467c7 100644
@@ -29,3 +29,8 @@ ENTRY(v7_secondary_startup)
 	set_diag_reg
 	b	secondary_startup
 ENDPROC(v7_secondary_startup)
+
+ENTRY(ca7_secondary_startup)
+	bl      v7_invalidate_l1
+	b       secondary_startup
+ENDPROC(ca7_secondary_startup)
arch/arm/mach-imx/hotplug.c +7 -0
index b35e99c..9a7a00f 100644
@@ -68,3 +68,10 @@ int imx_cpu_kill(unsigned int cpu)
 	imx_set_cpu_arg(cpu, 0);
 	return 1;
 }
+
+int imx_cpu_ca7_kill(unsigned int cpu)
+{
+	if (imx_cpu_kill(cpu))
+		imx_gpcv2_set_core1_pdn_pup_by_software(true);
+	return 1;
+}
arch/arm/mach-imx/mach-imx7d.c +2 -0
index 4d4a190..6657645 100644
@@ -27,6 +27,7 @@ static void __init imx7d_init_machine(void)
 static void __init imx7d_init_irq(void)
 {
 	imx_init_revision_from_anatop();
+	imx_gpcv2_init("fsl,imx7d-gpc");
 	imx_src_init();
 	irqchip_init();
 }
@@ -37,6 +38,7 @@ static const char *imx7d_dt_compat[] __initconst = {
 };
 
 DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
+	.smp            = smp_ops(imx_smp_ca7_ops),
 	.init_irq	= imx7d_init_irq,
 	.init_machine	= imx7d_init_machine,
 	.dt_compat	= imx7d_dt_compat,
arch/arm/mach-imx/platsmp.c +14 -0
index 7f27001..119f96a 100644
@@ -98,6 +98,20 @@ struct smp_operations  imx_smp_ops __initdata = {
 #endif
 };
 
+static int imx_ca7_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	imx_set_cpu_jump(cpu, ca7_secondary_startup);
+	imx_enable_cpu_ca7(cpu, true);
+	return 0;
+}
+
+struct smp_operations  imx_smp_ca7_ops __initdata = {
+	.smp_boot_secondary	= imx_ca7_boot_secondary,
+#ifdef CONFIG_HOTPLUG_CPU
+	.cpu_die		= imx_cpu_die,
+	.cpu_kill		= imx_cpu_ca7_kill,
+#endif
+};
 #define DCFG_CCSR_SCRATCHRW1	0x200
 
 static int ls1021a_boot_secondary(unsigned int cpu, struct task_struct *idle)
arch/arm/mach-imx/src.c +35 -3
index 45f7f4e..f4ea055 100644
@@ -30,8 +30,17 @@
 #define BP_SRC_SCR_CORE1_RST		14
 #define BP_SRC_SCR_CORE1_ENABLE		22
 
+/* below is for i.MX7D */
+#define SRC_GPR1_V2                     0x074
+#define SRC_A7RCR0                      0x004
+#define SRC_A7RCR1                      0x008
+
+#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);
+static int	src_gpr_offset;
 
 static const int sw_reset_bits[5] = {
 	BP_SRC_SCR_SW_GPU_RST,
@@ -96,23 +105,39 @@ void imx_enable_cpu(int cpu, bool enable)
 	spin_unlock(&scr_lock);
 }
 
+void imx_enable_cpu_ca7(int cpu, bool enable)
+{
+	u32 mask, val;
+
+	cpu = cpu_logical_map(cpu);
+	spin_lock(&scr_lock);
+	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);
+	spin_unlock(&scr_lock);
+
+}
+
 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 + src_gpr_offset + 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 + src_gpr_offset + 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 + src_gpr_offset + cpu * 8 + 4);
 }
 
 void __init imx_src_init(void)
@@ -120,6 +145,8 @@ void __init imx_src_init(void)
 	struct device_node *np;
 	u32 val;
 
+	src_gpr_offset = SRC_GPR1;
+
 	np = of_find_compatible_node(NULL, NULL, "fsl,imx51-src");
 	if (!np)
 		return;
@@ -127,6 +154,11 @@ void __init imx_src_init(void)
 	WARN_ON(!src_base);
 
 	imx_reset_controller.of_node = np;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-src");
+	if (np)
+		src_gpr_offset = SRC_GPR1_V2;
+
 	if (IS_ENABLED(CONFIG_RESET_CONTROLLER))
 		reset_controller_register(&imx_reset_controller);
 
Signature · 1 line
1.9.1
#1 lore raw

PATCH 2/2 ARM: dts: imx7d: added arch timer and gpc

From: Frank Li <[email protected]>

add cortex a7 arch timer and gpc to enable smp uboot need clear virtual timer offset.
if not, need add "arm,cpu-registers-not-fw-configured" in arch timer section to boot up.

---
 arch/arm/boot/dts/imx7d.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
arch/arm/boot/dts/imx7d.dtsi +15 -0
index c42cf8d..0318437 100644
@@ -121,6 +121,15 @@
 		clock-output-names = "osc";
 	};
 
+	timer {
+		compatible = "arm,armv7-timer";
+		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>;
+	};
+
 	soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
@@ -320,6 +329,12 @@
 				interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
 				#reset-cells = <1>;
 			};
+
+			gpc: gpc at 303a0000 {
+				compatible = "fsl,imx7d-gpc";
+				reg = <0x303a0000 0x10000>;
+				interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+			};
 		};
 
 		aips3: aips-bus at 30800000 {
Signature · 1 line
1.9.1
#2 lore raw

PATCH 1/2 ARM: imx: enable smp for i.MX7D

In reply to Frank.Li at freescale.com
Quoted 26 lines · Frank.Li at freescale.com

From: Frank Li <[email protected]>

i.MX7D have two cores.
enable multicore support.
enable cpu hotplug support.

---
 arch/arm/mach-imx/Kconfig      |   1 +
 arch/arm/mach-imx/Makefile     |   2 +-
 arch/arm/mach-imx/common.h     |   6 ++
 arch/arm/mach-imx/gpcv2.c      | 137 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-imx/headsmp.S    |   5 ++
 arch/arm/mach-imx/hotplug.c    |   7 +++
 arch/arm/mach-imx/mach-imx7d.c |   2 +
 arch/arm/mach-imx/platsmp.c    |  14 +++++
 arch/arm/mach-imx/src.c        |  38 +++++++++++-
 9 files changed, 208 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/mach-imx/gpcv2.c
arch/arm/mach-imx/Kconfig +0 -0
index 1d87078..0e50ce7 100644

arch/arm/mach-imx/Kconfig config SOC_IMX7D · line 589

Quoted 4 lines
arch/arm/mach-imx/Kconfig +1 -0
@@ -586,6 +586,7 @@ config SOC_IMX7D
 	bool "i.MX7 Dual support"
 	select PINCTRL_IMX7D
 	select ARM_GIC
+	select ARM_ARCH_TIMER

Why does this change belong to smp enablement?

arch/arm/mach-imx/Makefile obj-$(CONFIG_MACH_VPR200) += mach-vpr200.o · line 77

Quoted 13 lines
arch/arm/mach-imx/Kconfig +0 -0
 	help
 		This enables support for Freescale i.MX7 Dual processor.
 
arch/arm/mach-imx/Makefile +1 -1
index 40df12a..8a83366 100644
@@ -74,7 +74,7 @@ obj-$(CONFIG_MACH_VPR200) += mach-vpr200.o
 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_GPC) += gpc.o gpcv2.o

Have a new option for it.

arch/arm/mach-imx/common.h void imx_gpc_check_dt(void); · line 70

Quoted 12 lines
arch/arm/mach-imx/Makefile +0 -0
 obj-$(CONFIG_HAVE_IMX_MMDC) += mmdc.o
 obj-$(CONFIG_HAVE_IMX_SRC) += src.o
 ifneq ($(CONFIG_SOC_IMX6)$(CONFIG_SOC_LS1021A),)
arch/arm/mach-imx/common.h +1 -0
index d1e2873..216031b 100644
@@ -67,6 +67,7 @@ void imx_gpc_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);

Can this function name be a little shorter?

Quoted 30 lines
arch/arm/mach-imx/common.h +4 -0
 enum mxc_cpu_pwr_mode {
 	WAIT_CLOCKED,		/* wfi only */
@@ -86,11 +87,13 @@ enum mx3_cpu_pwr_mode {
 void mx3_cpu_lp_set(enum mx3_cpu_pwr_mode mode);
 
 void imx_enable_cpu(int cpu, bool enable);
+void imx_enable_cpu_ca7(int cpu, bool enable);
 void imx_set_cpu_jump(int cpu, void *jump_addr);
 u32 imx_get_cpu_arg(int cpu);
 void imx_set_cpu_arg(int cpu, u32 arg);
 #ifdef CONFIG_SMP
 void v7_secondary_startup(void);
+void ca7_secondary_startup(void);
 void imx_scu_map_io(void);
 void imx_smp_prepare(void);
 #else
@@ -111,9 +114,11 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode);
 void imx6q_set_int_mem_clk_lpm(bool enable);
 void imx6sl_set_wait_clk(bool enter);
 int imx_mmdc_get_ddr_type(void);
+int imx_gpcv2_init(const char *gpc_compat);
 
 void imx_cpu_die(unsigned int cpu);
 int imx_cpu_kill(unsigned int cpu);
+int imx_cpu_ca7_kill(unsigned int cpu);
 
 #ifdef CONFIG_SUSPEND
 void v7_cpu_resume(void);

arch/arm/mach-imx/common.h static inline void imx_init_l2cache(void) {} · line 157

Quoted 4 lines
arch/arm/mach-imx/common.h +1 -0
@@ -149,6 +154,7 @@ static inline void imx_init_l2cache(void) {}
 #endif
 
 extern struct smp_operations imx_smp_ops;
+extern struct smp_operations imx_smp_ca7_ops;

Use CPU_METHOD_OF_DECLARE() to save this declaration.

Quoted 21 lines
arch/arm/mach-imx/common.h +0 -0
 extern struct smp_operations ls1021a_smp_ops;
 
 #endif
arch/arm/mach-imx/gpcv2.c new file +12 -0
new file mode 100644
index 0000000..554dd5e
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2015 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>

arch/arm/mach-imx/gpcv2.c line 22

Quoted 10 lines
arch/arm/mach-imx/gpcv2.c new file +10 -0
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/regulator/consumer.h>
+#include <linux/irqchip/arm-gic.h>

Sort it alphabetically, and have a new line here.

Quoted 48 lines
arch/arm/mach-imx/gpcv2.c new file +48 -0
+#include "common.h"
+#include "hardware.h"
+
+#define IMR_NUM			4
+#define GPC_LPCR_A7_BSC		0x0
+#define GPC_LPCR_A7_AD		0x4
+#define GPC_LPCR_M4		0x8
+#define GPC_SLPCR		0x14
+#define GPC_PGC_ACK_SEL_A7	0x24
+#define GPC_MISC		0x2c
+#define GPC_IMR1_CORE0		0x30
+#define GPC_IMR1_CORE1		0x40
+#define GPC_SLOT0_CFG		0xb0
+#define GPC_PGC_CPU_MAPPING	0xec
+#define GPC_CPU_PGC_SW_PUP_REQ	0xf0
+#define GPC_PU_PGC_SW_PUP_REQ	0xf8
+#define GPC_CPU_PGC_SW_PDN_REQ	0xfc
+#define GPC_PU_PGC_SW_PDN_REQ	0x104
+#define GPC_GTOR		0x124
+#define GPC_PGC_C0		0x800
+#define GPC_PGC_C1		0x840
+#define GPC_PGC_SCU		0x880
+#define GPC_PGC_FM		0xa00
+#define GPC_PGC_MIPI_PHY	0xc00
+#define GPC_PGC_PCIE_PHY	0xc40
+#define GPC_PGC_USB_OTG1_PHY	0xc80
+#define GPC_PGC_USB_OTG2_PHY	0xcc0
+#define GPC_PGC_USB_HSIC_PHY	0xd00
+
+#define BM_LPCR_A7_BSC_IRQ_SRC_A7_WAKEUP	0x70000000
+#define BM_LPCR_A7_BSC_CPU_CLK_ON_LPM		0x4000
+#define BM_LPCR_A7_BSC_LPM1			0xc
+#define BM_LPCR_A7_BSC_LPM0			0x3
+#define BP_LPCR_A7_BSC_LPM1			2
+#define BP_LPCR_A7_BSC_LPM0			0
+#define BM_LPCR_M4_MASK_DSM_TRIGGER		0x80000000
+#define BM_SLPCR_EN_DSM				0x80000000
+#define BM_SLPCR_RBC_EN				0x40000000
+#define BM_SLPCR_VSTBY				0x4
+#define BM_SLPCR_SBYOS				0x2
+#define BM_SLPCR_BYPASS_PMIC_READY		0x1
+
+#define BM_LPCR_A7_AD_EN_C1_PUP			0x800
+#define BM_LPCR_A7_AD_EN_C1_IRQ_PUP		0x400
+#define BM_LPCR_A7_AD_EN_C0_PUP			0x200
+#define BM_LPCR_A7_AD_EN_C0_IRQ_PUP		0x100
+#define BM_LPCR_A7_AD_EN_PLAT_PDN		0x10
+#define BM_LPCR_A7_AD_EN_C1_PDN			0x8

arch/arm/mach-imx/gpcv2.c line 80

Quoted 10 lines
arch/arm/mach-imx/gpcv2.c new file +10 -0
+#define BM_LPCR_A7_AD_EN_C1_WFI_PDN		0x4
+#define BM_LPCR_A7_AD_EN_C0_PDN			0x2
+#define BM_LPCR_A7_AD_EN_C0_WFI_PDN		0x1
+
+#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7	0x2
+
+#define BM_GPC_PGC_ACK_SEL_A7_DUMMY_PUP_ACK	0x80000000
+#define BM_GPC_PGC_ACK_SEL_A7_DUMMY_PDN_ACK	0x8000
+
+#define BP_LPCR_A7_BSC_IRQ_SRC			28

Define bit fields only when you actually need them.

arch/arm/mach-imx/gpcv2.c line 86

Quoted 6 lines
arch/arm/mach-imx/gpcv2.c new file +6 -0
+
+#define MAX_SLOT_NUMBER				10
+#define A7_LPM_WAIT				0x5
+#define A7_LPM_STOP				0xa
+
+#define GPC_MAX_IRQS            (IMR_NUM * 32)

Define macros only when you actually need them.

arch/arm/mach-imx/gpcv2.c line 99

Quoted 13 lines
arch/arm/mach-imx/gpcv2.c new file +13 -0
+
+enum imx_gpc_slot {
+	CORE0_A7,
+	CORE1_A7,
+	SCU_A7,
+	FAST_MEGA_MIX,
+	MIPI_PHY,
+	PCIE_PHY,
+	USB_OTG1_PHY,
+	USB_OTG2_PHY,
+	USB_HSIC_PHY,
+	CORE0_M4,
+};

Unused.

arch/arm/mach-imx/gpcv2.c line 103

Quoted 4 lines
arch/arm/mach-imx/gpcv2.c new file +4 -0
+
+static void __iomem *gpc_base;
+
+

One new line is enough.

arch/arm/mach-imx/gpcv2.c line 104

Quoted 1 line
arch/arm/mach-imx/gpcv2.c new file +1 -0
+void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset)

static inline?

arch/arm/mach-imx/gpcv2.c line 106

Quoted 2 lines
arch/arm/mach-imx/gpcv2.c new file +2 -0
+{
+	writel_relaxed(enable, gpc_base + offset);

Why does offset need to be an argument instead of using GPC_PGC_C1 directly?

arch/arm/mach-imx/gpcv2.c line 113

Quoted 7 lines
arch/arm/mach-imx/gpcv2.c new file +7 -0
+}
+
+void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn)
+{
+
+	u32 val = readl_relaxed(gpc_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));

This ternary expression is used for a couple of times in this function, and it may be worth a variable.

arch/arm/mach-imx/gpcv2.c line 115

Quoted 2 lines
arch/arm/mach-imx/gpcv2.c new file +2 -0
+
+	imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1);

Have a new line here, move the val assignment here to make the register read/modify/write be grouped.

arch/arm/mach-imx/gpcv2.c line 123

Quoted 8 lines
arch/arm/mach-imx/gpcv2.c new file +8 -0
+	val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
+	writel_relaxed(val, gpc_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
+
+	while ((readl_relaxed(gpc_base + (pdn ?
+		GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ)) &
+		BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
+		;

Timeout check?

arch/arm/mach-imx/gpcv2.c line 134

Quoted 11 lines
arch/arm/mach-imx/gpcv2.c new file +11 -0
+	imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1);
+}
+
+int __init imx_gpcv2_init(const char *gpc_compat)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, gpc_compat);
+	gpc_base = of_iomap(np, 0);
+	if (WARN_ON(!gpc_base))
+		return -ENOMEM;

No need of a fat warning, since you return error code and caller should be handling it. Or, you use BUG_ON() and do not return error.

Quoted 8 lines
arch/arm/mach-imx/gpcv2.c new file +3 -0
+
+	return 0;
+}
arch/arm/mach-imx/headsmp.S +0 -0
index de5047c..95467c7 100644

arch/arm/mach-imx/headsmp.S ENTRY(v7_secondary_startup) · line 36

Quoted 8 lines
arch/arm/mach-imx/headsmp.S +5 -0
@@ -29,3 +29,8 @@ ENTRY(v7_secondary_startup)
 	set_diag_reg
 	b	secondary_startup
 ENDPROC(v7_secondary_startup)
+
+ENTRY(ca7_secondary_startup)
+	bl      v7_invalidate_l1
+	b       secondary_startup
+ENDPROC(ca7_secondary_startup)

v7_secondary_startup doesn't work for you?

arch/arm/mach-imx/hotplug.c int imx_cpu_kill(unsigned int cpu) · line 75

Quoted 13 lines
arch/arm/mach-imx/hotplug.c +5 -0
index b35e99c..9a7a00f 100644
@@ -68,3 +68,10 @@ int imx_cpu_kill(unsigned int cpu)
 	imx_set_cpu_arg(cpu, 0);
 	return 1;
 }
+
+int imx_cpu_ca7_kill(unsigned int cpu)
+{
+	if (imx_cpu_kill(cpu))
+		imx_gpcv2_set_core1_pdn_pup_by_software(true);

Can you put some comment to explain the additional operation required by imx_cpu_ca7_kill()?

Quoted 52 lines
arch/arm/mach-imx/hotplug.c +2 -0
+	return 1;
+}
arch/arm/mach-imx/mach-imx7d.c +2 -0
index 4d4a190..6657645 100644
@@ -27,6 +27,7 @@ static void __init imx7d_init_machine(void)
 static void __init imx7d_init_irq(void)
 {
 	imx_init_revision_from_anatop();
+	imx_gpcv2_init("fsl,imx7d-gpc");
 	imx_src_init();
 	irqchip_init();
 }
@@ -37,6 +38,7 @@ static const char *imx7d_dt_compat[] __initconst = {
 };
 
 DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
+	.smp            = smp_ops(imx_smp_ca7_ops),
 	.init_irq	= imx7d_init_irq,
 	.init_machine	= imx7d_init_machine,
 	.dt_compat	= imx7d_dt_compat,
arch/arm/mach-imx/platsmp.c +14 -0
index 7f27001..119f96a 100644
@@ -98,6 +98,20 @@ struct smp_operations  imx_smp_ops __initdata = {
 #endif
 };
 
+static int imx_ca7_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	imx_set_cpu_jump(cpu, ca7_secondary_startup);
+	imx_enable_cpu_ca7(cpu, true);
+	return 0;
+}
+
+struct smp_operations  imx_smp_ca7_ops __initdata = {
+	.smp_boot_secondary	= imx_ca7_boot_secondary,
+#ifdef CONFIG_HOTPLUG_CPU
+	.cpu_die		= imx_cpu_die,
+	.cpu_kill		= imx_cpu_ca7_kill,
+#endif
+};
 #define DCFG_CCSR_SCRATCHRW1	0x200
 
 static int ls1021a_boot_secondary(unsigned int cpu, struct task_struct *idle)
arch/arm/mach-imx/src.c +0 -0
index 45f7f4e..f4ea055 100644

arch/arm/mach-imx/src.c line 34

Quoted 5 lines
arch/arm/mach-imx/src.c +2 -0
@@ -30,8 +30,17 @@
 #define BP_SRC_SCR_CORE1_RST		14
 #define BP_SRC_SCR_CORE1_ENABLE		22
 
+/* below is for i.MX7D */
+#define SRC_GPR1_V2                     0x074

Can we have the updated SRC and GPC named after imx7d or something instead of the software version number which is quite arbitrary?

arch/arm/mach-imx/src.c line 43

Quoted 9 lines
arch/arm/mach-imx/src.c +7 -0
+#define SRC_A7RCR0                      0x004
+#define SRC_A7RCR1                      0x008
+
+#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);
+static int	src_gpr_offset;

Space instead of tab.

Quoted 11 lines
arch/arm/mach-imx/src.c +4 -0
 static const int sw_reset_bits[5] = {
 	BP_SRC_SCR_SW_GPU_RST,
@@ -96,23 +105,39 @@ void imx_enable_cpu(int cpu, bool enable)
 	spin_unlock(&scr_lock);
 }
 
+void imx_enable_cpu_ca7(int cpu, bool enable)
+{
+	u32 mask, val;
+

arch/arm/mach-imx/src.c void imx_enable_cpu(int cpu, bool enable) · line 121

Quoted 10 lines
arch/arm/mach-imx/src.c +10 -0
+	cpu = cpu_logical_map(cpu);
+	spin_lock(&scr_lock);
+	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);
+	spin_unlock(&scr_lock);
+

Drop this new line.

Quoted 26 lines
arch/arm/mach-imx/src.c +5 -3
+}
+
 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 + src_gpr_offset + 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 + src_gpr_offset + 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 + src_gpr_offset + cpu * 8 + 4);
 }
 
 void __init imx_src_init(void)

arch/arm/mach-imx/src.c void __init imx_src_init(void) · line 150

Quoted 6 lines
arch/arm/mach-imx/src.c +2 -0
@@ -120,6 +145,8 @@ void __init imx_src_init(void)
 	struct device_node *np;
 	u32 val;
 
+	src_gpr_offset = SRC_GPR1;
+
 	np = of_find_compatible_node(NULL, NULL, "fsl,imx51-src");

In the second patch, I do not see that gpc node has "fsl,imx51-src" in the compatible. How does the src_base get mapped in this case?

Shawn

Quoted 22 lines
arch/arm/mach-imx/src.c +5 -0
 	if (!np)
 		return;
@@ -127,6 +154,11 @@ void __init imx_src_init(void)
 	WARN_ON(!src_base);
 
 	imx_reset_controller.of_node = np;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-src");
+	if (np)
+		src_gpr_offset = SRC_GPR1_V2;
+
 	if (IS_ENABLED(CONFIG_RESET_CONTROLLER))
 		reset_controller_register(&imx_reset_controller);
 
Signature · 7 lines
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#3 lore raw

PATCH 1/2 ARM: imx: enable smp for i.MX7D

Quoted 22 lines · Shawn Guo
Quoted 21 lines · Frank.Li at freescale.com

From: Frank Li <[email protected]>

i.MX7D have two cores.
enable multicore support.
enable cpu hotplug support.

---
 arch/arm/mach-imx/Kconfig      |   1 +
 arch/arm/mach-imx/Makefile     |   2 +-
 arch/arm/mach-imx/common.h     |   6 ++
 arch/arm/mach-imx/gpcv2.c      | 137 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-imx/headsmp.S    |   5 ++
 arch/arm/mach-imx/hotplug.c    |   7 +++
 arch/arm/mach-imx/mach-imx7d.c |   2 +
 arch/arm/mach-imx/platsmp.c    |  14 +++++
 arch/arm/mach-imx/src.c        |  38 +++++++++++-
 9 files changed, 208 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/mach-imx/gpcv2.c
Quoted 11 lines

arch/arm/mach-imx/Kconfig config SOC_IMX7D · line 589

Quoted 9 lines
arch/arm/mach-imx/Kconfig +1 -0
index 1d87078..0e50ce7 100644
@@ -586,6 +586,7 @@ config SOC_IMX7D
      bool "i.MX7 Dual support"
      select PINCTRL_IMX7D
      select ARM_GIC
+     select ARM_ARCH_TIMER

Why does this change belong to smp enablement?

Without ARCH timer, SMP can't boot now.

Quoted 198 lines
Quoted 9 lines
arch/arm/mach-imx/Kconfig +0 -0
      help
              This enables support for Freescale i.MX7 Dual processor.
 
arch/arm/mach-imx/Makefile +0 -0
index 40df12a..8a83366 100644

arch/arm/mach-imx/Makefile obj-$(CONFIG_MACH_VPR200) += mach-vpr200.o · line 77

Quoted 5 lines
arch/arm/mach-imx/Makefile +1 -1
@@ -74,7 +74,7 @@ obj-$(CONFIG_MACH_VPR200) += mach-vpr200.o
 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_GPC) += gpc.o gpcv2.o

Have a new option for it.

arch/arm/mach-imx/common.h void imx_gpc_check_dt(void); · line 70

Quoted 12 lines
arch/arm/mach-imx/Makefile +0 -0
 obj-$(CONFIG_HAVE_IMX_MMDC) += mmdc.o
 obj-$(CONFIG_HAVE_IMX_SRC) += src.o
 ifneq ($(CONFIG_SOC_IMX6)$(CONFIG_SOC_LS1021A),)
arch/arm/mach-imx/common.h +1 -0
index d1e2873..216031b 100644
@@ -67,6 +67,7 @@ void imx_gpc_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);

Can this function name be a little shorter?

Quoted 30 lines
arch/arm/mach-imx/common.h +4 -0
 enum mxc_cpu_pwr_mode {
      WAIT_CLOCKED,           /* wfi only */
@@ -86,11 +87,13 @@ enum mx3_cpu_pwr_mode {
 void mx3_cpu_lp_set(enum mx3_cpu_pwr_mode mode);
 
 void imx_enable_cpu(int cpu, bool enable);
+void imx_enable_cpu_ca7(int cpu, bool enable);
 void imx_set_cpu_jump(int cpu, void *jump_addr);
 u32 imx_get_cpu_arg(int cpu);
 void imx_set_cpu_arg(int cpu, u32 arg);
 #ifdef CONFIG_SMP
 void v7_secondary_startup(void);
+void ca7_secondary_startup(void);
 void imx_scu_map_io(void);
 void imx_smp_prepare(void);
 #else
@@ -111,9 +114,11 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode);
 void imx6q_set_int_mem_clk_lpm(bool enable);
 void imx6sl_set_wait_clk(bool enter);
 int imx_mmdc_get_ddr_type(void);
+int imx_gpcv2_init(const char *gpc_compat);
 
 void imx_cpu_die(unsigned int cpu);
 int imx_cpu_kill(unsigned int cpu);
+int imx_cpu_ca7_kill(unsigned int cpu);
 
 #ifdef CONFIG_SUSPEND
 void v7_cpu_resume(void);

arch/arm/mach-imx/common.h static inline void imx_init_l2cache(void) {} · line 157

Quoted 4 lines
arch/arm/mach-imx/common.h +1 -0
@@ -149,6 +154,7 @@ static inline void imx_init_l2cache(void) {}
 #endif
 
 extern struct smp_operations imx_smp_ops;
+extern struct smp_operations imx_smp_ca7_ops;

Use CPU_METHOD_OF_DECLARE() to save this declaration.

Quoted 21 lines
arch/arm/mach-imx/common.h +0 -0
 extern struct smp_operations ls1021a_smp_ops;
 
 #endif
arch/arm/mach-imx/gpcv2.c new file +12 -0
new file mode 100644
index 0000000..554dd5e
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2015 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>

arch/arm/mach-imx/gpcv2.c line 22

Quoted 10 lines
arch/arm/mach-imx/gpcv2.c new file +10 -0
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/regulator/consumer.h>
+#include <linux/irqchip/arm-gic.h>

Sort it alphabetically, and have a new line here.

Quoted 48 lines
arch/arm/mach-imx/gpcv2.c new file +48 -0
+#include "common.h"
+#include "hardware.h"
+
+#define IMR_NUM                      4
+#define GPC_LPCR_A7_BSC              0x0
+#define GPC_LPCR_A7_AD               0x4
+#define GPC_LPCR_M4          0x8
+#define GPC_SLPCR            0x14
+#define GPC_PGC_ACK_SEL_A7   0x24
+#define GPC_MISC             0x2c
+#define GPC_IMR1_CORE0               0x30
+#define GPC_IMR1_CORE1               0x40
+#define GPC_SLOT0_CFG                0xb0
+#define GPC_PGC_CPU_MAPPING  0xec
+#define GPC_CPU_PGC_SW_PUP_REQ       0xf0
+#define GPC_PU_PGC_SW_PUP_REQ        0xf8
+#define GPC_CPU_PGC_SW_PDN_REQ       0xfc
+#define GPC_PU_PGC_SW_PDN_REQ        0x104
+#define GPC_GTOR             0x124
+#define GPC_PGC_C0           0x800
+#define GPC_PGC_C1           0x840
+#define GPC_PGC_SCU          0x880
+#define GPC_PGC_FM           0xa00
+#define GPC_PGC_MIPI_PHY     0xc00
+#define GPC_PGC_PCIE_PHY     0xc40
+#define GPC_PGC_USB_OTG1_PHY 0xc80
+#define GPC_PGC_USB_OTG2_PHY 0xcc0
+#define GPC_PGC_USB_HSIC_PHY 0xd00
+
+#define BM_LPCR_A7_BSC_IRQ_SRC_A7_WAKEUP     0x70000000
+#define BM_LPCR_A7_BSC_CPU_CLK_ON_LPM                0x4000
+#define BM_LPCR_A7_BSC_LPM1                  0xc
+#define BM_LPCR_A7_BSC_LPM0                  0x3
+#define BP_LPCR_A7_BSC_LPM1                  2
+#define BP_LPCR_A7_BSC_LPM0                  0
+#define BM_LPCR_M4_MASK_DSM_TRIGGER          0x80000000
+#define BM_SLPCR_EN_DSM                              0x80000000
+#define BM_SLPCR_RBC_EN                              0x40000000
+#define BM_SLPCR_VSTBY                               0x4
+#define BM_SLPCR_SBYOS                               0x2
+#define BM_SLPCR_BYPASS_PMIC_READY           0x1
+
+#define BM_LPCR_A7_AD_EN_C1_PUP                      0x800
+#define BM_LPCR_A7_AD_EN_C1_IRQ_PUP          0x400
+#define BM_LPCR_A7_AD_EN_C0_PUP                      0x200
+#define BM_LPCR_A7_AD_EN_C0_IRQ_PUP          0x100
+#define BM_LPCR_A7_AD_EN_PLAT_PDN            0x10
+#define BM_LPCR_A7_AD_EN_C1_PDN                      0x8

arch/arm/mach-imx/gpcv2.c line 80

Quoted 10 lines
arch/arm/mach-imx/gpcv2.c new file +10 -0
+#define BM_LPCR_A7_AD_EN_C1_WFI_PDN          0x4
+#define BM_LPCR_A7_AD_EN_C0_PDN                      0x2
+#define BM_LPCR_A7_AD_EN_C0_WFI_PDN          0x1
+
+#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7   0x2
+
+#define BM_GPC_PGC_ACK_SEL_A7_DUMMY_PUP_ACK  0x80000000
+#define BM_GPC_PGC_ACK_SEL_A7_DUMMY_PDN_ACK  0x8000
+
+#define BP_LPCR_A7_BSC_IRQ_SRC                       28

Define bit fields only when you actually need them.

arch/arm/mach-imx/gpcv2.c line 86

Quoted 6 lines
arch/arm/mach-imx/gpcv2.c new file +6 -0
+
+#define MAX_SLOT_NUMBER                              10
+#define A7_LPM_WAIT                          0x5
+#define A7_LPM_STOP                          0xa
+
+#define GPC_MAX_IRQS            (IMR_NUM * 32)

Define macros only when you actually need them.

arch/arm/mach-imx/gpcv2.c line 99

Quoted 13 lines
arch/arm/mach-imx/gpcv2.c new file +13 -0
+
+enum imx_gpc_slot {
+     CORE0_A7,
+     CORE1_A7,
+     SCU_A7,
+     FAST_MEGA_MIX,
+     MIPI_PHY,
+     PCIE_PHY,
+     USB_OTG1_PHY,
+     USB_OTG2_PHY,
+     USB_HSIC_PHY,
+     CORE0_M4,
+};

Unused.

arch/arm/mach-imx/gpcv2.c line 103

Quoted 4 lines
arch/arm/mach-imx/gpcv2.c new file +4 -0
+
+static void __iomem *gpc_base;
+
+

One new line is enough.

arch/arm/mach-imx/gpcv2.c line 104

Quoted 1 line
arch/arm/mach-imx/gpcv2.c new file +1 -0
+void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset)
Quoted 7 lines

static inline?

arch/arm/mach-imx/gpcv2.c line 106

Quoted 2 lines
arch/arm/mach-imx/gpcv2.c new file +2 -0
+{
+     writel_relaxed(enable, gpc_base + offset);

Why does offset need to be an argument instead of using GPC_PGC_C1 directly?

It can be used to power off CPU0 when suspend.

Quoted 54 lines

arch/arm/mach-imx/gpcv2.c line 113

Quoted 8 lines
arch/arm/mach-imx/gpcv2.c new file +7 -0
+}
+
+void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn)
+{
+
+     u32 val = readl_relaxed(gpc_base + (pdn ?
+             GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));

This ternary expression is used for a couple of times in this function, and it may be worth a variable.

arch/arm/mach-imx/gpcv2.c line 115

Quoted 2 lines
arch/arm/mach-imx/gpcv2.c new file +2 -0
+
+     imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1);

Have a new line here, move the val assignment here to make the register read/modify/write be grouped.

arch/arm/mach-imx/gpcv2.c line 123

Quoted 8 lines
arch/arm/mach-imx/gpcv2.c new file +8 -0
+     val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
+     writel_relaxed(val, gpc_base + (pdn ?
+             GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ));
+
+     while ((readl_relaxed(gpc_base + (pdn ?
+             GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ)) &
+             BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
+             ;

Timeout check?

arch/arm/mach-imx/gpcv2.c line 134

Quoted 11 lines
arch/arm/mach-imx/gpcv2.c new file +11 -0
+     imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1);
+}
+
+int __init imx_gpcv2_init(const char *gpc_compat)
+{
+     struct device_node *np;
+
+     np = of_find_compatible_node(NULL, NULL, gpc_compat);
+     gpc_base = of_iomap(np, 0);
+     if (WARN_ON(!gpc_base))
+             return -ENOMEM;

No need of a fat warning, since you return error code and caller should be handling it. Or, you use BUG_ON() and do not return error.

Quoted 10 lines
arch/arm/mach-imx/gpcv2.c new file +3 -0
+
+     return 0;
+}
arch/arm/mach-imx/headsmp.S +0 -0
index de5047c..95467c7 100644
@@ -29,3 +29,8 @@ ENTRY(v7_secondary_startup)
      set_diag_reg
      b       secondary_startup
Quoted 8 lines

arch/arm/mach-imx/headsmp.S ENTRY(v7_secondary_startup) · line 36

Quoted 6 lines
arch/arm/mach-imx/headsmp.S +5 -0
 ENDPROC(v7_secondary_startup)
+
+ENTRY(ca7_secondary_startup)
+     bl      v7_invalidate_l1
+     b       secondary_startup
+ENDPROC(ca7_secondary_startup)

v7_secondary_startup doesn't work for you?

Yes,
v7_secondary_startup check debug register and cause halt.

Quoted 175 lines
Quoted 6 lines
arch/arm/mach-imx/hotplug.c +0 -0
index b35e99c..9a7a00f 100644

arch/arm/mach-imx/hotplug.c int imx_cpu_kill(unsigned int cpu) · line 75

Quoted 8 lines
arch/arm/mach-imx/hotplug.c +5 -0
@@ -68,3 +68,10 @@ int imx_cpu_kill(unsigned int cpu)
      imx_set_cpu_arg(cpu, 0);
      return 1;
 }
+
+int imx_cpu_ca7_kill(unsigned int cpu)
+{
+     if (imx_cpu_kill(cpu))
+             imx_gpcv2_set_core1_pdn_pup_by_software(true);

Can you put some comment to explain the additional operation required by imx_cpu_ca7_kill()?

Quoted 52 lines
arch/arm/mach-imx/hotplug.c +2 -0
+     return 1;
+}
arch/arm/mach-imx/mach-imx7d.c +2 -0
index 4d4a190..6657645 100644
@@ -27,6 +27,7 @@ static void __init imx7d_init_machine(void)
 static void __init imx7d_init_irq(void)
 {
      imx_init_revision_from_anatop();
+     imx_gpcv2_init("fsl,imx7d-gpc");
      imx_src_init();
      irqchip_init();
 }
@@ -37,6 +38,7 @@ static const char *imx7d_dt_compat[] __initconst = {
 };
 
 DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual (Device Tree)")
+     .smp            = smp_ops(imx_smp_ca7_ops),
      .init_irq       = imx7d_init_irq,
      .init_machine   = imx7d_init_machine,
      .dt_compat      = imx7d_dt_compat,
arch/arm/mach-imx/platsmp.c +14 -0
index 7f27001..119f96a 100644
@@ -98,6 +98,20 @@ struct smp_operations  imx_smp_ops __initdata = {
 #endif
 };
 
+static int imx_ca7_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+     imx_set_cpu_jump(cpu, ca7_secondary_startup);
+     imx_enable_cpu_ca7(cpu, true);
+     return 0;
+}
+
+struct smp_operations  imx_smp_ca7_ops __initdata = {
+     .smp_boot_secondary     = imx_ca7_boot_secondary,
+#ifdef CONFIG_HOTPLUG_CPU
+     .cpu_die                = imx_cpu_die,
+     .cpu_kill               = imx_cpu_ca7_kill,
+#endif
+};
 #define DCFG_CCSR_SCRATCHRW1 0x200
 
 static int ls1021a_boot_secondary(unsigned int cpu, struct task_struct *idle)
arch/arm/mach-imx/src.c +0 -0
index 45f7f4e..f4ea055 100644

arch/arm/mach-imx/src.c line 34

Quoted 5 lines
arch/arm/mach-imx/src.c +2 -0
@@ -30,8 +30,17 @@
 #define BP_SRC_SCR_CORE1_RST         14
 #define BP_SRC_SCR_CORE1_ENABLE              22
 
+/* below is for i.MX7D */
+#define SRC_GPR1_V2                     0x074

Can we have the updated SRC and GPC named after imx7d or something instead of the software version number which is quite arbitrary?

arch/arm/mach-imx/src.c line 43

Quoted 9 lines
arch/arm/mach-imx/src.c +7 -0
+#define SRC_A7RCR0                      0x004
+#define SRC_A7RCR1                      0x008
+
+#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);
+static int   src_gpr_offset;

Space instead of tab.

Quoted 11 lines
arch/arm/mach-imx/src.c +4 -0
 static const int sw_reset_bits[5] = {
      BP_SRC_SCR_SW_GPU_RST,
@@ -96,23 +105,39 @@ void imx_enable_cpu(int cpu, bool enable)
      spin_unlock(&scr_lock);
 }
 
+void imx_enable_cpu_ca7(int cpu, bool enable)
+{
+     u32 mask, val;
+

arch/arm/mach-imx/src.c void imx_enable_cpu(int cpu, bool enable) · line 121

Quoted 10 lines
arch/arm/mach-imx/src.c +10 -0
+     cpu = cpu_logical_map(cpu);
+     spin_lock(&scr_lock);
+     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);
+     spin_unlock(&scr_lock);
+

Drop this new line.

Quoted 26 lines
arch/arm/mach-imx/src.c +5 -3
+}
+
 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 + src_gpr_offset + 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 + src_gpr_offset + 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 + src_gpr_offset + cpu * 8 + 4);
 }
 
 void __init imx_src_init(void)

arch/arm/mach-imx/src.c void __init imx_src_init(void) · line 150

Quoted 6 lines
arch/arm/mach-imx/src.c +2 -0
@@ -120,6 +145,8 @@ void __init imx_src_init(void)
      struct device_node *np;
      u32 val;
 
+     src_gpr_offset = SRC_GPR1;
+
      np = of_find_compatible_node(NULL, NULL, "fsl,imx51-src");

In the second patch, I do not see that gpc node has "fsl,imx51-src" in the compatible. How does the src_base get mapped in this case?

Shawn

Quoted 22 lines
arch/arm/mach-imx/src.c +5 -1
      if (!np)
              return;
@@ -127,6 +154,11 @@ void __init imx_src_init(void)
      WARN_ON(!src_base);
 
      imx_reset_controller.of_node = np;
+
+     np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-src");
+     if (np)
+             src_gpr_offset = SRC_GPR1_V2;
+
      if (IS_ENABLED(CONFIG_RESET_CONTROLLER))
              reset_controller_register(&imx_reset_controller);
 
--

1.9.1

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

#4 lore raw

PATCH 1/2 ARM: imx: enable smp for i.MX7D

Quoted 17 lines · Zhi Li
Quoted 7 lines · Shawn Guo
Quoted 7 lines
arch/arm/mach-imx/headsmp.S +0 -0
index de5047c..95467c7 100644
@@ -29,3 +29,8 @@ ENTRY(v7_secondary_startup)
      set_diag_reg
      b       secondary_startup
Quoted 8 lines

arch/arm/mach-imx/headsmp.S ENTRY(v7_secondary_startup) · line 36

Quoted 6 lines
arch/arm/mach-imx/headsmp.S +5 -0
 ENDPROC(v7_secondary_startup)
+
+ENTRY(ca7_secondary_startup)
+     bl      v7_invalidate_l1
+     b       secondary_startup
+ENDPROC(ca7_secondary_startup)

v7_secondary_startup doesn't work for you?

Quoted 2 lines

Yes,
v7_secondary_startup check debug register and cause halt.

A better question is, why do we have the diagnostic register setting at platform level... and should we move it into the core SMP code so that we can guarantee that all CPUs are setup the same. I guess there are problems on big.LITTLE systems where we have to deal with different CPU types.

A separate issue is the v7_invalidate_l1, and looking at this again, I wonder whether the v7_flush_dcache_louis in __v7_setup is really the right thing to be doing there. At that point, the caches are disabled, so we're not using them. Any dirty cache lines at that point would be inaccessible to the CPU, and so would be a bug. I think that should become v7_invalidate_l1, which would then elimiante all the platforms which are doing this local hack.

In fact, on CPUs prior to ARMv6, we have always invalidated the caches, rather than asking them to write back dirty cache lines.

Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
#5 lore raw

PATCH 1/2 ARM: imx: enable smp for i.MX7D

Quoted 18 lines · Russell King - ARM Linux
Quoted 17 lines · Zhi Li
Quoted 15 lines · Shawn Guo

arch/arm/mach-imx/headsmp.S ENTRY(v7_secondary_startup) · line 36

Quoted 13 lines
arch/arm/mach-imx/headsmp.S +5 -0
index de5047c..95467c7 100644
@@ -29,3 +29,8 @@ ENTRY(v7_secondary_startup)
      set_diag_reg
      b       secondary_startup
 ENDPROC(v7_secondary_startup)
+
+ENTRY(ca7_secondary_startup)
+     bl      v7_invalidate_l1
+     b       secondary_startup
+ENDPROC(ca7_secondary_startup)

v7_secondary_startup doesn't work for you?

Quoted 8 lines
Quoted 2 lines

Yes,
v7_secondary_startup check debug register and cause halt.

A better question is, why do we have the diagnostic register setting at platform level... and should we move it into the core SMP code so that we can guarantee that all CPUs are setup the same. I guess there are problems on big.LITTLE systems where we have to deal with different CPU types.

This code was added in below commit. I agree it is good at moving into the core SMP
Code. But this patch just want to enable smp at i.MX7 only.

commit 087bb28329f3ccda4e50cb4cf297542d7e773e20

Author: Shawn Guo <[email protected]>
Date:   Tue Apr 16 22:11:19 2013 +0800

ARM: imx: replicate the diagnostic register of boot cpu into secondary cores

The diagnostic register holds the errata bits. Mostly bootloader does not bring up secondary cores, so that when errata bits are set in bootloader, they are set only for boot cpu. But on a SMP configuration, it should be equally done on every single core. Set up the diagnostic register for secondary cores by replicating the register from boot cpu.

Quoted 6 lines

A separate issue is the v7_invalidate_l1, and looking at this again, I wonder whether the v7_flush_dcache_louis in __v7_setup is really the right thing to be doing there. At that point, the caches are disabled, so we're not using them. Any dirty cache lines at that point would be inaccessible to the CPU, and so would be a bug.

But why we have to do that. I remember we meet random crash without v7_invalidate_l1.

Quoted 3 lines

I think that should
become v7_invalidate_l1, which would then elimiante all the platforms which are doing this local hack.

I think this is good.
What can I do for this patch?

Quoted 7 lines

In fact, on CPUs prior to ARMv6, we have always invalidated the caches, rather than asking them to write back dirty cache lines.

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.

#6 lore raw

PATCH 1/2 ARM: imx: enable smp for i.MX7D

Quoted 8 lines · Zhi Li
Quoted 3 lines · Russell King - ARM Linux

I think that should
become v7_invalidate_l1, which would then elimiante all the platforms which are doing this local hack.

I think this is good.
What can I do for this patch?

Just wait for it to filter through the mailing list - it's on its way. :)

Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
#7 lore raw

PATCH 1/2 ARM: imx: enable smp for i.MX7D

In reply to Shawn Guo
Quoted 4 lines · Shawn Guo

In the second patch, I do not see that gpc node has "fsl,imx51-src" in the compatible. How does the src_base get mapped in this case?

Shawn

It is in src node, not GPC node.

src node already in imx7d.dtsi.

best regards
Frank Li