| Message ID | 1438336405-9640-1-git-send-email-aneesh.bansal@freescale.com |
|---|---|
| State | Superseded |
| Delegated to: | York Sun |
| Headers | show |
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@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. > > /* > @@ -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. > 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). -Scott
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Friday, July 31, 2015 10:51 PM > To: Bansal Aneesh-B39320 > Cc: u-boot@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@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. 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 ? > > > 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. > -Scott
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@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@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
> > > > 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 already defined in arm. 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 The problem is with LS1043, where core in LE and CAAM is BE. 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.
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
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); } /* @@ -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) diff --git a/arch/arm/include/asm/types.h b/arch/arm/include/asm/types.h index ee77c41..d87f955 100644 --- a/arch/arm/include/asm/types.h +++ b/arch/arm/include/asm/types.h @@ -45,12 +45,15 @@ typedef unsigned long long u64; #define BITS_PER_LONG 32 #endif /* CONFIG_ARM64 */ -/* Dma addresses are 32-bits wide. */ - +#ifdef CONFIG_PHYS_64BIT +typedef u64 dma_addr_t; +typedef u64 phys_addr_t; +typedef u64 phys_size_t; +#else typedef u32 dma_addr_t; - -typedef unsigned long phys_addr_t; -typedef unsigned long phys_size_t; +typedef u32 phys_addr_t; +typedef u32 phys_size_t; +#endif #endif /* __KERNEL__ */ 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); +} + + /* Clear and set bits in one shot. These macros can be used to clear and * set multiple bits in a register using a single call. These macros can * also be used to set a multiple-bit bit pattern using a mask, by
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@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(-)