On Thu, Sep 12, 2013 at 07:51:24AM +0100, Fan Rong wrote: > Signed-off-by: Fan Rong <cin...@gmail.com> > --- > arch/arm/mach-sunxi/Makefile | 2 + > arch/arm/mach-sunxi/headsmp.S | 12 ++ > arch/arm/mach-sunxi/platform.h | 346 > ++++++++++++++++++++++++++++++++++++++++ > arch/arm/mach-sunxi/platsmp.c | 166 +++++++++++++++++++ > arch/arm/mach-sunxi/sunxi.c | 4 + > 5 files changed, 530 insertions(+) > create mode 100644 arch/arm/mach-sunxi/headsmp.S > create mode 100644 arch/arm/mach-sunxi/platform.h > create mode 100644 arch/arm/mach-sunxi/platsmp.c >
[...] > +static struct of_device_id sunxi_cc_ids[] = { > + { .compatible = "allwinner,sun7i-cc"}, > + { /*sentinel*/ } > +}; NAK - There's no binding document for this in mainline or this series. > + > + > +static void enable_aw_cpu(int cpu) > +{ > + long paddr; > + u32 pwr_reg; > + > + struct device_node *np; > + np = of_find_matching_node(NULL, sunxi_cc_ids); > + cc_base = of_iomap(np, 0); This seems to get called repeatedly in sunxi7i_boot_secondary, so you're repeatedly trying to find the cc node and mapping it, but never unmapping it. That's both a waste of time and a waste of address space. It would be nicer to map this once at the start. That seems simpler than stashing the physical address and mapping/unmapping it repeatedly. smp_boot_secondary. > + pr_debug("cc_base is %x\n", (unsigned int)cc_base); > + if (!cc_base) { You can use %p to print pointers, without casting to an integer type. > + pr_debug("error map cc configure\n"); > + return; As this may be called repeatedly, from a function that can return errors, it would be nice to propagate an error code if there's a failure. > + } > + > + paddr = virt_to_phys(sun7i_secondary_startup); > + writel(paddr, cc_base + AW_CPUCFG_P_REG0); > + /* step1: Assert nCOREPORESET LOW and hold L1RSTDISABLE LOW. > + Ensure DBGPWRDUP is held LOW to prevent any external > + debug access to the processor. > + */ > + /* assert cpu core reset */ > + writel(0, cc_base + CPUX_RESET_CTL(cpu)); > + /* L1RSTDISABLE hold low */ > + pwr_reg = readl(cc_base + AW_CPUCFG_GENCTL); > + pwr_reg &= ~(1<<cpu); > + writel(pwr_reg, cc_base + AW_CPUCFG_GENCTL); > + /* DBGPWRDUP hold low */ > + pwr_reg = readl(cc_base + AW_CPUCFG_DBGCTL1); > + pwr_reg &= ~(1<<cpu); > + writel(pwr_reg, cc_base + AW_CPUCFG_DBGCTL1); > + > + /* step2: release power clamp */ > + writel(0xff, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x7f, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x3f, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x1f, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x0f, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x07, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x03, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x01, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x00, cc_base + AW_CPU1_PWR_CLAMP); > + mdelay(10); > + > + /* step3: clear power-off gating */ > + pwr_reg = readl(cc_base + AW_CPU1_PWROFF_REG); > + pwr_reg &= ~(1); > + writel(pwr_reg, cc_base + AW_CPU1_PWROFF_REG); > + mdelay(1); > + > + /* step4: de-assert core reset */ > + writel(3, cc_base + CPUX_RESET_CTL(cpu)); > + > + /* step5: assert DBGPWRDUP signal */ > + pwr_reg = readl(cc_base + AW_CPUCFG_DBGCTL1); > + pwr_reg |= (1<<cpu); > + writel(pwr_reg, cc_base + AW_CPUCFG_DBGCTL1); > + > +} > + > + > + > +static void sunxi7i_smp_init_cpus(void) > +{ > + unsigned int i, ncores; > + > + > + /* HDG: we do not use scu_get_core_count() here as that does not > + work on the A20 ? */ > + > + /* Read current CP15 Cache Size ID Register */ Invalid comment. Judging by the encoding, this is the L2CTLR, not the CCSIDR. > + asm volatile ("mrc p15, 1, %0, c9, c0, 2" : "=r" (ncores)); > + ncores = ((ncores >> 24) & 0x3) + 1; > + > + pr_debug("[%s] ncores=%d\n", __func__, ncores); > + > + for (i = 0; i < ncores; i++) > + set_cpu_possible(i, true); > + > +} Even ignoring the above, as long as your dt is correct arm_dt_init_cpu_maps (called from stup_arch) will set the cpus as possible (and handles arbitrary MPIDR values as may be the case in multi-cluster). You don't need this function -- please remove it. > + > +/* > + * for arch/arm/kernel/smp.c:smp_prepare_cpus(unsigned int max_cpus) > + */ > +static void sunxi7i_smp_prepare_cpus(unsigned int max_cpus) > +{ > + /* > + * HDG: we do not call scu_enable() here as the sun6i source dump has > + * a modified arch/arm/kernel/smp_scu.c, where scu_enable() is a nop. > + */ > +} A look in smp.c shows smp_prepare_cpus is perfectly happy to not have a smp_prepare_cpus callback. You don't need this function -- please remove it. > + > + > + > + > + > + Why so much space here (and elsewhere)? > +/* > + * for linux/arch/arm/kernel/smp.c:__cpu_up(..) > + */ > +static int sunxi7i_boot_secondary(unsigned int cpu, struct task_struct *idle) > +{ > + pr_debug("[%s] enter cpu %d\n", __func__, cpu); > + spin_lock(&boot_lock); > + enable_aw_cpu(cpu); This can fail. You should propagate the error when it does. > + spin_unlock(&boot_lock); > + return 0; > +} > + > + > + > + > +struct smp_operations sunxi7i_smp_ops __initdata = { > + .smp_init_cpus = sunxi7i_smp_init_cpus, > + .smp_prepare_cpus = sunxi7i_smp_prepare_cpus, > + .smp_boot_secondary = sunxi7i_boot_secondary, > +}; I believe only smp_boot_secondary is necessary here. Thanks, Mark. > + > + > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c > index e79fb34..f3594e6 100644 > --- a/arch/arm/mach-sunxi/sunxi.c > +++ b/arch/arm/mach-sunxi/sunxi.c > @@ -26,6 +26,9 @@ > #include <asm/mach/map.h> > #include <asm/system_misc.h> > > +#include "platform.h" > + > + > #define SUN4I_WATCHDOG_CTRL_REG 0x00 > #define SUN4I_WATCHDOG_CTRL_RESTART BIT(0) > #define SUN4I_WATCHDOG_MODE_REG 0x04 > @@ -139,6 +142,7 @@ static const char * const sunxi_board_dt_compat[] = { > }; > > DT_MACHINE_START(SUNXI_DT, "Allwinner A1X (Device Tree)") > + .smp = smp_ops(sunxi7i_smp_ops), > .init_machine = sunxi_dt_init, > .init_time = sunxi_timer_init, > .dt_compat = sunxi_board_dt_compat, > -- > 1.7.9.5 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/