[PATCH] video/fbdev/core: Mark debug-only variable as __maybe_unused

[PATCH] video/fbdev/core: Mark debug-only variable as __maybe_unused

[Thomas Zimmermann]

Compiling fbcon.c gives

../drivers/video/fbdev/core/fbcon.c: In function 'fbcon_exit':
../drivers/video/fbdev/core/fbcon.c:3358:7: warning: variable 'pending' set but not used [-Wunused-but-set-variable]
 3358 |   int pending = 0;
      |       ^~~~~~~

The variable pending is only used for fbcon debugging. It's unused
otherwise. Mark it accordingly.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/core/fbcon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index d03f62369734..aca1512b6e86 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -3355,7 +3355,7 @@ static void fbcon_exit(void)
 #endif
 
 	for_each_registered_fb(i) {
-		int pending = 0;
+		int __maybe_unused pending = 0;
 
 		mapped = 0;
 		info = registered_fb[i];
-- 
2.28.0

Re: [PATCH] video/fbdev/core: Mark debug-only variable as __maybe_unused

[Sam Ravnborg]

Hi Thomas.

On Wed, Oct 21, 2020 at 02:15:12PM +0200, Thomas Zimmermann wrote:
> Compiling fbcon.c gives
> 
> ../drivers/video/fbdev/core/fbcon.c: In function 'fbcon_exit':
> ../drivers/video/fbdev/core/fbcon.c:3358:7: warning: variable 'pending' set but not used [-Wunused-but-set-variable]
>  3358 |   int pending = 0;
>       |       ^~~~~~~
> 
> The variable pending is only used for fbcon debugging. It's unused
> otherwise. Mark it accordingly.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

A better fix would be to replace the few uses of DPRINTK() with
pr_dbg(). pr_info() is alread in use.

ofc, the next step would be to replace all prink() with their pr_
counterparts.

Peilin, maybe this is one for your nice cleanups in fbcon?

	Sam

Re: [PATCH] video/fbdev/core: Mark debug-only variable as __maybe_unused

[Peilin Ye]

Hi Sam,

On Sun, Nov 01, 2020 at 10:47:18AM +0100, Sam Ravnborg wrote:
> Hi Thomas.
> 
> On Wed, Oct 21, 2020 at 02:15:12PM +0200, Thomas Zimmermann wrote:
> > Compiling fbcon.c gives
> > 
> > ../drivers/video/fbdev/core/fbcon.c: In function 'fbcon_exit':
> > ../drivers/video/fbdev/core/fbcon.c:3358:7: warning: variable 'pending' set but not used [-Wunused-but-set-variable]
> >  3358 |   int pending = 0;
> >       |       ^~~~~~~
> > 
> > The variable pending is only used for fbcon debugging. It's unused
> > otherwise. Mark it accordingly.
> > 
> > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> A better fix would be to replace the few uses of DPRINTK() with
> pr_dbg(). pr_info() is alread in use.
> 
> ofc, the next step would be to replace all prink() with their pr_
> counterparts.
> 
> Peilin, maybe this is one for your nice cleanups in fbcon?

Sure, I will send a patch replacing all printk() except DPRINTK(). Thank
you for the suggestion!

Peilin

[PATCH] fbcon: Replace printk() with pr_*()

[Peilin Ye]

Replace printk() with pr_err(), pr_warn() and pr_info(). Do not split long
strings, for easier grepping. Use `__func__` whenever applicable.

fbcon_prepare_logo() has more than one callers, use "fbcon_prepare_logo:"
instead of "fbcon_init:", for less confusion.

Suggested-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
---
 drivers/video/fbdev/core/fbcon.c | 42 +++++++++++++-------------------
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index cef437817b0d..a3e87ab0e523 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -659,8 +659,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct fb_info *info,
 
 	if (logo_lines > vc->vc_bottom) {
 		logo_shown = FBCON_LOGO_CANSHOW;
-		printk(KERN_INFO
-		       "fbcon_init: disable boot-logo (boot-logo bigger than screen).\n");
+		pr_info("%s: disable boot-logo (boot-logo bigger than screen).\n", __func__);
 	} else {
 		logo_shown = FBCON_LOGO_DRAW;
 		vc->vc_top = logo_lines;
@@ -785,9 +784,8 @@ static int con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo,
 			ret = newinfo->fbops->fb_set_par(newinfo);
 
 			if (ret)
-				printk(KERN_ERR "con2fb_release_oldinfo: "
-					"detected unhandled fb_set_par error, "
-					"error code %d\n", ret);
+				pr_err("%s: detected unhandled fb_set_par error, error code %d\n",
+				       __func__, ret);
 		}
 	}
 
@@ -806,9 +804,8 @@ static void con2fb_init_display(struct vc_data *vc, struct fb_info *info,
 		ret = info->fbops->fb_set_par(info);
 
 		if (ret)
-			printk(KERN_ERR "con2fb_init_display: detected "
-				"unhandled fb_set_par error, "
-				"error code %d\n", ret);
+			pr_err("%s: detected unhandled fb_set_par error, error code %d\n",
+			       __func__, ret);
 	}
 
 	ops->flags |= FBCON_FLAGS_INIT;
@@ -1137,9 +1134,8 @@ static void fbcon_init(struct vc_data *vc, int init)
 			ret = info->fbops->fb_set_par(info);
 
 			if (ret)
-				printk(KERN_ERR "fbcon_init: detected "
-					"unhandled fb_set_par error, "
-					"error code %d\n", ret);
+				pr_err("%s: detected unhandled fb_set_par error, error code %d\n",
+				       __func__, ret);
 		}
 
 		ops->flags |= FBCON_FLAGS_INIT;
@@ -2126,9 +2122,8 @@ static int fbcon_switch(struct vc_data *vc)
 			ret = info->fbops->fb_set_par(info);
 
 			if (ret)
-				printk(KERN_ERR "fbcon_switch: detected "
-					"unhandled fb_set_par error, "
-					"error code %d\n", ret);
+				pr_err("%s: detected unhandled fb_set_par error, error code %d\n",
+				       __func__, ret);
 		}
 
 		if (old_info != info)
@@ -2899,9 +2894,8 @@ void fbcon_remap_all(struct fb_info *info)
 		set_con2fb_map(i, idx, 0);
 
 	if (con_is_bound(&fb_con)) {
-		printk(KERN_INFO "fbcon: Remapping primary device, "
-		       "fb%i, to tty %i-%i\n", idx,
-		       first_fb_vc + 1, last_fb_vc + 1);
+		pr_info("fbcon: Remapping primary device, fb%i, to tty %i-%i\n",
+			idx, first_fb_vc + 1, last_fb_vc + 1);
 		info_idx = idx;
 	}
 	console_unlock();
@@ -2914,17 +2908,16 @@ static void fbcon_select_primary(struct fb_info *info)
 	    fb_is_primary_device(info)) {
 		int i;
 
-		printk(KERN_INFO "fbcon: %s (fb%i) is primary device\n",
-		       info->fix.id, info->node);
+		pr_info("fbcon: %s (fb%i) is primary device\n",
+			info->fix.id, info->node);
 		primary_device = info->node;
 
 		for (i = first_fb_vc; i <= last_fb_vc; i++)
 			con2fb_map_boot[i] = primary_device;
 
 		if (con_is_bound(&fb_con)) {
-			printk(KERN_INFO "fbcon: Remapping primary device, "
-			       "fb%i, to tty %i-%i\n", info->node,
-			       first_fb_vc + 1, last_fb_vc + 1);
+			pr_info("fbcon: Remapping primary device, fb%i, to tty %i-%i\n",
+				info->node, first_fb_vc + 1, last_fb_vc + 1);
 			info_idx = primary_device;
 		}
 	}
@@ -3394,9 +3387,8 @@ void __init fb_console_init(void)
 				     "fbcon");
 
 	if (IS_ERR(fbcon_device)) {
-		printk(KERN_WARNING "Unable to create device "
-		       "for fbcon; errno = %ld\n",
-		       PTR_ERR(fbcon_device));
+		pr_warn("Unable to create device for fbcon; errno = %ld\n",
+			PTR_ERR(fbcon_device));
 		fbcon_device = NULL;
 	} else
 		fbcon_init_device();
-- 
2.25.1

Re: [PATCH] fbcon: Replace printk() with pr_*()

[Greg Kroah-Hartman]

On Sun, Nov 01, 2020 at 09:49:04AM -0500, Peilin Ye wrote:
> Replace printk() with pr_err(), pr_warn() and pr_info(). Do not split long
> strings, for easier grepping. Use `__func__` whenever applicable.
> 
> fbcon_prepare_logo() has more than one callers, use "fbcon_prepare_logo:"
> instead of "fbcon_init:", for less confusion.
> 
> Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> ---
>  drivers/video/fbdev/core/fbcon.c | 42 +++++++++++++-------------------
>  1 file changed, 17 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index cef437817b0d..a3e87ab0e523 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -659,8 +659,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct fb_info *info,
>  
>  	if (logo_lines > vc->vc_bottom) {
>  		logo_shown = FBCON_LOGO_CANSHOW;
> -		printk(KERN_INFO
> -		       "fbcon_init: disable boot-logo (boot-logo bigger than screen).\n");
> +		pr_info("%s: disable boot-logo (boot-logo bigger than screen).\n", __func__);

Shouldn't this be:
		dev_info(info->dev, "...");
instead?

It's a driver, and you have access to the struct device that is being
worked on, so always try to use the dev_* versions of these calls
instead when ever possible.

thanks,

greg k-h

Re: [PATCH] fbcon: Replace printk() with pr_*()

[Peilin Ye]

On Sun, Nov 01, 2020 at 04:41:13PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Nov 01, 2020 at 09:49:04AM -0500, Peilin Ye wrote:
> > Replace printk() with pr_err(), pr_warn() and pr_info(). Do not split long
> > strings, for easier grepping. Use `__func__` whenever applicable.
> > 
> > fbcon_prepare_logo() has more than one callers, use "fbcon_prepare_logo:"
> > instead of "fbcon_init:", for less confusion.
> > 
> > Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> > ---
> >  drivers/video/fbdev/core/fbcon.c | 42 +++++++++++++-------------------
> >  1 file changed, 17 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> > index cef437817b0d..a3e87ab0e523 100644
> > --- a/drivers/video/fbdev/core/fbcon.c
> > +++ b/drivers/video/fbdev/core/fbcon.c
> > @@ -659,8 +659,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct fb_info *info,
> >  
> >  	if (logo_lines > vc->vc_bottom) {
> >  		logo_shown = FBCON_LOGO_CANSHOW;
> > -		printk(KERN_INFO
> > -		       "fbcon_init: disable boot-logo (boot-logo bigger than screen).\n");
> > +		pr_info("%s: disable boot-logo (boot-logo bigger than screen).\n", __func__);
> 
> Shouldn't this be:
> 		dev_info(info->dev, "...");
> instead?
> 
> It's a driver, and you have access to the struct device that is being
> worked on, so always try to use the dev_* versions of these calls
> instead when ever possible.

Ah, I see. I'll fix them in v2, thank you!

Peilin