Marc Kleine-Budde wrote:
> Wolfgang Grandegger wrote:
>> Marc Kleine-Budde wrote:
>>> Wolfgang Grandegger wrote:
>>>> Marc Kleine-Budde wrote:
>>>>> christian pellegrin wrote:
>>>>>> The following patch adds the possibility to monitor TEC/REC. I found
>>>>>> it quite useful for understanding how the bus works. I see that not
>>>>>> all controllers support is but for those that do it might be useful.
>>>>>>
>>>>>> >From 8a1260a1ac881aa49c0e8ebd92e56d83c722a7fd Mon Sep 17 00:00:00 2001
>>>>>> From: Christian Pellegrin <[email protected]>
>>>>>> Date: Sun, 7 Feb 2010 09:57:47 +0100
>>>>>> Subject: [PATCH] rec tec
>>>>>>
>>>>>> ---
>>>>>>  drivers/net/can/dev.c       |    3 +++
>>>>>>  drivers/net/can/mcp251x.c   |   13 +++++++++++++
>>>>>>  include/linux/can/dev.h     |    1 +
>>>>>>  include/linux/can/netlink.h |    2 ++
>>>>>>  4 files changed, 19 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
>>>>>> index f08f120..900a467 100644
>>>>>> --- a/drivers/net/can/dev.c
>>>>>> +++ b/drivers/net/can/dev.c
>>>>>> @@ -688,6 +688,9 @@ static int can_fill_xstats(struct sk_buff *skb,
>>>>>> const struct net_device *dev)
>>>>>>  {
>>>>>>          struct can_priv *priv = netdev_priv(dev);
>>>>>>
>>>>>> +        if (priv->do_update_stats)
>>>>>> +                priv->do_update_stats(dev);
>>>>>> +
>>>>>>          NLA_PUT(skb, IFLA_INFO_XSTATS,
>>>>>>                  sizeof(priv->can_stats), &priv->can_stats);
>>>>>>
>>>>>> diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
>>>>>> index 2608817..e343bbf 100644
>>>>>> --- a/drivers/net/can/mcp251x.c
>>>>>> +++ b/drivers/net/can/mcp251x.c
>>>>>> @@ -506,6 +506,18 @@ static int mcp251x_do_set_mode(struct net_device
>>>>>> *net, enum can_mode mode)
>>>>>>          return 0;
>>>>>>  }
>>>>>>
>>>>>> +static int mcp251x_do_update_stats(struct net_device *net)
>>>>>> +{
>>>>>> +        struct mcp251x_priv *priv = netdev_priv(net);
>>>>>> +
>>>>>> +        mutex_lock(&priv->mcp_lock);
>>>>>> +        priv->can.can_stats.tec = mcp251x_read_reg(priv->spi, TEC);
>>>>>> +        priv->can.can_stats.rec = mcp251x_read_reg(priv->spi, REC);
>>>>>> +        mutex_unlock(&priv->mcp_lock);
>>>>>> +        return 0;
>>>>>> +}
>>>>>> +
>>>>>> +
>>>>>>  static int mcp251x_set_normal_mode(struct spi_device *spi)
>>>>>>  {
>>>>>>          struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
>>>>>> @@ -1015,6 +1027,7 @@ static int __devinit mcp251x_can_probe(struct
>>>>>> spi_device *spi)
>>>>>>          priv = netdev_priv(net);
>>>>>>          priv->can.bittiming_const = &mcp251x_bittiming_const;
>>>>>>          priv->can.do_set_mode = mcp251x_do_set_mode;
>>>>>> +        priv->can.do_update_stats = mcp251x_do_update_stats;
>>>>>>          priv->can.clock.freq = pdata->oscillator_frequency / 2;
>>>>>>          priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
>>>>>>                  CAN_CTRLMODE_LOOPBACK | CAN_CTRLMODE_LISTENONLY |
>>>>>> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
>>>>>> index 4cb4f72..2bfecab 100644
>>>>>> --- a/include/linux/can/dev.h
>>>>>> +++ b/include/linux/can/dev.h
>>>>>> @@ -47,6 +47,7 @@ struct can_priv {
>>>>>>          int (*do_set_mode)(struct net_device *dev, enum can_mode mode);
>>>>>>          int (*do_get_state)(const struct net_device *dev,
>>>>>>                              enum can_state *state);
>>>>>> +        int (*do_update_stats)(const struct net_device *dev);
>>>>>>
>>>>>>          unsigned int echo_skb_max;
>>>>>>          struct sk_buff **echo_skb;
>>>>>> diff --git a/include/linux/can/netlink.h b/include/linux/can/netlink.h
>>>>>> index 49960e4..ef6fbf1 100644
>>>>>> --- a/include/linux/can/netlink.h
>>>>>> +++ b/include/linux/can/netlink.h
>>>>>> @@ -93,6 +93,8 @@ struct can_device_stats {
>>>>>>          __u32 bus_off;          /* Changes to bus off state */
>>>>>>          __u32 arbitration_lost; /* Arbitration lost errors */
>>>>>>          __u32 restarts;         /* CAN controller re-starts */
>>>>>> +        __u8 tec;
>>>>>> +        __u8 rec;
>>>>> One of these errors can go to 255 which is 0x100, and doesn't fit into
>>>>> an u8.
>>>> Hm, I think 255 == 0xff !?
>>> Indeed, off by one, but it can go to 256 which is 0x100.
>> On which CAN controller? On the AT91 CAN and SJA1000 it is a 8-bit
>> field. 256 means bus-off. Should the TEC or REC be set to 256 by
>> software if we are in bus-off? Hm, seems overkill to me.
> 
> Yes, on the at91 the field is only 8 bit long and in case of an bus
> error it shows "0", due to an overflow. The atmel support confirmed that
> problem; the next chip revision will have at least 9 bit wide field.

>From the SJA1000 manual:

"If a bus-off event occurs, the RX error counter is initialized to logic 0."
"If a bus-off event occurs, the TX error counter is initialized to 127 to 
count the minimum protocol-defined time (128 occurrences of the bus-free
signal). Reading the TX error counter during this time gives information
about the status of the bus-off recovery."

I think it makes little sense to adjust the RX/TX error counters by software,
as the exact meaning is hardware dependent. We should just read and list the
value from the hardware register.

Wolfgang.

_______________________________________________
Socketcan-users mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-users

Reply via email to