[PATCH 1/2] ARM: imx: enable smp for i.MX7D
Shawn Guo
shawnguo at kernel.org
Mon May 18 19:57:58 PDT 2015
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?
> 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?
> +}
> +
> +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?
> 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