From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Sun, 1 Mar 2015 19:24:14 -0700 Subject: [U-Boot] [RFC PATCH v4 02/23] common: Make sure arch-specific map_sysmem() is defined In-Reply-To: References: <1423618233-11397-1-git-send-email-joe.hershberger@ni.com> <1424822552-4366-1-git-send-email-joe.hershberger@ni.com> <1424822552-4366-3-git-send-email-joe.hershberger@ni.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Joe, On 1 March 2015 at 14:16, Joe Hershberger wrote: > Hi Simon, > > > On Sun, Mar 1, 2015 at 12:07 PM, Simon Glass wrote: >> >> Hi Joe, >> >> On 24 February 2015 at 17:02, Joe Hershberger >> 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 >> > >> > --- >> > >> > 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 >> > +# 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 Regards, Simon