[PATCH 0/5] staging: sm750fb: various code cleanups

[PATCH 0/5] staging: sm750fb: various code cleanups

[Ahmet Sezgin Duran]

This series performs several cleanups on the sm750fb staging driver
to improve code readability and remove redundancy.

The changes include:
- Removing a block of commented-out forward declarations.
- Removing redundant variable initializations.
- Removing unused struct fields.
- Replacing a hardcoded loop bound with ARRAY_SIZE().
- Deduplicating per-index fbinfo handling in suspend/resume using
  a loop.

No functional changes are intended.

Ahmet Sezgin Duran (5):
  staging: sm750fb: remove commented-out forward declarations
  staging: sm750fb: remove unnecessary initializations
  staging: sm750fb: remove unused struct fields
  staging: sm750fb: use ARRAY_SIZE macro in fb_find_mode loop
  staging: sm750fb: deduplicate fbinfo loop in suspend/resume

 drivers/staging/sm750fb/sm750.c | 47 ++++++++++-----------------------
 drivers/staging/sm750fb/sm750.h |  3 ---
 2 files changed, 14 insertions(+), 36 deletions(-)


base-commit: 7cb1c5b32a2bfde961fff8d5204526b609bcb30a
-- 
2.53.0

[PATCH 1/5] staging: sm750fb: remove commented-out forward declarations

[Ahmet Sezgin Duran]

The block at the top of sm750.c declares lynxfb_ops_write() and
lynxfb_ops_read() inside a commented-out #ifdef __BIG_ENDIAN guard.
Neither function is defined anywhere in the driver, so the block
is dead. Remove it.

Signed-off-by: Ahmet Sezgin Duran <ahmet@sezginduran.net>
---
 drivers/staging/sm750fb/sm750.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 89c811e0806c..02db1418476b 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -8,15 +8,6 @@
 #include "sm750_accel.h"
 #include "sm750_cursor.h"
 
-/*
- * #ifdef __BIG_ENDIAN
- * ssize_t lynxfb_ops_write(struct fb_info *info, const char __user *buf,
- * size_t count, loff_t *ppos);
- * ssize_t lynxfb_ops_read(struct fb_info *info, char __user *buf,
- * size_t count, loff_t *ppos);
- * #endif
- */
-
 /* common var for all device */
 static int g_hwcursor = 1;
 static int g_noaccel __ro_after_init;
-- 
2.53.0

[PATCH 2/5] staging: sm750fb: remove unnecessary initializations

[Ahmet Sezgin Duran]

Remove two instances of `ret = 0` initializations since the
variable is overridden unconditionally before being used.

Signed-off-by: Ahmet Sezgin Duran <ahmet@sezginduran.net>
---
 drivers/staging/sm750fb/sm750.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 02db1418476b..fff9c35ee7b0 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -343,7 +343,6 @@ static int lynxfb_ops_set_par(struct fb_info *info)
 	if (!info)
 		return -EINVAL;
 
-	ret = 0;
 	par = info->par;
 	crtc = &par->crtc;
 	output = &par->output;
@@ -463,7 +462,6 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo *var,
 	if (!var->pixclock)
 		return -EINVAL;
 
-	ret = 0;
 	par = info->par;
 	crtc = &par->crtc;
 
-- 
2.53.0

[PATCH 3/5] staging: sm750fb: remove unused struct fields

[Ahmet Sezgin Duran]

Remove `void *priv` pointer field in following struct definitions:

- `struct lynxfb_crtc`
- `struct lynxfb_output`

Verified that no other code references them.

No functional changes.

Signed-off-by: Ahmet Sezgin Duran <ahmet@sezginduran.net>
---
 drivers/staging/sm750fb/sm750.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index d2c522e67f26..56d7e1fa4557 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -145,8 +145,6 @@ struct lynxfb_crtc {
 	u16 ypanstep;
 	u16 ywrapstep;
 
-	void *priv;
-
 	/* cursor information */
 	struct lynx_cursor cursor;
 };
@@ -168,7 +166,6 @@ struct lynxfb_output {
 	 * *channel=1 means secondary channel
 	 * output->channel ==> &crtc->channel
 	 */
-	void *priv;
 };
 
 struct lynxfb_par {
-- 
2.53.0

[PATCH 4/5] staging: sm750fb: use ARRAY_SIZE macro in fb_find_mode loop

[Ahmet Sezgin Duran]

The loop in lynxfb_set_fbinfo() iterates over pdb[] and cdb[] using
a hardcoded bound of 3.

Replace it with ARRAY_SIZE(pdb) so the bound tracks the array.

Signed-off-by: Ahmet Sezgin Duran <ahmet@sezginduran.net>
---
 drivers/staging/sm750fb/sm750.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index fff9c35ee7b0..465615b528fd 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -764,7 +764,7 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
 			g_fbmode[index] = g_fbmode[0];
 	}
 
-	for (i = 0; i < 3; i++) {
+	for (i = 0; i < ARRAY_SIZE(pdb); i++) {
 		ret = fb_find_mode(var, info, g_fbmode[index],
 				   pdb[i], cdb[i], NULL, 8);
 
-- 
2.53.0

[PATCH 5/5] staging: sm750fb: deduplicate fbinfo loop in suspend/resume

[Ahmet Sezgin Duran]

lynxfb_suspend() and lynxfb_resume() both walk sm750_dev->fbinfo[]
via duplicated per-index blocks for fbinfo[0] and fbinfo[1].

Replace each pair of blocks with a for-loop bounded by
sm750_dev->fb_count, the number of successfully registered
framebuffers.

No functional changes intended.

Signed-off-by: Ahmet Sezgin Duran <ahmet@sezginduran.net>
---
 drivers/staging/sm750fb/sm750.c | 34 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 465615b528fd..279471d6cd14 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -388,18 +388,18 @@ static int __maybe_unused lynxfb_suspend(struct device *dev)
 {
 	struct fb_info *info;
 	struct sm750_dev *sm750_dev;
+	int i;
 
 	sm750_dev = dev_get_drvdata(dev);
 
 	console_lock();
-	info = sm750_dev->fbinfo[0];
-	if (info)
-		/* 1 means do suspend */
-		fb_set_suspend(info, 1);
-	info = sm750_dev->fbinfo[1];
-	if (info)
-		/* 1 means do suspend */
-		fb_set_suspend(info, 1);
+
+	for (i = 0; i < sm750_dev->fb_count; i++) {
+		info = sm750_dev->fbinfo[i];
+		if (info)
+			/* 1 means do suspend */
+			fb_set_suspend(info, 1);
+	}
 
 	console_unlock();
 	return 0;
@@ -414,6 +414,7 @@ static int __maybe_unused lynxfb_resume(struct device *dev)
 	struct lynxfb_par *par;
 	struct lynxfb_crtc *crtc;
 	struct lynx_cursor *cursor;
+	int i;
 
 	sm750_dev = pci_get_drvdata(pdev);
 
@@ -421,21 +422,12 @@ static int __maybe_unused lynxfb_resume(struct device *dev)
 
 	hw_sm750_inithw(sm750_dev, pdev);
 
-	info = sm750_dev->fbinfo[0];
-
-	if (info) {
-		par = info->par;
-		crtc = &par->crtc;
-		cursor = &crtc->cursor;
-		memset_io(cursor->vstart, 0x0, cursor->size);
-		memset_io(crtc->v_screen, 0x0, crtc->vidmem_size);
-		lynxfb_ops_set_par(info);
-		fb_set_suspend(info, 0);
-	}
+	for (i = 0; i < sm750_dev->fb_count; i++) {
+		info = sm750_dev->fbinfo[i];
 
-	info = sm750_dev->fbinfo[1];
+		if (!info)
+			continue;
 
-	if (info) {
 		par = info->par;
 		crtc = &par->crtc;
 		cursor = &crtc->cursor;
-- 
2.53.0

Re: [PATCH 4/5] staging: sm750fb: use ARRAY_SIZE macro in fb_find_mode loop

[Dan Carpenter]

On Sat, May 23, 2026 at 05:15:08AM +0000, Ahmet Sezgin Duran wrote:
> The loop in lynxfb_set_fbinfo() iterates over pdb[] and cdb[] using
> a hardcoded bound of 3.
> 
> Replace it with ARRAY_SIZE(pdb) so the bound tracks the array.

I don't love this.  As you mentioned, there are two arrays and they
both have 3 elements.  Why prefer one over the other?  This patch
makes the code look simpler than it really is.  I would just leave
it as 3.

regards,
dan carpenter

Re: [PATCH 4/5] staging: sm750fb: use ARRAY_SIZE macro in fb_find_mode loop

[Ahmet Sezgin Duran]

On 5/23/26 1:07 PM, Dan Carpenter wrote:

> I don't love this.  As you mentioned, there are two arrays and they
> both have 3 elements.  Why prefer one over the other?  This patch
> makes the code look simpler than it really is.  I would just leave
> it as 3.
> 
> regards,
> dan carpenter
> 

Fair. Going to drop that and send v2.

Thanks for the review.

Regards,
Ahmet Sezgin Duran

Re: [PATCH 4/5] staging: sm750fb: use ARRAY_SIZE macro in fb_find_mode loop

[David Laight]

On Sat, 23 May 2026 13:07:49 +0300
Dan Carpenter <error27@gmail.com> wrote:

> On Sat, May 23, 2026 at 05:15:08AM +0000, Ahmet Sezgin Duran wrote:
> > The loop in lynxfb_set_fbinfo() iterates over pdb[] and cdb[] using
> > a hardcoded bound of 3.
> > 
> > Replace it with ARRAY_SIZE(pdb) so the bound tracks the array.  
> 
> I don't love this.  As you mentioned, there are two arrays and they
> both have 3 elements.  Why prefer one over the other?  This patch
> makes the code look simpler than it really is.  I would just leave
> it as 3.

Or change the code to have one array of a struct that contains the ptr:count
pair and iterate over that.
Both pdb[] and cdb[] (or what replaces them) should (probably) be static.

This interface is strange, the NULL:0 requests the modes from xfree86
(which aren't visible) whereas the vesa modes that are defined just
after them have to be requested by ptr:count,

-- David


> 
> regards,
> dan carpenter
> 
> 

Re: [PATCH 5/5] staging: sm750fb: deduplicate fbinfo loop in suspend/resume

[Dan Carpenter]

I was waiting to see if anyone had other comments bout this patchset
to decide if I should mention these minor nits.  But then I was
confused about v2 so I think there might end up being comments... :P

On Sat, May 23, 2026 at 05:15:09AM +0000, Ahmet Sezgin Duran wrote:
> @@ -388,18 +388,18 @@ static int __maybe_unused lynxfb_suspend(struct device *dev)
>  {
>  	struct fb_info *info;
>  	struct sm750_dev *sm750_dev;
> +	int i;
>  
>  	sm750_dev = dev_get_drvdata(dev);
>  
>  	console_lock();
> -	info = sm750_dev->fbinfo[0];
> -	if (info)
> -		/* 1 means do suspend */
> -		fb_set_suspend(info, 1);
> -	info = sm750_dev->fbinfo[1];
> -	if (info)
> -		/* 1 means do suspend */
> -		fb_set_suspend(info, 1);
> +
> +	for (i = 0; i < sm750_dev->fb_count; i++) {
> +		info = sm750_dev->fbinfo[i];
> +		if (info)
> +			/* 1 means do suspend */
> +			fb_set_suspend(info, 1);

You didn't introduce this, but the rule is the multi-line indents get
curly braces for readabilitly even if they're not required.

> +	}
>  
>  	console_unlock();
>  	return 0;
> @@ -414,6 +414,7 @@ static int __maybe_unused lynxfb_resume(struct device *dev)
>  	struct lynxfb_par *par;
>  	struct lynxfb_crtc *crtc;
>  	struct lynx_cursor *cursor;
> +	int i;
>  
>  	sm750_dev = pci_get_drvdata(pdev);
>  
> @@ -421,21 +422,12 @@ static int __maybe_unused lynxfb_resume(struct device *dev)
>  
>  	hw_sm750_inithw(sm750_dev, pdev);
>  
> -	info = sm750_dev->fbinfo[0];
> -
> -	if (info) {
> -		par = info->par;
> -		crtc = &par->crtc;
> -		cursor = &crtc->cursor;
> -		memset_io(cursor->vstart, 0x0, cursor->size);
> -		memset_io(crtc->v_screen, 0x0, crtc->vidmem_size);
> -		lynxfb_ops_set_par(info);
> -		fb_set_suspend(info, 0);
> -	}
> +	for (i = 0; i < sm750_dev->fb_count; i++) {
> +		info = sm750_dev->fbinfo[i];
>  

Better to delete this blank line so the NULL check is next to the
assignment.

regards,
dan carpenter

> -	info = sm750_dev->fbinfo[1];
> +		if (!info)
> +			continue;
>  
> -	if (info) {
>  		par = info->par;
>  		crtc = &par->crtc;
>  		cursor = &crtc->cursor;

Re: [PATCH 5/5] staging: sm750fb: deduplicate fbinfo loop in suspend/resume

[Ahmet Sezgin Duran]

On 5/25/26 11:01 AM, Dan Carpenter wrote:
> I was waiting to see if anyone had other comments bout this patchset
> to decide if I should mention these minor nits.  But then I was
> confused about v2 so I think there might end up being comments... :P

v2 was only about dropping another patch from series, no change for this 
patch. (would be better to talk this in v2 mails, but I guess it's no 
big deal.)
>> +
>> +	for (i = 0; i < sm750_dev->fb_count; i++) {
>> +		info = sm750_dev->fbinfo[i];
>> +		if (info)
>> +			/* 1 means do suspend */
>> +			fb_set_suspend(info, 1);
> 
> You didn't introduce this, but the rule is the multi-line indents get
> curly braces for readabilitly even if they're not required.

Oh ok, just tried your suggestion, checkpatch didn't generate a warning. 
Got it.
>> -	}
>> +	for (i = 0; i < sm750_dev->fb_count; i++) {
>> +		info = sm750_dev->fbinfo[i];
>>   
> 
> Better to delete this blank line so the NULL check is next to the
> assignment.

Got it.

I'll send v3, thanks for the review.

Regards,
Ahmet Sezgin Duran

Re: [PATCH 5/5] staging: sm750fb: deduplicate fbinfo loop in suspend/resume

[Dan Carpenter]

On Mon, May 25, 2026 at 11:23:43AM +0300, Ahmet Sezgin Duran wrote:
> On 5/25/26 11:01 AM, Dan Carpenter wrote:
> > I was waiting to see if anyone had other comments bout this patchset
> > to decide if I should mention these minor nits.  But then I was
> > confused about v2 so I think there might end up being comments... :P
> 
> v2 was only about dropping another patch from series, no change for this
> patch. (would be better to talk this in v2 mails, but I guess it's no big
> deal.)

I thought you were going to to abandon the ARRAY_SIZE(cdb) in v2.

regards,
dan carpenter

Re: [PATCH 5/5] staging: sm750fb: deduplicate fbinfo loop in suspend/resume

[Ahmet Sezgin Duran]

On 5/25/26 7:13 PM, Dan Carpenter wrote:

> 
> I thought you were going to to abandon the ARRAY_SIZE(cdb) in v2.
> 
> regards,
> dan carpenter
> 

Which, I did:

(v2 link): 
https://lore.kernel.org/linux-staging/20260523153459.177488-1-ahmet@sezginduran.net/

Then, per this review, I sent v3:

(v3 link): 
https://lore.kernel.org/linux-staging/20260525085808.171974-1-ahmet@sezginduran.net/

Please let me know if I misunderstood anything or made a mistake.

Regards,
Ahmet Sezgin Duran

Re: [PATCH 5/5] staging: sm750fb: deduplicate fbinfo loop in suspend/resume

[Dan Carpenter]

On Mon, May 25, 2026 at 07:31:33PM +0300, Ahmet Sezgin Duran wrote:
> On 5/25/26 7:13 PM, Dan Carpenter wrote:
> 
> > 
> > I thought you were going to to abandon the ARRAY_SIZE(cdb) in v2.
> > 
> > regards,
> > dan carpenter
> > 
> 
> Which, I did:
> 
> (v2 link): https://lore.kernel.org/linux-staging/20260523153459.177488-1-ahmet@sezginduran.net/
> 

Ah, right.  Fair enough.

regards,
dan carpenter