Hi Christian,

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.

It might be useful for debugging, I agree, but is it useful for the real
user? Kurt posted a similar patch using SysFS some time ago and we
didn't take it because it is not supported on all systems. Instead, we
would like to add the TEC and REC value, if possible, to the data[6..7]
fields of the error frame. This information is hardware specific anyhow.
Would that fit your needs?

>>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;
>  };

I think the tec and rec should not go to the CAN statistics struct.

Wolfgang.

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

Reply via email to