[U-Boot] [RFC PATCH v4 02/23] common: Make sure arch-specific map_sysmem() is defined

[Simon Glass]

Hi Joe,

On 1 March 2015 at 14:16, Joe Hershberger <joe.hershberger at gmail.com> wrote:
> Hi Simon,
>
>
> On Sun, Mar 1, 2015 at 12:07 PM, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Joe,
>>
>> On 24 February 2015 at 17:02, Joe Hershberger <joe.hershberger at ni.com>
>> wrote:
>> > In the case where the arch defines a custom map_sysmem(), make sure that
>> > including just common.h is sufficient to have these functions as they
>> > are when the arch does not override it.
>> >
>> > Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
>> >
>> > ---
>> >
>> > Changes in v4:
>> > -New to v4
>> >
>> > Changes in v3: None
>> > Changes in v2: None
>> >
>> >  include/common.h | 4 +++-
>> >  1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/include/common.h b/include/common.h
>> > index 77c55c6..6510efc 100644
>> > --- a/include/common.h
>> > +++ b/include/common.h
>> > @@ -846,7 +846,9 @@ int cpu_release(int nr, int argc, char * const
>> > argv[]);
>> >  #endif
>> >
>> >  /* Define a null map_sysmem() if the architecture doesn't use it */
>> > -# ifndef CONFIG_ARCH_MAP_SYSMEM
>> > +# ifdef CONFIG_ARCH_MAP_SYSMEM
>> > +#include <asm/io.h>
>> > +# else
>> >  static inline void *map_sysmem(phys_addr_t paddr, unsigned long len)
>> >  {
>> >         return (void *)(uintptr_t)paddr;
>>
>> Do we need this patch? Is it just for sandbox? It would be nice to
>> remove things from common.h rather than adding them!
>
> If you have a recommendation for where these static inline functions should
> move, then I'm happy to move it all to a new place. My assertion is that
> whatever it is that you include to get these static inlines should also be
> what you include when CONFIG_ARCH_MAP_SYSMEM is defined. You should not need
> to include the arch-specific header separately each place that one of these
> mapping functions is used.

Fair enough. I suppose there are two options - requiring all files to
include asm/io.h, and putting them in common.h (or some other file).

Overall I think I'd prefer that they go in a separate file (perhaps
mapmem.h) and include that file everywhere. What do you think?

>
>> Anyway if you want to go ahead I'm OK with it.
>>
>> Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon