Hi Alexander,
On 02/07/2011 01:35 PM, Alexander Stein wrote:
> Hello,
>
> I know I can get error flags reading can error messages.
> This works nicely and give me CAN_ERR_CRTL_RX_WARNING and/or
> CAN_ERR_CRTL_TX_WARNING, CAN_ERR_CRTL_RX_PASSIVE and CAN_ERR_CRTL_TX_PASSIVE
> when they are set.
> But how can I know if those flags are removed? Do I get another can error
> frame with those bits unset?
> Well, you can expand this question to all error flags.
I think have a related patch in my pipeline, which I have attached below.
Unfortunately, it's for an old kernel version but if it's what you are
looking for, I would adapt it to the most recent net-next-2.6 tree.
The RFC is open. Other opinion are welcome as well.
Wolfgang.
[RFC PATCH] can: improved CAN error state handling
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
that behavior.
This patch reports any state changes 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 the hard way and then restarted. I did
two test to analyze the state changes behavior:
1. First I forced the controller into the error-passive my 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 on the MPC2511.
2. I forced the controller into bus-off by short-circuiting
the low and high 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
On SJA1000:
# ./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.
What do you think? Would that patch/approach fit your/our needs?
Wolfgang.
---
drivers/net/can/dev.c | 63 +++++++++++++++++++++++++++++++++++++-
drivers/net/can/mscan/mscan.c | 56 +++++++++------------------------
drivers/net/can/sja1000/sja1000.c | 48 ++++++++++++----------------
include/linux/can/dev.h | 3 +
include/linux/can/error.h | 2 +
5 files changed, 102 insertions(+), 70 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
@@ -288,20 +288,6 @@ static netdev_tx_t mscan_start_xmit(stru
return NETDEV_TX_OK;
}
-/* This function returns the old state to see where we came from */
-static enum can_state check_set_state(struct net_device *dev, u8 canrflg)
-{
- struct mscan_priv *priv = netdev_priv(dev);
- enum can_state state, old_state = priv->can.state;
-
- if (canrflg & MSCAN_CSCIF && old_state <= CAN_STATE_BUS_OFF) {
- state = state_map[max(MSCAN_STATE_RX(canrflg),
- MSCAN_STATE_TX(canrflg))];
- priv->can.state = state;
- }
- return old_state;
-}
-
static void mscan_get_rx_frame(struct net_device *dev, struct can_frame *frame)
{
struct mscan_priv *priv = netdev_priv(dev);
@@ -345,7 +331,6 @@ static void mscan_get_err_frame(struct n
struct mscan_priv *priv = netdev_priv(dev);
struct mscan_regs *regs = (struct mscan_regs *)priv->reg_base;
struct net_device_stats *stats = &dev->stats;
- enum can_state old_state;
dev_dbg(dev->dev.parent, "error interrupt (canrflg=%#x)\n", canrflg);
frame->can_id = CAN_ERR_FLAG;
@@ -359,45 +344,34 @@ static void mscan_get_err_frame(struct n
frame->data[1] = 0;
}
- old_state = check_set_state(dev, canrflg);
- /* State changed */
- if (old_state != priv->can.state) {
+ if (canrflg & MSCAN_CSCIF) {
+ enum can_state state = state_map[max(MSCAN_STATE_RX(canrflg),
+ MSCAN_STATE_TX(canrflg))];
+ can_change_state(dev, frame, state);
+
+ /* correct data[1] field */
switch (priv->can.state) {
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;
- break;
- case CAN_STATE_BUS_OFF:
- frame->can_id |= CAN_ERR_BUSOFF;
- /*
- * The MSCAN on the MPC5200 does recover from bus-off
- * automatically. To avoid that we stop the chip doing
- * a light-weight stop (we are in irq-context).
- */
- if (priv->type != MSCAN_TYPE_MPC5121) {
- out_8(®s->cantier, 0);
- out_8(®s->canrier, 0);
- setbits8(®s->canctl0,
- MSCAN_SLPRQ | MSCAN_INITRQ);
- }
- can_bus_off(dev);
+ 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;
default:
break;
}
+ priv->shadow_statflg = canrflg & MSCAN_STAT_MSK;
}
- priv->shadow_statflg = canrflg & MSCAN_STAT_MSK;
frame->can_dlc = CAN_ERR_DLC;
out_8(®s->canrflg, MSCAN_ERR_IF);
}
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,16 +396,17 @@ 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;
- cf->can_id |= CAN_ERR_BUSOFF;
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 +438,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,27 +459,7 @@ 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)) {
- 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) {
- priv->can.can_stats.error_warning++;
- cf->data[1] = (txerr > rxerr) ?
- CAN_ERR_CRTL_TX_WARNING :
- CAN_ERR_CRTL_RX_WARNING;
- } else {
- priv->can.can_stats.error_passive++;
- cf->data[1] = (txerr > rxerr) ?
- CAN_ERR_CRTL_TX_PASSIVE :
- CAN_ERR_CRTL_RX_PASSIVE;
- }
- cf->data[6] = txerr;
- cf->data[7] = rxerr;
- }
-
- priv->can.state = state;
+ can_change_state(dev, cf, state);
netif_rx(skb);
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
@@ -26,6 +26,7 @@
#define CAN_ERR_BUSOFF 0x00000040U /* bus off */
#define CAN_ERR_BUSERROR 0x00000080U /* bus error (may flood!) */
#define CAN_ERR_RESTARTED 0x00000100U /* controller restarted */
+#define CAN_ERR_STATE_CHANGE 0x00000200U /* CAN error state change / data[1] */
/* arbitration lost in bit ... / data[0] */
#define CAN_ERR_LOSTARB_UNSPEC 0x00 /* unspecified */
@@ -41,6 +42,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 */
Index: net-next-2.6/drivers/net/can/dev.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/dev.c
+++ net-next-2.6/drivers/net/can/dev.c
@@ -230,6 +230,68 @@ int can_get_bittiming(struct net_device
return 0;
}
+void can_change_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;
+
+ if (state == priv->can.state)
+ return;
+
+ if (priv->can.do_get_berr_counter)
+ priv->can.do_get_berr_counter(&bec);
+
+ cf->can_id |= CAN_ERR_STATE_CHANGE;
+ if (state > priv->can.state && state != CAN_STATE_BUS_OFF)
+ cf->can_id |= CAN_ERR_CRTL;
+
+ switch (state) {
+ case CAN_STATE_ERROR_PASSIVE:
+ priv->can.can_stats.error_passive++;
+ if (priv->can.do_get_berr_counter)
+ cf->data[1] = (bec.txerr > bec.rxerr) ?
+ CAN_ERR_CRTL_TX_PASSIVE :
+ CAN_ERR_CRTL_RX_PASSIVE;
+ else
+ cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
+ CAN_ERR_CRTL_RX_PASSIVE;
+ break;
+
+ case CAN_STATE_ERROR_WARNING:
+ priv->can.can_stats.error_warning++;
+ if (priv->can.do_get_berr_counter)
+ cf->data[1] = (bec.txerr > bec.rxerr) ?
+ CAN_ERR_CRTL_TX_WARNING :
+ CAN_ERR_CRTL_RX_WARNING;
+ else
+ cf->data[1] = CAN_ERR_CRTL_TX_WARNING |
+ CAN_ERR_CRTL_RX_WARNING;
+ break;
+
+ case CAN_STATE_ERROR_ACTIVE:
+ cf->data[1] = CAN_ERR_CRTL_ACTIVE;
+ break;
+
+ case CAN_STATE_BUS_OFF:
+ priv->can.can_stats.bus_off++;
+ cf->can_id |= CAN_ERR_BUSOFF;
+ break;
+
+ default:
+ break;
+ }
+
+ if (has_bec) {
+ cf->data[6] = txerr;
+ cf->data[7] = rxerr;
+ }
+
+ /* Finally set new state */
+ priv->can.state = state;
+}
+EXPORT_SYMBOL_GPL(can_do_state_change);
+
/*
* Local echo of CAN messages
*
@@ -424,7 +486,6 @@ void can_bus_off(struct net_device *dev)
dev_dbg(dev->dev.parent, "bus-off\n");
netif_carrier_off(dev);
- priv->can_stats.bus_off++;
if (priv->restart_ms)
mod_timer(&priv->restart_timer,
Index: net-next-2.6/include/linux/can/dev.h
===================================================================
--- net-next-2.6.orig/include/linux/can/dev.h
+++ net-next-2.6/include/linux/can/dev.h
@@ -91,6 +91,9 @@ void unregister_candev(struct net_device
int can_restart_now(struct net_device *dev);
void can_bus_off(struct net_device *dev);
+void can_change_state(struct net_device *dev, struct can_frame *cf,
+ enum can_state new_state);
+
void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
unsigned int idx);
void can_get_echo_skb(struct net_device *dev, unsigned int idx);
_______________________________________________
Socketcan-users mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-users
