[PATCH] ARM: asm: add readq/writeq methods

[PATCH] ARM: asm: add readq/writeq methods

[Matthias Mann]

Add readq/writeq methods for 32 bit ARM to allow transfering 64 bit words over
PCIe as a single transfer.

Signed-off-by: Matthias Mann <m.mann@arkona-technologies.de>
---
This patch creates checkpatch warnings, but I used the style used for the
existing functions.
It is based on branch next/soc of the arm-soc tree.

 arch/arm/include/asm/io.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 3c597c2..0a8d015 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -94,6 +94,13 @@ static inline void __raw_writel(u32 val, volatile void __iomem *addr)
 		     : "r" (val));
 }
 
+static inline void __raw_writeq(u64 val, volatile void __iomem *addr)
+{
+	asm volatile("strd %1, %0"
+		     : "+Qo" (*(volatile u64 __force *)addr)
+		     : "r" (val));
+}
+
 static inline u8 __raw_readb(const volatile void __iomem *addr)
 {
 	u8 val;
@@ -112,6 +119,15 @@ static inline u32 __raw_readl(const volatile void __iomem *addr)
 	return val;
 }
 
+static inline u64 __raw_readq(const volatile void __iomem *addr)
+{
+	u64 val;
+	asm volatile("ldrd %1, %0"
+		     : "+Qo" (*(volatile u64 __force *)addr),
+		       "=r" (val));
+	return val;
+}
+
 /*
  * Architecture ioremap implementation.
  */
@@ -293,18 +309,23 @@ extern void _memset_io(volatile void __iomem *, int, size_t);
 					__raw_readw(c)); __r; })
 #define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
 					__raw_readl(c)); __r; })
+#define readq_relaxed(c) ({ u64 __r = le64_to_cpu((__force __le64) \
+					__raw_readq(c)); __r; })
 
 #define writeb_relaxed(v,c)	__raw_writeb(v,c)
 #define writew_relaxed(v,c)	__raw_writew((__force u16) cpu_to_le16(v),c)
 #define writel_relaxed(v,c)	__raw_writel((__force u32) cpu_to_le32(v),c)
+#define writeq_relaxed(v,c)	__raw_writeq((__force u64) cpu_to_le64(v),c)
 
 #define readb(c)		({ u8  __v = readb_relaxed(c); __iormb(); __v; })
 #define readw(c)		({ u16 __v = readw_relaxed(c); __iormb(); __v; })
 #define readl(c)		({ u32 __v = readl_relaxed(c); __iormb(); __v; })
+#define readq(c)		({ u64 __v = readq_relaxed(c); __iormb(); __v; })
 
 #define writeb(v,c)		({ __iowmb(); writeb_relaxed(v,c); })
 #define writew(v,c)		({ __iowmb(); writew_relaxed(v,c); })
 #define writel(v,c)		({ __iowmb(); writel_relaxed(v,c); })
+#define writeq(v,c)		({ __iowmb(); writeq_relaxed(v,c); })
 
 #define readsb(p,d,l)		__raw_readsb(p,d,l)
 #define readsw(p,d,l)		__raw_readsw(p,d,l)
-- 
1.8.3.2

[PATCH] ARM: asm: add readq/writeq methods

[Måns Rullgård]

Matthias Mann <M.Mann@arkona-technologies.de> writes:

> Add readq/writeq methods for 32 bit ARM to allow transfering 64 bit words over
> PCIe as a single transfer.
>
> Signed-off-by: Matthias Mann <m.mann@arkona-technologies.de>
> ---
> This patch creates checkpatch warnings, but I used the style used for the
> existing functions.
> It is based on branch next/soc of the arm-soc tree.
>
>  arch/arm/include/asm/io.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> index 3c597c2..0a8d015 100644
> --- a/arch/arm/include/asm/io.h
> +++ b/arch/arm/include/asm/io.h
> @@ -94,6 +94,13 @@ static inline void __raw_writel(u32 val, volatile void __iomem *addr)
>  		     : "r" (val));
>  }
>
> +static inline void __raw_writeq(u64 val, volatile void __iomem *addr)
> +{
> +	asm volatile("strd %1, %0"

Please use "strd %Q1, %R1, %0" here instead of relying on the
non-standard implicit second register operand.  Although current gcc
versions always allocate 64-bit values in even/odd register pairs, there
is no guarantee that this will always be the case (and llvm has no such
restriction).  In Thumb2, the registers do not need to be consecutive,
so implicitly adding 1 to the first register can silently result in
incorrect code.

For big endian, the register arguments need to be reversed.

> +		     : "+Qo" (*(volatile u64 __force *)addr)

The "o" constraint is not safe here.  The ldrd/strd instructions have a
limited offset range compared to ldr/str, so there is a risk that the
compiler-generated address is invalid.  Using "Q" forces the address to
be a single register, which is always safe.  This is why the 16-bit
versions of these functions use only "Q".  While this is slightly
suboptimal, there is unfortunately no constraint describing the
limitations of ldrd/strd addressing.

> +		     : "r" (val));
> +}

-- 
M?ns Rullg?rd
mans at mansr.com

Re: [PATCH] ARM: asm: add readq/writeq methods

[Alexander Shiyan]

> Matthias Mann <M.Mann@arkona-technologies.de> writes:
> 
> > Add readq/writeq methods for 32 bit ARM to allow transfering 64 bit words over
> > PCIe as a single transfer.
> >
> > Signed-off-by: Matthias Mann <m.mann@arkona-technologies.de>
> > ---
> > This patch creates checkpatch warnings, but I used the style used for the
> > existing functions.
> > It is based on branch next/soc of the arm-soc tree.
> >
> >  arch/arm/include/asm/io.h | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> > index 3c597c2..0a8d015 100644
> > --- a/arch/arm/include/asm/io.h
> > +++ b/arch/arm/include/asm/io.h
> > @@ -94,6 +94,13 @@ static inline void __raw_writel(u32 val, volatile void __iomem *addr)
> >  		     : "r" (val));
> >  }
> >
> > +static inline void __raw_writeq(u64 val, volatile void __iomem *addr)
> > +{
> > +	asm volatile("strd %1, %0"

Fixme, how this will work for ARMv4? Is this supported for systems above v5te
or I missed something?

---

[PATCH] ARM: asm: add readq/writeq methods

[Russell King - ARM Linux]

On Sat, Dec 07, 2013 at 03:11:20PM +0100, Matthias Mann wrote:
> Add readq/writeq methods for 32 bit ARM to allow transfering 64 bit words over
> PCIe as a single transfer.

Since these asm instructions are not universally implemented, use of these
will lead to build time errors if a driver attempts to make use of these
on an ARM architecture which doesn't support it.

Hence, these need to be conditional - since they first appeared (iirc) in
ARMv5, then they should be conditional on

#if __LINUX_ARM_ARCH__ >= 5

However, there's another issue here.  The use of readq/writeq() is
controlled with various preprocessor or configuration symbols:

#if BITS_PER_LONG >= 64
#ifdef CONFIG_64BIT
#ifndef readq
#ifndef writeq

I much prefer the latter two, as it means drivers can individually
implement the lacking support in a way which is appropriate for the device
they're driving - which is especially important if reading a register has
side effects.  In other words, readq() and writeq() is not something that
can be provided by an arch where no 64-bit read/write IO instructions
exist.

This requires that where an architecture provides them, that they be
accompanied by these definitions:

#define readq readq
#define writeq writeq

to prevent the drivers own private definitions of these accessors
conflicting.