linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] ARM: iwmmxt/pj4 fixes for v7/Thumb-2
@ 2011-09-07 15:59 Dave Martin
  2011-09-07 15:59 ` [RFC PATCH 1/3] ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2 Dave Martin
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Dave Martin @ 2011-09-07 15:59 UTC (permalink / raw)
  To: linux-arm-kernel

The iwmmxt and pj4 support code contains some code which isn't
compatible with Thumb-2 and looks possibly inappropriate for v7.

There is also an issue with Makefile rules causing iwmmxt.S to be
built for the wrong architecture.

This series attempts to fix those issues.  However, I'm not totally
happy with it -- if someone with relevant boards and/or experience
can test or comment, that would be appreciated.

In particular, the current makefile rules are still not really
right.  To avoid excessive pain and disruption, simpler but
somewhat incorrect changes are made in the Makefile, and iwmmxt.S
is changed for Thumb-2 kernels only.  This may be adequete, but I
don't kave the required knowledge to be sure.

If someone with some platform knowledge can comment on whether it
appears valid to replace the old instruction sequences with ISBs,
that would be appreciated too.  In this series I'm currently making
assumptions based on the core architecture.

Dave Martin (3):
  ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2
  ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2
  ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2

 arch/arm/kernel/Makefile  |    7 +++++++
 arch/arm/kernel/iwmmxt.S  |    9 +++++++++
 arch/arm/kernel/pj4-cp0.c |    4 ++++
 3 files changed, 20 insertions(+), 0 deletions(-)

-- 1.7.4.1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 1/3] ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2
  2011-09-07 15:59 [RFC PATCH 0/3] ARM: iwmmxt/pj4 fixes for v7/Thumb-2 Dave Martin
@ 2011-09-07 15:59 ` Dave Martin
  2011-09-07 16:10   ` Eric Miao
  2011-09-07 15:59 ` [RFC PATCH 2/3] ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2 Dave Martin
  2011-09-07 15:59 ` [RFC PATCH 3/3] ARM: pxa/pj4: Port problematic pj4 " Dave Martin
  2 siblings, 1 reply; 19+ messages in thread
From: Dave Martin @ 2011-09-07 15:59 UTC (permalink / raw)
  To: linux-arm-kernel

Because gcc/gas have no sane way to turn on individual CPU
extensions from the command-line, iwmmxt.S was previously built
with -mcpu=iwmmxt.  Unfortunately, this also downgrades the CPU to
v5, with the result that this file fails to build for a Thumb-2
kernel.

New versions of the tools support -march=<base arch>+iwmmxt, and it
seems reasonable to require up-to-date tools when building in
Thumb-2.  So, this patch uses -march=armv7-a+iwmmxt for
CONFIG_THUMB2_KERNEL=y.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/kernel/Makefile |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index f7887dc..8a58339 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -65,7 +65,14 @@ obj-$(CONFIG_CPU_PJ4)		+= pj4-cp0.o
 obj-$(CONFIG_IWMMXT)		+= iwmmxt.o
 obj-$(CONFIG_CPU_HAS_PMU)	+= pmu.o
 obj-$(CONFIG_HW_PERF_EVENTS)	+= perf_event.o
+
+# When enough people have binutils which support -march=...+iwmmxt, this
+# should change to something like if __LINUX_ARM_ARCH__ < 7.
+ifdef CONFIG_THUMB2_KERNEL
+AFLAGS_iwmmxt.o			:= -Wa,-march=armv7-a+iwmmxt
+else
 AFLAGS_iwmmxt.o			:= -Wa,-mcpu=iwmmxt
+endif
 
 ifneq ($(CONFIG_ARCH_EBSA110),y)
   obj-y		+= io.o
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH 2/3] ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2
  2011-09-07 15:59 [RFC PATCH 0/3] ARM: iwmmxt/pj4 fixes for v7/Thumb-2 Dave Martin
  2011-09-07 15:59 ` [RFC PATCH 1/3] ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2 Dave Martin
@ 2011-09-07 15:59 ` Dave Martin
  2011-09-07 16:13   ` Eric Miao
  2011-09-07 15:59 ` [RFC PATCH 3/3] ARM: pxa/pj4: Port problematic pj4 " Dave Martin
  2 siblings, 1 reply; 19+ messages in thread
From: Dave Martin @ 2011-09-07 15:59 UTC (permalink / raw)
  To: linux-arm-kernel

The iwmmxt code contains some code to implement a pseudo-ISB, but
this is not buildable for Thumb-2.

This patch replaces the pseudo-ISB with a real one for Thumb-2
kernels.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/kernel/iwmmxt.S |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/iwmmxt.S b/arch/arm/kernel/iwmmxt.S
index a087838..bee8a74 100644
--- a/arch/arm/kernel/iwmmxt.S
+++ b/arch/arm/kernel/iwmmxt.S
@@ -319,8 +319,17 @@ ENTRY(iwmmxt_task_switch)
 	PJ4(eor r1, r1, #0xf)
 	PJ4(mcr	p15, 0, r1, c1, c0, 2)
 
+/*
+ * When enough people have binutils which support -march=...+iwmmxt, this
+ * should change to #if __LINUX_ARM_ARCH__ < 7.
+ */
+#ifndef CONFIG_THUMB2_KERNEL
 	mrc	p15, 0, r1, c2, c0, 0
 	sub	pc, lr, r1, lsr #32		@ cpwait and return
+#else
+	isb					@ ISB needed instead on ARMv7
+	mov	pc, lr
+#endif
 
 /*
  * Remove Concan ownership of given task
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH 3/3] ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2
  2011-09-07 15:59 [RFC PATCH 0/3] ARM: iwmmxt/pj4 fixes for v7/Thumb-2 Dave Martin
  2011-09-07 15:59 ` [RFC PATCH 1/3] ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2 Dave Martin
  2011-09-07 15:59 ` [RFC PATCH 2/3] ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2 Dave Martin
@ 2011-09-07 15:59 ` Dave Martin
  2011-09-07 16:18   ` Eric Miao
  2011-09-08 11:10   ` Sergei Shtylyov
  2 siblings, 2 replies; 19+ messages in thread
From: Dave Martin @ 2011-09-07 15:59 UTC (permalink / raw)
  To: linux-arm-kernel

The iwmmxt code contains some code to implement a pseudo-ISB, but
this is not buildable for Thumb-2.

This patch replaces the pseudo-ISB with a real one on v7 and above.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/kernel/pj4-cp0.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/pj4-cp0.c b/arch/arm/kernel/pj4-cp0.c
index a4b1b07..5117d9d 100644
--- a/arch/arm/kernel/pj4-cp0.c
+++ b/arch/arm/kernel/pj4-cp0.c
@@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
 
 	__asm__ __volatile__ (
 		"mcr	p15, 0, %1, c1, c0, 2\n\t"
+#ifdef __LINUX_ARM_ARCH__ >= 7
+		"isb\n\t"
+#else
 		"mrc	p15, 0, %0, c1, c0, 2\n\t"
 		"mov	%0, %0\n\t"
 		"sub	pc, pc, #4\n\t"
+#endif
 		: "=r" (temp) : "r" (value));
 }
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH 1/3] ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2
  2011-09-07 15:59 ` [RFC PATCH 1/3] ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2 Dave Martin
@ 2011-09-07 16:10   ` Eric Miao
  2011-09-07 17:18     ` Nicolas Pitre
  0 siblings, 1 reply; 19+ messages in thread
From: Eric Miao @ 2011-09-07 16:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 7, 2011 at 8:59 AM, Dave Martin <dave.martin@linaro.org> wrote:
> Because gcc/gas have no sane way to turn on individual CPU
> extensions from the command-line, iwmmxt.S was previously built
> with -mcpu=iwmmxt. ?Unfortunately, this also downgrades the CPU to
> v5, with the result that this file fails to build for a Thumb-2
> kernel.
>
> New versions of the tools support -march=<base arch>+iwmmxt, and it
> seems reasonable to require up-to-date tools when building in
> Thumb-2. ?So, this patch uses -march=armv7-a+iwmmxt for
> CONFIG_THUMB2_KERNEL=y.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
> ?arch/arm/kernel/Makefile | ? ?7 +++++++
> ?1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index f7887dc..8a58339 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -65,7 +65,14 @@ obj-$(CONFIG_CPU_PJ4) ? ? ? ? ? ? ? ?+= pj4-cp0.o
> ?obj-$(CONFIG_IWMMXT) ? ? ? ? ? += iwmmxt.o
> ?obj-$(CONFIG_CPU_HAS_PMU) ? ? ?+= pmu.o
> ?obj-$(CONFIG_HW_PERF_EVENTS) ? += perf_event.o
> +
> +# When enough people have binutils which support -march=...+iwmmxt, this
> +# should change to something like if __LINUX_ARM_ARCH__ < 7.
> +ifdef CONFIG_THUMB2_KERNEL
> +AFLAGS_iwmmxt.o ? ? ? ? ? ? ? ? ? ? ? ?:= -Wa,-march=armv7-a+iwmmxt
> +else
> ?AFLAGS_iwmmxt.o ? ? ? ? ? ? ? ? ? ? ? ?:= -Wa,-mcpu=iwmmxt
> +endif

It looks more like the switch should depend on the compiler version.
Unless there is a clear way to decide if gcc supports this switch, I
think it's reasonable to have the change like above.

>
> ?ifneq ($(CONFIG_ARCH_EBSA110),y)
> ? obj-y ? ? ? ? ? ? ? ?+= io.o
> --
> 1.7.4.1
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 2/3] ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2
  2011-09-07 15:59 ` [RFC PATCH 2/3] ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2 Dave Martin
@ 2011-09-07 16:13   ` Eric Miao
  2011-09-07 20:35     ` Arnd Bergmann
  0 siblings, 1 reply; 19+ messages in thread
From: Eric Miao @ 2011-09-07 16:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 7, 2011 at 8:59 AM, Dave Martin <dave.martin@linaro.org> wrote:
> The iwmmxt code contains some code to implement a pseudo-ISB, but
> this is not buildable for Thumb-2.
>
> This patch replaces the pseudo-ISB with a real one for Thumb-2
> kernels.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
> ?arch/arm/kernel/iwmmxt.S | ? ?9 +++++++++
> ?1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/kernel/iwmmxt.S b/arch/arm/kernel/iwmmxt.S
> index a087838..bee8a74 100644
> --- a/arch/arm/kernel/iwmmxt.S
> +++ b/arch/arm/kernel/iwmmxt.S
> @@ -319,8 +319,17 @@ ENTRY(iwmmxt_task_switch)
> ? ? ? ?PJ4(eor r1, r1, #0xf)
> ? ? ? ?PJ4(mcr p15, 0, r1, c1, c0, 2)
>
> +/*
> + * When enough people have binutils which support -march=...+iwmmxt, this
> + * should change to #if __LINUX_ARM_ARCH__ < 7.
> + */
> +#ifndef CONFIG_THUMB2_KERNEL
> ? ? ? ?mrc ? ? p15, 0, r1, c2, c0, 0
> ? ? ? ?sub ? ? pc, lr, r1, lsr #32 ? ? ? ? ? ? @ cpwait and return
> +#else
> + ? ? ? isb ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? @ ISB needed instead on ARMv7
> + ? ? ? mov ? ? pc, lr
> +#endif

Or maybe using ARM() and THUMB() will be a better fit here?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 3/3] ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2
  2011-09-07 15:59 ` [RFC PATCH 3/3] ARM: pxa/pj4: Port problematic pj4 " Dave Martin
@ 2011-09-07 16:18   ` Eric Miao
  2011-09-07 20:27     ` Arnd Bergmann
  2011-09-08 11:10   ` Sergei Shtylyov
  1 sibling, 1 reply; 19+ messages in thread
From: Eric Miao @ 2011-09-07 16:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 7, 2011 at 8:59 AM, Dave Martin <dave.martin@linaro.org> wrote:
> The iwmmxt code contains some code to implement a pseudo-ISB, but
> this is not buildable for Thumb-2.
>
> This patch replaces the pseudo-ISB with a real one on v7 and above.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
> ?arch/arm/kernel/pj4-cp0.c | ? ?4 ++++
> ?1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/kernel/pj4-cp0.c b/arch/arm/kernel/pj4-cp0.c
> index a4b1b07..5117d9d 100644
> --- a/arch/arm/kernel/pj4-cp0.c
> +++ b/arch/arm/kernel/pj4-cp0.c
> @@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
>
> ? ? ? ?__asm__ __volatile__ (
> ? ? ? ? ? ? ? ?"mcr ? ?p15, 0, %1, c1, c0, 2\n\t"
> +#ifdef __LINUX_ARM_ARCH__ >= 7
> + ? ? ? ? ? ? ? "isb\n\t"
> +#else
> ? ? ? ? ? ? ? ?"mrc ? ?p15, 0, %0, c1, c0, 2\n\t"
> ? ? ? ? ? ? ? ?"mov ? ?%0, %0\n\t"
> ? ? ? ? ? ? ? ?"sub ? ?pc, pc, #4\n\t"
> +#endif

Haojian,

Could you check internally if isb() will work here as PJ4 is both v6/v7
compatible, and if it's in v7 mode, I guess isb() can be safely used here?

> ? ? ? ? ? ? ? ?: "=r" (temp) : "r" (value));
> ?}
>
> --
> 1.7.4.1
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 1/3] ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2
  2011-09-07 16:10   ` Eric Miao
@ 2011-09-07 17:18     ` Nicolas Pitre
  2011-09-07 20:32       ` Arnd Bergmann
  0 siblings, 1 reply; 19+ messages in thread
From: Nicolas Pitre @ 2011-09-07 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 7 Sep 2011, Eric Miao wrote:

> On Wed, Sep 7, 2011 at 8:59 AM, Dave Martin <dave.martin@linaro.org> wrote:
> > Because gcc/gas have no sane way to turn on individual CPU
> > extensions from the command-line, iwmmxt.S was previously built
> > with -mcpu=iwmmxt. ?Unfortunately, this also downgrades the CPU to
> > v5, with the result that this file fails to build for a Thumb-2
> > kernel.
> >
> > New versions of the tools support -march=<base arch>+iwmmxt, and it
> > seems reasonable to require up-to-date tools when building in
> > Thumb-2. ?So, this patch uses -march=armv7-a+iwmmxt for
> > CONFIG_THUMB2_KERNEL=y.
> >
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > ---
> > ?arch/arm/kernel/Makefile | ? ?7 +++++++
> > ?1 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> > index f7887dc..8a58339 100644
> > --- a/arch/arm/kernel/Makefile
> > +++ b/arch/arm/kernel/Makefile
> > @@ -65,7 +65,14 @@ obj-$(CONFIG_CPU_PJ4) ? ? ? ? ? ? ? ?+= pj4-cp0.o
> > ?obj-$(CONFIG_IWMMXT) ? ? ? ? ? += iwmmxt.o
> > ?obj-$(CONFIG_CPU_HAS_PMU) ? ? ?+= pmu.o
> > ?obj-$(CONFIG_HW_PERF_EVENTS) ? += perf_event.o
> > +
> > +# When enough people have binutils which support -march=...+iwmmxt, this
> > +# should change to something like if __LINUX_ARM_ARCH__ < 7.
> > +ifdef CONFIG_THUMB2_KERNEL
> > +AFLAGS_iwmmxt.o ? ? ? ? ? ? ? ? ? ? ? ?:= -Wa,-march=armv7-a+iwmmxt
> > +else
> > ?AFLAGS_iwmmxt.o ? ? ? ? ? ? ? ? ? ? ? ?:= -Wa,-mcpu=iwmmxt
> > +endif
> 
> It looks more like the switch should depend on the compiler version.
> Unless there is a clear way to decide if gcc supports this switch, I
> think it's reasonable to have the change like above.

Normally the way to go with gcc version dependent alternatives is to use 
something like:

AFLAGS_foo.o := $(call cc-option,<the_new_flag>,<the_fallback_flag>)

This will test if <the_new_flag> is supported by the used gcc, and use 
the fallback otherwise.


Nicolas

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 3/3] ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2
  2011-09-07 16:18   ` Eric Miao
@ 2011-09-07 20:27     ` Arnd Bergmann
  2011-09-08  8:58       ` Haojian Zhuang
  0 siblings, 1 reply; 19+ messages in thread
From: Arnd Bergmann @ 2011-09-07 20:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 07 September 2011 09:18:15 Eric Miao wrote:
> > diff --git a/arch/arm/kernel/pj4-cp0.c b/arch/arm/kernel/pj4-cp0.c
> > index a4b1b07..5117d9d 100644
> > --- a/arch/arm/kernel/pj4-cp0.c
> > +++ b/arch/arm/kernel/pj4-cp0.c
> > @@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
> >
> >        __asm__ __volatile__ (
> >                "mcr    p15, 0, %1, c1, c0, 2\n\t"
> > +#ifdef __LINUX_ARM_ARCH__ >= 7
> > +               "isb\n\t"
> > +#else
> >                "mrc    p15, 0, %0, c1, c0, 2\n\t"
> >                "mov    %0, %0\n\t"
> >                "sub    pc, pc, #4\n\t"
> > +#endif
> 
> Haojian,
> 
> Could you check internally if isb() will work here as PJ4 is both v6/v7
> compatible, and if it's in v7 mode, I guess isb() can be safely used here?

I thought we only support pj4 in v7 mode in Linux anyway.

	Arnd

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 1/3] ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2
  2011-09-07 17:18     ` Nicolas Pitre
@ 2011-09-07 20:32       ` Arnd Bergmann
  2011-09-08  8:53         ` Dave Martin
  0 siblings, 1 reply; 19+ messages in thread
From: Arnd Bergmann @ 2011-09-07 20:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 07 September 2011 13:18:21 Nicolas Pitre wrote:
> > > +
> > > +# When enough people have binutils which support -march=...+iwmmxt, this
> > > +# should change to something like if __LINUX_ARM_ARCH__ < 7.
> > > +ifdef CONFIG_THUMB2_KERNEL
> > > +AFLAGS_iwmmxt.o                        := -Wa,-march=armv7-a+iwmmxt
> > > +else
> > >  AFLAGS_iwmmxt.o                        := -Wa,-mcpu=iwmmxt
> > > +endif
> > 
> > It looks more like the switch should depend on the compiler version.
> > Unless there is a clear way to decide if gcc supports this switch, I
> > think it's reasonable to have the change like above.
> 
> Normally the way to go with gcc version dependent alternatives is to use 
> something like:
> 
> AFLAGS_foo.o := $(call cc-option,<the_new_flag>,<the_fallback_flag>)
> 
> This will test if <the_new_flag> is supported by the used gcc, and use 
> the fallback otherwise.

Yes, that's possible here, but it's not actually correct either, because the
CPU core that we are running on is either a v5 XScale with iwmmxt or
a v7 pj4 with iwmmxt. Now, it should not really matter if we build the
code with flags for a different more complex instruction set, but it can
potentially hide bugs.

I think the simple solution that Dave posted is actually more appropriate.
The three possible cases are:

v5+iwmmxt: always use -Wa,-mcpu=iwmmxt as we've always done, and it's correct
v7+iwmmxt+arm: still use -Wa,-mcpu=iwmmxt, not correct but close enough and
               is known to build the file with all existing toolchaings
v7+iwmmxt+thumb2: always use -Wa,-march=armv7-a+iwmmxt, which is correct and
		      the only possible way to build this file anyway. Old toolchains
		      will fail and there is nothing we can do about it.

	Arnd

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 2/3] ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2
  2011-09-07 16:13   ` Eric Miao
@ 2011-09-07 20:35     ` Arnd Bergmann
  2011-09-08  8:58       ` Dave Martin
  0 siblings, 1 reply; 19+ messages in thread
From: Arnd Bergmann @ 2011-09-07 20:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 07 September 2011 09:13:38 Eric Miao wrote:
> >
> > +/*
> > + * When enough people have binutils which support -march=...+iwmmxt, this
> > + * should change to #if __LINUX_ARM_ARCH__ < 7.
> > + */
> > +#ifndef CONFIG_THUMB2_KERNEL
> >        mrc     p15, 0, r1, c2, c0, 0
> >        sub     pc, lr, r1, lsr #32             @ cpwait and return
> > +#else
> > +       isb                                     @ ISB needed instead on ARMv7
> > +       mov     pc, lr
> > +#endif
> 
> Or maybe using ARM() and THUMB() will be a better fit here?

I think it would be logical to use XSC() and PJ4() instead, as those are used
elsehere in this file. However, that doesn't work with the only binutils.

If the !THUMB code above is actually correct for v7 in ARM mode (i.e. it performs
the equivalent of an ISB), then I think we should leave it.

It does look like it's whitespace broken though.

	Arnd

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 1/3] ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2
  2011-09-07 20:32       ` Arnd Bergmann
@ 2011-09-08  8:53         ` Dave Martin
  0 siblings, 0 replies; 19+ messages in thread
From: Dave Martin @ 2011-09-08  8:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 07, 2011 at 10:32:09PM +0200, Arnd Bergmann wrote:
> On Wednesday 07 September 2011 13:18:21 Nicolas Pitre wrote:
> > > > +
> > > > +# When enough people have binutils which support -march=...+iwmmxt, this
> > > > +# should change to something like if __LINUX_ARM_ARCH__ < 7.
> > > > +ifdef CONFIG_THUMB2_KERNEL
> > > > +AFLAGS_iwmmxt.o                        := -Wa,-march=armv7-a+iwmmxt
> > > > +else
> > > >  AFLAGS_iwmmxt.o                        := -Wa,-mcpu=iwmmxt
> > > > +endif
> > > 
> > > It looks more like the switch should depend on the compiler version.
> > > Unless there is a clear way to decide if gcc supports this switch, I
> > > think it's reasonable to have the change like above.
> > 
> > Normally the way to go with gcc version dependent alternatives is to use 
> > something like:
> > 
> > AFLAGS_foo.o := $(call cc-option,<the_new_flag>,<the_fallback_flag>)
> > 
> > This will test if <the_new_flag> is supported by the used gcc, and use 
> > the fallback otherwise.
> 
> Yes, that's possible here, but it's not actually correct either, because the
> CPU core that we are running on is either a v5 XScale with iwmmxt or
> a v7 pj4 with iwmmxt. Now, it should not really matter if we build the
> code with flags for a different more complex instruction set, but it can
> potentially hide bugs.
> 
> I think the simple solution that Dave posted is actually more appropriate.
> The three possible cases are:
> 
> v5+iwmmxt: always use -Wa,-mcpu=iwmmxt as we've always done, and it's correct
> v7+iwmmxt+arm: still use -Wa,-mcpu=iwmmxt, not correct but close enough and
>                is known to build the file with all existing toolchaings
> v7+iwmmxt+thumb2: always use -Wa,-march=armv7-a+iwmmxt, which is correct and
> 		      the only possible way to build this file anyway. Old toolchains
> 		      will fail and there is nothing we can do about it.

There is another option, which is to use cc-option and then check the
result in AFLAGS_iwmmxt.o, throwing an error from the Makefile as
necessary.

I'll have a go at that if nobody has any objections.

There doesn't seem to be any cleaner way to catch this error -- compiler
version issues are invisible to Kconfig.

Cheers
---Dave

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 2/3] ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2
  2011-09-07 20:35     ` Arnd Bergmann
@ 2011-09-08  8:58       ` Dave Martin
  2011-09-08  9:01         ` Russell King - ARM Linux
  0 siblings, 1 reply; 19+ messages in thread
From: Dave Martin @ 2011-09-08  8:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 07, 2011 at 10:35:46PM +0200, Arnd Bergmann wrote:
> On Wednesday 07 September 2011 09:13:38 Eric Miao wrote:
> > >
> > > +/*
> > > + * When enough people have binutils which support -march=...+iwmmxt, this
> > > + * should change to #if __LINUX_ARM_ARCH__ < 7.
> > > + */
> > > +#ifndef CONFIG_THUMB2_KERNEL
> > >        mrc     p15, 0, r1, c2, c0, 0
> > >        sub     pc, lr, r1, lsr #32             @ cpwait and return
> > > +#else
> > > +       isb                                     @ ISB needed instead on ARMv7
> > > +       mov     pc, lr
> > > +#endif
> > 
> > Or maybe using ARM() and THUMB() will be a better fit here?
> 
> I think it would be logical to use XSC() and PJ4() instead, as those are used
> elsehere in this file. However, that doesn't work with the only binutils.

I take pj4 and older platforms can never be built in the same kernel?
Otherwise, this approach is totally broken anyway-- we'll generate code
which won't run on some of the kernel's supported platforms.

> If the !THUMB code above is actually correct for v7 in ARM mode (i.e. it performs
> the equivalent of an ISB), then I think we should leave it.

With the proposed change to use cc-option in the Makefile,
we have the option of using the v7 code whenever binutils supports
it, and printing a warning otherwise.

> It does look like it's whitespace broken though.

I'll double-check that.

Cheers
---Dave

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 3/3] ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2
  2011-09-07 20:27     ` Arnd Bergmann
@ 2011-09-08  8:58       ` Haojian Zhuang
  0 siblings, 0 replies; 19+ messages in thread
From: Haojian Zhuang @ 2011-09-08  8:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 2011-09-07 at 13:27 -0700, Arnd Bergmann wrote:
> On Wednesday 07 September 2011 09:18:15 Eric Miao wrote:
> > > diff --git a/arch/arm/kernel/pj4-cp0.c b/arch/arm/kernel/pj4-cp0.c
> > > index a4b1b07..5117d9d 100644
> > > --- a/arch/arm/kernel/pj4-cp0.c
> > > +++ b/arch/arm/kernel/pj4-cp0.c
> > > @@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
> > >
> > >        __asm__ __volatile__ (
> > >                "mcr    p15, 0, %1, c1, c0, 2\n\t"
> > > +#ifdef __LINUX_ARM_ARCH__ >= 7
> > > +               "isb\n\t"
> > > +#else
> > >                "mrc    p15, 0, %0, c1, c0, 2\n\t"
> > >                "mov    %0, %0\n\t"
> > >                "sub    pc, pc, #4\n\t"
> > > +#endif
> > 
> > Haojian,
> > 
> > Could you check internally if isb() will work here as PJ4 is both v6/v7
> > compatible, and if it's in v7 mode, I guess isb() can be safely used here?
> 
PJ4 v6 mode is abandoned in real usage. So we only need to cover v7
mode.

> I thought we only support pj4 in v7 mode in Linux anyway.
> 
> 	Arnd

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 2/3] ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2
  2011-09-08  8:58       ` Dave Martin
@ 2011-09-08  9:01         ` Russell King - ARM Linux
  2011-09-08 11:33           ` Dave Martin
  0 siblings, 1 reply; 19+ messages in thread
From: Russell King - ARM Linux @ 2011-09-08  9:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 08, 2011 at 09:58:00AM +0100, Dave Martin wrote:
> I take pj4 and older platforms can never be built in the same kernel?
> Otherwise, this approach is totally broken anyway-- we'll generate code
> which won't run on some of the kernel's supported platforms.

We have a well enforced split between ARMv5 and older CPUs, and ARMv6
and later CPUs.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 3/3] ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2
  2011-09-07 15:59 ` [RFC PATCH 3/3] ARM: pxa/pj4: Port problematic pj4 " Dave Martin
  2011-09-07 16:18   ` Eric Miao
@ 2011-09-08 11:10   ` Sergei Shtylyov
  2011-09-08 11:35     ` Dave Martin
  1 sibling, 1 reply; 19+ messages in thread
From: Sergei Shtylyov @ 2011-09-08 11:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

On 07-09-2011 19:59, Dave Martin wrote:

> The iwmmxt code contains some code to implement a pseudo-ISB, but
> this is not buildable for Thumb-2.

> This patch replaces the pseudo-ISB with a real one on v7 and above.

> Signed-off-by: Dave Martin<dave.martin@linaro.org>
> ---
>   arch/arm/kernel/pj4-cp0.c |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)

> diff --git a/arch/arm/kernel/pj4-cp0.c b/arch/arm/kernel/pj4-cp0.c
> index a4b1b07..5117d9d 100644
> --- a/arch/arm/kernel/pj4-cp0.c
> +++ b/arch/arm/kernel/pj4-cp0.c
> @@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
>
>   	__asm__ __volatile__ (
>   		"mcr	p15, 0, %1, c1, c0, 2\n\t"
> +#ifdef __LINUX_ARM_ARCH__ >= 7

   Maybe #if?

WBR, Sergei

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 2/3] ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2
  2011-09-08  9:01         ` Russell King - ARM Linux
@ 2011-09-08 11:33           ` Dave Martin
  2011-09-08 14:40             ` Arnd Bergmann
  0 siblings, 1 reply; 19+ messages in thread
From: Dave Martin @ 2011-09-08 11:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 08, 2011 at 10:01:22AM +0100, Russell King - ARM Linux wrote:
> On Thu, Sep 08, 2011 at 09:58:00AM +0100, Dave Martin wrote:
> > I take pj4 and older platforms can never be built in the same kernel?
> > Otherwise, this approach is totally broken anyway-- we'll generate code
> > which won't run on some of the kernel's supported platforms.
> 
> We have a well enforced split between ARMv5 and older CPUs, and ARMv6
> and later CPUs.

Good point -- OK, I guess we don't need to worry about that scenario.

Cheers
---Dave

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 3/3] ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2
  2011-09-08 11:10   ` Sergei Shtylyov
@ 2011-09-08 11:35     ` Dave Martin
  0 siblings, 0 replies; 19+ messages in thread
From: Dave Martin @ 2011-09-08 11:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 08, 2011 at 03:10:30PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 07-09-2011 19:59, Dave Martin wrote:
> 
> >The iwmmxt code contains some code to implement a pseudo-ISB, but
> >this is not buildable for Thumb-2.
> 
> >This patch replaces the pseudo-ISB with a real one on v7 and above.
> 
> >Signed-off-by: Dave Martin<dave.martin@linaro.org>
> >---
> >  arch/arm/kernel/pj4-cp0.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> >diff --git a/arch/arm/kernel/pj4-cp0.c b/arch/arm/kernel/pj4-cp0.c
> >index a4b1b07..5117d9d 100644
> >--- a/arch/arm/kernel/pj4-cp0.c
> >+++ b/arch/arm/kernel/pj4-cp0.c
> >@@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
> >
> >  	__asm__ __volatile__ (
> >  		"mcr	p15, 0, %1, c1, c0, 2\n\t"
> >+#ifdef __LINUX_ARM_ARCH__ >= 7
> 
>   Maybe #if?
> 
> WBR, Sergei

Hmmm, yes.

With a bit of luck, we may be able to get rid of the conditional
anyway.

Cheers
---Dave

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 2/3] ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2
  2011-09-08 11:33           ` Dave Martin
@ 2011-09-08 14:40             ` Arnd Bergmann
  0 siblings, 0 replies; 19+ messages in thread
From: Arnd Bergmann @ 2011-09-08 14:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday 08 September 2011, Dave Martin wrote:
> On Thu, Sep 08, 2011 at 10:01:22AM +0100, Russell King - ARM Linux wrote:
> > On Thu, Sep 08, 2011 at 09:58:00AM +0100, Dave Martin wrote:
> > > I take pj4 and older platforms can never be built in the same kernel?
> > > Otherwise, this approach is totally broken anyway-- we'll generate code
> > > which won't run on some of the kernel's supported platforms.
> > 
> > We have a well enforced split between ARMv5 and older CPUs, and ARMv6
> > and later CPUs.
> 
> Good point -- OK, I guess we don't need to worry about that scenario.

I actually have a patch that enforces this rule in Kconfig for pxa.
Right now, you can happily select any combination of boards, which
blows up very early during the build phase if you combine v5 and v7.

	Arnd

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2011-09-08 14:40 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-07 15:59 [RFC PATCH 0/3] ARM: iwmmxt/pj4 fixes for v7/Thumb-2 Dave Martin
2011-09-07 15:59 ` [RFC PATCH 1/3] ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2 Dave Martin
2011-09-07 16:10   ` Eric Miao
2011-09-07 17:18     ` Nicolas Pitre
2011-09-07 20:32       ` Arnd Bergmann
2011-09-08  8:53         ` Dave Martin
2011-09-07 15:59 ` [RFC PATCH 2/3] ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2 Dave Martin
2011-09-07 16:13   ` Eric Miao
2011-09-07 20:35     ` Arnd Bergmann
2011-09-08  8:58       ` Dave Martin
2011-09-08  9:01         ` Russell King - ARM Linux
2011-09-08 11:33           ` Dave Martin
2011-09-08 14:40             ` Arnd Bergmann
2011-09-07 15:59 ` [RFC PATCH 3/3] ARM: pxa/pj4: Port problematic pj4 " Dave Martin
2011-09-07 16:18   ` Eric Miao
2011-09-07 20:27     ` Arnd Bergmann
2011-09-08  8:58       ` Haojian Zhuang
2011-09-08 11:10   ` Sergei Shtylyov
2011-09-08 11:35     ` Dave Martin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).