[PATCH 00/03] fbdev: maxine: clean up build issues

[PATCH 00/03] fbdev: maxine: clean up build issues

[Randy Dunlap]

This is all ancient source code from the beginning of git time.
Now builds cleanly on 32-bit and 64-bit MACH_DECSTATION.


Subject: [PATCH 01/03] fbdev: maxine: make functions static
Subject: [PATCH 02/03] fbdev: maxine: elide an unused function
Subject: [PATCH 03/03] fbdev: maxine: fix 64-bit build error

 drivers/video/fbdev/maxinefb.c |    9 ++++++---
 include/video/maxinefb.h       |    4 ++--
 2 files changed, 8 insertions(+), 5 deletions(-)

Cc: Helge Deller <deller@gmx.de>
Cc: linux-fbdev@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>

[PATCH 01/03] fbdev: maxine: make functions static

[Randy Dunlap]

Make 3 functions static so that they don't need to be declared in a
header file.

../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)
../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:114:12: warning: no previous prototype for 'maxinefb_init' [-Wmissing-prototypes]
  114 | int __init maxinefb_init(void)

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
Cc: Helge Deller <deller@gmx.de>
Cc: linux-fbdev@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>

 drivers/video/fbdev/maxinefb.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- linux-next-20260812.orig/drivers/video/fbdev/maxinefb.c
+++ linux-next-20260812/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,7 +71,7 @@ void maxinefb_ims332_write_register(int
 	*((volatile unsigned short *) (wptr)) = val;
 }
 
-unsigned int maxinefb_ims332_read_register(int regno)
+static unsigned int maxinefb_ims332_read_register(int regno)
 {
 	register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
 	unsigned char *rptr;
@@ -111,7 +111,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;

[PATCH 02/03] fbdev: maxine: elide an unused function

[Randy Dunlap]

maxinefb_ims332_read_register() is not used, but since it describes a
hardware interface, leave it in the source file as documentation
and surround it inside an #if 0/#endif block.

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

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
Cc: Helge Deller <deller@gmx.de>
Cc: linux-fbdev@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>

 drivers/video/fbdev/maxinefb.c |    3 +++
 1 file changed, 3 insertions(+)

--- linux-next-20260812.orig/drivers/video/fbdev/maxinefb.c
+++ linux-next-20260812/drivers/video/fbdev/maxinefb.c
@@ -71,6 +71,8 @@ static void maxinefb_ims332_write_regist
 	*((volatile unsigned short *) (wptr)) = val;
 }
 
+#if 0
+/* dead code: leave here for hardware interface documentation */
 static unsigned int maxinefb_ims332_read_register(int regno)
 {
 	register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
@@ -83,6 +85,7 @@ static unsigned int maxinefb_ims332_read
 
 	return (j & 0xffff) | ((k & 0xff00) << 8);
 }
+#endif
 
 /* Set the palette */
 static int maxinefb_setcolreg(unsigned regno, unsigned red, unsigned green,

[PATCH 03/03] fbdev: maxine: fix 64-bit build error

[Randy Dunlap]

The KSEG1ADDR() macro is only defined for non-64BIT builds.
Use the CKSEG1ADDR() macro instead.

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;

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
Cc: Helge Deller <deller@gmx.de>
Cc: linux-fbdev@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>

 include/video/maxinefb.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- linux-next-20260812.orig/include/video/maxinefb.h
+++ linux-next-20260812/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

Re: [PATCH 01/03] fbdev: maxine: make functions static

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Making the entirely unused function `maxinefb_ims332_read_register` static introduces a new `-Wunused-function` compiler warning.
--

--- Patch [1]: [PATCH 01/03] fbdev: maxine: make functions static ---
commit dbf41183475fd7e7f21a5345da1fcb01f0239170
Author: Randy Dunlap <rdunlap@infradead.org>

fbdev: maxine: make functions static

This commit makes three functions static in the maxinefb driver to resolve
missing prototype warnings since they are not exposed via a header file.

> diff --git a/drivers/video/fbdev/maxinefb.c b/drivers/video/fbdev/maxinefb.c
> index 52528eb4dfb41..a9410a553113d 100644
> --- a/drivers/video/fbdev/maxinefb.c
> +++ b/drivers/video/fbdev/maxinefb.c
[ ... ]
> @@ -71,7 +71,7 @@ void maxinefb_ims332_write_register(int regno, register unsigned int val)
>  	*((volatile unsigned short *) (wptr)) = val;
>  }
>  
> -unsigned int maxinefb_ims332_read_register(int regno)
> +static unsigned int maxinefb_ims332_read_register(int regno)

[Severity: Low]
Does making this function static introduce a new -Wunused-function compiler
warning? 

It appears maxinefb_ims332_read_register() is never called anywhere in the 
codebase. Adding the static keyword to an unused function transforms the 
previous missing prototype warning into an unused function warning, which 
can break builds that enable CONFIG_WERROR. 

Would it be better to remove this unused code entirely rather than marking
it static?

>  {
>  	register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
>  	unsigned char *rptr;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813232934.2718291-1-rdunlap@infradead.org?part=1

Re: [PATCH 02/03] fbdev: maxine: elide an unused function

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch uses an `#if 0` block to retain the unused function `maxinefb_ims332_read_register` instead of removing it from the source file.
--

--- Patch [2]: [PATCH 02/03] fbdev: maxine: elide an unused function ---
commit 603e8df0aa8d23f85b3ef46ca55422bc6d08d6c9
Author: Randy Dunlap <rdunlap@infradead.org>

fbdev: maxine: elide an unused function

This commit addresses a compiler warning about an unused function by wrapping
maxinefb_ims332_read_register() in an #if 0 block. The author notes that the
function is intentionally retained to serve as documentation for the hardware
interface.

> diff --git a/drivers/video/fbdev/maxinefb.c b/drivers/video/fbdev/maxinefb.c
> --- a/drivers/video/fbdev/maxinefb.c
> +++ b/drivers/video/fbdev/maxinefb.c
> @@ -71,6 +71,8 @@ static void maxinefb_ims332_write_register(int regno, register unsigned int val)
>  	*((volatile unsigned short *) (wptr)) = val;
>  }
>  
> +#if 0
> +/* dead code: leave here for hardware interface documentation */
>  static unsigned int maxinefb_ims332_read_register(int regno)

[Severity: Low]
Is it better to just delete this unused function entirely? Section 21 of the
kernel coding style states that if a function always goes unused, it should be
deleted rather than commented out or hidden behind preprocessor conditionals.

While the comment notes it acts as hardware documentation, leaving uncompiled
code in the tree skips compiler checks and often leads to bit-rot over time.
Could this be removed, relying on git history if anyone needs to reference
the hardware interface in the future?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813232934.2718291-1-rdunlap@infradead.org?part=2

Re: [PATCH 03/03] fbdev: maxine: fix 64-bit build error

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `smem_start` is incorrectly populated with a virtual address instead of a physical address, which breaks user-space mmap.
--

--- Patch [3]: [PATCH 03/03] fbdev: maxine: fix 64-bit build error ---
commit 4b24e26365b585612a868800c1d954fcade38bb4
Author: Randy Dunlap <rdunlap@infradead.org>
fbdev: maxine: fix 64-bit build error

This commit resolves a 64-bit build error in the maxinefb driver by
replacing the KSEG1ADDR() macro, which is only defined for non-64BIT
builds, with the CKSEG1ADDR() macro.

> 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
[ ... ]
> @@ -19,7 +19,7 @@
>  /*
>   * 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)

[Severity: High]
This is a pre-existing issue, but does assigning this virtual address to
the framebuffer's fix.smem_start break user-space mmap?

When user-space applications call mmap() on the framebuffer device, fb_mmap 
calculates the physical frame number by shifting info->fix.smem_start. Since 
this macro defines a virtual CKSEG1 address instead of a physical address 
(0x0a000000), the resulting physical frame number maps to invalid physical 
memory space.

Looking at drivers/video/fbdev/maxinefb.c in maxinefb_init():

    fb_start = DS5000_xx_ONBOARD_FBMEM_START;
    ...
    maxinefb_fix.smem_start = fb_start;

Should smem_start be populated with the actual physical address rather than
the CKSEG1-mapped virtual address to prevent bus errors or unintended physical
memory access during user-space mappings?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813232934.2718291-1-rdunlap@infradead.org?part=3