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

[Scott Wood <scottwood@freescale.com>]

On Fri, 2015-07-31 at 13:32 -0500, Bansal Aneesh-B39320 wrote:
> > > > > 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
> > 
> It is required to define I/O accessor for 64 bit access.

It is only required if you need to do actual 64-bit I/O, which you can't do 
in a 32-bit U-Boot (and all PPC U-Boot is 32-bit).

> It is already defined in arm.

I think that should be limited to arm64.

> The driver will just call OUT_64 or IN_64.
> Now that might be a be64 or le64 depending upon the endianness of IP block 
> on that SoC.
> Further the definition of be64 and le64 will be different for ARM and 
> PowerPC.
> 
> We have a common CAAM driver which can't just have multiple out_be32() 
> instead of OUT_64.
> All PowerPC SOC's : Core is BE and CAAM is BE
> LS1020        : Core is LE and CAAM is LE

You already had addr_lo and addr_hi, with an CONFIG_SYS_FSL_SEC_LE ifdef.  
Why was that not sufficient?

> The problem is with LS1043, where core in LE and CAAM is BE.

The core's endianness is irrelevant.

> So in order, to have a unified CAAM driver we need accessors for 64 bit.
> 
> For ARM, these functions are directly defined in C as:
> out_16                (*(volatile unsigned short *)(a) = (v))
> out_32                (*(volatile unsigned int *)(a) = (v))
> out_64                (*(volatile unsigned long long *)(a) = (v))
> There is another macro defined which takes care of byte-swapping the value 
> for BE or LE
> 
> But in PowerPC, for 16 and 32 bit these are defined in ASM.
> I could not find asm instructions for 64 bit read/write. So, I defined 
> these in C.

You couldn't find them because they don't exist (except for floating point 
and such, but there's no reason to use that here), and thus the accessor 
shouldn't exist.

BTW, why are the patches of this series posted in separate threads?  Are you 
invoking git send-email separately for each patch?

-Scott