* [PATCH v6 0/6] fbdev: Move framebuffer I/O helpers to <asm/fb.h>
@ 2023-05-10 11:05 Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces Thomas Zimmermann
` (5 more replies)
0 siblings, 6 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-10 11:05 UTC (permalink / raw)
To: deller, geert, javierm, daniel, vgupta, chenhuacai, kernel,
davem, James.Bottomley, arnd, sam, suijingfeng
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc, Thomas Zimmermann
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 patch a simple whitespace cleanup.
Until now, <linux/fb.h> contained an include of <asm/io.h>. As this
will go away, patches 2 to 4 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 5 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 6 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.
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 (6):
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/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 +----------
26 files changed, 210 insertions(+), 77 deletions(-)
--
2.40.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces
2023-05-10 11:05 [PATCH v6 0/6] fbdev: Move framebuffer I/O helpers to <asm/fb.h> Thomas Zimmermann
@ 2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 18:20 ` Sui Jingfeng
2023-05-10 11:05 ` [PATCH v6 2/6] ipu-v3: Include <linux/io.h> Thomas Zimmermann
` (4 subsequent siblings)
5 siblings, 1 reply; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-10 11:05 UTC (permalink / raw)
To: deller, geert, javierm, daniel, vgupta, chenhuacai, kernel,
davem, James.Bottomley, arnd, sam, suijingfeng
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc, Thomas Zimmermann
Fix coding style. No functional changes.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
Tested-by: Sui Jingfeng <suijingfeng@loongson.cn>
---
drivers/video/fbdev/matrox/matroxfb_accel.c | 6 +++---
drivers/video/fbdev/matrox/matroxfb_base.h | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/matrox/matroxfb_accel.c b/drivers/video/fbdev/matrox/matroxfb_accel.c
index 9cb0685feddd..ce51227798a1 100644
--- a/drivers/video/fbdev/matrox/matroxfb_accel.c
+++ b/drivers/video/fbdev/matrox/matroxfb_accel.c
@@ -88,7 +88,7 @@
static inline void matrox_cfb4_pal(u_int32_t* pal) {
unsigned int i;
-
+
for (i = 0; i < 16; i++) {
pal[i] = i * 0x11111111U;
}
@@ -96,7 +96,7 @@ static inline void matrox_cfb4_pal(u_int32_t* pal) {
static inline void matrox_cfb8_pal(u_int32_t* pal) {
unsigned int i;
-
+
for (i = 0; i < 16; i++) {
pal[i] = i * 0x01010101U;
}
@@ -482,7 +482,7 @@ static void matroxfb_1bpp_imageblit(struct matrox_fb_info *minfo, u_int32_t fgx,
/* Tell... well, why bother... */
while (height--) {
size_t i;
-
+
for (i = 0; i < step; i += 4) {
/* Hope that there are at least three readable bytes beyond the end of bitmap */
fb_writel(get_unaligned((u_int32_t*)(chardata + i)),mmio.vaddr);
diff --git a/drivers/video/fbdev/matrox/matroxfb_base.h b/drivers/video/fbdev/matrox/matroxfb_base.h
index 958be6805f87..c93c69bbcd57 100644
--- a/drivers/video/fbdev/matrox/matroxfb_base.h
+++ b/drivers/video/fbdev/matrox/matroxfb_base.h
@@ -301,9 +301,9 @@ struct matrox_altout {
int (*verifymode)(void* altout_dev, u_int32_t mode);
int (*getqueryctrl)(void* altout_dev,
struct v4l2_queryctrl* ctrl);
- int (*getctrl)(void* altout_dev,
+ int (*getctrl)(void *altout_dev,
struct v4l2_control* ctrl);
- int (*setctrl)(void* altout_dev,
+ int (*setctrl)(void *altout_dev,
struct v4l2_control* ctrl);
};
--
2.40.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v6 2/6] ipu-v3: Include <linux/io.h>
2023-05-10 11:05 [PATCH v6 0/6] fbdev: Move framebuffer I/O helpers to <asm/fb.h> Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces Thomas Zimmermann
@ 2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 3/6] fbdev: Include <linux/io.h> in various drivers Thomas Zimmermann
` (3 subsequent siblings)
5 siblings, 0 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-10 11:05 UTC (permalink / raw)
To: deller, geert, javierm, daniel, vgupta, chenhuacai, kernel,
davem, James.Bottomley, arnd, sam, suijingfeng
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc, Thomas Zimmermann
The code uses readl() and writel(). Include the header file to
get the declarations.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
---
drivers/gpu/ipu-v3/ipu-prv.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/ipu-v3/ipu-prv.h b/drivers/gpu/ipu-v3/ipu-prv.h
index 291ac1bab66d..d4621b1ea7f1 100644
--- a/drivers/gpu/ipu-v3/ipu-prv.h
+++ b/drivers/gpu/ipu-v3/ipu-prv.h
@@ -8,6 +8,7 @@
struct ipu_soc;
+#include <linux/io.h>
#include <linux/types.h>
#include <linux/device.h>
#include <linux/clk.h>
--
2.40.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v6 3/6] fbdev: Include <linux/io.h> in various drivers
2023-05-10 11:05 [PATCH v6 0/6] fbdev: Move framebuffer I/O helpers to <asm/fb.h> Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 2/6] ipu-v3: Include <linux/io.h> Thomas Zimmermann
@ 2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 4/6] fbdev: Include <linux/fb.h> instead of <asm/fb.h> Thomas Zimmermann
` (2 subsequent siblings)
5 siblings, 0 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-10 11:05 UTC (permalink / raw)
To: deller, geert, javierm, daniel, vgupta, chenhuacai, kernel,
davem, James.Bottomley, arnd, sam, suijingfeng
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc, Thomas Zimmermann
The code uses writel() and similar I/O-memory helpers. Include
the header file to get the declarations.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
---
drivers/video/fbdev/arcfb.c | 1 +
drivers/video/fbdev/aty/atyfb.h | 2 ++
drivers/video/fbdev/wmt_ge_rops.c | 2 ++
3 files changed, 5 insertions(+)
diff --git a/drivers/video/fbdev/arcfb.c b/drivers/video/fbdev/arcfb.c
index 7750e020839e..ace104850e50 100644
--- a/drivers/video/fbdev/arcfb.c
+++ b/drivers/video/fbdev/arcfb.c
@@ -41,6 +41,7 @@
#include <linux/vmalloc.h>
#include <linux/delay.h>
#include <linux/interrupt.h>
+#include <linux/io.h>
#include <linux/fb.h>
#include <linux/init.h>
#include <linux/arcfb.h>
diff --git a/drivers/video/fbdev/aty/atyfb.h b/drivers/video/fbdev/aty/atyfb.h
index 465f55beb97f..30da3e82ed3c 100644
--- a/drivers/video/fbdev/aty/atyfb.h
+++ b/drivers/video/fbdev/aty/atyfb.h
@@ -3,8 +3,10 @@
* ATI Frame Buffer Device Driver Core Definitions
*/
+#include <linux/io.h>
#include <linux/spinlock.h>
#include <linux/wait.h>
+
/*
* Elements of the hardware specific atyfb_par structure
*/
diff --git a/drivers/video/fbdev/wmt_ge_rops.c b/drivers/video/fbdev/wmt_ge_rops.c
index 42255d27a1db..99c7b0aea615 100644
--- a/drivers/video/fbdev/wmt_ge_rops.c
+++ b/drivers/video/fbdev/wmt_ge_rops.c
@@ -9,7 +9,9 @@
#include <linux/module.h>
#include <linux/fb.h>
+#include <linux/io.h>
#include <linux/platform_device.h>
+
#include "core/fb_draw.h"
#include "wmt_ge_rops.h"
--
2.40.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v6 4/6] fbdev: Include <linux/fb.h> instead of <asm/fb.h>
2023-05-10 11:05 [PATCH v6 0/6] fbdev: Move framebuffer I/O helpers to <asm/fb.h> Thomas Zimmermann
` (2 preceding siblings ...)
2023-05-10 11:05 ` [PATCH v6 3/6] fbdev: Include <linux/io.h> in various drivers Thomas Zimmermann
@ 2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h> Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 6/6] fbdev: Rename fb_mem*() helpers Thomas Zimmermann
5 siblings, 0 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-10 11:05 UTC (permalink / raw)
To: deller, geert, javierm, daniel, vgupta, chenhuacai, kernel,
davem, James.Bottomley, arnd, sam, suijingfeng
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc, Thomas Zimmermann
Replace include statements for <asm/fb.h> with <linux/fb.h>. Fixes
the coding style: if a header is available in asm/ and linux/, it
is preferable to include the header from linux/. This only affects
a few source files, most of which already include <linux/fb.h>.
Suggested-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
arch/parisc/video/fbdev.c | 3 +--
arch/sparc/video/fbdev.c | 1 -
arch/x86/video/fbdev.c | 2 --
drivers/staging/sm750fb/sm750.c | 2 +-
drivers/video/fbdev/core/fbcon.c | 1 -
drivers/video/fbdev/core/fbmem.c | 2 --
include/linux/fb.h | 2 ++
7 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/arch/parisc/video/fbdev.c b/arch/parisc/video/fbdev.c
index 4a0ae08fc75b..137561d98246 100644
--- a/arch/parisc/video/fbdev.c
+++ b/arch/parisc/video/fbdev.c
@@ -5,10 +5,9 @@
* Copyright (C) 2001-2002 Thomas Bogendoerfer <tsbogend@alpha.franken.de>
*/
+#include <linux/fb.h>
#include <linux/module.h>
-#include <asm/fb.h>
-
#include <video/sticore.h>
int fb_is_primary_device(struct fb_info *info)
diff --git a/arch/sparc/video/fbdev.c b/arch/sparc/video/fbdev.c
index dadd5799fbb3..25837f128132 100644
--- a/arch/sparc/video/fbdev.c
+++ b/arch/sparc/video/fbdev.c
@@ -4,7 +4,6 @@
#include <linux/fb.h>
#include <linux/module.h>
-#include <asm/fb.h>
#include <asm/prom.h>
int fb_is_primary_device(struct fb_info *info)
diff --git a/arch/x86/video/fbdev.c b/arch/x86/video/fbdev.c
index 57ee3c158f97..f41a17ebac48 100644
--- a/arch/x86/video/fbdev.c
+++ b/arch/x86/video/fbdev.c
@@ -7,8 +7,6 @@
*
*/
-#include <asm/fb.h>
-
#include <linux/fb.h>
#include <linux/module.h>
#include <linux/pci.h>
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 22ace3168723..55e302a27847 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -16,7 +16,7 @@
#include <linux/pagemap.h>
#include <linux/screen_info.h>
#include <linux/console.h>
-#include <asm/fb.h>
+
#include "sm750.h"
#include "sm750_accel.h"
#include "sm750_cursor.h"
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index eb565a10e5cd..c6c9d040bdec 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -75,7 +75,6 @@
#include <linux/interrupt.h>
#include <linux/crc32.h> /* For counting font checksums */
#include <linux/uaccess.h>
-#include <asm/fb.h>
#include <asm/irq.h>
#include "fbcon.h"
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 4035a57df116..41b0706be635 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -37,8 +37,6 @@
#include <linux/mem_encrypt.h>
#include <linux/pci.h>
-#include <asm/fb.h>
-
#include <video/nomodeset.h>
#include <video/vga.h>
diff --git a/include/linux/fb.h b/include/linux/fb.h
index ec978a4969a9..4b4d9a5d200a 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -15,6 +15,8 @@
#include <linux/list.h>
#include <linux/backlight.h>
#include <linux/slab.h>
+
+#include <asm/fb.h>
#include <asm/io.h>
struct vm_area_struct;
--
2.40.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 11:05 [PATCH v6 0/6] fbdev: Move framebuffer I/O helpers to <asm/fb.h> Thomas Zimmermann
` (3 preceding siblings ...)
2023-05-10 11:05 ` [PATCH v6 4/6] fbdev: Include <linux/fb.h> instead of <asm/fb.h> Thomas Zimmermann
@ 2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 12:34 ` Geert Uytterhoeven
2023-05-10 14:03 ` kernel test robot
2023-05-10 11:05 ` [PATCH v6 6/6] fbdev: Rename fb_mem*() helpers Thomas Zimmermann
5 siblings, 2 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-10 11:05 UTC (permalink / raw)
To: deller, geert, javierm, daniel, vgupta, chenhuacai, kernel,
davem, James.Bottomley, arnd, sam, suijingfeng
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc, Thomas Zimmermann
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()
---
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v6 6/6] fbdev: Rename fb_mem*() helpers
2023-05-10 11:05 [PATCH v6 0/6] fbdev: Move framebuffer I/O helpers to <asm/fb.h> Thomas Zimmermann
` (4 preceding siblings ...)
2023-05-10 11:05 ` [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h> Thomas Zimmermann
@ 2023-05-10 11:05 ` Thomas Zimmermann
5 siblings, 0 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-10 11:05 UTC (permalink / raw)
To: deller, geert, javierm, daniel, vgupta, chenhuacai, kernel,
davem, James.Bottomley, arnd, sam, suijingfeng
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc, Thomas Zimmermann
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 <tzimmermann@suse.de>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
---
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 11:05 ` [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h> Thomas Zimmermann
@ 2023-05-10 12:34 ` Geert Uytterhoeven
2023-05-10 14:20 ` Thomas Zimmermann
2023-05-10 14:03 ` kernel test robot
1 sibling, 1 reply; 31+ messages in thread
From: Geert Uytterhoeven @ 2023-05-10 12:34 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: deller, javierm, daniel, vgupta, chenhuacai, kernel, davem,
James.Bottomley, arnd, sam, suijingfeng, linux-fbdev, dri-devel,
linux-arch, linux-snps-arc, linux-kernel, linux-ia64, loongarch,
linux-m68k, sparclinux, linux-arm-kernel, linux-parisc
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()
> --- 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
__raw_readq() and __raw_writeq()...
> + *
> + * 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?
> +}
> +#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
--
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 11:05 ` [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h> Thomas Zimmermann
2023-05-10 12:34 ` Geert Uytterhoeven
@ 2023-05-10 14:03 ` kernel test robot
2023-05-10 14:15 ` Arnd Bergmann
1 sibling, 1 reply; 31+ messages in thread
From: kernel test robot @ 2023-05-10 14:03 UTC (permalink / raw)
To: Thomas Zimmermann, deller, geert, javierm, daniel, vgupta,
chenhuacai, kernel, davem, James.Bottomley, arnd, sam,
suijingfeng
Cc: oe-kbuild-all, linux-arch, linux-fbdev, linux-ia64,
Thomas Zimmermann, linux-parisc, linux-kernel, dri-devel,
linux-m68k, loongarch, sparclinux, linux-snps-arc,
linux-arm-kernel
Hi Thomas,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[cannot apply to deller-parisc/for-next arnd-asm-generic/master linus/master davem-sparc/master v6.4-rc1 next-20230510]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/fbdev-matrox-Remove-trailing-whitespaces/20230510-190752
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20230510110557.14343-6-tzimmermann%40suse.de
patch subject: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
config: sh-randconfig-r024-20230509 (https://download.01.org/0day-ci/archive/20230510/202305102136.eMjTSPwH-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/46cc7edd7f28cc167c5b38d0e4f0aa8c3ac67328
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Thomas-Zimmermann/fbdev-matrox-Remove-trailing-whitespaces/20230510-190752
git checkout 46cc7edd7f28cc167c5b38d0e4f0aa8c3ac67328
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash drivers/video/fbdev/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305102136.eMjTSPwH-lkp@intel.com/
All warnings (new ones prefixed by >>):
cc1: warning: arch/sh/include/mach-hp6xx: No such file or directory [-Wmissing-include-dirs]
cc1: warning: arch/sh/include/mach-hp6xx: No such file or directory [-Wmissing-include-dirs]
In file included from drivers/video/fbdev/hitfb.c:27:
drivers/video/fbdev/hitfb.c: In function 'hitfb_accel_wait':
>> arch/sh/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/sh/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/sh/include/asm/fb.h:5,
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)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/video/fbdev/hitfb.c: In function 'hitfb_accel_start':
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/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:53:30: note: in expansion of macro 'HD64461_GRCFGR'
53 | fb_writew(6, HD64461_GRCFGR);
| ^~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/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:55:30: note: in expansion of macro 'HD64461_GRCFGR'
55 | fb_writew(7, HD64461_GRCFGR);
| ^~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/video/fbdev/hitfb.c: In function 'hitfb_accel_set_dest':
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:116:33: note: in expansion of macro 'HD64461_IO_OFFSET'
116 | #define HD64461_BBTDWR HD64461_IO_OFFSET(0x105c) /* Destination Block Width Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:66:28: note: in expansion of macro 'HD64461_BBTDWR'
66 | fb_writew(width-1, HD64461_BBTDWR);
| ^~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:117:33: note: in expansion of macro 'HD64461_IO_OFFSET'
117 | #define HD64461_BBTDHR HD64461_IO_OFFSET(0x105e) /* Destination Block Height Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:67:29: note: in expansion of macro 'HD64461_BBTDHR'
67 | fb_writew(height-1, HD64461_BBTDHR);
| ^~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:115:33: note: in expansion of macro 'HD64461_IO_OFFSET'
115 | #define HD64461_BBTDSARL HD64461_IO_OFFSET(0x105a) /* Destination Start Address Register (L) */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:69:35: note: in expansion of macro 'HD64461_BBTDSARL'
69 | fb_writew(saddr & 0xffff, HD64461_BBTDSARL);
| ^~~~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:114:33: note: in expansion of macro 'HD64461_IO_OFFSET'
114 | #define HD64461_BBTDSARH HD64461_IO_OFFSET(0x1058) /* Destination Start Address Register (H) */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:70:32: note: in expansion of macro 'HD64461_BBTDSARH'
70 | fb_writew(saddr >> 16, HD64461_BBTDSARH);
| ^~~~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/video/fbdev/hitfb.c: In function 'hitfb_accel_bitblt':
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:122:33: note: in expansion of macro 'HD64461_IO_OFFSET'
122 | #define HD64461_BBTROPR HD64461_IO_OFFSET(0x1068) /* ROP Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:83:24: note: in expansion of macro 'HD64461_BBTROPR'
83 | fb_writew(rop, HD64461_BBTROPR);
| ^~~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:123:33: note: in expansion of macro 'HD64461_IO_OFFSET'
123 | #define HD64461_BBTMDR HD64461_IO_OFFSET(0x106a) /* BitBLT Mode Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:94:49: note: in expansion of macro 'HD64461_BBTMDR'
94 | fb_writew((1 << 5) | 1, HD64461_BBTMDR);
| ^~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:123:33: note: in expansion of macro 'HD64461_IO_OFFSET'
123 | #define HD64461_BBTMDR HD64461_IO_OFFSET(0x106a) /* BitBLT Mode Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:96:38: note: in expansion of macro 'HD64461_BBTMDR'
96 | fb_writew(1, HD64461_BBTMDR);
| ^~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:123:33: note: in expansion of macro 'HD64461_IO_OFFSET'
123 | #define HD64461_BBTMDR HD64461_IO_OFFSET(0x106a) /* BitBLT Mode Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:101:45: note: in expansion of macro 'HD64461_BBTMDR'
101 | fb_writew((1 << 5), HD64461_BBTMDR);
| ^~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:123:33: note: in expansion of macro 'HD64461_IO_OFFSET'
123 | #define HD64461_BBTMDR HD64461_IO_OFFSET(0x106a) /* BitBLT Mode Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:103:38: note: in expansion of macro 'HD64461_BBTMDR'
103 | fb_writew(0, HD64461_BBTMDR);
| ^~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:116:33: note: in expansion of macro 'HD64461_IO_OFFSET'
116 | #define HD64461_BBTDWR HD64461_IO_OFFSET(0x105c) /* Destination Block Width Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:110:26: note: in expansion of macro 'HD64461_BBTDWR'
110 | fb_writew(width, HD64461_BBTDWR);
| ^~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:117:33: note: in expansion of macro 'HD64461_IO_OFFSET'
117 | #define HD64461_BBTDHR HD64461_IO_OFFSET(0x105e) /* Destination Block Height Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:111:27: note: in expansion of macro 'HD64461_BBTDHR'
111 | fb_writew(height, HD64461_BBTDHR);
| ^~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:113:33: note: in expansion of macro 'HD64461_IO_OFFSET'
113 | #define HD64461_BBTSSARL HD64461_IO_OFFSET(0x1056) /* Source Start Address Register (L) */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:112:35: note: in expansion of macro 'HD64461_BBTSSARL'
112 | fb_writew(saddr & 0xffff, HD64461_BBTSSARL);
| ^~~~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:112:33: note: in expansion of macro 'HD64461_IO_OFFSET'
112 | #define HD64461_BBTSSARH HD64461_IO_OFFSET(0x1054) /* Source Start Address Register (H) */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:113:32: note: in expansion of macro 'HD64461_BBTSSARH'
113 | fb_writew(saddr >> 16, HD64461_BBTSSARH);
| ^~~~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:115:33: note: in expansion of macro 'HD64461_IO_OFFSET'
115 | #define HD64461_BBTDSARL HD64461_IO_OFFSET(0x105a) /* Destination Start Address Register (L) */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:114:35: note: in expansion of macro 'HD64461_BBTDSARL'
114 | fb_writew(daddr & 0xffff, HD64461_BBTDSARL);
| ^~~~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:114:33: note: in expansion of macro 'HD64461_IO_OFFSET'
114 | #define HD64461_BBTDSARH HD64461_IO_OFFSET(0x1058) /* Destination Start Address Register (H) */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:115:32: note: in expansion of macro 'HD64461_BBTDSARH'
115 | fb_writew(daddr >> 16, HD64461_BBTDSARH);
| ^~~~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:121:33: note: in expansion of macro 'HD64461_IO_OFFSET'
121 | #define HD64461_BBTMARL HD64461_IO_OFFSET(0x1066) /* Mask Start Address Register (L) */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:118:43: note: in expansion of macro 'HD64461_BBTMARL'
118 | fb_writew(maddr & 0xffff, HD64461_BBTMARL);
| ^~~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:120:33: note: in expansion of macro 'HD64461_IO_OFFSET'
120 | #define HD64461_BBTMARH HD64461_IO_OFFSET(0x1064) /* Mask Start Address Register (H) */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:119:40: note: in expansion of macro 'HD64461_BBTMARH'
119 | fb_writew(maddr >> 16, HD64461_BBTMARH);
| ^~~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/video/fbdev/hitfb.c: In function 'hitfb_fillrect':
arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:122:33: note: in expansion of macro 'HD64461_IO_OFFSET'
122 | #define HD64461_BBTROPR HD64461_IO_OFFSET(0x1068) /* ROP Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:130:35: note: in expansion of macro 'HD64461_BBTROPR'
130 | fb_writew(0x00f0, HD64461_BBTROPR);
| ^~~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:123:33: note: in expansion of macro 'HD64461_IO_OFFSET'
123 | #define HD64461_BBTMDR HD64461_IO_OFFSET(0x106a) /* BitBLT Mode Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:131:31: note: in expansion of macro 'HD64461_BBTMDR'
131 | fb_writew(16, HD64461_BBTMDR);
| ^~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:92:33: note: in expansion of macro 'HD64461_IO_OFFSET'
92 | #define HD64461_GRSCR HD64461_IO_OFFSET(0x1042) /* Solid Color Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:135:35: note: in expansion of macro 'HD64461_GRSCR'
135 | HD64461_GRSCR);
| ^~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:92:33: note: in expansion of macro 'HD64461_IO_OFFSET'
92 | #define HD64461_GRSCR HD64461_IO_OFFSET(0x1042) /* Solid Color Register */
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:140:48: note: in expansion of macro 'HD64461_GRSCR'
140 | fb_writew(rect->color, HD64461_GRSCR);
| ^~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/video/fbdev/hitfb.c: In function 'hitfb_pan_display':
arch/sh/include/asm/hd64461.h:18:33: warning: passing argument 2 of 'fb_writew' makes pointer from integer without a cast [-Wint-conversion]
18 | #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| unsigned int
arch/sh/include/asm/hd64461.h:53:33: note: in expansion of macro 'HD64461_IO_OFFSET'
53 | #define HD64461_LCDCBAR HD64461_IO_OFFSET(0x1000)
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:165:56: note: in expansion of macro 'HD64461_LCDCBAR'
165 | fb_writew((yoffset*info->fix.line_length)>>10, HD64461_LCDCBAR);
| ^~~~~~~~~~~~~~~
include/asm-generic/fb.h:86:60: note: expected 'volatile void *' but argument is of type 'unsigned int'
86 | static inline void fb_writew(u16 b, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/video/fbdev/hitfb.c: At top level:
drivers/video/fbdev/hitfb.c:170:5: warning: no previous prototype for 'hitfb_blank' [-Wmissing-prototypes]
170 | int hitfb_blank(int blank_mode, struct fb_info *info)
| ^~~~~~~~~~~
drivers/video/fbdev/hitfb.c: In function 'hitfb_blank':
arch/sh/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/sh/include/asm/hd64461.h:70:33: note: in expansion of macro 'HD64461_IO_OFFSET'
70 | #define HD64461_LDR1 HD64461_IO_OFFSET(0x1010)
| ^~~~~~~~~~~~~~~~~
drivers/video/fbdev/hitfb.c:175:30: note: in expansion of macro 'HD64461_LDR1'
175 | v = fb_readw(HD64461_LDR1);
vim +/fb_readw +18 arch/sh/include/asm/hd64461.h
^1da177e4c3f41 include/asm-sh/hd64461/hd64461.h Linus Torvalds 2005-04-16 15
be15d65d97f924 include/asm-sh/hd64461.h Kristoffer Ericson 2007-07-12 16 /* Area 6 - Slot 0 - memory and/or IO card */
bec36eca6f5d1d arch/sh/include/asm/hd64461.h Paul Mundt 2009-05-15 17 #define HD64461_IOBASE 0xb0000000
bec36eca6f5d1d arch/sh/include/asm/hd64461.h Paul Mundt 2009-05-15 @18 #define HD64461_IO_OFFSET(x) (HD64461_IOBASE + (x))
bec36eca6f5d1d arch/sh/include/asm/hd64461.h Paul Mundt 2009-05-15 19 #define HD64461_PCC0_BASE HD64461_IO_OFFSET(0x8000000)
be15d65d97f924 include/asm-sh/hd64461.h Kristoffer Ericson 2007-07-12 20 #define HD64461_PCC0_ATTR (HD64461_PCC0_BASE) /* 0xb80000000 */
be15d65d97f924 include/asm-sh/hd64461.h Kristoffer Ericson 2007-07-12 21 #define HD64461_PCC0_COMM (HD64461_PCC0_BASE+HD64461_PCC_WINDOW) /* 0xb90000000 */
be15d65d97f924 include/asm-sh/hd64461.h Kristoffer Ericson 2007-07-12 22 #define HD64461_PCC0_IO (HD64461_PCC0_BASE+2*HD64461_PCC_WINDOW) /* 0xba0000000 */
^1da177e4c3f41 include/asm-sh/hd64461/hd64461.h Linus Torvalds 2005-04-16 23
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 14:03 ` kernel test robot
@ 2023-05-10 14:15 ` Arnd Bergmann
2023-05-10 14:27 ` Thomas Zimmermann
0 siblings, 1 reply; 31+ messages in thread
From: Arnd Bergmann @ 2023-05-10 14:15 UTC (permalink / raw)
To: kernel test robot, Thomas Zimmermann, Helge Deller,
Geert Uytterhoeven, Javier Martinez Canillas, Daniel Vetter,
Vineet Gupta, Huacai Chen, WANG Xuerui, David S . Miller,
James E . J . Bottomley, Sam Ravnborg, suijingfeng
Cc: oe-kbuild-all, Linux-Arch, linux-fbdev, linux-ia64, linux-parisc,
linux-kernel, dri-devel, linux-m68k, loongarch, sparclinux,
linux-snps-arc, linux-arm-kernel
On Wed, May 10, 2023, at 16:03, kernel test robot wrote:
>
> cc1: warning: arch/sh/include/mach-hp6xx: No such file or directory
> [-Wmissing-include-dirs]
> cc1: warning: arch/sh/include/mach-hp6xx: No such file or directory
> [-Wmissing-include-dirs]
> In file included from drivers/video/fbdev/hitfb.c:27:
> drivers/video/fbdev/hitfb.c: In function 'hitfb_accel_wait':
>>> arch/sh/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/sh/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) ;
I think that's a preexisting bug and I have no idea what the
correct solution is. Looking for HD64461 shows it being used
both with inw/outw and readw/writew, so there is no way to have
the correct type. The sh __raw_readw() definition hides this bug,
but that is a problem with arch/sh and it probably hides others
as well.
Arnd
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 12:34 ` Geert Uytterhoeven
@ 2023-05-10 14:20 ` Thomas Zimmermann
2023-05-10 14:34 ` Geert Uytterhoeven
0 siblings, 1 reply; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-10 14:20 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: deller, javierm, daniel, vgupta, chenhuacai, kernel, davem,
James.Bottomley, arnd, sam, suijingfeng, linux-fbdev, dri-devel,
linux-arch, linux-snps-arc, linux-kernel, linux-ia64, loongarch,
linux-m68k, sparclinux, linux-arm-kernel, linux-parisc
[-- Attachment #1.1: Type: text/plain, Size: 3412 bytes --]
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)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 14:15 ` Arnd Bergmann
@ 2023-05-10 14:27 ` Thomas Zimmermann
2023-05-10 15:54 ` Arnd Bergmann
0 siblings, 1 reply; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-10 14:27 UTC (permalink / raw)
To: Arnd Bergmann, kernel test robot, Helge Deller,
Geert Uytterhoeven, Javier Martinez Canillas, Daniel Vetter,
Vineet Gupta, Huacai Chen, WANG Xuerui, David S . Miller,
James E . J . Bottomley, Sam Ravnborg, suijingfeng
Cc: oe-kbuild-all, Linux-Arch, linux-fbdev, linux-ia64, linux-parisc,
linux-kernel, dri-devel, linux-m68k, loongarch, sparclinux,
linux-snps-arc, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 2376 bytes --]
Hi
Am 10.05.23 um 16:15 schrieb Arnd Bergmann:
> On Wed, May 10, 2023, at 16:03, kernel test robot wrote:
>
>>
>> cc1: warning: arch/sh/include/mach-hp6xx: No such file or directory
>> [-Wmissing-include-dirs]
>> cc1: warning: arch/sh/include/mach-hp6xx: No such file or directory
>> [-Wmissing-include-dirs]
>> In file included from drivers/video/fbdev/hitfb.c:27:
>> drivers/video/fbdev/hitfb.c: In function 'hitfb_accel_wait':
>>>> arch/sh/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/sh/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) ;
>
> I think that's a preexisting bug and I have no idea what the
> correct solution is. Looking for HD64461 shows it being used
> both with inw/outw and readw/writew, so there is no way to have
> the correct type. The sh __raw_readw() definition hides this bug,
> but that is a problem with arch/sh and it probably hides others
> as well.
The constant HD64461_IOBASE is defined as integer at
https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/hd64461.h#L17
but fb_readw() expects a volatile-void pointer. I guess we could add a
cast somewhere to silence the problem. In the current upstream code,
that appears to be done by sh's __raw_readw() internally:
https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L35
Best regards
Thomas
>
> Arnd
--
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)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 14:20 ` Thomas Zimmermann
@ 2023-05-10 14:34 ` Geert Uytterhoeven
2023-05-10 15:11 ` Thomas Zimmermann
0 siblings, 1 reply; 31+ messages in thread
From: Geert Uytterhoeven @ 2023-05-10 14:34 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: deller, javierm, daniel, vgupta, chenhuacai, kernel, davem,
James.Bottomley, arnd, sam, suijingfeng, linux-fbdev, dri-devel,
linux-arch, linux-snps-arc, linux-kernel, linux-ia64, loongarch,
linux-m68k, sparclinux, linux-arm-kernel, linux-parisc
Hi Thomas,
On Wed, May 10, 2023 at 4:20 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Am 10.05.23 um 14:34 schrieb Geert Uytterhoeven:
> > 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>
> >> --- 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_*().
Thanks, I forgot MIPS is using these grep-unfriendly factories...
> >> + *
> >> + * 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 14:34 ` Geert Uytterhoeven
@ 2023-05-10 15:11 ` Thomas Zimmermann
0 siblings, 0 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-10 15:11 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: deller, javierm, daniel, vgupta, chenhuacai, kernel, davem,
James.Bottomley, arnd, sam, suijingfeng, linux-fbdev, dri-devel,
linux-arch, linux-snps-arc, linux-kernel, linux-ia64, loongarch,
linux-m68k, sparclinux, linux-arm-kernel, linux-parisc
[-- Attachment #1.1: Type: text/plain, Size: 3847 bytes --]
Hi Geert
Am 10.05.23 um 16:34 schrieb Geert Uytterhoeven:
> Hi Thomas,
>
> On Wed, May 10, 2023 at 4:20 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Am 10.05.23 um 14:34 schrieb Geert Uytterhoeven:
>>> 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>
>
>>>> --- 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_*().
>
> Thanks, I forgot MIPS is using these grep-unfriendly factories...
>
>>>> + *
>>>> + * 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?
That works. I had a patch that adds all missing defines to MIPS' io.h.
Then I went with the current fix, which is self-contained within fbdev.
But I'd leave it to arch maintainers.
Best regards
Thomas
>
> 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)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 14:27 ` Thomas Zimmermann
@ 2023-05-10 15:54 ` Arnd Bergmann
2023-05-11 10:53 ` Thomas Zimmermann
` (2 more replies)
0 siblings, 3 replies; 31+ messages in thread
From: Arnd Bergmann @ 2023-05-10 15:54 UTC (permalink / raw)
To: Thomas Zimmermann, kernel test robot, Helge Deller,
Geert Uytterhoeven, Javier Martinez Canillas, Daniel Vetter,
Vineet Gupta, Huacai Chen, WANG Xuerui, David S . Miller,
James E . J . Bottomley, Sam Ravnborg, suijingfeng
Cc: oe-kbuild-all, Linux-Arch, linux-fbdev, linux-ia64, linux-parisc,
linux-kernel, dri-devel, linux-m68k, loongarch, sparclinux,
linux-snps-arc, linux-arm-kernel
On Wed, May 10, 2023, at 16:27, Thomas Zimmermann wrote:
> Am 10.05.23 um 16:15 schrieb Arnd Bergmann:
>> On Wed, May 10, 2023, at 16:03, kernel test robot wrote:
>> I think that's a preexisting bug and I have no idea what the
>> correct solution is. Looking for HD64461 shows it being used
>> both with inw/outw and readw/writew, so there is no way to have
>> the correct type. The sh __raw_readw() definition hides this bug,
>> but that is a problem with arch/sh and it probably hides others
>> as well.
>
> The constant HD64461_IOBASE is defined as integer at
>
>
> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/hd64461.h#L17
>
> but fb_readw() expects a volatile-void pointer. I guess we could add a
> cast somewhere to silence the problem. In the current upstream code,
> that appears to be done by sh's __raw_readw() internally:
>
>
> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L35
Sure, that would make it build again, but that still doesn't make the
code correct, since it's completely unclear what base address the
HD64461_IOBASE is relative to. The hp6xx platform code only passes it
through inw()/outw(), which take an offset relative to sh_io_port_base,
but that is not initialized on hp6xx. I tried to find in the history
when it broke, apparently that was in 2007 commit 34a780a0afeb ("sh:
hp6xx pata_platform support."), which removed the custom inw/outw
implementations.
Arnd
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces
2023-05-10 11:05 ` [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces Thomas Zimmermann
@ 2023-05-10 18:20 ` Sui Jingfeng
2023-05-11 7:55 ` Thomas Zimmermann
0 siblings, 1 reply; 31+ messages in thread
From: Sui Jingfeng @ 2023-05-10 18:20 UTC (permalink / raw)
To: Thomas Zimmermann, deller, geert, javierm, daniel, vgupta,
chenhuacai, kernel, davem, James.Bottomley, arnd, sam
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc
Hi, Thomas
I love your patch, yet something to improve:
On 2023/5/10 19:05, Thomas Zimmermann wrote:
> Fix coding style. No functional changes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
> Tested-by: Sui Jingfeng <suijingfeng@loongson.cn>
> ---
> drivers/video/fbdev/matrox/matroxfb_accel.c | 6 +++---
> drivers/video/fbdev/matrox/matroxfb_base.h | 4 ++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/fbdev/matrox/matroxfb_accel.c b/drivers/video/fbdev/matrox/matroxfb_accel.c
> index 9cb0685feddd..ce51227798a1 100644
> --- a/drivers/video/fbdev/matrox/matroxfb_accel.c
> +++ b/drivers/video/fbdev/matrox/matroxfb_accel.c
> @@ -88,7 +88,7 @@
>
> static inline void matrox_cfb4_pal(u_int32_t* pal) {
> unsigned int i;
> -
> +
> for (i = 0; i < 16; i++) {
> pal[i] = i * 0x11111111U;
> }
> @@ -96,7 +96,7 @@ static inline void matrox_cfb4_pal(u_int32_t* pal) {
>
> static inline void matrox_cfb8_pal(u_int32_t* pal) {
> unsigned int i;
> -
> +
> for (i = 0; i < 16; i++) {
> pal[i] = i * 0x01010101U;
> }
> @@ -482,7 +482,7 @@ static void matroxfb_1bpp_imageblit(struct matrox_fb_info *minfo, u_int32_t fgx,
> /* Tell... well, why bother... */
> while (height--) {
> size_t i;
> -
> +
> for (i = 0; i < step; i += 4) {
> /* Hope that there are at least three readable bytes beyond the end of bitmap */
> fb_writel(get_unaligned((u_int32_t*)(chardata + i)),mmio.vaddr);
> diff --git a/drivers/video/fbdev/matrox/matroxfb_base.h b/drivers/video/fbdev/matrox/matroxfb_base.h
> index 958be6805f87..c93c69bbcd57 100644
> --- a/drivers/video/fbdev/matrox/matroxfb_base.h
> +++ b/drivers/video/fbdev/matrox/matroxfb_base.h
> @@ -301,9 +301,9 @@ struct matrox_altout {
> int (*verifymode)(void* altout_dev, u_int32_t mode);
> int (*getqueryctrl)(void* altout_dev,
> struct v4l2_queryctrl* ctrl);
Noticed that there are plenty of coding style problems in matroxfb_base.h,
why you only fix a few of them? Take this two line as an example,
shouldn't
they be fixed also as following?
int (*verifymode)(void *altout_dev, u_int32_t mode);
int (*getqueryctrl)(void *altout_dev,
struct v4l2_queryctrl *ctrl);
> - int (*getctrl)(void* altout_dev,
> + int (*getctrl)(void *altout_dev,
> struct v4l2_control* ctrl);
> - int (*setctrl)(void* altout_dev,
> + int (*setctrl)(void *altout_dev,
> struct v4l2_control* ctrl);
> };
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces
2023-05-10 18:20 ` Sui Jingfeng
@ 2023-05-11 7:55 ` Thomas Zimmermann
2023-05-11 9:24 ` Sui Jingfeng
2023-05-11 13:05 ` Helge Deller
0 siblings, 2 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-11 7:55 UTC (permalink / raw)
To: Sui Jingfeng, deller, geert, javierm, daniel, vgupta, chenhuacai,
kernel, davem, James.Bottomley, arnd, sam
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc
[-- Attachment #1.1: Type: text/plain, Size: 4433 bytes --]
Hi
Am 10.05.23 um 20:20 schrieb Sui Jingfeng:
> Hi, Thomas
>
>
> I love your patch, yet something to improve:
>
>
> On 2023/5/10 19:05, Thomas Zimmermann wrote:
>> Fix coding style. No functional changes.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
>> Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
>> Tested-by: Sui Jingfeng <suijingfeng@loongson.cn>
>> ---
>> drivers/video/fbdev/matrox/matroxfb_accel.c | 6 +++---
>> drivers/video/fbdev/matrox/matroxfb_base.h | 4 ++--
>> 2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/matrox/matroxfb_accel.c
>> b/drivers/video/fbdev/matrox/matroxfb_accel.c
>> index 9cb0685feddd..ce51227798a1 100644
>> --- a/drivers/video/fbdev/matrox/matroxfb_accel.c
>> +++ b/drivers/video/fbdev/matrox/matroxfb_accel.c
>> @@ -88,7 +88,7 @@
>> static inline void matrox_cfb4_pal(u_int32_t* pal) {
>> unsigned int i;
>> -
>> +
>> for (i = 0; i < 16; i++) {
>> pal[i] = i * 0x11111111U;
>> }
>> @@ -96,7 +96,7 @@ static inline void matrox_cfb4_pal(u_int32_t* pal) {
>> static inline void matrox_cfb8_pal(u_int32_t* pal) {
>> unsigned int i;
>> -
>> +
>> for (i = 0; i < 16; i++) {
>> pal[i] = i * 0x01010101U;
>> }
>> @@ -482,7 +482,7 @@ static void matroxfb_1bpp_imageblit(struct
>> matrox_fb_info *minfo, u_int32_t fgx,
>> /* Tell... well, why bother... */
>> while (height--) {
>> size_t i;
>> -
>> +
>> for (i = 0; i < step; i += 4) {
>> /* Hope that there are at least three readable
>> bytes beyond the end of bitmap */
>> fb_writel(get_unaligned((u_int32_t*)(chardata +
>> i)),mmio.vaddr);
>> diff --git a/drivers/video/fbdev/matrox/matroxfb_base.h
>> b/drivers/video/fbdev/matrox/matroxfb_base.h
>> index 958be6805f87..c93c69bbcd57 100644
>> --- a/drivers/video/fbdev/matrox/matroxfb_base.h
>> +++ b/drivers/video/fbdev/matrox/matroxfb_base.h
>> @@ -301,9 +301,9 @@ struct matrox_altout {
>> int (*verifymode)(void* altout_dev, u_int32_t mode);
>> int (*getqueryctrl)(void* altout_dev,
>> struct v4l2_queryctrl* ctrl);
>
> Noticed that there are plenty of coding style problems in matroxfb_base.h,
>
> why you only fix a few of them? Take this two line as an example,
> shouldn't
>
> they be fixed also as following?
I configured my text editor to remove trailing whitespaces
automatically. That keeps my own patches free of them. But the editor
removes all trailing whitespaces, including those that have been there
before. If I encounter such a case, I split out the whitespace fix and
submit it separately.
But the work I do within fbdev is mostly for improving DRM. For the
other issues in this file, I don't think that matroxfb should even be
around any longer. Fbdev has been deprecated for a long time. But a
small number of drivers are still in use and we still need its
framebuffer console. So someone should either put significant effort
into maintaining fbdev, or it should be phased out. But neither is
happening.
Best regards
Thomas
>
>
> int (*verifymode)(void *altout_dev, u_int32_t mode);
> int (*getqueryctrl)(void *altout_dev,
> struct v4l2_queryctrl *ctrl);
>
>
>> - int (*getctrl)(void* altout_dev,
>> + int (*getctrl)(void *altout_dev,
>> struct v4l2_control* ctrl);
>> - int (*setctrl)(void* altout_dev,
>> + int (*setctrl)(void *altout_dev,
>> struct v4l2_control* ctrl);
>> };
--
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)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces
2023-05-11 7:55 ` Thomas Zimmermann
@ 2023-05-11 9:24 ` Sui Jingfeng
2023-05-11 13:05 ` Helge Deller
1 sibling, 0 replies; 31+ messages in thread
From: Sui Jingfeng @ 2023-05-11 9:24 UTC (permalink / raw)
To: Thomas Zimmermann, deller, geert, javierm, daniel, vgupta,
chenhuacai, kernel, davem, James.Bottomley, arnd, sam
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc
Hi
On 2023/5/11 15:55, Thomas Zimmermann wrote:
> Hi
>
> Am 10.05.23 um 20:20 schrieb Sui Jingfeng:
>> Hi, Thomas
>>
>>
>> I love your patch, yet something to improve:
>>
>>
>> On 2023/5/10 19:05, Thomas Zimmermann wrote:
>>> Fix coding style. No functional changes.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>>> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
>>> Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
>>> Tested-by: Sui Jingfeng <suijingfeng@loongson.cn>
>>> ---
>>> drivers/video/fbdev/matrox/matroxfb_accel.c | 6 +++---
>>> drivers/video/fbdev/matrox/matroxfb_base.h | 4 ++--
>>> 2 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/video/fbdev/matrox/matroxfb_accel.c
>>> b/drivers/video/fbdev/matrox/matroxfb_accel.c
>>> index 9cb0685feddd..ce51227798a1 100644
>>> --- a/drivers/video/fbdev/matrox/matroxfb_accel.c
>>> +++ b/drivers/video/fbdev/matrox/matroxfb_accel.c
>>> @@ -88,7 +88,7 @@
>>> static inline void matrox_cfb4_pal(u_int32_t* pal) {
>>> unsigned int i;
>>> -
>>> +
>>> for (i = 0; i < 16; i++) {
>>> pal[i] = i * 0x11111111U;
>>> }
>>> @@ -96,7 +96,7 @@ static inline void matrox_cfb4_pal(u_int32_t* pal) {
>>> static inline void matrox_cfb8_pal(u_int32_t* pal) {
>>> unsigned int i;
>>> -
>>> +
>>> for (i = 0; i < 16; i++) {
>>> pal[i] = i * 0x01010101U;
>>> }
>>> @@ -482,7 +482,7 @@ static void matroxfb_1bpp_imageblit(struct
>>> matrox_fb_info *minfo, u_int32_t fgx,
>>> /* Tell... well, why bother... */
>>> while (height--) {
>>> size_t i;
>>> -
>>> +
>>> for (i = 0; i < step; i += 4) {
>>> /* Hope that there are at least three readable
>>> bytes beyond the end of bitmap */
>>> fb_writel(get_unaligned((u_int32_t*)(chardata + i)),mmio.vaddr);
>>> diff --git a/drivers/video/fbdev/matrox/matroxfb_base.h
>>> b/drivers/video/fbdev/matrox/matroxfb_base.h
>>> index 958be6805f87..c93c69bbcd57 100644
>>> --- a/drivers/video/fbdev/matrox/matroxfb_base.h
>>> +++ b/drivers/video/fbdev/matrox/matroxfb_base.h
>>> @@ -301,9 +301,9 @@ struct matrox_altout {
>>> int (*verifymode)(void* altout_dev, u_int32_t mode);
>>> int (*getqueryctrl)(void* altout_dev,
>>> struct v4l2_queryctrl* ctrl);
>>
>> Noticed that there are plenty of coding style problems in
>> matroxfb_base.h,
>>
>> why you only fix a few of them? Take this two line as an example,
>> shouldn't
>>
>> they be fixed also as following?
>
> I configured my text editor to remove trailing whitespaces
> automatically. That keeps my own patches free of them. But the editor
> removes all trailing whitespaces, including those that have been there
> before. If I encounter such a case, I split out the whitespace fix and
> submit it separately.
>
> But the work I do within fbdev is mostly for improving DRM. For the
> other issues in this file, I don't think that matroxfb should even be
> around any longer. Fbdev has been deprecated for a long time. But a
> small number of drivers are still in use and we still need its
> framebuffer console. So someone should either put significant effort
> into maintaining fbdev, or it should be phased out. But neither is
> happening.
>
Ok, no problem, that sound fine and reasonable then.
The lines being modified has trailing whitespaces.
And I tested your patch again last night on loongarch and mips platform.
It still works in my testing case.
> Best regards
> Thomas
>
>>
>>
>> int (*verifymode)(void *altout_dev, u_int32_t mode);
>> int (*getqueryctrl)(void *altout_dev,
>> struct v4l2_queryctrl *ctrl);
>>
>>
>>> - int (*getctrl)(void* altout_dev,
>>> + int (*getctrl)(void *altout_dev,
>>> struct v4l2_control* ctrl);
>>> - int (*setctrl)(void* altout_dev,
>>> + int (*setctrl)(void *altout_dev,
>>> struct v4l2_control* ctrl);
>>> };
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 15:54 ` Arnd Bergmann
@ 2023-05-11 10:53 ` Thomas Zimmermann
2023-05-11 12:35 ` Geert Uytterhoeven
2023-05-11 13:12 ` Thomas Zimmermann
2 siblings, 0 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-11 10:53 UTC (permalink / raw)
To: Arnd Bergmann, kernel test robot, Helge Deller,
Geert Uytterhoeven, Javier Martinez Canillas, Daniel Vetter,
Vineet Gupta, Huacai Chen, WANG Xuerui, David S . Miller,
James E . J . Bottomley, Sam Ravnborg, suijingfeng
Cc: Linux-Arch, linux-fbdev, linux-ia64, linux-parisc, linux-kernel,
dri-devel, linux-m68k, loongarch, oe-kbuild-all, sparclinux,
linux-snps-arc, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 2139 bytes --]
Hi
Am 10.05.23 um 17:54 schrieb Arnd Bergmann:
> On Wed, May 10, 2023, at 16:27, Thomas Zimmermann wrote:
>> Am 10.05.23 um 16:15 schrieb Arnd Bergmann:
>>> On Wed, May 10, 2023, at 16:03, kernel test robot wrote:
>
>>> I think that's a preexisting bug and I have no idea what the
>>> correct solution is. Looking for HD64461 shows it being used
>>> both with inw/outw and readw/writew, so there is no way to have
>>> the correct type. The sh __raw_readw() definition hides this bug,
>>> but that is a problem with arch/sh and it probably hides others
>>> as well.
>>
>> The constant HD64461_IOBASE is defined as integer at
>>
>>
>> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/hd64461.h#L17
>>
>> but fb_readw() expects a volatile-void pointer. I guess we could add a
>> cast somewhere to silence the problem. In the current upstream code,
>> that appears to be done by sh's __raw_readw() internally:
>>
>>
>> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L35
>
> Sure, that would make it build again, but that still doesn't make the
> code correct, since it's completely unclear what base address the
Oh, OK. I thought it's like vgafb, which grabs the fixed VGA framebuffer
range.
> HD64461_IOBASE is relative to. The hp6xx platform code only passes it
> through inw()/outw(), which take an offset relative to sh_io_port_base,
> but that is not initialized on hp6xx. I tried to find in the history
> when it broke, apparently that was in 2007 commit 34a780a0afeb ("sh:
> hp6xx pata_platform support."), which removed the custom inw/outw
> implementations.
Thanks for looking this up. If this driver has been broken for 15 years,
the correct fix is to delete it. I've meanwhile prepared the second-best
fix, which is the type casting. It'll be in the next patchset.
Best regards
Thomas
>
> Arnd
--
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)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 15:54 ` Arnd Bergmann
2023-05-11 10:53 ` Thomas Zimmermann
@ 2023-05-11 12:35 ` Geert Uytterhoeven
2023-05-11 12:48 ` Arnd Bergmann
2023-05-11 13:22 ` Artur Rojek
2023-05-11 13:12 ` Thomas Zimmermann
2 siblings, 2 replies; 31+ messages in thread
From: Geert Uytterhoeven @ 2023-05-11 12:35 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Thomas Zimmermann, kernel test robot, Helge Deller,
Javier Martinez Canillas, Daniel Vetter, Vineet Gupta,
Huacai Chen, WANG Xuerui, David S . Miller,
James E . J . Bottomley, Sam Ravnborg, suijingfeng,
oe-kbuild-all, Linux-Arch, linux-fbdev, linux-ia64, linux-parisc,
linux-kernel, dri-devel, linux-m68k, loongarch, sparclinux,
linux-snps-arc, linux-arm-kernel, Artur Rojek
Hi Arnd,
CC Artur, who's working on HP Jornada 680.
On Wed, May 10, 2023 at 5:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Wed, May 10, 2023, at 16:27, Thomas Zimmermann wrote:
> > Am 10.05.23 um 16:15 schrieb Arnd Bergmann:
> >> On Wed, May 10, 2023, at 16:03, kernel test robot wrote:
>
> >> I think that's a preexisting bug and I have no idea what the
> >> correct solution is. Looking for HD64461 shows it being used
> >> both with inw/outw and readw/writew, so there is no way to have
> >> the correct type. The sh __raw_readw() definition hides this bug,
> >> but that is a problem with arch/sh and it probably hides others
> >> as well.
> >
> > The constant HD64461_IOBASE is defined as integer at
> >
> >
> > https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/hd64461.h#L17
> >
> > but fb_readw() expects a volatile-void pointer. I guess we could add a
> > cast somewhere to silence the problem. In the current upstream code,
> > that appears to be done by sh's __raw_readw() internally:
> >
> >
> > https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L35
>
> Sure, that would make it build again, but that still doesn't make the
> code correct, since it's completely unclear what base address the
> HD64461_IOBASE is relative to. The hp6xx platform code only passes it
> through inw()/outw(), which take an offset relative to sh_io_port_base,
> but that is not initialized on hp6xx. I tried to find in the history
> when it broke, apparently that was in 2007 commit 34a780a0afeb ("sh:
> hp6xx pata_platform support."), which removed the custom inw/outw
> implementations.
See also commit 4aafae27d0ce73f8 ("sh: hd64461 tidying."), which
claims they are no longer needed.
Don't the I/O port macros just treat the port as an absolute base address
when sh_io_port_base isn't set?
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-11 12:35 ` Geert Uytterhoeven
@ 2023-05-11 12:48 ` Arnd Bergmann
2023-05-11 13:22 ` Artur Rojek
1 sibling, 0 replies; 31+ messages in thread
From: Arnd Bergmann @ 2023-05-11 12:48 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Thomas Zimmermann, kernel test robot, Helge Deller,
Javier Martinez Canillas, Daniel Vetter, Vineet Gupta,
Huacai Chen, WANG Xuerui, David S . Miller,
James E . J . Bottomley, Sam Ravnborg, suijingfeng,
oe-kbuild-all, Linux-Arch, linux-fbdev, linux-ia64, linux-parisc,
linux-kernel, dri-devel, linux-m68k, loongarch, sparclinux,
linux-snps-arc, linux-arm-kernel, Artur Rojek
On Thu, May 11, 2023, at 14:35, Geert Uytterhoeven wrote:
> CC Artur, who's working on HP Jornada 680.
>
> On Wed, May 10, 2023 at 5:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Wed, May 10, 2023, at 16:27, Thomas Zimmermann wrote:
>> > Am 10.05.23 um 16:15 schrieb Arnd Bergmann:
>> >> On Wed, May 10, 2023, at 16:03, kernel test robot wrote:
>
> See also commit 4aafae27d0ce73f8 ("sh: hd64461 tidying."), which
> claims they are no longer needed.
>
> Don't the I/O port macros just treat the port as an absolute base address
> when sh_io_port_base isn't set?
As far as I can tell, sh_io_port_base gets initialized to '-1'
specifically to prevent that from working by accident. So it's
almost treated as an absolute base address, but the off-by-one
offset ensures this never actually works unless it was first
set to the correct value.
Arnd
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces
2023-05-11 7:55 ` Thomas Zimmermann
2023-05-11 9:24 ` Sui Jingfeng
@ 2023-05-11 13:05 ` Helge Deller
2023-05-11 13:10 ` Geert Uytterhoeven
2023-05-11 14:27 ` Thomas Zimmermann
1 sibling, 2 replies; 31+ messages in thread
From: Helge Deller @ 2023-05-11 13:05 UTC (permalink / raw)
To: Thomas Zimmermann, Sui Jingfeng, geert, javierm, daniel, vgupta,
chenhuacai, kernel, davem, arnd, sam
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc
On 5/11/23 09:55, Thomas Zimmermann wrote:
> Hi
>
> Am 10.05.23 um 20:20 schrieb Sui Jingfeng:
>> Hi, Thomas
>>
>>
>> I love your patch, yet something to improve:
>>
>>
>> On 2023/5/10 19:05, Thomas Zimmermann wrote:
>>> Fix coding style. No functional changes.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>>> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
>>> Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
>>> Tested-by: Sui Jingfeng <suijingfeng@loongson.cn>
>>> ---
>>> drivers/video/fbdev/matrox/matroxfb_accel.c | 6 +++---
>>> drivers/video/fbdev/matrox/matroxfb_base.h | 4 ++--
>>> 2 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/video/fbdev/matrox/matroxfb_accel.c b/drivers/video/fbdev/matrox/matroxfb_accel.c
>>> index 9cb0685feddd..ce51227798a1 100644
>>> --- a/drivers/video/fbdev/matrox/matroxfb_accel.c
>>> +++ b/drivers/video/fbdev/matrox/matroxfb_accel.c
>>> @@ -88,7 +88,7 @@
>>> static inline void matrox_cfb4_pal(u_int32_t* pal) {
>>> unsigned int i;
>>> -
>>> +
>>> for (i = 0; i < 16; i++) {
>>> pal[i] = i * 0x11111111U;
>>> }
>>> @@ -96,7 +96,7 @@ static inline void matrox_cfb4_pal(u_int32_t* pal) {
>>> static inline void matrox_cfb8_pal(u_int32_t* pal) {
>>> unsigned int i;
>>> -
>>> +
>>> for (i = 0; i < 16; i++) {
>>> pal[i] = i * 0x01010101U;
>>> }
>>> @@ -482,7 +482,7 @@ static void matroxfb_1bpp_imageblit(struct matrox_fb_info *minfo, u_int32_t fgx,
>>> /* Tell... well, why bother... */
>>> while (height--) {
>>> size_t i;
>>> -
>>> +
>>> for (i = 0; i < step; i += 4) {
>>> /* Hope that there are at least three readable bytes beyond the end of bitmap */
>>> fb_writel(get_unaligned((u_int32_t*)(chardata + i)),mmio.vaddr);
>>> diff --git a/drivers/video/fbdev/matrox/matroxfb_base.h b/drivers/video/fbdev/matrox/matroxfb_base.h
>>> index 958be6805f87..c93c69bbcd57 100644
>>> --- a/drivers/video/fbdev/matrox/matroxfb_base.h
>>> +++ b/drivers/video/fbdev/matrox/matroxfb_base.h
>>> @@ -301,9 +301,9 @@ struct matrox_altout {
>>> int (*verifymode)(void* altout_dev, u_int32_t mode);
>>> int (*getqueryctrl)(void* altout_dev,
>>> struct v4l2_queryctrl* ctrl);
>>
>> Noticed that there are plenty of coding style problems in matroxfb_base.h,
>>
>> why you only fix a few of them? Take this two line as an example, shouldn't
>>
>> they be fixed also as following?
>
> I configured my text editor to remove trailing whitespaces
> automatically. That keeps my own patches free of them. But the
> editor removes all trailing whitespaces, including those that have
> been there before. If I encounter such a case, I split out the
> whitespace fix and submit it separately.
>
> But the work I do within fbdev is mostly for improving DRM.
Sure.
> For the
> other issues in this file, I don't think that matroxfb should even be
> around any longer. Fbdev has been deprecated for a long time. But a
> small number of drivers are still in use and we still need its
> framebuffer console. So someone should either put significant effort
> into maintaining fbdev, or it should be phased out. But neither is
> happening.
You're wrong.
You don't mention that for most older machines DRM isn't an acceptable
way to go due to it's limitations, e.g. it's low-speed due to missing
2D-acceleration for older cards and and it's incapability to change screen
resolution at runtime (just to name two of the bigger limitations here).
So, unless we somehow find a good way to move such drivers over to DRM
(with a set of minimal 2D acceleration), they are still important.
Actually, I just did test matroxfb and pm2fb successfully a few days back, and
they worked. For some smaller issues I've prepared patches, which are on hold
due conflicts with your latest file-move-around- and whitespace-changes which are partly
in drm-misc.
And I do have some upcoming additional patches for console support.
Helge
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces
2023-05-11 13:05 ` Helge Deller
@ 2023-05-11 13:10 ` Geert Uytterhoeven
2023-05-11 16:23 ` Helge Deller
2023-05-11 14:27 ` Thomas Zimmermann
1 sibling, 1 reply; 31+ messages in thread
From: Geert Uytterhoeven @ 2023-05-11 13:10 UTC (permalink / raw)
To: Helge Deller
Cc: Thomas Zimmermann, Sui Jingfeng, javierm, daniel, vgupta,
chenhuacai, kernel, davem, arnd, sam, linux-fbdev, dri-devel,
linux-arch, linux-snps-arc, linux-kernel, linux-ia64, loongarch,
linux-m68k, sparclinux, linux-arm-kernel, linux-parisc
Hi Helge,
On Thu, May 11, 2023 at 3:05 PM Helge Deller <deller@gmx.de> wrote:
> On 5/11/23 09:55, Thomas Zimmermann wrote:
> > But the work I do within fbdev is mostly for improving DRM.
>
> Sure.
>
> > For the
> > other issues in this file, I don't think that matroxfb should even be
> > around any longer. Fbdev has been deprecated for a long time. But a
> > small number of drivers are still in use and we still need its
> > framebuffer console. So someone should either put significant effort
> > into maintaining fbdev, or it should be phased out. But neither is
> > happening.
>
> You're wrong.
>
> You don't mention that for most older machines DRM isn't an acceptable
> way to go due to it's limitations, e.g. it's low-speed due to missing
> 2D-acceleration for older cards and and it's incapability to change screen
> resolution at runtime (just to name two of the bigger limitations here).
> So, unless we somehow find a good way to move such drivers over to DRM
> (with a set of minimal 2D acceleration), they are still important.
DRM can change resolution at runtime, just not through the fbdev API.
Or do you mean the resolution of the text console, akin to
"fbset <mode>"? I have to admit I do not know if there is a command
line tool to do that...
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-10 15:54 ` Arnd Bergmann
2023-05-11 10:53 ` Thomas Zimmermann
2023-05-11 12:35 ` Geert Uytterhoeven
@ 2023-05-11 13:12 ` Thomas Zimmermann
2 siblings, 0 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-11 13:12 UTC (permalink / raw)
To: Arnd Bergmann, kernel test robot, Helge Deller,
Geert Uytterhoeven, Javier Martinez Canillas, Daniel Vetter,
Vineet Gupta, Huacai Chen, WANG Xuerui, David S . Miller,
James E . J . Bottomley, Sam Ravnborg, suijingfeng
Cc: Linux-Arch, linux-fbdev, linux-ia64, linux-parisc, linux-kernel,
dri-devel, linux-m68k, loongarch, oe-kbuild-all, sparclinux,
linux-snps-arc, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 2375 bytes --]
Hi
Am 10.05.23 um 17:54 schrieb Arnd Bergmann:
> On Wed, May 10, 2023, at 16:27, Thomas Zimmermann wrote:
>> Am 10.05.23 um 16:15 schrieb Arnd Bergmann:
>>> On Wed, May 10, 2023, at 16:03, kernel test robot wrote:
>
>>> I think that's a preexisting bug and I have no idea what the
>>> correct solution is. Looking for HD64461 shows it being used
>>> both with inw/outw and readw/writew, so there is no way to have
>>> the correct type. The sh __raw_readw() definition hides this bug,
>>> but that is a problem with arch/sh and it probably hides others
>>> as well.
>>
>> The constant HD64461_IOBASE is defined as integer at
>>
>>
>> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/hd64461.h#L17
>>
>> but fb_readw() expects a volatile-void pointer. I guess we could add a
>> cast somewhere to silence the problem. In the current upstream code,
>> that appears to be done by sh's __raw_readw() internally:
>>
>>
>> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L35
>
> Sure, that would make it build again, but that still doesn't make the
> code correct, since it's completely unclear what base address the
> HD64461_IOBASE is relative to. The hp6xx platform code only passes it
> through inw()/outw(), which take an offset relative to sh_io_port_base,
> but that is not initialized on hp6xx. I tried to find in the history
> when it broke, apparently that was in 2007 commit 34a780a0afeb ("sh:
> hp6xx pata_platform support."), which removed the custom inw/outw
> implementations.
It just occured to me that these fb_read and fb_write calls are probably
all wrong. The fb_ interfaces are for framebuffer I/O memory. The driver
uses them to access the regular state registers. The writew() on sh is
definitely different. [1]
I assume that it only works because CONFIG_SWAP_IO_SPACE [2] is not set
in hp6xx_defconfig.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L55
[2]
https://elixir.bootlin.com/linux/latest/source/arch/sh/include/mach-common/mach/mangle-port.h#L22
>
> Arnd
--
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)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-11 12:35 ` Geert Uytterhoeven
2023-05-11 12:48 ` Arnd Bergmann
@ 2023-05-11 13:22 ` Artur Rojek
2023-05-11 13:40 ` Arnd Bergmann
1 sibling, 1 reply; 31+ messages in thread
From: Artur Rojek @ 2023-05-11 13:22 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Arnd Bergmann, Thomas Zimmermann, kernel test robot,
Helge Deller, Javier Martinez Canillas, Daniel Vetter,
Vineet Gupta, Huacai Chen, WANG Xuerui, David S . Miller,
James E . J . Bottomley, Sam Ravnborg, suijingfeng,
oe-kbuild-all, Linux-Arch, linux-fbdev, linux-ia64, linux-parisc,
linux-kernel, dri-devel, linux-m68k, loongarch, sparclinux,
linux-snps-arc, linux-arm-kernel
On 2023-05-11 14:35, Geert Uytterhoeven wrote:
> Hi Arnd,
>
> CC Artur, who's working on HP Jornada 680.
Thanks for CC'ing me - I faced this exact issue while working on my
(still not upstreamed) hd6446x PCMCIA controller driver. The PCMCIA
subsystem uses `inb/outb`, which expect the `sh_io_port_base` to be set
to something else than the default `-1`. At first I tried to set it to
`0xa0000000`, so that all I/O goes through the fixed, non-cacheable P2
area. That however broke some other driver code (I had no time to debug
which one). Eventually I ended up taking a suggestion from a MIPS PCMCIA
driver [1] and simply substract the broken `sh_io_port_base` address
from `HD64461_IOBASE`, as the base for `socket.io_offset`. This way all
the PCMCIA `inb/outb` accesses are absolute, no matter what the
`sh_io_port_base` is set to. This of course is a very ugly solution and
we should instead fix the root cause of this mess. I will have a better
look at this patch set and the problem at hand at a later date.
Cheers,
Artur
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pcmcia/db1xxx_ss.c?h=v6.4-rc1#n527
>
> On Wed, May 10, 2023 at 5:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Wed, May 10, 2023, at 16:27, Thomas Zimmermann wrote:
>> > Am 10.05.23 um 16:15 schrieb Arnd Bergmann:
>> >> On Wed, May 10, 2023, at 16:03, kernel test robot wrote:
>>
>> >> I think that's a preexisting bug and I have no idea what the
>> >> correct solution is. Looking for HD64461 shows it being used
>> >> both with inw/outw and readw/writew, so there is no way to have
>> >> the correct type. The sh __raw_readw() definition hides this bug,
>> >> but that is a problem with arch/sh and it probably hides others
>> >> as well.
>> >
>> > The constant HD64461_IOBASE is defined as integer at
>> >
>> >
>> > https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/hd64461.h#L17
>> >
>> > but fb_readw() expects a volatile-void pointer. I guess we could add a
>> > cast somewhere to silence the problem. In the current upstream code,
>> > that appears to be done by sh's __raw_readw() internally:
>> >
>> >
>> > https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L35
>>
>> Sure, that would make it build again, but that still doesn't make the
>> code correct, since it's completely unclear what base address the
>> HD64461_IOBASE is relative to. The hp6xx platform code only passes it
>> through inw()/outw(), which take an offset relative to
>> sh_io_port_base,
>> but that is not initialized on hp6xx. I tried to find in the history
>> when it broke, apparently that was in 2007 commit 34a780a0afeb ("sh:
>> hp6xx pata_platform support."), which removed the custom inw/outw
>> implementations.
>
> See also commit 4aafae27d0ce73f8 ("sh: hd64461 tidying."), which
> claims they are no longer needed.
>
> Don't the I/O port macros just treat the port as an absolute base
> address
> when sh_io_port_base isn't set?
>
> Gr{oetje,eeting}s,
>
> Geert
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
2023-05-11 13:22 ` Artur Rojek
@ 2023-05-11 13:40 ` Arnd Bergmann
0 siblings, 0 replies; 31+ messages in thread
From: Arnd Bergmann @ 2023-05-11 13:40 UTC (permalink / raw)
To: Artur Rojek, Geert Uytterhoeven
Cc: Thomas Zimmermann, kernel test robot, Helge Deller,
Javier Martinez Canillas, Daniel Vetter, Vineet Gupta,
Huacai Chen, WANG Xuerui, David S . Miller,
James E . J . Bottomley, Sam Ravnborg, suijingfeng,
oe-kbuild-all, Linux-Arch, linux-fbdev, linux-ia64, linux-parisc,
linux-kernel, dri-devel, linux-m68k, loongarch, sparclinux,
linux-snps-arc, linux-arm-kernel
On Thu, May 11, 2023, at 15:22, Artur Rojek wrote:
> On 2023-05-11 14:35, Geert Uytterhoeven wrote:
>>
>> CC Artur, who's working on HP Jornada 680.
> Thanks for CC'ing me - I faced this exact issue while working on my
> (still not upstreamed) hd6446x PCMCIA controller driver. The PCMCIA
> subsystem uses `inb/outb`, which expect the `sh_io_port_base` to be set
> to something else than the default `-1`. At first I tried to set it to
> `0xa0000000`, so that all I/O goes through the fixed, non-cacheable P2
> area. That however broke some other driver code (I had no time to debug
> which one). Eventually I ended up taking a suggestion from a MIPS PCMCIA
> driver [1] and simply substract the broken `sh_io_port_base` address
> from `HD64461_IOBASE`, as the base for `socket.io_offset`. This way all
> the PCMCIA `inb/outb` accesses are absolute, no matter what the
> `sh_io_port_base` is set to. This of course is a very ugly solution and
> we should instead fix the root cause of this mess. I will have a better
> look at this patch set and the problem at hand at a later date.
>
> Cheers,
> Artur
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pcmcia/db1xxx_ss.c?h=v6.4-rc1#n527
I think the best fix would be to change all those drivers away
from using inb/outb to readb/writeb, except when they access the
actual PCMCIA I/O space behind the bridge.
On most of the modern architectures, inb(addr) now turns into
approximately readb(PCI_IOBASE + addr), with a bit of extra
logic to deal with endianess and barrier semantics.
PCI_IOBASE in turn tends to be a hardcoded virtual address
to which the physical I/O space window gets mapped during
early boot, though you can also #define it to sh_io_port_base
if you want to allocate the virtual address dynamically and
leave the existing logic unchanged.
Setting sh_io_port_base to zero however is a problem for any
driver that passes a small port number into it -- this then
turns into a user space pointer dereference, which is trivially
exploitable.
Arnd
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces
2023-05-11 13:05 ` Helge Deller
2023-05-11 13:10 ` Geert Uytterhoeven
@ 2023-05-11 14:27 ` Thomas Zimmermann
2023-05-11 17:02 ` Helge Deller
2023-05-12 10:04 ` Finn Thain
1 sibling, 2 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-11 14:27 UTC (permalink / raw)
To: Helge Deller, Sui Jingfeng, geert, javierm, daniel, vgupta,
chenhuacai, kernel, davem, arnd, sam
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc
[-- Attachment #1.1: Type: text/plain, Size: 6494 bytes --]
Hi
Am 11.05.23 um 15:05 schrieb Helge Deller:
> On 5/11/23 09:55, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 10.05.23 um 20:20 schrieb Sui Jingfeng:
>>> Hi, Thomas
>>>
>>>
>>> I love your patch, yet something to improve:
>>>
>>>
>>> On 2023/5/10 19:05, Thomas Zimmermann wrote:
>>>> Fix coding style. No functional changes.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>>>> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
>>>> Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
>>>> Tested-by: Sui Jingfeng <suijingfeng@loongson.cn>
>>>> ---
>>>> drivers/video/fbdev/matrox/matroxfb_accel.c | 6 +++---
>>>> drivers/video/fbdev/matrox/matroxfb_base.h | 4 ++--
>>>> 2 files changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/video/fbdev/matrox/matroxfb_accel.c
>>>> b/drivers/video/fbdev/matrox/matroxfb_accel.c
>>>> index 9cb0685feddd..ce51227798a1 100644
>>>> --- a/drivers/video/fbdev/matrox/matroxfb_accel.c
>>>> +++ b/drivers/video/fbdev/matrox/matroxfb_accel.c
>>>> @@ -88,7 +88,7 @@
>>>> static inline void matrox_cfb4_pal(u_int32_t* pal) {
>>>> unsigned int i;
>>>> -
>>>> +
>>>> for (i = 0; i < 16; i++) {
>>>> pal[i] = i * 0x11111111U;
>>>> }
>>>> @@ -96,7 +96,7 @@ static inline void matrox_cfb4_pal(u_int32_t* pal) {
>>>> static inline void matrox_cfb8_pal(u_int32_t* pal) {
>>>> unsigned int i;
>>>> -
>>>> +
>>>> for (i = 0; i < 16; i++) {
>>>> pal[i] = i * 0x01010101U;
>>>> }
>>>> @@ -482,7 +482,7 @@ static void matroxfb_1bpp_imageblit(struct
>>>> matrox_fb_info *minfo, u_int32_t fgx,
>>>> /* Tell... well, why bother... */
>>>> while (height--) {
>>>> size_t i;
>>>> -
>>>> +
>>>> for (i = 0; i < step; i += 4) {
>>>> /* Hope that there are at least three readable
>>>> bytes beyond the end of bitmap */
>>>> fb_writel(get_unaligned((u_int32_t*)(chardata
>>>> + i)),mmio.vaddr);
>>>> diff --git a/drivers/video/fbdev/matrox/matroxfb_base.h
>>>> b/drivers/video/fbdev/matrox/matroxfb_base.h
>>>> index 958be6805f87..c93c69bbcd57 100644
>>>> --- a/drivers/video/fbdev/matrox/matroxfb_base.h
>>>> +++ b/drivers/video/fbdev/matrox/matroxfb_base.h
>>>> @@ -301,9 +301,9 @@ struct matrox_altout {
>>>> int (*verifymode)(void* altout_dev, u_int32_t mode);
>>>> int (*getqueryctrl)(void* altout_dev,
>>>> struct v4l2_queryctrl* ctrl);
>>>
>>> Noticed that there are plenty of coding style problems in
>>> matroxfb_base.h,
>>>
>>> why you only fix a few of them? Take this two line as an example,
>>> shouldn't
>>>
>>> they be fixed also as following?
>>
>> I configured my text editor to remove trailing whitespaces
>> automatically. That keeps my own patches free of them. But the
>> editor removes all trailing whitespaces, including those that have
>> been there before. If I encounter such a case, I split out the
>> whitespace fix and submit it separately.
>>
>> But the work I do within fbdev is mostly for improving DRM.
>
> Sure.
>
>> For the
>> other issues in this file, I don't think that matroxfb should even be
>> around any longer. Fbdev has been deprecated for a long time. But a
>> small number of drivers are still in use and we still need its
>> framebuffer console. So someone should either put significant effort
>> into maintaining fbdev, or it should be phased out. But neither is
>> happening.
>
> You're wrong.
I'm not. I don't claim that these drivers are all broken. But fbdev as a
whole is bit-rotting and no one attempts to address this. There are
several recent examples of this:
* I recently send out a 100-patches series to improve parameter
parsing and avoid memory leaks. That got shot down. I didn't attempt to
support parameter parsing for module builds.
* There's been a 15-yrs old bug in fbdev's read/write where they
return an incorrect value.
* See the other discussion on this patchset on the state of hitfb.
* The fbdev code I recently cleaned up had bugs in how it uses some of
fbdev's basic building blocks (see the screen_base/screen_buffer confusion).
* <asm-generic/fb.h> has been in the tree since 2009 and no one
attempted to include it until now.
None of this is a sign of good maintenance.
As I've worked on DRM's fbdev emulation a lot, I try to be a good kernel
citizen and clean up in fbdev as well when I see a problem. But I'd
really like to see most of these drivers being moved into staging and
deleted soon afterwards. Users will complain about those drivers that
are really still required. Those might be worth to spend effort on.
>
> You don't mention that for most older machines DRM isn't an acceptable
> way to go due to it's limitations, e.g. it's low-speed due to missing
> 2D-acceleration for older cards and and it's incapability to change screen
> resolution at runtime (just to name two of the bigger limitations here).
You can change resolution at runtime; just not through fbdev ioctls.
There's no technical limitation here. No one found any use for this, so
it's not there.
> So, unless we somehow find a good way to move such drivers over to DRM
> (with a set of minimal 2D acceleration), they are still important.
2d acceleration is mostly useful for the framebuffer console. You can do
that with DRM and drivers have (nouveau). It just didn't make a
meaningful difference in most cases.
Best regards
Thomas
>
> Actually, I just did test matroxfb and pm2fb successfully a few days
> back, and
> they worked. For some smaller issues I've prepared patches, which are on
> hold
> due conflicts with your latest file-move-around- and whitespace-changes
> which are partly
> in drm-misc.
> And I do have some upcoming additional patches for console support.
>
> Helge
--
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)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces
2023-05-11 13:10 ` Geert Uytterhoeven
@ 2023-05-11 16:23 ` Helge Deller
0 siblings, 0 replies; 31+ messages in thread
From: Helge Deller @ 2023-05-11 16:23 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Thomas Zimmermann, Sui Jingfeng, javierm, daniel, vgupta,
chenhuacai, kernel, davem, arnd, sam, linux-fbdev, dri-devel,
linux-arch, linux-snps-arc, linux-kernel, linux-ia64, loongarch,
linux-m68k, sparclinux, linux-arm-kernel, linux-parisc
On 5/11/23 15:10, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Thu, May 11, 2023 at 3:05 PM Helge Deller <deller@gmx.de> wrote:
>> On 5/11/23 09:55, Thomas Zimmermann wrote:
>>> But the work I do within fbdev is mostly for improving DRM.
>>
>> Sure.
>>
>>> For the
>>> other issues in this file, I don't think that matroxfb should even be
>>> around any longer. Fbdev has been deprecated for a long time. But a
>>> small number of drivers are still in use and we still need its
>>> framebuffer console. So someone should either put significant effort
>>> into maintaining fbdev, or it should be phased out. But neither is
>>> happening.
>>
>> You're wrong.
>>
>> You don't mention that for most older machines DRM isn't an acceptable
>> way to go due to it's limitations, e.g. it's low-speed due to missing
>> 2D-acceleration for older cards and and it's incapability to change screen
>> resolution at runtime (just to name two of the bigger limitations here).
>> So, unless we somehow find a good way to move such drivers over to DRM
>> (with a set of minimal 2D acceleration), they are still important.
>
> DRM can change resolution at runtime,
Right, sure.
> just not through the fbdev API.
... and sadly the simpledrm-based drivers neither.
> Or do you mean the resolution of the text console, akin to
> "fbset <mode>"?
yes.
> I have to admit I do not know if there is a command
> line tool to do that...
Helge
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces
2023-05-11 14:27 ` Thomas Zimmermann
@ 2023-05-11 17:02 ` Helge Deller
2023-05-12 7:14 ` Thomas Zimmermann
2023-05-12 10:04 ` Finn Thain
1 sibling, 1 reply; 31+ messages in thread
From: Helge Deller @ 2023-05-11 17:02 UTC (permalink / raw)
To: Thomas Zimmermann, Sui Jingfeng, geert, javierm, daniel, vgupta,
chenhuacai, kernel, davem, arnd, sam
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc
On 5/11/23 16:27, Thomas Zimmermann wrote:
>>> But the work I do within fbdev is mostly for improving DRM.
>>
>> Sure.
>>
>>> For the
>>> other issues in this file, I don't think that matroxfb should even be
>>> around any longer. Fbdev has been deprecated for a long time. But a
>>> small number of drivers are still in use and we still need its
>>> framebuffer console. So someone should either put significant effort
>>> into maintaining fbdev, or it should be phased out. But neither is
>>> happening.
>>
>> You're wrong.
>
> I'm not. I don't claim that these drivers are all broken. But fbdev
> as a whole is bit-rotting and no one attempts to address this. There
> are several recent examples of this:
>
> * I recently send out a 100-patches series to improve parameter
> parsing and avoid memory leaks. That got shot down. I didn't attempt
> to support parameter parsing for module builds.
Your work is appreciated and it wasn't shot down, but it wasn't perfect either.
> * There's been a 15-yrs old bug in fbdev's read/write where they
> return an incorrect value.
?
> * See the other discussion on this patchset on the state of hitfb.
>
> * The fbdev code I recently cleaned up had bugs in how it uses some
> of fbdev's basic building blocks (see the screen_base/screen_buffer
> confusion).
As you said... some (little-used/outdated) drivers may have issues
which of course show up if one starts to clean up, as you do.
On a per-driver basis it can make sense to drop a specific driver.
> * <asm-generic/fb.h> has been in the tree since 2009 and no one
> attempted to include it until now.
>
> None of this is a sign of good maintenance.
> As I've worked on DRM's fbdev emulation a lot, I try to be a good
> kernel citizen and clean up in fbdev as well when I see a problem.> But I'd really like to see most of these drivers being moved into
> staging and deleted soon afterwards. Users will complain about those
> drivers that are really still required. Those might be worth to spend
> effort on.
I'd really like to see a way forward and get the required drivers over to
DRM, e.g. based on your simpledrm driver.
If there is a way to get basic on-screen 2D bitblt and fillrect support,
it would drop the need for most of the fbdev drivers.
The current way of bitblt'ing from a buffer on regular basis istoo slow for such older cards. Even on new hardware in emulators there
is a big slowdown visible.
>> You don't mention that for most older machines DRM isn't an
>> acceptable way to go due to it's limitations, e.g. it's low-speed
>> due to missing 2D-acceleration for older cards and and it's
>> incapability to change screen resolution at runtime (just to name
>> two of the bigger limitations here).
>
> You can change resolution at runtime; just not through fbdev ioctls.
> There's no technical limitation here. No one found any use for this,
> so it's not there.
fbdev drivers would need that when ported to DRM.
>> So, unless we somehow find a good way to move such drivers over to
>> DRM (with a set of minimal 2D acceleration), they are still
>> important.
>
> 2d acceleration is mostly useful for the framebuffer console.
and X11
> You can
> do that with DRM and drivers have (nouveau). It just didn't make a
> meaningful difference in most cases.
if nouveau got it, can't it be done for simpledrm in a generic way too?
>> Actually, I just did test matroxfb and pm2fb successfully a few
>> days back, and they worked. For some smaller issues I've prepared
>> patches, which are on hold due conflicts with your latest
>> file-move-around- and whitespace-changes which are partly in
>> drm-misc. And I do have some upcoming additional patches for
>> console support.
Helge
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces
2023-05-11 17:02 ` Helge Deller
@ 2023-05-12 7:14 ` Thomas Zimmermann
0 siblings, 0 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2023-05-12 7:14 UTC (permalink / raw)
To: Helge Deller, Sui Jingfeng, geert, javierm, daniel, vgupta,
chenhuacai, kernel, davem, arnd, sam
Cc: linux-fbdev, dri-devel, linux-arch, linux-snps-arc, linux-kernel,
linux-ia64, loongarch, linux-m68k, sparclinux, linux-arm-kernel,
linux-parisc
[-- Attachment #1.1: Type: text/plain, Size: 4861 bytes --]
Hi
Am 11.05.23 um 19:02 schrieb Helge Deller:
> On 5/11/23 16:27, Thomas Zimmermann wrote:
>>>> But the work I do within fbdev is mostly for improving DRM.
>>>
>>> Sure.
>>>
>>>> For the
>>>> other issues in this file, I don't think that matroxfb should even be
>>>> around any longer. Fbdev has been deprecated for a long time. But a
>>>> small number of drivers are still in use and we still need its
>>>> framebuffer console. So someone should either put significant effort
>>>> into maintaining fbdev, or it should be phased out. But neither is
>>>> happening.
>>>
>>> You're wrong.
>>
>
>> I'm not. I don't claim that these drivers are all broken. But fbdev
>> as a whole is bit-rotting and no one attempts to address this. There
>> are several recent examples of this:
>>
>> * I recently send out a 100-patches series to improve parameter
>> parsing and avoid memory leaks. That got shot down. I didn't attempt
>> to support parameter parsing for module builds.
>
> Your work is appreciated and it wasn't shot down, but it wasn't perfect
> either.
>
>> * There's been a 15-yrs old bug in fbdev's read/write where they
>> return an incorrect value.
>
> ?
This one:
https://patchwork.freedesktop.org/patch/534668/?series=116931&rev=2
>
>> * See the other discussion on this patchset on the state of hitfb.
>>
>> * The fbdev code I recently cleaned up had bugs in how it uses some
>> of fbdev's basic building blocks (see the screen_base/screen_buffer
>> confusion).
>
> As you said... some (little-used/outdated) drivers may have issues
> which of course show up if one starts to clean up, as you do.
> On a per-driver basis it can make sense to drop a specific driver.
>
>> * <asm-generic/fb.h> has been in the tree since 2009 and no one
>> attempted to include it until now.
>>
>> None of this is a sign of good maintenance.
Let me add that I'm not pointing fingers at anyone. It's just the
current status AFAICT.
>> As I've worked on DRM's fbdev emulation a lot, I try to be a good
>> kernel citizen and clean up in fbdev as well when I see a problem.>
>> But I'd really like to see most of these drivers being moved into
>> staging and deleted soon afterwards. Users will complain about those
>> drivers that are really still required. Those might be worth to spend
>> effort on.
>
> I'd really like to see a way forward and get the required drivers over to
> DRM, e.g. based on your simpledrm driver.
> If there is a way to get basic on-screen 2D bitblt and fillrect support,
> it would drop the need for most of the fbdev drivers.
> The current way of bitblt'ing from a buffer on regular basis istoo slow
> for such older cards. Even on new hardware in emulators there
> is a big slowdown visible.
I'd be happy to try to drop the unused/obsolete/broken drivers. For the
rest, I'd designate fbdev as a graphics console for systems without text
mode. I think that was the original intention.
>
>>> You don't mention that for most older machines DRM isn't an
>>> acceptable way to go due to it's limitations, e.g. it's low-speed
>>> due to missing 2D-acceleration for older cards and and it's
>>> incapability to change screen resolution at runtime (just to name
>>> two of the bigger limitations here).
>>
>> You can change resolution at runtime; just not through fbdev ioctls.
>> There's no technical limitation here. No one found any use for this,
>> so it's not there.
>
> fbdev drivers would need that when ported to DRM.
Why? Userspace would then simply use DRM ioctls to set the display mode.
>>> So, unless we somehow find a good way to move such drivers over to
>>> DRM (with a set of minimal 2D acceleration), they are still
>>> important.
>>
>> 2d acceleration is mostly useful for the framebuffer console.
>
> and X11
>
>> You can
>> do that with DRM and drivers have (nouveau). It just didn't make a
>> meaningful difference in most cases.
>
> if nouveau got it, can't it be done for simpledrm in a generic way too?
Probably no, as it depends on the hardware features. The DRM driver
would have to implement it's own fbdev support. That's not too
complicated, but still not portable among drivers.
>
>>> Actually, I just did test matroxfb and pm2fb successfully a few
>>> days back, and they worked. For some smaller issues I've prepared
>>> patches, which are on hold due conflicts with your latest
>>> file-move-around- and whitespace-changes which are partly in
>>> drm-misc. And I do have some upcoming additional patches for
>>> console support.
>
> Helge
--
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)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces
2023-05-11 14:27 ` Thomas Zimmermann
2023-05-11 17:02 ` Helge Deller
@ 2023-05-12 10:04 ` Finn Thain
1 sibling, 0 replies; 31+ messages in thread
From: Finn Thain @ 2023-05-12 10:04 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Helge Deller, Sui Jingfeng, geert, javierm, daniel, vgupta,
chenhuacai, kernel, davem, arnd, sam, linux-fbdev, dri-devel,
linux-arch, linux-snps-arc, linux-kernel, linux-ia64, loongarch,
linux-m68k, sparclinux, linux-arm-kernel, linux-parisc
On Thu, 11 May 2023, Thomas Zimmermann wrote:
> But I'd really like to see most of these drivers being moved into
> staging and deleted soon afterwards. Users will complain about those
> drivers that are really still required. Those might be worth to spend
> effort on.
>
That strategy is not going to find out what functionality is required.
Instead it will find out which beneficiaries are capable of overcoming all
of the hurdles to reverting deletion:
- Find out how to report a regression correctly.
- Gather all the necessary information.
- Obtain buy-in from a sympathetic developer.
- Build a patched kernel, test it and provide the results. (And possibly
repeat the same until neglected code becomes accepted.)
- Wait for the relevant distro to release the relevant kernel update.
Developers tend to overlook the burden of process because it's ostensibly
done to raise code quality. But it seems to me that affected users are
more likely to seek a workaround than undertake the process.
So deletion doesn't discover end-user requirements. What it does is
advertise a vacancy for an unpaid adoptive maintainer, somehow presumed to
be found amongst a very well remunerated and very small pool of talent.
The way I look at it, the maintainence of old code is the price of a
so-called "right to repair". But there ain't no free lunch and if we want
that right we should seek ways to reduce that price. For example, by
making a larger talent pool more effective, by re-using more code and by
improving the tooling and automation.
The code I'd delete first wouldn't be a small amount of old code in need
of sponsorship. Or even the most buggy code. The first to go would be that
code which will never find an actual end user because some portion of the
code required to actually use certain platforms was never mainlined by the
vendor -- and never will be without some push-back.
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2023-05-12 10:05 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-10 11:05 [PATCH v6 0/6] fbdev: Move framebuffer I/O helpers to <asm/fb.h> Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces Thomas Zimmermann
2023-05-10 18:20 ` Sui Jingfeng
2023-05-11 7:55 ` Thomas Zimmermann
2023-05-11 9:24 ` Sui Jingfeng
2023-05-11 13:05 ` Helge Deller
2023-05-11 13:10 ` Geert Uytterhoeven
2023-05-11 16:23 ` Helge Deller
2023-05-11 14:27 ` Thomas Zimmermann
2023-05-11 17:02 ` Helge Deller
2023-05-12 7:14 ` Thomas Zimmermann
2023-05-12 10:04 ` Finn Thain
2023-05-10 11:05 ` [PATCH v6 2/6] ipu-v3: Include <linux/io.h> Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 3/6] fbdev: Include <linux/io.h> in various drivers Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 4/6] fbdev: Include <linux/fb.h> instead of <asm/fb.h> Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h> Thomas Zimmermann
2023-05-10 12:34 ` Geert Uytterhoeven
2023-05-10 14:20 ` Thomas Zimmermann
2023-05-10 14:34 ` Geert Uytterhoeven
2023-05-10 15:11 ` Thomas Zimmermann
2023-05-10 14:03 ` kernel test robot
2023-05-10 14:15 ` Arnd Bergmann
2023-05-10 14:27 ` Thomas Zimmermann
2023-05-10 15:54 ` Arnd Bergmann
2023-05-11 10:53 ` Thomas Zimmermann
2023-05-11 12:35 ` Geert Uytterhoeven
2023-05-11 12:48 ` Arnd Bergmann
2023-05-11 13:22 ` Artur Rojek
2023-05-11 13:40 ` Arnd Bergmann
2023-05-11 13:12 ` Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 6/6] fbdev: Rename fb_mem*() helpers Thomas Zimmermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).