Re: [PATCH] add MPC512x support to mscan

[Wolfgang Grandegger]

Wolfgang Grandegger wrote:
> Wolfram Sang wrote:
>> Hi Wolfgang,
>>
>>> 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)
>> Hmm, I've been checking the specs to see if there are special conditions for
>> this kind of interrupt but didn't find any so far. What kind of test-program 
>> do
>> you use? ... Oh, ehrm, mscan_stop disables all interrupts. Maybe we are 
>> missing
>> the one because of this? (I am still not full into the state machine yet).
> 
> If "restart-ms>0", automatic recovery is allowed and we do *not* stop
> the MSCAN. Obviously it does recover, but it does not send a state
> change interrupt. We need that to send a RESTARTED message to the
> application.
> 
>>> 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.
>> Is it okay if we disallow automatic recovery for now if we can't find a
>> solution for this today? I'd like to post the patch to netdev today if 
>> somehow
>> possible.
> 
> Yes, that's my proposal. On the MPC512x we will disable automatic
> recovery anyhow.
> 
>>> I'm also not happy that we call mscan_stop() from the interrupt context.
>> Would it be an idea to use a subset of mscan_set_mode() in the interrupt
>> context?
> 
> Not sure due to the problems Fu discovered. I have prepared two patches
> for SVN trunk, which I will send in a minute.

This works as well:

                        /*
                         * The MSCAN on the MPC5200 does recover from bus-off
                         * automatically. To avoid that we stop the chip.
                         */
  #if 0
                        mscan_stop(dev);
                        priv->can.state = CAN_STATE_BUS_OFF;
  #else
                        /* Light way stop */
                        out_8(&regs->cantier, 0);
                        out_8(&regs->canrier, 0);
                        out_8(&regs->canctl0, in_8(&regs->canctl0) |
                              MSCAN_SLPRQ | MSCAN_INITRQ);
  #endif

Wolfgang.
_______________________________________________
Socketcan-core mailing list
Socketcan-core@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/socketcan-core