On 02/07/2011 03:47 PM, Alexander Stein wrote:
> Hi Wolfgang,
>
> On Monday 07 February 2011, 14:20:57 Wolfgang Grandegger wrote:
>> 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.
>
> See some comments below.
>
>> [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?
>
> Mh, it seems you have some other patches for candump.c/lib.c to actually
> print
> the state change.
I added the strings like "ERROR_WARNING" at the end manually. I think
that's what you mean!? I once started to make a patch for candump to
print the meaning of the CAN id and data fields in a human readable
format. That would be a nice to have. I'm going to dig for it.
> Well, on the other hand, at91_can, flexcan and pch_can are the only
> controllers currently sending an CAN_ERR_ACK in can_id. Which you will be
> spammed with.
Yes, that are the infamous "ack slot" errors. Strictly speaking the
CAN_ERR_ACK is not correct. It sneaked in some time ago. It's a normal
bus error and it should therefore be:
cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
cf->data[2] |= CAN_ERR_PROT_UNSPEC;
cf->data[3] |= CAN_ERR_PROT_LOC_ACK; /* ACK slot */
That's what the SJA1000 reports and a few other drivers need to be fixed
as well.
See also
http://www.mail-archive.com/[email protected]/msg00212.html
> But using an own application to send/receive some CAN messages where I parse
> the error frames (ignoring ACK error though), I currently use
> CAN_ERR_PROT_ACTIVE to know when I got back active.
That was implemented by Mark but we need a general solution for all drivers.
> The idea behind your patch seems ok to me. ATM I'm getting used to socketcan
> using at91_can on an 2.6.35.9 kernel.
>
>> --- 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);
> ^^^^^^^^^^^^
> This seems wrong to me.
Obviously it needs to be fixed. Anyway, it does not harm.
>> + 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)
> ^^^^^
> As far as I can see, this should also be new_state.
OK, I need to check that patch more thoroughly. Maybe I just picked an
old version.
Wolfgang.
_______________________________________________
Socketcan-users mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-users
