Dave Martin <dave.martin@xxxxxxxxxx> writes: > 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... Yes, that sounds right. Kevin -- 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
