| Message ID | 1315411158-17479-3-git-send-email-dave.martin@linaro.org |
|---|---|
| State | RFC |
| Headers | show |
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?
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
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
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.
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
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
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
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(-)