Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file

[Mylène Josserand]

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

[Chen-Yu Tsai]

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

[Maxime Ripard]

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

[Chen-Yu Tsai]

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

[Maxime Ripard]

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

[kbuild test robot]

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

Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file

[kbuild test robot]

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: 

Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file

[Chen-Yu Tsai]

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