On Mon, Feb 14, 2011 at 11:15 PM, Kevin Hilman <khilman@xxxxxx> wrote: > 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. 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... Cheers ---Dave -- 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
