Implement framebuffer I/O helpers, such as fb_read*() and fb_write*(),
in the architecture's <asm/fb.h> header file or the generic one.
The common case has been the use of regular I/O functions, such as
__raw_readb() or memset_io(). A few architectures used plain system-
memory reads and writes. Sparc used helpers for its SBus.
The architectures that used special cases provide the same code in
their __raw_*() I/O helpers. So the patch replaces this code with the
__raw_*() functions and moves it to <asm-generic/fb.h> for all
architectures.
v6:
* fix fb_readq()/fb_writeq() on 64-bit mips (kernel test robot)
v5:
* include <linux/io.h> in <asm-generic/fb>; fix s390 build
v4:
* ia64, loongarch, sparc64: add fb_mem*() to arch headers
to keep current semantics (Arnd)
v3:
* implement all architectures with generic helpers
* support reordering and native byte order (Geert, Arnd)
Signed-off-by: Thomas Zimmermann <[email protected]>
Tested-by: Sui Jingfeng <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>
add mips fb_q()
---
arch/ia64/include/asm/fb.h | 20 +++++++
arch/loongarch/include/asm/fb.h | 21 +++++++
arch/mips/include/asm/fb.h | 22 +++++++
arch/sparc/include/asm/fb.h | 20 +++++++
include/asm-generic/fb.h | 102 ++++++++++++++++++++++++++++++++
include/linux/fb.h | 53 -----------------
6 files changed, 185 insertions(+), 53 deletions(-)
diff --git a/arch/ia64/include/asm/fb.h b/arch/ia64/include/asm/fb.h
index 0208f64a0da0..bcf982043a5c 100644
--- a/arch/ia64/include/asm/fb.h
+++ b/arch/ia64/include/asm/fb.h
@@ -2,7 +2,9 @@
#ifndef _ASM_FB_H_
#define _ASM_FB_H_
+#include <linux/compiler.h>
#include <linux/efi.h>
+#include <linux/string.h>
#include <asm/page.h>
@@ -18,6 +20,24 @@ static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
}
#define fb_pgprotect fb_pgprotect
+static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+{
+ memcpy(to, (void __force *)from, n);
+}
+#define fb_memcpy_fromfb fb_memcpy_fromfb
+
+static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+{
+ memcpy((void __force *)to, from, n);
+}
+#define fb_memcpy_tofb fb_memcpy_tofb
+
+static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+{
+ memset((void __force *)addr, c, n);
+}
+#define fb_memset fb_memset
+
#include <asm-generic/fb.h>
#endif /* _ASM_FB_H_ */
diff --git a/arch/loongarch/include/asm/fb.h b/arch/loongarch/include/asm/fb.h
index ff82f20685c8..c6fc7ef374a4 100644
--- a/arch/loongarch/include/asm/fb.h
+++ b/arch/loongarch/include/asm/fb.h
@@ -5,6 +5,27 @@
#ifndef _ASM_FB_H_
#define _ASM_FB_H_
+#include <linux/compiler.h>
+#include <linux/string.h>
+
+static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+{
+ memcpy(to, (void __force *)from, n);
+}
+#define fb_memcpy_fromfb fb_memcpy_fromfb
+
+static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+{
+ memcpy((void __force *)to, from, n);
+}
+#define fb_memcpy_tofb fb_memcpy_tofb
+
+static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+{
+ memset((void __force *)addr, c, n);
+}
+#define fb_memset fb_memset
+
#include <asm-generic/fb.h>
#endif /* _ASM_FB_H_ */
diff --git a/arch/mips/include/asm/fb.h b/arch/mips/include/asm/fb.h
index 6bda0a81d8ca..18b7226403ba 100644
--- a/arch/mips/include/asm/fb.h
+++ b/arch/mips/include/asm/fb.h
@@ -12,6 +12,28 @@ static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
}
#define fb_pgprotect fb_pgprotect
+/*
+ * MIPS doesn't define __raw_ I/O macros, so the helpers
+ * in <asm-generic/fb.h> don't generate fb_readq() and
+ * fb_write(). We have to provide them here.
+ *
+ * TODO: Convert MIPS to generic I/O. The helpers below can
+ * then be removed.
+ */
+#ifdef CONFIG_64BIT
+static inline u64 fb_readq(const volatile void __iomem *addr)
+{
+ return __raw_readq(addr);
+}
+#define fb_readq fb_readq
+
+static inline void fb_writeq(u64 b, volatile void __iomem *addr)
+{
+ __raw_writeq(b, addr);
+}
+#define fb_writeq fb_writeq
+#endif
+
#include <asm-generic/fb.h>
#endif /* _ASM_FB_H_ */
diff --git a/arch/sparc/include/asm/fb.h b/arch/sparc/include/asm/fb.h
index 689ee5c60054..077da91aeba1 100644
--- a/arch/sparc/include/asm/fb.h
+++ b/arch/sparc/include/asm/fb.h
@@ -2,6 +2,8 @@
#ifndef _SPARC_FB_H_
#define _SPARC_FB_H_
+#include <linux/io.h>
+
struct fb_info;
struct file;
struct vm_area_struct;
@@ -16,6 +18,24 @@ static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
int fb_is_primary_device(struct fb_info *info);
#define fb_is_primary_device fb_is_primary_device
+static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+{
+ sbus_memcpy_fromio(to, from, n);
+}
+#define fb_memcpy_fromfb fb_memcpy_fromfb
+
+static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+{
+ sbus_memcpy_toio(to, from, n);
+}
+#define fb_memcpy_tofb fb_memcpy_tofb
+
+static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+{
+ sbus_memset_io(addr, c, n);
+}
+#define fb_memset fb_memset
+
#include <asm-generic/fb.h>
#endif /* _SPARC_FB_H_ */
diff --git a/include/asm-generic/fb.h b/include/asm-generic/fb.h
index c8af99f5a535..0540eccdbeca 100644
--- a/include/asm-generic/fb.h
+++ b/include/asm-generic/fb.h
@@ -7,6 +7,7 @@
* Only include this header file from your architecture's <asm/fb.h>.
*/
+#include <linux/io.h>
#include <linux/mm_types.h>
#include <linux/pgtable.h>
@@ -30,4 +31,105 @@ static inline int fb_is_primary_device(struct fb_info *info)
}
#endif
+/*
+ * I/O helpers for the framebuffer. Prefer these functions over their
+ * regular counterparts. The regular I/O functions provide in-order
+ * access and swap bytes to/from little-endian ordering. Neither is
+ * required for framebuffers. Instead, the helpers read and write
+ * raw framebuffer data. Independent operations can be reordered for
+ * improved performance.
+ */
+
+#ifndef fb_readb
+static inline u8 fb_readb(const volatile void __iomem *addr)
+{
+ return __raw_readb(addr);
+}
+#define fb_readb fb_readb
+#endif
+
+#ifndef fb_readw
+static inline u16 fb_readw(const volatile void __iomem *addr)
+{
+ return __raw_readw(addr);
+}
+#define fb_readw fb_readw
+#endif
+
+#ifndef fb_readl
+static inline u32 fb_readl(const volatile void __iomem *addr)
+{
+ return __raw_readl(addr);
+}
+#define fb_readl fb_readl
+#endif
+
+#ifndef fb_readq
+#if defined(__raw_readq)
+static inline u64 fb_readq(const volatile void __iomem *addr)
+{
+ return __raw_readq(addr);
+}
+#define fb_readq fb_readq
+#endif
+#endif
+
+#ifndef fb_writeb
+static inline void fb_writeb(u8 b, volatile void __iomem *addr)
+{
+ __raw_writeb(b, addr);
+}
+#define fb_writeb fb_writeb
+#endif
+
+#ifndef fb_writew
+static inline void fb_writew(u16 b, volatile void __iomem *addr)
+{
+ __raw_writew(b, addr);
+}
+#define fb_writew fb_writew
+#endif
+
+#ifndef fb_writel
+static inline void fb_writel(u32 b, volatile void __iomem *addr)
+{
+ __raw_writel(b, addr);
+}
+#define fb_writel fb_writel
+#endif
+
+#ifndef fb_writeq
+#if defined(__raw_writeq)
+static inline void fb_writeq(u64 b, volatile void __iomem *addr)
+{
+ __raw_writeq(b, addr);
+}
+#define fb_writeq fb_writeq
+#endif
+#endif
+
+#ifndef fb_memcpy_fromfb
+static inline void fb_memcpy_fromfb(void *to, const volatile void __iomem *from, size_t n)
+{
+ memcpy_fromio(to, from, n);
+}
+#define fb_memcpy_fromfb fb_memcpy_fromfb
+#endif
+
+#ifndef fb_memcpy_tofb
+static inline void fb_memcpy_tofb(volatile void __iomem *to, const void *from, size_t n)
+{
+ memcpy_toio(to, from, n);
+}
+#define fb_memcpy_tofb fb_memcpy_tofb
+#endif
+
+#ifndef fb_memset
+static inline void fb_memset(volatile void __iomem *addr, int c, size_t n)
+{
+ memset_io(addr, c, n);
+}
+#define fb_memset fb_memset
+#endif
+
#endif /* __ASM_GENERIC_FB_H_ */
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 4b4d9a5d200a..2cf8efcb9e32 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -17,7 +17,6 @@
#include <linux/slab.h>
#include <asm/fb.h>
-#include <asm/io.h>
struct vm_area_struct;
struct fb_info;
@@ -513,58 +512,6 @@ struct fb_info {
*/
#define STUPID_ACCELF_TEXT_SHIT
-// This will go away
-#if defined(__sparc__)
-
-/* We map all of our framebuffers such that big-endian accesses
- * are what we want, so the following is sufficient.
- */
-
-// This will go away
-#define fb_readb sbus_readb
-#define fb_readw sbus_readw
-#define fb_readl sbus_readl
-#define fb_readq sbus_readq
-#define fb_writeb sbus_writeb
-#define fb_writew sbus_writew
-#define fb_writel sbus_writel
-#define fb_writeq sbus_writeq
-#define fb_memset sbus_memset_io
-#define fb_memcpy_fromfb sbus_memcpy_fromio
-#define fb_memcpy_tofb sbus_memcpy_toio
-
-#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) || \
- defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || \
- defined(__arm__) || defined(__aarch64__) || defined(__mips__)
-
-#define fb_readb __raw_readb
-#define fb_readw __raw_readw
-#define fb_readl __raw_readl
-#define fb_readq __raw_readq
-#define fb_writeb __raw_writeb
-#define fb_writew __raw_writew
-#define fb_writel __raw_writel
-#define fb_writeq __raw_writeq
-#define fb_memset memset_io
-#define fb_memcpy_fromfb memcpy_fromio
-#define fb_memcpy_tofb memcpy_toio
-
-#else
-
-#define fb_readb(addr) (*(volatile u8 *) (addr))
-#define fb_readw(addr) (*(volatile u16 *) (addr))
-#define fb_readl(addr) (*(volatile u32 *) (addr))
-#define fb_readq(addr) (*(volatile u64 *) (addr))
-#define fb_writeb(b,addr) (*(volatile u8 *) (addr) = (b))
-#define fb_writew(b,addr) (*(volatile u16 *) (addr) = (b))
-#define fb_writel(b,addr) (*(volatile u32 *) (addr) = (b))
-#define fb_writeq(b,addr) (*(volatile u64 *) (addr) = (b))
-#define fb_memset memset
-#define fb_memcpy_fromfb memcpy
-#define fb_memcpy_tofb memcpy
-
-#endif
-
#define FB_LEFT_POS(p, bpp) (fb_be_math(p) ? (32 - (bpp)) : 0)
#define FB_SHIFT_HIGH(p, val, bits) (fb_be_math(p) ? (val) >> (bits) : \
(val) << (bits))
--
2.40.1
Hi Thomas,
On Wed, May 10, 2023 at 1:06 PM Thomas Zimmermann <[email protected]> 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 <[email protected]>
> Tested-by: Sui Jingfeng <[email protected]>
> Reviewed-by: Arnd Bergmann <[email protected]>
>
> 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 -- [email protected]
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
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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
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
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 <[email protected]> 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 <[email protected]>
>> Tested-by: Sui Jingfeng <[email protected]>
>> Reviewed-by: Arnd Bergmann <[email protected]>
>>
>> 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)
Hi Thomas,
On Wed, May 10, 2023 at 4:20 PM Thomas Zimmermann <[email protected]> wrote:
> Am 10.05.23 um 14:34 schrieb Geert Uytterhoeven:
> > On Wed, May 10, 2023 at 1:06 PM Thomas Zimmermann <[email protected]> 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 <[email protected]>
> >> Tested-by: Sui Jingfeng <[email protected]>
> >> Reviewed-by: Arnd Bergmann <[email protected]>
> >> --- 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 -- [email protected]
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
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)
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 <[email protected]> wrote:
>> Am 10.05.23 um 14:34 schrieb Geert Uytterhoeven:
>>> On Wed, May 10, 2023 at 1:06 PM Thomas Zimmermann <[email protected]> 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 <[email protected]>
>>>> Tested-by: Sui Jingfeng <[email protected]>
>>>> Reviewed-by: Arnd Bergmann <[email protected]>
>
>>>> --- 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)
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
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)
Hi Arnd,
CC Artur, who's working on HP Jornada 680.
On Wed, May 10, 2023 at 5:55 PM Arnd Bergmann <[email protected]> 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 -- [email protected]
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
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 <[email protected]> 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
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)
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 <[email protected]> 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
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