| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <37936bec442a4325bdda03a0fe2e90f8@SOC-EX02V.e01.socionext.com>
Date: Wed, 21 Nov 2018 09:33:29 +0000
From: <sugaya.taichi@...ionext.com>
To: <robh+dt@...nel.org>
CC: <linux-clk@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <linux-serial@...r.kernel.org>,
<mturquette@...libre.com>, <sboyd@...nel.org>,
<mark.rutland@....com>, <gregkh@...uxfoundation.org>,
<daniel.lezcano@...aro.org>, <tglx@...utronix.de>,
<linux@...linux.org.uk>, <jslaby@...e.com>,
<masami.hiramatsu@...aro.org>, <jaswinder.singh@...aro.org>
Subject: RE: [PATCH 01/14] ARM: milbeaut: Add basic support for Milbeaut m10v
SoC
Hi Rob
Thank you for your comments.
> -----Original Message-----
> From: Rob Herring [mailto:robh+dt@...nel.org]
> Sent: Tuesday, November 20, 2018 1:24 AM
> To: Sugaya, Taichi
> Cc: linux-clk; devicetree@...r.kernel.org; moderated list:ARM/FREESCALE
> IMX / MXC ARM ARCHITECTURE; linux-kernel@...r.kernel.org; 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@...ionext.com> 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@...ionext.com>
> > ---
> > 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
> >
Powered by blists - more mailing lists