lkml.org 
[lkml]   [2022]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 08/12] can: kvaser_usb_leaf: Fix improved state not being reported
    From

    On 5/16/22 15:47, Anssi Hannula wrote:
    > The tested 0bfd:0017 Kvaser Memorator Professional HS/HS FW 2.0.50 and
    > 0bfd:0124 Kvaser Mini PCI Express 2xHS FW 4.18.778 do not seem to send
    > any unsolicited events when error counters decrease or when the device
    > transitions from ERROR_PASSIVE to ERROR_ACTIVE (or WARNING).
    >
    > This causes the interface to e.g. indefinitely stay in the ERROR_PASSIVE
    > state.
    >
    > Fix that by asking for chip state (inc. counters) event every 0.5 secs
    > when error counters are non-zero.
    >
    > Since the driver seems to be prepared for non-error-counter devices
    > (!KVASER_USB_HAS_TXRX_ERRORS) as well, also always poll in ERROR_PASSIVE
    > even if the counters show zero.
    >
    > Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
    > Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>

    Looks good to me.
    Tested-by: Jimmy Assarsson <extja@kvaser.com>

    > ---
    > drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 7 +++
    > .../net/can/usb/kvaser_usb/kvaser_usb_core.c | 18 +++++-
    > .../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 58 +++++++++++++++++++
    > 3 files changed, 80 insertions(+), 3 deletions(-)
    >
    > diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
    > index f1bea13a3829..70aa7a9ed35b 100644
    > --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
    > +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
    > @@ -107,6 +107,9 @@ struct kvaser_usb_net_priv {
    > struct can_priv can;
    > struct can_berr_counter bec;
    >
    > + /* subdriver-specific data */
    > + void *sub_priv;
    > +
    > struct kvaser_usb *dev;
    > struct net_device *netdev;
    > int channel;
    > @@ -128,6 +131,8 @@ struct kvaser_usb_net_priv {
    > *
    > * @dev_setup_endpoints: setup USB in and out endpoints
    > * @dev_init_card: initialize card
    > + * @dev_init_channel: initialize channel
    > + * @dev_remove_channel: uninitialize channel
    > * @dev_get_software_info: get software info
    > * @dev_get_software_details: get software details
    > * @dev_get_card_info: get card info
    > @@ -149,6 +154,8 @@ struct kvaser_usb_dev_ops {
    > struct can_berr_counter *bec);
    > int (*dev_setup_endpoints)(struct kvaser_usb *dev);
    > int (*dev_init_card)(struct kvaser_usb *dev);
    > + int (*dev_init_channel)(struct kvaser_usb_net_priv *priv);
    > + void (*dev_remove_channel)(struct kvaser_usb_net_priv *priv);
    > int (*dev_get_software_info)(struct kvaser_usb *dev);
    > int (*dev_get_software_details)(struct kvaser_usb *dev);
    > int (*dev_get_card_info)(struct kvaser_usb *dev);
    > diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
    > index a8d72fb8291a..6a1ebdd9ba85 100644
    > --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
    > +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
    > @@ -645,6 +645,9 @@ static void kvaser_usb_remove_interfaces(struct kvaser_usb *dev)
    > if (!dev->nets[i])
    > continue;
    >
    > + if (dev->ops->dev_remove_channel)
    > + dev->ops->dev_remove_channel(dev->nets[i]);
    > +
    > free_candev(dev->nets[i]->netdev);
    > }
    > }
    > @@ -712,17 +715,26 @@ static int kvaser_usb_init_one(struct kvaser_usb *dev,
    >
    > dev->nets[channel] = priv;
    >
    > + if (dev->ops->dev_init_channel) {
    > + err = dev->ops->dev_init_channel(priv);
    > + if (err)
    > + goto err;
    > + }
    > +
    > err = register_candev(netdev);
    > if (err) {
    > dev_err(&dev->intf->dev, "Failed to register CAN device\n");
    > - free_candev(netdev);
    > - dev->nets[channel] = NULL;
    > - return err;
    > + goto err;
    > }
    >
    > netdev_dbg(netdev, "device registered\n");
    >
    > return 0;
    > +
    > +err:
    > + free_candev(netdev);
    > + dev->nets[channel] = NULL;
    > + return err;
    > }
    >
    > static int kvaser_usb_probe(struct usb_interface *intf,
    > diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
    > index 5f27c00179c1..abb681808a28 100644
    > --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
    > +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
    > @@ -21,6 +21,7 @@
    > #include <linux/types.h>
    > #include <linux/units.h>
    > #include <linux/usb.h>
    > +#include <linux/workqueue.h>
    >
    > #include <linux/can.h>
    > #include <linux/can/dev.h>
    > @@ -56,6 +57,7 @@
    > #define CMD_RX_EXT_MESSAGE 14
    > #define CMD_TX_EXT_MESSAGE 15
    > #define CMD_SET_BUS_PARAMS 16
    > +#define CMD_GET_CHIP_STATE 19
    > #define CMD_CHIP_STATE_EVENT 20
    > #define CMD_SET_CTRL_MODE 21
    > #define CMD_RESET_CHIP 24
    > @@ -375,6 +377,12 @@ struct kvaser_usb_err_summary {
    > };
    > };
    >
    > +struct kvaser_usb_net_leaf_priv {
    > + struct kvaser_usb_net_priv *net;
    > +
    > + struct delayed_work chip_state_req_work;
    > +};
    > +
    > static const struct can_bittiming_const kvaser_usb_leaf_bittiming_const = {
    > .name = "kvaser_usb",
    > .tseg1_min = KVASER_USB_TSEG1_MIN,
    > @@ -757,6 +765,16 @@ static int kvaser_usb_leaf_simple_cmd_async(struct kvaser_usb_net_priv *priv,
    > return err;
    > }
    >
    > +static void kvaser_usb_leaf_chip_state_req_work(struct work_struct *work)
    > +{
    > + struct kvaser_usb_net_leaf_priv *leaf =
    > + container_of(work, struct kvaser_usb_net_leaf_priv,
    > + chip_state_req_work.work);
    > + struct kvaser_usb_net_priv *priv = leaf->net;
    > +
    > + kvaser_usb_leaf_simple_cmd_async(priv, CMD_GET_CHIP_STATE);
    > +}
    > +
    > static void
    > kvaser_usb_leaf_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
    > const struct kvaser_usb_err_summary *es,
    > @@ -828,6 +846,7 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
    > struct sk_buff *skb;
    > struct net_device_stats *stats;
    > struct kvaser_usb_net_priv *priv;
    > + struct kvaser_usb_net_leaf_priv *leaf;
    > enum can_state old_state, new_state;
    >
    > if (es->channel >= dev->nchannels) {
    > @@ -837,6 +856,7 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
    > }
    >
    > priv = dev->nets[es->channel];
    > + leaf = priv->sub_priv;
    > stats = &priv->netdev->stats;
    >
    > /* Update all of the CAN interface's state and error counters before
    > @@ -853,6 +873,14 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
    > kvaser_usb_leaf_rx_error_update_can_state(priv, es, &tmp_cf);
    > new_state = priv->can.state;
    >
    > + /* If there are errors, request status updates periodically as we do
    > + * not get automatic notifications of improved state.
    > + */
    > + if (new_state < CAN_STATE_BUS_OFF &&
    > + (es->rxerr || es->txerr || new_state == CAN_STATE_ERROR_PASSIVE))
    > + schedule_delayed_work(&leaf->chip_state_req_work,
    > + msecs_to_jiffies(500));
    > +
    > skb = alloc_can_err_skb(priv->netdev, &cf);
    > if (!skb) {
    > stats->rx_dropped++;
    > @@ -1312,10 +1340,13 @@ static int kvaser_usb_leaf_start_chip(struct kvaser_usb_net_priv *priv)
    >
    > static int kvaser_usb_leaf_stop_chip(struct kvaser_usb_net_priv *priv)
    > {
    > + struct kvaser_usb_net_leaf_priv *leaf = priv->sub_priv;
    > int err;
    >
    > reinit_completion(&priv->stop_comp);
    >
    > + cancel_delayed_work(&leaf->chip_state_req_work);
    > +
    > err = kvaser_usb_leaf_send_simple_cmd(priv->dev, CMD_STOP_CHIP,
    > priv->channel);
    > if (err)
    > @@ -1362,6 +1393,31 @@ static int kvaser_usb_leaf_init_card(struct kvaser_usb *dev)
    > return 0;
    > }
    >
    > +static int kvaser_usb_leaf_init_channel(struct kvaser_usb_net_priv *priv)
    > +{
    > + struct kvaser_usb_net_leaf_priv *leaf;
    > +
    > + leaf = devm_kzalloc(&priv->dev->intf->dev, sizeof(*leaf), GFP_KERNEL);
    > + if (!leaf)
    > + return -ENOMEM;
    > +
    > + leaf->net = priv;
    > + INIT_DELAYED_WORK(&leaf->chip_state_req_work,
    > + kvaser_usb_leaf_chip_state_req_work);
    > +
    > + priv->sub_priv = leaf;
    > +
    > + return 0;
    > +}
    > +
    > +static void kvaser_usb_leaf_remove_channel(struct kvaser_usb_net_priv *priv)
    > +{
    > + struct kvaser_usb_net_leaf_priv *leaf = priv->sub_priv;
    > +
    > + if (leaf)
    > + cancel_delayed_work_sync(&leaf->chip_state_req_work);
    > +}
    > +
    > static int kvaser_usb_leaf_set_bittiming(struct net_device *netdev)
    > {
    > struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
    > @@ -1463,6 +1519,8 @@ const struct kvaser_usb_dev_ops kvaser_usb_leaf_dev_ops = {
    > .dev_get_berr_counter = kvaser_usb_leaf_get_berr_counter,
    > .dev_setup_endpoints = kvaser_usb_leaf_setup_endpoints,
    > .dev_init_card = kvaser_usb_leaf_init_card,
    > + .dev_init_channel = kvaser_usb_leaf_init_channel,
    > + .dev_remove_channel = kvaser_usb_leaf_remove_channel,
    > .dev_get_software_info = kvaser_usb_leaf_get_software_info,
    > .dev_get_software_details = NULL,
    > .dev_get_card_info = kvaser_usb_leaf_get_card_info,

    \
     
     \ /
      Last update: 2022-06-06 17:33    [W:2.220 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site