Hi Dave, Dave Martin <dave.martin@xxxxxxxxxx> writes: > On Mon, Feb 14, 2011 at 10:00:23AM -0500, Nicolas Pitre wrote: >> On Mon, 14 Feb 2011, Dave Martin wrote: >> >> > @@ -289,8 +297,20 @@ clean_l2: >> > * - should be faster and will change with kernel >> > * - 'might' have to copy address, load and jump to it >> > */ >> > +#ifdef CONFIG_THUMB2_KERNEL >> > + /* kernel is non-interworking : must do this from Thumb */ >> > + adr r1, . + 1 >> > + bx r1 >> > + .thumb >> > +#endif >> > ldr r1, kernel_flush >> >> Didn't you mean this instead: >> >> /* kernel is non-interworking : must do this from Thumb */ >> adr r1, 1f + 1 >> bx r1 >> .thumb >> 1: ldr r1, kernel_flush >> ... > > Note that this is intended as an experimental hack, not a real patch > (apologies if I didn't make that clear...) > > Well, actually I meant "add r1, pc, #1" ... which means I was too > busy trying to be clever... oops! > > That is of course exactly equivalent to your code... > >> >> ? >> >> > blx r1 >> > +#ifdef CONFIG_THUMB2_KERNEL >> > + .align >> > + bx pc >> > + nop >> > + .arm >> >> Also here, the .align has the potential to introduce a zero halfword in >> the instruction stream before the bx. What about: >> >> adr r3, 1f >> bx r3 >> .align >> .arm >> 1: ... > > .align inserts a 16-bit nop when misaligned in Thumb in a text section, > and a word-aligned bx pc is a specific architecturally allowed way > to do an inline switch to ARM. The linker uses this trick for PLT > veneers etc. > > A nicer fix for doing this sort of call from low-level code which > might be ARM is to convert arch/arm/mm/*-v7.S to use "bx lr" to return. > > Generally, we can do this for all arches >= v5, without any > incompatibility. However, since the need for it will be rare and it > will generate patch noise for not much real benefit, > I haven't proposed this. > > Updated patch below. I tested the updated patch on top of your "dirty" branch I tested with last week, and now see off-mode working just fine. Kevin > Cheers > ---Dave > > diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S > index a204c78..6ae8a92 100644 > --- a/arch/arm/mach-omap2/sleep34xx.S > +++ b/arch/arm/mach-omap2/sleep34xx.S > @@ -32,6 +32,14 @@ > #include "sdrc.h" > #include "control.h" > > +#undef ARM > +#undef THUMB > +#undef BSYM > +#define ARM(x...) x > +#define THUMB(x...) > +#define BSYM(x) (x) > + .arm > + > /* > * Registers access definitions > */ > @@ -289,8 +297,20 @@ clean_l2: > * - should be faster and will change with kernel > * - 'might' have to copy address, load and jump to it > */ > - ldr r1, kernel_flush > +#ifdef CONFIG_THUMB2_KERNEL > + /* kernel is non-interworking : must do this from Thumb */ > + adr r1, 1f + 1 > + bx r1 > + .thumb > +#endif > +1: ldr r1, kernel_flush > blx r1 > +#ifdef CONFIG_THUMB2_KERNEL > + .align > + bx pc > + nop > + .arm > +#endif > > omap3_do_wfi: > ldr r4, sdrc_power @ read the SDRC_POWER register > diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S > index 829d235..64faab8 100644 > --- a/arch/arm/mach-omap2/sram34xx.S > +++ b/arch/arm/mach-omap2/sram34xx.S > @@ -34,6 +34,14 @@ > #include "sdrc.h" > #include "cm2xxx_3xxx.h" > > +#undef ARM > +#undef THUMB > +#undef BSYM > +#define ARM(x...) x > +#define THUMB(x...) > +#define BSYM(x) (x) > + .arm > + > .text > > /* r1 parameters */ -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
