Skip to contentSkip to conversation

spi: pxa2xx: Mark expected switch fall-through

PATCH In review Gustavo A. R. Silva 2 participants 4 messages Oct 3, 2018

Subsystems (3)
SPI PXA2xx/PXA3xx support Intel LPSS (Low Power SubSystem) drivers
lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 spi: pxa2xx: Mark expected switch fall-through

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

Addresses-Coverity-ID: 1056539 ("Missing break in switch")

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/spi/spi-pxa2xx.c | 1 +
 1 file changed, 1 insertion(+)
drivers/spi/spi-pxa2xx.c +1 -0
index fc9aac2..728b5f3 100644
@@ -666,6 +666,7 @@ static irqreturn_t interrupt_transfer(struct driver_data *drv_data)
 			switch (drv_data->n_bytes) {
 			case 4:
 				bytes_left >>= 1;
+				/* Fall through */
 			case 2:
 				bytes_left >>= 1;
 			}
Signature · 1 line
2.7.4
maintainer
#1 lore raw

drivers/spi/spi-pxa2xx.c static irqreturn_t interrupt_transfer(struct driver_data *drv_data) · line 672

Quoted 7 lines · Gustavo A. R. Silva
drivers/spi/spi-pxa2xx.c +1 -0
 			switch (drv_data->n_bytes) {
 			case 4:
 				bytes_left >>= 1;
+				/* Fall through */
 			case 2:
 				bytes_left >>= 1;
 			}

I think this code is just being too cute and it'd be better to just
rewrite it to directly do the expected number of shifts directly in each
case and have break statements; your fix is good but still not ideal for
readability I think.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20181003/7c2fbfa9/attachment.sig>

author
#2 lore raw
Quoted 15 lines · Mark Brown

drivers/spi/spi-pxa2xx.c static irqreturn_t interrupt_transfer(struct driver_data *drv_data) · line 672

Quoted 7 lines · Gustavo A. R. Silva
drivers/spi/spi-pxa2xx.c +1 -0
 			switch (drv_data->n_bytes) {
 			case 4:
 				bytes_left >>= 1;
+				/* Fall through */
 			case 2:
 				bytes_left >>= 1;
 			}

I think this code is just being too cute and it'd be better to just rewrite it to directly do the expected number of shifts directly in each case and have break statements; your fix is good but still not ideal for readability I think.

Okay. I agree. I'll rewrite and send v2.

Thanks
--
Gustavo

author
#3 lore raw
Quoted 19 lines · Gustavo A. R. Silva
Quoted 15 lines · Mark Brown

drivers/spi/spi-pxa2xx.c static irqreturn_t interrupt_transfer(struct driver_data *drv_data) · line 672

Quoted 7 lines · Gustavo A. R. Silva
drivers/spi/spi-pxa2xx.c +1 -0
 			switch (drv_data->n_bytes) {
 			case 4:
 				bytes_left >>= 1;
+				/* Fall through */
 			case 2:
 				bytes_left >>= 1;
 			}

I think this code is just being too cute and it'd be better to just rewrite it to directly do the expected number of shifts directly in each case and have break statements; your fix is good but still not ideal for readability I think.

Quoted 2 lines

Okay. I agree. I'll rewrite and send v2.

I'll actually send a completely new patch for this.

Thanks
--
Gustavo