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;