Hi Marc, Thank you for the review. On Wed, 4 Apr 2018 14:01:48 +0100 Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > Hi Mylène, > > On 03/04/18 07:18, Mylène Josserand wrote: > > The CNTVOFF register from arch timer is uninitialized. > > It should be done by the bootloader but it is currently not the case, > > even for boot CPU because this SoC is booting in secure mode. > > It leads to an random offset value meaning that each CPU will have a > > different time, which isn't working very well. > > > > Add assembly code used for boot CPU and secondary CPU cores to make > > sure that the CNTVOFF register is initialized. Because this code can > > be used by different platforms, add this assembly file in ARM's common > > folder. > > > > Signed-off-by: Mylène Josserand <mylene.josserand@xxxxxxxxxxx> > > --- > > arch/arm/common/Makefile | 1 + > > arch/arm/common/smp_cntvoff.S | 35 +++++++++++++++++++++++++++++++++++ > > arch/arm/include/asm/smp_cntvoff.h | 8 ++++++++ > > 3 files changed, 44 insertions(+) > > create mode 100644 arch/arm/common/smp_cntvoff.S > > create mode 100644 arch/arm/include/asm/smp_cntvoff.h > > > > diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile > > index 70b4a14ed993..83117deb86c8 100644 > > --- a/arch/arm/common/Makefile > > +++ b/arch/arm/common/Makefile > > @@ -10,6 +10,7 @@ obj-$(CONFIG_DMABOUNCE) += dmabounce.o > > obj-$(CONFIG_SHARP_LOCOMO) += locomo.o > > obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o > > obj-$(CONFIG_SHARP_SCOOP) += scoop.o > > +obj-$(CONFIG_SMP) += smp_cntvoff.o > > obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o > > obj-$(CONFIG_MCPM) += mcpm_head.o mcpm_entry.o mcpm_platsmp.o vlock.o > > CFLAGS_REMOVE_mcpm_entry.o = -pg > > diff --git a/arch/arm/common/smp_cntvoff.S b/arch/arm/common/smp_cntvoff.S > > new file mode 100644 > > index 000000000000..65ed199a50fe > > --- /dev/null > > +++ b/arch/arm/common/smp_cntvoff.S > > @@ -0,0 +1,35 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2018 Chen-Yu Tsai > > + * Copyright (c) 2018 Bootlin > > + * > > + * Chen-Yu Tsai <wens@xxxxxxxx> > > + * Mylène Josserand <mylene.josserand@xxxxxxxxxxx> > > Given that this is literally lifted from shmobile_init_cntvoff, the > whole attribution is a bit dubious. Yes, sorry, I will fix that. > > > + * > > + * SMP support for sunxi based systems with Cortex A7/A15 > > That's not restricted to sunxi, is it? Nope, I will update this line too (bad copy-paste from sunxi/headsmp.S...) > > > + * > > + */ > > + > > +#include <linux/linkage.h> > > +#include <asm/assembler.h> > > + > > +ENTRY(smp_init_cntvoff) > > The name doesn't quite reflect the usage constraints. This will only > work if used from secure, and is UNPREDICTABLE otherwise (see the CPS > instruction). Also, the "smp" prefix is quite misleading, as it only > affects the current CPU, and not any other. > > How about secure_cntvoff_init instead? Same thing for the file name. Sure, this name is fine for me. > > > + .arch armv7-a > > + /* > > + * CNTVOFF has to be initialized either from non-secure Hypervisor > > + * mode or secure Monitor mode with SCR.NS==1. If TrustZone is enabled > > + * then it should be handled by the secure code > > + */ > > + cps #MON_MODE > > + mrc p15, 0, r1, c1, c1, 0 /* Get Secure Config */ > > + orr r0, r1, #1 > > + mcr p15, 0, r0, c1, c1, 0 /* Set Non Secure bit */ > > + isb > > + mov r0, #0 > > + mcrr p15, 4, r0, r0, c14 /* CNTVOFF = 0 */ > > + isb > > + mcr p15, 0, r1, c1, c1, 0 /* Set Secure bit */ > > + isb > > + cps #SVC_MODE > > + ret lr > > +ENDPROC(smp_init_cntvoff) > > diff --git a/arch/arm/include/asm/smp_cntvoff.h b/arch/arm/include/asm/smp_cntvoff.h > > new file mode 100644 > > index 000000000000..59a95f7604ee > > --- /dev/null > > +++ b/arch/arm/include/asm/smp_cntvoff.h > > @@ -0,0 +1,8 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +#ifndef __ASMARM_ARCH_CNTVOFF_H > > +#define __ASMARM_ARCH_CNTVOFF_H > > + > > +extern void smp_init_cntvoff(void); > > + > > +#endif > > > > It'd be good to take this opportunity to refactor the shmobile code. I can do it in this series but I do not have any shmobile platforms so I will not be able to test my modifications (only compilation). If someone can test it for me (who?), it is okay for me to refactor this code :) Best regards, -- Mylène Josserand, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
