[U-Boot] [PATCH 2/3] Data types defined for 64 bit physical address

[Scott Wood]

On Fri, 2015-07-31 at 12:28 -0500, Bansal Aneesh-B39320 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Friday, July 31, 2015 10:51 PM
> > To: Bansal Aneesh-B39320
> > Cc: u-boot at lists.denx.de; Sun York-R58495; Gupta Ruchika-R66431; Kushwaha
> > Prabhakar-B32579
> > Subject: Re: [PATCH 2/3] Data types defined for 64 bit physical address
> > 
> > On Fri, 2015-07-31 at 15:23 +0530, Aneesh Bansal wrote:
> > > Data types and I/O functions have been defined for
> > > 64 bit physical addresses in arm and powerpc
> > > 
> > > Signed-off-by: Aneesh Bansal <aneesh.bansal at freescale.com>
> > > ---
> > >  arch/arm/include/asm/io.h     |  4 +++-
> > >  arch/arm/include/asm/types.h  | 13 ++++++++-----
> > > arch/powerpc/include/asm/io.h | 13 +++++++++++++
> > >  3 files changed, 24 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> > > index bfbe0a0..09d192d 100644
> > > --- a/arch/arm/include/asm/io.h
> > > +++ b/arch/arm/include/asm/io.h
> > > @@ -59,7 +59,7 @@ static inline void unmap_physmem(void *vaddr,
> > > unsigned long flags)
> > > 
> > >  static inline phys_addr_t virt_to_phys(void * vaddr)  {
> > > -     return (phys_addr_t)(vaddr);
> > > +     return (phys_addr_t)((unsigned long)vaddr);
> > >  }
> > 
> > Unnecessary parens.
> > 
> Ok, I will remove these.
> > > 
> > >  /*
> > > @@ -183,9 +183,11 @@ static inline void __raw_readsl(unsigned long
> > > addr, void *data, int longlen)
> > >  #define in_le32(a)   in_arch(l,le32,a)
> > >  #define in_le16(a)   in_arch(w,le16,a)
> > > 
> > > +#define out_be64(a, v)       out_arch(q, be64, a, v)
> > >  #define out_be32(a,v)        out_arch(l,be32,a,v)
> > >  #define out_be16(a,v)        out_arch(w,be16,a,v)
> > > 
> > > +#define in_be64(a)   in_arch(q, be64, a)
> > >  #define in_be32(a)   in_arch(l,be32,a)
> > >  #define in_be16(a)   in_arch(w,be16,a)
> > 
> > Inconsistent whitespace.
> Without the space, I was getting an error with Checkpatch.

So?

> Should, I correct it all lines already present in the file or even add new 
> lines in the same way as other lines already present even though 
> checl=kpatch reports it as an error ?

Checkpatch is a tool, not a dictator.


> > > diff --git a/arch/powerpc/include/asm/io.h
> > > b/arch/powerpc/include/asm/io.h index a5257e9..8c6f47e 100644
> > > --- a/arch/powerpc/include/asm/io.h
> > > +++ b/arch/powerpc/include/asm/io.h
> > > @@ -246,6 +246,19 @@ extern inline void out_be32(volatile unsigned
> > > __iomem *addr, u32 val)
> > >       __asm__ __volatile__("sync; stw%U0%X0 %1,%0" : "=m" (*addr) :
> > > "r" (val));  }
> > > 
> > > +extern inline u64 in_be64(const u64 *addr) {
> > > +     return ((u64)in_be32((u32 *)addr) << 32) |
> > > +     (in_be32((u32 *)addr + 1));
> > > +}
> > > +
> > > +extern inline void out_be64(u64 *addr, u64 val) {
> > > +     out_be32((u32 *)addr, (u32)(val >> 32));
> > > +     out_be32((u32 *)addr + 1, (u32)val); }
> > 
> > What do you need these for?  I don't think it's a good idea to have I/O 
> > accessors that
> > look atomic but aren't (same goes for arm32).
> > 
> 64 bit read and writes are required for CAAM operations. That is why I have 
> added these in powerpc and arm.

No, it's not required.  You could just as well perform multiple out_be32() in 
the driver.

-Scott