Skip to contentSkip to conversation

can: at91_can: mark expected switch fall-throughs

PATCH In review Gustavo A. R. Silva 3 participants 5 messages Feb 8, 2019 – Feb 14, 2019

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 can: at91_can: mark expected switch fall-throughs

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

Notice that, in this particular case, the /* fall through */ comments are placed at the bottom of the case statement, which is what GCC is expecting to find.

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

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

---
 drivers/net/can/at91_can.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/can/at91_can.c +4 -2
index d98c69045b17..1718c20f9c99 100644
@@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
 				CAN_ERR_CRTL_TX_WARNING :
 				CAN_ERR_CRTL_RX_WARNING;
 		}
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
+		/* fall through */
+	case CAN_STATE_ERROR_WARNING:
 		/*
 		 * from: ERROR_ACTIVE, ERROR_WARNING
 		 * to  : ERROR_PASSIVE, BUS_OFF
@@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
 		netdev_dbg(dev, "Error Active\n");
 		cf->can_id |= CAN_ERR_PROT;
 		cf->data[2] = CAN_ERR_PROT_ACTIVE;
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
+		/* fall through */
+	case CAN_STATE_ERROR_WARNING:
 		reg_idr = AT91_IRQ_ERRA | AT91_IRQ_WARN | AT91_IRQ_BOFF;
 		reg_ier = AT91_IRQ_ERRP;
 		break;
Signature · 1 line
2.20.1
#1 lore raw

Hello!

Quoted 22 lines · Gustavo A. R. Silva

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

Notice that, in this particular case, the /* fall through */ comments are placed at the bottom of the case statement, which is what GCC is expecting to find.

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

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

---
 drivers/net/can/at91_can.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/can/at91_can.c +0 -0
index d98c69045b17..1718c20f9c99 100644

drivers/net/can/at91_can.c static void at91_irq_err_state(struct net_device *dev, · line 905

Quoted 5 lines
drivers/net/can/at91_can.c +1 -1
@@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
 				CAN_ERR_CRTL_TX_WARNING :
 				CAN_ERR_CRTL_RX_WARNING;
 		}
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
+		/* fall through */

Why do we need this comment at all? Just remove it, that's all.

drivers/net/can/at91_can.c static void at91_irq_err_state(struct net_device *dev, · line 955

Quoted 10 lines
drivers/net/can/at91_can.c +2 -1
+	case CAN_STATE_ERROR_WARNING:
 		/*
 		 * from: ERROR_ACTIVE, ERROR_WARNING
 		 * to  : ERROR_PASSIVE, BUS_OFF
@@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
 		netdev_dbg(dev, "Error Active\n");
 		cf->can_id |= CAN_ERR_PROT;
 		cf->data[2] = CAN_ERR_PROT_ACTIVE;
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
+		/* fall through */

Again, we don;t need it here.

drivers/net/can/at91_can.c static void at91_irq_err_state(struct net_device *dev, · line 959

Quoted 4 lines
drivers/net/can/at91_can.c +1 -0
+	case CAN_STATE_ERROR_WARNING:
 		reg_idr = AT91_IRQ_ERRA | AT91_IRQ_WARN | AT91_IRQ_BOFF;
 		reg_ier = AT91_IRQ_ERRP;
 		break;

MBR, Serfei

#2 lore raw
Quoted 43 lines · Sergei Shtylyov
Quoted 22 lines

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

Notice that, in this particular case, the /* fall through */ comments are placed at the bottom of the case statement, which is what GCC is expecting to find.

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

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

---
 drivers/net/can/at91_can.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/can/at91_can.c +0 -0
index d98c69045b17..1718c20f9c99 100644

drivers/net/can/at91_can.c static void at91_irq_err_state(struct net_device *dev, · line 905

Quoted 5 lines
drivers/net/can/at91_can.c +1 -1
@@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
 				CAN_ERR_CRTL_TX_WARNING :
 				CAN_ERR_CRTL_RX_WARNING;
 		}
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
+		/* fall through */

Why do we need this comment at all? Just remove it, that's all.

drivers/net/can/at91_can.c static void at91_irq_err_state(struct net_device *dev, · line 955

Quoted 10 lines
drivers/net/can/at91_can.c +2 -1
+	case CAN_STATE_ERROR_WARNING:
 		/*
 		 * from: ERROR_ACTIVE, ERROR_WARNING
 		 * to  : ERROR_PASSIVE, BUS_OFF
@@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
 		netdev_dbg(dev, "Error Active\n");
 		cf->can_id |= CAN_ERR_PROT;
 		cf->data[2] = CAN_ERR_PROT_ACTIVE;
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
+		/* fall through */

Again, we don;t need it here.

Quoted 4 lines

drivers/net/can/at91_can.c static void at91_irq_err_state(struct net_device *dev, · line 959

Quoted 4 lines
drivers/net/can/at91_can.c +1 -0
+	case CAN_STATE_ERROR_WARNING:
 		reg_idr = AT91_IRQ_ERRA | AT91_IRQ_WARN | AT91_IRQ_BOFF;
 		reg_ier = AT91_IRQ_ERRP;
 		break;

Ignore me, I misread the code...

MBR, Sergei

maintainer Acked-by on v1 1/1
#3 lore raw
In reply to Gustavo A. R. Silva
Quoted 7 lines · Gustavo A. R. Silva

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

Notice that, in this particular case, the /* fall through */ comments are placed at the bottom of the case statement, which is what GCC is expecting to find.

Quoted 6 lines

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

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

Looks good to me:

Quoted 29 lines
---
  drivers/net/can/at91_can.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/can/at91_can.c +4 -2
index d98c69045b17..1718c20f9c99 100644
@@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
  				CAN_ERR_CRTL_TX_WARNING :
  				CAN_ERR_CRTL_RX_WARNING;
  		}
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
+		/* fall through */
+	case CAN_STATE_ERROR_WARNING:
  		/*
  		 * from: ERROR_ACTIVE, ERROR_WARNING
  		 * to  : ERROR_PASSIVE, BUS_OFF
@@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
  		netdev_dbg(dev, "Error Active\n");
  		cf->can_id |= CAN_ERR_PROT;
  		cf->data[2] = CAN_ERR_PROT_ACTIVE;
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
+		/* fall through */
+	case CAN_STATE_ERROR_WARNING:
  		reg_idr = AT91_IRQ_ERRA | AT91_IRQ_WARN | AT91_IRQ_BOFF;
  		reg_ier = AT91_IRQ_ERRP;
  		break;
Signature · 1 line
Nicolas Ferre
#4 lore raw
Quoted 10 lines · On 2/11/19 3:03 AM, [email protected] wrote:
Quoted 9 lines · Gustavo A. R. Silva

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

Notice that, in this particular case, the /* fall through */ comments are placed at the bottom of the case statement, which is what GCC is expecting to find.

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

Quoted 8 lines
Quoted 4 lines

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

Looks good to me:

Thanks, Nicolas.

--
Gustavo

Quoted 31 lines
Quoted 29 lines
---
  drivers/net/can/at91_can.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/can/at91_can.c +4 -2
index d98c69045b17..1718c20f9c99 100644
@@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
  				CAN_ERR_CRTL_TX_WARNING :
  				CAN_ERR_CRTL_RX_WARNING;
  		}
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
+		/* fall through */
+	case CAN_STATE_ERROR_WARNING:
  		/*
  		 * from: ERROR_ACTIVE, ERROR_WARNING
  		 * to  : ERROR_PASSIVE, BUS_OFF
@@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
  		netdev_dbg(dev, "Error Active\n");
  		cf->can_id |= CAN_ERR_PROT;
  		cf->data[2] = CAN_ERR_PROT_ACTIVE;
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
+		/* fall through */
+	case CAN_STATE_ERROR_WARNING:
  		reg_idr = AT91_IRQ_ERRA | AT91_IRQ_WARN | AT91_IRQ_BOFF;
  		reg_ier = AT91_IRQ_ERRP;
  		break;