| Message ID | 20201128224114.1033617-14-sam@ravnborg.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | drivers/video: W=1 warning fixes | expand |
Am 28.11.20 um 23:40 schrieb Sam Ravnborg: > Fix W=1 warnings: > - Fix kernel-doc > - Drop unused variables/code > > v2: > - Updated subject (Lee) > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Antonino Daplas <adaplas@gmail.com> > Cc: linux-fbdev@vger.kernel.org > Cc: Lee Jones <lee.jones@linaro.org> > --- > drivers/video/fbdev/riva/fbdev.c | 9 ++++----- > drivers/video/fbdev/riva/riva_hw.c | 28 ++++++++-------------------- > 2 files changed, 12 insertions(+), 25 deletions(-) > > diff --git a/drivers/video/fbdev/riva/fbdev.c b/drivers/video/fbdev/riva/fbdev.c > index ce55b9d2e862..55554b0433cb 100644 > --- a/drivers/video/fbdev/riva/fbdev.c > +++ b/drivers/video/fbdev/riva/fbdev.c > @@ -464,7 +464,7 @@ static inline void reverse_order(u32 *l) > > /** > * rivafb_load_cursor_image - load cursor image to hardware > - * @data: address to monochrome bitmap (1 = foreground color, 0 = background) > + * @data8: address to monochrome bitmap (1 = foreground color, 0 = background) > * @par: pointer to private data > * @w: width of cursor image in pixels > * @h: height of cursor image in scanlines > @@ -843,9 +843,9 @@ static void riva_update_var(struct fb_var_screeninfo *var, > /** > * rivafb_do_maximize - > * @info: pointer to fb_info object containing info for current riva board > - * @var: > - * @nom: > - * @den: > + * @var: standard kernel fb changeable data > + * @nom: nom > + * @den: den Well, it fixes the warning ;) Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > * > * DESCRIPTION: > * . > @@ -1214,7 +1214,6 @@ static int rivafb_set_par(struct fb_info *info) > /** > * rivafb_pan_display > * @var: standard kernel fb changeable data > - * @con: TODO > * @info: pointer to fb_info object containing info for current riva board > * > * DESCRIPTION: > diff --git a/drivers/video/fbdev/riva/riva_hw.c b/drivers/video/fbdev/riva/riva_hw.c > index bcf9c4b4de31..8b829b720064 100644 > --- a/drivers/video/fbdev/riva/riva_hw.c > +++ b/drivers/video/fbdev/riva/riva_hw.c > @@ -836,17 +836,17 @@ static void nv10CalcArbitration > nv10_sim_state *arb > ) > { > - int data, pagemiss, cas,width, video_enable, bpp; > - int nvclks, mclks, pclks, vpagemiss, crtpagemiss, vbs; > - int nvclk_fill, us_extra; > + int data, pagemiss, width, video_enable, bpp; > + int nvclks, mclks, pclks, vpagemiss, crtpagemiss; > + int nvclk_fill; > int found, mclk_extra, mclk_loop, cbs, m1; > int mclk_freq, pclk_freq, nvclk_freq, mp_enable; > - int us_m, us_m_min, us_n, us_p, video_drain_rate, crtc_drain_rate; > - int vus_m, vus_n, vus_p; > - int vpm_us, us_video, vlwm, cpm_us, us_crt,clwm; > + int us_m, us_m_min, us_n, us_p, crtc_drain_rate; > + int vus_m; > + int vpm_us, us_video, cpm_us, us_crt,clwm; > int clwm_rnd_down; > - int craw, m2us, us_pipe, us_pipe_min, vus_pipe, p1clk, p2; > - int pclks_2_top_fifo, min_mclk_extra; > + int m2us, us_pipe_min, p1clk, p2; > + int min_mclk_extra; > int us_min_mclk_extra; > > fifo->valid = 1; > @@ -854,16 +854,13 @@ static void nv10CalcArbitration > mclk_freq = arb->mclk_khz; > nvclk_freq = arb->nvclk_khz; > pagemiss = arb->mem_page_miss; > - cas = arb->mem_latency; > width = arb->memory_width/64; > video_enable = arb->enable_video; > bpp = arb->pix_bpp; > mp_enable = arb->enable_mp; > clwm = 0; > - vlwm = 1024; > > cbs = 512; > - vbs = 512; > > pclks = 4; /* lwm detect. */ > > @@ -924,17 +921,11 @@ static void nv10CalcArbitration > us_min_mclk_extra = min_mclk_extra *1000*1000 / mclk_freq; > us_n = nvclks*1000*1000 / nvclk_freq;/* nvclk latency in us */ > us_p = pclks*1000*1000 / pclk_freq;/* nvclk latency in us */ > - us_pipe = us_m + us_n + us_p; > us_pipe_min = us_m_min + us_n + us_p; > - us_extra = 0; > > vus_m = mclk_loop *1000*1000 / mclk_freq; /* Mclk latency in us */ > - vus_n = (4)*1000*1000 / nvclk_freq;/* nvclk latency in us */ > - vus_p = 0*1000*1000 / pclk_freq;/* pclk latency in us */ > - vus_pipe = vus_m + vus_n + vus_p; > > if(video_enable) { > - video_drain_rate = pclk_freq * 4; /* MB/s */ > crtc_drain_rate = pclk_freq * bpp/8; /* MB/s */ > > vpagemiss = 1; /* self generating page miss */ > @@ -993,7 +984,6 @@ static void nv10CalcArbitration > else if(crtc_drain_rate * 100 >= nvclk_fill * 98) { > clwm = 1024; > cbs = 512; > - us_extra = (cbs * 1000 * 1000)/ (8*width)/mclk_freq ; > } > } > } > @@ -1010,7 +1000,6 @@ static void nv10CalcArbitration > > m1 = clwm + cbs - 1024; /* Amount of overfill */ > m2us = us_pipe_min + us_min_mclk_extra; > - pclks_2_top_fifo = (1024-clwm)/(8*width); > > /* pclk cycles to drain */ > p1clk = m2us * pclk_freq/(1000*1000); > @@ -1038,7 +1027,6 @@ static void nv10CalcArbitration > min_mclk_extra--; > } > } > - craw = clwm; > > if(clwm < (1024-cbs+8)) clwm = 1024-cbs+8; > data = (int)(clwm); >
On Mon, Nov 30, 2020 at 01:14:52PM +0100, Thomas Zimmermann wrote: > > > Am 28.11.20 um 23:40 schrieb Sam Ravnborg: > > Fix W=1 warnings: > > - Fix kernel-doc > > - Drop unused variables/code > > > > v2: > > - Updated subject (Lee) > > > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > > Cc: Antonino Daplas <adaplas@gmail.com> > > Cc: linux-fbdev@vger.kernel.org > > Cc: Lee Jones <lee.jones@linaro.org> > > --- > > drivers/video/fbdev/riva/fbdev.c | 9 ++++----- > > drivers/video/fbdev/riva/riva_hw.c | 28 ++++++++-------------------- > > 2 files changed, 12 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/video/fbdev/riva/fbdev.c b/drivers/video/fbdev/riva/fbdev.c > > index ce55b9d2e862..55554b0433cb 100644 > > --- a/drivers/video/fbdev/riva/fbdev.c > > +++ b/drivers/video/fbdev/riva/fbdev.c > > @@ -464,7 +464,7 @@ static inline void reverse_order(u32 *l) > > /** > > * rivafb_load_cursor_image - load cursor image to hardware > > - * @data: address to monochrome bitmap (1 = foreground color, 0 = background) > > + * @data8: address to monochrome bitmap (1 = foreground color, 0 = background) > > * @par: pointer to private data > > * @w: width of cursor image in pixels > > * @h: height of cursor image in scanlines > > @@ -843,9 +843,9 @@ static void riva_update_var(struct fb_var_screeninfo *var, > > /** > > * rivafb_do_maximize - > > * @info: pointer to fb_info object containing info for current riva board > > - * @var: > > - * @nom: > > - * @den: > > + * @var: standard kernel fb changeable data > > + * @nom: nom > > + * @den: den > > Well, it fixes the warning ;) Yeah, I could not dig up anything useful to say here. Was tempted to just drop all the kernel-doc syntax but that was a larger change. Sam
On Mon, 30 Nov 2020, Sam Ravnborg wrote: > On Mon, Nov 30, 2020 at 01:14:52PM +0100, Thomas Zimmermann wrote: > > > > > > Am 28.11.20 um 23:40 schrieb Sam Ravnborg: > > > Fix W=1 warnings: > > > - Fix kernel-doc > > > - Drop unused variables/code > > > > > > v2: > > > - Updated subject (Lee) > > > > > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > > > Cc: Antonino Daplas <adaplas@gmail.com> > > > Cc: linux-fbdev@vger.kernel.org > > > Cc: Lee Jones <lee.jones@linaro.org> > > > --- > > > drivers/video/fbdev/riva/fbdev.c | 9 ++++----- > > > drivers/video/fbdev/riva/riva_hw.c | 28 ++++++++-------------------- > > > 2 files changed, 12 insertions(+), 25 deletions(-) > > > > > > diff --git a/drivers/video/fbdev/riva/fbdev.c b/drivers/video/fbdev/riva/fbdev.c > > > index ce55b9d2e862..55554b0433cb 100644 > > > --- a/drivers/video/fbdev/riva/fbdev.c > > > +++ b/drivers/video/fbdev/riva/fbdev.c > > > @@ -464,7 +464,7 @@ static inline void reverse_order(u32 *l) > > > /** > > > * rivafb_load_cursor_image - load cursor image to hardware > > > - * @data: address to monochrome bitmap (1 = foreground color, 0 = background) > > > + * @data8: address to monochrome bitmap (1 = foreground color, 0 = background) > > > * @par: pointer to private data > > > * @w: width of cursor image in pixels > > > * @h: height of cursor image in scanlines > > > @@ -843,9 +843,9 @@ static void riva_update_var(struct fb_var_screeninfo *var, > > > /** > > > * rivafb_do_maximize - > > > * @info: pointer to fb_info object containing info for current riva board > > > - * @var: > > > - * @nom: > > > - * @den: > > > + * @var: standard kernel fb changeable data > > > + * @nom: nom > > > + * @den: den Cop-out! Do what I do and make something up (joke)! :'D > > Well, it fixes the warning ;) > > Yeah, I could not dig up anything useful to say here. > Was tempted to just drop all the kernel-doc syntax but that > was a larger change. Did you trace it from it's origin down to it's final use?
Hi Lee, > > Cop-out! > > Do what I do and make something up (joke)! :'D If I thought anyone would actually read the comments then maybe yes. But I assume that apart from this thread no-one will read it. > > > > Well, it fixes the warning ;) > > > > Yeah, I could not dig up anything useful to say here. > > Was tempted to just drop all the kernel-doc syntax but that > > was a larger change. > > Did you trace it from it's origin down to it's final use? Yeah, but not something that seemed useful. I could have added "translating from pixels->bytes" as they are described somewhere else. But I could not convince myself this was right so I just silenced the warning. The only reason I kept the kernel-doc in the first place is that I am told some editors use it. The only effect the kernel-doc in fbdev has right now is burning effort that could have been spent (better?) somewhere else, and I would personally prefer to drop the kernel-doc annotations. But I already discussed this in another thread (not fbdev related) and I was told it was useful for some, so it is kept. Sam
On Tue, 01 Dec 2020, Sam Ravnborg wrote: > Hi Lee, > > > > > Cop-out! > > > > Do what I do and make something up (joke)! :'D > > If I thought anyone would actually read the comments then maybe yes. > But I assume that apart from this thread no-one will read it. > > > > > > > Well, it fixes the warning ;) > > > > > > Yeah, I could not dig up anything useful to say here. > > > Was tempted to just drop all the kernel-doc syntax but that > > > was a larger change. > > > > Did you trace it from it's origin down to it's final use? > Yeah, but not something that seemed useful. > I could have added "translating from pixels->bytes" as they > are described somewhere else. But I could not convince myself > this was right so I just silenced the warning. > > The only reason I kept the kernel-doc in the first place is > that I am told some editors use it. > > The only effect the kernel-doc in fbdev has right now is burning > effort that could have been spent (better?) somewhere else, and > I would personally prefer to drop the kernel-doc annotations. > > But I already discussed this in another thread (not fbdev related) > and I was told it was useful for some, so it is kept. I personally think they should be kept. Despite not being referenced by any kernel-doc:: key-words. As you say, it can be helpful as an in-code reference for driver writers, people debugging code, et al. Not sure I would just repeat the variable name just to silence the warning though - that is definitely a hack. In the thousands (literally!) of these that I've fixed thus far, I haven't needed to do that.
On Tue, Dec 1, 2020 at 10:46 AM Lee Jones <lee.jones@linaro.org> wrote: > > On Tue, 01 Dec 2020, Sam Ravnborg wrote: > > > Hi Lee, > > > > > > > > Cop-out! > > > > > > Do what I do and make something up (joke)! :'D > > > > If I thought anyone would actually read the comments then maybe yes. > > But I assume that apart from this thread no-one will read it. > > > > > > > > > > Well, it fixes the warning ;) > > > > > > > > Yeah, I could not dig up anything useful to say here. > > > > Was tempted to just drop all the kernel-doc syntax but that > > > > was a larger change. > > > > > > Did you trace it from it's origin down to it's final use? > > Yeah, but not something that seemed useful. > > I could have added "translating from pixels->bytes" as they > > are described somewhere else. But I could not convince myself > > this was right so I just silenced the warning. > > > > The only reason I kept the kernel-doc in the first place is > > that I am told some editors use it. > > > > The only effect the kernel-doc in fbdev has right now is burning > > effort that could have been spent (better?) somewhere else, and > > I would personally prefer to drop the kernel-doc annotations. > > > > But I already discussed this in another thread (not fbdev related) > > and I was told it was useful for some, so it is kept. > > I personally think they should be kept. Despite not being referenced > by any kernel-doc:: key-words. As you say, it can be helpful as an > in-code reference for driver writers, people debugging code, et al. > > Not sure I would just repeat the variable name just to silence the > warning though - that is definitely a hack. In the thousands > (literally!) of these that I've fixed thus far, I haven't needed to do > that. Personally what I've done is to just remove the kerneldoc marker (and anything else that's obviously wrong) and leave plain comments behind. At least for old outdated code that no one actively maintains anymore. Keeps the comment as maybe something useful, and avoids pointless busy work of inventing kerneldoc which might or might not actually be correctly describing what's going on. -Daniel
On Tue, 01 Dec 2020, Daniel Vetter wrote: > On Tue, Dec 1, 2020 at 10:46 AM Lee Jones <lee.jones@linaro.org> wrote: > > > > On Tue, 01 Dec 2020, Sam Ravnborg wrote: > > > > > Hi Lee, > > > > > > > > > > > Cop-out! > > > > > > > > Do what I do and make something up (joke)! :'D > > > > > > If I thought anyone would actually read the comments then maybe yes. > > > But I assume that apart from this thread no-one will read it. > > > > > > > > > > > > > Well, it fixes the warning ;) > > > > > > > > > > Yeah, I could not dig up anything useful to say here. > > > > > Was tempted to just drop all the kernel-doc syntax but that > > > > > was a larger change. > > > > > > > > Did you trace it from it's origin down to it's final use? > > > Yeah, but not something that seemed useful. > > > I could have added "translating from pixels->bytes" as they > > > are described somewhere else. But I could not convince myself > > > this was right so I just silenced the warning. > > > > > > The only reason I kept the kernel-doc in the first place is > > > that I am told some editors use it. > > > > > > The only effect the kernel-doc in fbdev has right now is burning > > > effort that could have been spent (better?) somewhere else, and > > > I would personally prefer to drop the kernel-doc annotations. > > > > > > But I already discussed this in another thread (not fbdev related) > > > and I was told it was useful for some, so it is kept. > > > > I personally think they should be kept. Despite not being referenced > > by any kernel-doc:: key-words. As you say, it can be helpful as an > > in-code reference for driver writers, people debugging code, et al. > > > > Not sure I would just repeat the variable name just to silence the > > warning though - that is definitely a hack. In the thousands > > (literally!) of these that I've fixed thus far, I haven't needed to do > > that. > > Personally what I've done is to just remove the kerneldoc marker (and > anything else that's obviously wrong) and leave plain comments behind. > At least for old outdated code that no one actively maintains anymore. > Keeps the comment as maybe something useful, and avoids pointless busy > work of inventing kerneldoc which might or might not actually be > correctly describing what's going on. Right. Demoting is also a good option if in doubt.
diff --git a/drivers/video/fbdev/riva/fbdev.c b/drivers/video/fbdev/riva/fbdev.c index ce55b9d2e862..55554b0433cb 100644 --- a/drivers/video/fbdev/riva/fbdev.c +++ b/drivers/video/fbdev/riva/fbdev.c @@ -464,7 +464,7 @@ static inline void reverse_order(u32 *l) /** * rivafb_load_cursor_image - load cursor image to hardware - * @data: address to monochrome bitmap (1 = foreground color, 0 = background) + * @data8: address to monochrome bitmap (1 = foreground color, 0 = background) * @par: pointer to private data * @w: width of cursor image in pixels * @h: height of cursor image in scanlines @@ -843,9 +843,9 @@ static void riva_update_var(struct fb_var_screeninfo *var, /** * rivafb_do_maximize - * @info: pointer to fb_info object containing info for current riva board - * @var: - * @nom: - * @den: + * @var: standard kernel fb changeable data + * @nom: nom + * @den: den * * DESCRIPTION: * . @@ -1214,7 +1214,6 @@ static int rivafb_set_par(struct fb_info *info) /** * rivafb_pan_display * @var: standard kernel fb changeable data - * @con: TODO * @info: pointer to fb_info object containing info for current riva board * * DESCRIPTION: diff --git a/drivers/video/fbdev/riva/riva_hw.c b/drivers/video/fbdev/riva/riva_hw.c index bcf9c4b4de31..8b829b720064 100644 --- a/drivers/video/fbdev/riva/riva_hw.c +++ b/drivers/video/fbdev/riva/riva_hw.c @@ -836,17 +836,17 @@ static void nv10CalcArbitration nv10_sim_state *arb ) { - int data, pagemiss, cas,width, video_enable, bpp; - int nvclks, mclks, pclks, vpagemiss, crtpagemiss, vbs; - int nvclk_fill, us_extra; + int data, pagemiss, width, video_enable, bpp; + int nvclks, mclks, pclks, vpagemiss, crtpagemiss; + int nvclk_fill; int found, mclk_extra, mclk_loop, cbs, m1; int mclk_freq, pclk_freq, nvclk_freq, mp_enable; - int us_m, us_m_min, us_n, us_p, video_drain_rate, crtc_drain_rate; - int vus_m, vus_n, vus_p; - int vpm_us, us_video, vlwm, cpm_us, us_crt,clwm; + int us_m, us_m_min, us_n, us_p, crtc_drain_rate; + int vus_m; + int vpm_us, us_video, cpm_us, us_crt,clwm; int clwm_rnd_down; - int craw, m2us, us_pipe, us_pipe_min, vus_pipe, p1clk, p2; - int pclks_2_top_fifo, min_mclk_extra; + int m2us, us_pipe_min, p1clk, p2; + int min_mclk_extra; int us_min_mclk_extra; fifo->valid = 1; @@ -854,16 +854,13 @@ static void nv10CalcArbitration mclk_freq = arb->mclk_khz; nvclk_freq = arb->nvclk_khz; pagemiss = arb->mem_page_miss; - cas = arb->mem_latency; width = arb->memory_width/64; video_enable = arb->enable_video; bpp = arb->pix_bpp; mp_enable = arb->enable_mp; clwm = 0; - vlwm = 1024; cbs = 512; - vbs = 512; pclks = 4; /* lwm detect. */ @@ -924,17 +921,11 @@ static void nv10CalcArbitration us_min_mclk_extra = min_mclk_extra *1000*1000 / mclk_freq; us_n = nvclks*1000*1000 / nvclk_freq;/* nvclk latency in us */ us_p = pclks*1000*1000 / pclk_freq;/* nvclk latency in us */ - us_pipe = us_m + us_n + us_p; us_pipe_min = us_m_min + us_n + us_p; - us_extra = 0; vus_m = mclk_loop *1000*1000 / mclk_freq; /* Mclk latency in us */ - vus_n = (4)*1000*1000 / nvclk_freq;/* nvclk latency in us */ - vus_p = 0*1000*1000 / pclk_freq;/* pclk latency in us */ - vus_pipe = vus_m + vus_n + vus_p; if(video_enable) { - video_drain_rate = pclk_freq * 4; /* MB/s */ crtc_drain_rate = pclk_freq * bpp/8; /* MB/s */ vpagemiss = 1; /* self generating page miss */ @@ -993,7 +984,6 @@ static void nv10CalcArbitration else if(crtc_drain_rate * 100 >= nvclk_fill * 98) { clwm = 1024; cbs = 512; - us_extra = (cbs * 1000 * 1000)/ (8*width)/mclk_freq ; } } } @@ -1010,7 +1000,6 @@ static void nv10CalcArbitration m1 = clwm + cbs - 1024; /* Amount of overfill */ m2us = us_pipe_min + us_min_mclk_extra; - pclks_2_top_fifo = (1024-clwm)/(8*width); /* pclk cycles to drain */ p1clk = m2us * pclk_freq/(1000*1000); @@ -1038,7 +1027,6 @@ static void nv10CalcArbitration min_mclk_extra--; } } - craw = clwm; if(clwm < (1024-cbs+8)) clwm = 1024-cbs+8; data = (int)(clwm);
Fix W=1 warnings: - Fix kernel-doc - Drop unused variables/code v2: - Updated subject (Lee) Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Antonino Daplas <adaplas@gmail.com> Cc: linux-fbdev@vger.kernel.org Cc: Lee Jones <lee.jones@linaro.org> --- drivers/video/fbdev/riva/fbdev.c | 9 ++++----- drivers/video/fbdev/riva/riva_hw.c | 28 ++++++++-------------------- 2 files changed, 12 insertions(+), 25 deletions(-)