Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file
Hello Chen-Yu, On Wed, 18 Apr 2018 18:05:03 +0800 Chen-Yu Tsai wrote: > On Wed, Apr 18, 2018 at 4:45 PM, Maxime Ripard > wrote: > > On Tue, Apr 17, 2018 at 07:25:15PM +0800, Chen-Yu Tsai wrote: > >> On Tue, Apr 17, 2018 at 7:17 PM, Maxime Ripard > >> wrote: > >> > On Tue, Apr 17, 2018 at 11:12:41AM +0800, Chen-Yu Tsai wrote: > >> >> On Tue, Apr 17, 2018 at 5:50 AM, Mylène Josserand > >> >> wrote: > >> >> > Move the assembly code for cluster cache enabling and resuming > >> >> > into an assembly file instead of having it directly in C code. > >> >> > > >> >> > Remove the CFLAGS because we are using the ARM directive "arch" > >> >> > instead. > >> >> > > >> >> > Signed-off-by: Mylène Josserand > >> >> > --- > >> >> > arch/arm/mach-sunxi/Makefile | 4 +-- > >> >> > arch/arm/mach-sunxi/headsmp.S | 80 > >> >> > + > >> >> > arch/arm/mach-sunxi/mc_smp.c | 82 > >> >> > +++ > >> >> > 3 files changed, 85 insertions(+), 81 deletions(-) > >> >> > create mode 100644 arch/arm/mach-sunxi/headsmp.S > >> >> > >> >> I'm still not convinced about this whole "move ASM to separate > >> >> file" thing, especially now that you aren't actually adding any > >> >> sunxi-specific ASM code beyond a simple function call. > >> >> > >> >> Could you drop this for now? > >> > > >> > I'd really like to have this merged actually. There's a significant > >> > readibility improvement, so even if there's no particular functional > >> > improvement, I'd still call it a win. > >> > >> What parts do you consider hard to read? The extra quotes? Trailing > >> newline? Or perhaps the __stringify bits? > > > > All of this, plus the clobbers and operands. > > Ok. Lets move it then. Ok, I will not drop this patch then. > > The kbuild reports indicate this still needs some work though. Yes, this is "normal" because the patch, that I depend on, is not applied yet (even if my cover-letter can let think that, it is not the case, sorry). It is applied on Broadcom ARM SoC since Tuesday: https://lkml.org/lkml/2018/2/23/1263 https://github.com/Broadcom/stblinux/commits/soc/next With this patch, it should fix the errors reported by kbuild. Best regards, -- Mylène Josserand, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com
Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file
On Wed, Apr 18, 2018 at 4:45 PM, Maxime Ripard wrote: > On Tue, Apr 17, 2018 at 07:25:15PM +0800, Chen-Yu Tsai wrote: >> On Tue, Apr 17, 2018 at 7:17 PM, Maxime Ripard >> wrote: >> > On Tue, Apr 17, 2018 at 11:12:41AM +0800, Chen-Yu Tsai wrote: >> >> On Tue, Apr 17, 2018 at 5:50 AM, Mylène Josserand >> >> wrote: >> >> > Move the assembly code for cluster cache enabling and resuming >> >> > into an assembly file instead of having it directly in C code. >> >> > >> >> > Remove the CFLAGS because we are using the ARM directive "arch" >> >> > instead. >> >> > >> >> > Signed-off-by: Mylène Josserand >> >> > --- >> >> > arch/arm/mach-sunxi/Makefile | 4 +-- >> >> > arch/arm/mach-sunxi/headsmp.S | 80 >> >> > + >> >> > arch/arm/mach-sunxi/mc_smp.c | 82 >> >> > +++ >> >> > 3 files changed, 85 insertions(+), 81 deletions(-) >> >> > create mode 100644 arch/arm/mach-sunxi/headsmp.S >> >> >> >> I'm still not convinced about this whole "move ASM to separate >> >> file" thing, especially now that you aren't actually adding any >> >> sunxi-specific ASM code beyond a simple function call. >> >> >> >> Could you drop this for now? >> > >> > I'd really like to have this merged actually. There's a significant >> > readibility improvement, so even if there's no particular functional >> > improvement, I'd still call it a win. >> >> What parts do you consider hard to read? The extra quotes? Trailing >> newline? Or perhaps the __stringify bits? > > All of this, plus the clobbers and operands. Ok. Lets move it then. The kbuild reports indicate this still needs some work though. ChenYu
Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file
On Tue, Apr 17, 2018 at 07:25:15PM +0800, Chen-Yu Tsai wrote: > On Tue, Apr 17, 2018 at 7:17 PM, Maxime Ripard > wrote: > > On Tue, Apr 17, 2018 at 11:12:41AM +0800, Chen-Yu Tsai wrote: > >> On Tue, Apr 17, 2018 at 5:50 AM, Mylène Josserand > >> wrote: > >> > Move the assembly code for cluster cache enabling and resuming > >> > into an assembly file instead of having it directly in C code. > >> > > >> > Remove the CFLAGS because we are using the ARM directive "arch" > >> > instead. > >> > > >> > Signed-off-by: Mylène Josserand > >> > --- > >> > arch/arm/mach-sunxi/Makefile | 4 +-- > >> > arch/arm/mach-sunxi/headsmp.S | 80 > >> > + > >> > arch/arm/mach-sunxi/mc_smp.c | 82 > >> > +++ > >> > 3 files changed, 85 insertions(+), 81 deletions(-) > >> > create mode 100644 arch/arm/mach-sunxi/headsmp.S > >> > >> I'm still not convinced about this whole "move ASM to separate > >> file" thing, especially now that you aren't actually adding any > >> sunxi-specific ASM code beyond a simple function call. > >> > >> Could you drop this for now? > > > > I'd really like to have this merged actually. There's a significant > > readibility improvement, so even if there's no particular functional > > improvement, I'd still call it a win. > > What parts do you consider hard to read? The extra quotes? Trailing > newline? Or perhaps the __stringify bits? All of this, plus the clobbers and operands. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file
On Tue, Apr 17, 2018 at 7:17 PM, Maxime Ripard wrote: > On Tue, Apr 17, 2018 at 11:12:41AM +0800, Chen-Yu Tsai wrote: >> On Tue, Apr 17, 2018 at 5:50 AM, Mylène Josserand >> wrote: >> > Move the assembly code for cluster cache enabling and resuming >> > into an assembly file instead of having it directly in C code. >> > >> > Remove the CFLAGS because we are using the ARM directive "arch" >> > instead. >> > >> > Signed-off-by: Mylène Josserand >> > --- >> > arch/arm/mach-sunxi/Makefile | 4 +-- >> > arch/arm/mach-sunxi/headsmp.S | 80 >> > + >> > arch/arm/mach-sunxi/mc_smp.c | 82 >> > +++ >> > 3 files changed, 85 insertions(+), 81 deletions(-) >> > create mode 100644 arch/arm/mach-sunxi/headsmp.S >> >> I'm still not convinced about this whole "move ASM to separate >> file" thing, especially now that you aren't actually adding any >> sunxi-specific ASM code beyond a simple function call. >> >> Could you drop this for now? > > I'd really like to have this merged actually. There's a significant > readibility improvement, so even if there's no particular functional > improvement, I'd still call it a win. What parts do you consider hard to read? The extra quotes? Trailing newline? Or perhaps the __stringify bits? ChenYu
Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file
On Tue, Apr 17, 2018 at 11:12:41AM +0800, Chen-Yu Tsai wrote: > On Tue, Apr 17, 2018 at 5:50 AM, Mylène Josserand > wrote: > > Move the assembly code for cluster cache enabling and resuming > > into an assembly file instead of having it directly in C code. > > > > Remove the CFLAGS because we are using the ARM directive "arch" > > instead. > > > > Signed-off-by: Mylène Josserand > > --- > > arch/arm/mach-sunxi/Makefile | 4 +-- > > arch/arm/mach-sunxi/headsmp.S | 80 > > + > > arch/arm/mach-sunxi/mc_smp.c | 82 > > +++ > > 3 files changed, 85 insertions(+), 81 deletions(-) > > create mode 100644 arch/arm/mach-sunxi/headsmp.S > > I'm still not convinced about this whole "move ASM to separate > file" thing, especially now that you aren't actually adding any > sunxi-specific ASM code beyond a simple function call. > > Could you drop this for now? I'd really like to have this merged actually. There's a significant readibility improvement, so even if there's no particular functional improvement, I'd still call it a win. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file
Hi Mylène, Thank you for the patch! Yet something to improve: [auto build test ERROR on arm-soc/for-next] [also build test ERROR on v4.17-rc1 next-20180417] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Myl-ne-Josserand/Sunxi-Add-SMP-support-on-A83T/20180417-113911 base: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git for-next config: arm-allmodconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm All error/warnings (new ones prefixed by >>): In file included from arch/arm/include/asm/cputype.h:6:0, from arch/arm/mach-sunxi/headsmp.S:15: >> include/linux/kernel.h:57:0: warning: "ALIGN" redefined #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) In file included from arch/arm/mach-sunxi/headsmp.S:13:0: include/linux/linkage.h:79:0: note: this is the location of the previous definition #define ALIGN __ALIGN /usr/lib/gcc-cross/arm-linux-gnueabi/7/include/stdarg.h: Assembler messages: >> /usr/lib/gcc-cross/arm-linux-gnueabi/7/include/stdarg.h:40: Error: bad >> instruction `typedef __builtin_va_list __gnuc_va_list' >> /usr/lib/gcc-cross/arm-linux-gnueabi/7/include/stdarg.h:99: Error: bad >> instruction `typedef __gnuc_va_list va_list' >> include/linux/stddef.h:10: Error: bad instruction `enum {' >> include/linux/stddef.h:11: Error: junk at end of line, first unrecognized >> character is `,' include/linux/stddef.h:13: Error: junk at end of line, first unrecognized character is `}' >> include/linux/bitops.h:29: Error: bad instruction `extern unsigned int >> __sw_hweight8(unsigned int w)' >> include/linux/bitops.h:30: Error: bad instruction `extern unsigned int >> __sw_hweight16(unsigned int w)' >> include/linux/bitops.h:31: Error: bad instruction `extern unsigned int >> __sw_hweight32(unsigned int w)' >> include/linux/bitops.h:32: Error: bad instruction `extern unsigned long >> __sw_hweight64(__u64 w)' >> arch/arm/include/asm/irqflags.h:25: Error: bad instruction `static inline >> unsigned long arch_local_irq_save(void)' >> arch/arm/include/asm/irqflags.h:26: Error: junk at end of line, first >> unrecognized character is `{' >> arch/arm/include/asm/irqflags.h:27: Error: bad instruction `unsigned long >> flags' >> arch/arm/include/asm/irqflags.h:29: Error: bad instruction `asm volatile(' arch/arm/include/asm/irqflags.h:30: Error: bad instruction ` mrs %0, ""cpsr""@ arch_local_irq_save\n"' >> arch/arm/include/asm/irqflags.h:31: Error: bad instruction ` cpsid i"' >> arch/arm/include/asm/irqflags.h:32: Error: junk at end of line, first >> unrecognized character is `:' >> arch/arm/include/asm/irqflags.h:33: Error: bad instruction `return flags' arch/arm/include/asm/irqflags.h:34: Error: junk at end of line, first unrecognized character is `}' >> arch/arm/include/asm/irqflags.h:37: Error: bad instruction `static inline >> void arch_local_irq_enable(void)' arch/arm/include/asm/irqflags.h:38: Error: junk at end of line, first unrecognized character is `{' arch/arm/include/asm/irqflags.h:39: Error: bad instruction `asm volatile(' >> arch/arm/include/asm/irqflags.h:40: Error: bad instruction ` cpsie i >> @ arch_local_irq_enable"' arch/arm/include/asm/irqflags.h:41: Error: junk at end of line, first unrecognized character is `:' arch/arm/include/asm/irqflags.h:42: Error: junk at end of line, first unrecognized character is `:' arch/arm/include/asm/irqflags.h:43: Error: junk at end of line, first unrecognized character is `:' arch/arm/include/asm/irqflags.h:44: Error: junk at end of line, first unrecognized character is `}' >> arch/arm/include/asm/irqflags.h:47: Error: bad instruction `static inline >> void arch_local_irq_disable(void)' arch/arm/include/asm/irqflags.h:48: Error: junk at end of line, first unrecognized character is `{' arch/arm/include/asm/irqflags.h:49: Error: bad instruction `asm volatile(' >> arch/arm/include/asm/irqflags.h:50: Error: bad instruction ` cpsid i >> @ arch_local_irq_disable"' arch/arm/include/asm/irqflags.h:51: Error: junk at end of line, first unrecognized character is `:' arch/arm/include/asm/irqflags.h:52: Error: junk at end of line, first unrecognized character is `:' arch/arm/include/asm/irqflags.h:53: Error: junk at end of line, first unrecognized character is `:' arch/arm/include/asm/irqflags.h:54: Error: junk at end of line, first unrecognized character is `}' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Descri
Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file
Hi Mylène, Thank you for the patch! Yet something to improve: [auto build test ERROR on arm-soc/for-next] [also build test ERROR on v4.17-rc1 next-20180416] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Myl-ne-Josserand/Sunxi-Add-SMP-support-on-A83T/20180417-113911 base: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git for-next config: arm-sunxi_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm All errors (new ones prefixed by >>): include/asm-generic/bitops/sched.h:14: Error: junk at end of line, first unrecognized character is `{' include/asm-generic/bitops/sched.h:20: Error: bad instruction `if (b[0])' include/asm-generic/bitops/sched.h:21: Error: bad instruction `return __ffs(b[0])' include/asm-generic/bitops/sched.h:22: Error: bad instruction `if (b[1])' include/asm-generic/bitops/sched.h:23: Error: bad instruction `return __ffs(b[1])+32' include/asm-generic/bitops/sched.h:24: Error: bad instruction `if (b[2])' include/asm-generic/bitops/sched.h:25: Error: bad instruction `return __ffs(b[2])+64' include/asm-generic/bitops/sched.h:26: Error: bad instruction `return __ffs(b[3])+96' include/asm-generic/bitops/sched.h:30: Error: junk at end of line, first unrecognized character is `}' include/asm-generic/bitops/arch_hweight.h:7: Error: bad instruction `static inline unsigned int __arch_hweight32(unsigned int w)' include/asm-generic/bitops/arch_hweight.h:8: Error: junk at end of line, first unrecognized character is `{' include/asm-generic/bitops/arch_hweight.h:9: Error: bad instruction `return __sw_hweight32(w)' include/asm-generic/bitops/arch_hweight.h:10: Error: junk at end of line, first unrecognized character is `}' include/asm-generic/bitops/arch_hweight.h:12: Error: bad instruction `static inline unsigned int __arch_hweight16(unsigned int w)' include/asm-generic/bitops/arch_hweight.h:13: Error: junk at end of line, first unrecognized character is `{' include/asm-generic/bitops/arch_hweight.h:14: Error: bad instruction `return __sw_hweight16(w)' include/asm-generic/bitops/arch_hweight.h:15: Error: junk at end of line, first unrecognized character is `}' include/asm-generic/bitops/arch_hweight.h:17: Error: bad instruction `static inline unsigned int __arch_hweight8(unsigned int w)' include/asm-generic/bitops/arch_hweight.h:18: Error: junk at end of line, first unrecognized character is `{' include/asm-generic/bitops/arch_hweight.h:19: Error: bad instruction `return __sw_hweight8(w)' include/asm-generic/bitops/arch_hweight.h:20: Error: junk at end of line, first unrecognized character is `}' include/asm-generic/bitops/arch_hweight.h:22: Error: bad instruction `static inline unsigned long __arch_hweight64(__u64 w)' include/asm-generic/bitops/arch_hweight.h:23: Error: junk at end of line, first unrecognized character is `{' include/asm-generic/bitops/arch_hweight.h:24: Error: bad instruction `return __sw_hweight64(w)' include/asm-generic/bitops/arch_hweight.h:25: Error: junk at end of line, first unrecognized character is `}' include/asm-generic/bitops/find.h:30: Error: bad instruction `extern unsigned long find_next_and_bit(const unsigned long*addr1,' include/asm-generic/bitops/find.h:31: Error: bad instruction `const unsigned long*addr2,unsigned long size,' include/asm-generic/bitops/find.h:32: Error: bad instruction `unsigned long offset)' arch/arm/include/asm/swab.h:23: Error: bad instruction `static inline __u32 __arch_swahb32(__u32 x)' arch/arm/include/asm/swab.h:24: Error: junk at end of line, first unrecognized character is `{' arch/arm/include/asm/swab.h:25: Error: bad instruction `__asm__ ("rev16 %0, %1":"=r"(x):"r"(x))' arch/arm/include/asm/swab.h:26: Error: bad instruction `return x' arch/arm/include/asm/swab.h:27: Error: junk at end of line, first unrecognized character is `}' arch/arm/include/asm/swab.h:31: Error: bad instruction `static inline __u32 __arch_swab32(__u32 x)' arch/arm/include/asm/swab.h:32: Error: junk at end of line, first unrecognized character is `{' arch/arm/include/asm/swab.h:33: Error: bad instruction `__asm__ ("rev %0, %1":"=r"(x):"r"(x))' arch/arm/include/asm/swab.h:34: Error: bad instruction `return x' arch/arm/include/asm/swab.h:35: Error: junk at end of line, first unrecognized character is `}' include/uapi/linux/swab.h:47: Error: bad instruction `static inline __u16 __fswab16(__u16 val)' include/uapi/linux/swab.h:48: Error: junk at end of line, first unrecognized character is `{' include/uapi/linux/swab.h:50: Error
Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file
On Tue, Apr 17, 2018 at 5:50 AM, Mylène Josserand wrote: > Move the assembly code for cluster cache enabling and resuming > into an assembly file instead of having it directly in C code. > > Remove the CFLAGS because we are using the ARM directive "arch" > instead. > > Signed-off-by: Mylène Josserand > --- > arch/arm/mach-sunxi/Makefile | 4 +-- > arch/arm/mach-sunxi/headsmp.S | 80 + > arch/arm/mach-sunxi/mc_smp.c | 82 > +++ > 3 files changed, 85 insertions(+), 81 deletions(-) > create mode 100644 arch/arm/mach-sunxi/headsmp.S I'm still not convinced about this whole "move ASM to separate file" thing, especially now that you aren't actually adding any sunxi-specific ASM code beyond a simple function call. Could you drop this for now? ChenYu