Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1672847 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations Logan Gunthorpe <logang@deltatee.com> - 2017-06-22 19:00 +0200
    Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations "Stephen  Bates" <sbates@raithlin.com> - 2017-06-22 19:30 +0200
      Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations Logan Gunthorpe <logang@deltatee.com> - 2017-06-22 19:40 +0200
    Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64  implementations Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-22 22:10 +0200
      Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations Logan Gunthorpe <logang@deltatee.com> - 2017-06-22 22:20 +0200
        Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations Arnd Bergmann <arnd@arndb.de> - 2017-06-22 23:10 +0200
          Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations Logan Gunthorpe <logang@deltatee.com> - 2017-06-22 23:20 +0200
    Re: [PATCH 4/7] alpha: provide ioread64 and iowrite64 implementations Richard Henderson <rth@twiddle.net> - 2017-06-22 23:30 +0200

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

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

[toc] | [next] | [standalone]

#1672902

> +#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

[toc] | [prev] | [next] | [standalone]

#1672911

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

#1673025

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

[toc] | [prev] | [next] | [standalone]

#1673050

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

[toc] | [prev] | [next] | [standalone]

#1673052

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

[toc] | [prev] | [next] | [standalone]

#1673058

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web