[PATCH 0/7] cleanup issues with io{read|write}64

[PATCH 0/7] cleanup issues with io{read|write}64

[Logan Gunthorpe]

Hi,

Presently, the 64bit IO functions are not very usable in drivers because
they are not universally available in all architectures. This leads to
a bunch of hacks in the kernel to work around this. (See the last 3
patches in this series.) As part of my switchtec_ntb submission which
added another one of these warts, Greg asked me to look into fixing
it[1].

So this patchset attempts to solve this issue by filling in the missing
implementations in iomap.c and io.h. After that, the alpha architecture is
the only one I found that also needed a fix for this. Finally, this
patchset removes the hacks that have accumulated in the kernel,
thus far, for working around this.

This set is based off of v4.12-rc6.

Thanks,

Logan

[1] https://marc.info/?l=linux-kernel&m=149774601910663&w=2

Logan Gunthorpe (7):
  drm/tilcdc: don't use volatile with iowrite64
  iomap: implement ioread64 and iowrite64
  asm-generic/io.h: make ioread64 and iowrite64 universally available
  alpha: provide ioread64 and iowrite64 implementations
  ntb: ntb_hw_intel: remove ioread64 and iowrite64 hacks
  drm/tilcdc: clean up ifdef hacks around iowrite64
  crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

 arch/alpha/include/asm/io.h          |  2 ++
 arch/alpha/kernel/io.c               | 18 +++++++++++
 arch/powerpc/include/asm/io.h        |  2 ++
 drivers/crypto/caam/regs.h           | 29 -----------------
 drivers/gpu/drm/tilcdc/tilcdc_regs.h |  8 +----
 drivers/ntb/hw/intel/ntb_hw_intel.c  | 30 -----------------
 include/asm-generic/io.h             | 54 ++++++++++++++++++++++++-------
 include/asm-generic/iomap.h          |  4 ---
 lib/iomap.c                          | 62 ++++++++++++++++++++++++++++++++++++
 9 files changed, 127 insertions(+), 82 deletions(-)

--
2.11.0

[PATCH 1/7] drm/tilcdc: don't use volatile with iowrite64

[Logan Gunthorpe]

This is a prep patch for adding a universal iowrite64.

The patch is to prevent compiler warnings when we add iowrite64 that
would occur because there is an unnecessary volatile in this driver.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: David Airlie <airlied@linux.ie>
---
 drivers/gpu/drm/tilcdc/tilcdc_regs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
index 9d528c0a67a4..e9ce725698a9 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
@@ -131,14 +131,14 @@ static inline void tilcdc_write(struct drm_device *dev, u32 reg, u32 data)
 static inline void tilcdc_write64(struct drm_device *dev, u32 reg, u64 data)
 {
 	struct tilcdc_drm_private *priv = dev->dev_private;
-	volatile void __iomem *addr = priv->mmio + reg;
+	void __iomem *addr = priv->mmio + reg;
 
 #ifdef iowrite64
 	iowrite64(data, addr);
 #else
 	__iowmb();
 	/* This compiles to strd (=64-bit write) on ARM7 */
-	*(volatile u64 __force *)addr = __cpu_to_le64(data);
+	*(u64 __force *)addr = __cpu_to_le64(data);
 #endif
 }
 
-- 
2.11.0

Re: [PATCH 1/7] drm/tilcdc: don't use volatile with iowrite64

[Jyri Sarha]

On 06/22/17 19:48, Logan Gunthorpe wrote:
> This is a prep patch for adding a universal iowrite64.
> 
> The patch is to prevent compiler warnings when we add iowrite64 that
> would occur because there is an unnecessary volatile in this driver.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: David Airlie <airlied@linux.ie>

Acked-by: Jyri Sarha <jsarha@ti.com>

> ---
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> index 9d528c0a67a4..e9ce725698a9 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> @@ -131,14 +131,14 @@ static inline void tilcdc_write(struct drm_device *dev, u32 reg, u32 data)
>  static inline void tilcdc_write64(struct drm_device *dev, u32 reg, u64 data)
>  {
>  	struct tilcdc_drm_private *priv = dev->dev_private;
> -	volatile void __iomem *addr = priv->mmio + reg;
> +	void __iomem *addr = priv->mmio + reg;
>  
>  #ifdef iowrite64
>  	iowrite64(data, addr);
>  #else
>  	__iowmb();
>  	/* This compiles to strd (=64-bit write) on ARM7 */
> -	*(volatile u64 __force *)addr = __cpu_to_le64(data);
> +	*(u64 __force *)addr = __cpu_to_le64(data);
>  #endif
>  }
>  
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/7] drm/tilcdc: don't use volatile with iowrite64

[Jyri Sarha]

On 06/22/17 19:48, Logan Gunthorpe wrote:
> This is a prep patch for adding a universal iowrite64.
> 
> The patch is to prevent compiler warnings when we add iowrite64 that
> would occur because there is an unnecessary volatile in this driver.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: David Airlie <airlied@linux.ie>

Acked-by: Jyri Sarha <jsarha@ti.com>

> ---
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> index 9d528c0a67a4..e9ce725698a9 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> @@ -131,14 +131,14 @@ static inline void tilcdc_write(struct drm_device *dev, u32 reg, u32 data)
>  static inline void tilcdc_write64(struct drm_device *dev, u32 reg, u64 data)
>  {
>  	struct tilcdc_drm_private *priv = dev->dev_private;
> -	volatile void __iomem *addr = priv->mmio + reg;
> +	void __iomem *addr = priv->mmio + reg;
>  
>  #ifdef iowrite64
>  	iowrite64(data, addr);
>  #else
>  	__iowmb();
>  	/* This compiles to strd (=64-bit write) on ARM7 */
> -	*(volatile u64 __force *)addr = __cpu_to_le64(data);
> +	*(u64 __force *)addr = __cpu_to_le64(data);
>  #endif
>  }
>  
> 

Re: [PATCH 1/7] drm/tilcdc: don't use volatile with iowrite64

[Jyri Sarha]

On 06/22/17 19:48, Logan Gunthorpe wrote:
> This is a prep patch for adding a universal iowrite64.
> 
> The patch is to prevent compiler warnings when we add iowrite64 that
> would occur because there is an unnecessary volatile in this driver.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: David Airlie <airlied@linux.ie>

Acked-by: Jyri Sarha <jsarha@ti.com>

> ---
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> index 9d528c0a67a4..e9ce725698a9 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> @@ -131,14 +131,14 @@ static inline void tilcdc_write(struct drm_device *dev, u32 reg, u32 data)
>  static inline void tilcdc_write64(struct drm_device *dev, u32 reg, u64 data)
>  {
>  	struct tilcdc_drm_private *priv = dev->dev_private;
> -	volatile void __iomem *addr = priv->mmio + reg;
> +	void __iomem *addr = priv->mmio + reg;
>  
>  #ifdef iowrite64
>  	iowrite64(data, addr);
>  #else
>  	__iowmb();
>  	/* This compiles to strd (=64-bit write) on ARM7 */
> -	*(volatile u64 __force *)addr = __cpu_to_le64(data);
> +	*(u64 __force *)addr = __cpu_to_le64(data);
>  #endif
>  }
>  
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/7] drm/tilcdc: don't use volatile with iowrite64

[Jyri Sarha]

On 06/22/17 19:48, Logan Gunthorpe wrote:
> This is a prep patch for adding a universal iowrite64.
> 
> The patch is to prevent compiler warnings when we add iowrite64 that
> would occur because there is an unnecessary volatile in this driver.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: David Airlie <airlied@linux.ie>

Acked-by: Jyri Sarha <jsarha@ti.com>

> ---
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> index 9d528c0a67a4..e9ce725698a9 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> @@ -131,14 +131,14 @@ static inline void tilcdc_write(struct drm_device *dev, u32 reg, u32 data)
>  static inline void tilcdc_write64(struct drm_device *dev, u32 reg, u64 data)
>  {
>  	struct tilcdc_drm_private *priv = dev->dev_private;
> -	volatile void __iomem *addr = priv->mmio + reg;
> +	void __iomem *addr = priv->mmio + reg;
>  
>  #ifdef iowrite64
>  	iowrite64(data, addr);
>  #else
>  	__iowmb();
>  	/* This compiles to strd (=64-bit write) on ARM7 */
> -	*(volatile u64 __force *)addr = __cpu_to_le64(data);
> +	*(u64 __force *)addr = __cpu_to_le64(data);
>  #endif
>  }
>  
> 

[PATCH 2/7] iomap: implement ioread64 and iowrite64

[Logan Gunthorpe]

Currently, ioread64 and iowrite64 are not impleminted in the generic
iomap implementation. The prototypes are defined if CONFIG_64BIT is set
but there is no actual implementation.

Seeing the functions are not universally available, they are unusable
for driver developers. This leads to ugly hacks such as those at
the top of

drivers/ntb/hw/intel/ntb_hw_intel.c

This patch adds generic implementations for these functions. We add
the obvious version if readq/writeq are implemented and fall back
to using two io32 calls in cases that don't provide direct 64bit
accesses. Thus making the functions universally available to
configurations with CONFIG_GENERIC_IOMAP=y.

For any pio accesses, the 64bit operations remain unsupported and
simply call bad_io_access in cases readq would be called.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Suresh Warrier <warrier@linux.vnet.ibm.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/io.h |  2 ++
 include/asm-generic/iomap.h   |  4 ---
 lib/iomap.c                   | 62 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 422f99cf9924..11a83667d2c3 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -788,8 +788,10 @@ extern void __iounmap_at(void *ea, unsigned long size);
 
 #define mmio_read16be(addr)		readw_be(addr)
 #define mmio_read32be(addr)		readl_be(addr)
+#define mmio_read64be(addr)		readq_be(addr)
 #define mmio_write16be(val, addr)	writew_be(val, addr)
 #define mmio_write32be(val, addr)	writel_be(val, addr)
+#define mmio_write64be(val, addr)	writeq_be(val, addr)
 #define mmio_insb(addr, dst, count)	readsb(addr, dst, count)
 #define mmio_insw(addr, dst, count)	readsw(addr, dst, count)
 #define mmio_insl(addr, dst, count)	readsl(addr, dst, count)
diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h
index 650fede33c25..43ec4ea9f6f9 100644
--- a/include/asm-generic/iomap.h
+++ b/include/asm-generic/iomap.h
@@ -30,20 +30,16 @@ extern unsigned int ioread16(void __iomem *);
 extern unsigned int ioread16be(void __iomem *);
 extern unsigned int ioread32(void __iomem *);
 extern unsigned int ioread32be(void __iomem *);
-#ifdef CONFIG_64BIT
 extern u64 ioread64(void __iomem *);
 extern u64 ioread64be(void __iomem *);
-#endif
 
 extern void iowrite8(u8, void __iomem *);
 extern void iowrite16(u16, void __iomem *);
 extern void iowrite16be(u16, void __iomem *);
 extern void iowrite32(u32, void __iomem *);
 extern void iowrite32be(u32, void __iomem *);
-#ifdef CONFIG_64BIT
 extern void iowrite64(u64, void __iomem *);
 extern void iowrite64be(u64, void __iomem *);
-#endif
 
 /*
  * "string" versions of the above. Note that they
diff --git a/lib/iomap.c b/lib/iomap.c
index fc3dcb4b238e..e38e036cb52f 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -66,6 +66,7 @@ static void bad_io_access(unsigned long port, const char *access)
 #ifndef mmio_read16be
 #define mmio_read16be(addr) be16_to_cpu(__raw_readw(addr))
 #define mmio_read32be(addr) be32_to_cpu(__raw_readl(addr))
+#define mmio_read64be(addr) be64_to_cpu(__raw_readq(addr))
 #endif
 
 unsigned int ioread8(void __iomem *addr)
@@ -93,11 +94,45 @@ unsigned int ioread32be(void __iomem *addr)
 	IO_COND(addr, return pio_read32be(port), return mmio_read32be(addr));
 	return 0xffffffff;
 }
+
+#ifdef readq
+u64 ioread64(void __iomem *addr)
+{
+	IO_COND(addr, bad_io_access(port, "ioread64"), return readq(addr));
+	return 0xffffffffffffffffLL;
+}
+u64 ioread64be(void __iomem *addr)
+{
+	IO_COND(addr, bad_io_access(port, "ioread64be"),
+		return mmio_read64be(addr));
+	return 0xffffffffffffffffLL;
+}
+#else
+u64 ioread64(void __iomem *addr)
+{
+	u64 low, high;
+
+	low = ioread32(addr);
+	high = ioread32(addr + sizeof(u32));
+	return low | (high << 32);
+}
+u64 ioread64be(void __iomem *addr)
+{
+	u64 low, high;
+
+	low = ioread32be(addr + sizeof(u32));
+	high = ioread32be(addr);
+	return low | (high << 32);
+}
+#endif
+
 EXPORT_SYMBOL(ioread8);
 EXPORT_SYMBOL(ioread16);
 EXPORT_SYMBOL(ioread16be);
 EXPORT_SYMBOL(ioread32);
 EXPORT_SYMBOL(ioread32be);
+EXPORT_SYMBOL(ioread64);
+EXPORT_SYMBOL(ioread64be);
 
 #ifndef pio_write16be
 #define pio_write16be(val,port) outw(swab16(val),port)
@@ -107,6 +142,7 @@ EXPORT_SYMBOL(ioread32be);
 #ifndef mmio_write16be
 #define mmio_write16be(val,port) __raw_writew(be16_to_cpu(val),port)
 #define mmio_write32be(val,port) __raw_writel(be32_to_cpu(val),port)
+#define mmio_write64be(val,port) __raw_writeq(be64_to_cpu(val),port)
 #endif
 
 void iowrite8(u8 val, void __iomem *addr)
@@ -129,11 +165,37 @@ void iowrite32be(u32 val, void __iomem *addr)
 {
 	IO_COND(addr, pio_write32be(val,port), mmio_write32be(val, addr));
 }
+
+#ifdef writeq
+void iowrite64(u64 val, void __iomem *addr)
+{
+	IO_COND(addr, bad_io_access(port, "iowrite64"), writeq(val, addr));
+}
+void iowrite64be(u64 val, void __iomem *addr)
+{
+	IO_COND(addr, bad_io_access(port, "iowrite64be"),
+		mmio_write64be(val, addr));
+}
+#else
+void iowrite64(u64 val, void __iomem *addr)
+{
+	iowrite32(val, addr);
+	iowrite32(val >> 32, addr + sizeof(u32));
+}
+void iowrite64be(u64 val, void __iomem *addr)
+{
+	iowrite32be(val >> 32, addr);
+	iowrite32be(val, addr + sizeof(u32));
+}
+#endif
+
 EXPORT_SYMBOL(iowrite8);
 EXPORT_SYMBOL(iowrite16);
 EXPORT_SYMBOL(iowrite16be);
 EXPORT_SYMBOL(iowrite32);
 EXPORT_SYMBOL(iowrite32be);
+EXPORT_SYMBOL(iowrite64);
+EXPORT_SYMBOL(iowrite64be);
 
 /*
  * These are the "repeat MMIO read/write" functions.
-- 
2.11.0

Re: [PATCH 2/7] iomap: implement ioread64 and iowrite64

[Arnd Bergmann]

> +u64 ioread64(void __iomem *addr)
> +{
> +       u64 low, high;
> +
> +       low = ioread32(addr);
> +       high = ioread32(addr + sizeof(u32));
> +       return low | (high << 32);
> +}
> +u64 ioread64be(void __iomem *addr)
> +{
> +       u64 low, high;
> +
> +       low = ioread32be(addr + sizeof(u32));
> +       high = ioread32be(addr);
> +       return low | (high << 32);
> +}
> +#endif

This hardcodes the behavior of include/linux/io-64-nonatomic-hi-lo.h, which
I find rather confusing, as only about one in five drivers wants this
behavior.

I'd suggest you don't add it in lib/iomap.c at all for 32-bit architectures,
but rather use the same logic that we have for readq/writeq in
io-64-nonatomic-hi-lo.h and io-64-nonatomic-lo-hi.h, adding
{lo_hi,hi_lo}_{ioread,iowrite}{,be} to the same files, and provide
the {ioread,iowrite}{,be} macros only if they have not been defined
at that point.

       Arnd

Re: [PATCH 2/7] iomap: implement ioread64 and iowrite64

[Logan Gunthorpe]

On 6/26/2017 2:43 PM, Arnd Bergmann wrote:
> This hardcodes the behavior of include/linux/io-64-nonatomic-hi-lo.h, which
> I find rather confusing, as only about one in five drivers wants this
> behavior.
>
> I'd suggest you don't add it in lib/iomap.c at all for 32-bit architectures,
> but rather use the same logic that we have for readq/writeq in
> io-64-nonatomic-hi-lo.h and io-64-nonatomic-lo-hi.h, adding
> {lo_hi,hi_lo}_{ioread,iowrite}{,be} to the same files, and provide
> the {ioread,iowrite}{,be} macros only if they have not been defined
> at that point.

Thanks Arnd. Yes, I'm already reworking this patchset to do exactly that.

Logan

[PATCH 3/7] asm-generic/io.h: make ioread64 and iowrite64 universally available

[Logan Gunthorpe]

Currently, ioread64 and iowrite64 are only available io CONFIG_64BIT=y
and CONFIG_GENERIC_IOMAP=n. Thus, seeing the functions are not
universally available, it makes them unusable for driver developers.
This leads to ugly hacks such as those at the top of

drivers/ntb/hw/intel/ntb_hw_intel.c

This patch adds fallback implementations for when CONFIG_64BIT and
CONFIG_GENERIC_IOMAP are not set. These functions use two io32 based
calls to complete the operation.

Note, we do not use the volatile keyword in these functions like the
others in the same file. It is necessary to avoid a compiler warning
on arm.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 include/asm-generic/io.h | 54 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 7ef015eb3403..817edaa3da78 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -585,15 +585,24 @@ static inline u32 ioread32(const volatile void __iomem *addr)
 }
 #endif
 
-#ifdef CONFIG_64BIT
 #ifndef ioread64
 #define ioread64 ioread64
-static inline u64 ioread64(const volatile void __iomem *addr)
+#ifdef readq
+static inline u64 ioread64(const void __iomem *addr)
 {
 	return readq(addr);
 }
+#else
+static inline u64 ioread64(const void __iomem *addr)
+{
+	u64 low, high;
+
+	low = ioread32(addr);
+	high = ioread32(addr + sizeof(u32));
+	return low | (high << 32);
+}
+#endif
 #endif
-#endif /* CONFIG_64BIT */
 
 #ifndef iowrite8
 #define iowrite8 iowrite8
@@ -619,15 +628,21 @@ static inline void iowrite32(u32 value, volatile void __iomem *addr)
 }
 #endif
 
-#ifdef CONFIG_64BIT
 #ifndef iowrite64
 #define iowrite64 iowrite64
-static inline void iowrite64(u64 value, volatile void __iomem *addr)
+#ifdef writeq
+static inline void iowrite64(u64 value, void __iomem *addr)
 {
 	writeq(value, addr);
 }
+#else
+static inline void iowrite64(u64 value, void __iomem *addr)
+{
+	iowrite32(value, addr);
+	iowrite32(value >> 32, addr + sizeof(u32));
+}
+#endif
 #endif
-#endif /* CONFIG_64BIT */
 
 #ifndef ioread16be
 #define ioread16be ioread16be
@@ -645,15 +660,24 @@ static inline u32 ioread32be(const volatile void __iomem *addr)
 }
 #endif
 
-#ifdef CONFIG_64BIT
 #ifndef ioread64be
 #define ioread64be ioread64be
-static inline u64 ioread64be(const volatile void __iomem *addr)
+#ifdef readq
+static inline u64 ioread64be(const void __iomem *addr)
 {
 	return swab64(readq(addr));
 }
+#else
+static inline u64 ioread64be(const void __iomem *addr)
+{
+	u64 low, high;
+
+	low = ioread32be(addr + sizeof(u32));
+	high = ioread32be(addr);
+	return low | (high << 32);
+}
+#endif
 #endif
-#endif /* CONFIG_64BIT */
 
 #ifndef iowrite16be
 #define iowrite16be iowrite16be
@@ -671,15 +695,21 @@ static inline void iowrite32be(u32 value, volatile void __iomem *addr)
 }
 #endif
 
-#ifdef CONFIG_64BIT
 #ifndef iowrite64be
 #define iowrite64be iowrite64be
-static inline void iowrite64be(u64 value, volatile void __iomem *addr)
+#ifdef writeq
+static inline void iowrite64be(u64 value, void __iomem *addr)
 {
 	writeq(swab64(value), addr);
 }
+#else
+static inline void iowrite64be(u64 value, void __iomem *addr)
+{
+	iowrite32be(value >> 32, addr);
+	iowrite32be(value, addr + sizeof(u32));
+}
+#endif
 #endif
-#endif /* CONFIG_64BIT */
 
 #ifndef ioread8_rep
 #define ioread8_rep ioread8_rep
-- 
2.11.0

Re: [PATCH 3/7] asm-generic/io.h: make ioread64 and iowrite64 universally available

[Alan Cox]

On Thu, 22 Jun 2017 10:48:13 -0600
Logan Gunthorpe <logang@deltatee.com> wrote:

> Currently, ioread64 and iowrite64 are only available io CONFIG_64BIT=y
> and CONFIG_GENERIC_IOMAP=n. Thus, seeing the functions are not
> universally available, it makes them unusable for driver developers.
> This leads to ugly hacks such as those at the top of
> 
> drivers/ntb/hw/intel/ntb_hw_intel.c
> 
> This patch adds fallback implementations for when CONFIG_64BIT and
> CONFIG_GENERIC_IOMAP are not set. These functions use two io32 based
> calls to complete the operation.
> 
> Note, we do not use the volatile keyword in these functions like the
> others in the same file. It is necessary to avoid a compiler warning
> on arm.

This is a really really bad idea as per the Alpha comment.

ioread64 and iowrite64 generate a single 64bit bus transaction. There is
hardware where mmio operations have side effects so simply using a pair
of 32bit operations blindly does not work (consider something as trivial
as reading a 64bit performance counter or incrementing pointer).

If a platform doesn't support 64bit I/O operations from the CPU then you
either need to use some kind of platform/architecture specific interface
if present or accept you don't have one.

It's not safe to split it. Possibly for some use cases you could add an

ioread64_maysplit()

but you cannot blindly break ioread64/write64() and expect it to
magically allow you to use drivers that depend upon it.

What btw is the actual ARM compiler warning ? Is the compiler also trying
to tell you it's a bad idea ?

Alan
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/7] asm-generic/io.h: make ioread64 and iowrite64 universally available

[Alan Cox]

On Thu, 22 Jun 2017 10:48:13 -0600
Logan Gunthorpe <logang@deltatee.com> wrote:

> Currently, ioread64 and iowrite64 are only available io CONFIG_64BIT=y
> and CONFIG_GENERIC_IOMAP=n. Thus, seeing the functions are not
> universally available, it makes them unusable for driver developers.
> This leads to ugly hacks such as those at the top of
> 
> drivers/ntb/hw/intel/ntb_hw_intel.c
> 
> This patch adds fallback implementations for when CONFIG_64BIT and
> CONFIG_GENERIC_IOMAP are not set. These functions use two io32 based
> calls to complete the operation.
> 
> Note, we do not use the volatile keyword in these functions like the
> others in the same file. It is necessary to avoid a compiler warning
> on arm.

This is a really really bad idea as per the Alpha comment.

ioread64 and iowrite64 generate a single 64bit bus transaction. There is
hardware where mmio operations have side effects so simply using a pair
of 32bit operations blindly does not work (consider something as trivial
as reading a 64bit performance counter or incrementing pointer).

If a platform doesn't support 64bit I/O operations from the CPU then you
either need to use some kind of platform/architecture specific interface
if present or accept you don't have one.

It's not safe to split it. Possibly for some use cases you could add an

ioread64_maysplit()

but you cannot blindly break ioread64/write64() and expect it to
magically allow you to use drivers that depend upon it.

What btw is the actual ARM compiler warning ? Is the compiler also trying
to tell you it's a bad idea ?

Alan

Re: [PATCH 3/7] asm-generic/io.h: make ioread64 and iowrite64 universally available

[Logan Gunthorpe]

On 6/22/2017 2:14 PM, Alan Cox wrote:
> If a platform doesn't support 64bit I/O operations from the CPU then you
> either need to use some kind of platform/architecture specific interface
> if present or accept you don't have one.

Yes, I understand that.

The thing is that every user that's currently using it right now is 
patching in their own version that splits it on non-64bit systems.

> It's not safe to split it. Possibly for some use cases you could add an
> ioread64_maysplit()

I'm open to doing something like that.

> What btw is the actual ARM compiler warning ? Is the compiler also trying
> to tell you it's a bad idea ?

It's just the compiler noting that you are mixing volatile and 
non-volatile pointers. Strangely some io{read|write}XX use volatile but 
most do not. But it's nothing crazy.

Logan

Re: [PATCH 3/7] asm-generic/io.h: make ioread64 and iowrite64 universally available

[Alan Cox]

On Thu, 22 Jun 2017 14:24:58 -0600
Logan Gunthorpe <logang@deltatee.com> wrote:

> On 6/22/2017 2:14 PM, Alan Cox wrote:
> > If a platform doesn't support 64bit I/O operations from the CPU then you
> > either need to use some kind of platform/architecture specific interface
> > if present or accept you don't have one.  
> 
> Yes, I understand that.
> 
> The thing is that every user that's currently using it right now is 
> patching in their own version that splits it on non-64bit systems.
> 
> > It's not safe to split it. Possibly for some use cases you could add an
> > ioread64_maysplit()  
> 
> I'm open to doing something like that.

I think that makes sense for the platforms with that problem. I'm not
sure there are many that can't do it for mmio at least. 486SX can't do it
and I guess some ARM32 but I think almost everyone else can including
most 32bit x86.

What's more of a problem is a lot of platforms can do 64bit MMIO via
ioread/write64 but not 64bit port I/O, and it's not clear how you
represent that via an ioread/write API that abstracts it away.

Alan
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/7] asm-generic/io.h: make ioread64 and iowrite64 universally available

[Alan Cox]

On Thu, 22 Jun 2017 14:24:58 -0600
Logan Gunthorpe <logang@deltatee.com> wrote:

> On 6/22/2017 2:14 PM, Alan Cox wrote:
> > If a platform doesn't support 64bit I/O operations from the CPU then you
> > either need to use some kind of platform/architecture specific interface
> > if present or accept you don't have one.  
> 
> Yes, I understand that.
> 
> The thing is that every user that's currently using it right now is 
> patching in their own version that splits it on non-64bit systems.
> 
> > It's not safe to split it. Possibly for some use cases you could add an
> > ioread64_maysplit()  
> 
> I'm open to doing something like that.

I think that makes sense for the platforms with that problem. I'm not
sure there are many that can't do it for mmio at least. 486SX can't do it
and I guess some ARM32 but I think almost everyone else can including
most 32bit x86.

What's more of a problem is a lot of platforms can do 64bit MMIO via
ioread/write64 but not 64bit port I/O, and it's not clear how you
represent that via an ioread/write API that abstracts it away.

Alan

Re: [PATCH 3/7] asm-generic/io.h: make ioread64 and iowrite64 universally available

[Logan Gunthorpe]

On 6/22/2017 2:36 PM, Alan Cox wrote:
> I think that makes sense for the platforms with that problem. I'm not
> sure there are many that can't do it for mmio at least. 486SX can't do it
> and I guess some ARM32 but I think almost everyone else can including
> most 32bit x86.
>
> What's more of a problem is a lot of platforms can do 64bit MMIO via
> ioread/write64 but not 64bit port I/O, and it's not clear how you
> represent that via an ioread/write API that abstracts it away.

In Patch 2, we call bad_io_access for anyone trying to do 64bit accesses 
on port I/O.

Logan

[PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations

[Logan Gunthorpe]

Alpha implements its own io operation and doesn't use the
common library. Thus to make ioread64 and iowrite64 globally
available we need to add implementations for alpha.

For this, we simply use calls that chain two 32-bit operations.
(mostly because I don't really understand the alpha architecture.)

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
---
 arch/alpha/include/asm/io.h |  2 ++
 arch/alpha/kernel/io.c      | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
index ff4049155c84..15588092c062 100644
--- a/arch/alpha/include/asm/io.h
+++ b/arch/alpha/include/asm/io.h
@@ -493,8 +493,10 @@ extern inline void writeq(u64 b, volatile void __iomem *addr)
 
 #define ioread16be(p) be16_to_cpu(ioread16(p))
 #define ioread32be(p) be32_to_cpu(ioread32(p))
+#define ioread64be(p) be64_to_cpu(ioread64(p))
 #define iowrite16be(v,p) iowrite16(cpu_to_be16(v), (p))
 #define iowrite32be(v,p) iowrite32(cpu_to_be32(v), (p))
+#define iowrite64be(v,p) iowrite32(cpu_to_be64(v), (p))
 
 #define inb_p		inb
 #define inw_p		inw
diff --git a/arch/alpha/kernel/io.c b/arch/alpha/kernel/io.c
index 19c5875ab398..8c28026f7849 100644
--- a/arch/alpha/kernel/io.c
+++ b/arch/alpha/kernel/io.c
@@ -59,6 +59,24 @@ EXPORT_SYMBOL(iowrite8);
 EXPORT_SYMBOL(iowrite16);
 EXPORT_SYMBOL(iowrite32);
 
+u64 ioread64(void __iomem *addr)
+{
+	u64 low, high;
+
+	low = ioread32(addr);
+	high = ioread32(addr + sizeof(u32));
+	return low | (high << 32);
+}
+
+void iowrite64(u64 val, void __iomem *addr)
+{
+	iowrite32(val, addr);
+	iowrite32(val >> 32, addr + sizeof(u32));
+}
+
+EXPORT_SYMBOL(ioread64);
+EXPORT_SYMBOL(iowrite64);
+
 u8 inb(unsigned long port)
 {
 	return ioread8(ioport_map(port, 1));
-- 
2.11.0

Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations

[Stephen Bates]

> +#define iowrite64be(v,p) iowrite32(cpu_to_be64(v), (p))
 
Logan, thanks for taking this cleanup on. I think this should be iowrite64 not iowrite32?

Stephen

Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations

[Stephen Bates]

PiArI2RlZmluZSBpb3dyaXRlNjRiZSh2LHApIGlvd3JpdGUzMihjcHVfdG9fYmU2NCh2KSwgKHAp
KQ0KIA0KTG9nYW4sIHRoYW5rcyBmb3IgdGFraW5nIHRoaXMgY2xlYW51cCBvbi4gSSB0aGluayB0
aGlzIHNob3VsZCBiZSBpb3dyaXRlNjQgbm90IGlvd3JpdGUzMj8NCg0KU3RlcGhlbg0KDQoNCg==

Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations

[Logan Gunthorpe]

On 6/22/2017 11:29 AM, Stephen  Bates wrote:
>> +#define iowrite64be(v,p) iowrite32(cpu_to_be64(v), (p))
>
> Logan, thanks for taking this cleanup on. I think this should be iowrite64 not iowrite32?

Yup, good catch. Thanks. I'll fix it in a v2 of this series.

Logan

Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations

[Alan Cox]

On Thu, 22 Jun 2017 10:48:14 -0600
Logan Gunthorpe <logang@deltatee.com> wrote:

> Alpha implements its own io operation and doesn't use the
> common library. Thus to make ioread64 and iowrite64 globally
> available we need to add implementations for alpha.
> 
> For this, we simply use calls that chain two 32-bit operations.
> (mostly because I don't really understand the alpha architecture.)

But this does not do the same thing as an ioread64 with regards to
atomicity or side effects on the device. The same is true of the other
hacks. You either have a real 64bit single read/write from MMIO space or
you don't. You can't fake it.


Alan

Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations

[Logan Gunthorpe]

On 6/22/2017 2:08 PM, Alan Cox wrote:
> But this does not do the same thing as an ioread64 with regards to
> atomicity or side effects on the device. The same is true of the other
> hacks. You either have a real 64bit single read/write from MMIO space or
> you don't. You can't fake it.

Yes, I know. But is it not better than having every driver that wants to 
use these functions fake it themselves?

Logan

Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations

[Arnd Bergmann]

On Thu, Jun 22, 2017 at 10:09 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
> On 6/22/2017 2:08 PM, Alan Cox wrote:
>>
>> But this does not do the same thing as an ioread64 with regards to
>> atomicity or side effects on the device. The same is true of the other
>> hacks. You either have a real 64bit single read/write from MMIO space or
>> you don't. You can't fake it.
>
>
> Yes, I know. But is it not better than having every driver that wants to use
> these functions fake it themselves?

Drivers that want a non-atomic variant should include either
include/linux/io-64-nonatomic-hi-lo.h or include/linux/io-64-nonatomic-lo-hi.h
depending on what they need. Drivers that require 64-bit I/O should
probably just depend on CONFIG_64BIT and maybe use readq/writeq.

I see that there are exactly two drivers calling ioread64/iowrite64:
drivers/crypto/caam/ is architecture specific and
drivers/ntb/hw/intel/ntb_hw_intel.c already has a workaround that should
make it build on alpha.

        Arnd
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations

[Arnd Bergmann]

On Thu, Jun 22, 2017 at 10:09 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
> On 6/22/2017 2:08 PM, Alan Cox wrote:
>>
>> But this does not do the same thing as an ioread64 with regards to
>> atomicity or side effects on the device. The same is true of the other
>> hacks. You either have a real 64bit single read/write from MMIO space or
>> you don't. You can't fake it.
>
>
> Yes, I know. But is it not better than having every driver that wants to use
> these functions fake it themselves?

Drivers that want a non-atomic variant should include either
include/linux/io-64-nonatomic-hi-lo.h or include/linux/io-64-nonatomic-lo-hi.h
depending on what they need. Drivers that require 64-bit I/O should
probably just depend on CONFIG_64BIT and maybe use readq/writeq.

I see that there are exactly two drivers calling ioread64/iowrite64:
drivers/crypto/caam/ is architecture specific and
drivers/ntb/hw/intel/ntb_hw_intel.c already has a workaround that should
make it build on alpha.

        Arnd

Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations

[Logan Gunthorpe]

On 6/22/2017 3:03 PM, Arnd Bergmann wrote:
> Drivers that want a non-atomic variant should include either
> include/linux/io-64-nonatomic-hi-lo.h or include/linux/io-64-nonatomic-lo-hi.h
> depending on what they need. Drivers that require 64-bit I/O should
> probably just depend on CONFIG_64BIT and maybe use readq/writeq.

Ok, I will work something like that up.

We'll still need a patch similar to patch 2 (less the non-atomic 
versions) seeing even CONFIG_GENERIC_IOMAP arches don't actually have a 
working ioread64/iowrite64 implementation.

Thanks,

Logan

Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations

[Richard Henderson]

On 06/22/2017 09:48 AM, Logan Gunthorpe wrote:
> Alpha implements its own io operation and doesn't use the
> common library. Thus to make ioread64 and iowrite64 globally
> available we need to add implementations for alpha.
> 
> For this, we simply use calls that chain two 32-bit operations.
> (mostly because I don't really understand the alpha architecture.)

It's not difficult to provide this interface[*].  I believe the only reason I 
didn't do so from the beginning is that it wasn't used.



r~


* At least for systems other than Jensen, which cannot generate 64-bit I/O.  On 
the other hand, Jensen doesn't have PCI (EISA only), and so won't have any 
devices that care.

[PATCH 5/7] ntb: ntb_hw_intel: remove ioread64 and iowrite64 hacks

[Logan Gunthorpe]

Now that ioread64 and iowrite64 are available generically we can
remove the hack at the top of ntb_hw_intel.c that patches them in
when they are not available.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Jon Mason <jdmason@kudzu.us>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Allen Hubbe <Allen.Hubbe@emc.com>
---
 drivers/ntb/hw/intel/ntb_hw_intel.c | 30 ------------------------------
 1 file changed, 30 deletions(-)

diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.c b/drivers/ntb/hw/intel/ntb_hw_intel.c
index c00238491673..56221d540c2b 100644
--- a/drivers/ntb/hw/intel/ntb_hw_intel.c
+++ b/drivers/ntb/hw/intel/ntb_hw_intel.c
@@ -153,35 +153,6 @@ MODULE_PARM_DESC(xeon_b2b_dsd_bar5_addr32,
 static inline enum ntb_topo xeon_ppd_topo(struct intel_ntb_dev *ndev, u8 ppd);
 static int xeon_init_isr(struct intel_ntb_dev *ndev);
 
-#ifndef ioread64
-#ifdef readq
-#define ioread64 readq
-#else
-#define ioread64 _ioread64
-static inline u64 _ioread64(void __iomem *mmio)
-{
-	u64 low, high;
-
-	low = ioread32(mmio);
-	high = ioread32(mmio + sizeof(u32));
-	return low | (high << 32);
-}
-#endif
-#endif
-
-#ifndef iowrite64
-#ifdef writeq
-#define iowrite64 writeq
-#else
-#define iowrite64 _iowrite64
-static inline void _iowrite64(u64 val, void __iomem *mmio)
-{
-	iowrite32(val, mmio);
-	iowrite32(val >> 32, mmio + sizeof(u32));
-}
-#endif
-#endif
-
 static inline int pdev_is_atom(struct pci_dev *pdev)
 {
 	switch (pdev->device) {
@@ -3008,4 +2979,3 @@ static void __exit intel_ntb_pci_driver_exit(void)
 	debugfs_remove_recursive(debugfs_dir);
 }
 module_exit(intel_ntb_pci_driver_exit);
-
-- 
2.11.0

RE: [PATCH 5/7] ntb: ntb_hw_intel: remove ioread64 and iowrite64 hacks

[Jiang, Dave]

> -----Original Message-----
> From: linux-ntb@googlegroups.com [mailto:linux-ntb@googlegroups.com] On Behalf Of Logan Gunthorpe
> Sent: Thursday, June 22, 2017 9:48 AM
> To: linux-kernel@vger.kernel.org; linux-arch@vger.kernel.org; linux-ntb@googlegroups.com; linux-alpha@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org; linux-crypto@vger.kernel.org; dri-devel@lists.freedesktop.org
> Cc: Arnd Bergmann <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Stephen Bates <sbates@raithlin.com>;
> Logan Gunthorpe <logang@deltatee.com>; Jon Mason <jdmason@kudzu.us>; Jiang, Dave <dave.jiang@intel.com>; Allen Hubbe
> <Allen.Hubbe@emc.com>
> Subject: [PATCH 5/7] ntb: ntb_hw_intel: remove ioread64 and iowrite64 hacks
> 
> Now that ioread64 and iowrite64 are available generically we can
> remove the hack at the top of ntb_hw_intel.c that patches them in
> when they are not available.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jon Mason <jdmason@kudzu.us>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Allen Hubbe <Allen.Hubbe@emc.com>

Thanks for doing this Logan. 

Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/hw/intel/ntb_hw_intel.c | 30 ------------------------------
>  1 file changed, 30 deletions(-)
> 
> diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.c b/drivers/ntb/hw/intel/ntb_hw_intel.c
> index c00238491673..56221d540c2b 100644
> --- a/drivers/ntb/hw/intel/ntb_hw_intel.c
> +++ b/drivers/ntb/hw/intel/ntb_hw_intel.c
> @@ -153,35 +153,6 @@ MODULE_PARM_DESC(xeon_b2b_dsd_bar5_addr32,
>  static inline enum ntb_topo xeon_ppd_topo(struct intel_ntb_dev *ndev, u8 ppd);
>  static int xeon_init_isr(struct intel_ntb_dev *ndev);
> 
> -#ifndef ioread64
> -#ifdef readq
> -#define ioread64 readq
> -#else
> -#define ioread64 _ioread64
> -static inline u64 _ioread64(void __iomem *mmio)
> -{
> -	u64 low, high;
> -
> -	low = ioread32(mmio);
> -	high = ioread32(mmio + sizeof(u32));
> -	return low | (high << 32);
> -}
> -#endif
> -#endif
> -
> -#ifndef iowrite64
> -#ifdef writeq
> -#define iowrite64 writeq
> -#else
> -#define iowrite64 _iowrite64
> -static inline void _iowrite64(u64 val, void __iomem *mmio)
> -{
> -	iowrite32(val, mmio);
> -	iowrite32(val >> 32, mmio + sizeof(u32));
> -}
> -#endif
> -#endif
> -
>  static inline int pdev_is_atom(struct pci_dev *pdev)
>  {
>  	switch (pdev->device) {
> @@ -3008,4 +2979,3 @@ static void __exit intel_ntb_pci_driver_exit(void)
>  	debugfs_remove_recursive(debugfs_dir);
>  }
>  module_exit(intel_ntb_pci_driver_exit);
> -
> --
> 2.11.0
> 
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com.
> To post to this group, send email to linux-ntb@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/20170622164817.25515-6-
> logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH 5/7] ntb: ntb_hw_intel: remove ioread64 and iowrite64 hacks

[Jiang, Dave]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogbGludXgtbnRiQGdvb2ds
ZWdyb3Vwcy5jb20gW21haWx0bzpsaW51eC1udGJAZ29vZ2xlZ3JvdXBzLmNvbV0gT24gQmVoYWxm
IE9mIExvZ2FuIEd1bnRob3JwZQ0KPiBTZW50OiBUaHVyc2RheSwgSnVuZSAyMiwgMjAxNyA5OjQ4
IEFNDQo+IFRvOiBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOyBsaW51eC1hcmNoQHZnZXIu
a2VybmVsLm9yZzsgbGludXgtbnRiQGdvb2dsZWdyb3Vwcy5jb207IGxpbnV4LWFscGhhQHZnZXIu
a2VybmVsLm9yZzsgbGludXhwcGMtDQo+IGRldkBsaXN0cy5vemxhYnMub3JnOyBsaW51eC1jcnlw
dG9Admdlci5rZXJuZWwub3JnOyBkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnDQo+IENj
OiBBcm5kIEJlcmdtYW5uIDxhcm5kQGFybmRiLmRlPjsgR3JlZyBLcm9haC1IYXJ0bWFuIDxncmVn
a2hAbGludXhmb3VuZGF0aW9uLm9yZz47IFN0ZXBoZW4gQmF0ZXMgPHNiYXRlc0ByYWl0aGxpbi5j
b20+Ow0KPiBMb2dhbiBHdW50aG9ycGUgPGxvZ2FuZ0BkZWx0YXRlZS5jb20+OyBKb24gTWFzb24g
PGpkbWFzb25Aa3VkenUudXM+OyBKaWFuZywgRGF2ZSA8ZGF2ZS5qaWFuZ0BpbnRlbC5jb20+OyBB
bGxlbiBIdWJiZQ0KPiA8QWxsZW4uSHViYmVAZW1jLmNvbT4NCj4gU3ViamVjdDogW1BBVENIIDUv
N10gbnRiOiBudGJfaHdfaW50ZWw6IHJlbW92ZSBpb3JlYWQ2NCBhbmQgaW93cml0ZTY0IGhhY2tz
DQo+IA0KPiBOb3cgdGhhdCBpb3JlYWQ2NCBhbmQgaW93cml0ZTY0IGFyZSBhdmFpbGFibGUgZ2Vu
ZXJpY2FsbHkgd2UgY2FuDQo+IHJlbW92ZSB0aGUgaGFjayBhdCB0aGUgdG9wIG9mIG50Yl9od19p
bnRlbC5jIHRoYXQgcGF0Y2hlcyB0aGVtIGluDQo+IHdoZW4gdGhleSBhcmUgbm90IGF2YWlsYWJs
ZS4NCj4gDQo+IFNpZ25lZC1vZmYtYnk6IExvZ2FuIEd1bnRob3JwZSA8bG9nYW5nQGRlbHRhdGVl
LmNvbT4NCj4gQ2M6IEpvbiBNYXNvbiA8amRtYXNvbkBrdWR6dS51cz4NCj4gQ2M6IERhdmUgSmlh
bmcgPGRhdmUuamlhbmdAaW50ZWwuY29tPg0KPiBDYzogQWxsZW4gSHViYmUgPEFsbGVuLkh1YmJl
QGVtYy5jb20+DQoNClRoYW5rcyBmb3IgZG9pbmcgdGhpcyBMb2dhbi4gDQoNCkFja2VkLWJ5OiBE
YXZlIEppYW5nIDxkYXZlLmppYW5nQGludGVsLmNvbT4NCg0KPiAtLS0NCj4gIGRyaXZlcnMvbnRi
L2h3L2ludGVsL250Yl9od19pbnRlbC5jIHwgMzAgLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tDQo+ICAxIGZpbGUgY2hhbmdlZCwgMzAgZGVsZXRpb25zKC0pDQo+IA0KPiBkaWZmIC0tZ2l0
IGEvZHJpdmVycy9udGIvaHcvaW50ZWwvbnRiX2h3X2ludGVsLmMgYi9kcml2ZXJzL250Yi9ody9p
bnRlbC9udGJfaHdfaW50ZWwuYw0KPiBpbmRleCBjMDAyMzg0OTE2NzMuLjU2MjIxZDU0MGMyYiAx
MDA2NDQNCj4gLS0tIGEvZHJpdmVycy9udGIvaHcvaW50ZWwvbnRiX2h3X2ludGVsLmMNCj4gKysr
IGIvZHJpdmVycy9udGIvaHcvaW50ZWwvbnRiX2h3X2ludGVsLmMNCj4gQEAgLTE1MywzNSArMTUz
LDYgQEAgTU9EVUxFX1BBUk1fREVTQyh4ZW9uX2IyYl9kc2RfYmFyNV9hZGRyMzIsDQo+ICBzdGF0
aWMgaW5saW5lIGVudW0gbnRiX3RvcG8geGVvbl9wcGRfdG9wbyhzdHJ1Y3QgaW50ZWxfbnRiX2Rl
diAqbmRldiwgdTggcHBkKTsNCj4gIHN0YXRpYyBpbnQgeGVvbl9pbml0X2lzcihzdHJ1Y3QgaW50
ZWxfbnRiX2RldiAqbmRldik7DQo+IA0KPiAtI2lmbmRlZiBpb3JlYWQ2NA0KPiAtI2lmZGVmIHJl
YWRxDQo+IC0jZGVmaW5lIGlvcmVhZDY0IHJlYWRxDQo+IC0jZWxzZQ0KPiAtI2RlZmluZSBpb3Jl
YWQ2NCBfaW9yZWFkNjQNCj4gLXN0YXRpYyBpbmxpbmUgdTY0IF9pb3JlYWQ2NCh2b2lkIF9faW9t
ZW0gKm1taW8pDQo+IC17DQo+IC0JdTY0IGxvdywgaGlnaDsNCj4gLQ0KPiAtCWxvdyA9IGlvcmVh
ZDMyKG1taW8pOw0KPiAtCWhpZ2ggPSBpb3JlYWQzMihtbWlvICsgc2l6ZW9mKHUzMikpOw0KPiAt
CXJldHVybiBsb3cgfCAoaGlnaCA8PCAzMik7DQo+IC19DQo+IC0jZW5kaWYNCj4gLSNlbmRpZg0K
PiAtDQo+IC0jaWZuZGVmIGlvd3JpdGU2NA0KPiAtI2lmZGVmIHdyaXRlcQ0KPiAtI2RlZmluZSBp
b3dyaXRlNjQgd3JpdGVxDQo+IC0jZWxzZQ0KPiAtI2RlZmluZSBpb3dyaXRlNjQgX2lvd3JpdGU2
NA0KPiAtc3RhdGljIGlubGluZSB2b2lkIF9pb3dyaXRlNjQodTY0IHZhbCwgdm9pZCBfX2lvbWVt
ICptbWlvKQ0KPiAtew0KPiAtCWlvd3JpdGUzMih2YWwsIG1taW8pOw0KPiAtCWlvd3JpdGUzMih2
YWwgPj4gMzIsIG1taW8gKyBzaXplb2YodTMyKSk7DQo+IC19DQo+IC0jZW5kaWYNCj4gLSNlbmRp
Zg0KPiAtDQo+ICBzdGF0aWMgaW5saW5lIGludCBwZGV2X2lzX2F0b20oc3RydWN0IHBjaV9kZXYg
KnBkZXYpDQo+ICB7DQo+ICAJc3dpdGNoIChwZGV2LT5kZXZpY2UpIHsNCj4gQEAgLTMwMDgsNCAr
Mjk3OSwzIEBAIHN0YXRpYyB2b2lkIF9fZXhpdCBpbnRlbF9udGJfcGNpX2RyaXZlcl9leGl0KHZv
aWQpDQo+ICAJZGVidWdmc19yZW1vdmVfcmVjdXJzaXZlKGRlYnVnZnNfZGlyKTsNCj4gIH0NCj4g
IG1vZHVsZV9leGl0KGludGVsX250Yl9wY2lfZHJpdmVyX2V4aXQpOw0KPiAtDQo+IC0tDQo+IDIu
MTEuMA0KPiANCj4gLS0NCj4gWW91IHJlY2VpdmVkIHRoaXMgbWVzc2FnZSBiZWNhdXNlIHlvdSBh
cmUgc3Vic2NyaWJlZCB0byB0aGUgR29vZ2xlIEdyb3VwcyAibGludXgtbnRiIiBncm91cC4NCj4g
VG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGdyb3VwIGFuZCBzdG9wIHJlY2VpdmluZyBlbWFpbHMg
ZnJvbSBpdCwgc2VuZCBhbiBlbWFpbCB0byBsaW51eC1udGIrdW5zdWJzY3JpYmVAZ29vZ2xlZ3Jv
dXBzLmNvbS4NCj4gVG8gcG9zdCB0byB0aGlzIGdyb3VwLCBzZW5kIGVtYWlsIHRvIGxpbnV4LW50
YkBnb29nbGVncm91cHMuY29tLg0KPiBUbyB2aWV3IHRoaXMgZGlzY3Vzc2lvbiBvbiB0aGUgd2Vi
IHZpc2l0IGh0dHBzOi8vZ3JvdXBzLmdvb2dsZS5jb20vZC9tc2dpZC9saW51eC1udGIvMjAxNzA2
MjIxNjQ4MTcuMjU1MTUtNi0NCj4gbG9nYW5nJTQwZGVsdGF0ZWUuY29tLg0KPiBGb3IgbW9yZSBv
cHRpb25zLCB2aXNpdCBodHRwczovL2dyb3Vwcy5nb29nbGUuY29tL2Qvb3B0b3V0Lg0K

RE: [PATCH 5/7] ntb: ntb_hw_intel: remove ioread64 and iowrite64 hacks

[Jiang, Dave]

> -----Original Message-----
> From: linux-ntb@googlegroups.com [mailto:linux-ntb@googlegroups.com] On Behalf Of Logan Gunthorpe
> Sent: Thursday, June 22, 2017 9:48 AM
> To: linux-kernel@vger.kernel.org; linux-arch@vger.kernel.org; linux-ntb@googlegroups.com; linux-alpha@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org; linux-crypto@vger.kernel.org; dri-devel@lists.freedesktop.org
> Cc: Arnd Bergmann <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Stephen Bates <sbates@raithlin.com>;
> Logan Gunthorpe <logang@deltatee.com>; Jon Mason <jdmason@kudzu.us>; Jiang, Dave <dave.jiang@intel.com>; Allen Hubbe
> <Allen.Hubbe@emc.com>
> Subject: [PATCH 5/7] ntb: ntb_hw_intel: remove ioread64 and iowrite64 hacks
> 
> Now that ioread64 and iowrite64 are available generically we can
> remove the hack at the top of ntb_hw_intel.c that patches them in
> when they are not available.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jon Mason <jdmason@kudzu.us>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Allen Hubbe <Allen.Hubbe@emc.com>

Thanks for doing this Logan. 

Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/hw/intel/ntb_hw_intel.c | 30 ------------------------------
>  1 file changed, 30 deletions(-)
> 
> diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.c b/drivers/ntb/hw/intel/ntb_hw_intel.c
> index c00238491673..56221d540c2b 100644
> --- a/drivers/ntb/hw/intel/ntb_hw_intel.c
> +++ b/drivers/ntb/hw/intel/ntb_hw_intel.c
> @@ -153,35 +153,6 @@ MODULE_PARM_DESC(xeon_b2b_dsd_bar5_addr32,
>  static inline enum ntb_topo xeon_ppd_topo(struct intel_ntb_dev *ndev, u8 ppd);
>  static int xeon_init_isr(struct intel_ntb_dev *ndev);
> 
> -#ifndef ioread64
> -#ifdef readq
> -#define ioread64 readq
> -#else
> -#define ioread64 _ioread64
> -static inline u64 _ioread64(void __iomem *mmio)
> -{
> -	u64 low, high;
> -
> -	low = ioread32(mmio);
> -	high = ioread32(mmio + sizeof(u32));
> -	return low | (high << 32);
> -}
> -#endif
> -#endif
> -
> -#ifndef iowrite64
> -#ifdef writeq
> -#define iowrite64 writeq
> -#else
> -#define iowrite64 _iowrite64
> -static inline void _iowrite64(u64 val, void __iomem *mmio)
> -{
> -	iowrite32(val, mmio);
> -	iowrite32(val >> 32, mmio + sizeof(u32));
> -}
> -#endif
> -#endif
> -
>  static inline int pdev_is_atom(struct pci_dev *pdev)
>  {
>  	switch (pdev->device) {
> @@ -3008,4 +2979,3 @@ static void __exit intel_ntb_pci_driver_exit(void)
>  	debugfs_remove_recursive(debugfs_dir);
>  }
>  module_exit(intel_ntb_pci_driver_exit);
> -
> --
> 2.11.0
> 
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com.
> To post to this group, send email to linux-ntb@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/20170622164817.25515-6-
> logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

[PATCH 6/7] drm/tilcdc: clean up ifdef hacks around iowrite64

[Logan Gunthorpe]

Now that we can expect iowrite64 to always exist the hack is no longer
necessary so we just call iowrite64 directly.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: David Airlie <airlied@linux.ie>
---
 drivers/gpu/drm/tilcdc/tilcdc_regs.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
index e9ce725698a9..0b901405f30a 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
@@ -133,13 +133,7 @@ static inline void tilcdc_write64(struct drm_device *dev, u32 reg, u64 data)
 	struct tilcdc_drm_private *priv = dev->dev_private;
 	void __iomem *addr = priv->mmio + reg;
 
-#ifdef iowrite64
 	iowrite64(data, addr);
-#else
-	__iowmb();
-	/* This compiles to strd (=64-bit write) on ARM7 */
-	*(u64 __force *)addr = __cpu_to_le64(data);
-#endif
 }
 
 static inline u32 tilcdc_read(struct drm_device *dev, u32 reg)
-- 
2.11.0

Re: [PATCH 6/7] drm/tilcdc: clean up ifdef hacks around iowrite64

[Jyri Sarha]

On 06/22/17 19:48, Logan Gunthorpe wrote:
> Now that we can expect iowrite64 to always exist the hack is no longer
> necessary so we just call iowrite64 directly.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: David Airlie <airlied@linux.ie>

Acked-by: Jyri Sarha <jsarha@ti.com>

And thanks!

> ---
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> index e9ce725698a9..0b901405f30a 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> @@ -133,13 +133,7 @@ static inline void tilcdc_write64(struct drm_device *dev, u32 reg, u64 data)
>  	struct tilcdc_drm_private *priv = dev->dev_private;
>  	void __iomem *addr = priv->mmio + reg;
>  
> -#ifdef iowrite64
>  	iowrite64(data, addr);
> -#else
> -	__iowmb();
> -	/* This compiles to strd (=64-bit write) on ARM7 */
> -	*(u64 __force *)addr = __cpu_to_le64(data);
> -#endif
>  }
>  
>  static inline u32 tilcdc_read(struct drm_device *dev, u32 reg)
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm/tilcdc: clean up ifdef hacks around iowrite64

[Jyri Sarha]

On 06/22/17 19:48, Logan Gunthorpe wrote:
> Now that we can expect iowrite64 to always exist the hack is no longer
> necessary so we just call iowrite64 directly.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: David Airlie <airlied@linux.ie>

Acked-by: Jyri Sarha <jsarha@ti.com>

And thanks!

> ---
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> index e9ce725698a9..0b901405f30a 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> @@ -133,13 +133,7 @@ static inline void tilcdc_write64(struct drm_device *dev, u32 reg, u64 data)
>  	struct tilcdc_drm_private *priv = dev->dev_private;
>  	void __iomem *addr = priv->mmio + reg;
>  
> -#ifdef iowrite64
>  	iowrite64(data, addr);
> -#else
> -	__iowmb();
> -	/* This compiles to strd (=64-bit write) on ARM7 */
> -	*(u64 __force *)addr = __cpu_to_le64(data);
> -#endif
>  }
>  
>  static inline u32 tilcdc_read(struct drm_device *dev, u32 reg)
> 

Re: [PATCH 6/7] drm/tilcdc: clean up ifdef hacks around iowrite64

[Jyri Sarha]

On 06/22/17 19:48, Logan Gunthorpe wrote:
> Now that we can expect iowrite64 to always exist the hack is no longer
> necessary so we just call iowrite64 directly.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: David Airlie <airlied@linux.ie>

Acked-by: Jyri Sarha <jsarha@ti.com>

And thanks!

> ---
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> index e9ce725698a9..0b901405f30a 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> @@ -133,13 +133,7 @@ static inline void tilcdc_write64(struct drm_device *dev, u32 reg, u64 data)
>  	struct tilcdc_drm_private *priv = dev->dev_private;
>  	void __iomem *addr = priv->mmio + reg;
>  
> -#ifdef iowrite64
>  	iowrite64(data, addr);
> -#else
> -	__iowmb();
> -	/* This compiles to strd (=64-bit write) on ARM7 */
> -	*(u64 __force *)addr = __cpu_to_le64(data);
> -#endif
>  }
>  
>  static inline u32 tilcdc_read(struct drm_device *dev, u32 reg)
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm/tilcdc: clean up ifdef hacks around iowrite64

[Jyri Sarha]

On 06/22/17 19:48, Logan Gunthorpe wrote:
> Now that we can expect iowrite64 to always exist the hack is no longer
> necessary so we just call iowrite64 directly.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: David Airlie <airlied@linux.ie>

Acked-by: Jyri Sarha <jsarha@ti.com>

And thanks!

> ---
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> index e9ce725698a9..0b901405f30a 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> @@ -133,13 +133,7 @@ static inline void tilcdc_write64(struct drm_device *dev, u32 reg, u64 data)
>  	struct tilcdc_drm_private *priv = dev->dev_private;
>  	void __iomem *addr = priv->mmio + reg;
>  
> -#ifdef iowrite64
>  	iowrite64(data, addr);
> -#else
> -	__iowmb();
> -	/* This compiles to strd (=64-bit write) on ARM7 */
> -	*(u64 __force *)addr = __cpu_to_le64(data);
> -#endif
>  }
>  
>  static inline u32 tilcdc_read(struct drm_device *dev, u32 reg)
> 

Re: [PATCH 6/7] drm/tilcdc: clean up ifdef hacks around iowrite64

[Logan Gunthorpe]

Hi Jyri,

Thanks for the ack. However, I'm reworking this patch set to use the
include/linux/io-64-nonatomic* headers which will explicitly devolve
into two 32-bit transfers. It's not clear whether this is appropriate
for the tilcdc driver as it was never setup to use 32-bit transfers
(unlike the others I had patched).

If you think it's ok, I can still patch this driver to use the
non-atomic headers. Otherwise I can leave it out. Please let me know.

Thanks,

Logan


On 26/06/17 02:55 AM, Jyri Sarha wrote:
> Acked-by: Jyri Sarha <jsarha@ti.com>
> 
> And thanks!
> 
>> ---
>>  drivers/gpu/drm/tilcdc/tilcdc_regs.h | 6 ------
>>  1 file changed, 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
>> index e9ce725698a9..0b901405f30a 100644
>> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
>> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
>> @@ -133,13 +133,7 @@ static inline void tilcdc_write64(struct drm_device *dev, u32 reg, u64 data)
>>  	struct tilcdc_drm_private *priv = dev->dev_private;
>>  	void __iomem *addr = priv->mmio + reg;
>>  
>> -#ifdef iowrite64
>>  	iowrite64(data, addr);
>> -#else
>> -	__iowmb();
>> -	/* This compiles to strd (=64-bit write) on ARM7 */
>> -	*(u64 __force *)addr = __cpu_to_le64(data);
>> -#endif
>>  }
>>  
>>  static inline u32 tilcdc_read(struct drm_device *dev, u32 reg)
>>
> 

Re: [PATCH 6/7] drm/tilcdc: clean up ifdef hacks around iowrite64

[Arnd Bergmann]

On Mon, Jun 26, 2017 at 6:26 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
> Hi Jyri,
>
> Thanks for the ack. However, I'm reworking this patch set to use the
> include/linux/io-64-nonatomic* headers which will explicitly devolve
> into two 32-bit transfers. It's not clear whether this is appropriate
> for the tilcdc driver as it was never setup to use 32-bit transfers
> (unlike the others I had patched).
>
> If you think it's ok, I can still patch this driver to use the
> non-atomic headers. Otherwise I can leave it out. Please let me know.

You'd have to first figure out whether this device is of the lo-hi
or the hi-lo variant, or doesn't allow the I/O to be split at all.

Note that we could theoretically define ARM to use strd/ldrd
for writeq/readq, but I would expect that to be wrong with many
other devices that can use the existing io-64-nonatomic headers.

The comment in set_scanout() suggests that we actually do rely
on the write64 to be atomic, so we probably don't want to change
this driver.

         Arnd

[PATCH 7/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

[Logan Gunthorpe]

Now that ioread64 and iowrite64 are always available we don't
need the ugly ifdefs to change their implementation when they
are not.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: "Horia Geantă" <horia.geanta@nxp.com>
Cc: Dan Douglass <dan.douglass@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
---
 drivers/crypto/caam/regs.h | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 84d2f838a063..26fc19dd0c39 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -134,7 +134,6 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set)
  *    base + 0x0000 : least-significant 32 bits
  *    base + 0x0004 : most-significant 32 bits
  */
-#ifdef CONFIG_64BIT
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
 	if (caam_little_end)
@@ -151,34 +150,6 @@ static inline u64 rd_reg64(void __iomem *reg)
 		return ioread64be(reg);
 }
 
-#else /* CONFIG_64BIT */
-static inline void wr_reg64(void __iomem *reg, u64 data)
-{
-#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
-	if (caam_little_end) {
-		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
-		wr_reg32((u32 __iomem *)(reg), data);
-	} else
-#endif
-	{
-		wr_reg32((u32 __iomem *)(reg), data >> 32);
-		wr_reg32((u32 __iomem *)(reg) + 1, data);
-	}
-}
-
-static inline u64 rd_reg64(void __iomem *reg)
-{
-#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
-	if (caam_little_end)
-		return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
-			(u64)rd_reg32((u32 __iomem *)(reg)));
-	else
-#endif
-		return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
-			(u64)rd_reg32((u32 __iomem *)(reg) + 1));
-}
-#endif /* CONFIG_64BIT  */
-
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 #ifdef CONFIG_SOC_IMX7D
 #define cpu_to_caam_dma(value) \
-- 
2.11.0

[PATCH 7/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

[Logan Gunthorpe]

Now that ioread64 and iowrite64 are always available we don't
need the ugly ifdefs to change their implementation when they
are not.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: "Horia Geantă" <horia.geanta@nxp.com>
Cc: Dan Douglass <dan.douglass@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
---
 drivers/crypto/caam/regs.h | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 84d2f838a063..26fc19dd0c39 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -134,7 +134,6 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set)
  *    base + 0x0000 : least-significant 32 bits
  *    base + 0x0004 : most-significant 32 bits
  */
-#ifdef CONFIG_64BIT
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
 	if (caam_little_end)
@@ -151,34 +150,6 @@ static inline u64 rd_reg64(void __iomem *reg)
 		return ioread64be(reg);
 }
 
-#else /* CONFIG_64BIT */
-static inline void wr_reg64(void __iomem *reg, u64 data)
-{
-#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
-	if (caam_little_end) {
-		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
-		wr_reg32((u32 __iomem *)(reg), data);
-	} else
-#endif
-	{
-		wr_reg32((u32 __iomem *)(reg), data >> 32);
-		wr_reg32((u32 __iomem *)(reg) + 1, data);
-	}
-}
-
-static inline u64 rd_reg64(void __iomem *reg)
-{
-#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
-	if (caam_little_end)
-		return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
-			(u64)rd_reg32((u32 __iomem *)(reg)));
-	else
-#endif
-		return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
-			(u64)rd_reg32((u32 __iomem *)(reg) + 1));
-}
-#endif /* CONFIG_64BIT  */
-
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 #ifdef CONFIG_SOC_IMX7D
 #define cpu_to_caam_dma(value) \
-- 
2.11.0

Re: [PATCH 7/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

[Horia Geantă]

On 6/22/2017 7:49 PM, Logan Gunthorpe wrote:
> Now that ioread64 and iowrite64 are always available we don't
> need the ugly ifdefs to change their implementation when they
> are not.
> 
Thanks Logan.

Note however this is not equivalent - it changes the behaviour, since
CAAM engine on i.MX6S/SL/D/Q platforms is broken in terms of 64-bit
register endianness - see CONFIG_CRYPTO_DEV_FSL_CAAM_IMX usage in code
you are removing.

[Yes, current code has its problems, as it does not differentiate b/w
i.MX platforms with and without the (unofficial) erratum, but this
should be fixed separately.]

Below is the change that would keep current logic - still forcing i.MX
to write CAAM 64-bit registers in BE even if the engine is LE (yes, diff
is doing a poor job).

Horia

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 84d2f838a063..b893ebb24e65 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -134,50 +134,25 @@ static inline void clrsetbits_32(void __iomem
*reg, u32 clear, u32 set)
  *    base + 0x0000 : least-significant 32 bits
  *    base + 0x0004 : most-significant 32 bits
  */
-#ifdef CONFIG_64BIT
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
+#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
        if (caam_little_end)
                iowrite64(data, reg);
        else
-               iowrite64be(data, reg);
-}
-
-static inline u64 rd_reg64(void __iomem *reg)
-{
-       if (caam_little_end)
-               return ioread64(reg);
-       else
-               return ioread64be(reg);
-}
-
-#else /* CONFIG_64BIT */
-static inline void wr_reg64(void __iomem *reg, u64 data)
-{
-#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
-       if (caam_little_end) {
-               wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
-               wr_reg32((u32 __iomem *)(reg), data);
-       } else
 #endif
-       {
-               wr_reg32((u32 __iomem *)(reg), data >> 32);
-               wr_reg32((u32 __iomem *)(reg) + 1, data);
-       }
+               iowrite64be(data, reg);
 }

 static inline u64 rd_reg64(void __iomem *reg)
 {
 #ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
        if (caam_little_end)
-               return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
-                       (u64)rd_reg32((u32 __iomem *)(reg)));
+               return ioread64(reg);
        else
 #endif
-               return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
-                       (u64)rd_reg32((u32 __iomem *)(reg) + 1));
+               return ioread64be(reg);
 }
-#endif /* CONFIG_64BIT  */

 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 #ifdef CONFIG_SOC_IMX7D


> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: "Horia Geantă" <horia.geanta@nxp.com>
> Cc: Dan Douglass <dan.douglass@nxp.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> ---
>  drivers/crypto/caam/regs.h | 29 -----------------------------
>  1 file changed, 29 deletions(-)
> 
> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
> index 84d2f838a063..26fc19dd0c39 100644
> --- a/drivers/crypto/caam/regs.h
> +++ b/drivers/crypto/caam/regs.h
> @@ -134,7 +134,6 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set)
>   *    base + 0x0000 : least-significant 32 bits
>   *    base + 0x0004 : most-significant 32 bits
>   */
> -#ifdef CONFIG_64BIT
>  static inline void wr_reg64(void __iomem *reg, u64 data)
>  {
>  	if (caam_little_end)
> @@ -151,34 +150,6 @@ static inline u64 rd_reg64(void __iomem *reg)
>  		return ioread64be(reg);
>  }
>  
> -#else /* CONFIG_64BIT */
> -static inline void wr_reg64(void __iomem *reg, u64 data)
> -{
> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
> -	if (caam_little_end) {
> -		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
> -		wr_reg32((u32 __iomem *)(reg), data);
> -	} else
> -#endif
> -	{
> -		wr_reg32((u32 __iomem *)(reg), data >> 32);
> -		wr_reg32((u32 __iomem *)(reg) + 1, data);
> -	}
> -}
> -
> -static inline u64 rd_reg64(void __iomem *reg)
> -{
> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
> -	if (caam_little_end)
> -		return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
> -			(u64)rd_reg32((u32 __iomem *)(reg)));
> -	else
> -#endif
> -		return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
> -			(u64)rd_reg32((u32 __iomem *)(reg) + 1));
> -}
> -#endif /* CONFIG_64BIT  */
> -
>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>  #ifdef CONFIG_SOC_IMX7D
>  #define cpu_to_caam_dma(value) \
> 

Re: [PATCH 7/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

[Horia Geantă]

On 6/22/2017 7:49 PM, Logan Gunthorpe wrote:
> Now that ioread64 and iowrite64 are always available we don't
> need the ugly ifdefs to change their implementation when they
> are not.
> 
Thanks Logan.

Note however this is not equivalent - it changes the behaviour, since
CAAM engine on i.MX6S/SL/D/Q platforms is broken in terms of 64-bit
register endianness - see CONFIG_CRYPTO_DEV_FSL_CAAM_IMX usage in code
you are removing.

[Yes, current code has its problems, as it does not differentiate b/w
i.MX platforms with and without the (unofficial) erratum, but this
should be fixed separately.]

Below is the change that would keep current logic - still forcing i.MX
to write CAAM 64-bit registers in BE even if the engine is LE (yes, diff
is doing a poor job).

Horia

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 84d2f838a063..b893ebb24e65 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -134,50 +134,25 @@ static inline void clrsetbits_32(void __iomem
*reg, u32 clear, u32 set)
  *    base + 0x0000 : least-significant 32 bits
  *    base + 0x0004 : most-significant 32 bits
  */
-#ifdef CONFIG_64BIT
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
+#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
        if (caam_little_end)
                iowrite64(data, reg);
        else
-               iowrite64be(data, reg);
-}
-
-static inline u64 rd_reg64(void __iomem *reg)
-{
-       if (caam_little_end)
-               return ioread64(reg);
-       else
-               return ioread64be(reg);
-}
-
-#else /* CONFIG_64BIT */
-static inline void wr_reg64(void __iomem *reg, u64 data)
-{
-#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
-       if (caam_little_end) {
-               wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
-               wr_reg32((u32 __iomem *)(reg), data);
-       } else
 #endif
-       {
-               wr_reg32((u32 __iomem *)(reg), data >> 32);
-               wr_reg32((u32 __iomem *)(reg) + 1, data);
-       }
+               iowrite64be(data, reg);
 }

 static inline u64 rd_reg64(void __iomem *reg)
 {
 #ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
        if (caam_little_end)
-               return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
-                       (u64)rd_reg32((u32 __iomem *)(reg)));
+               return ioread64(reg);
        else
 #endif
-               return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
-                       (u64)rd_reg32((u32 __iomem *)(reg) + 1));
+               return ioread64be(reg);
 }
-#endif /* CONFIG_64BIT  */

 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 #ifdef CONFIG_SOC_IMX7D


> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: "Horia Geantã" <horia.geanta@nxp.com>
> Cc: Dan Douglass <dan.douglass@nxp.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> ---
>  drivers/crypto/caam/regs.h | 29 -----------------------------
>  1 file changed, 29 deletions(-)
> 
> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
> index 84d2f838a063..26fc19dd0c39 100644
> --- a/drivers/crypto/caam/regs.h
> +++ b/drivers/crypto/caam/regs.h
> @@ -134,7 +134,6 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set)
>   *    base + 0x0000 : least-significant 32 bits
>   *    base + 0x0004 : most-significant 32 bits
>   */
> -#ifdef CONFIG_64BIT
>  static inline void wr_reg64(void __iomem *reg, u64 data)
>  {
>  	if (caam_little_end)
> @@ -151,34 +150,6 @@ static inline u64 rd_reg64(void __iomem *reg)
>  		return ioread64be(reg);
>  }
>  
> -#else /* CONFIG_64BIT */
> -static inline void wr_reg64(void __iomem *reg, u64 data)
> -{
> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
> -	if (caam_little_end) {
> -		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
> -		wr_reg32((u32 __iomem *)(reg), data);
> -	} else
> -#endif
> -	{
> -		wr_reg32((u32 __iomem *)(reg), data >> 32);
> -		wr_reg32((u32 __iomem *)(reg) + 1, data);
> -	}
> -}
> -
> -static inline u64 rd_reg64(void __iomem *reg)
> -{
> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
> -	if (caam_little_end)
> -		return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
> -			(u64)rd_reg32((u32 __iomem *)(reg)));
> -	else
> -#endif
> -		return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
> -			(u64)rd_reg32((u32 __iomem *)(reg) + 1));
> -}
> -#endif /* CONFIG_64BIT  */
> -
>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>  #ifdef CONFIG_SOC_IMX7D
>  #define cpu_to_caam_dma(value) \
> 

Re: [PATCH 7/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

[Horia Geantă]

On 6/22/2017 7:49 PM, Logan Gunthorpe wrote:=0A=
> Now that ioread64 and iowrite64 are always available we don't=0A=
> need the ugly ifdefs to change their implementation when they=0A=
> are not.=0A=
> =0A=
Thanks Logan.=0A=
=0A=
Note however this is not equivalent - it changes the behaviour, since=0A=
CAAM engine on i.MX6S/SL/D/Q platforms is broken in terms of 64-bit=0A=
register endianness - see CONFIG_CRYPTO_DEV_FSL_CAAM_IMX usage in code=0A=
you are removing.=0A=
=0A=
[Yes, current code has its problems, as it does not differentiate b/w=0A=
i.MX platforms with and without the (unofficial) erratum, but this=0A=
should be fixed separately.]=0A=
=0A=
Below is the change that would keep current logic - still forcing i.MX=0A=
to write CAAM 64-bit registers in BE even if the engine is LE (yes, diff=0A=
is doing a poor job).=0A=
=0A=
Horia=0A=
=0A=
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h=0A=
index 84d2f838a063..b893ebb24e65 100644=0A=
--- a/drivers/crypto/caam/regs.h=0A=
+++ b/drivers/crypto/caam/regs.h=0A=
@@ -134,50 +134,25 @@ static inline void clrsetbits_32(void __iomem=0A=
*reg, u32 clear, u32 set)=0A=
  *    base + 0x0000 : least-significant 32 bits=0A=
  *    base + 0x0004 : most-significant 32 bits=0A=
  */=0A=
-#ifdef CONFIG_64BIT=0A=
 static inline void wr_reg64(void __iomem *reg, u64 data)=0A=
 {=0A=
+#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX=0A=
        if (caam_little_end)=0A=
                iowrite64(data, reg);=0A=
        else=0A=
-               iowrite64be(data, reg);=0A=
-}=0A=
-=0A=
-static inline u64 rd_reg64(void __iomem *reg)=0A=
-{=0A=
-       if (caam_little_end)=0A=
-               return ioread64(reg);=0A=
-       else=0A=
-               return ioread64be(reg);=0A=
-}=0A=
-=0A=
-#else /* CONFIG_64BIT */=0A=
-static inline void wr_reg64(void __iomem *reg, u64 data)=0A=
-{=0A=
-#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX=0A=
-       if (caam_little_end) {=0A=
-               wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);=0A=
-               wr_reg32((u32 __iomem *)(reg), data);=0A=
-       } else=0A=
 #endif=0A=
-       {=0A=
-               wr_reg32((u32 __iomem *)(reg), data >> 32);=0A=
-               wr_reg32((u32 __iomem *)(reg) + 1, data);=0A=
-       }=0A=
+               iowrite64be(data, reg);=0A=
 }=0A=
=0A=
 static inline u64 rd_reg64(void __iomem *reg)=0A=
 {=0A=
 #ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX=0A=
        if (caam_little_end)=0A=
-               return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |=0A=
-                       (u64)rd_reg32((u32 __iomem *)(reg)));=0A=
+               return ioread64(reg);=0A=
        else=0A=
 #endif=0A=
-               return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |=0A=
-                       (u64)rd_reg32((u32 __iomem *)(reg) + 1));=0A=
+               return ioread64be(reg);=0A=
 }=0A=
-#endif /* CONFIG_64BIT  */=0A=
=0A=
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT=0A=
 #ifdef CONFIG_SOC_IMX7D=0A=
=0A=
=0A=
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>=0A=
> Cc: "Horia Geant=E3" <horia.geanta@nxp.com>=0A=
> Cc: Dan Douglass <dan.douglass@nxp.com>=0A=
> Cc: Herbert Xu <herbert@gondor.apana.org.au>=0A=
> Cc: "David S. Miller" <davem@davemloft.net>=0A=
> ---=0A=
>  drivers/crypto/caam/regs.h | 29 -----------------------------=0A=
>  1 file changed, 29 deletions(-)=0A=
> =0A=
> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h=0A=
> index 84d2f838a063..26fc19dd0c39 100644=0A=
> --- a/drivers/crypto/caam/regs.h=0A=
> +++ b/drivers/crypto/caam/regs.h=0A=
> @@ -134,7 +134,6 @@ static inline void clrsetbits_32(void __iomem *reg, u=
32 clear, u32 set)=0A=
>   *    base + 0x0000 : least-significant 32 bits=0A=
>   *    base + 0x0004 : most-significant 32 bits=0A=
>   */=0A=
> -#ifdef CONFIG_64BIT=0A=
>  static inline void wr_reg64(void __iomem *reg, u64 data)=0A=
>  {=0A=
>  	if (caam_little_end)=0A=
> @@ -151,34 +150,6 @@ static inline u64 rd_reg64(void __iomem *reg)=0A=
>  		return ioread64be(reg);=0A=
>  }=0A=
>  =0A=
> -#else /* CONFIG_64BIT */=0A=
> -static inline void wr_reg64(void __iomem *reg, u64 data)=0A=
> -{=0A=
> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX=0A=
> -	if (caam_little_end) {=0A=
> -		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);=0A=
> -		wr_reg32((u32 __iomem *)(reg), data);=0A=
> -	} else=0A=
> -#endif=0A=
> -	{=0A=
> -		wr_reg32((u32 __iomem *)(reg), data >> 32);=0A=
> -		wr_reg32((u32 __iomem *)(reg) + 1, data);=0A=
> -	}=0A=
> -}=0A=
> -=0A=
> -static inline u64 rd_reg64(void __iomem *reg)=0A=
> -{=0A=
> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX=0A=
> -	if (caam_little_end)=0A=
> -		return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |=0A=
> -			(u64)rd_reg32((u32 __iomem *)(reg)));=0A=
> -	else=0A=
> -#endif=0A=
> -		return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |=0A=
> -			(u64)rd_reg32((u32 __iomem *)(reg) + 1));=0A=
> -}=0A=
> -#endif /* CONFIG_64BIT  */=0A=
> -=0A=
>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT=0A=
>  #ifdef CONFIG_SOC_IMX7D=0A=
>  #define cpu_to_caam_dma(value) \=0A=
> =0A=

Re: [PATCH 7/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

[Logan Gunthorpe]

Thanks Horia.

I'm inclined to just use your patch verbatim. I can set you as author,
but no matter how I do it, I'll need your Signed-off-by.

Logan

On 23/06/17 12:51 AM, Horia Geantă wrote:
> On 6/22/2017 7:49 PM, Logan Gunthorpe wrote:
>> Now that ioread64 and iowrite64 are always available we don't
>> need the ugly ifdefs to change their implementation when they
>> are not.
>>
> Thanks Logan.
> 
> Note however this is not equivalent - it changes the behaviour, since
> CAAM engine on i.MX6S/SL/D/Q platforms is broken in terms of 64-bit
> register endianness - see CONFIG_CRYPTO_DEV_FSL_CAAM_IMX usage in code
> you are removing.
> 
> [Yes, current code has its problems, as it does not differentiate b/w
> i.MX platforms with and without the (unofficial) erratum, but this
> should be fixed separately.]
> 
> Below is the change that would keep current logic - still forcing i.MX
> to write CAAM 64-bit registers in BE even if the engine is LE (yes, diff
> is doing a poor job).
> 
> Horia
> 
> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
> index 84d2f838a063..b893ebb24e65 100644
> --- a/drivers/crypto/caam/regs.h
> +++ b/drivers/crypto/caam/regs.h
> @@ -134,50 +134,25 @@ static inline void clrsetbits_32(void __iomem
> *reg, u32 clear, u32 set)
>   *    base + 0x0000 : least-significant 32 bits
>   *    base + 0x0004 : most-significant 32 bits
>   */
> -#ifdef CONFIG_64BIT
>  static inline void wr_reg64(void __iomem *reg, u64 data)
>  {
> +#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>         if (caam_little_end)
>                 iowrite64(data, reg);
>         else
> -               iowrite64be(data, reg);
> -}
> -
> -static inline u64 rd_reg64(void __iomem *reg)
> -{
> -       if (caam_little_end)
> -               return ioread64(reg);
> -       else
> -               return ioread64be(reg);
> -}
> -
> -#else /* CONFIG_64BIT */
> -static inline void wr_reg64(void __iomem *reg, u64 data)
> -{
> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
> -       if (caam_little_end) {
> -               wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
> -               wr_reg32((u32 __iomem *)(reg), data);
> -       } else
>  #endif
> -       {
> -               wr_reg32((u32 __iomem *)(reg), data >> 32);
> -               wr_reg32((u32 __iomem *)(reg) + 1, data);
> -       }
> +               iowrite64be(data, reg);
>  }
> 
>  static inline u64 rd_reg64(void __iomem *reg)
>  {
>  #ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>         if (caam_little_end)
> -               return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
> -                       (u64)rd_reg32((u32 __iomem *)(reg)));
> +               return ioread64(reg);
>         else
>  #endif
> -               return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
> -                       (u64)rd_reg32((u32 __iomem *)(reg) + 1));
> +               return ioread64be(reg);
>  }
> -#endif /* CONFIG_64BIT  */
> 
>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>  #ifdef CONFIG_SOC_IMX7D
> 
> 
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>> Cc: "Horia Geantă" <horia.geanta@nxp.com>
>> Cc: Dan Douglass <dan.douglass@nxp.com>
>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> ---
>>  drivers/crypto/caam/regs.h | 29 -----------------------------
>>  1 file changed, 29 deletions(-)
>>
>> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
>> index 84d2f838a063..26fc19dd0c39 100644
>> --- a/drivers/crypto/caam/regs.h
>> +++ b/drivers/crypto/caam/regs.h
>> @@ -134,7 +134,6 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set)
>>   *    base + 0x0000 : least-significant 32 bits
>>   *    base + 0x0004 : most-significant 32 bits
>>   */
>> -#ifdef CONFIG_64BIT
>>  static inline void wr_reg64(void __iomem *reg, u64 data)
>>  {
>>  	if (caam_little_end)
>> @@ -151,34 +150,6 @@ static inline u64 rd_reg64(void __iomem *reg)
>>  		return ioread64be(reg);
>>  }
>>  
>> -#else /* CONFIG_64BIT */
>> -static inline void wr_reg64(void __iomem *reg, u64 data)
>> -{
>> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>> -	if (caam_little_end) {
>> -		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
>> -		wr_reg32((u32 __iomem *)(reg), data);
>> -	} else
>> -#endif
>> -	{
>> -		wr_reg32((u32 __iomem *)(reg), data >> 32);
>> -		wr_reg32((u32 __iomem *)(reg) + 1, data);
>> -	}
>> -}
>> -
>> -static inline u64 rd_reg64(void __iomem *reg)
>> -{
>> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>> -	if (caam_little_end)
>> -		return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
>> -			(u64)rd_reg32((u32 __iomem *)(reg)));
>> -	else
>> -#endif
>> -		return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
>> -			(u64)rd_reg32((u32 __iomem *)(reg) + 1));
>> -}
>> -#endif /* CONFIG_64BIT  */
>> -
>>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>  #ifdef CONFIG_SOC_IMX7D
>>  #define cpu_to_caam_dma(value) \
>>

Re: [PATCH 7/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

[Logan Gunthorpe]

Thanks Horia.

I'm inclined to just use your patch verbatim. I can set you as author,
but no matter how I do it, I'll need your Signed-off-by.

Logan

On 23/06/17 12:51 AM, Horia Geantã wrote:
> On 6/22/2017 7:49 PM, Logan Gunthorpe wrote:
>> Now that ioread64 and iowrite64 are always available we don't
>> need the ugly ifdefs to change their implementation when they
>> are not.
>>
> Thanks Logan.
> 
> Note however this is not equivalent - it changes the behaviour, since
> CAAM engine on i.MX6S/SL/D/Q platforms is broken in terms of 64-bit
> register endianness - see CONFIG_CRYPTO_DEV_FSL_CAAM_IMX usage in code
> you are removing.
> 
> [Yes, current code has its problems, as it does not differentiate b/w
> i.MX platforms with and without the (unofficial) erratum, but this
> should be fixed separately.]
> 
> Below is the change that would keep current logic - still forcing i.MX
> to write CAAM 64-bit registers in BE even if the engine is LE (yes, diff
> is doing a poor job).
> 
> Horia
> 
> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
> index 84d2f838a063..b893ebb24e65 100644
> --- a/drivers/crypto/caam/regs.h
> +++ b/drivers/crypto/caam/regs.h
> @@ -134,50 +134,25 @@ static inline void clrsetbits_32(void __iomem
> *reg, u32 clear, u32 set)
>   *    base + 0x0000 : least-significant 32 bits
>   *    base + 0x0004 : most-significant 32 bits
>   */
> -#ifdef CONFIG_64BIT
>  static inline void wr_reg64(void __iomem *reg, u64 data)
>  {
> +#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>         if (caam_little_end)
>                 iowrite64(data, reg);
>         else
> -               iowrite64be(data, reg);
> -}
> -
> -static inline u64 rd_reg64(void __iomem *reg)
> -{
> -       if (caam_little_end)
> -               return ioread64(reg);
> -       else
> -               return ioread64be(reg);
> -}
> -
> -#else /* CONFIG_64BIT */
> -static inline void wr_reg64(void __iomem *reg, u64 data)
> -{
> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
> -       if (caam_little_end) {
> -               wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
> -               wr_reg32((u32 __iomem *)(reg), data);
> -       } else
>  #endif
> -       {
> -               wr_reg32((u32 __iomem *)(reg), data >> 32);
> -               wr_reg32((u32 __iomem *)(reg) + 1, data);
> -       }
> +               iowrite64be(data, reg);
>  }
> 
>  static inline u64 rd_reg64(void __iomem *reg)
>  {
>  #ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>         if (caam_little_end)
> -               return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
> -                       (u64)rd_reg32((u32 __iomem *)(reg)));
> +               return ioread64(reg);
>         else
>  #endif
> -               return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
> -                       (u64)rd_reg32((u32 __iomem *)(reg) + 1));
> +               return ioread64be(reg);
>  }
> -#endif /* CONFIG_64BIT  */
> 
>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>  #ifdef CONFIG_SOC_IMX7D
> 
> 
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>> Cc: "Horia Geantã" <horia.geanta@nxp.com>
>> Cc: Dan Douglass <dan.douglass@nxp.com>
>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> ---
>>  drivers/crypto/caam/regs.h | 29 -----------------------------
>>  1 file changed, 29 deletions(-)
>>
>> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
>> index 84d2f838a063..26fc19dd0c39 100644
>> --- a/drivers/crypto/caam/regs.h
>> +++ b/drivers/crypto/caam/regs.h
>> @@ -134,7 +134,6 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set)
>>   *    base + 0x0000 : least-significant 32 bits
>>   *    base + 0x0004 : most-significant 32 bits
>>   */
>> -#ifdef CONFIG_64BIT
>>  static inline void wr_reg64(void __iomem *reg, u64 data)
>>  {
>>  	if (caam_little_end)
>> @@ -151,34 +150,6 @@ static inline u64 rd_reg64(void __iomem *reg)
>>  		return ioread64be(reg);
>>  }
>>  
>> -#else /* CONFIG_64BIT */
>> -static inline void wr_reg64(void __iomem *reg, u64 data)
>> -{
>> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>> -	if (caam_little_end) {
>> -		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
>> -		wr_reg32((u32 __iomem *)(reg), data);
>> -	} else
>> -#endif
>> -	{
>> -		wr_reg32((u32 __iomem *)(reg), data >> 32);
>> -		wr_reg32((u32 __iomem *)(reg) + 1, data);
>> -	}
>> -}
>> -
>> -static inline u64 rd_reg64(void __iomem *reg)
>> -{
>> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>> -	if (caam_little_end)
>> -		return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
>> -			(u64)rd_reg32((u32 __iomem *)(reg)));
>> -	else
>> -#endif
>> -		return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
>> -			(u64)rd_reg32((u32 __iomem *)(reg) + 1));
>> -}
>> -#endif /* CONFIG_64BIT  */
>> -
>>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>  #ifdef CONFIG_SOC_IMX7D
>>  #define cpu_to_caam_dma(value) \
>>

Re: [PATCH 7/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

[Logan Gunthorpe]

Thanks Horia.

I'm inclined to just use your patch verbatim. I can set you as author,
but no matter how I do it, I'll need your Signed-off-by.

Logan

On 23/06/17 12:51 AM, Horia Geantďż˝ wrote:
> On 6/22/2017 7:49 PM, Logan Gunthorpe wrote:
>> Now that ioread64 and iowrite64 are always available we don't
>> need the ugly ifdefs to change their implementation when they
>> are not.
>>
> Thanks Logan.
> 
> Note however this is not equivalent - it changes the behaviour, since
> CAAM engine on i.MX6S/SL/D/Q platforms is broken in terms of 64-bit
> register endianness - see CONFIG_CRYPTO_DEV_FSL_CAAM_IMX usage in code
> you are removing.
> 
> [Yes, current code has its problems, as it does not differentiate b/w
> i.MX platforms with and without the (unofficial) erratum, but this
> should be fixed separately.]
> 
> Below is the change that would keep current logic - still forcing i.MX
> to write CAAM 64-bit registers in BE even if the engine is LE (yes, diff
> is doing a poor job).
> 
> Horia
> 
> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
> index 84d2f838a063..b893ebb24e65 100644
> --- a/drivers/crypto/caam/regs.h
> +++ b/drivers/crypto/caam/regs.h
> @@ -134,50 +134,25 @@ static inline void clrsetbits_32(void __iomem
> *reg, u32 clear, u32 set)
>   *    base + 0x0000 : least-significant 32 bits
>   *    base + 0x0004 : most-significant 32 bits
>   */
> -#ifdef CONFIG_64BIT
>  static inline void wr_reg64(void __iomem *reg, u64 data)
>  {
> +#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>         if (caam_little_end)
>                 iowrite64(data, reg);
>         else
> -               iowrite64be(data, reg);
> -}
> -
> -static inline u64 rd_reg64(void __iomem *reg)
> -{
> -       if (caam_little_end)
> -               return ioread64(reg);
> -       else
> -               return ioread64be(reg);
> -}
> -
> -#else /* CONFIG_64BIT */
> -static inline void wr_reg64(void __iomem *reg, u64 data)
> -{
> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
> -       if (caam_little_end) {
> -               wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
> -               wr_reg32((u32 __iomem *)(reg), data);
> -       } else
>  #endif
> -       {
> -               wr_reg32((u32 __iomem *)(reg), data >> 32);
> -               wr_reg32((u32 __iomem *)(reg) + 1, data);
> -       }
> +               iowrite64be(data, reg);
>  }
> 
>  static inline u64 rd_reg64(void __iomem *reg)
>  {
>  #ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>         if (caam_little_end)
> -               return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
> -                       (u64)rd_reg32((u32 __iomem *)(reg)));
> +               return ioread64(reg);
>         else
>  #endif
> -               return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
> -                       (u64)rd_reg32((u32 __iomem *)(reg) + 1));
> +               return ioread64be(reg);
>  }
> -#endif /* CONFIG_64BIT  */
> 
>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>  #ifdef CONFIG_SOC_IMX7D
> 
> 
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>> Cc: "Horia Geantďż˝" <horia.geanta@nxp.com>
>> Cc: Dan Douglass <dan.douglass@nxp.com>
>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> ---
>>  drivers/crypto/caam/regs.h | 29 -----------------------------
>>  1 file changed, 29 deletions(-)
>>
>> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
>> index 84d2f838a063..26fc19dd0c39 100644
>> --- a/drivers/crypto/caam/regs.h
>> +++ b/drivers/crypto/caam/regs.h
>> @@ -134,7 +134,6 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set)
>>   *    base + 0x0000 : least-significant 32 bits
>>   *    base + 0x0004 : most-significant 32 bits
>>   */
>> -#ifdef CONFIG_64BIT
>>  static inline void wr_reg64(void __iomem *reg, u64 data)
>>  {
>>  	if (caam_little_end)
>> @@ -151,34 +150,6 @@ static inline u64 rd_reg64(void __iomem *reg)
>>  		return ioread64be(reg);
>>  }
>>  
>> -#else /* CONFIG_64BIT */
>> -static inline void wr_reg64(void __iomem *reg, u64 data)
>> -{
>> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>> -	if (caam_little_end) {
>> -		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
>> -		wr_reg32((u32 __iomem *)(reg), data);
>> -	} else
>> -#endif
>> -	{
>> -		wr_reg32((u32 __iomem *)(reg), data >> 32);
>> -		wr_reg32((u32 __iomem *)(reg) + 1, data);
>> -	}
>> -}
>> -
>> -static inline u64 rd_reg64(void __iomem *reg)
>> -{
>> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>> -	if (caam_little_end)
>> -		return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
>> -			(u64)rd_reg32((u32 __iomem *)(reg)));
>> -	else
>> -#endif
>> -		return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
>> -			(u64)rd_reg32((u32 __iomem *)(reg) + 1));
>> -}
>> -#endif /* CONFIG_64BIT  */
>> -
>>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>  #ifdef CONFIG_SOC_IMX7D
>>  #define cpu_to_caam_dma(value) \
>>

[PATCH v2 7/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

[Horia Geantă]

Now that ioread64 and iowrite64 are always available we don't
need the ugly ifdefs to change their implementation when they
are not.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Dan Douglass <dan.douglass@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>

Updated patch such that behaviour does not change
from i.MX workaround point of view.

Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
---
 drivers/crypto/caam/regs.h | 33 ++++-----------------------------
 1 file changed, 4 insertions(+), 29 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 84d2f838a063..b893ebb24e65 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -134,50 +134,25 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set)
  *    base + 0x0000 : least-significant 32 bits
  *    base + 0x0004 : most-significant 32 bits
  */
-#ifdef CONFIG_64BIT
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
+#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
 	if (caam_little_end)
 		iowrite64(data, reg);
 	else
-		iowrite64be(data, reg);
-}
-
-static inline u64 rd_reg64(void __iomem *reg)
-{
-	if (caam_little_end)
-		return ioread64(reg);
-	else
-		return ioread64be(reg);
-}
-
-#else /* CONFIG_64BIT */
-static inline void wr_reg64(void __iomem *reg, u64 data)
-{
-#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
-	if (caam_little_end) {
-		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
-		wr_reg32((u32 __iomem *)(reg), data);
-	} else
 #endif
-	{
-		wr_reg32((u32 __iomem *)(reg), data >> 32);
-		wr_reg32((u32 __iomem *)(reg) + 1, data);
-	}
+		iowrite64be(data, reg);
 }
 
 static inline u64 rd_reg64(void __iomem *reg)
 {
 #ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
 	if (caam_little_end)
-		return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
-			(u64)rd_reg32((u32 __iomem *)(reg)));
+		return ioread64(reg);
 	else
 #endif
-		return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
-			(u64)rd_reg32((u32 __iomem *)(reg) + 1));
+		return ioread64be(reg);
 }
-#endif /* CONFIG_64BIT  */
 
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 #ifdef CONFIG_SOC_IMX7D
-- 
2.12.0.264.gd6db3f216544

[PATCH v2 7/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64

[Horia Geantă]

Now that ioread64 and iowrite64 are always available we don't
need the ugly ifdefs to change their implementation when they
are not.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Dan Douglass <dan.douglass@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>

Updated patch such that behaviour does not change
from i.MX workaround point of view.

Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
---
 drivers/crypto/caam/regs.h | 33 ++++-----------------------------
 1 file changed, 4 insertions(+), 29 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 84d2f838a063..b893ebb24e65 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -134,50 +134,25 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set)
  *    base + 0x0000 : least-significant 32 bits
  *    base + 0x0004 : most-significant 32 bits
  */
-#ifdef CONFIG_64BIT
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
+#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
 	if (caam_little_end)
 		iowrite64(data, reg);
 	else
-		iowrite64be(data, reg);
-}
-
-static inline u64 rd_reg64(void __iomem *reg)
-{
-	if (caam_little_end)
-		return ioread64(reg);
-	else
-		return ioread64be(reg);
-}
-
-#else /* CONFIG_64BIT */
-static inline void wr_reg64(void __iomem *reg, u64 data)
-{
-#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
-	if (caam_little_end) {
-		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
-		wr_reg32((u32 __iomem *)(reg), data);
-	} else
 #endif
-	{
-		wr_reg32((u32 __iomem *)(reg), data >> 32);
-		wr_reg32((u32 __iomem *)(reg) + 1, data);
-	}
+		iowrite64be(data, reg);
 }
 
 static inline u64 rd_reg64(void __iomem *reg)
 {
 #ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
 	if (caam_little_end)
-		return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
-			(u64)rd_reg32((u32 __iomem *)(reg)));
+		return ioread64(reg);
 	else
 #endif
-		return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
-			(u64)rd_reg32((u32 __iomem *)(reg) + 1));
+		return ioread64be(reg);
 }
-#endif /* CONFIG_64BIT  */
 
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 #ifdef CONFIG_SOC_IMX7D
-- 
2.12.0.264.gd6db3f216544

[PATCH] alpha: provide ioread64 and iowrite64 implementations

[Richard Henderson]

All Alpha hosts except for Jensen provide 64-bit I/O operations.

Jensen is EISA only, so there ought not be any devices that even
attempt such operations.  But just in case, use 2 32-bit operations.

Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 arch/alpha/include/asm/core_apecs.h  | 16 ++++++++++++++++
 arch/alpha/include/asm/core_cia.h    | 16 ++++++++++++++++
 arch/alpha/include/asm/core_lca.h    | 16 ++++++++++++++++
 arch/alpha/include/asm/core_mcpcia.h | 21 +++++++++++++++++++++
 arch/alpha/include/asm/core_t2.h     | 24 +++++++++++++++++++-----
 arch/alpha/include/asm/io.h          | 27 +++++++++++++++++++++++++++
 arch/alpha/include/asm/io_trivial.h  | 12 ++++++++++++
 arch/alpha/include/asm/jensen.h      | 32 +++++++++++++++++++++++++++-----
 arch/alpha/include/asm/machvec.h     |  2 ++
 arch/alpha/kernel/io.c               | 27 +++++++++++++++++++++++++++
 arch/alpha/kernel/machvec_impl.h     |  2 ++
 11 files changed, 185 insertions(+), 10 deletions(-)

diff --git a/arch/alpha/include/asm/core_apecs.h b/arch/alpha/include/asm/core_apecs.h
index 6785ff7..7e2ff13 100644
--- a/arch/alpha/include/asm/core_apecs.h
+++ b/arch/alpha/include/asm/core_apecs.h
@@ -471,6 +471,22 @@ __EXTERN_INLINE void apecs_iowrite32(u32 b, void __iomem *xaddr)
 	*(vuip)addr = b;
 }
 
+__EXTERN_INLINE u64 apecs_ioread64(void __iomem *xaddr)
+{
+	unsigned long addr = (unsigned long) xaddr;
+	if (addr < APECS_DENSE_MEM)
+		addr = ((addr - APECS_IO) << 5) + APECS_IO + 0x78;
+	return *(vulp)addr;
+}
+
+__EXTERN_INLINE void apecs_iowrite64(u64 b, void __iomem *xaddr)
+{
+	unsigned long addr = (unsigned long) xaddr;
+	if (addr < APECS_DENSE_MEM)
+		addr = ((addr - APECS_IO) << 5) + APECS_IO + 0x78;
+	*(vulp)addr = b;
+}
+
 __EXTERN_INLINE void __iomem *apecs_ioportmap(unsigned long addr)
 {
 	return (void __iomem *)(addr + APECS_IO);
diff --git a/arch/alpha/include/asm/core_cia.h b/arch/alpha/include/asm/core_cia.h
index 9e0516c..54792b3 100644
--- a/arch/alpha/include/asm/core_cia.h
+++ b/arch/alpha/include/asm/core_cia.h
@@ -419,6 +419,22 @@ __EXTERN_INLINE void cia_iowrite32(u32 b, void __iomem *xaddr)
 	*(vuip)addr = b;
 }
 
+__EXTERN_INLINE u64 cia_ioread64(void __iomem *xaddr)
+{
+	unsigned long addr = (unsigned long) xaddr;
+	if (addr < CIA_DENSE_MEM)
+		addr = ((addr - CIA_IO) << 5) + CIA_IO + 0x78;
+	return *(vulp)addr;
+}
+
+__EXTERN_INLINE void cia_iowrite64(u64 b, void __iomem *xaddr)
+{
+	unsigned long addr = (unsigned long) xaddr;
+	if (addr < CIA_DENSE_MEM)
+		addr = ((addr - CIA_IO) << 5) + CIA_IO + 0x78;
+	*(vulp)addr = b;
+}
+
 __EXTERN_INLINE void __iomem *cia_ioportmap(unsigned long addr)
 {
 	return (void __iomem *)(addr + CIA_IO);
diff --git a/arch/alpha/include/asm/core_lca.h b/arch/alpha/include/asm/core_lca.h
index 8ee6c51..b8e43cd 100644
--- a/arch/alpha/include/asm/core_lca.h
+++ b/arch/alpha/include/asm/core_lca.h
@@ -317,6 +317,22 @@ __EXTERN_INLINE void lca_iowrite32(u32 b, void __iomem *xaddr)
 	*(vuip)addr = b;
 }
 
+__EXTERN_INLINE u64 lca_ioread64(void __iomem *xaddr)
+{
+	unsigned long addr = (unsigned long) xaddr;
+	if (addr < LCA_DENSE_MEM)
+		addr = ((addr - LCA_IO) << 5) + LCA_IO + 0x78;
+	return *(vulp)addr;
+}
+
+__EXTERN_INLINE void lca_iowrite64(u64 b, void __iomem *xaddr)
+{
+	unsigned long addr = (unsigned long) xaddr;
+	if (addr < LCA_DENSE_MEM)
+		addr = ((addr - LCA_IO) << 5) + LCA_IO + 0x78;
+	*(vulp)addr = b;
+}
+
 __EXTERN_INLINE void __iomem *lca_ioportmap(unsigned long addr)
 {
 	return (void __iomem *)(addr + LCA_IO);
diff --git a/arch/alpha/include/asm/core_mcpcia.h b/arch/alpha/include/asm/core_mcpcia.h
index ad44bef..e4d9139 100644
--- a/arch/alpha/include/asm/core_mcpcia.h
+++ b/arch/alpha/include/asm/core_mcpcia.h
@@ -247,6 +247,7 @@ struct el_MCPCIA_uncorrected_frame_mcheck {
 
 #define vip	volatile int __force *
 #define vuip	volatile unsigned int __force *
+#define vulp	volatile unsigned long __force *
 
 #ifndef MCPCIA_ONE_HAE_WINDOW
 #define MCPCIA_FROB_MMIO						\
@@ -334,6 +335,25 @@ __EXTERN_INLINE void mcpcia_iowrite32(u32 b, void __iomem *xaddr)
 	*(vuip)addr = b;
 }
 
+__EXTERN_INLINE u64 mcpcia_ioread64(void __iomem *xaddr)
+{
+	unsigned long addr = (unsigned long)xaddr;
+
+	if (!__mcpcia_is_mmio(addr))
+		addr = ((addr & 0xffff) << 5) + (addr & ~0xfffful) + 0x78;
+
+	return *(vulp)addr;
+}
+
+__EXTERN_INLINE void mcpcia_iowrite64(u64 b, void __iomem *xaddr)
+{
+	unsigned long addr = (unsigned long)xaddr;
+
+	if (!__mcpcia_is_mmio(addr))
+		addr = ((addr & 0xffff) << 5) + (addr & ~0xfffful) + 0x78;
+
+	*(vulp)addr = b;
+}
 
 __EXTERN_INLINE void __iomem *mcpcia_ioportmap(unsigned long addr)
 {
@@ -361,6 +381,7 @@ __EXTERN_INLINE int mcpcia_is_mmio(const volatile void __iomem *xaddr)
 
 #undef vip
 #undef vuip
+#undef vulp
 
 #undef __IO_PREFIX
 #define __IO_PREFIX		mcpcia
diff --git a/arch/alpha/include/asm/core_t2.h b/arch/alpha/include/asm/core_t2.h
index ade9d92..cb3467d 100644
--- a/arch/alpha/include/asm/core_t2.h
+++ b/arch/alpha/include/asm/core_t2.h
@@ -359,6 +359,7 @@ struct el_t2_frame_corrected {
 
 #define vip	volatile int *
 #define vuip	volatile unsigned int *
+#define vulp	volatile unsigned long *
 
 extern inline u8 t2_inb(unsigned long addr)
 {
@@ -401,6 +402,17 @@ extern inline void t2_outl(u32 b, unsigned long addr)
 	mb();
 }
 
+extern inline u64 t2_inq(unsigned long addr)
+{
+	return *(vulp) ((addr << 5) + T2_IO + 0x78);
+}
+
+extern inline void t2_outq(u64 b, unsigned long addr)
+{
+	*(vulp) ((addr << 5) + T2_IO + 0x78) = b;
+	mb();
+}
+
 
 /*
  * Memory functions.
@@ -570,8 +582,8 @@ __EXTERN_INLINE int t2_is_mmio(const volatile void __iomem *addr)
 /* New-style ioread interface.  The mmio routines are so ugly for T2 that
    it doesn't make sense to merge the pio and mmio routines.  */
 
-#define IOPORT(OS, NS)							\
-__EXTERN_INLINE unsigned int t2_ioread##NS(void __iomem *xaddr)		\
+#define IOPORT(OS, NS, RT)						\
+__EXTERN_INLINE RT t2_ioread##NS(void __iomem *xaddr)		\
 {									\
 	if (t2_is_mmio(xaddr))						\
 		return t2_read##OS(xaddr);				\
@@ -586,14 +598,16 @@ __EXTERN_INLINE void t2_iowrite##NS(u##NS b, void __iomem *xaddr)	\
 		t2_out##OS(b, (unsigned long)xaddr - T2_IO);		\
 }
 
-IOPORT(b, 8)
-IOPORT(w, 16)
-IOPORT(l, 32)
+IOPORT(b, 8, unsigned int)
+IOPORT(w, 16, unsigned int)
+IOPORT(l, 32, unsigned int)
+IOPORT(q, 64, u64)
 
 #undef IOPORT
 
 #undef vip
 #undef vuip
+#undef vulp
 
 #undef __IO_PREFIX
 #define __IO_PREFIX		t2
diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
index ff40491..16426e8 100644
--- a/arch/alpha/include/asm/io.h
+++ b/arch/alpha/include/asm/io.h
@@ -158,6 +158,7 @@ static inline void generic_##NAME(TYPE b, QUAL void __iomem *addr)	\
 REMAP1(unsigned int, ioread8, /**/)
 REMAP1(unsigned int, ioread16, /**/)
 REMAP1(unsigned int, ioread32, /**/)
+REMAP1(u64, ioread64, /**/)
 REMAP1(u8, readb, const volatile)
 REMAP1(u16, readw, const volatile)
 REMAP1(u32, readl, const volatile)
@@ -166,6 +167,7 @@ REMAP1(u64, readq, const volatile)
 REMAP2(u8, iowrite8, /**/)
 REMAP2(u16, iowrite16, /**/)
 REMAP2(u32, iowrite32, /**/)
+REMAP2(u64, iowrite64, /**/)
 REMAP2(u8, writeb, volatile)
 REMAP2(u16, writew, volatile)
 REMAP2(u32, writel, volatile)
@@ -384,6 +386,19 @@ extern inline void iowrite32(u32 b, void __iomem *addr)
 	mb();
 }
 
+extern inline u64 ioread64(void __iomem *addr)
+{
+	unsigned int ret = IO_CONCAT(__IO_PREFIX,ioread64)(addr);
+	mb();
+	return ret;
+}
+
+extern inline void iowrite64(u64 b, void __iomem *addr)
+{
+	IO_CONCAT(__IO_PREFIX,iowrite64)(b, addr);
+	mb();
+}
+
 extern inline u32 inl(unsigned long port)
 {
 	return ioread32(ioport_map(port, 4));
@@ -393,6 +408,16 @@ extern inline void outl(u32 b, unsigned long port)
 {
 	iowrite32(b, ioport_map(port, 4));
 }
+
+extern inline u64 inq(unsigned long port)
+{
+	return ioread64(ioport_map(port, 8));
+}
+
+extern inline void outq(u64 b, unsigned long port)
+{
+	iowrite64(b, ioport_map(port, 8));
+}
 #endif
 
 #if IO_CONCAT(__IO_PREFIX,trivial_rw_bw) == 1
@@ -493,8 +518,10 @@ extern inline void writeq(u64 b, volatile void __iomem *addr)
 
 #define ioread16be(p) be16_to_cpu(ioread16(p))
 #define ioread32be(p) be32_to_cpu(ioread32(p))
+#define ioread64be(p) be64_to_cpu(ioread64(p))
 #define iowrite16be(v,p) iowrite16(cpu_to_be16(v), (p))
 #define iowrite32be(v,p) iowrite32(cpu_to_be32(v), (p))
+#define iowrite64be(v,p) iowrite64(cpu_to_be64(v), (p))
 
 #define inb_p		inb
 #define inw_p		inw
diff --git a/arch/alpha/include/asm/io_trivial.h b/arch/alpha/include/asm/io_trivial.h
index 1c77f10..ae97730 100644
--- a/arch/alpha/include/asm/io_trivial.h
+++ b/arch/alpha/include/asm/io_trivial.h
@@ -37,11 +37,23 @@ IO_CONCAT(__IO_PREFIX,ioread32)(void __iomem *a)
 	return *(volatile u32 __force *)a;
 }
 
+__EXTERN_INLINE u64
+IO_CONCAT(__IO_PREFIX,ioread64)(void __iomem *a)
+{
+	return *(volatile u64 __force *)a;
+}
+
 __EXTERN_INLINE void
 IO_CONCAT(__IO_PREFIX,iowrite32)(u32 b, void __iomem *a)
 {
 	*(volatile u32 __force *)a = b;
 }
+
+__EXTERN_INLINE void
+IO_CONCAT(__IO_PREFIX,iowrite64)(u64 b, void __iomem *a)
+{
+	*(volatile u64 __force *)a = b;
+}
 #endif
 
 #if IO_CONCAT(__IO_PREFIX,trivial_rw_bw) == 1
diff --git a/arch/alpha/include/asm/jensen.h b/arch/alpha/include/asm/jensen.h
index 964b06e..7e945cf 100644
--- a/arch/alpha/include/asm/jensen.h
+++ b/arch/alpha/include/asm/jensen.h
@@ -182,6 +182,17 @@ __EXTERN_INLINE u32 jensen_inl(unsigned long addr)
 	return *(vuip) ((addr << 7) + EISA_IO + 0x60);
 }
 
+__EXTERN_INLINE u64 jensen_inq(unsigned long addr)
+{
+	unsigned long ioaddr = (addr << 7) + EISA_IO + 0x60;
+	unsigned long l, h;
+
+	jensen_set_hae(0);
+	l = *(vuip)ioaddr;
+	h = *(vuip)(ioaddr + (4 << 7));
+	return h << 32 | l;
+}
+
 __EXTERN_INLINE void jensen_outw(u16 b, unsigned long addr)
 {
 	jensen_set_hae(0);
@@ -196,6 +207,16 @@ __EXTERN_INLINE void jensen_outl(u32 b, unsigned long addr)
 	mb();
 }
 
+__EXTERN_INLINE void jensen_outq(u64 b, unsigned long addr)
+{
+	unsigned long ioaddr = (addr << 7) + EISA_IO + 0x60;
+
+	jensen_set_hae(0);
+	*(vuip)ioaddr = b;
+	*(vuip)(ioaddr + (4 << 7)) = b >> 32;
+	mb();
+}
+
 /*
  * Memory functions.
  */
@@ -303,8 +324,8 @@ __EXTERN_INLINE int jensen_is_mmio(const volatile void __iomem *addr)
 /* New-style ioread interface.  All the routines are so ugly for Jensen
    that it doesn't make sense to merge them.  */
 
-#define IOPORT(OS, NS)							\
-__EXTERN_INLINE unsigned int jensen_ioread##NS(void __iomem *xaddr)	\
+#define IOPORT(OS, NS, RT)						\
+__EXTERN_INLINE RT jensen_ioread##NS(void __iomem *xaddr)		\
 {									\
 	if (jensen_is_mmio(xaddr))					\
 		return jensen_read##OS(xaddr - 0x100000000ul);		\
@@ -319,9 +340,10 @@ __EXTERN_INLINE void jensen_iowrite##NS(u##NS b, void __iomem *xaddr)	\
 		jensen_out##OS(b, (unsigned long)xaddr);		\
 }
 
-IOPORT(b, 8)
-IOPORT(w, 16)
-IOPORT(l, 32)
+IOPORT(b, 8, unsigned int)
+IOPORT(w, 16, unsigned int)
+IOPORT(l, 32, unsigned int)
+IOPORT(q, 64, u64)
 
 #undef IOPORT
 
diff --git a/arch/alpha/include/asm/machvec.h b/arch/alpha/include/asm/machvec.h
index 75cb364..c037c7b 100644
--- a/arch/alpha/include/asm/machvec.h
+++ b/arch/alpha/include/asm/machvec.h
@@ -48,10 +48,12 @@ struct alpha_machine_vector
 	unsigned int (*mv_ioread8)(void __iomem *);
 	unsigned int (*mv_ioread16)(void __iomem *);
 	unsigned int (*mv_ioread32)(void __iomem *);
+	u64 (*mv_ioread64)(void __iomem *);
 
 	void (*mv_iowrite8)(u8, void __iomem *);
 	void (*mv_iowrite16)(u16, void __iomem *);
 	void (*mv_iowrite32)(u32, void __iomem *);
+	void (*mv_iowrite64)(u64, void __iomem *);
 
 	u8 (*mv_readb)(const volatile void __iomem *);
 	u16 (*mv_readw)(const volatile void __iomem *);
diff --git a/arch/alpha/kernel/io.c b/arch/alpha/kernel/io.c
index 19c5875..d36146f 100644
--- a/arch/alpha/kernel/io.c
+++ b/arch/alpha/kernel/io.c
@@ -34,6 +34,13 @@ unsigned int ioread32(void __iomem *addr)
 	return ret;
 }
 
+u64 ioread64(void __iomem *addr)
+{
+	u64 ret = IO_CONCAT(__IO_PREFIX,ioread64)(addr);
+	mb();
+	return ret;
+}
+
 void iowrite8(u8 b, void __iomem *addr)
 {
 	IO_CONCAT(__IO_PREFIX,iowrite8)(b, addr);
@@ -52,12 +59,20 @@ void iowrite32(u32 b, void __iomem *addr)
 	mb();
 }
 
+void iowrite64(u64 b, void __iomem *addr)
+{
+	IO_CONCAT(__IO_PREFIX,iowrite64)(b, addr);
+	mb();
+}
+
 EXPORT_SYMBOL(ioread8);
 EXPORT_SYMBOL(ioread16);
 EXPORT_SYMBOL(ioread32);
+EXPORT_SYMBOL(ioread64);
 EXPORT_SYMBOL(iowrite8);
 EXPORT_SYMBOL(iowrite16);
 EXPORT_SYMBOL(iowrite32);
+EXPORT_SYMBOL(iowrite64);
 
 u8 inb(unsigned long port)
 {
@@ -74,6 +89,11 @@ u32 inl(unsigned long port)
 	return ioread32(ioport_map(port, 4));
 }
 
+u64 inq(unsigned long port)
+{
+	return ioread64(ioport_map(port, 8));
+}
+
 void outb(u8 b, unsigned long port)
 {
 	iowrite8(b, ioport_map(port, 1));
@@ -89,12 +109,19 @@ void outl(u32 b, unsigned long port)
 	iowrite32(b, ioport_map(port, 4));
 }
 
+void outq(u64 b, unsigned long port)
+{
+	iowrite64(b, ioport_map(port, 8));
+}
+
 EXPORT_SYMBOL(inb);
 EXPORT_SYMBOL(inw);
 EXPORT_SYMBOL(inl);
+EXPORT_SYMBOL(inq);
 EXPORT_SYMBOL(outb);
 EXPORT_SYMBOL(outw);
 EXPORT_SYMBOL(outl);
+EXPORT_SYMBOL(outq);
 
 u8 __raw_readb(const volatile void __iomem *addr)
 {
diff --git a/arch/alpha/kernel/machvec_impl.h b/arch/alpha/kernel/machvec_impl.h
index b7d6960..9b37e19 100644
--- a/arch/alpha/kernel/machvec_impl.h
+++ b/arch/alpha/kernel/machvec_impl.h
@@ -79,9 +79,11 @@
 	.mv_ioread8 =		CAT(low,_ioread8),			\
 	.mv_ioread16 =		CAT(low,_ioread16),			\
 	.mv_ioread32 =		CAT(low,_ioread32),			\
+	.mv_ioread64 =		CAT(low,_ioread64),			\
 	.mv_iowrite8 =		CAT(low,_iowrite8),			\
 	.mv_iowrite16 =		CAT(low,_iowrite16),			\
 	.mv_iowrite32 =		CAT(low,_iowrite32),			\
+	.mv_iowrite64 =		CAT(low,_iowrite64),			\
 	.mv_readb =		CAT(low,_readb),			\
 	.mv_readw =		CAT(low,_readw),			\
 	.mv_readl =		CAT(low,_readl),			\
-- 
2.9.4

Re: [PATCH] alpha: provide ioread64 and iowrite64 implementations

[Logan Gunthorpe]

Hey,

On 24/06/17 09:13 AM, Richard Henderson wrote:
> All Alpha hosts except for Jensen provide 64-bit I/O operations.
> 
> Jensen is EISA only, so there ought not be any devices that even
> attempt such operations.  But just in case, use 2 32-bit operations.

Thanks for this, Richard.

However, I was recently enlightened by the existence of the
linux/io-64-nonatomic* headers. This is where the 2 32-bit operation
functions belong. So you should probably remove them from this patch and
let drivers that need them just include those headers.

Logan

> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> Cc: Matt Turner <mattst88@gmail.com>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  arch/alpha/include/asm/core_apecs.h  | 16 ++++++++++++++++
>  arch/alpha/include/asm/core_cia.h    | 16 ++++++++++++++++
>  arch/alpha/include/asm/core_lca.h    | 16 ++++++++++++++++
>  arch/alpha/include/asm/core_mcpcia.h | 21 +++++++++++++++++++++
>  arch/alpha/include/asm/core_t2.h     | 24 +++++++++++++++++++-----
>  arch/alpha/include/asm/io.h          | 27 +++++++++++++++++++++++++++
>  arch/alpha/include/asm/io_trivial.h  | 12 ++++++++++++
>  arch/alpha/include/asm/jensen.h      | 32 +++++++++++++++++++++++++++-----
>  arch/alpha/include/asm/machvec.h     |  2 ++
>  arch/alpha/kernel/io.c               | 27 +++++++++++++++++++++++++++
>  arch/alpha/kernel/machvec_impl.h     |  2 ++
>  11 files changed, 185 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/core_apecs.h b/arch/alpha/include/asm/core_apecs.h
> index 6785ff7..7e2ff13 100644
> --- a/arch/alpha/include/asm/core_apecs.h
> +++ b/arch/alpha/include/asm/core_apecs.h
> @@ -471,6 +471,22 @@ __EXTERN_INLINE void apecs_iowrite32(u32 b, void __iomem *xaddr)
>  	*(vuip)addr = b;
>  }
>  
> +__EXTERN_INLINE u64 apecs_ioread64(void __iomem *xaddr)
> +{
> +	unsigned long addr = (unsigned long) xaddr;
> +	if (addr < APECS_DENSE_MEM)
> +		addr = ((addr - APECS_IO) << 5) + APECS_IO + 0x78;
> +	return *(vulp)addr;
> +}
> +
> +__EXTERN_INLINE void apecs_iowrite64(u64 b, void __iomem *xaddr)
> +{
> +	unsigned long addr = (unsigned long) xaddr;
> +	if (addr < APECS_DENSE_MEM)
> +		addr = ((addr - APECS_IO) << 5) + APECS_IO + 0x78;
> +	*(vulp)addr = b;
> +}
> +
>  __EXTERN_INLINE void __iomem *apecs_ioportmap(unsigned long addr)
>  {
>  	return (void __iomem *)(addr + APECS_IO);
> diff --git a/arch/alpha/include/asm/core_cia.h b/arch/alpha/include/asm/core_cia.h
> index 9e0516c..54792b3 100644
> --- a/arch/alpha/include/asm/core_cia.h
> +++ b/arch/alpha/include/asm/core_cia.h
> @@ -419,6 +419,22 @@ __EXTERN_INLINE void cia_iowrite32(u32 b, void __iomem *xaddr)
>  	*(vuip)addr = b;
>  }
>  
> +__EXTERN_INLINE u64 cia_ioread64(void __iomem *xaddr)
> +{
> +	unsigned long addr = (unsigned long) xaddr;
> +	if (addr < CIA_DENSE_MEM)
> +		addr = ((addr - CIA_IO) << 5) + CIA_IO + 0x78;
> +	return *(vulp)addr;
> +}
> +
> +__EXTERN_INLINE void cia_iowrite64(u64 b, void __iomem *xaddr)
> +{
> +	unsigned long addr = (unsigned long) xaddr;
> +	if (addr < CIA_DENSE_MEM)
> +		addr = ((addr - CIA_IO) << 5) + CIA_IO + 0x78;
> +	*(vulp)addr = b;
> +}
> +
>  __EXTERN_INLINE void __iomem *cia_ioportmap(unsigned long addr)
>  {
>  	return (void __iomem *)(addr + CIA_IO);
> diff --git a/arch/alpha/include/asm/core_lca.h b/arch/alpha/include/asm/core_lca.h
> index 8ee6c51..b8e43cd 100644
> --- a/arch/alpha/include/asm/core_lca.h
> +++ b/arch/alpha/include/asm/core_lca.h
> @@ -317,6 +317,22 @@ __EXTERN_INLINE void lca_iowrite32(u32 b, void __iomem *xaddr)
>  	*(vuip)addr = b;
>  }
>  
> +__EXTERN_INLINE u64 lca_ioread64(void __iomem *xaddr)
> +{
> +	unsigned long addr = (unsigned long) xaddr;
> +	if (addr < LCA_DENSE_MEM)
> +		addr = ((addr - LCA_IO) << 5) + LCA_IO + 0x78;
> +	return *(vulp)addr;
> +}
> +
> +__EXTERN_INLINE void lca_iowrite64(u64 b, void __iomem *xaddr)
> +{
> +	unsigned long addr = (unsigned long) xaddr;
> +	if (addr < LCA_DENSE_MEM)
> +		addr = ((addr - LCA_IO) << 5) + LCA_IO + 0x78;
> +	*(vulp)addr = b;
> +}
> +
>  __EXTERN_INLINE void __iomem *lca_ioportmap(unsigned long addr)
>  {
>  	return (void __iomem *)(addr + LCA_IO);
> diff --git a/arch/alpha/include/asm/core_mcpcia.h b/arch/alpha/include/asm/core_mcpcia.h
> index ad44bef..e4d9139 100644
> --- a/arch/alpha/include/asm/core_mcpcia.h
> +++ b/arch/alpha/include/asm/core_mcpcia.h
> @@ -247,6 +247,7 @@ struct el_MCPCIA_uncorrected_frame_mcheck {
>  
>  #define vip	volatile int __force *
>  #define vuip	volatile unsigned int __force *
> +#define vulp	volatile unsigned long __force *
>  
>  #ifndef MCPCIA_ONE_HAE_WINDOW
>  #define MCPCIA_FROB_MMIO						\
> @@ -334,6 +335,25 @@ __EXTERN_INLINE void mcpcia_iowrite32(u32 b, void __iomem *xaddr)
>  	*(vuip)addr = b;
>  }
>  
> +__EXTERN_INLINE u64 mcpcia_ioread64(void __iomem *xaddr)
> +{
> +	unsigned long addr = (unsigned long)xaddr;
> +
> +	if (!__mcpcia_is_mmio(addr))
> +		addr = ((addr & 0xffff) << 5) + (addr & ~0xfffful) + 0x78;
> +
> +	return *(vulp)addr;
> +}
> +
> +__EXTERN_INLINE void mcpcia_iowrite64(u64 b, void __iomem *xaddr)
> +{
> +	unsigned long addr = (unsigned long)xaddr;
> +
> +	if (!__mcpcia_is_mmio(addr))
> +		addr = ((addr & 0xffff) << 5) + (addr & ~0xfffful) + 0x78;
> +
> +	*(vulp)addr = b;
> +}
>  
>  __EXTERN_INLINE void __iomem *mcpcia_ioportmap(unsigned long addr)
>  {
> @@ -361,6 +381,7 @@ __EXTERN_INLINE int mcpcia_is_mmio(const volatile void __iomem *xaddr)
>  
>  #undef vip
>  #undef vuip
> +#undef vulp
>  
>  #undef __IO_PREFIX
>  #define __IO_PREFIX		mcpcia
> diff --git a/arch/alpha/include/asm/core_t2.h b/arch/alpha/include/asm/core_t2.h
> index ade9d92..cb3467d 100644
> --- a/arch/alpha/include/asm/core_t2.h
> +++ b/arch/alpha/include/asm/core_t2.h
> @@ -359,6 +359,7 @@ struct el_t2_frame_corrected {
>  
>  #define vip	volatile int *
>  #define vuip	volatile unsigned int *
> +#define vulp	volatile unsigned long *
>  
>  extern inline u8 t2_inb(unsigned long addr)
>  {
> @@ -401,6 +402,17 @@ extern inline void t2_outl(u32 b, unsigned long addr)
>  	mb();
>  }
>  
> +extern inline u64 t2_inq(unsigned long addr)
> +{
> +	return *(vulp) ((addr << 5) + T2_IO + 0x78);
> +}
> +
> +extern inline void t2_outq(u64 b, unsigned long addr)
> +{
> +	*(vulp) ((addr << 5) + T2_IO + 0x78) = b;
> +	mb();
> +}
> +
>  
>  /*
>   * Memory functions.
> @@ -570,8 +582,8 @@ __EXTERN_INLINE int t2_is_mmio(const volatile void __iomem *addr)
>  /* New-style ioread interface.  The mmio routines are so ugly for T2 that
>     it doesn't make sense to merge the pio and mmio routines.  */
>  
> -#define IOPORT(OS, NS)							\
> -__EXTERN_INLINE unsigned int t2_ioread##NS(void __iomem *xaddr)		\
> +#define IOPORT(OS, NS, RT)						\
> +__EXTERN_INLINE RT t2_ioread##NS(void __iomem *xaddr)		\
>  {									\
>  	if (t2_is_mmio(xaddr))						\
>  		return t2_read##OS(xaddr);				\
> @@ -586,14 +598,16 @@ __EXTERN_INLINE void t2_iowrite##NS(u##NS b, void __iomem *xaddr)	\
>  		t2_out##OS(b, (unsigned long)xaddr - T2_IO);		\
>  }
>  
> -IOPORT(b, 8)
> -IOPORT(w, 16)
> -IOPORT(l, 32)
> +IOPORT(b, 8, unsigned int)
> +IOPORT(w, 16, unsigned int)
> +IOPORT(l, 32, unsigned int)
> +IOPORT(q, 64, u64)
>  
>  #undef IOPORT
>  
>  #undef vip
>  #undef vuip
> +#undef vulp
>  
>  #undef __IO_PREFIX
>  #define __IO_PREFIX		t2
> diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
> index ff40491..16426e8 100644
> --- a/arch/alpha/include/asm/io.h
> +++ b/arch/alpha/include/asm/io.h
> @@ -158,6 +158,7 @@ static inline void generic_##NAME(TYPE b, QUAL void __iomem *addr)	\
>  REMAP1(unsigned int, ioread8, /**/)
>  REMAP1(unsigned int, ioread16, /**/)
>  REMAP1(unsigned int, ioread32, /**/)
> +REMAP1(u64, ioread64, /**/)
>  REMAP1(u8, readb, const volatile)
>  REMAP1(u16, readw, const volatile)
>  REMAP1(u32, readl, const volatile)
> @@ -166,6 +167,7 @@ REMAP1(u64, readq, const volatile)
>  REMAP2(u8, iowrite8, /**/)
>  REMAP2(u16, iowrite16, /**/)
>  REMAP2(u32, iowrite32, /**/)
> +REMAP2(u64, iowrite64, /**/)
>  REMAP2(u8, writeb, volatile)
>  REMAP2(u16, writew, volatile)
>  REMAP2(u32, writel, volatile)
> @@ -384,6 +386,19 @@ extern inline void iowrite32(u32 b, void __iomem *addr)
>  	mb();
>  }
>  
> +extern inline u64 ioread64(void __iomem *addr)
> +{
> +	unsigned int ret = IO_CONCAT(__IO_PREFIX,ioread64)(addr);
> +	mb();
> +	return ret;
> +}
> +
> +extern inline void iowrite64(u64 b, void __iomem *addr)
> +{
> +	IO_CONCAT(__IO_PREFIX,iowrite64)(b, addr);
> +	mb();
> +}
> +
>  extern inline u32 inl(unsigned long port)
>  {
>  	return ioread32(ioport_map(port, 4));
> @@ -393,6 +408,16 @@ extern inline void outl(u32 b, unsigned long port)
>  {
>  	iowrite32(b, ioport_map(port, 4));
>  }
> +
> +extern inline u64 inq(unsigned long port)
> +{
> +	return ioread64(ioport_map(port, 8));
> +}
> +
> +extern inline void outq(u64 b, unsigned long port)
> +{
> +	iowrite64(b, ioport_map(port, 8));
> +}
>  #endif
>  
>  #if IO_CONCAT(__IO_PREFIX,trivial_rw_bw) == 1
> @@ -493,8 +518,10 @@ extern inline void writeq(u64 b, volatile void __iomem *addr)
>  
>  #define ioread16be(p) be16_to_cpu(ioread16(p))
>  #define ioread32be(p) be32_to_cpu(ioread32(p))
> +#define ioread64be(p) be64_to_cpu(ioread64(p))
>  #define iowrite16be(v,p) iowrite16(cpu_to_be16(v), (p))
>  #define iowrite32be(v,p) iowrite32(cpu_to_be32(v), (p))
> +#define iowrite64be(v,p) iowrite64(cpu_to_be64(v), (p))
>  
>  #define inb_p		inb
>  #define inw_p		inw
> diff --git a/arch/alpha/include/asm/io_trivial.h b/arch/alpha/include/asm/io_trivial.h
> index 1c77f10..ae97730 100644
> --- a/arch/alpha/include/asm/io_trivial.h
> +++ b/arch/alpha/include/asm/io_trivial.h
> @@ -37,11 +37,23 @@ IO_CONCAT(__IO_PREFIX,ioread32)(void __iomem *a)
>  	return *(volatile u32 __force *)a;
>  }
>  
> +__EXTERN_INLINE u64
> +IO_CONCAT(__IO_PREFIX,ioread64)(void __iomem *a)
> +{
> +	return *(volatile u64 __force *)a;
> +}
> +
>  __EXTERN_INLINE void
>  IO_CONCAT(__IO_PREFIX,iowrite32)(u32 b, void __iomem *a)
>  {
>  	*(volatile u32 __force *)a = b;
>  }
> +
> +__EXTERN_INLINE void
> +IO_CONCAT(__IO_PREFIX,iowrite64)(u64 b, void __iomem *a)
> +{
> +	*(volatile u64 __force *)a = b;
> +}
>  #endif
>  
>  #if IO_CONCAT(__IO_PREFIX,trivial_rw_bw) == 1
> diff --git a/arch/alpha/include/asm/jensen.h b/arch/alpha/include/asm/jensen.h
> index 964b06e..7e945cf 100644
> --- a/arch/alpha/include/asm/jensen.h
> +++ b/arch/alpha/include/asm/jensen.h
> @@ -182,6 +182,17 @@ __EXTERN_INLINE u32 jensen_inl(unsigned long addr)
>  	return *(vuip) ((addr << 7) + EISA_IO + 0x60);
>  }
>  
> +__EXTERN_INLINE u64 jensen_inq(unsigned long addr)
> +{
> +	unsigned long ioaddr = (addr << 7) + EISA_IO + 0x60;
> +	unsigned long l, h;
> +
> +	jensen_set_hae(0);
> +	l = *(vuip)ioaddr;
> +	h = *(vuip)(ioaddr + (4 << 7));
> +	return h << 32 | l;
> +}
> +
>  __EXTERN_INLINE void jensen_outw(u16 b, unsigned long addr)
>  {
>  	jensen_set_hae(0);
> @@ -196,6 +207,16 @@ __EXTERN_INLINE void jensen_outl(u32 b, unsigned long addr)
>  	mb();
>  }
>  
> +__EXTERN_INLINE void jensen_outq(u64 b, unsigned long addr)
> +{
> +	unsigned long ioaddr = (addr << 7) + EISA_IO + 0x60;
> +
> +	jensen_set_hae(0);
> +	*(vuip)ioaddr = b;
> +	*(vuip)(ioaddr + (4 << 7)) = b >> 32;
> +	mb();
> +}
> +
>  /*
>   * Memory functions.
>   */
> @@ -303,8 +324,8 @@ __EXTERN_INLINE int jensen_is_mmio(const volatile void __iomem *addr)
>  /* New-style ioread interface.  All the routines are so ugly for Jensen
>     that it doesn't make sense to merge them.  */
>  
> -#define IOPORT(OS, NS)							\
> -__EXTERN_INLINE unsigned int jensen_ioread##NS(void __iomem *xaddr)	\
> +#define IOPORT(OS, NS, RT)						\
> +__EXTERN_INLINE RT jensen_ioread##NS(void __iomem *xaddr)		\
>  {									\
>  	if (jensen_is_mmio(xaddr))					\
>  		return jensen_read##OS(xaddr - 0x100000000ul);		\
> @@ -319,9 +340,10 @@ __EXTERN_INLINE void jensen_iowrite##NS(u##NS b, void __iomem *xaddr)	\
>  		jensen_out##OS(b, (unsigned long)xaddr);		\
>  }
>  
> -IOPORT(b, 8)
> -IOPORT(w, 16)
> -IOPORT(l, 32)
> +IOPORT(b, 8, unsigned int)
> +IOPORT(w, 16, unsigned int)
> +IOPORT(l, 32, unsigned int)
> +IOPORT(q, 64, u64)
>  
>  #undef IOPORT
>  
> diff --git a/arch/alpha/include/asm/machvec.h b/arch/alpha/include/asm/machvec.h
> index 75cb364..c037c7b 100644
> --- a/arch/alpha/include/asm/machvec.h
> +++ b/arch/alpha/include/asm/machvec.h
> @@ -48,10 +48,12 @@ struct alpha_machine_vector
>  	unsigned int (*mv_ioread8)(void __iomem *);
>  	unsigned int (*mv_ioread16)(void __iomem *);
>  	unsigned int (*mv_ioread32)(void __iomem *);
> +	u64 (*mv_ioread64)(void __iomem *);
>  
>  	void (*mv_iowrite8)(u8, void __iomem *);
>  	void (*mv_iowrite16)(u16, void __iomem *);
>  	void (*mv_iowrite32)(u32, void __iomem *);
> +	void (*mv_iowrite64)(u64, void __iomem *);
>  
>  	u8 (*mv_readb)(const volatile void __iomem *);
>  	u16 (*mv_readw)(const volatile void __iomem *);
> diff --git a/arch/alpha/kernel/io.c b/arch/alpha/kernel/io.c
> index 19c5875..d36146f 100644
> --- a/arch/alpha/kernel/io.c
> +++ b/arch/alpha/kernel/io.c
> @@ -34,6 +34,13 @@ unsigned int ioread32(void __iomem *addr)
>  	return ret;
>  }
>  
> +u64 ioread64(void __iomem *addr)
> +{
> +	u64 ret = IO_CONCAT(__IO_PREFIX,ioread64)(addr);
> +	mb();
> +	return ret;
> +}
> +
>  void iowrite8(u8 b, void __iomem *addr)
>  {
>  	IO_CONCAT(__IO_PREFIX,iowrite8)(b, addr);
> @@ -52,12 +59,20 @@ void iowrite32(u32 b, void __iomem *addr)
>  	mb();
>  }
>  
> +void iowrite64(u64 b, void __iomem *addr)
> +{
> +	IO_CONCAT(__IO_PREFIX,iowrite64)(b, addr);
> +	mb();
> +}
> +
>  EXPORT_SYMBOL(ioread8);
>  EXPORT_SYMBOL(ioread16);
>  EXPORT_SYMBOL(ioread32);
> +EXPORT_SYMBOL(ioread64);
>  EXPORT_SYMBOL(iowrite8);
>  EXPORT_SYMBOL(iowrite16);
>  EXPORT_SYMBOL(iowrite32);
> +EXPORT_SYMBOL(iowrite64);
>  
>  u8 inb(unsigned long port)
>  {
> @@ -74,6 +89,11 @@ u32 inl(unsigned long port)
>  	return ioread32(ioport_map(port, 4));
>  }
>  
> +u64 inq(unsigned long port)
> +{
> +	return ioread64(ioport_map(port, 8));
> +}
> +
>  void outb(u8 b, unsigned long port)
>  {
>  	iowrite8(b, ioport_map(port, 1));
> @@ -89,12 +109,19 @@ void outl(u32 b, unsigned long port)
>  	iowrite32(b, ioport_map(port, 4));
>  }
>  
> +void outq(u64 b, unsigned long port)
> +{
> +	iowrite64(b, ioport_map(port, 8));
> +}
> +
>  EXPORT_SYMBOL(inb);
>  EXPORT_SYMBOL(inw);
>  EXPORT_SYMBOL(inl);
> +EXPORT_SYMBOL(inq);
>  EXPORT_SYMBOL(outb);
>  EXPORT_SYMBOL(outw);
>  EXPORT_SYMBOL(outl);
> +EXPORT_SYMBOL(outq);
>  
>  u8 __raw_readb(const volatile void __iomem *addr)
>  {
> diff --git a/arch/alpha/kernel/machvec_impl.h b/arch/alpha/kernel/machvec_impl.h
> index b7d6960..9b37e19 100644
> --- a/arch/alpha/kernel/machvec_impl.h
> +++ b/arch/alpha/kernel/machvec_impl.h
> @@ -79,9 +79,11 @@
>  	.mv_ioread8 =		CAT(low,_ioread8),			\
>  	.mv_ioread16 =		CAT(low,_ioread16),			\
>  	.mv_ioread32 =		CAT(low,_ioread32),			\
> +	.mv_ioread64 =		CAT(low,_ioread64),			\
>  	.mv_iowrite8 =		CAT(low,_iowrite8),			\
>  	.mv_iowrite16 =		CAT(low,_iowrite16),			\
>  	.mv_iowrite32 =		CAT(low,_iowrite32),			\
> +	.mv_iowrite64 =		CAT(low,_iowrite64),			\
>  	.mv_readb =		CAT(low,_readb),			\
>  	.mv_readw =		CAT(low,_readw),			\
>  	.mv_readl =		CAT(low,_readl),			\
> 

Re: [PATCH] alpha: provide ioread64 and iowrite64 implementations

[Richard Henderson]

On 06/24/2017 08:19 AM, Logan Gunthorpe wrote:
> Hey,
> 
> On 24/06/17 09:13 AM, Richard Henderson wrote:
>> All Alpha hosts except for Jensen provide 64-bit I/O operations.
>>
>> Jensen is EISA only, so there ought not be any devices that even
>> attempt such operations.  But just in case, use 2 32-bit operations.
> 
> Thanks for this, Richard.
> 
> However, I was recently enlightened by the existence of the
> linux/io-64-nonatomic* headers. This is where the 2 32-bit operation
> functions belong. So you should probably remove them from this patch and
> let drivers that need them just include those headers.

This has nothing to do with drivers.

This is all about providing 64-bit operations for the 99.999% of alphas that do 
have them.


r~

Re: [PATCH] alpha: provide ioread64 and iowrite64 implementations

[Logan Gunthorpe]

On 24/06/17 09:25 AM, Richard Henderson wrote:
> On 06/24/2017 08:19 AM, Logan Gunthorpe wrote:
>> Hey,
>>
>> On 24/06/17 09:13 AM, Richard Henderson wrote:
>>> All Alpha hosts except for Jensen provide 64-bit I/O operations.
>>>
>>> Jensen is EISA only, so there ought not be any devices that even
>>> attempt such operations.  But just in case, use 2 32-bit operations.
>>
>> Thanks for this, Richard.
>>
>> However, I was recently enlightened by the existence of the
>> linux/io-64-nonatomic* headers. This is where the 2 32-bit operation
>> functions belong. So you should probably remove them from this patch and
>> let drivers that need them just include those headers.
> 
> This has nothing to do with drivers.
> 
> This is all about providing 64-bit operations for the 99.999% of alphas
> that do have them.

As per the discussion with Arnd and Alan, that's not what they want.
Arches that have native 64bit operations should supply them. For drivers
that want to use them in the presence of arches that don't supply them,
they simply include one of the linux/io-64-nonatomic helper headers.

So, IMO, the Jensen inq and outq funtions in your patch, which I did
read, should probably just not be provided.

Logan

Re: [PATCH] alpha: provide ioread64 and iowrite64 implementations

[Richard Henderson]

On 06/24/2017 08:32 AM, Logan Gunthorpe wrote:
> So, IMO, the Jensen inq and outq funtions in your patch, which I did
> read, should probably just not be provided.

So you would prefer a function containing BUG?
Because I have to have *something* to put into the machvec function pointer.


r~

Re: [PATCH] alpha: provide ioread64 and iowrite64 implementations

[Logan Gunthorpe]

On 24/06/17 10:14 AM, Richard Henderson wrote:
> So you would prefer a function containing BUG?

Don't be so obtuse. Of course not.

> Because I have to have *something* to put into the machvec function
> pointer.

Well I can't say I understand the point of the machvec code, but if
you're saying it's impossible to have the Jensen sub-arch not supply a
64bit io functions then, fine, leave it as is.

Logan