* [RFC PATCH] fbdev: maxinefb: fix build issues
@ 2026-08-09 23:48 Randy Dunlap
[not found] ` <20260810000251.55AD61F000E9@smtp.kernel.org>
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Randy Dunlap @ 2026-08-09 23:48 UTC (permalink / raw)
To: linux-kernel
Cc: Randy Dunlap, Maciej W. Rozycki, Helge Deller, linux-fbdev,
dri-devel, Thomas Bogendoerfer, linux-mips
This is all ancient source code from the beginning of git time.
Now builds cleanly on 32-bit and 64-bit MACH_DECSTATION.
Fixes these build errors/warnings:
../drivers/video/fbdev/maxinefb.c:64:6: warning: no previous prototype for 'maxinefb_ims332_write_register' [-Wmissing-prototypes]
64 | void maxinefb_ims332_write_register(int regno, register unsigned int val)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../drivers/video/fbdev/maxinefb.c:34:
../drivers/video/fbdev/maxinefb.c: In function 'maxinefb_ims332_write_register':
../include/video/maxinefb.h:16:41: error: implicit declaration of function 'KSEG1ADDR'; did you mean 'CKSEG1ADDR'? [-Wimplicit-function-declaration]
16 | #define MAXINEFB_IMS332_ADDRESS KSEG1ADDR(0x1c140000)
| ^~~~~~~~~
../drivers/video/fbdev/maxinefb.c:66:49: note: in expansion of macro 'MAXINEFB_IMS332_ADDRESS'
66 | register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
| ^~~~~~~~~~~~~~~~~~~~~~~
../drivers/video/fbdev/maxinefb.c:66:40: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
66 | register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
| ^
../drivers/video/fbdev/maxinefb.c: At top level:
../drivers/video/fbdev/maxinefb.c:74:14: warning: no previous prototype for 'maxinefb_ims332_read_register' [-Wmissing-prototypes]
74 | unsigned int maxinefb_ims332_read_register(int regno)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/video/fbdev/maxinefb.c: At top level:
../drivers/video/fbdev/maxinefb.c:74:21: warning: 'maxinefb_ims332_read_register' defined but not used [-Wunused-function]
74 | static unsigned int maxinefb_ims332_read_register(int regno)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/video/fbdev/maxinefb.c: In function 'maxinefb_ims332_read_register':
../drivers/video/fbdev/maxinefb.c:76:40: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
76 | register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
| ^
../drivers/video/fbdev/maxinefb.c: At top level:
../drivers/video/fbdev/maxinefb.c:114:12: warning: no previous prototype for 'maxinefb_init' [-Wmissing-prototypes]
114 | int __init maxinefb_init(void)
| ^~~~~~~~~~~~~
In file included from ../drivers/video/fbdev/maxinefb.c:34:
../drivers/video/fbdev/maxinefb.c: In function 'maxinefb_init':
../include/video/maxinefb.h:22:41: error: implicit declaration of function 'KSEG1ADDR'; did you mean 'CKSEG1ADDR'? [-Wimplicit-function-declaration]
22 | #define DS5000_xx_ONBOARD_FBMEM_START KSEG1ADDR(0x0a000000)
| ^~~~~~~~~
../drivers/video/fbdev/maxinefb.c:119:20: note: in expansion of macro 'DS5000_xx_ONBOARD_FBMEM_START'
119 | fb_start = DS5000_xx_ONBOARD_FBMEM_START;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Suggested-by: "Maciej W. Rozycki" <macro@orcam.me.uk> # for s/KSEG1ADDR/CKSEG1ADDR/
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-fbdev@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: linux-mips@vger.kernel.org
drivers/video/fbdev/maxinefb.c | 17 ++---------------
include/video/maxinefb.h | 4 ++--
2 files changed, 4 insertions(+), 17 deletions(-)
--- linux-next-20260807.orig/drivers/video/fbdev/maxinefb.c
+++ linux-next-20260807/drivers/video/fbdev/maxinefb.c
@@ -61,7 +61,7 @@ static struct fb_fix_screeninfo maxinefb
/* Handle the funny Inmos RamDAC/video controller ... */
-void maxinefb_ims332_write_register(int regno, register unsigned int val)
+static void maxinefb_ims332_write_register(int regno, register unsigned int val)
{
register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
unsigned char *wptr;
@@ -71,19 +71,6 @@ void maxinefb_ims332_write_register(int
*((volatile unsigned short *) (wptr)) = val;
}
-unsigned int maxinefb_ims332_read_register(int regno)
-{
- register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
- unsigned char *rptr;
- register unsigned int j, k;
-
- rptr = regs + 0x80000 + (regno << 4);
- j = *((volatile unsigned short *) rptr);
- k = *((volatile unsigned short *) regs);
-
- return (j & 0xffff) | ((k & 0xff00) << 8);
-}
-
/* Set the palette */
static int maxinefb_setcolreg(unsigned regno, unsigned red, unsigned green,
unsigned blue, unsigned transp, struct fb_info *info)
@@ -111,7 +98,7 @@ static const struct fb_ops maxinefb_ops
.fb_setcolreg = maxinefb_setcolreg,
};
-int __init maxinefb_init(void)
+static int __init maxinefb_init(void)
{
unsigned long fboff;
unsigned long fb_start;
--- linux-next-20260807.orig/include/video/maxinefb.h
+++ linux-next-20260807/include/video/maxinefb.h
@@ -13,13 +13,13 @@
/*
* IMS332 video controller register base address
*/
-#define MAXINEFB_IMS332_ADDRESS KSEG1ADDR(0x1c140000)
+#define MAXINEFB_IMS332_ADDRESS CKSEG1ADDR(0x1c140000)
/*
* Begin of DECstation 5000/xx onboard framebuffer memory, default resolution
* is 1024x768x8
*/
-#define DS5000_xx_ONBOARD_FBMEM_START KSEG1ADDR(0x0a000000)
+#define DS5000_xx_ONBOARD_FBMEM_START CKSEG1ADDR(0x0a000000)
/*
* The IMS 332 video controller used in the DECstation 5000/xx series
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] fbdev: maxinefb: fix build issues
[not found] ` <20260810000251.55AD61F000E9@smtp.kernel.org>
@ 2026-08-10 0:27 ` Randy Dunlap
2026-08-14 2:46 ` Randy Dunlap
0 siblings, 1 reply; 11+ messages in thread
From: Randy Dunlap @ 2026-08-10 0:27 UTC (permalink / raw)
To: sashiko-reviews
Cc: dri-devel, Maciej W. Rozycki, Helge Deller, linux-fbdev,
linux-mips, Thomas Bogendoerfer
[adding Cc:s]
On 8/9/26 5:02 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Initialization function returns positive value `1` on failure, breaking module load semantics and causing subsequent `registered_fb` array corruption on unload.
> - [High] `maxinefb_fix.smem_start` is initialized with a virtual address (`CKSEG1ADDR`) instead of the hardware physical address, breaking user-space `mmap()`.
> --
>
> --- Patch [1]: [RFC PATCH] fbdev: maxinefb: fix build issues ---
> commit e391418fa2f21f8c420eccb513c031674ef184c6
> Author: Randy Dunlap <rdunlap@infradead.org>
>
> fbdev: maxinefb: fix build issues
>
> This commit fixes build errors and warnings for the maxinefb driver on
> 32-bit and 64-bit MACH_DECSTATION. It updates KSEG1ADDR to CKSEG1ADDR and
> adds the static keyword to missing prototypes.
>
>> diff --git a/drivers/video/fbdev/maxinefb.c b/drivers/video/fbdev/maxinefb.c
>> index 52528eb4dfb41..8aa1389e2d4c8 100644
>> --- a/drivers/video/fbdev/maxinefb.c
>> +++ b/drivers/video/fbdev/maxinefb.c
> [ ... ]
>> @@ -111,7 +98,7 @@ static const struct fb_ops maxinefb_ops = {
>> .fb_setcolreg = maxinefb_setcolreg,
>> };
>>
>> -int __init maxinefb_init(void)
>> +static int __init maxinefb_init(void)
>
> [Severity: High]
> This is a pre-existing issue, but does returning 1 on failure in this function
> cause issues with module load semantics?
>
> In maxinefb_init():
>
> if (register_framebuffer(&fb_info) < 0)
> return 1;
>
> If register_framebuffer() fails, returning a positive integer causes the kernel
> to treat the module as successfully loaded because the module init framework
> expects negative error codes for failure.
Ack. I'll change that.
> Later, when the module is unloaded, maxinefb_exit() calls
> unregister_framebuffer() on the unregistered fb_info. Since it was never
> successfully registered, its node field is 0.
>
> Does this cause the generic do_unregister_framebuffer() to blindly execute
> registered_fb[0] = NULL, corrupting the kernel's framebuffer state by deleting
> the reference to whatever valid framebuffer occupied node 0?
>
>> diff --git a/include/video/maxinefb.h b/include/video/maxinefb.h
>> index 6aeb4acca2bdb..2b66b32f355cf 100644
>> --- a/include/video/maxinefb.h
>> +++ b/include/video/maxinefb.h
> [ ... ]
>> @@ -13,13 +13,13 @@
>> /*
>> * IMS332 video controller register base address
>> */
>> -#define MAXINEFB_IMS332_ADDRESS KSEG1ADDR(0x1c140000)
>> +#define MAXINEFB_IMS332_ADDRESS CKSEG1ADDR(0x1c140000)
>>
>> /*
>> * Begin of DECstation 5000/xx onboard framebuffer memory, default resolution
>> * is 1024x768x8
>> */
>> -#define DS5000_xx_ONBOARD_FBMEM_START KSEG1ADDR(0x0a000000)
>> +#define DS5000_xx_ONBOARD_FBMEM_START CKSEG1ADDR(0x0a000000)
Maciej or anyone:
can you comment of this, please?
> [Severity: High]
> This is a pre-existing issue, but does assigning a virtual address to
> maxinefb_fix.smem_start break user-space mmap()?
>
> In maxinefb_init():
>
> fb_start = DS5000_xx_ONBOARD_FBMEM_START;
> ...
> maxinefb_fix.smem_start = fb_start;
>
> The smem_start field is passed to vm_iomap_memory() by the generic framebuffer
> mmap handler (fb_mmap()). Because vm_iomap_memory() strictly expects a physical
> address, does passing a KSEG1 virtual address set up user-space page tables
> pointing to an invalid physical address?
>
> This could trigger an invalid physical memory access and a kernel panic when
> user-space accesses the mapped memory.
>
--
~Randy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] fbdev: maxinefb: fix build issues
2026-08-09 23:48 [RFC PATCH] fbdev: maxinefb: fix build issues Randy Dunlap
[not found] ` <20260810000251.55AD61F000E9@smtp.kernel.org>
@ 2026-08-10 3:24 ` Philippe Mathieu-Daudé
2026-08-10 3:29 ` Randy Dunlap
2026-08-13 16:11 ` Helge Deller
2026-08-13 17:31 ` Maciej W. Rozycki
3 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-08-10 3:24 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel
Cc: Maciej W. Rozycki, Helge Deller, linux-fbdev, dri-devel,
Thomas Bogendoerfer, linux-mips
Hi Randy,
On 10/8/26 01:48, Randy Dunlap wrote:
> This is all ancient source code from the beginning of git time.
> Now builds cleanly on 32-bit and 64-bit MACH_DECSTATION.
>
> Fixes these build errors/warnings:
>
> ../drivers/video/fbdev/maxinefb.c:64:6: warning: no previous prototype for 'maxinefb_ims332_write_register' [-Wmissing-prototypes]
> 64 | void maxinefb_ims332_write_register(int regno, register unsigned int val)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> In file included from ../drivers/video/fbdev/maxinefb.c:34:
> ../drivers/video/fbdev/maxinefb.c: In function 'maxinefb_ims332_write_register':
> ../include/video/maxinefb.h:16:41: error: implicit declaration of function 'KSEG1ADDR'; did you mean 'CKSEG1ADDR'? [-Wimplicit-function-declaration]
> 16 | #define MAXINEFB_IMS332_ADDRESS KSEG1ADDR(0x1c140000)
> | ^~~~~~~~~
> ../drivers/video/fbdev/maxinefb.c:66:49: note: in expansion of macro 'MAXINEFB_IMS332_ADDRESS'
> 66 | register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
> | ^~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/video/fbdev/maxinefb.c:66:40: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> 66 | register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
> | ^
> ../drivers/video/fbdev/maxinefb.c: At top level:
> ../drivers/video/fbdev/maxinefb.c:74:14: warning: no previous prototype for 'maxinefb_ims332_read_register' [-Wmissing-prototypes]
> 74 | unsigned int maxinefb_ims332_read_register(int regno)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/video/fbdev/maxinefb.c: At top level:
> ../drivers/video/fbdev/maxinefb.c:74:21: warning: 'maxinefb_ims332_read_register' defined but not used [-Wunused-function]
> 74 | static unsigned int maxinefb_ims332_read_register(int regno)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/video/fbdev/maxinefb.c: In function 'maxinefb_ims332_read_register':
> ../drivers/video/fbdev/maxinefb.c:76:40: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> 76 | register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
> | ^
> ../drivers/video/fbdev/maxinefb.c: At top level:
> ../drivers/video/fbdev/maxinefb.c:114:12: warning: no previous prototype for 'maxinefb_init' [-Wmissing-prototypes]
> 114 | int __init maxinefb_init(void)
> | ^~~~~~~~~~~~~
> In file included from ../drivers/video/fbdev/maxinefb.c:34:
> ../drivers/video/fbdev/maxinefb.c: In function 'maxinefb_init':
> ../include/video/maxinefb.h:22:41: error: implicit declaration of function 'KSEG1ADDR'; did you mean 'CKSEG1ADDR'? [-Wimplicit-function-declaration]
> 22 | #define DS5000_xx_ONBOARD_FBMEM_START KSEG1ADDR(0x0a000000)
> | ^~~~~~~~~
> ../drivers/video/fbdev/maxinefb.c:119:20: note: in expansion of macro 'DS5000_xx_ONBOARD_FBMEM_START'
> 119 | fb_start = DS5000_xx_ONBOARD_FBMEM_START;
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Suggested-by: "Maciej W. Rozycki" <macro@orcam.me.uk> # for s/KSEG1ADDR/CKSEG1ADDR/
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
> Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>
> Cc: Helge Deller <deller@gmx.de>
> Cc: linux-fbdev@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: linux-mips@vger.kernel.org
>
> drivers/video/fbdev/maxinefb.c | 17 ++---------------
> include/video/maxinefb.h | 4 ++--
> 2 files changed, 4 insertions(+), 17 deletions(-)
>
> --- linux-next-20260807.orig/drivers/video/fbdev/maxinefb.c
> +++ linux-next-20260807/drivers/video/fbdev/maxinefb.c
> @@ -61,7 +61,7 @@ static struct fb_fix_screeninfo maxinefb
>
> /* Handle the funny Inmos RamDAC/video controller ... */
>
> -void maxinefb_ims332_write_register(int regno, register unsigned int val)
> +static void maxinefb_ims332_write_register(int regno, register unsigned int val)
> {
> register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
> unsigned char *wptr;
> @@ -71,19 +71,6 @@ void maxinefb_ims332_write_register(int
> *((volatile unsigned short *) (wptr)) = val;
> }
>
> -unsigned int maxinefb_ims332_read_register(int regno)
> -{
> - register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
> - unsigned char *rptr;
> - register unsigned int j, k;
> -
> - rptr = regs + 0x80000 + (regno << 4);
> - j = *((volatile unsigned short *) rptr);
> - k = *((volatile unsigned short *) regs);
> -
> - return (j & 0xffff) | ((k & 0xff00) << 8);
> -}
> -
> /* Set the palette */
> static int maxinefb_setcolreg(unsigned regno, unsigned red, unsigned green,
> unsigned blue, unsigned transp, struct fb_info *info)
> @@ -111,7 +98,7 @@ static const struct fb_ops maxinefb_ops
> .fb_setcolreg = maxinefb_setcolreg,
> };
>
> -int __init maxinefb_init(void)
> +static int __init maxinefb_init(void)
> {
> unsigned long fboff;
> unsigned long fb_start;
> --- linux-next-20260807.orig/include/video/maxinefb.h
> +++ linux-next-20260807/include/video/maxinefb.h
> @@ -13,13 +13,13 @@
> /*
> * IMS332 video controller register base address
> */
> -#define MAXINEFB_IMS332_ADDRESS KSEG1ADDR(0x1c140000)
> +#define MAXINEFB_IMS332_ADDRESS CKSEG1ADDR(0x1c140000)
>
> /*
> * Begin of DECstation 5000/xx onboard framebuffer memory, default resolution
> * is 1024x768x8
> */
> -#define DS5000_xx_ONBOARD_FBMEM_START KSEG1ADDR(0x0a000000)
> +#define DS5000_xx_ONBOARD_FBMEM_START CKSEG1ADDR(0x0a000000)
The change looks correct, but clearly this has never been tested
on a 64-bit DECstation, so I'm not sure having this code being
now built is an improvement.
What about:
-- >8 --
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index ac9ac4287c6a..ff17afe6b8b3 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1436,7 +1436,7 @@ config FB_PMAGB_B
config FB_MAXINE
bool "Maxine (Personal DECstation) onboard framebuffer support"
- depends on (FB = y) && MACH_DECSTATION
+ depends on (FB = y) && MACH_DECSTATION && 32BIT
select FB_IOMEM_HELPERS
help
Support for the onboard framebuffer (1024x768x8) in the Personal
---
?
Regards,
Phil.
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] fbdev: maxinefb: fix build issues
2026-08-10 3:24 ` Philippe Mathieu-Daudé
@ 2026-08-10 3:29 ` Randy Dunlap
2026-08-13 18:14 ` Maciej W. Rozycki
0 siblings, 1 reply; 11+ messages in thread
From: Randy Dunlap @ 2026-08-10 3:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, linux-kernel
Cc: Maciej W. Rozycki, Helge Deller, linux-fbdev, dri-devel,
Thomas Bogendoerfer, linux-mips
Hi,
On 8/9/26 8:24 PM, Philippe Mathieu-Daudé wrote:
> Hi Randy,
>
> On 10/8/26 01:48, Randy Dunlap wrote:
>> This is all ancient source code from the beginning of git time.
>> Now builds cleanly on 32-bit and 64-bit MACH_DECSTATION.
>>
>> Fixes these build errors/warnings:
>>
>> ../drivers/video/fbdev/maxinefb.c:64:6: warning: no previous prototype for 'maxinefb_ims332_write_register' [-Wmissing-prototypes]
>> 64 | void maxinefb_ims332_write_register(int regno, register unsigned int val)
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> In file included from ../drivers/video/fbdev/maxinefb.c:34:
>> ../drivers/video/fbdev/maxinefb.c: In function 'maxinefb_ims332_write_register':
>> ../include/video/maxinefb.h:16:41: error: implicit declaration of function 'KSEG1ADDR'; did you mean 'CKSEG1ADDR'? [-Wimplicit-function-declaration]
>> 16 | #define MAXINEFB_IMS332_ADDRESS KSEG1ADDR(0x1c140000)
>> | ^~~~~~~~~
>> ../drivers/video/fbdev/maxinefb.c:66:49: note: in expansion of macro 'MAXINEFB_IMS332_ADDRESS'
>> 66 | register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
>> | ^~~~~~~~~~~~~~~~~~~~~~~
>> ../drivers/video/fbdev/maxinefb.c:66:40: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>> 66 | register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
>> | ^
>> ../drivers/video/fbdev/maxinefb.c: At top level:
>> ../drivers/video/fbdev/maxinefb.c:74:14: warning: no previous prototype for 'maxinefb_ims332_read_register' [-Wmissing-prototypes]
>> 74 | unsigned int maxinefb_ims332_read_register(int regno)
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ../drivers/video/fbdev/maxinefb.c: At top level:
>> ../drivers/video/fbdev/maxinefb.c:74:21: warning: 'maxinefb_ims332_read_register' defined but not used [-Wunused-function]
>> 74 | static unsigned int maxinefb_ims332_read_register(int regno)
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ../drivers/video/fbdev/maxinefb.c: In function 'maxinefb_ims332_read_register':
>> ../drivers/video/fbdev/maxinefb.c:76:40: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>> 76 | register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
>> | ^
>> ../drivers/video/fbdev/maxinefb.c: At top level:
>> ../drivers/video/fbdev/maxinefb.c:114:12: warning: no previous prototype for 'maxinefb_init' [-Wmissing-prototypes]
>> 114 | int __init maxinefb_init(void)
>> | ^~~~~~~~~~~~~
>> In file included from ../drivers/video/fbdev/maxinefb.c:34:
>> ../drivers/video/fbdev/maxinefb.c: In function 'maxinefb_init':
>> ../include/video/maxinefb.h:22:41: error: implicit declaration of function 'KSEG1ADDR'; did you mean 'CKSEG1ADDR'? [-Wimplicit-function-declaration]
>> 22 | #define DS5000_xx_ONBOARD_FBMEM_START KSEG1ADDR(0x0a000000)
>> | ^~~~~~~~~
>> ../drivers/video/fbdev/maxinefb.c:119:20: note: in expansion of macro 'DS5000_xx_ONBOARD_FBMEM_START'
>> 119 | fb_start = DS5000_xx_ONBOARD_FBMEM_START;
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Suggested-by: "Maciej W. Rozycki" <macro@orcam.me.uk> # for s/KSEG1ADDR/CKSEG1ADDR/
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> ---
>> Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>
>> Cc: Helge Deller <deller@gmx.de>
>> Cc: linux-fbdev@vger.kernel.org
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
>> Cc: linux-mips@vger.kernel.org
>>
>> drivers/video/fbdev/maxinefb.c | 17 ++---------------
>> include/video/maxinefb.h | 4 ++--
>> 2 files changed, 4 insertions(+), 17 deletions(-)
>>
>> --- linux-next-20260807.orig/drivers/video/fbdev/maxinefb.c
>> +++ linux-next-20260807/drivers/video/fbdev/maxinefb.c
>> @@ -61,7 +61,7 @@ static struct fb_fix_screeninfo maxinefb
>> /* Handle the funny Inmos RamDAC/video controller ... */
>> -void maxinefb_ims332_write_register(int regno, register unsigned int val)
>> +static void maxinefb_ims332_write_register(int regno, register unsigned int val)
>> {
>> register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
>> unsigned char *wptr;
>> @@ -71,19 +71,6 @@ void maxinefb_ims332_write_register(int
>> *((volatile unsigned short *) (wptr)) = val;
>> }
>> -unsigned int maxinefb_ims332_read_register(int regno)
>> -{
>> - register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
>> - unsigned char *rptr;
>> - register unsigned int j, k;
>> -
>> - rptr = regs + 0x80000 + (regno << 4);
>> - j = *((volatile unsigned short *) rptr);
>> - k = *((volatile unsigned short *) regs);
>> -
>> - return (j & 0xffff) | ((k & 0xff00) << 8);
>> -}
>> -
>> /* Set the palette */
>> static int maxinefb_setcolreg(unsigned regno, unsigned red, unsigned green,
>> unsigned blue, unsigned transp, struct fb_info *info)
>> @@ -111,7 +98,7 @@ static const struct fb_ops maxinefb_ops
>> .fb_setcolreg = maxinefb_setcolreg,
>> };
>> -int __init maxinefb_init(void)
>> +static int __init maxinefb_init(void)
>> {
>> unsigned long fboff;
>> unsigned long fb_start;
>> --- linux-next-20260807.orig/include/video/maxinefb.h
>> +++ linux-next-20260807/include/video/maxinefb.h
>> @@ -13,13 +13,13 @@
>> /*
>> * IMS332 video controller register base address
>> */
>> -#define MAXINEFB_IMS332_ADDRESS KSEG1ADDR(0x1c140000)
>> +#define MAXINEFB_IMS332_ADDRESS CKSEG1ADDR(0x1c140000)
>> /*
>> * Begin of DECstation 5000/xx onboard framebuffer memory, default resolution
>> * is 1024x768x8
>> */
>> -#define DS5000_xx_ONBOARD_FBMEM_START KSEG1ADDR(0x0a000000)
>> +#define DS5000_xx_ONBOARD_FBMEM_START CKSEG1ADDR(0x0a000000)
>
> The change looks correct, but clearly this has never been tested
> on a 64-bit DECstation, so I'm not sure having this code being
> now built is an improvement.
>
> What about:
>
> -- >8 --
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index ac9ac4287c6a..ff17afe6b8b3 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -1436,7 +1436,7 @@ config FB_PMAGB_B
>
> config FB_MAXINE
> bool "Maxine (Personal DECstation) onboard framebuffer support"
> - depends on (FB = y) && MACH_DECSTATION
> + depends on (FB = y) && MACH_DECSTATION && 32BIT
> select FB_IOMEM_HELPERS
> help
> Support for the onboard framebuffer (1024x768x8) in the Personal
> ---
>
> ?
I guess I'm OK with that. I'm even OK with removing the driver completely.
Maciej did say in https://lore.kernel.org/all/alpine.DEB.2.21.2308152255080.8596@angie.orcam.me.uk/
that: "the proper way would be by using `ioremap' really. I'll make a patch."
[That was 15.Aug.2023.]
--
~Randy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] fbdev: maxinefb: fix build issues
2026-08-09 23:48 [RFC PATCH] fbdev: maxinefb: fix build issues Randy Dunlap
[not found] ` <20260810000251.55AD61F000E9@smtp.kernel.org>
2026-08-10 3:24 ` Philippe Mathieu-Daudé
@ 2026-08-13 16:11 ` Helge Deller
2026-08-13 18:16 ` Maciej W. Rozycki
2026-08-13 17:31 ` Maciej W. Rozycki
3 siblings, 1 reply; 11+ messages in thread
From: Helge Deller @ 2026-08-13 16:11 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel
Cc: Maciej W. Rozycki, linux-fbdev, dri-devel, Thomas Bogendoerfer,
linux-mips
On 8/10/26 01:48, Randy Dunlap wrote:
> This is all ancient source code from the beginning of git time.
> Now builds cleanly on 32-bit and 64-bit MACH_DECSTATION.
>
> Fixes these build errors/warnings:
> .....
> drivers/video/fbdev/maxinefb.c | 17 ++---------------
> include/video/maxinefb.h | 4 ++--
> 2 files changed, 4 insertions(+), 17 deletions(-)
I've added the various comments from the mail thread to the
patch and applied the patch as-is for now.
Thanks!
Helge
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] fbdev: maxinefb: fix build issues
2026-08-09 23:48 [RFC PATCH] fbdev: maxinefb: fix build issues Randy Dunlap
` (2 preceding siblings ...)
2026-08-13 16:11 ` Helge Deller
@ 2026-08-13 17:31 ` Maciej W. Rozycki
2026-08-13 21:46 ` Randy Dunlap
3 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2026-08-13 17:31 UTC (permalink / raw)
To: Randy Dunlap
Cc: linux-kernel, Helge Deller, linux-fbdev, dri-devel,
Thomas Bogendoerfer, linux-mips
On Sun, 9 Aug 2026, Randy Dunlap wrote:
> This is all ancient source code from the beginning of git time.
I can dig out any further details you may need from the old LMO git tree.
Eventually it's planned to be put back online, but resources.
> Now builds cleanly on 32-bit and 64-bit MACH_DECSTATION.
Thanks for doing this work.
> Suggested-by: "Maciej W. Rozycki" <macro@orcam.me.uk> # for s/KSEG1ADDR/CKSEG1ADDR/
Eventually this code should be converted to a platform device, so such
hacks won't be necessary. Sadly I declined an offer back in 2009 and I
have been chasing a specimen for a while now. And since it's an onboard
device unlike all the TURBOchannel framebuffers, there's no alternative.
> --- linux-next-20260807.orig/drivers/video/fbdev/maxinefb.c
> +++ linux-next-20260807/drivers/video/fbdev/maxinefb.c
> @@ -71,19 +71,6 @@ void maxinefb_ims332_write_register(int
> *((volatile unsigned short *) (wptr)) = val;
> }
>
> -unsigned int maxinefb_ims332_read_register(int regno)
> -{
> - register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
> - unsigned char *rptr;
> - register unsigned int j, k;
> -
> - rptr = regs + 0x80000 + (regno << 4);
> - j = *((volatile unsigned short *) rptr);
> - k = *((volatile unsigned short *) regs);
> -
> - return (j & 0xffff) | ((k & 0xff00) << 8);
> -}
> -
> /* Set the palette */
> static int maxinefb_setcolreg(unsigned regno, unsigned red, unsigned green,
> unsigned blue, unsigned transp, struct fb_info *info)
It might be worth mentioning in the change description that this is dead
code (and overall provide a summary as to making functions static, etc.).
Though I'm not entirely convinced as to the removal since this function
documents the hardware interface, which must have been the intent to put
it there. Perhaps #if 0?
> --- linux-next-20260807.orig/include/video/maxinefb.h
> +++ linux-next-20260807/include/video/maxinefb.h
> @@ -13,13 +13,13 @@
> /*
> * IMS332 video controller register base address
> */
> -#define MAXINEFB_IMS332_ADDRESS KSEG1ADDR(0x1c140000)
> +#define MAXINEFB_IMS332_ADDRESS CKSEG1ADDR(0x1c140000)
>
> /*
> * Begin of DECstation 5000/xx onboard framebuffer memory, default resolution
> * is 1024x768x8
> */
> -#define DS5000_xx_ONBOARD_FBMEM_START KSEG1ADDR(0x0a000000)
> +#define DS5000_xx_ONBOARD_FBMEM_START CKSEG1ADDR(0x0a000000)
>
> /*
> * The IMS 332 video controller used in the DECstation 5000/xx series
This would probably best be a separate change, especially as it's not
needed for 32BIT.
I'm not sure how the actual maintainers see it (I should probably list
myself for all the scattered DECstation stuff), but I think this would
best be a patch series of 3 for each of the individual classes of fixes.
And since this borders on the obvious I wouldn't be too concerned as to
the change descriptions; just one sentence plus the relevant error message
quoted will do. And your introduction should suffice as the cover letter.
Let me know if this works for you and if you post such a series, I'll ack
it right away.
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] fbdev: maxinefb: fix build issues
2026-08-10 3:29 ` Randy Dunlap
@ 2026-08-13 18:14 ` Maciej W. Rozycki
0 siblings, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2026-08-13 18:14 UTC (permalink / raw)
To: Randy Dunlap
Cc: Philippe Mathieu-Daudé, linux-kernel, Helge Deller,
linux-fbdev, dri-devel, Thomas Bogendoerfer, linux-mips
On Sun, 9 Aug 2026, Randy Dunlap wrote:
> >> --- linux-next-20260807.orig/include/video/maxinefb.h
> >> +++ linux-next-20260807/include/video/maxinefb.h
> >> @@ -13,13 +13,13 @@
> >> /*
> >> * IMS332 video controller register base address
> >> */
> >> -#define MAXINEFB_IMS332_ADDRESS KSEG1ADDR(0x1c140000)
> >> +#define MAXINEFB_IMS332_ADDRESS CKSEG1ADDR(0x1c140000)
> >> /*
> >> * Begin of DECstation 5000/xx onboard framebuffer memory, default resolution
> >> * is 1024x768x8
> >> */
> >> -#define DS5000_xx_ONBOARD_FBMEM_START KSEG1ADDR(0x0a000000)
> >> +#define DS5000_xx_ONBOARD_FBMEM_START CKSEG1ADDR(0x0a000000)
> >
> > The change looks correct, but clearly this has never been tested
> > on a 64-bit DECstation, so I'm not sure having this code being
> > now built is an improvement.
I don't expect any issues and FWIW the Maxine does support a 64-bit CPU
module.
> > diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> > index ac9ac4287c6a..ff17afe6b8b3 100644
> > --- a/drivers/video/fbdev/Kconfig
> > +++ b/drivers/video/fbdev/Kconfig
> > @@ -1436,7 +1436,7 @@ config FB_PMAGB_B
> >
> > config FB_MAXINE
> > bool "Maxine (Personal DECstation) onboard framebuffer support"
> > - depends on (FB = y) && MACH_DECSTATION
> > + depends on (FB = y) && MACH_DECSTATION && 32BIT
> > select FB_IOMEM_HELPERS
> > help
> > Support for the onboard framebuffer (1024x768x8) in the Personal
> > ---
> >
> > ?
>
> I guess I'm OK with that. I'm even OK with removing the driver completely.
No need to limit the driver to 32BIT and as I suggest in the other reply
fixing 64BIT will best be a separate change.
> Maciej did say in https://lore.kernel.org/all/alpine.DEB.2.21.2308152255080.8596@angie.orcam.me.uk/
> that: "the proper way would be by using `ioremap' really. I'll make a patch."
> [That was 15.Aug.2023.]
Even more properly this will best be converted to a platform device and
then resource handling will DTRT. I do hope to get a specimen eventually,
so I think there's no need to rush making changes on paper.
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] fbdev: maxinefb: fix build issues
2026-08-13 16:11 ` Helge Deller
@ 2026-08-13 18:16 ` Maciej W. Rozycki
2026-08-13 20:52 ` Helge Deller
0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2026-08-13 18:16 UTC (permalink / raw)
To: Helge Deller
Cc: Randy Dunlap, linux-kernel, linux-fbdev, dri-devel,
Thomas Bogendoerfer, linux-mips
On Thu, 13 Aug 2026, Helge Deller wrote:
> > This is all ancient source code from the beginning of git time.
> > Now builds cleanly on 32-bit and 64-bit MACH_DECSTATION.
> >
> I've added the various comments from the mail thread to the
> patch and applied the patch as-is for now.
Given the statement of the problem as per the quotation above I don't
think there was a need to fast-track it without giving me a chance to
chime in.
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] fbdev: maxinefb: fix build issues
2026-08-13 18:16 ` Maciej W. Rozycki
@ 2026-08-13 20:52 ` Helge Deller
0 siblings, 0 replies; 11+ messages in thread
From: Helge Deller @ 2026-08-13 20:52 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Randy Dunlap, linux-kernel, linux-fbdev, dri-devel,
Thomas Bogendoerfer, linux-mips
On 8/13/26 20:16, Maciej W. Rozycki wrote:
> On Thu, 13 Aug 2026, Helge Deller wrote:
>
>>> This is all ancient source code from the beginning of git time.
>>> Now builds cleanly on 32-bit and 64-bit MACH_DECSTATION.
>>>
>> I've added the various comments from the mail thread to the
>> patch and applied the patch as-is for now.
>
> Given the statement of the problem as per the quotation above I don't
> think there was a need to fast-track it without giving me a chance to
> chime in.
Don't worry. I'm happy to drop the intermediate patch anytime.
Helge
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] fbdev: maxinefb: fix build issues
2026-08-13 17:31 ` Maciej W. Rozycki
@ 2026-08-13 21:46 ` Randy Dunlap
0 siblings, 0 replies; 11+ messages in thread
From: Randy Dunlap @ 2026-08-13 21:46 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: linux-kernel, Helge Deller, linux-fbdev, dri-devel,
Thomas Bogendoerfer, linux-mips
On 8/13/26 10:31 AM, Maciej W. Rozycki wrote:
> On Sun, 9 Aug 2026, Randy Dunlap wrote:
>
>> This is all ancient source code from the beginning of git time.
>
> I can dig out any further details you may need from the old LMO git tree.
> Eventually it's planned to be put back online, but resources.
>
>> Now builds cleanly on 32-bit and 64-bit MACH_DECSTATION.
>
> Thanks for doing this work.
>
>> Suggested-by: "Maciej W. Rozycki" <macro@orcam.me.uk> # for s/KSEG1ADDR/CKSEG1ADDR/
>
> Eventually this code should be converted to a platform device, so such
> hacks won't be necessary. Sadly I declined an offer back in 2009 and I
> have been chasing a specimen for a while now. And since it's an onboard
> device unlike all the TURBOchannel framebuffers, there's no alternative.
>
>> --- linux-next-20260807.orig/drivers/video/fbdev/maxinefb.c
>> +++ linux-next-20260807/drivers/video/fbdev/maxinefb.c
>> @@ -71,19 +71,6 @@ void maxinefb_ims332_write_register(int
>> *((volatile unsigned short *) (wptr)) = val;
>> }
>>
>> -unsigned int maxinefb_ims332_read_register(int regno)
>> -{
>> - register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
>> - unsigned char *rptr;
>> - register unsigned int j, k;
>> -
>> - rptr = regs + 0x80000 + (regno << 4);
>> - j = *((volatile unsigned short *) rptr);
>> - k = *((volatile unsigned short *) regs);
>> -
>> - return (j & 0xffff) | ((k & 0xff00) << 8);
>> -}
>> -
>> /* Set the palette */
>> static int maxinefb_setcolreg(unsigned regno, unsigned red, unsigned green,
>> unsigned blue, unsigned transp, struct fb_info *info)
>
> It might be worth mentioning in the change description that this is dead
> code (and overall provide a summary as to making functions static, etc.).
> Though I'm not entirely convinced as to the removal since this function
> documents the hardware interface, which must have been the intent to put
> it there. Perhaps #if 0?
>
>> --- linux-next-20260807.orig/include/video/maxinefb.h
>> +++ linux-next-20260807/include/video/maxinefb.h
>> @@ -13,13 +13,13 @@
>> /*
>> * IMS332 video controller register base address
>> */
>> -#define MAXINEFB_IMS332_ADDRESS KSEG1ADDR(0x1c140000)
>> +#define MAXINEFB_IMS332_ADDRESS CKSEG1ADDR(0x1c140000)
>>
>> /*
>> * Begin of DECstation 5000/xx onboard framebuffer memory, default resolution
>> * is 1024x768x8
>> */
>> -#define DS5000_xx_ONBOARD_FBMEM_START KSEG1ADDR(0x0a000000)
>> +#define DS5000_xx_ONBOARD_FBMEM_START CKSEG1ADDR(0x0a000000)
>>
>> /*
>> * The IMS 332 video controller used in the DECstation 5000/xx series
>
> This would probably best be a separate change, especially as it's not
> needed for 32BIT.
>
> I'm not sure how the actual maintainers see it (I should probably list
> myself for all the scattered DECstation stuff), but I think this would
> best be a patch series of 3 for each of the individual classes of fixes.
> And since this borders on the obvious I wouldn't be too concerned as to
> the change descriptions; just one sentence plus the relevant error message
> quoted will do. And your introduction should suffice as the cover letter.
>
> Let me know if this works for you and if you post such a series, I'll ack
> it right away.
OK, I'll work on that.
--
~Randy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH] fbdev: maxinefb: fix build issues
2026-08-10 0:27 ` Randy Dunlap
@ 2026-08-14 2:46 ` Randy Dunlap
0 siblings, 0 replies; 11+ messages in thread
From: Randy Dunlap @ 2026-08-14 2:46 UTC (permalink / raw)
To: sashiko-reviews
Cc: dri-devel, Maciej W. Rozycki, Helge Deller, linux-fbdev,
linux-mips, Thomas Bogendoerfer
On 8/9/26 5:27 PM, Randy Dunlap wrote:
> [adding Cc:s]
>
> On 8/9/26 5:02 PM, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>
>> Pre-existing issues:
>> - [High] Initialization function returns positive value `1` on failure, breaking module load semantics and causing subsequent `registered_fb` array corruption on unload.
>> - [High] `maxinefb_fix.smem_start` is initialized with a virtual address (`CKSEG1ADDR`) instead of the hardware physical address, breaking user-space `mmap()`.
>> --
>>
>> --- Patch [1]: [RFC PATCH] fbdev: maxinefb: fix build issues ---
>> commit e391418fa2f21f8c420eccb513c031674ef184c6
>> Author: Randy Dunlap <rdunlap@infradead.org>
>>
>> fbdev: maxinefb: fix build issues
>>
>> This commit fixes build errors and warnings for the maxinefb driver on
>> 32-bit and 64-bit MACH_DECSTATION. It updates KSEG1ADDR to CKSEG1ADDR and
>> adds the static keyword to missing prototypes.
>>
>>> diff --git a/drivers/video/fbdev/maxinefb.c b/drivers/video/fbdev/maxinefb.c
>>> index 52528eb4dfb41..8aa1389e2d4c8 100644
>>> --- a/drivers/video/fbdev/maxinefb.c
>>> +++ b/drivers/video/fbdev/maxinefb.c
>> [ ... ]
>>> @@ -111,7 +98,7 @@ static const struct fb_ops maxinefb_ops = {
>>> .fb_setcolreg = maxinefb_setcolreg,
>>> };
>>>
>>> -int __init maxinefb_init(void)
>>> +static int __init maxinefb_init(void)
>>
>> [Severity: High]
>> This is a pre-existing issue, but does returning 1 on failure in this function
>> cause issues with module load semantics?
>>
>> In maxinefb_init():
>>
>> if (register_framebuffer(&fb_info) < 0)
>> return 1;
>>
>> If register_framebuffer() fails, returning a positive integer causes the kernel
>> to treat the module as successfully loaded because the module init framework
>> expects negative error codes for failure.
>
> Ack. I'll change that.
I had this fixed locally and then forgot about it in the 3-patch series.
I'll add it as patch 04/04.
--
~Randy
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-08-14 2:47 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-09 23:48 [RFC PATCH] fbdev: maxinefb: fix build issues Randy Dunlap
[not found] ` <20260810000251.55AD61F000E9@smtp.kernel.org>
2026-08-10 0:27 ` Randy Dunlap
2026-08-14 2:46 ` Randy Dunlap
2026-08-10 3:24 ` Philippe Mathieu-Daudé
2026-08-10 3:29 ` Randy Dunlap
2026-08-13 18:14 ` Maciej W. Rozycki
2026-08-13 16:11 ` Helge Deller
2026-08-13 18:16 ` Maciej W. Rozycki
2026-08-13 20:52 ` Helge Deller
2026-08-13 17:31 ` Maciej W. Rozycki
2026-08-13 21:46 ` Randy Dunlap
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox