[PATCH 0/9] mark expected switch fall-throughs

[PATCH 0/9] mark expected switch fall-throughs

[Gustavo A. R. Silva]

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

In Kees Cook words:
"This is an unfortunate omission in the C language, and thankfully both
gcc and clang have stepped up to solve this the same way static
analyzers have solved it. It does both document the intention for
humans and provide a way for analyzers to report issues.

Having the compiler help us not make mistakes is quite handy."

In some cases there were "else FALL THROUGH" or "then fallthrough..."
comments already in place. So I replaced them with proper "fall through"
comments, which is what GCC is expecting to find.

Thanks!

Gustavo A. R. Silva (9):
  usb: host: fotg210-hcd: mark expected switch fall-through
  usb: host: xhci: mark expected switch fall-through
  usb: host: xhci-mem: mark expected switch fall-through
  usb: host: ohci-hcd: mark expected switch fall-through
  usb: host: ehci-hcd: mark expected switch fall-through
  usb: host: isp1362-hcd: mark expected switch fall-through
  usb: host: oxu210hp-hcd: mark expected switch fall-through
  usb: host: xhci-hub: mark expected switch fall-through
  usb: host: pci-quirks: mark expected switch fall-through

 drivers/usb/host/ehci-hcd.c     | 2 +-
 drivers/usb/host/fotg210-hcd.c  | 2 +-
 drivers/usb/host/isp1362-hcd.c  | 1 +
 drivers/usb/host/ohci-hcd.c     | 2 +-
 drivers/usb/host/oxu210hp-hcd.c | 2 +-
 drivers/usb/host/pci-quirks.c   | 2 +-
 drivers/usb/host/xhci-hub.c     | 1 +
 drivers/usb/host/xhci-mem.c     | 1 +
 drivers/usb/host/xhci.c         | 1 +
 9 files changed, 9 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH 1/9] usb: host: fotg210-hcd: mark expected switch fall-through

[Gustavo A. R. Silva]

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

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/usb/host/fotg210-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 457cc65..33a4f7e 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -5449,7 +5449,7 @@ static void fotg210_endpoint_disable(struct usb_hcd *hcd,
 			qh_destroy(fotg210, qh);
 			break;
 		}
-		/* else FALL THROUGH */
+		/* fall through */
 	default:
 		/* caller was supposed to have unlinked any requests;
 		 * that's not our job.  just leak this memory.
-- 
2.7.4

[PATCH 2/9] usb: host: xhci: mark expected switch fall-through

[Gustavo A. R. Silva]

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

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/usb/host/xhci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ee077a2..05db6e97 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4294,6 +4294,7 @@ static unsigned long long xhci_calculate_intel_u1_timeout(
 			break;
 		}
 		/* Otherwise the calculation is the same as isoc eps */
+		/* fall through */
 	case USB_ENDPOINT_XFER_ISOC:
 		timeout_ns = xhci_service_interval_to_ns(desc);
 		timeout_ns = DIV_ROUND_UP_ULL(timeout_ns * 105, 100);
-- 
2.7.4

[PATCH 3/9] usb: host: xhci-mem: mark expected switch fall-through

[Gustavo A. R. Silva]

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

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/usb/host/xhci-mem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 795219a..57be885 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1311,6 +1311,7 @@ static unsigned int xhci_get_endpoint_interval(struct usb_device *udev,
 		 * since it uses the same rules as low speed interrupt
 		 * endpoints.
 		 */
+		/* fall through */
 
 	case USB_SPEED_LOW:
 		if (usb_endpoint_xfer_int(&ep->desc) ||
-- 
2.7.4

[PATCH 4/9] usb: host: ohci-hcd: mark expected switch fall-through

[Gustavo A. R. Silva]

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

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/usb/host/ohci-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 4492482..15ec8f9 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -382,7 +382,7 @@ ohci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep)
 			ed_free (ohci, ed);
 			break;
 		}
-		/* else FALL THROUGH */
+		/* fall through */
 	default:
 		/* caller was supposed to have unlinked any requests;
 		 * that's not our job.  can't recover; must leak ed.
-- 
2.7.4

[PATCH 5/9] usb: host: ehci-hcd: mark expected switch fall-through

[Gustavo A. R. Silva]

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

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/usb/host/ehci-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 6e834b83..c560a01 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1012,7 +1012,7 @@ ehci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep)
 			qh_destroy(ehci, qh);
 			break;
 		}
-		/* else FALL THROUGH */
+		/* fall through */
 	default:
 		/* caller was supposed to have unlinked any requests;
 		 * that's not our job.  just leak this memory.
-- 
2.7.4

[PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through

[Gustavo A. R. Silva]

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

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/usb/host/isp1362-hcd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c
index 9b7e307..753d576 100644
--- a/drivers/usb/host/isp1362-hcd.c
+++ b/drivers/usb/host/isp1362-hcd.c
@@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			spin_lock_irqsave(&isp1362_hcd->lock, flags);
 			isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC);
 			spin_unlock_irqrestore(&isp1362_hcd->lock, flags);
+			/* fall through */
 		case C_HUB_LOCAL_POWER:
 			DBG(0, "C_HUB_LOCAL_POWER\n");
 			break;
-- 
2.7.4

[PATCH 7/9] usb: host: oxu210hp-hcd: mark expected switch fall-through

[Gustavo A. R. Silva]

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

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/usb/host/oxu210hp-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index ed20fb3..9f8c61e 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -3040,7 +3040,7 @@ static void oxu_endpoint_disable(struct usb_hcd *hcd,
 			qh_put(qh);
 			break;
 		}
-		/* else FALL THROUGH */
+		/* fall through */
 	default:
 nogood:
 		/* caller was supposed to have unlinked any requests;
-- 
2.7.4

[PATCH 8/9] usb: host: xhci-hub: mark expected switch fall-through

[Gustavo A. R. Silva]

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

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/usb/host/xhci-hub.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 9762333..3693b1f 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1377,6 +1377,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			break;
 		case USB_PORT_FEAT_C_SUSPEND:
 			bus_state->port_c_suspend &= ~(1 << wIndex);
+			/* fall through */
 		case USB_PORT_FEAT_C_RESET:
 		case USB_PORT_FEAT_C_BH_PORT_RESET:
 		case USB_PORT_FEAT_C_CONNECTION:
-- 
2.7.4

[PATCH 9/9] usb: host: pci-quirks: mark expected switch fall-through

[Gustavo A. R. Silva]

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

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/usb/host/pci-quirks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 6dda362..6731f8d8d 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -841,7 +841,7 @@ static void quirk_usb_disable_ehci(struct pci_dev *pdev)
 			ehci_bios_handoff(pdev, op_reg_base, cap, offset);
 			break;
 		case 0: /* Illegal reserved cap, set cap=0 so we exit */
-			cap = 0; /* then fallthrough... */
+			cap = 0; /* fall through */
 		default:
 			dev_warn(&pdev->dev,
 				 "EHCI: unrecognized capability %02x\n",
-- 
2.7.4

Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through

[Gustavo A. R. Silva]

Greg,

Quoting "Gustavo A. R. Silva" <garsilva@embeddedor.com>:

> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
>  drivers/usb/host/isp1362-hcd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c
> index 9b7e307..753d576 100644
> --- a/drivers/usb/host/isp1362-hcd.c
> +++ b/drivers/usb/host/isp1362-hcd.c
> @@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd  
> *hcd, u16 typeReq, u16 wValue,
>  			spin_lock_irqsave(&isp1362_hcd->lock, flags);
>  			isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC);
>  			spin_unlock_irqrestore(&isp1362_hcd->lock, flags);
> +			/* fall through */

I'm suspicious this should be a 'break' instead.

What do you think?

>  		case C_HUB_LOCAL_POWER:
>  			DBG(0, "C_HUB_LOCAL_POWER\n");
>  			break;
> --
> 2.7.4

Thanks
--
Gustavo A. R. Silva

Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through

[Greg Kroah-Hartman]

On Wed, Oct 25, 2017 at 02:05:05PM -0500, Gustavo A. R. Silva wrote:
> Greg,
> 
> Quoting "Gustavo A. R. Silva" <garsilva@embeddedor.com>:
> 
> > In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> > where we are expecting to fall through.
> > 
> > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> > ---
> >  drivers/usb/host/isp1362-hcd.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c
> > index 9b7e307..753d576 100644
> > --- a/drivers/usb/host/isp1362-hcd.c
> > +++ b/drivers/usb/host/isp1362-hcd.c
> > @@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd
> > *hcd, u16 typeReq, u16 wValue,
> >  			spin_lock_irqsave(&isp1362_hcd->lock, flags);
> >  			isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC);
> >  			spin_unlock_irqrestore(&isp1362_hcd->lock, flags);
> > +			/* fall through */
> 
> I'm suspicious this should be a 'break' instead.
> 
> What do you think?

Yeah, this should be a 'break', care to make that patch up instead?

thanks,

greg k-h

Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through

[Gustavo A. R. Silva]

Quoting Greg Kroah-Hartman <gregkh@linuxfoundation.org>:

> On Wed, Oct 25, 2017 at 02:05:05PM -0500, Gustavo A. R. Silva wrote:
>> Greg,
>>
>> Quoting "Gustavo A. R. Silva" <garsilva@embeddedor.com>:
>>
>> > In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>> > where we are expecting to fall through.
>> >
>> > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> > ---
>> >  drivers/usb/host/isp1362-hcd.c | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/drivers/usb/host/isp1362-hcd.c  
>> b/drivers/usb/host/isp1362-hcd.c
>> > index 9b7e307..753d576 100644
>> > --- a/drivers/usb/host/isp1362-hcd.c
>> > +++ b/drivers/usb/host/isp1362-hcd.c
>> > @@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd
>> > *hcd, u16 typeReq, u16 wValue,
>> >  			spin_lock_irqsave(&isp1362_hcd->lock, flags);
>> >  			isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC);
>> >  			spin_unlock_irqrestore(&isp1362_hcd->lock, flags);
>> > +			/* fall through */
>>
>> I'm suspicious this should be a 'break' instead.
>>
>> What do you think?
>
> Yeah, this should be a 'break', care to make that patch up instead?
>

Sure thing.

Just some questions about the process to follow:

Should I send a v2 replying to this particular thread only? like  
[PATCH v2 6/9]
or should I send just a new patch separated from this patch series? I  
guess this is the case.

Some maintainers have told me that in cases where a particular patch  
in the series needs an update, the complete
patchset should be sent again. But I think that depends on the  
functional impact the patch has over the whole patchset.

Thanks
--
Gustavo A. R. Silva

Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through

[Greg Kroah-Hartman]

On Wed, Nov 01, 2017 at 12:27:40PM -0500, Gustavo A. R. Silva wrote:
> 
> Quoting Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> 
> > On Wed, Oct 25, 2017 at 02:05:05PM -0500, Gustavo A. R. Silva wrote:
> > > Greg,
> > > 
> > > Quoting "Gustavo A. R. Silva" <garsilva@embeddedor.com>:
> > > 
> > > > In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> > > > where we are expecting to fall through.
> > > >
> > > > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> > > > ---
> > > >  drivers/usb/host/isp1362-hcd.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/usb/host/isp1362-hcd.c
> > > b/drivers/usb/host/isp1362-hcd.c
> > > > index 9b7e307..753d576 100644
> > > > --- a/drivers/usb/host/isp1362-hcd.c
> > > > +++ b/drivers/usb/host/isp1362-hcd.c
> > > > @@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd
> > > > *hcd, u16 typeReq, u16 wValue,
> > > >  			spin_lock_irqsave(&isp1362_hcd->lock, flags);
> > > >  			isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC);
> > > >  			spin_unlock_irqrestore(&isp1362_hcd->lock, flags);
> > > > +			/* fall through */
> > > 
> > > I'm suspicious this should be a 'break' instead.
> > > 
> > > What do you think?
> > 
> > Yeah, this should be a 'break', care to make that patch up instead?
> > 
> 
> Sure thing.
> 
> Just some questions about the process to follow:
> 
> Should I send a v2 replying to this particular thread only? like [PATCH v2
> 6/9]
> or should I send just a new patch separated from this patch series? I guess
> this is the case.

Brand new patch is fine, this is gone from my patch queue.

> Some maintainers have told me that in cases where a particular patch in the
> series needs an update, the complete patchset should be sent again.
> But I think that depends on the functional impact the patch has over
> the whole patchset.

Yes, it all depends, the rest of these patches are already in my tree.

thanks,

greg k-h

Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through

[Gustavo A. R. Silva]

Quoting Greg Kroah-Hartman <gregkh@linuxfoundation.org>:

[..]
>>
>> Sure thing.
>>
>> Just some questions about the process to follow:
>>
>> Should I send a v2 replying to this particular thread only? like [PATCH v2
>> 6/9]
>> or should I send just a new patch separated from this patch series? I guess
>> this is the case.
>
> Brand new patch is fine, this is gone from my patch queue.
>
>> Some maintainers have told me that in cases where a particular patch in the
>> series needs an update, the complete patchset should be sent again.
>> But I think that depends on the functional impact the patch has over
>> the whole patchset.
>
> Yes, it all depends, the rest of these patches are already in my tree.
>

OK. I'll send a new patch shortly.

Thanks!
--
Gustavo A. R. Silva