On Fri, Sep 27, 2024, at 08:19, Julian Vetter wrote:
> On 26.09.24 09:14, Arnd Bergmann wrote:
>>> +#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?
Yes
> 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'?
Yes, I think that would be the best way to do it. Alternatively,
arch/um could provide a dummy implementation of these.
>> 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.
Sorry for being unclear, I meant only the architectures that
you are already touching.
> 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?!
Ideally we should end up with only one copy, but I'd leave the
rest for a future cleanup. In particular, alpha probably still
needs a custom function.
Arnd
