On 26.09.24 09:14, Arnd Bergmann wrote:
On Wed, Sep 25, 2024, at 13:24, Julian Vetter wrote:
Various architectures have almost the same implementations for
__memcpy_{to,from}io and __memset_io functions. So, consolidate them
into the existing lib/iomap_copy.c.
Reviewed-by: Yann Sionneau <ysionneau@xxxxxxxxxxxxx>
Signed-off-by: Julian Vetter <jvetter@xxxxxxxxxxxxx>
---
Signed-off-by: Julian Vetter <jvetter@xxxxxxxxxxxxx>
You have a duplicated signoff here.
Yes, thank you. I will remove it in the next patch revision.
+#ifndef __memcpy_fromio
+void __memcpy_fromio(void *to, const volatile void __iomem *from,
size_t count);
+#endif
+
+#ifndef __memcpy_toio
+void __memcpy_toio(volatile void __iomem *to, const void *from, size_t
count);
+#endif
+
+#ifndef __memset_io
+void __memset_io(volatile void __iomem *dst, int c, size_t count);
+#endif
I'm not entirely sure about the purpose of the #ifdef here, since
nothing ever overrides the double-underscore versions, both before
and after your patches.
Unless I'm missing something here, I think a more logical
sequence would be:
1. add the definitions in this file without the underscores,
by: "...in this file..." you mean the 'lib/iomap_copy.c' file, right?
But what if an architecture does not select 'CONFIG_HAS_IOMEM'. Then
'iomap_copy.c' is not compiled and we don't have an implementation,
right? I tried to compile with ARCH=um, with some MTD chip driver, like
the robot did and it indeed fails, because um has 'NO_IOMEM' set. and
the driver uses memcpy_fromio. I mean it's a strange combination,
because apparently we try to use IO memory? Is this an invalid
combination? But shouldn't the driver then 'depends on HAS_IOMEM'?
as memcpy_fromio/memcpy_toio/memset_io, with the #ifdef
for that name that is always set at this point
Right. I will remove it in my next patch revision.
2. replace the default implementation in asm-generic/io.h
with extern prototypes, remove the #define from those
Yes, I have done this now.
3. convert the other architectures, removing both the
implementations and the prototypes.
I have removed the prototypes and have aligned the function arguments in
m68k, alpha, parisc, and sh, which all have their own implementation,
but had slightly different function arguments. Btw, I have not removed
their implementations because some of them seem to have optimized
implementations (e.g., alpha and m68k), that I didn't want to touch. But
you're right others (e.g., sh) just do byte wise accesses and have a
comment "This needs to be optimized." Maybe I should remove these and
let them use the new version?!
Arnd
