Hi Rob Thank you for your comments. > -----Original Message----- > From: Rob Herring [mailto:robh+dt@xxxxxxxxxx] > Sent: Tuesday, November 20, 2018 1:24 AM > To: Sugaya, Taichi > Cc: linux-clk; devicetree@xxxxxxxxxxxxxxx; moderated list:ARM/FREESCALE > IMX / MXC ARM ARCHITECTURE; linux-kernel@xxxxxxxxxxxxxxx; open list:SERIAL > DRIVERS; Michael Turquette; Stephen Boyd; Mark Rutland; Greg Kroah-Hartman; > Daniel Lezcano; Thomas Gleixner; Russell King; Jiri Slaby; Masami Hiramatsu; > Jassi Brar > Subject: Re: [PATCH 01/14] ARM: milbeaut: Add basic support for Milbeaut > m10v SoC > > On Sun, Nov 18, 2018 at 7:00 PM Sugaya Taichi > <sugaya.taichi@xxxxxxxxxxxxx> wrote: > > > > This adds the basic M10V SoC support under arch/arm. > > Since all cores are activated in the custom bootloader before booting > > linux, it is necessary to wait for sub-cores using the trampoline area. > > > > Signed-off-by: Sugaya Taichi <sugaya.taichi@xxxxxxxxxxxxx> > > --- > > arch/arm/Kconfig | 2 + > > arch/arm/Makefile | 1 + > > arch/arm/mach-milbeaut/Kconfig | 28 +++++++ > > arch/arm/mach-milbeaut/Makefile | 3 + > > arch/arm/mach-milbeaut/m10v_evb.c | 31 ++++++++ > > arch/arm/mach-milbeaut/platsmp.c | 157 > ++++++++++++++++++++++++++++++++++++++ > > 6 files changed, 222 insertions(+) > > create mode 100644 arch/arm/mach-milbeaut/Kconfig > > create mode 100644 arch/arm/mach-milbeaut/Makefile > > create mode 100644 arch/arm/mach-milbeaut/m10v_evb.c > > create mode 100644 arch/arm/mach-milbeaut/platsmp.c > > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > > index 91be74d..0b8a1af 100644 > > --- a/arch/arm/Kconfig > > +++ b/arch/arm/Kconfig > > @@ -767,6 +767,8 @@ source "arch/arm/mach-mediatek/Kconfig" > > > > source "arch/arm/mach-meson/Kconfig" > > > > +source "arch/arm/mach-milbeaut/Kconfig" > > + > > source "arch/arm/mach-mmp/Kconfig" > > > > source "arch/arm/mach-moxart/Kconfig" > > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > > index 05a91d8..627853c 100644 > > --- a/arch/arm/Makefile > > +++ b/arch/arm/Makefile > > @@ -190,6 +190,7 @@ machine-$(CONFIG_ARCH_MV78XX0) += > mv78xx0 > > machine-$(CONFIG_ARCH_MVEBU) += mvebu > > machine-$(CONFIG_ARCH_MXC) += imx > > machine-$(CONFIG_ARCH_MEDIATEK) += mediatek > > +machine-$(CONFIG_ARCH_MILBEAUT) += milbeaut > > machine-$(CONFIG_ARCH_MXS) += mxs > > machine-$(CONFIG_ARCH_NETX) += netx > > machine-$(CONFIG_ARCH_NOMADIK) += nomadik > > diff --git a/arch/arm/mach-milbeaut/Kconfig > b/arch/arm/mach-milbeaut/Kconfig > > new file mode 100644 > > index 0000000..63b6f69 > > --- /dev/null > > +++ b/arch/arm/mach-milbeaut/Kconfig > > @@ -0,0 +1,28 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +menuconfig ARCH_MILBEAUT > > + bool "Socionext Milbeaut SoCs" > > + depends on ARCH_MULTI_V7 > > + select ARM_GIC > > > + select CLKDEV_LOOKUP > > + select GENERIC_CLOCKEVENTS > > + select CLKSRC_MMIO > > The clock and timer drivers' kconfig entries should select these. OK. move these to the correct kconfig. > > > + select ZONE_DMA > > Why is this needed? Ah, it may not be needed yet. confirm it. > > > + help > > + This enables support for Socionext Milbeaut SoCs > > + > > +if ARCH_MILBEAUT > > + > > +config ARCH_MILBEAUT_M10V > > + bool "Milbeaut SC2000/M10V platform" > > + select ARM_ARCH_TIMER > > + select M10V_TIMER > > + select PINCTRL > > + select PINCTRL_M10V > > + help > > + Support for Socionext's MILBEAUT M10V based systems > > + > > +config MACH_M10V_EVB > > + bool "Support for Milbeaut Evaluation boards" > > You shouldn't need a kconfig entry for each board. I see. > > > + default y > > + > > +endif > > diff --git a/arch/arm/mach-milbeaut/Makefile > b/arch/arm/mach-milbeaut/Makefile > > new file mode 100644 > > index 0000000..64f6f52 > > --- /dev/null > > +++ b/arch/arm/mach-milbeaut/Makefile > > @@ -0,0 +1,3 @@ > > +obj-$(CONFIG_SMP) += platsmp.o > > +obj-$(CONFIG_MACH_M10V_EVB) += m10v_evb.o > > + > > diff --git a/arch/arm/mach-milbeaut/m10v_evb.c > b/arch/arm/mach-milbeaut/m10v_evb.c > > new file mode 100644 > > index 0000000..a1fa7c3 > > --- /dev/null > > +++ b/arch/arm/mach-milbeaut/m10v_evb.c > > This all looks SoC specific, not board specific. Um that is right.. > > > @@ -0,0 +1,31 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2018 Socionext Inc. > > + * Copyright: (C) 2015 Linaro Ltd. > > + */ > > + > > +#include <linux/of.h> > > +#include <linux/platform_device.h> > > Not needed. OK. > > > + > > +#include <asm/mach/arch.h> > > +#include <asm/mach/map.h> > > + > > +static struct map_desc m10v_io_desc[] __initdata = { > > +}; > > + > > +void __init m10v_map_io(void) > > +{ > > + debug_ll_io_init(); > > If you use earlycon instead, then this isn't needed. > > > + iotable_init(m10v_io_desc, ARRAY_SIZE(m10v_io_desc)); > > This isn't needed. OK. > > > +} > > + > > +static const char * const m10v_compat[] = { > > + "socionext,milbeaut-m10v-evb", > > + NULL, > > +}; > > + > > +DT_MACHINE_START(M10V_REB, "Socionext Milbeaut") > > + .dt_compat = m10v_compat, > > + .l2c_aux_mask = 0xffffffff, > > + .map_io = m10v_map_io, > > It looks like you can remove this entire file and use the default machine > desc. OK, try to use the default machine descriptor instead. > > > +MACHINE_END > > diff --git a/arch/arm/mach-milbeaut/platsmp.c > b/arch/arm/mach-milbeaut/platsmp.c > > new file mode 100644 > > index 0000000..b706851 > > --- /dev/null > > +++ b/arch/arm/mach-milbeaut/platsmp.c > > @@ -0,0 +1,157 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2018 Socionext Inc. > > + * Copyright: (C) 2015 Linaro Ltd. > > + */ > > + > > +#include <linux/cpu_pm.h> > > +#include <linux/irqchip/arm-gic.h> > > +#include <linux/of_device.h> > > Not needed. Okay. > > > +#include <linux/of_address.h> > > +#include <linux/platform_device.h> > > Not needed. Okay. > > > +#include <linux/suspend.h> > > + > > +#include <asm/cacheflush.h> > > +#include <asm/cp15.h> > > +#include <asm/idmap.h> > > +#include <asm/smp_plat.h> > > +#include <asm/suspend.h> > > + > > +#define M10V_MAX_CPU 4 > > + > > +#define KERNEL_UNBOOT_FLAG 0x12345678 > > +#define CPU_FINISH_SUSPEND_FLAG 0x56784321 > > + > > +static void __iomem *trampoline; > > + > > +static int m10v_boot_secondary(unsigned int l_cpu, struct task_struct > *idle) > > +{ > > + unsigned int mpidr, cpu, cluster; > > + > > + mpidr = cpu_logical_map(l_cpu); > > + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0); > > + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1); > > + > > + if (cpu >= M10V_MAX_CPU) > > + return -EINVAL; > > + > > + pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster); > > + > > + writel(virt_to_phys(secondary_startup), trampoline + cpu * 4); > > + arch_send_wakeup_ipi_mask(cpumask_of(l_cpu)); > > + > > + return 0; > > +} > > + > > +static void m10v_cpu_die(unsigned int l_cpu) > > +{ > > + gic_cpu_if_down(0); > > + > > + v7_exit_coherency_flush(louis); > > + > > + /* Now we are prepared for power-down, do it: */ > > + wfi(); > > +} > > + > > +static int m10v_cpu_kill(unsigned int l_cpu) > > +{ > > + unsigned int mpidr, cpu; > > + > > + mpidr = cpu_logical_map(l_cpu); > > + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0); > > + > > + writel(KERNEL_UNBOOT_FLAG, trampoline + cpu * 4); > > + > > + return 1; > > +} > > + > > +static struct smp_operations m10v_smp_ops __initdata = { > > + .smp_boot_secondary = m10v_boot_secondary, > > + .cpu_die = m10v_cpu_die, > > + .cpu_kill = m10v_cpu_kill, > > +}; > > + > > +static int __init m10v_smp_init(void) > > +{ > > + unsigned int mpidr, cpu, cluster; > > + struct device_node *np; > > + > > + np = of_find_compatible_node(NULL, NULL, > "socionext,milbeaut-m10v-evb"); > > + if (!np || !of_device_is_available(np)) > > Just use of_machine_is_compatible() here. I got it, use the function instead. Thanks Sugaya Taichi > > > + return -ENODEV; > > + of_node_put(np); > > + > > + np = of_find_compatible_node(NULL, NULL, > "socionext,smp-trampoline"); > > + if (!np) > > + return -ENODEV; > > + > > + trampoline = of_iomap(np, 0); > > + if (!trampoline) > > + return -ENODEV; > > + of_node_put(np); > > + > > + mpidr = read_cpuid_mpidr(); > > + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0); > > + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1); > > + > > + pr_info("MCPM boot on cpu_%u cluster_%u\n", cpu, cluster); > > + > > + for (cpu = 0; cpu < M10V_MAX_CPU; cpu++) > > + writel(KERNEL_UNBOOT_FLAG, trampoline + cpu * 4); > > + > > + smp_set_ops(&m10v_smp_ops); > > + > > + return 0; > > +} > > +early_initcall(m10v_smp_init); > > + > > +static int m10v_pm_valid(suspend_state_t state) > > +{ > > + return (state == PM_SUSPEND_STANDBY) || (state == > PM_SUSPEND_MEM); > > +} > > + > > +typedef void (*phys_reset_t)(unsigned long); > > +static phys_reset_t phys_reset; > > + > > +static int m10v_die(unsigned long arg) > > +{ > > + setup_mm_for_reboot(); > > + asm("wfi"); > > + /* Boot just like a secondary */ > > + phys_reset = (phys_reset_t)(unsigned > long)virt_to_phys(cpu_reset); > > + phys_reset(virt_to_phys(cpu_resume)); > > + > > + return 0; > > +} > > + > > +static int m10v_pm_enter(suspend_state_t state) > > +{ > > + switch (state) { > > + case PM_SUSPEND_STANDBY: > > + pr_err("STANDBY\n"); > > + asm("wfi"); > > + break; > > + case PM_SUSPEND_MEM: > > + pr_err("SUSPEND\n"); > > + cpu_pm_enter(); > > + cpu_suspend(0, m10v_die); > > + cpu_pm_exit(); > > + break; > > + } > > + return 0; > > +} > > + > > +static const struct platform_suspend_ops m10v_pm_ops = { > > + .valid = m10v_pm_valid, > > + .enter = m10v_pm_enter, > > +}; > > + > > +struct clk *m10v_clclk_register(struct device *cpu_dev); > > + > > +static int __init m10v_pm_init(void) > > +{ > > + suspend_set_ops(&m10v_pm_ops); > > + > > + return 0; > > +} > > +late_initcall(m10v_pm_init); > > -- > > 1.9.1 > >
