Subject: Re: [v5 1/2] lib/raid6: Build proper files on corresponding arch

From: Daniel Axtens <[email-masked]>
Date: Wed, 02 Aug 2017 08:27:43 +1000

Hi Matt,

> --- a/lib/raid6/test/Makefile
> +++ b/lib/raid6/test/Makefile
> @@ -44,10 +44,12 @@ else ifeq ($(HAS_NEON),yes)
>          CFLAGS += -DCONFIG_KERNEL_MODE_NEON=1
>  else
>          HAS_ALTIVEC := $(shell printf '\#include <altivec.h>\nvector int a;\n' |\
> -                         gcc -c -x c - >&/dev/null && \
> -                         rm ./-.o && echo yes)
> +                         gcc -c -x c - >/dev/null && rm ./-.o && echo yes)

>>From memory the change here (s/>&/>/) was necessary to get the build to
succeed - did we ever figure out why that was? I'm not enough of a shell
guru to grok the difference. If it's easy to explain it would be good to
put it in the commit message, rather than just saying you fixed an
unspecified bug.

>          ifeq ($(HAS_ALTIVEC),yes)
> -                OBJS += altivec1.o altivec2.o altivec4.o altivec8.o
> +                CFLAGS += -I../../../arch/powerpc/include
> +                CFLAGS += -DCONFIG_ALTIVEC
> +                OBJS += altivec1.o altivec2.o altivec4.o altivec8.o \
> +                        vpermxor1.o vpermxor2.o vpermxor4.o vpermxor8.o

You've added vpermxor here, but you don't define them until the next
patch, so the tests will fail. Please move the change to OBJS to the
next patch.

With that change, I'd be happy to formally Review this patch.

Regards,
Daniel

>          endif
>  endif
>  ifeq ($(ARCH),tilegx)
> -- 
> 2.9.3