Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH] staging: sm750fb: select FB_IOMEM_FOPS in Kconfig
@ 2026-07-19 19:58 MishraMohit21
  2026-07-19 19:58 ` [PATCH] staging: sm750fb: Refactor init_status to use initchip_param MishraMohit21
  2026-07-20  7:50 ` [PATCH] staging: sm750fb: select FB_IOMEM_FOPS in Kconfig Ahmet Sezgin Duran
  0 siblings, 2 replies; 3+ messages in thread
From: MishraMohit21 @ 2026-07-19 19:58 UTC (permalink / raw)
  To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
  Cc: linux-fbdev, linux-staging, linux-kernel, MishraMohit21

The driver uses the FB_DEFAULT_IOMEM_OPS macro in sm750.c to define its
framebuffer operations. This macro references core framebuffer operations
fb_io_read, fb_io_write, and fb_io_mmap. These symbols are defined in
fb_io_fops.c, which is compiled only under CONFIG_FB_IOMEM_FOPS.

Since config FB_SM750 does not currently select FB_IOMEM_FOPS, compiling
the driver on a config that has no other framebuffer drivers enabled
fails with undefined symbol link errors during MODPOST.

Add 'select FB_IOMEM_FOPS' to Kconfig to ensure the driver core helpers
build successfully in all configurations. Keep the select statements
sorted alphabetically.

Signed-off-by: MishraMohit21 <mishraloopmohit@gmail.com>
---
 drivers/staging/sm750fb/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/sm750fb/Kconfig b/drivers/staging/sm750fb/Kconfig
index 08bcccdd0f1c..7d5ad423e41f 100644
--- a/drivers/staging/sm750fb/Kconfig
+++ b/drivers/staging/sm750fb/Kconfig
@@ -2,10 +2,11 @@
 config FB_SM750
 	tristate "Silicon Motion SM750 framebuffer support"
 	depends on FB && PCI && HAS_IOPORT
-	select FB_MODE_HELPERS
-	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
+	select FB_CFB_FILLRECT
 	select FB_CFB_IMAGEBLIT
+	select FB_IOMEM_FOPS
+	select FB_MODE_HELPERS
 	help
 	  Frame buffer driver for the Silicon Motion SM750 chip
 	  with 2D acceleration and dual head support.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH] staging: sm750fb: Refactor init_status to use initchip_param
  2026-07-19 19:58 [PATCH] staging: sm750fb: select FB_IOMEM_FOPS in Kconfig MishraMohit21
@ 2026-07-19 19:58 ` MishraMohit21
  2026-07-20  7:50 ` [PATCH] staging: sm750fb: select FB_IOMEM_FOPS in Kconfig Ahmet Sezgin Duran
  1 sibling, 0 replies; 3+ messages in thread
From: MishraMohit21 @ 2026-07-19 19:58 UTC (permalink / raw)
  To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
  Cc: linux-fbdev, linux-staging, linux-kernel, MishraMohit21

The driver initializes the hardware by casting 'struct init_status *' to
'struct initchip_param *' in ddk750_init_hw(). This is technically
undefined behavior, violates strict-aliasing rules, and is fragile.

Furthermore, 'struct init_status' defines the 'reset_memory' field as a
2-byte 'ushort', while 'struct initchip_param' defines it as a 1-byte
'unsigned char'. On little-endian architectures, reading the low byte
of the ushort happens to evaluate to the correct value (0 or 1) by accident.
However, on big-endian architectures, casting and reading this field
reads the high byte (0x00) instead, causing a silent failure where the
memory controller is never reset.

This endianness layout mismatch was empirically verified using a standalone
test harness (scratch/be_test.c) compiled under a mips-linux-gnu-gcc
cross-compiler and executed under qemu-mips.

Resolve this by removing the duplicate 'struct init_status' entirely and
using 'struct initchip_param' directly. This removes the unsafe pointer
cast and ensures endian-safe hardware initialization.

This change has been compile-tested only. No hardware was available to
verify runtime behavior.

Signed-off-by: MishraMohit21 <mishraloopmohit@gmail.com>
---
 drivers/staging/sm750fb/sm750.c    |  8 ++++----
 drivers/staging/sm750fb/sm750.h    | 12 ++----------
 drivers/staging/sm750fb/sm750_hw.c | 16 ++++++++--------
 3 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 89c811e0806c..5986dbef67c0 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -844,11 +844,11 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, char *src)
 
 	swap = 0;
 
-	sm750_dev->init_parm.chip_clk = 0;
-	sm750_dev->init_parm.mem_clk = 0;
-	sm750_dev->init_parm.master_clk = 0;
+	sm750_dev->init_parm.chip_clock = 0;
+	sm750_dev->init_parm.mem_clock = 0;
+	sm750_dev->init_parm.master_clock = 0;
 	sm750_dev->init_parm.power_mode = 0;
-	sm750_dev->init_parm.setAllEngOff = 0;
+	sm750_dev->init_parm.set_all_eng_off = 0;
 	sm750_dev->init_parm.reset_memory = 1;
 
 	/* defaultly turn g_hwcursor on for both view */
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index d2c522e67f26..313c2683bf6c 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -38,15 +38,7 @@ enum sm750_path {
 	sm750_pnc = 3,	/* panel and crt */
 };
 
-struct init_status {
-	ushort power_mode;
-	/* below three clocks are in unit of MHZ*/
-	ushort chip_clk;
-	ushort mem_clk;
-	ushort master_clk;
-	ushort setAllEngOff;
-	ushort reset_memory;
-};
+#include "ddk750_chip.h"
 
 struct lynx_accel {
 	/* base virtual address of DPR registers */
@@ -102,7 +94,7 @@ struct sm750_dev {
 	/* locks*/
 	spinlock_t slock;
 
-	struct init_status init_parm;
+	struct initchip_param init_parm;
 	enum sm750_pnltype pnltype;
 	enum sm750_dataflow dataflow;
 	int nocrt;
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index 34a837fb4b64..54c1b241ae6e 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -66,20 +66,20 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
 
 int hw_sm750_inithw(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
 {
-	struct init_status *parm;
+	struct initchip_param *parm;
 
 	parm = &sm750_dev->init_parm;
-	if (parm->chip_clk == 0)
-		parm->chip_clk = (sm750_get_chip_type() == SM750LE) ?
+	if (parm->chip_clock == 0)
+		parm->chip_clock = (sm750_get_chip_type() == SM750LE) ?
 					       DEFAULT_SM750LE_CHIP_CLOCK :
 					       DEFAULT_SM750_CHIP_CLOCK;
 
-	if (parm->mem_clk == 0)
-		parm->mem_clk = parm->chip_clk;
-	if (parm->master_clk == 0)
-		parm->master_clk = parm->chip_clk / 3;
+	if (parm->mem_clock == 0)
+		parm->mem_clock = parm->chip_clock;
+	if (parm->master_clock == 0)
+		parm->master_clock = parm->chip_clock / 3;
 
-	ddk750_init_hw((struct initchip_param *)&sm750_dev->init_parm);
+	ddk750_init_hw(&sm750_dev->init_parm);
 	/* for sm718, open pci burst */
 	if (sm750_dev->devid == 0x718) {
 		poke32(SYSTEM_CTRL,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] staging: sm750fb: select FB_IOMEM_FOPS in Kconfig
  2026-07-19 19:58 [PATCH] staging: sm750fb: select FB_IOMEM_FOPS in Kconfig MishraMohit21
  2026-07-19 19:58 ` [PATCH] staging: sm750fb: Refactor init_status to use initchip_param MishraMohit21
@ 2026-07-20  7:50 ` Ahmet Sezgin Duran
  1 sibling, 0 replies; 3+ messages in thread
From: Ahmet Sezgin Duran @ 2026-07-20  7:50 UTC (permalink / raw)
  To: MishraMohit21, Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
  Cc: linux-fbdev, linux-staging, linux-kernel

On 7/19/26 10:58 PM, MishraMohit21 wrote:
> The driver uses the FB_DEFAULT_IOMEM_OPS macro in sm750.c to define its
> framebuffer operations. This macro references core framebuffer operations
> fb_io_read, fb_io_write, and fb_io_mmap. These symbols are defined in
> fb_io_fops.c, which is compiled only under CONFIG_FB_IOMEM_FOPS.
> 
> Since config FB_SM750 does not currently select FB_IOMEM_FOPS, compiling
> the driver on a config that has no other framebuffer drivers enabled
> fails with undefined symbol link errors during MODPOST.
> 
> Add 'select FB_IOMEM_FOPS' to Kconfig to ensure the driver core helpers
> build successfully in all configurations. Keep the select statements
> sorted alphabetically.
> 
> Signed-off-by: MishraMohit21 <mishraloopmohit@gmail.com>
> ---
>   drivers/staging/sm750fb/Kconfig | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/Kconfig b/drivers/staging/sm750fb/Kconfig
> index 08bcccdd0f1c..7d5ad423e41f 100644
> --- a/drivers/staging/sm750fb/Kconfig
> +++ b/drivers/staging/sm750fb/Kconfig
> @@ -2,10 +2,11 @@
>   config FB_SM750
>   	tristate "Silicon Motion SM750 framebuffer support"
>   	depends on FB && PCI && HAS_IOPORT
> -	select FB_MODE_HELPERS
> -	select FB_CFB_FILLRECT
>   	select FB_CFB_COPYAREA
> +	select FB_CFB_FILLRECT
>   	select FB_CFB_IMAGEBLIT
> +	select FB_IOMEM_FOPS
> +	select FB_MODE_HELPERS
>   	help
>   	  Frame buffer driver for the Silicon Motion SM750 chip
>   	  with 2D acceleration and dual head support.

Does not apply to Greg's staging-testing branch. `FB_IOMEM_FOPS` already 
exists in Kconfig.

Also you need to put a fullname in `Signed-off-by` part.

Regards,
Ahmet Sezgin Duran

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-20  7:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-19 19:58 [PATCH] staging: sm750fb: select FB_IOMEM_FOPS in Kconfig MishraMohit21
2026-07-19 19:58 ` [PATCH] staging: sm750fb: Refactor init_status to use initchip_param MishraMohit21
2026-07-20  7:50 ` [PATCH] staging: sm750fb: select FB_IOMEM_FOPS in Kconfig Ahmet Sezgin Duran

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox