2023-05-12 10:27:06

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH v7 0/7] fbdev: Move framebuffer I/O helpers to <asm/fb.h>

Fbdev provides helpers for framebuffer I/O, such as fb_readl(),
fb_writel() or fb_memcpy_to_fb(). The implementation of each helper
depends on the architecture, but they are all equivalent to regular
I/O functions of similar names. So use regular functions instead and
move all helpers into <asm-generic/fb.h>

The first two patch are driver cleanups.

Until now, <linux/fb.h> contained an include of <asm/io.h>. As this
will go away, patches 3 to 5 prepare include statements in the various
drivers. Source files that use regular I/O helpers, such as readl(),
now include <linux/io.h>. Source files that use framebuffer I/O
helpers, such as fb_readl(), now include <linux/fb.h>.

Patch 6 replaces the architecture-based if-else branching in
<linux/fb.h> by helpers in <asm-generic/fb.h>. All helpers use Linux'
existing I/O functions.

Patch 7 harmonizes naming among fbdev and existing I/O functions.

The patchset has been built for a variety of platforms, such as x86-64,
arm, aarch64, ppc64, parisc, m64k, mips and sparc.

v7:
* fix hitfb build warnings (kernel test robot)
v6:
* fix build on 64-bit mips (kernel test robot)
* update fb_io_fops.c
v5:
* fix build on s390
v4:
* keep fb_mem*() as-is on ia64, loongarch, sparc64 (Arnd)
* don't include <asm/fb.h> (Sam)
v3:
* add the new helpers in <asm-generic/fb.h>
* support reordering and native byte order (Geert, Arnd)
v2:
* use Linux I/O helpers (Sam, Arnd)

Thomas Zimmermann (7):
fbdev/hitfb: Cast I/O offset to address
fbdev/matrox: Remove trailing whitespaces
ipu-v3: Include <linux/io.h>
fbdev: Include <linux/io.h> in various drivers
fbdev: Include <linux/fb.h> instead of <asm/fb.h>
fbdev: Move framebuffer I/O helpers into <asm/fb.h>
fbdev: Rename fb_mem*() helpers

arch/ia64/include/asm/fb.h | 20 ++++
arch/loongarch/include/asm/fb.h | 21 ++++
arch/mips/include/asm/fb.h | 22 ++++
arch/parisc/video/fbdev.c | 3 +-
arch/sparc/include/asm/fb.h | 20 ++++
arch/sparc/video/fbdev.c | 1 -
arch/x86/video/fbdev.c | 2 -
drivers/gpu/ipu-v3/ipu-prv.h | 1 +
drivers/staging/sm750fb/sm750.c | 2 +-
drivers/video/fbdev/arcfb.c | 1 +
drivers/video/fbdev/aty/atyfb.h | 2 +
drivers/video/fbdev/aty/mach64_cursor.c | 2 +-
drivers/video/fbdev/chipsfb.c | 2 +-
drivers/video/fbdev/core/fb_io_fops.c | 4 +-
drivers/video/fbdev/core/fbcon.c | 1 -
drivers/video/fbdev/core/fbmem.c | 2 -
drivers/video/fbdev/hitfb.c | 122 +++++++++++---------
drivers/video/fbdev/kyro/fbdev.c | 2 +-
drivers/video/fbdev/matrox/matroxfb_accel.c | 6 +-
drivers/video/fbdev/matrox/matroxfb_base.h | 4 +-
drivers/video/fbdev/pvr2fb.c | 2 +-
drivers/video/fbdev/sstfb.c | 2 +-
drivers/video/fbdev/stifb.c | 4 +-
drivers/video/fbdev/tdfxfb.c | 2 +-
drivers/video/fbdev/wmt_ge_rops.c | 2 +
include/asm-generic/fb.h | 102 ++++++++++++++++
include/linux/fb.h | 55 +--------
27 files changed, 279 insertions(+), 130 deletions(-)

--
2.40.1



2023-05-12 10:27:25

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH v7 6/7] fbdev: Move framebuffer I/O helpers into <asm/fb.h>

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 <[email protected]>
Tested-by: Sui Jingfeng <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>

add mips fb_q()
---
arch/ia64/include/asm/fb.h | 20 +++++++
arch/loongarch/include/asm/fb.h | 21 +++++++
arch/mips/include/asm/fb.h | 22 +++++++
arch/sparc/include/asm/fb.h | 20 +++++++
include/asm-generic/fb.h | 102 ++++++++++++++++++++++++++++++++
include/linux/fb.h | 53 -----------------
6 files changed, 185 insertions(+), 53 deletions(-)

diff --git a/arch/ia64/include/asm/fb.h b/arch/ia64/include/asm/fb.h
index 0208f64a0da0..bcf982043a5c 100644
--- a/arch/ia64/include/asm/fb.h
+++ b/arch/ia64/include/asm/fb.h
@@ -2,7 +2,9 @@
#ifndef _ASM_FB_H_
#define _ASM_FB_H_

+#include <linux/compiler.h>
#include <linux/efi.h>
+#include <linux/string.h>

#include <asm/page.h>

@@ -18,6 +20,24 @@ static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
}
#define fb_pgprotect fb_pgprotect

+static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+{
+ memcpy(to, (void __force *)from, n);
+}
+#define fb_memcpy_fromfb fb_memcpy_fromfb
+
+static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+{
+ memcpy((void __force *)to, from, n);
+}
+#define fb_memcpy_tofb fb_memcpy_tofb
+
+static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+{
+ memset((void __force *)addr, c, n);
+}
+#define fb_memset fb_memset
+
#include <asm-generic/fb.h>

#endif /* _ASM_FB_H_ */
diff --git a/arch/loongarch/include/asm/fb.h b/arch/loongarch/include/asm/fb.h
index ff82f20685c8..c6fc7ef374a4 100644
--- a/arch/loongarch/include/asm/fb.h
+++ b/arch/loongarch/include/asm/fb.h
@@ -5,6 +5,27 @@
#ifndef _ASM_FB_H_
#define _ASM_FB_H_

+#include <linux/compiler.h>
+#include <linux/string.h>
+
+static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+{
+ memcpy(to, (void __force *)from, n);
+}
+#define fb_memcpy_fromfb fb_memcpy_fromfb
+
+static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+{
+ memcpy((void __force *)to, from, n);
+}
+#define fb_memcpy_tofb fb_memcpy_tofb
+
+static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+{
+ memset((void __force *)addr, c, n);
+}
+#define fb_memset fb_memset
+
#include <asm-generic/fb.h>

#endif /* _ASM_FB_H_ */
diff --git a/arch/mips/include/asm/fb.h b/arch/mips/include/asm/fb.h
index 6bda0a81d8ca..18b7226403ba 100644
--- 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.
+ *
+ * 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);
+}
+#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>

#endif /* _ASM_FB_H_ */
diff --git a/arch/sparc/include/asm/fb.h b/arch/sparc/include/asm/fb.h
index 689ee5c60054..077da91aeba1 100644
--- a/arch/sparc/include/asm/fb.h
+++ b/arch/sparc/include/asm/fb.h
@@ -2,6 +2,8 @@
#ifndef _SPARC_FB_H_
#define _SPARC_FB_H_

+#include <linux/io.h>
+
struct fb_info;
struct file;
struct vm_area_struct;
@@ -16,6 +18,24 @@ static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
int fb_is_primary_device(struct fb_info *info);
#define fb_is_primary_device fb_is_primary_device

+static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+{
+ sbus_memcpy_fromio(to, from, n);
+}
+#define fb_memcpy_fromfb fb_memcpy_fromfb
+
+static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+{
+ sbus_memcpy_toio(to, from, n);
+}
+#define fb_memcpy_tofb fb_memcpy_tofb
+
+static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+{
+ sbus_memset_io(addr, c, n);
+}
+#define fb_memset fb_memset
+
#include <asm-generic/fb.h>

#endif /* _SPARC_FB_H_ */
diff --git a/include/asm-generic/fb.h b/include/asm-generic/fb.h
index c8af99f5a535..0540eccdbeca 100644
--- a/include/asm-generic/fb.h
+++ b/include/asm-generic/fb.h
@@ -7,6 +7,7 @@
* Only include this header file from your architecture's <asm/fb.h>.
*/

+#include <linux/io.h>
#include <linux/mm_types.h>
#include <linux/pgtable.h>

@@ -30,4 +31,105 @@ static inline int fb_is_primary_device(struct fb_info *info)
}
#endif

+/*
+ * I/O helpers for the framebuffer. Prefer these functions over their
+ * regular counterparts. The regular I/O functions provide in-order
+ * access and swap bytes to/from little-endian ordering. Neither is
+ * required for framebuffers. Instead, the helpers read and write
+ * raw framebuffer data. Independent operations can be reordered for
+ * improved performance.
+ */
+
+#ifndef fb_readb
+static inline u8 fb_readb(const volatile void __iomem *addr)
+{
+ return __raw_readb(addr);
+}
+#define fb_readb fb_readb
+#endif
+
+#ifndef fb_readw
+static inline u16 fb_readw(const volatile void __iomem *addr)
+{
+ return __raw_readw(addr);
+}
+#define fb_readw fb_readw
+#endif
+
+#ifndef fb_readl
+static inline u32 fb_readl(const volatile void __iomem *addr)
+{
+ return __raw_readl(addr);
+}
+#define fb_readl fb_readl
+#endif
+
+#ifndef fb_readq
+#if defined(__raw_readq)
+static inline u64 fb_readq(const volatile void __iomem *addr)
+{
+ return __raw_readq(addr);
+}
+#define fb_readq fb_readq
+#endif
+#endif
+
+#ifndef fb_writeb
+static inline void fb_writeb(u8 b, volatile void __iomem *addr)
+{
+ __raw_writeb(b, addr);
+}
+#define fb_writeb fb_writeb
+#endif
+
+#ifndef fb_writew
+static inline void fb_writew(u16 b, volatile void __iomem *addr)
+{
+ __raw_writew(b, addr);
+}
+#define fb_writew fb_writew
+#endif
+
+#ifndef fb_writel
+static inline void fb_writel(u32 b, volatile void __iomem *addr)
+{
+ __raw_writel(b, addr);
+}
+#define fb_writel fb_writel
+#endif
+
+#ifndef fb_writeq
+#if defined(__raw_writeq)
+static inline void fb_writeq(u64 b, volatile void __iomem *addr)
+{
+ __raw_writeq(b, addr);
+}
+#define fb_writeq fb_writeq
+#endif
+#endif
+
+#ifndef fb_memcpy_fromfb
+static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+{
+ memcpy_fromio(to, from, n);
+}
+#define fb_memcpy_fromfb fb_memcpy_fromfb
+#endif
+
+#ifndef fb_memcpy_tofb
+static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+{
+ memcpy_toio(to, from, n);
+}
+#define fb_memcpy_tofb fb_memcpy_tofb
+#endif
+
+#ifndef fb_memset
+static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+{
+ memset_io(addr, c, n);
+}
+#define fb_memset fb_memset
+#endif
+
#endif /* __ASM_GENERIC_FB_H_ */
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 4b4d9a5d200a..2cf8efcb9e32 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -17,7 +17,6 @@
#include <linux/slab.h>

#include <asm/fb.h>
-#include <asm/io.h>

struct vm_area_struct;
struct fb_info;
@@ -513,58 +512,6 @@ struct fb_info {
*/
#define STUPID_ACCELF_TEXT_SHIT

-// This will go away
-#if defined(__sparc__)
-
-/* We map all of our framebuffers such that big-endian accesses
- * are what we want, so the following is sufficient.
- */
-
-// This will go away
-#define fb_readb sbus_readb
-#define fb_readw sbus_readw
-#define fb_readl sbus_readl
-#define fb_readq sbus_readq
-#define fb_writeb sbus_writeb
-#define fb_writew sbus_writew
-#define fb_writel sbus_writel
-#define fb_writeq sbus_writeq
-#define fb_memset sbus_memset_io
-#define fb_memcpy_fromfb sbus_memcpy_fromio
-#define fb_memcpy_tofb sbus_memcpy_toio
-
-#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) || \
- defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || \
- defined(__arm__) || defined(__aarch64__) || defined(__mips__)
-
-#define fb_readb __raw_readb
-#define fb_readw __raw_readw
-#define fb_readl __raw_readl
-#define fb_readq __raw_readq
-#define fb_writeb __raw_writeb
-#define fb_writew __raw_writew
-#define fb_writel __raw_writel
-#define fb_writeq __raw_writeq
-#define fb_memset memset_io
-#define fb_memcpy_fromfb memcpy_fromio
-#define fb_memcpy_tofb memcpy_toio
-
-#else
-
-#define fb_readb(addr) (*(volatile u8 *) (addr))
-#define fb_readw(addr) (*(volatile u16 *) (addr))
-#define fb_readl(addr) (*(volatile u32 *) (addr))
-#define fb_readq(addr) (*(volatile u64 *) (addr))
-#define fb_writeb(b,addr) (*(volatile u8 *) (addr) = (b))
-#define fb_writew(b,addr) (*(volatile u16 *) (addr) = (b))
-#define fb_writel(b,addr) (*(volatile u32 *) (addr) = (b))
-#define fb_writeq(b,addr) (*(volatile u64 *) (addr) = (b))
-#define fb_memset memset
-#define fb_memcpy_fromfb memcpy
-#define fb_memcpy_tofb memcpy
-
-#endif
-
#define FB_LEFT_POS(p, bpp) (fb_be_math(p) ? (32 - (bpp)) : 0)
#define FB_SHIFT_HIGH(p, val, bits) (fb_be_math(p) ? (val) >> (bits) : \
(val) << (bits))
--
2.40.1


2023-05-12 10:28:37

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH v7 7/7] fbdev: Rename fb_mem*() helpers

Update the names of the fb_mem*() helpers to be consistent with their
regular counterparts. Hence, fb_memset() now becomes fb_memset_io(),
fb_memcpy_fromfb() now becomes fb_memcpy_fromio() and fb_memcpy_tofb()
becomes fb_memcpy_toio(). No functional changes.

v6:
* update new file fb_io_fops.c

Signed-off-by: Thomas Zimmermann <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>
Reviewed-by: Sam Ravnborg <[email protected]>
Reviewed-by: Sui Jingfeng <[email protected]>
---
arch/ia64/include/asm/fb.h | 12 ++++++------
arch/loongarch/include/asm/fb.h | 12 ++++++------
arch/sparc/include/asm/fb.h | 12 ++++++------
drivers/video/fbdev/aty/mach64_cursor.c | 2 +-
drivers/video/fbdev/chipsfb.c | 2 +-
drivers/video/fbdev/core/fb_io_fops.c | 4 ++--
drivers/video/fbdev/kyro/fbdev.c | 2 +-
drivers/video/fbdev/pvr2fb.c | 2 +-
drivers/video/fbdev/sstfb.c | 2 +-
drivers/video/fbdev/stifb.c | 4 ++--
drivers/video/fbdev/tdfxfb.c | 2 +-
include/asm-generic/fb.h | 16 ++++++++--------
12 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/arch/ia64/include/asm/fb.h b/arch/ia64/include/asm/fb.h
index bcf982043a5c..1717b26fd423 100644
--- a/arch/ia64/include/asm/fb.h
+++ b/arch/ia64/include/asm/fb.h
@@ -20,23 +20,23 @@ static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
}
#define fb_pgprotect fb_pgprotect

-static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+static inline void fb_memcpy_fromio(void *to, const volatile void __iomem *from, size_t n)
{
memcpy(to, (void __force *)from, n);
}
-#define fb_memcpy_fromfb fb_memcpy_fromfb
+#define fb_memcpy_fromio fb_memcpy_fromio

-static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+static inline void fb_memcpy_toio(volatile void __iomem *to, const void *from, size_t n)
{
memcpy((void __force *)to, from, n);
}
-#define fb_memcpy_tofb fb_memcpy_tofb
+#define fb_memcpy_toio fb_memcpy_toio

-static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+static inline void fb_memset_io(volatile void __iomem *addr, int c, size_t n)
{
memset((void __force *)addr, c, n);
}
-#define fb_memset fb_memset
+#define fb_memset fb_memset_io

#include <asm-generic/fb.h>

diff --git a/arch/loongarch/include/asm/fb.h b/arch/loongarch/include/asm/fb.h
index c6fc7ef374a4..0b218b10a9ec 100644
--- a/arch/loongarch/include/asm/fb.h
+++ b/arch/loongarch/include/asm/fb.h
@@ -8,23 +8,23 @@
#include <linux/compiler.h>
#include <linux/string.h>

-static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+static inline void fb_memcpy_fromio(void *to, const volatile void __iomem *from, size_t n)
{
memcpy(to, (void __force *)from, n);
}
-#define fb_memcpy_fromfb fb_memcpy_fromfb
+#define fb_memcpy_fromio fb_memcpy_fromio

-static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+static inline void fb_memcpy_toio(volatile void __iomem *to, const void *from, size_t n)
{
memcpy((void __force *)to, from, n);
}
-#define fb_memcpy_tofb fb_memcpy_tofb
+#define fb_memcpy_toio fb_memcpy_toio

-static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+static inline void fb_memset_io(volatile void __iomem *addr, int c, size_t n)
{
memset((void __force *)addr, c, n);
}
-#define fb_memset fb_memset
+#define fb_memset fb_memset_io

#include <asm-generic/fb.h>

diff --git a/arch/sparc/include/asm/fb.h b/arch/sparc/include/asm/fb.h
index 077da91aeba1..572ecd3e1cc4 100644
--- a/arch/sparc/include/asm/fb.h
+++ b/arch/sparc/include/asm/fb.h
@@ -18,23 +18,23 @@ static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
int fb_is_primary_device(struct fb_info *info);
#define fb_is_primary_device fb_is_primary_device

-static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+static inline void fb_memcpy_fromio(void *to, const volatile void __iomem *from, size_t n)
{
sbus_memcpy_fromio(to, from, n);
}
-#define fb_memcpy_fromfb fb_memcpy_fromfb
+#define fb_memcpy_fromio fb_memcpy_fromio

-static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+static inline void fb_memcpy_toio(volatile void __iomem *to, const void *from, size_t n)
{
sbus_memcpy_toio(to, from, n);
}
-#define fb_memcpy_tofb fb_memcpy_tofb
+#define fb_memcpy_toio fb_memcpy_toio

-static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+static inline void fb_memset_io(volatile void __iomem *addr, int c, size_t n)
{
sbus_memset_io(addr, c, n);
}
-#define fb_memset fb_memset
+#define fb_memset fb_memset_io

#include <asm-generic/fb.h>

diff --git a/drivers/video/fbdev/aty/mach64_cursor.c b/drivers/video/fbdev/aty/mach64_cursor.c
index 4ad0331a8c57..971355c2cd7e 100644
--- a/drivers/video/fbdev/aty/mach64_cursor.c
+++ b/drivers/video/fbdev/aty/mach64_cursor.c
@@ -153,7 +153,7 @@ static int atyfb_cursor(struct fb_info *info, struct fb_cursor *cursor)
u8 m, b;

// Clear cursor image with 1010101010...
- fb_memset(dst, 0xaa, 1024);
+ fb_memset_io(dst, 0xaa, 1024);

offset = align - width*2;

diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
index 7799d52a651f..2a27ba94f652 100644
--- a/drivers/video/fbdev/chipsfb.c
+++ b/drivers/video/fbdev/chipsfb.c
@@ -332,7 +332,7 @@ static const struct fb_var_screeninfo chipsfb_var = {

static void init_chips(struct fb_info *p, unsigned long addr)
{
- fb_memset(p->screen_base, 0, 0x100000);
+ fb_memset_io(p->screen_base, 0, 0x100000);

p->fix = chipsfb_fix;
p->fix.smem_start = addr;
diff --git a/drivers/video/fbdev/core/fb_io_fops.c b/drivers/video/fbdev/core/fb_io_fops.c
index f5299d50f33b..5985e5e1b040 100644
--- a/drivers/video/fbdev/core/fb_io_fops.c
+++ b/drivers/video/fbdev/core/fb_io_fops.c
@@ -42,7 +42,7 @@ ssize_t fb_io_read(struct fb_info *info, char __user *buf, size_t count, loff_t
while (count) {
c = (count > PAGE_SIZE) ? PAGE_SIZE : count;
dst = buffer;
- fb_memcpy_fromfb(dst, src, c);
+ fb_memcpy_fromio(dst, src, c);
dst += c;
src += c;

@@ -117,7 +117,7 @@ ssize_t fb_io_write(struct fb_info *info, const char __user *buf, size_t count,
}
c -= trailing;

- fb_memcpy_tofb(dst, src, c);
+ fb_memcpy_toio(dst, src, c);
dst += c;
src += c;
*ppos += c;
diff --git a/drivers/video/fbdev/kyro/fbdev.c b/drivers/video/fbdev/kyro/fbdev.c
index 0596573ef140..3f277bdb3a32 100644
--- a/drivers/video/fbdev/kyro/fbdev.c
+++ b/drivers/video/fbdev/kyro/fbdev.c
@@ -737,7 +737,7 @@ static int kyrofb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
info->var.bits_per_pixel);
size *= info->var.yres_virtual;

- fb_memset(info->screen_base, 0, size);
+ fb_memset_io(info->screen_base, 0, size);

if (register_framebuffer(info) < 0)
goto out_unmap;
diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
index 550fdb5b4d41..c692cd597ce3 100644
--- a/drivers/video/fbdev/pvr2fb.c
+++ b/drivers/video/fbdev/pvr2fb.c
@@ -801,7 +801,7 @@ static int __maybe_unused pvr2fb_common_init(void)
goto out_err;
}

- fb_memset(fb_info->screen_base, 0, pvr2_fix.smem_len);
+ fb_memset_io(fb_info->screen_base, 0, pvr2_fix.smem_len);

pvr2_fix.ypanstep = nopan ? 0 : 1;
pvr2_fix.ywrapstep = nowrap ? 0 : 1;
diff --git a/drivers/video/fbdev/sstfb.c b/drivers/video/fbdev/sstfb.c
index da296b2ab54a..582324f5d869 100644
--- a/drivers/video/fbdev/sstfb.c
+++ b/drivers/video/fbdev/sstfb.c
@@ -335,7 +335,7 @@ static int sst_calc_pll(const int freq, int *freq_out, struct pll_timing *t)
static void sstfb_clear_screen(struct fb_info *info)
{
/* clear screen */
- fb_memset(info->screen_base, 0, info->fix.smem_len);
+ fb_memset_io(info->screen_base, 0, info->fix.smem_len);
}


diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
index baca6974e288..01363dccfdaf 100644
--- a/drivers/video/fbdev/stifb.c
+++ b/drivers/video/fbdev/stifb.c
@@ -527,8 +527,8 @@ rattlerSetupPlanes(struct stifb_info *fb)
fb->id = saved_id;

for (y = 0; y < fb->info.var.yres; ++y)
- fb_memset(fb->info.screen_base + y * fb->info.fix.line_length,
- 0xff, fb->info.var.xres * fb->info.var.bits_per_pixel/8);
+ fb_memset_io(fb->info.screen_base + y * fb->info.fix.line_length,
+ 0xff, fb->info.var.xres * fb->info.var.bits_per_pixel/8);

CRX24_SET_OVLY_MASK(fb);
SETUP_FB(fb);
diff --git a/drivers/video/fbdev/tdfxfb.c b/drivers/video/fbdev/tdfxfb.c
index d17e5e1472aa..cdf8e9fe9948 100644
--- a/drivers/video/fbdev/tdfxfb.c
+++ b/drivers/video/fbdev/tdfxfb.c
@@ -1116,7 +1116,7 @@ static int tdfxfb_cursor(struct fb_info *info, struct fb_cursor *cursor)
u8 *mask = (u8 *)cursor->mask;
int i;

- fb_memset(cursorbase, 0, 1024);
+ fb_memset_io(cursorbase, 0, 1024);

for (i = 0; i < cursor->image.height; i++) {
int h = 0;
diff --git a/include/asm-generic/fb.h b/include/asm-generic/fb.h
index 0540eccdbeca..bb7ee9c70e60 100644
--- a/include/asm-generic/fb.h
+++ b/include/asm-generic/fb.h
@@ -108,28 +108,28 @@ static inline void fb_writeq(u64 b, volatile void __iomem *addr)
#endif
#endif

-#ifndef fb_memcpy_fromfb
-static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+#ifndef fb_memcpy_fromio
+static inline void fb_memcpy_fromio(void *to, const volatile void __iomem *from, size_t n)
{
memcpy_fromio(to, from, n);
}
-#define fb_memcpy_fromfb fb_memcpy_fromfb
+#define fb_memcpy_fromio fb_memcpy_fromio
#endif

-#ifndef fb_memcpy_tofb
-static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+#ifndef fb_memcpy_toio
+static inline void fb_memcpy_toio(volatile void __iomem *to, const void *from, size_t n)
{
memcpy_toio(to, from, n);
}
-#define fb_memcpy_tofb fb_memcpy_tofb
+#define fb_memcpy_toio fb_memcpy_toio
#endif

#ifndef fb_memset
-static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+static inline void fb_memset_io(volatile void __iomem *addr, int c, size_t n)
{
memset_io(addr, c, n);
}
-#define fb_memset fb_memset
+#define fb_memset fb_memset_io
#endif

#endif /* __ASM_GENERIC_FB_H_ */
--
2.40.1


2023-05-12 10:31:17

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH v7 1/7] fbdev/hitfb: Cast I/O offset to address

Cast I/O offsets to pointers to use them with I/O functions. The I/O
functions expect pointers of type 'volatile void __iomem *', but the
offsets are plain integers. Build warnings are

../drivers/video/fbdev/hitfb.c: In function 'hitfb_accel_wait':
../arch/x86/include/asm/hd64461.h:18:33: warning: passing argument 1 of 'fb_readw' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
../arch/x86/include/asm/hd64461.h:93:33: note: in expansion of macro 'HD64461_IO_OFFSET'
93 | #define HD64461_GRCFGR HD64461_IO_OFFSET(0x1044) /* Accelerator Configuration Register */
| ^~~~~~~~~~~~~~~~~
../drivers/video/fbdev/hitfb.c:47:25: note: in expansion of macro 'HD64461_GRCFGR'
47 | while (fb_readw(HD64461_GRCFGR) & HD64461_GRCFGR_ACCSTATUS) ;
| ^~~~~~~~~~~~~~
In file included from ../arch/x86/include/asm/fb.h:15,
from ../include/linux/fb.h:19,
from ../drivers/video/fbdev/hitfb.c:22:
../include/asm-generic/fb.h:52:57: note: expected 'const volatile void *' but argument is of type 'unsigned int'
52 | static inline u16 fb_readw(const volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~

This patch only fixes the build warnings. It's not clear if the I/O
offsets can legally be passed to the I/O helpers. It was apparently
broken in 2007 when custom inw()/outw() helpers got removed by
commit 34a780a0afeb ("sh: hp6xx pata_platform support."). Fixing the
driver would require setting the I/O base address.

Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Artur Rojek <[email protected]>
---
drivers/video/fbdev/hitfb.c | 122 ++++++++++++++++++++----------------
1 file changed, 69 insertions(+), 53 deletions(-)

diff --git a/drivers/video/fbdev/hitfb.c b/drivers/video/fbdev/hitfb.c
index 3033f5056976..7737923b7a0a 100644
--- a/drivers/video/fbdev/hitfb.c
+++ b/drivers/video/fbdev/hitfb.c
@@ -42,17 +42,33 @@ static struct fb_fix_screeninfo hitfb_fix = {
.accel = FB_ACCEL_NONE,
};

+static volatile void __iomem *hitfb_offset_to_addr(unsigned int offset)
+{
+ return (__force volatile void __iomem *)(uintptr_t)offset;
+}
+
+static u16 hitfb_readw(unsigned int offset)
+{
+ return fb_readw(hitfb_offset_to_addr(offset));
+}
+
+static void hitfb_writew(u16 value, unsigned int offset)
+{
+ fb_writew(value, hitfb_offset_to_addr(offset));
+}
+
static inline void hitfb_accel_wait(void)
{
- while (fb_readw(HD64461_GRCFGR) & HD64461_GRCFGR_ACCSTATUS) ;
+ while (hitfb_readw(HD64461_GRCFGR) & HD64461_GRCFGR_ACCSTATUS)
+ ;
}

static inline void hitfb_accel_start(int truecolor)
{
if (truecolor) {
- fb_writew(6, HD64461_GRCFGR);
+ hitfb_writew(6, HD64461_GRCFGR);
} else {
- fb_writew(7, HD64461_GRCFGR);
+ hitfb_writew(7, HD64461_GRCFGR);
}
}

@@ -63,11 +79,11 @@ static inline void hitfb_accel_set_dest(int truecolor, u16 dx, u16 dy,
if (truecolor)
saddr <<= 1;

- fb_writew(width-1, HD64461_BBTDWR);
- fb_writew(height-1, HD64461_BBTDHR);
+ hitfb_writew(width-1, HD64461_BBTDWR);
+ hitfb_writew(height-1, HD64461_BBTDHR);

- fb_writew(saddr & 0xffff, HD64461_BBTDSARL);
- fb_writew(saddr >> 16, HD64461_BBTDSARH);
+ hitfb_writew(saddr & 0xffff, HD64461_BBTDSARL);
+ hitfb_writew(saddr >> 16, HD64461_BBTDSARH);

}

@@ -80,7 +96,7 @@ static inline void hitfb_accel_bitblt(int truecolor, u16 sx, u16 sy, u16 dx,

height--;
width--;
- fb_writew(rop, HD64461_BBTROPR);
+ hitfb_writew(rop, HD64461_BBTROPR);
if ((sy < dy) || ((sy == dy) && (sx <= dx))) {
saddr = WIDTH * (sy + height) + sx + width;
daddr = WIDTH * (dy + height) + dx + width;
@@ -91,32 +107,32 @@ static inline void hitfb_accel_bitblt(int truecolor, u16 sx, u16 sy, u16 dx,
maddr =
(((width >> 4) + 1) * (height + 1) - 1) * 2;

- fb_writew((1 << 5) | 1, HD64461_BBTMDR);
+ hitfb_writew((1 << 5) | 1, HD64461_BBTMDR);
} else
- fb_writew(1, HD64461_BBTMDR);
+ hitfb_writew(1, HD64461_BBTMDR);
} else {
saddr = WIDTH * sy + sx;
daddr = WIDTH * dy + dx;
if (mask_addr) {
- fb_writew((1 << 5), HD64461_BBTMDR);
+ hitfb_writew((1 << 5), HD64461_BBTMDR);
} else {
- fb_writew(0, HD64461_BBTMDR);
+ hitfb_writew(0, HD64461_BBTMDR);
}
}
if (truecolor) {
saddr <<= 1;
daddr <<= 1;
}
- fb_writew(width, HD64461_BBTDWR);
- fb_writew(height, HD64461_BBTDHR);
- fb_writew(saddr & 0xffff, HD64461_BBTSSARL);
- fb_writew(saddr >> 16, HD64461_BBTSSARH);
- fb_writew(daddr & 0xffff, HD64461_BBTDSARL);
- fb_writew(daddr >> 16, HD64461_BBTDSARH);
+ hitfb_writew(width, HD64461_BBTDWR);
+ hitfb_writew(height, HD64461_BBTDHR);
+ hitfb_writew(saddr & 0xffff, HD64461_BBTSSARL);
+ hitfb_writew(saddr >> 16, HD64461_BBTSSARH);
+ hitfb_writew(daddr & 0xffff, HD64461_BBTDSARL);
+ hitfb_writew(daddr >> 16, HD64461_BBTDSARH);
if (mask_addr) {
maddr += mask_addr;
- fb_writew(maddr & 0xffff, HD64461_BBTMARL);
- fb_writew(maddr >> 16, HD64461_BBTMARH);
+ hitfb_writew(maddr & 0xffff, HD64461_BBTMARL);
+ hitfb_writew(maddr >> 16, HD64461_BBTMARH);
}
hitfb_accel_start(truecolor);
}
@@ -127,17 +143,17 @@ static void hitfb_fillrect(struct fb_info *p, const struct fb_fillrect *rect)
cfb_fillrect(p, rect);
else {
hitfb_accel_wait();
- fb_writew(0x00f0, HD64461_BBTROPR);
- fb_writew(16, HD64461_BBTMDR);
+ hitfb_writew(0x00f0, HD64461_BBTROPR);
+ hitfb_writew(16, HD64461_BBTMDR);

if (p->var.bits_per_pixel == 16) {
- fb_writew(((u32 *) (p->pseudo_palette))[rect->color],
+ hitfb_writew(((u32 *) (p->pseudo_palette))[rect->color],
HD64461_GRSCR);
hitfb_accel_set_dest(1, rect->dx, rect->dy, rect->width,
rect->height);
hitfb_accel_start(1);
} else {
- fb_writew(rect->color, HD64461_GRSCR);
+ hitfb_writew(rect->color, HD64461_GRSCR);
hitfb_accel_set_dest(0, rect->dx, rect->dy, rect->width,
rect->height);
hitfb_accel_start(0);
@@ -162,7 +178,7 @@ static int hitfb_pan_display(struct fb_var_screeninfo *var,
if (xoffset != 0)
return -EINVAL;

- fb_writew((yoffset*info->fix.line_length)>>10, HD64461_LCDCBAR);
+ hitfb_writew((yoffset*info->fix.line_length)>>10, HD64461_LCDCBAR);

return 0;
}
@@ -172,33 +188,33 @@ int hitfb_blank(int blank_mode, struct fb_info *info)
unsigned short v;

if (blank_mode) {
- v = fb_readw(HD64461_LDR1);
+ v = hitfb_readw(HD64461_LDR1);
v &= ~HD64461_LDR1_DON;
- fb_writew(v, HD64461_LDR1);
+ hitfb_writew(v, HD64461_LDR1);

- v = fb_readw(HD64461_LCDCCR);
+ v = hitfb_readw(HD64461_LCDCCR);
v |= HD64461_LCDCCR_MOFF;
- fb_writew(v, HD64461_LCDCCR);
+ hitfb_writew(v, HD64461_LCDCCR);

- v = fb_readw(HD64461_STBCR);
+ v = hitfb_readw(HD64461_STBCR);
v |= HD64461_STBCR_SLCDST;
- fb_writew(v, HD64461_STBCR);
+ hitfb_writew(v, HD64461_STBCR);
} else {
- v = fb_readw(HD64461_STBCR);
+ v = hitfb_readw(HD64461_STBCR);
v &= ~HD64461_STBCR_SLCDST;
- fb_writew(v, HD64461_STBCR);
+ hitfb_writew(v, HD64461_STBCR);

- v = fb_readw(HD64461_LCDCCR);
+ v = hitfb_readw(HD64461_LCDCCR);
v &= ~(HD64461_LCDCCR_MOFF | HD64461_LCDCCR_STREQ);
- fb_writew(v, HD64461_LCDCCR);
+ hitfb_writew(v, HD64461_LCDCCR);

do {
- v = fb_readw(HD64461_LCDCCR);
+ v = hitfb_readw(HD64461_LCDCCR);
} while(v&HD64461_LCDCCR_STBACK);

- v = fb_readw(HD64461_LDR1);
+ v = hitfb_readw(HD64461_LDR1);
v |= HD64461_LDR1_DON;
- fb_writew(v, HD64461_LDR1);
+ hitfb_writew(v, HD64461_LDR1);
}
return 0;
}
@@ -211,10 +227,10 @@ static int hitfb_setcolreg(unsigned regno, unsigned red, unsigned green,

switch (info->var.bits_per_pixel) {
case 8:
- fb_writew(regno << 8, HD64461_CPTWAR);
- fb_writew(red >> 10, HD64461_CPTWDR);
- fb_writew(green >> 10, HD64461_CPTWDR);
- fb_writew(blue >> 10, HD64461_CPTWDR);
+ hitfb_writew(regno << 8, HD64461_CPTWAR);
+ hitfb_writew(red >> 10, HD64461_CPTWDR);
+ hitfb_writew(green >> 10, HD64461_CPTWDR);
+ hitfb_writew(blue >> 10, HD64461_CPTWDR);
break;
case 16:
if (regno >= 16)
@@ -302,11 +318,11 @@ static int hitfb_set_par(struct fb_info *info)
break;
}

- fb_writew(info->fix.line_length, HD64461_LCDCLOR);
- ldr3 = fb_readw(HD64461_LDR3);
+ hitfb_writew(info->fix.line_length, HD64461_LCDCLOR);
+ ldr3 = hitfb_readw(HD64461_LDR3);
ldr3 &= ~15;
ldr3 |= (info->var.bits_per_pixel == 8) ? 4 : 8;
- fb_writew(ldr3, HD64461_LDR3);
+ hitfb_writew(ldr3, HD64461_LDR3);
return 0;
}

@@ -337,9 +353,9 @@ static int hitfb_probe(struct platform_device *dev)
hitfb_fix.smem_start = HD64461_IO_OFFSET(0x02000000);
hitfb_fix.smem_len = 512 * 1024;

- lcdclor = fb_readw(HD64461_LCDCLOR);
- ldvndr = fb_readw(HD64461_LDVNDR);
- ldr3 = fb_readw(HD64461_LDR3);
+ lcdclor = hitfb_readw(HD64461_LCDCLOR);
+ ldvndr = hitfb_readw(HD64461_LDVNDR);
+ ldr3 = hitfb_readw(HD64461_LDR3);

switch (ldr3 & 15) {
default:
@@ -429,9 +445,9 @@ static int hitfb_suspend(struct device *dev)
u16 v;

hitfb_blank(1,0);
- v = fb_readw(HD64461_STBCR);
+ v = hitfb_readw(HD64461_STBCR);
v |= HD64461_STBCR_SLCKE_IST;
- fb_writew(v, HD64461_STBCR);
+ hitfb_writew(v, HD64461_STBCR);

return 0;
}
@@ -440,12 +456,12 @@ static int hitfb_resume(struct device *dev)
{
u16 v;

- v = fb_readw(HD64461_STBCR);
+ v = hitfb_readw(HD64461_STBCR);
v &= ~HD64461_STBCR_SLCKE_OST;
msleep(100);
- v = fb_readw(HD64461_STBCR);
+ v = hitfb_readw(HD64461_STBCR);
v &= ~HD64461_STBCR_SLCKE_IST;
- fb_writew(v, HD64461_STBCR);
+ hitfb_writew(v, HD64461_STBCR);
hitfb_blank(0,0);

return 0;
--
2.40.1


2023-05-12 11:48:15

by David Laight

[permalink] [raw]
Subject: RE: [PATCH v7 1/7] fbdev/hitfb: Cast I/O offset to address

From: Thomas Zimmermann
> Sent: 12 May 2023 11:25
>
> Cast I/O offsets to pointers to use them with I/O functions. The I/O
> functions expect pointers of type 'volatile void __iomem *', but the
> offsets are plain integers. Build warnings are
>
> ../drivers/video/fbdev/hitfb.c: In function 'hitfb_accel_wait':
> ../arch/x86/include/asm/hd64461.h:18:33: warning: passing argument 1 of 'fb_readw' makes pointer
> from integer without a cast [-Wint-conversion]
> 18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
> | ^~~~~~~~~~~~~~~~~~~~~~
...
> 52 | static inline u16 fb_readw(const volatile void __iomem *addr)
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
>
> This patch only fixes the build warnings. It's not clear if the I/O
> offsets can legally be passed to the I/O helpers. It was apparently
> broken in 2007 when custom inw()/outw() helpers got removed by
> commit 34a780a0afeb ("sh: hp6xx pata_platform support."). Fixing the
> driver would require setting the I/O base address.

Did you try changing the definition of HD64461_IOBASE to include
a (volatile void __iomem *) cast?
A lot less churn...

I'm guessing that 'sh' deosn't have in/out instructions so this
is something that is always mapped at a fixed kernel virtual address?

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


2023-05-12 11:54:48

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH v7 1/7] fbdev/hitfb: Cast I/O offset to address

Hi

Am 12.05.23 um 13:16 schrieb David Laight:
> From: Thomas Zimmermann
>> Sent: 12 May 2023 11:25
>>
>> Cast I/O offsets to pointers to use them with I/O functions. The I/O
>> functions expect pointers of type 'volatile void __iomem *', but the
>> offsets are plain integers. Build warnings are
>>
>> ../drivers/video/fbdev/hitfb.c: In function 'hitfb_accel_wait':
>> ../arch/x86/include/asm/hd64461.h:18:33: warning: passing argument 1 of 'fb_readw' makes pointer
>> from integer without a cast [-Wint-conversion]
>> 18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
>> | ^~~~~~~~~~~~~~~~~~~~~~
> ...
>> 52 | static inline u16 fb_readw(const volatile void __iomem *addr)
>> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
>>
>> This patch only fixes the build warnings. It's not clear if the I/O
>> offsets can legally be passed to the I/O helpers. It was apparently
>> broken in 2007 when custom inw()/outw() helpers got removed by
>> commit 34a780a0afeb ("sh: hp6xx pata_platform support."). Fixing the
>> driver would require setting the I/O base address.
>
> Did you try changing the definition of HD64461_IOBASE to include
> a (volatile void __iomem *) cast?

I thought about it, but didn't try it. I didn't want bend the meaning of
OFFSET and IOBASE too much. They sound like integer constants to me.

> A lot less churn...
>
> I'm guessing that 'sh' deosn't have in/out instructions so this
> is something that is always mapped at a fixed kernel virtual address?

No idea. I cannot try the driver and was only able to build it by
hacking up something that makes COMPILE_TEST work.

The current patch seemed like the safest bet, even with the churn.

Best regards
Thomas

>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

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


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2023-05-16 17:47:26

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH v7 1/7] fbdev/hitfb: Cast I/O offset to address

On 5/12/23 12:24, Thomas Zimmermann wrote:
> Cast I/O offsets to pointers to use them with I/O functions. The I/O
> functions expect pointers of type 'volatile void __iomem *', but the
> offsets are plain integers. Build warnings are
>
> ../drivers/video/fbdev/hitfb.c: In function 'hitfb_accel_wait':
> ../arch/x86/include/asm/hd64461.h:18:33: warning: passing argument 1 of 'fb_readw' makes pointer from integer without a cast [-Wint-conversion]
> 18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
> | ^~~~~~~~~~~~~~~~~~~~~~
> | |
> | unsigned int
> ../arch/x86/include/asm/hd64461.h:93:33: note: in expansion of macro 'HD64461_IO_OFFSET'
> 93 | #define HD64461_GRCFGR HD64461_IO_OFFSET(0x1044) /* Accelerator Configuration Register */
> | ^~~~~~~~~~~~~~~~~
> ../drivers/video/fbdev/hitfb.c:47:25: note: in expansion of macro 'HD64461_GRCFGR'
> 47 | while (fb_readw(HD64461_GRCFGR) & HD64461_GRCFGR_ACCSTATUS) ;
> | ^~~~~~~~~~~~~~
> In file included from ../arch/x86/include/asm/fb.h:15,
> from ../include/linux/fb.h:19,
> from ../drivers/video/fbdev/hitfb.c:22:
> ../include/asm-generic/fb.h:52:57: note: expected 'const volatile void *' but argument is of type 'unsigned int'
> 52 | static inline u16 fb_readw(const volatile void __iomem *addr)
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
>
> This patch only fixes the build warnings. It's not clear if the I/O
> offsets can legally be passed to the I/O helpers. It was apparently
> broken in 2007 when custom inw()/outw() helpers got removed by
> commit 34a780a0afeb ("sh: hp6xx pata_platform support."). Fixing the
> driver would require setting the I/O base address.

I think your patch is the best you can do for now... So...

Acked-by: Helge Deller <[email protected]>

Thanks!
Helge


> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Artur Rojek <[email protected]>
> ---
> drivers/video/fbdev/hitfb.c | 122 ++++++++++++++++++++----------------
> 1 file changed, 69 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/video/fbdev/hitfb.c b/drivers/video/fbdev/hitfb.c
> index 3033f5056976..7737923b7a0a 100644
> --- a/drivers/video/fbdev/hitfb.c
> +++ b/drivers/video/fbdev/hitfb.c
> @@ -42,17 +42,33 @@ static struct fb_fix_screeninfo hitfb_fix = {
> .accel = FB_ACCEL_NONE,
> };
>
> +static volatile void __iomem *hitfb_offset_to_addr(unsigned int offset)
> +{
> + return (__force volatile void __iomem *)(uintptr_t)offset;
> +}
> +
> +static u16 hitfb_readw(unsigned int offset)
> +{
> + return fb_readw(hitfb_offset_to_addr(offset));
> +}
> +
> +static void hitfb_writew(u16 value, unsigned int offset)
> +{
> + fb_writew(value, hitfb_offset_to_addr(offset));
> +}
> +
> static inline void hitfb_accel_wait(void)
> {
> - while (fb_readw(HD64461_GRCFGR) & HD64461_GRCFGR_ACCSTATUS) ;
> + while (hitfb_readw(HD64461_GRCFGR) & HD64461_GRCFGR_ACCSTATUS)
> + ;
> }
>
> static inline void hitfb_accel_start(int truecolor)
> {
> if (truecolor) {
> - fb_writew(6, HD64461_GRCFGR);
> + hitfb_writew(6, HD64461_GRCFGR);
> } else {
> - fb_writew(7, HD64461_GRCFGR);
> + hitfb_writew(7, HD64461_GRCFGR);
> }
> }
>
> @@ -63,11 +79,11 @@ static inline void hitfb_accel_set_dest(int truecolor, u16 dx, u16 dy,
> if (truecolor)
> saddr <<= 1;
>
> - fb_writew(width-1, HD64461_BBTDWR);
> - fb_writew(height-1, HD64461_BBTDHR);
> + hitfb_writew(width-1, HD64461_BBTDWR);
> + hitfb_writew(height-1, HD64461_BBTDHR);
>
> - fb_writew(saddr & 0xffff, HD64461_BBTDSARL);
> - fb_writew(saddr >> 16, HD64461_BBTDSARH);
> + hitfb_writew(saddr & 0xffff, HD64461_BBTDSARL);
> + hitfb_writew(saddr >> 16, HD64461_BBTDSARH);
>
> }
>
> @@ -80,7 +96,7 @@ static inline void hitfb_accel_bitblt(int truecolor, u16 sx, u16 sy, u16 dx,
>
> height--;
> width--;
> - fb_writew(rop, HD64461_BBTROPR);
> + hitfb_writew(rop, HD64461_BBTROPR);
> if ((sy < dy) || ((sy == dy) && (sx <= dx))) {
> saddr = WIDTH * (sy + height) + sx + width;
> daddr = WIDTH * (dy + height) + dx + width;
> @@ -91,32 +107,32 @@ static inline void hitfb_accel_bitblt(int truecolor, u16 sx, u16 sy, u16 dx,
> maddr =
> (((width >> 4) + 1) * (height + 1) - 1) * 2;
>
> - fb_writew((1 << 5) | 1, HD64461_BBTMDR);
> + hitfb_writew((1 << 5) | 1, HD64461_BBTMDR);
> } else
> - fb_writew(1, HD64461_BBTMDR);
> + hitfb_writew(1, HD64461_BBTMDR);
> } else {
> saddr = WIDTH * sy + sx;
> daddr = WIDTH * dy + dx;
> if (mask_addr) {
> - fb_writew((1 << 5), HD64461_BBTMDR);
> + hitfb_writew((1 << 5), HD64461_BBTMDR);
> } else {
> - fb_writew(0, HD64461_BBTMDR);
> + hitfb_writew(0, HD64461_BBTMDR);
> }
> }
> if (truecolor) {
> saddr <<= 1;
> daddr <<= 1;
> }
> - fb_writew(width, HD64461_BBTDWR);
> - fb_writew(height, HD64461_BBTDHR);
> - fb_writew(saddr & 0xffff, HD64461_BBTSSARL);
> - fb_writew(saddr >> 16, HD64461_BBTSSARH);
> - fb_writew(daddr & 0xffff, HD64461_BBTDSARL);
> - fb_writew(daddr >> 16, HD64461_BBTDSARH);
> + hitfb_writew(width, HD64461_BBTDWR);
> + hitfb_writew(height, HD64461_BBTDHR);
> + hitfb_writew(saddr & 0xffff, HD64461_BBTSSARL);
> + hitfb_writew(saddr >> 16, HD64461_BBTSSARH);
> + hitfb_writew(daddr & 0xffff, HD64461_BBTDSARL);
> + hitfb_writew(daddr >> 16, HD64461_BBTDSARH);
> if (mask_addr) {
> maddr += mask_addr;
> - fb_writew(maddr & 0xffff, HD64461_BBTMARL);
> - fb_writew(maddr >> 16, HD64461_BBTMARH);
> + hitfb_writew(maddr & 0xffff, HD64461_BBTMARL);
> + hitfb_writew(maddr >> 16, HD64461_BBTMARH);
> }
> hitfb_accel_start(truecolor);
> }
> @@ -127,17 +143,17 @@ static void hitfb_fillrect(struct fb_info *p, const struct fb_fillrect *rect)
> cfb_fillrect(p, rect);
> else {
> hitfb_accel_wait();
> - fb_writew(0x00f0, HD64461_BBTROPR);
> - fb_writew(16, HD64461_BBTMDR);
> + hitfb_writew(0x00f0, HD64461_BBTROPR);
> + hitfb_writew(16, HD64461_BBTMDR);
>
> if (p->var.bits_per_pixel == 16) {
> - fb_writew(((u32 *) (p->pseudo_palette))[rect->color],
> + hitfb_writew(((u32 *) (p->pseudo_palette))[rect->color],
> HD64461_GRSCR);
> hitfb_accel_set_dest(1, rect->dx, rect->dy, rect->width,
> rect->height);
> hitfb_accel_start(1);
> } else {
> - fb_writew(rect->color, HD64461_GRSCR);
> + hitfb_writew(rect->color, HD64461_GRSCR);
> hitfb_accel_set_dest(0, rect->dx, rect->dy, rect->width,
> rect->height);
> hitfb_accel_start(0);
> @@ -162,7 +178,7 @@ static int hitfb_pan_display(struct fb_var_screeninfo *var,
> if (xoffset != 0)
> return -EINVAL;
>
> - fb_writew((yoffset*info->fix.line_length)>>10, HD64461_LCDCBAR);
> + hitfb_writew((yoffset*info->fix.line_length)>>10, HD64461_LCDCBAR);
>
> return 0;
> }
> @@ -172,33 +188,33 @@ int hitfb_blank(int blank_mode, struct fb_info *info)
> unsigned short v;
>
> if (blank_mode) {
> - v = fb_readw(HD64461_LDR1);
> + v = hitfb_readw(HD64461_LDR1);
> v &= ~HD64461_LDR1_DON;
> - fb_writew(v, HD64461_LDR1);
> + hitfb_writew(v, HD64461_LDR1);
>
> - v = fb_readw(HD64461_LCDCCR);
> + v = hitfb_readw(HD64461_LCDCCR);
> v |= HD64461_LCDCCR_MOFF;
> - fb_writew(v, HD64461_LCDCCR);
> + hitfb_writew(v, HD64461_LCDCCR);
>
> - v = fb_readw(HD64461_STBCR);
> + v = hitfb_readw(HD64461_STBCR);
> v |= HD64461_STBCR_SLCDST;
> - fb_writew(v, HD64461_STBCR);
> + hitfb_writew(v, HD64461_STBCR);
> } else {
> - v = fb_readw(HD64461_STBCR);
> + v = hitfb_readw(HD64461_STBCR);
> v &= ~HD64461_STBCR_SLCDST;
> - fb_writew(v, HD64461_STBCR);
> + hitfb_writew(v, HD64461_STBCR);
>
> - v = fb_readw(HD64461_LCDCCR);
> + v = hitfb_readw(HD64461_LCDCCR);
> v &= ~(HD64461_LCDCCR_MOFF | HD64461_LCDCCR_STREQ);
> - fb_writew(v, HD64461_LCDCCR);
> + hitfb_writew(v, HD64461_LCDCCR);
>
> do {
> - v = fb_readw(HD64461_LCDCCR);
> + v = hitfb_readw(HD64461_LCDCCR);
> } while(v&HD64461_LCDCCR_STBACK);
>
> - v = fb_readw(HD64461_LDR1);
> + v = hitfb_readw(HD64461_LDR1);
> v |= HD64461_LDR1_DON;
> - fb_writew(v, HD64461_LDR1);
> + hitfb_writew(v, HD64461_LDR1);
> }
> return 0;
> }
> @@ -211,10 +227,10 @@ static int hitfb_setcolreg(unsigned regno, unsigned red, unsigned green,
>
> switch (info->var.bits_per_pixel) {
> case 8:
> - fb_writew(regno << 8, HD64461_CPTWAR);
> - fb_writew(red >> 10, HD64461_CPTWDR);
> - fb_writew(green >> 10, HD64461_CPTWDR);
> - fb_writew(blue >> 10, HD64461_CPTWDR);
> + hitfb_writew(regno << 8, HD64461_CPTWAR);
> + hitfb_writew(red >> 10, HD64461_CPTWDR);
> + hitfb_writew(green >> 10, HD64461_CPTWDR);
> + hitfb_writew(blue >> 10, HD64461_CPTWDR);
> break;
> case 16:
> if (regno >= 16)
> @@ -302,11 +318,11 @@ static int hitfb_set_par(struct fb_info *info)
> break;
> }
>
> - fb_writew(info->fix.line_length, HD64461_LCDCLOR);
> - ldr3 = fb_readw(HD64461_LDR3);
> + hitfb_writew(info->fix.line_length, HD64461_LCDCLOR);
> + ldr3 = hitfb_readw(HD64461_LDR3);
> ldr3 &= ~15;
> ldr3 |= (info->var.bits_per_pixel == 8) ? 4 : 8;
> - fb_writew(ldr3, HD64461_LDR3);
> + hitfb_writew(ldr3, HD64461_LDR3);
> return 0;
> }
>
> @@ -337,9 +353,9 @@ static int hitfb_probe(struct platform_device *dev)
> hitfb_fix.smem_start = HD64461_IO_OFFSET(0x02000000);
> hitfb_fix.smem_len = 512 * 1024;
>
> - lcdclor = fb_readw(HD64461_LCDCLOR);
> - ldvndr = fb_readw(HD64461_LDVNDR);
> - ldr3 = fb_readw(HD64461_LDR3);
> + lcdclor = hitfb_readw(HD64461_LCDCLOR);
> + ldvndr = hitfb_readw(HD64461_LDVNDR);
> + ldr3 = hitfb_readw(HD64461_LDR3);
>
> switch (ldr3 & 15) {
> default:
> @@ -429,9 +445,9 @@ static int hitfb_suspend(struct device *dev)
> u16 v;
>
> hitfb_blank(1,0);
> - v = fb_readw(HD64461_STBCR);
> + v = hitfb_readw(HD64461_STBCR);
> v |= HD64461_STBCR_SLCKE_IST;
> - fb_writew(v, HD64461_STBCR);
> + hitfb_writew(v, HD64461_STBCR);
>
> return 0;
> }
> @@ -440,12 +456,12 @@ static int hitfb_resume(struct device *dev)
> {
> u16 v;
>
> - v = fb_readw(HD64461_STBCR);
> + v = hitfb_readw(HD64461_STBCR);
> v &= ~HD64461_STBCR_SLCKE_OST;
> msleep(100);
> - v = fb_readw(HD64461_STBCR);
> + v = hitfb_readw(HD64461_STBCR);
> v &= ~HD64461_STBCR_SLCKE_IST;
> - fb_writew(v, HD64461_STBCR);
> + hitfb_writew(v, HD64461_STBCR);
> hitfb_blank(0,0);
>
> return 0;