[PATCH] video: fbdev: s3c-fb: Mark expected switch fall-throughs

[PATCH] video: fbdev: s3c-fb: Mark expected switch fall-throughs

[Gustavo A. R. Silva]

In preparation to enabling -Wimplicit-fallthrough, mark switch
cases where we are expecting to fall through.

This patch fixes the following warnings:

drivers/video/fbdev/s3c-fb.c: In function ‘s3c_fb_blank’:
drivers/video/fbdev/s3c-fb.c:811:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
   sfb->enabled &= ~(1 << index);
   ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
drivers/video/fbdev/s3c-fb.c:814:2: note: here
  case FB_BLANK_NORMAL:
  ^~~~
  LD [M]  drivers/staging/greybus/gb-light.o
  CC [M]  drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gp10b.o
drivers/video/fbdev/s3c-fb.c: In function ‘s3c_fb_check_var’:
drivers/video/fbdev/s3c-fb.c:286:22: warning: this statement may fall through [-Wimplicit-fallthrough=]
   var->transp.length = 1;
   ~~~~~~~~~~~~~~~~~~~^~~
drivers/video/fbdev/s3c-fb.c:288:2: note: here
  case 18:
  ^~~~
drivers/video/fbdev/s3c-fb.c:314:22: warning: this statement may fall through [-Wimplicit-fallthrough=]
   var->transp.offset = 24;
   ~~~~~~~~~~~~~~~~~~~^~~~
drivers/video/fbdev/s3c-fb.c:316:2: note: here
  case 24:
  ^~~~

Warning level 3 was used: -Wimplicit-fallthrough=3

Notice that, in this particular case, the code comments are modified
in accordance with what GCC is expecting to find.

This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/video/fbdev/s3c-fb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/s3c-fb.c b/drivers/video/fbdev/s3c-fb.c
index 288300035164..f0b0643ab195 100644
--- a/drivers/video/fbdev/s3c-fb.c
+++ b/drivers/video/fbdev/s3c-fb.c
@@ -284,7 +284,7 @@ static int s3c_fb_check_var(struct fb_var_screeninfo *var,
 		/* 666 with one bit alpha/transparency */
 		var->transp.offset	= 18;
 		var->transp.length	= 1;
-		/* drop through */
+		/* fall through */
 	case 18:
 		var->bits_per_pixel	= 32;
 
@@ -312,7 +312,7 @@ static int s3c_fb_check_var(struct fb_var_screeninfo *var,
 	case 25:
 		var->transp.length	= var->bits_per_pixel - 24;
 		var->transp.offset	= 24;
-		/* drop through */
+		/* fall through */
 	case 24:
 		/* our 24bpp is unpacked, so 32bpp */
 		var->bits_per_pixel	= 32;
@@ -809,7 +809,7 @@ static int s3c_fb_blank(int blank_mode, struct fb_info *info)
 	case FB_BLANK_POWERDOWN:
 		wincon &= ~WINCONx_ENWIN;
 		sfb->enabled &= ~(1 << index);
-		/* fall through to FB_BLANK_NORMAL */
+		/* fall through - to FB_BLANK_NORMAL */
 
 	case FB_BLANK_NORMAL:
 		/* disable the DMA and display 0x0 (black) */
-- 
2.21.0

Re: [PATCH] video: fbdev: s3c-fb: Mark expected switch fall-throughs

[Joe Perches]

On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch
> cases where we are expecting to fall through.
[]
> This patch is part of the ongoing efforts to enable
> -Wimplicit-fallthrough.

Just enable the thing already.

If you stopped trying to do it all yourself, others
will help resolve any new build warnings.

For instance: a build of -next x86/64 defconfig has 2.  nbd.
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 5102b2bbd224..df909ffdfcdb 100644
--- a/Makefile
+++ b/Makefile
@@ -690,6 +690,7 @@ endif # may-sync-config
 endif # $(dot-config)
 
 KBUILD_CFLAGS	+= $(call cc-option,-fno-delete-null-pointer-checks,)
+KBUILD_CFLAGS	+= $(call cc-option, -Wimplicit-fallthrough)
 KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
 KBUILD_CFLAGS	+= $(call cc-disable-warning, format-truncation)
 KBUILD_CFLAGS	+= $(call cc-disable-warning, format-overflow)

Re: [PATCH] video: fbdev: s3c-fb: Mark expected switch fall-throughs

[Gustavo A. R. Silva]

On 6/25/19 11:52 AM, Joe Perches wrote:
> On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote:
>> In preparation to enabling -Wimplicit-fallthrough, mark switch
>> cases where we are expecting to fall through.
> []
>> This patch is part of the ongoing efforts to enable
>> -Wimplicit-fallthrough.
> 
> Just enable the thing already.
> 
> If you stopped trying to do it all yourself, others

What are you talking about?

Anyone can enable it, I'm adding this to every commit:

Warning level 3 was used: -Wimplicit-fallthrough=3

And I'll send a PR with a proper patch for the Makefile
during the next merge window.

If had the power I would have enabled this option since day 1,
so every developer can take care of their own code.

Lately, you are not being of much help, Joe.

--
Gustavo

Re: [PATCH] video: fbdev: s3c-fb: Mark expected switch fall-throughs

[Kees Cook]

On Tue, Jun 25, 2019 at 09:52:23AM -0700, Joe Perches wrote:
> On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote:
> > In preparation to enabling -Wimplicit-fallthrough, mark switch
> > cases where we are expecting to fall through.
> []
> > This patch is part of the ongoing efforts to enable
> > -Wimplicit-fallthrough.
> 
> Just enable the thing already.

Linus has been pretty clear about not wanting warning options enabled
without first fixing all the cases it warns about first. If that's
changed, then sure, let's do it; but we went through a lot of coordination
with sfr for linux-next nag-mail coverage (where maintainers are now
fixing it themselves) when this came up during the last kernel summit,
and this approach was the agreed solution.

-- 
Kees Cook

Re: [PATCH] video: fbdev: s3c-fb: Mark expected switch fall-throughs

[Joe Perches]

On Tue, 2019-06-25 at 12:06 -0500, Gustavo A. R. Silva wrote:
> 
> On 6/25/19 11:52 AM, Joe Perches wrote:
> > On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote:
> > > In preparation to enabling -Wimplicit-fallthrough, mark switch
> > > cases where we are expecting to fall through.
> > []
> > > This patch is part of the ongoing efforts to enable
> > > -Wimplicit-fallthrough.
> > 
> > Just enable the thing already.
> > 
> > If you stopped trying to do it all yourself, others
> 
> What are you talking about?
> 
> Anyone can enable it, I'm adding this to every commit:
> 
> Warning level 3 was used: -Wimplicit-fallthrough=3

No one does that by default and almost no one
is helping eliminate these.  Not even on th

Almost no one uses make W=<levels> either as it's
generally extremely noisy and can emit a lot of
false positives.

> And I'll send a PR with a proper patch for the Makefile
> during the next merge window.

That's great.

> If had the power I would have enabled this option since day 1,
> so every developer can take care of their own code.

You have always had the power to send a patch.

You also seem to believe the build needs to be
completely clean before enabling the switch.

I don't.

> Lately, you are not being of much help, Joe.

<smile> What kind of help are you expecting?

I'm not submitting patches adding fallthough comments
as I think that's not a good form.  I've said so
repeatedly.

I believe I suggested months ago you default enable
the compiler switch.

So it's up to you to either do it or not.

Re: [PATCH] video: fbdev: s3c-fb: Mark expected switch fall-throughs

[Joe Perches]

On Tue, 2019-06-25 at 10:31 -0700, Kees Cook wrote:
> On Tue, Jun 25, 2019 at 09:52:23AM -0700, Joe Perches wrote:
> > On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote:
> > > In preparation to enabling -Wimplicit-fallthrough, mark switch
> > > cases where we are expecting to fall through.
> > []
> > > This patch is part of the ongoing efforts to enable
> > > -Wimplicit-fallthrough.
> > 
> > Just enable the thing already.
> 
> Linus has been pretty clear about not wanting warning options enabled
> without first fixing all the cases it warns about first.

Hey Kees.

I don't recall that particular tidbit.  Got a link?  

cheers, Joe

Re: [PATCH] video: fbdev: s3c-fb: Mark expected switch fall-throughs

[Kees Cook]

On Tue, Jun 25, 2019 at 10:49:01AM -0700, Joe Perches wrote:
> On Tue, 2019-06-25 at 10:31 -0700, Kees Cook wrote:
> > On Tue, Jun 25, 2019 at 09:52:23AM -0700, Joe Perches wrote:
> > > On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote:
> > > > In preparation to enabling -Wimplicit-fallthrough, mark switch
> > > > cases where we are expecting to fall through.
> > > []
> > > > This patch is part of the ongoing efforts to enable
> > > > -Wimplicit-fallthrough.
> > > 
> > > Just enable the thing already.
> > 
> > Linus has been pretty clear about not wanting warning options enabled
> > without first fixing all the cases it warns about first.
> 
> Hey Kees.
> 
> I don't recall that particular tidbit.  Got a link?  

It was spread out over the discussion around removing __deprecated,
about enabling -Wvla, and in person at the kernel summit when asking
what approach to take for -Wimplicit-fallthrough.

-- 
Kees Cook

Re: [PATCH] video: fbdev: s3c-fb: Mark expected switch fall-throughs

[Bartlomiej Zolnierkiewicz]

On 6/25/19 6:01 PM, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch
> cases where we are expecting to fall through.
> 
> This patch fixes the following warnings:
> 
> drivers/video/fbdev/s3c-fb.c: In function ‘s3c_fb_blank’:
> drivers/video/fbdev/s3c-fb.c:811:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
>    sfb->enabled &= ~(1 << index);
>    ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
> drivers/video/fbdev/s3c-fb.c:814:2: note: here
>   case FB_BLANK_NORMAL:
>   ^~~~
>   LD [M]  drivers/staging/greybus/gb-light.o
>   CC [M]  drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gp10b.o
> drivers/video/fbdev/s3c-fb.c: In function ‘s3c_fb_check_var’:
> drivers/video/fbdev/s3c-fb.c:286:22: warning: this statement may fall through [-Wimplicit-fallthrough=]
>    var->transp.length = 1;
>    ~~~~~~~~~~~~~~~~~~~^~~
> drivers/video/fbdev/s3c-fb.c:288:2: note: here
>   case 18:
>   ^~~~
> drivers/video/fbdev/s3c-fb.c:314:22: warning: this statement may fall through [-Wimplicit-fallthrough=]
>    var->transp.offset = 24;
>    ~~~~~~~~~~~~~~~~~~~^~~~
> drivers/video/fbdev/s3c-fb.c:316:2: note: here
>   case 24:
>   ^~~~
> 
> Warning level 3 was used: -Wimplicit-fallthrough=3
> 
> Notice that, in this particular case, the code comments are modified
> in accordance with what GCC is expecting to find.
> 
> This patch is part of the ongoing efforts to enable
> -Wimplicit-fallthrough.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Patch queued for v5.3, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics