Hi Wolfram,
Wolfgang Grandegger wrote:
> Wolfgang Grandegger wrote:
>> Wolfram Sang wrote:
>>>> Well, to save a test cycle, I'm awaiting for your patch first.
>>> That's probably a good idea, I don't even know if this patch applies without
>>> the rest of my branch.
>>>
>>> I just saw there is one FIXME left in the code. If you can comment on this,
>>> this would be much appreciated, as I don't know the napi-framework a bit and
>>> cannot judge if it should return 0 there or better -ENOSOMETHING in that
>>> case.
>
> I have attached my untested patch for SVN trunk fixing the bus-off
> recovery.
>
>> Well, I tried to apply the patch to the SVN trunk, which failed :-).
>> Will fix and test now.
>
> Failed again because you fixed the open issue with base_address :-).
> Please send your patch and I will check and test.
I'm doing some test now, but the beast does not behave like expected :-(:
restart-ms=0 works fine:
short-circuiting wires
can2 87 [4] 12 34 56 67
can2 20000004 [8] 00 08 00 00 00 00 00 00 ERRORFRAME
can2 20000040 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
^ bus-off
can2 20000100 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
^ manual restart
restart>0 does not:
short-circuiting wires
can2 87 [4] 12 34 56 67
can2 20000004 [8] 00 08 00 00 00 00 00 00 ERRORFRAME
can2 20000040 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
removing short-circuit
can2 87 [4] 12 34 56 67
As you can see, the message gets through after removing the
short-circuit but no state change interrupt is triggered. This is
confirmed by the dmesg lines below:
[ 4290.511451] mpc52xx_can f0000900.can: error interrupt (canrflg=0x44)
[ 4290.514611] mpc52xx_can f0000900.can: error interrupt (canrflg=0x7c)
[ 4290.514671] mpc52xx_can f0000900.can: bus-off
[ 4292.142115] mpc52xx_can f0000900.can: TX done (canrflg=0x8)
Either we capture the TX done after BUS-OFF and check the state (which
is non-trivial) or we do not allow automatic recovery at all. Attached
is my revised patch for this test. I'm also not happy that we call
mscan_stop() from the interrupt context.
Wolfgang.
---
kernel/2.6/drivers/net/can/mscan/mscan.c | 61 +++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 14 deletions(-)
Index: trunk/kernel/2.6/drivers/net/can/mscan/mscan.c
===================================================================
--- trunk.orig/kernel/2.6/drivers/net/can/mscan/mscan.c
+++ trunk/kernel/2.6/drivers/net/can/mscan/mscan.c
@@ -237,6 +237,15 @@ static int mscan_start(struct net_device
return 0;
}
+static void mscan_stop(struct net_device *dev)
+{
+ struct mscan_regs *regs = (struct mscan_regs *)dev->base_addr;
+
+ out_8(®s->cantier, 0);
+ out_8(®s->canrier, 0);
+ mscan_set_mode(dev, MSCAN_INIT_MODE);
+}
+
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
static int mscan_start_xmit(struct sk_buff *skb, struct net_device *dev)
#else
@@ -326,21 +335,20 @@ static netdev_tx_t mscan_start_xmit(stru
return NETDEV_TX_OK;
}
-static inline int check_set_state(struct net_device *dev, u8 canrflg)
+/* 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;
- int ret = 0;
+ enum can_state state, old_state = priv->can.state;
- if (!(canrflg & MSCAN_CSCIF) || priv->can.state > CAN_STATE_BUS_OFF)
- return 0;
+ if (!(canrflg & MSCAN_CSCIF) || old_state > CAN_STATE_BUS_OFF)
+ return old_state;
state = state_map[max(MSCAN_STATE_RX(canrflg),
MSCAN_STATE_TX(canrflg))];
- if (priv->can.state < state)
- ret = 1;
+
priv->can.state = state;
- return ret;
+ return old_state;
}
#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,23)
@@ -365,6 +373,7 @@ static int mscan_rx_poll(struct net_devi
#else
struct net_device_stats *stats = &dev->stats;
#endif
+ enum can_state old_state;
int npackets = 0;
int ret = 1;
struct sk_buff *skb;
@@ -431,7 +440,16 @@ static int mscan_rx_poll(struct net_devi
} else
frame->data[1] = 0;
- if (check_set_state(dev, canrflg)) {
+ old_state = check_set_state(dev, canrflg);
+ /* State out of BUS_OFF (automatic recovery) */
+ if (old_state == CAN_STATE_BUS_OFF
+ && priv->can.state < old_state) {
+ frame->can_id |= CAN_ERR_RESTARTED;
+ priv->can.can_stats.restarts++;
+ netif_carrier_on(dev);
+ }
+ /* State got worse */
+ if (old_state < priv->can.state) {
switch (priv->can.state) {
case CAN_STATE_ERROR_WARNING:
frame->can_id |= CAN_ERR_CRTL;
@@ -456,7 +474,22 @@ static int mscan_rx_poll(struct net_devi
break;
case CAN_STATE_BUS_OFF:
frame->can_id |= CAN_ERR_BUSOFF;
- can_bus_off(dev);
+ /*
+ * The MSCAN on the MPC5200 does recover
+ * from bus-off automatically. Therefore
+ * we stop the chip if the user wants
+ * to restart the chip.
+ */
+ if (priv->can.restart_ms) {
+ netif_carrier_off(dev);
+ dev_dbg(ND2D(dev), "bus-off");
+ priv->can.can_stats.bus_off++;
+ } else {
+ mscan_stop(dev);
+ priv->can.state =
+ CAN_STATE_BUS_OFF;
+ can_bus_off(dev);
+ }
break;
default:
break;
@@ -516,6 +549,9 @@ static irqreturn_t mscan_isr(int irq, vo
struct list_head *tmp, *pos;
+ dev_dbg(ND2D(dev), "TX done (canrflg=%#x)\n",
+ in_8(®s->canrflg));
+
list_for_each_safe(pos, tmp, &priv->tx_head) {
struct tx_queue_entry *entry =
list_entry(pos, struct tx_queue_entry, list);
@@ -661,7 +697,6 @@ static int mscan_open(struct net_device
static int mscan_close(struct net_device *dev)
{
- struct mscan_regs *regs = (struct mscan_regs *)dev->base_addr;
struct mscan_priv *priv = netdev_priv(dev);
netif_stop_queue(dev);
@@ -669,9 +704,7 @@ static int mscan_close(struct net_device
napi_disable(&priv->napi);
#endif
- out_8(®s->cantier, 0);
- out_8(®s->canrier, 0);
- mscan_set_mode(dev, MSCAN_INIT_MODE);
+ mscan_stop(dev);
close_candev(dev);
free_irq(dev->irq, dev);
priv->open_time = 0;
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
