Hi Thomas,

On Wed, May 10, 2023 at 4:20 PM Thomas Zimmermann [off-list ref] wrote:

Am 10.05.23 um 14:34 schrieb Geert Uytterhoeven:

quoted

On Wed, May 10, 2023 at 1:06 PM Thomas Zimmermann [off-list ref] wrote:

quoted

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 <redacted>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>

quoted

quoted

--- 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.

quoted

__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_*().

Thanks, I forgot MIPS is using these grep-unfriendly factories...

quoted

quoted

+ *
+ * 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.

So isn't just adding

    #define __raw_readq __raw_readq
    #define __raw_writeq __raw_writeq

to arch/mips/include/asm/io.h sufficient to make <asm-generic/fb.h>
do the right thing?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds