| Message ID | 20201128224114.1033617-1-sam@ravnborg.org |
|---|---|
| Headers | show |
| Series | drivers/video: W=1 warning fixes | expand |
Am 28.11.20 um 23:40 schrieb Sam Ravnborg: > Replacing DPRINTK() statements with pr_debug fixes set but not used > warnings. And moves to a more standard logging setup at the same time. > > v2: > - Fix indent (Joe) > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Joe Perches <joe@perches.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Jiri Slaby <jirislaby@kernel.org> > Cc: Peilin Ye <yepeilin.cs@gmail.com> > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: George Kennedy <george.kennedy@oracle.com> > Cc: Nathan Chancellor <natechancellor@gmail.com> > Cc: Peter Rosin <peda@axentia.se> > --- > drivers/video/fbdev/core/fbcon.c | 25 ++++++++----------------- > 1 file changed, 8 insertions(+), 17 deletions(-) > > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > index bf61598bf1c3..44a5cd2f54cc 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -56,8 +56,6 @@ > * more details. > */ > > -#undef FBCONDEBUG > - I guess this was added for quick debugging during development. Anyway, I never liked these kinds of hacks. Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > #include <linux/module.h> > #include <linux/types.h> > #include <linux/fs.h> > @@ -82,12 +80,6 @@ > > #include "fbcon.h" > > -#ifdef FBCONDEBUG > -# define DPRINTK(fmt, args...) printk(KERN_DEBUG "%s: " fmt, __func__ , ## args) > -#else > -# define DPRINTK(fmt, args...) > -#endif > - > /* > * FIXME: Locking > * > @@ -1015,11 +1007,11 @@ static const char *fbcon_startup(void) > rows /= vc->vc_font.height; > vc_resize(vc, cols, rows); > > - DPRINTK("mode: %s\n", info->fix.id); > - DPRINTK("visual: %d\n", info->fix.visual); > - DPRINTK("res: %dx%d-%d\n", info->var.xres, > - info->var.yres, > - info->var.bits_per_pixel); > + pr_debug("mode: %s\n", info->fix.id); > + pr_debug("visual: %d\n", info->fix.visual); > + pr_debug("res: %dx%d-%d\n", info->var.xres, > + info->var.yres, > + info->var.bits_per_pixel); > > fbcon_add_cursor_timer(info); > return display_desc; > @@ -2013,7 +2005,7 @@ static int fbcon_resize(struct vc_data *vc, unsigned int width, > y_diff < 0 || y_diff > virt_fh) { > const struct fb_videomode *mode; > > - DPRINTK("attempting resize %ix%i\n", var.xres, var.yres); > + pr_debug("attempting resize %ix%i\n", var.xres, var.yres); > mode = fb_find_best_mode(&var, &info->modelist); > if (mode == NULL) > return -EINVAL; > @@ -2023,7 +2015,7 @@ static int fbcon_resize(struct vc_data *vc, unsigned int width, > if (virt_w > var.xres/virt_fw || virt_h > var.yres/virt_fh) > return -EINVAL; > > - DPRINTK("resize now %ix%i\n", var.xres, var.yres); > + pr_debug("resize now %ix%i\n", var.xres, var.yres); > if (con_is_visible(vc)) { > var.activate = FB_ACTIVATE_NOW | > FB_ACTIVATE_FORCE; > @@ -3299,8 +3291,7 @@ static void fbcon_exit(void) > > if (info->queue.func) > pending = cancel_work_sync(&info->queue); > - DPRINTK("fbcon: %s pending work\n", (pending ? "canceled" : > - "no")); > + pr_debug("fbcon: %s pending work\n", (pending ? "canceled" : "no")); > > for (j = first_fb_vc; j <= last_fb_vc; j++) { > if (con2fb_map[j] == i) { >
On 2020/11/29 19:03, Thomas Zimmermann wrote: > Am 28.11.20 um 23:40 schrieb Sam Ravnborg: >> Replacing DPRINTK() statements with pr_debug fixes set but not used >> warnings. And moves to a more standard logging setup at the same time. > > I guess this was added for quick debugging during development. Anyway, I never liked these kinds of hacks. > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > But replacing printk(KERN_DEBUG) with pr_debug() prevents __func__ from being printed when FBCONDEBUG is defined. Is such change what the author of this module expects?
On Sun, Nov 29, 2020 at 11:03:25AM +0100, Thomas Zimmermann wrote: > > > Am 28.11.20 um 23:40 schrieb Sam Ravnborg: > > Replacing DPRINTK() statements with pr_debug fixes set but not used > > warnings. And moves to a more standard logging setup at the same time. > > > > v2: > > - Fix indent (Joe) > > > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > > Cc: Joe Perches <joe@perches.com> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > Cc: Sam Ravnborg <sam@ravnborg.org> > > Cc: Jiri Slaby <jirislaby@kernel.org> > > Cc: Peilin Ye <yepeilin.cs@gmail.com> > > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > Cc: George Kennedy <george.kennedy@oracle.com> > > Cc: Nathan Chancellor <natechancellor@gmail.com> > > Cc: Peter Rosin <peda@axentia.se> > > --- > > drivers/video/fbdev/core/fbcon.c | 25 ++++++++----------------- > > 1 file changed, 8 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > > index bf61598bf1c3..44a5cd2f54cc 100644 > > --- a/drivers/video/fbdev/core/fbcon.c > > +++ b/drivers/video/fbdev/core/fbcon.c > > @@ -56,8 +56,6 @@ > > * more details. > > */ > > -#undef FBCONDEBUG > > - > > I guess this was added for quick debugging during development. Anyway, I > never liked these kinds of hacks. > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Thanks, applied to drm-misc-next. Sam
Hi Peilin, On Mon, Nov 30, 2020 at 01:38:05AM -0500, Peilin Ye wrote: > Hi Sam, > > On Sun, Nov 29, 2020 at 12:18:36PM +0100, Sam Ravnborg wrote: > > On Sun, Nov 29, 2020 at 07:28:08PM +0900, Tetsuo Handa wrote: > > > But replacing printk(KERN_DEBUG) with pr_debug() prevents __func__ from being printed > > > when FBCONDEBUG is defined. Is such change what the author of this module expects? > > > > When someone goes and enable DEBUG for fbcon they are also able to > > recognize the logging, so the printing of the function name is redundant > > in this case. > > > > There is likely limited to no use for these few logging entries, but if > > they should be dropped then I expect Peilin Ye to do so as he is the > > only one doing active maintenance of fbcon lately. > > Sure, I will take another look at them. Also sorry for the delay in that > printk() -> dev_*() patch you suggested, overwhelmed by some other > things this week. Sometimes fbcon.c accesses dev structs in a pretty > weird way (e.g. registered_fb[con2fb_map[vc->vc_num]]->dev), I will get > back to it when I understand this better. Please just keep up the good work cleaning up fbcon and related stuff. This is an area that needs some love and care and there is work for many long nights yet to do. Sam
On Mon, Nov 30, 2020 at 08:56:45AM +0100, Sam Ravnborg wrote: > Please just keep up the good work cleaning up fbcon and related stuff. > This is an area that needs some love and care and there is work for many > long nights yet to do. Thanks! I will see what I can do, Peilin Ye
Following the great work of Lee Jones in other subsystems here is a set of patches that address all remaining W=1 warnings in drivers/video/. Lee Jones already fixed all warnings in video/backlight/ so this is mostly fbdev related fixes. The general approach used were: - Fix kernel-doc, this is often very trivial - Drop unused local variables - Use no_printk for local logging support Build tested on a set of architectures with various configs. The patches do not depends on each other and in most cases all fixes for one driver is kept in a single patch. The individual changes are trivial so this is a great starter task to try to review these patches. A timely Reviewed-by: or Acked-by: would be very nice so we can get the warnings fixes before we cut for the merge window. v2: - Updated subject of the patches to tell what was fixed (Lee) - Fixed build error in one patch (kernel test robot) - A few editorials updates to the changelog messages Sam Sam Ravnborg (28): video: Fix kernel-doc warnings in of_display_timing + of_videomode video: fbcon: Fix warnings by using pr_debug() in fbcon video: fbdev: core: Fix kernel-doc warnings in fbmon + fb_notify video: fbdev: aty: Delete unused variable in radeon_monitor video: fbdev: aty: Fix set but not used warnings video: fbdev: aty: Fix set but not used warnings in mach64_ct video: fbdev: sis: Fix defined but not used warnings video: fbdev: sis: Fix defined but not used warning of SiS_TVDelay video: fbdev: sis: Fix set but not used warnings in init.c video: fbdev: sis: Fix set but not used warnings in sis_main video: fbdev: via: Fix set but not used warning for mode_crt_table video: fbdev: tdfx: Fix set but not used warning in att_outb() video: fbdev: riva: Fix kernel-doc and set but not used warnings video: fbdev: pm2fb: Fix kernel-doc warnings video: fbdev: neofb: Fix set but not used warning for CursorMem video: fbdev: hgafb: Fix kernel-doc warnings video: fbdev: tgafb: Fix kernel-doc and set but not used warnings video: fbdev: mx3fb: Fix kernel-doc, set but not used and string warnings video: fbdev: sstfb: Updated logging to fix set but not used warnings video: fbdev: nvidia: Fix set but not used warnings video: fbdev: tmiofb: Fix set but not used warnings video: fbdev: omapfb: Fix set but not used warnings in dsi video: fbdev: omapfb: Fix set but not used warnings in hdmi*_core video: fbdev: s3c-fb: Fix kernel-doc and set but not used warnings video: fbdev: uvesafb: Fix set but not used warning video: fbdev: uvesafb: Fix string related warnings video: fbdev: cirrusfb: Fix kernel-doc and set but not used warnings video: fbdev: s1d13xxxfb: Fix kernel-doc and set but not used warnings drivers/video/fbdev/aty/atyfb_base.c | 11 +++----- drivers/video/fbdev/aty/mach64_ct.c | 15 ++++++---- drivers/video/fbdev/aty/radeon_monitor.c | 4 +-- drivers/video/fbdev/cirrusfb.c | 20 ++++++------- drivers/video/fbdev/core/fb_notify.c | 3 +- drivers/video/fbdev/core/fbcon.c | 25 ++++++----------- drivers/video/fbdev/core/fbmon.c | 2 +- drivers/video/fbdev/hgafb.c | 4 +-- drivers/video/fbdev/mx3fb.c | 13 +++++---- drivers/video/fbdev/neofb.c | 4 --- drivers/video/fbdev/nvidia/nv_setup.c | 7 ++--- drivers/video/fbdev/omap2/omapfb/dss/dsi.c | 12 ++------ drivers/video/fbdev/omap2/omapfb/dss/hdmi4_core.c | 4 +-- drivers/video/fbdev/omap2/omapfb/dss/hdmi5_core.c | 4 +-- drivers/video/fbdev/pm2fb.c | 8 +++--- drivers/video/fbdev/riva/fbdev.c | 9 +++--- drivers/video/fbdev/riva/riva_hw.c | 28 ++++++------------- drivers/video/fbdev/s1d13xxxfb.c | 3 +- drivers/video/fbdev/s3c-fb.c | 11 ++++---- drivers/video/fbdev/sis/init.c | 34 ++++------------------- drivers/video/fbdev/sis/oem310.h | 2 ++ drivers/video/fbdev/sis/sis.h | 1 - drivers/video/fbdev/sis/sis_main.c | 9 +++--- drivers/video/fbdev/sstfb.c | 2 +- drivers/video/fbdev/tdfxfb.c | 4 +-- drivers/video/fbdev/tgafb.c | 7 ++--- drivers/video/fbdev/tmiofb.c | 6 ++-- drivers/video/fbdev/uvesafb.c | 8 +++--- drivers/video/fbdev/via/lcd.c | 4 +-- drivers/video/of_display_timing.c | 1 + drivers/video/of_videomode.c | 8 +++--- include/video/sstfb.h | 4 +-- 32 files changed, 107 insertions(+), 170 deletions(-)