[PATCH v2 1/3] ARM: omap: Enable low-level omap3 PM code to work withCONFIG_THUMB2_KERNEL

[Nicolas Pitre]

On Wed, 8 Dec 2010, Dave Martin wrote:

> Hi,
> 
> On Tue, Dec 7, 2010 at 7:15 PM, Nicolas Pitre <nicolas.pitre at linaro.org> wrote:
> > On Tue, 7 Dec 2010, Dave Martin wrote:
> >
> >> On Tue, Dec 7, 2010 at 2:53 PM, Dave Martin <dave.martin at linaro.org> wrote:
> >> [...]
> >> > Note that converting to C doesn't mean that code which attempts to
> >> > copy function bodies will work: you still need to handle the fact that
> >> > if f() is a C function symbol, then the value of the symbol f is
> >> > actually the function's base address + 1.  See my changes in sram.c,
> >>
> >> To clarify, this applies *if* f is a Thumb symbol.
> >
> > To make it generic, a new macro could be used:
> >
> > #define SYM_ADDR(x) ((void *)((long)(x) & ~1L))
> 
> Could do ... I wasn't sure if it was useful for just this one case,
> but I guess we may encounter others.  And it would make the code a lot
> less messy...

That also makes the code self documenting.

> if so, a macro for exracting just the Thumb bit, and a macro for
> rebasing a symbol while preserving the Thumb bit could also be useful.
> 
> #define SYM_STATE(x) ((long)(x) & 1)
> #define SYM_REBASE(new_addr, sym) ((void *)((long)SYM_ADDR(new_addr) |
> SYM_STATE(sym)))
> 
> The relationship could be a bit clearer if we use the name SYM_BASE
> instead of SYM_ADDR.
> 
> With these, the affected code becomes something like:
> 
> memcpy(buffer, SYM_BASE(f), size_of_f);
> new_f = SYM_REBASE(f, buffer);
> 
> What do you think?

Looks fine to me.

> Is there a recommended type for a pointer-sized integer?  I notice
> linux/types.h defines uintptr_t (as unsigned long).

If uintptr_t is already defined then it is probably a good idea to just 
use it.


Nicolas