Dave Martin <[email protected]> writes: > On Mon, Feb 14, 2011 at 11:15 PM, Kevin Hilman <[email protected]> wrote: >> Hi Dave, >> >> Dave Martin <[email protected]> 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. > > Thanks for testing-- that's great news. > > I will have a think about whether the patch can be tidied up to revert > most of the code back to Thumb, though that isn't essential. If I've > understood what's going on correctly, I think only the restore entry > points, SMC call sites and the entry to omap3_sram_configure_core_dpll > could need to be ARM; the rest shouldn't really make any difference...
Yes, that sounds right. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
