linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 4/4] can: flexcan: Consolidate and unify state change handling
@ 2014-09-26 17:31 Andri Yngvason
  2014-11-25 21:03 ` Wolfgang Grandegger
  0 siblings, 1 reply; 4+ messages in thread
From: Andri Yngvason @ 2014-09-26 17:31 UTC (permalink / raw)
  To: linux-can; +Cc: mkl, wg

Replacing error state change handling with the new mechanism.

Changes made since last proposal:
can: flexcan: improve rx/tx state recognition.

Note: Flexcan is problematic because there is no interrupt generated when
transitioning between error-active and error-passive. If the bus is
disconnected, there is no event that will trigger a napi poll, so the new state
will not be discovered until the bus is connected. This problem goes away if
berr-reporting is enabled, because then the ack-error takes care of it.

Another problem is that if nothing is happening on the bus, when it's connected
again, we won't see the new state until someone transmits something on the bus.

I'd say that these are minor issues because a "healthy" bus has traffic and the
"disconnected bus" issue has a work-around.

Signed-off-by: Andri Yngvason <andri.yngvason@marel.com>
---
 drivers/net/can/flexcan.c | 101 +++++++++-------------------------------------
 1 file changed, 18 insertions(+), 83 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 60f86bd..e7dea64 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -577,98 +577,30 @@ static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
 	return 1;
 }
 
-static void do_state(struct net_device *dev,
-		     struct can_frame *cf, enum can_state new_state)
-{
-	struct flexcan_priv *priv = netdev_priv(dev);
-	struct can_berr_counter bec;
-
-	__flexcan_get_berr_counter(dev, &bec);
-
-	switch (priv->can.state) {
-	case CAN_STATE_ERROR_ACTIVE:
-		/*
-		 * from: ERROR_ACTIVE
-		 * to  : ERROR_WARNING, ERROR_PASSIVE, BUS_OFF
-		 * =>  : there was a warning int
-		 */
-		if (new_state >= CAN_STATE_ERROR_WARNING &&
-		    new_state <= CAN_STATE_BUS_OFF) {
-			netdev_dbg(dev, "Error Warning IRQ\n");
-			priv->can.can_stats.error_warning++;
-
-			cf->can_id |= CAN_ERR_CRTL;
-			cf->data[1] = (bec.txerr > bec.rxerr) ?
-				CAN_ERR_CRTL_TX_WARNING :
-				CAN_ERR_CRTL_RX_WARNING;
-		}
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
-		/*
-		 * from: ERROR_ACTIVE, ERROR_WARNING
-		 * to  : ERROR_PASSIVE, BUS_OFF
-		 * =>  : error passive int
-		 */
-		if (new_state >= CAN_STATE_ERROR_PASSIVE &&
-		    new_state <= CAN_STATE_BUS_OFF) {
-			netdev_dbg(dev, "Error Passive IRQ\n");
-			priv->can.can_stats.error_passive++;
-
-			cf->can_id |= CAN_ERR_CRTL;
-			cf->data[1] = (bec.txerr > bec.rxerr) ?
-				CAN_ERR_CRTL_TX_PASSIVE :
-				CAN_ERR_CRTL_RX_PASSIVE;
-		}
-		break;
-	case CAN_STATE_BUS_OFF:
-		netdev_err(dev, "BUG! "
-			   "hardware recovered automatically from BUS_OFF\n");
-		break;
-	default:
-		break;
-	}
-
-	/* process state changes depending on the new state */
-	switch (new_state) {
-	case CAN_STATE_ERROR_WARNING:
-		netdev_dbg(dev, "Error Warning\n");
-		cf->can_id |= CAN_ERR_CRTL;
-		cf->data[1] = (bec.txerr > bec.rxerr) ?
-			CAN_ERR_CRTL_TX_WARNING :
-			CAN_ERR_CRTL_RX_WARNING;
-		break;
-	case CAN_STATE_ERROR_ACTIVE:
-		netdev_dbg(dev, "Error Active\n");
-		cf->can_id |= CAN_ERR_PROT;
-		cf->data[2] = CAN_ERR_PROT_ACTIVE;
-		break;
-	case CAN_STATE_BUS_OFF:
-		cf->can_id |= CAN_ERR_BUSOFF;
-		can_bus_off(dev);
-		break;
-	default:
-		break;
-	}
-}
-
 static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
 {
 	struct flexcan_priv *priv = netdev_priv(dev);
 	struct sk_buff *skb;
 	struct can_frame *cf;
-	enum can_state new_state;
+	enum can_state new_state = 0, rx_state = 0, tx_state = 0;
 	int flt;
+	struct can_berr_counter bec;
 
 	flt = reg_esr & FLEXCAN_ESR_FLT_CONF_MASK;
 	if (likely(flt == FLEXCAN_ESR_FLT_CONF_ACTIVE)) {
-		if (likely(!(reg_esr & (FLEXCAN_ESR_TX_WRN |
-					FLEXCAN_ESR_RX_WRN))))
-			new_state = CAN_STATE_ERROR_ACTIVE;
-		else
-			new_state = CAN_STATE_ERROR_WARNING;
-	} else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE))
+		tx_state = unlikely(reg_esr & FLEXCAN_ESR_TX_WRN) ?
+			   CAN_STATE_ERROR_WARNING : CAN_STATE_ERROR_ACTIVE;
+		rx_state = unlikely(reg_esr & FLEXCAN_ESR_RX_WRN) ?
+			   CAN_STATE_ERROR_WARNING : CAN_STATE_ERROR_ACTIVE;
+		new_state = max(tx_state, rx_state);
+	} else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE)) {
+		__flexcan_get_berr_counter(dev, &bec);
 		new_state = CAN_STATE_ERROR_PASSIVE;
-	else
+		rx_state = bec.rxerr >= bec.txerr ? new_state : 0;
+		tx_state = bec.rxerr <= bec.txerr ? new_state : 0;
+	} else {
 		new_state = CAN_STATE_BUS_OFF;
+	}
 
 	/* state hasn't changed */
 	if (likely(new_state == priv->can.state))
@@ -678,8 +610,11 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
 	if (unlikely(!skb))
 		return 0;
 
-	do_state(dev, cf, new_state);
-	priv->can.state = new_state;
+	can_change_state(dev, cf, new_state, tx_state, rx_state);
+
+	if (unlikely(new_state == CAN_STATE_BUS_OFF))
+		can_bus_off(dev);
+
 	netif_receive_skb(skb);
 
 	dev->stats.rx_packets++;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-11-26 11:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-26 17:31 [PATCH v3 4/4] can: flexcan: Consolidate and unify state change handling Andri Yngvason
2014-11-25 21:03 ` Wolfgang Grandegger
2014-11-26 11:06   ` Andri Yngvason
2014-11-26 11:28     ` Wolfgang Grandegger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).