[PATCH 1/2] ARM: imx: enable smp for i.MX7D
Zhi Li
lznuaa at gmail.com
Tue May 19 07:52:54 PDT 2015
On Mon, May 18, 2015 at 9:57 PM, Shawn Guo <shawnguo at kernel.org> wrote:
> On Tue, May 12, 2015 at 05:15:54AM +0800, Frank.Li at freescale.com wrote:
>> From: Frank Li <Frank.Li at freescale.com>
>>
>> i.MX7D have two cores.
>> enable multicore support.
>> enable cpu hotplug support.
>>
>> Signed-off-by: Frank Li <Frank.Li at freescale.com>
>> Signed-off-by: Anson Huang <b20788 at freescale.com>
>> ---
>> 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
>>
>> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
>> index 1d87078..0e50ce7 100644
>> --- a/arch/arm/mach-imx/Kconfig
>> +++ b/arch/arm/mach-imx/Kconfig
>> @@ -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.
>
>> 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 40df12a..8a83366 100644
>> --- a/arch/arm/mach-imx/Makefile
>> +++ b/arch/arm/mach-imx/Makefile
>> @@ -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.
>
>> 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 d1e2873..216031b 100644
>> --- a/arch/arm/mach-imx/common.h
>> +++ b/arch/arm/mach-imx/common.h
>> @@ -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?
>
>>
>> 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;
>
> Use CPU_METHOD_OF_DECLARE() to save this declaration.
>
>> extern struct smp_operations ls1021a_smp_ops;
>>
>> #endif
>> diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c
>> new file mode 100644
>> index 0000000..554dd5e
>> --- /dev/null
>> +++ b/arch/arm/mach-imx/gpcv2.c
>> @@ -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>
>> +#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.
>
>> +#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
>> +#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.
>
>> +
>> +#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.
>
>> +
>> +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.
>
>> +
>> +static void __iomem *gpc_base;
>> +
>> +
>
> One new line is enough.
>
>> +void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset)
>
> static inline?
>
>> +{
>> + 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.
>
>> +}
>> +
>> +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.
>
>> +
>> + 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.
>
>> + 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?
>
>> + 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.
>
>> +
>> + return 0;
>> +}
>> diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-imx/headsmp.S
>> index de5047c..95467c7 100644
>> --- a/arch/arm/mach-imx/headsmp.S
>> +++ b/arch/arm/mach-imx/headsmp.S
>> @@ -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?
Yes,
v7_secondary_startup check debug register and cause halt.
>
>> diff --git a/arch/arm/mach-imx/hotplug.c b/arch/arm/mach-imx/hotplug.c
>> index b35e99c..9a7a00f 100644
>> --- a/arch/arm/mach-imx/hotplug.c
>> +++ b/arch/arm/mach-imx/hotplug.c
>> @@ -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()?
>
>> + return 1;
>> +}
>> diff --git a/arch/arm/mach-imx/mach-imx7d.c b/arch/arm/mach-imx/mach-imx7d.c
>> index 4d4a190..6657645 100644
>> --- a/arch/arm/mach-imx/mach-imx7d.c
>> +++ b/arch/arm/mach-imx/mach-imx7d.c
>> @@ -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,
>> diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
>> index 7f27001..119f96a 100644
>> --- a/arch/arm/mach-imx/platsmp.c
>> +++ b/arch/arm/mach-imx/platsmp.c
>> @@ -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)
>> diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
>> index 45f7f4e..f4ea055 100644
>> --- a/arch/arm/mach-imx/src.c
>> +++ b/arch/arm/mach-imx/src.c
>> @@ -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?
>
>> +#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.
>
>>
>> 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);
>> +
>
> Drop this new line.
>
>> +}
>> +
>> 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");
>
> 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
>
>> 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
More information about the linux-arm-kernel
mailing list