Hi Geert

Am 10.05.23 um 14:34 schrieb Geert Uytterhoeven:
> Hi Thomas,
> 
> On Wed, May 10, 2023 at 1:06 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Implement framebuffer I/O helpers, such as fb_read*() and fb_write*(),
>> in the architecture's <asm/fb.h> header file or the generic one.
>>
>> The common case has been the use of regular I/O functions, such as
>> __raw_readb() or memset_io(). A few architectures used plain system-
>> memory reads and writes. Sparc used helpers for its SBus.
>>
>> The architectures that used special cases provide the same code in
>> their __raw_*() I/O helpers. So the patch replaces this code with the
>> __raw_*() functions and moves it to <asm-generic/fb.h> for all
>> architectures.
>>
>> v6:
>>          * fix fb_readq()/fb_writeq() on 64-bit mips (kernel test robot)
>> v5:
>>          * include <linux/io.h> in <asm-generic/fb>; fix s390 build
>> v4:
>>          * ia64, loongarch, sparc64: add fb_mem*() to arch headers
>>            to keep current semantics (Arnd)
>> v3:
>>          * implement all architectures with generic helpers
>>          * support reordering and native byte order (Geert, Arnd)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Tested-by: Sui Jingfeng <suijingfeng@loongson.cn>
>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>>
>> add mips fb_q()

Oops, left-over fom squashing patches.

> 
>> --- a/arch/mips/include/asm/fb.h
>> +++ b/arch/mips/include/asm/fb.h
>> @@ -12,6 +12,28 @@ static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
>>   }
>>   #define fb_pgprotect fb_pgprotect
>>
>> +/*
>> + * MIPS doesn't define __raw_ I/O macros, so the helpers
>> + * in <asm-generic/fb.h> don't generate fb_readq() and
>> + * fb_write(). We have to provide them here.
> 
> MIPS does not include <asm-generic/io.h>,  nor define its own

I know, that's why the TODO says to convert it to generic I/O.

> __raw_readq() and __raw_writeq()...

It doesn't define those macros, but it generates function calls of the 
same names. Follow the macros at

 
https://elixir.bootlin.com/linux/latest/source/arch/mips/include/asm/io.h#L357

It expands to a variety of helpers, including __raw_*().

> 
>> + *
>> + * TODO: Convert MIPS to generic I/O. The helpers below can
>> + *       then be removed.
>> + */
>> +#ifdef CONFIG_64BIT
>> +static inline u64 fb_readq(const volatile void __iomem *addr)
>> +{
>> +       return __raw_readq(addr);
> 
> ... so how can this call work?

On 64-bit builds, there's __raw_readq() and __raw_writeq().

At first, I tried to do the right thing and convert MIPS to work with 
<asm-generic/io.h>. But that created a ton of follow-up errors in other 
headers. So for now, it's better to handle this problem in asm/fb.h.

Best regards
Thomas

> 
>> +}
>> +#define fb_readq fb_readq
>> +
>> +static inline void fb_writeq(u64 b, volatile void __iomem *addr)
>> +{
>> +       __raw_writeq(b, addr);
>> +}
>> +#define fb_writeq fb_writeq
>> +#endif
>> +
>>   #include <asm-generic/fb.h>
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)