I found some time to work on this issue.
So far we only report CAN error state changes to the worse:
"error-active -> error-warning -> error-passive -> bus-off", but not
"bus-off -> error-active" or "error-passive -> error-warning ->
error-active". There have been request from other users to correct
this behavior.
This patch reports any state change reported by the SJA1000 and
MSCAN-MPC5200 controller. It introduces "CAN_ERR_CRTL_ACTIVE" to
signal state changes to error active. Furthermore, the SJA1000
triggers now the bus-off recovery procedure when restarted. So far,
the controller was *stopped* and then restarted. I did two tests to
analyze the state changes behavior:
1. First I forced the controller into the error-passive by sending
a message (with cansend) without cable connected (*). Then I
reconnected the cable (**) and continued to send messages (with
cangen) (***) until the error active state is reached:
On SJA1000:
# candump any,0:0,#FFFFFFFF
(*)
can0 20000004 [8] 00 08 00 00 00 00 60 00 ERROR-WARNING
can0 20000004 [8] 00 20 00 00 00 00 80 00 ERROR-PASSIVE
(**)
can0 123 [4] DE AD BE EF
(***)
can0 42A [1] 00
...
can0 42A [1] 07
can0 20000004 [8] 00 08 00 00 00 00 7F 00 ERROR-WARNING
can0 42A [1] 08
...
can0 42A [1] 27
can0 20000004 [8] 00 40 00 00 00 00 5F 00 ERROR-ACTIVE
\-- tx-err !!!
On MSCAN:
# candump any,0:0,#FFFFFFFF
(*)
can2 20000004 [8] 00 08 00 00 00 00 00 00 ERROR-WARNING
can2 20000004 [8] 00 20 00 00 00 00 00 00 ERROR-PASSIVE
(**)
can2 123 [4] DE AD BE EF
can2 20000004 [8] 00 08 00 00 00 00 00 00 ERROR-WARNING
(***)
can2 42A [1] 00
can2 42A [1] 01
...
can2 42A [1] 1E
can2 42A [1] 1F
can2 20000004 [8] 00 40 00 00 00 00 00 00 ERROR-ACTIVE
As you can see, the state change to error-active is signaled
differently. The MSCAN seems not to respect the specs :-(.
When time permits, I will also try the MSCAN of the MPC5121.
2. I forced the controller into bus-off by short-circuiting
the low and hi lines and sending a message (*), Then I
restarted the controller manually with
"ip link set can0 type can restart" (**):
On SJA1000:
# ./candump -t d any,0:0,#FFFFFFFF
(*)
can0 20000004 [8] 00 08 00 00 00 00 88 00 ERROR-WARNING
can0 20000004 [8] 00 20 00 00 00 00 88 00 ERROR-PASSIVE
can0 20000044 [8] 00 00 00 00 00 00 7F 00 BUS-OFF
(**)
can0 20000100 [8] 00 00 00 00 00 00 00 00 RESTARTED
can0 20000004 [8] 00 40 00 00 00 00 00 00 ERROR-ACTIVE
\-- tx-err !!!
On MSCAN:
# ./candump -t d any,0:0,#FFFFFFFF
(*)
can2 20000004 [8] 00 08 00 00 00 00 00 00 ERROR-WARNING
can2 20000040 [8] 00 00 00 00 00 00 00 00 BUS-OFF
(**)
can2 20000100 [8] 00 00 00 00 00 00 00 00 RESTARTED
can2 20000004 [8] 00 40 00 00 00 00 00 00 ERROR-ACTIVE
The MSCAN seems not to signal the ERROR-PASSIVE state before
going bus-off.
Would be interesting to see, how other CAN controllers behave.
Volunteers are welcome.
What do you think? Would that patch/approach fit your needs?
Wolfgang.
---
drivers/net/can/mscan/mscan.c | 17 ++++++++++----
drivers/net/can/sja1000/sja1000.c | 45 +++++++++++++++++++++++++++-----------
include/linux/can/error.h | 1
3 files changed, 46 insertions(+), 17 deletions(-)
Index: net-next-2.6/drivers/net/can/mscan/mscan.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/mscan/mscan.c
+++ net-next-2.6/drivers/net/can/mscan/mscan.c
@@ -363,20 +363,29 @@ static void mscan_get_err_frame(struct n
/* State changed */
if (old_state != priv->can.state) {
switch (priv->can.state) {
+ case CAN_STATE_ERROR_ACTIVE:
+ frame->can_id |= CAN_ERR_CRTL;
+ frame->data[1] = CAN_ERR_CRTL_ACTIVE;
+ break;
case CAN_STATE_ERROR_WARNING:
frame->can_id |= CAN_ERR_CRTL;
priv->can.can_stats.error_warning++;
- if ((priv->shadow_statflg & MSCAN_RSTAT_MSK) <
+ if ((priv->shadow_statflg & MSCAN_RSTAT_MSK) !=
(canrflg & MSCAN_RSTAT_MSK))
frame->data[1] |= CAN_ERR_CRTL_RX_WARNING;
- if ((priv->shadow_statflg & MSCAN_TSTAT_MSK) <
+ if ((priv->shadow_statflg & MSCAN_TSTAT_MSK) !=
(canrflg & MSCAN_TSTAT_MSK))
frame->data[1] |= CAN_ERR_CRTL_TX_WARNING;
break;
case CAN_STATE_ERROR_PASSIVE:
frame->can_id |= CAN_ERR_CRTL;
priv->can.can_stats.error_passive++;
- frame->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
+ if ((priv->shadow_statflg & MSCAN_RSTAT_MSK) !=
+ (canrflg & MSCAN_RSTAT_MSK))
+ frame->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
+ if ((priv->shadow_statflg & MSCAN_TSTAT_MSK) !=
+ (canrflg & MSCAN_TSTAT_MSK))
+ frame->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
break;
case CAN_STATE_BUS_OFF:
frame->can_id |= CAN_ERR_BUSOFF;
@@ -393,8 +402,6 @@ static void mscan_get_err_frame(struct n
}
can_bus_off(dev);
break;
- default:
- break;
}
}
priv->shadow_statflg = canrflg & MSCAN_STAT_MSK;
Index: net-next-2.6/drivers/net/can/sja1000/sja1000.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/sja1000/sja1000.c
+++ net-next-2.6/drivers/net/can/sja1000/sja1000.c
@@ -165,6 +165,12 @@ static void sja1000_start(struct net_dev
{
struct sja1000_priv *priv = netdev_priv(dev);
+ if (priv->can.state == CAN_STATE_BUS_OFF) {
+ /* trigger bus-off recovery? */
+ priv->write_reg(priv, REG_MOD, 0x00);
+ return;
+ }
+
/* leave reset mode */
if (priv->can.state != CAN_STATE_STOPPED)
set_reset_mode(dev);
@@ -390,7 +396,8 @@ static int sja1000_err(struct net_device
if (isrc & IRQ_EI) {
/* error warning interrupt */
- dev_dbg(dev->dev.parent, "error warning interrupt\n");
+ dev_dbg(dev->dev.parent, "error warning interrupt (sr=%02x)\n",
+ status);
if (status & SR_BS) {
state = CAN_STATE_BUS_OFF;
@@ -398,8 +405,9 @@ static int sja1000_err(struct net_device
can_bus_off(dev);
} else if (status & SR_ES) {
state = CAN_STATE_ERROR_WARNING;
- } else
+ } else {
state = CAN_STATE_ERROR_ACTIVE;
+ }
}
if (isrc & IRQ_BEI) {
/* bus error interrupt */
@@ -431,11 +439,16 @@ static int sja1000_err(struct net_device
}
if (isrc & IRQ_EPI) {
/* error passive interrupt */
- dev_dbg(dev->dev.parent, "error passive interrupt\n");
- if (status & SR_ES)
- state = CAN_STATE_ERROR_PASSIVE;
- else
+ dev_dbg(dev->dev.parent, "error passive interrupt (sr=%02x)\n",
+ status);
+ if (status & SR_ES) {
+ if (priv->can.state == CAN_STATE_ERROR_WARNING)
+ state = CAN_STATE_ERROR_PASSIVE;
+ else
+ state = CAN_STATE_ERROR_WARNING;
+ } else {
state = CAN_STATE_ERROR_ACTIVE;
+ }
}
if (isrc & IRQ_ALI) {
/* arbitration lost interrupt */
@@ -447,28 +460,36 @@ static int sja1000_err(struct net_device
cf->data[0] = alc & 0x1f;
}
- if (state != priv->can.state && (state == CAN_STATE_ERROR_WARNING ||
- state == CAN_STATE_ERROR_PASSIVE)) {
+ if (state != priv->can.state) {
uint8_t rxerr = priv->read_reg(priv, REG_RXERR);
uint8_t txerr = priv->read_reg(priv, REG_TXERR);
cf->can_id |= CAN_ERR_CRTL;
- if (state == CAN_STATE_ERROR_WARNING) {
+
+ switch (state) {
+ case CAN_STATE_ERROR_ACTIVE:
+ cf->data[1] = CAN_ERR_CRTL_ACTIVE;
+ break;
+ case CAN_STATE_ERROR_WARNING:
priv->can.can_stats.error_warning++;
cf->data[1] = (txerr > rxerr) ?
CAN_ERR_CRTL_TX_WARNING :
CAN_ERR_CRTL_RX_WARNING;
- } else {
+ break;
+ case CAN_STATE_ERROR_PASSIVE:
priv->can.can_stats.error_passive++;
cf->data[1] = (txerr > rxerr) ?
CAN_ERR_CRTL_TX_PASSIVE :
CAN_ERR_CRTL_RX_PASSIVE;
+ break;
+ default:
+ break;
}
+
cf->data[6] = txerr;
cf->data[7] = rxerr;
+ priv->can.state = state;
}
- priv->can.state = state;
-
netif_rx(skb);
stats->rx_packets++;
Index: net-next-2.6/include/linux/can/error.h
===================================================================
--- net-next-2.6.orig/include/linux/can/error.h
+++ net-next-2.6/include/linux/can/error.h
@@ -41,6 +41,7 @@
#define CAN_ERR_CRTL_TX_PASSIVE 0x20 /* reached error passive status TX */
/* (at least one error counter exceeds */
/* the protocol-defined level of 127) */
+#define CAN_ERR_CRTL_ACTIVE 0x40 /* recovered to error active state */
/* error in CAN protocol (type) / data[2] */
#define CAN_ERR_PROT_UNSPEC 0x00 /* unspecified */
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
