| Message ID | 1424822552-4366-3-git-send-email-joe.hershberger@ni.com |
|---|---|
| State | RFC |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Joe, On 24 February 2015 at 17:02, Joe Hershberger <joe.hershberger@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@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! Anyway if you want to go ahead I'm OK with it. Reviewed-by: Simon Glass <sjg@chromium.org> Regards, Simon
Hi Simon, On Sun, Mar 1, 2015 at 12:07 PM, Simon Glass <sjg@chromium.org> wrote: > > Hi Joe, > > On 24 February 2015 at 17:02, Joe Hershberger <joe.hershberger@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@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. > Anyway if you want to go ahead I'm OK with it. > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Regards, > Simon
Hi Joe, On 1 March 2015 at 14:16, Joe Hershberger <joe.hershberger@gmail.com> wrote: > Hi Simon, > > > On Sun, Mar 1, 2015 at 12:07 PM, Simon Glass <sjg@chromium.org> wrote: >> >> Hi Joe, >> >> On 24 February 2015 at 17:02, Joe Hershberger <joe.hershberger@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@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@chromium.org> Regards, Simon
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;
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@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(-)