[PATCH] parisc: Remove memcpy_fromio

[PATCH] parisc: Remove memcpy_fromio

[Julian Vetter]

Fully migrate parisc to the IO functions from lib/iomem_copy.c. In a
recent patch the functions memset_io and memcpy_toio were removed, but
the memcpy_fromio was kept, because for very short sequences it does
half word accesses, whereas the functions in lib/iomem_copy.c do byte
accesses until the memory is naturally aligned and then do machine word
accesses. But I don't think the single half-word access merits keeping
the arch specific implementation, so, remove it as well.

Signed-off-by: Julian Vetter <julian@outer-limits.org>
---
 arch/parisc/include/asm/io.h      |  3 --
 arch/parisc/kernel/parisc_ksyms.c |  1 -
 arch/parisc/lib/io.c              | 61 -------------------------------
 3 files changed, 65 deletions(-)

diff --git a/arch/parisc/include/asm/io.h b/arch/parisc/include/asm/io.h
index 3143cf29ce27..61173a2b38e4 100644
--- a/arch/parisc/include/asm/io.h
+++ b/arch/parisc/include/asm/io.h
@@ -135,9 +135,6 @@ static inline void gsc_writeq(unsigned long long val, unsigned long addr)
 
 #define pci_iounmap			pci_iounmap
 
-void memcpy_fromio(void *dst, const volatile void __iomem *src, int count);
-#define memcpy_fromio memcpy_fromio
-
 /* Port-space IO */
 
 #define inb_p inb
diff --git a/arch/parisc/kernel/parisc_ksyms.c b/arch/parisc/kernel/parisc_ksyms.c
index 1c366b0d3134..509146a52725 100644
--- a/arch/parisc/kernel/parisc_ksyms.c
+++ b/arch/parisc/kernel/parisc_ksyms.c
@@ -43,7 +43,6 @@ EXPORT_SYMBOL($global$);
 #endif
 
 #include <asm/io.h>
-EXPORT_SYMBOL(memcpy_fromio);
 
 extern void $$divI(void);
 extern void $$divU(void);
diff --git a/arch/parisc/lib/io.c b/arch/parisc/lib/io.c
index 6e81200dc87a..3c7e617f5a93 100644
--- a/arch/parisc/lib/io.c
+++ b/arch/parisc/lib/io.c
@@ -12,67 +12,6 @@
 #include <linux/module.h>
 #include <asm/io.h>
 
-/*
-** Copies a block of memory from a device in an efficient manner.
-** Assumes the device can cope with 32-bit transfers.  If it can't,
-** don't use this function.
-**
-** CR16 counts on C3000 reading 256 bytes from Symbios 896 RAM:
-**	27341/64    = 427 cyc per int
-**	61311/128   = 478 cyc per short
-**	122637/256  = 479 cyc per byte
-** Ergo bus latencies dominant (not transfer size).
-**      Minimize total number of transfers at cost of CPU cycles.
-**	TODO: only look at src alignment and adjust the stores to dest.
-*/
-void memcpy_fromio(void *dst, const volatile void __iomem *src, int count)
-{
-	/* first compare alignment of src/dst */ 
-	if ( (((unsigned long)dst ^ (unsigned long)src) & 1) || (count < 2) )
-		goto bytecopy;
-
-	if ( (((unsigned long)dst ^ (unsigned long)src) & 2) || (count < 4) )
-		goto shortcopy;
-
-	/* Then check for misaligned start address */
-	if ((unsigned long)src & 1) {
-		*(u8 *)dst = readb(src);
-		src++;
-		dst++;
-		count--;
-		if (count < 2) goto bytecopy;
-	}
-
-	if ((unsigned long)src & 2) {
-		*(u16 *)dst = __raw_readw(src);
-		src += 2;
-		dst += 2;
-		count -= 2;
-	}
-
-	while (count > 3) {
-		*(u32 *)dst = __raw_readl(src);
-		dst += 4;
-		src += 4;
-		count -= 4;
-	}
-
- shortcopy:
-	while (count > 1) {
-		*(u16 *)dst = __raw_readw(src);
-		src += 2;
-		dst += 2;
-		count -= 2;
-	}
-
- bytecopy:
-	while (count--) {
-		*(char *)dst = readb(src);
-		src++;
-		dst++;
-	}
-}
-
 /*
  * Read COUNT 8-bit bytes from port PORT into memory starting at
  * SRC.
-- 
2.34.1

Re: [PATCH] parisc: Remove memcpy_fromio

[Arnd Bergmann]

On Thu, Jan 30, 2025, at 14:48, Julian Vetter wrote:
> Fully migrate parisc to the IO functions from lib/iomem_copy.c. In a
> recent patch the functions memset_io and memcpy_toio were removed, but
> the memcpy_fromio was kept, because for very short sequences it does
> half word accesses, whereas the functions in lib/iomem_copy.c do byte
> accesses until the memory is naturally aligned and then do machine word
> accesses. But I don't think the single half-word access merits keeping
> the arch specific implementation, so, remove it as well.
>
> Signed-off-by: Julian Vetter <julian@outer-limits.org>

Acked-by: Arnd Bergmann <arnd@arndb.de>

This one looks fairly obvious. It might be nice to also
clean up the {in,out}s{b,w,l} helper functions in the same
file, but I don't understand why those are special
in the first place.

Those functions have been unchanged since before the git
history and there are some comments that I don't find helpful.
One thing they do is to deal with unaligned memory buffers,
which the generic ones don't, but that could be easily added
using get_unaligned/put_unaligned, expecting the compiler
to optimize the memory side of the transfer.

     Arnd

Re: [PATCH] parisc: Remove memcpy_fromio

[Julian Vetter]

On 1/31/25 08:28, Arnd Bergmann wrote:
> On Thu, Jan 30, 2025, at 14:48, Julian Vetter wrote:
>> Fully migrate parisc to the IO functions from lib/iomem_copy.c. In a
>> recent patch the functions memset_io and memcpy_toio were removed, but
>> the memcpy_fromio was kept, because for very short sequences it does
>> half word accesses, whereas the functions in lib/iomem_copy.c do byte
>> accesses until the memory is naturally aligned and then do machine word
>> accesses. But I don't think the single half-word access merits keeping
>> the arch specific implementation, so, remove it as well.
>>
>> Signed-off-by: Julian Vetter <julian@outer-limits.org>
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> This one looks fairly obvious. It might be nice to also
> clean up the {in,out}s{b,w,l} helper functions in the same
> file, but I don't understand why those are special
> in the first place.
> 
Not sure, either. But some comments contain: "just using the inlined 
version of the inw() breaks things". This is clear, and what you point 
out below already, these parisc specific functions deal with unaligned 
buffers, so using, e.g., the generic "inw()" will surely break things.

> Those functions have been unchanged since before the git
> history and there are some comments that I don't find helpful.
> One thing they do is to deal with unaligned memory buffers,
> which the generic ones don't, but that could be easily added
> using get_unaligned/put_unaligned, expecting the compiler
> to optimize the memory side of the transfer.

I'm not sure what you suggest. Do you mean adding 
get_unaligned/put_unaligned in the generic include/asm-generic/io.h 
functions, or just adding get_unaligned/put_unaligned to the body of the 
parisc specific functions? Instead of all the "switch... case 0x2..." code?

Julian

> 
>       Arnd

Re: [PATCH] parisc: Remove memcpy_fromio

[Arnd Bergmann]

On Mon, Feb 3, 2025, at 11:21, Julian Vetter wrote:
> On 1/31/25 08:28, Arnd Bergmann wrote:
>> Those functions have been unchanged since before the git
>> history and there are some comments that I don't find helpful.
>> One thing they do is to deal with unaligned memory buffers,
>> which the generic ones don't, but that could be easily added
>> using get_unaligned/put_unaligned, expecting the compiler
>> to optimize the memory side of the transfer.
>
> I'm not sure what you suggest. Do you mean adding 
> get_unaligned/put_unaligned in the generic include/asm-generic/io.h 
> functions, or just adding get_unaligned/put_unaligned to the body of the 
> parisc specific functions? Instead of all the "switch... case 0x2..." code?

I meant adding put_unaligned/get_unaligned like this:

--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -422,7 +422,8 @@ static inline void readsw(const volatile void __iomem *addr, void *buffer,
 
                do {
                        u16 x = __raw_readw(addr);
-                       *buf++ = x;
+                       put_unaligned(x, buf);
+                       buf++;
                } while (--count);
        }
 }

This has no effect on architectures that can handle unaligned
load/store on memory, and should behave the same way as the
parisc code otherwise. On powerpc and possibly others, there
needs to be a barrier (eieio() on powerpc) inside of the loop,
according to Segher's earlier comments.

     Arnd

Re: [PATCH] parisc: Remove memcpy_fromio

[Helge Deller]

On 1/31/25 08:28, Arnd Bergmann wrote:
> On Thu, Jan 30, 2025, at 14:48, Julian Vetter wrote:
>> Fully migrate parisc to the IO functions from lib/iomem_copy.c. In a
>> recent patch the functions memset_io and memcpy_toio were removed, but
>> the memcpy_fromio was kept, because for very short sequences it does
>> half word accesses, whereas the functions in lib/iomem_copy.c do byte
>> accesses until the memory is naturally aligned and then do machine word
>> accesses. But I don't think the single half-word access merits keeping
>> the arch specific implementation, so, remove it as well.
>>
>> Signed-off-by: Julian Vetter <julian@outer-limits.org>
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Thanks!
I've added the patch to the parisc git tree.

Helge