lkml.org 
[lkml]   [2022]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 11/12] can: kvaser_usb_leaf: Ignore stale bus-off after start
From
On 5/16/22 15:47, Anssi Hannula wrote:
> With 0bfd:0124 Kvaser Mini PCI Express 2xHS FW 4.18.778 it was observed
> that if the device was bus-off when stopped, at next start (either via
> interface down/up or manual bus-off restart) the initial
> CMD_CHIP_STATE_EVENT received just after CMD_START_CHIP_REPLY will have
> the M16C_STATE_BUS_OFF bit still set, causing the interface to
> immediately go bus-off again.

I'm able to reproduce this and it definitely looks like a bug in
firmware. I've created a case for this in our backlog, but I don't know
which priority it will get.

> The bit seems to internally clear quickly afterwards but we do not get
> another CMD_CHIP_STATE_EVENT.
>
> Fix the issue by ignoring any initial bus-off state until we see at
> least one bus-on state. Also, poll the state periodically until that
> occurs.
>
> It is possible we lose one actual immediately occurring bus-off event
> here in which case the HW will auto-recover and we see the recovery
> event. We will then catch the next bus-off event, if any.
>
> This issue did not reproduce with 0bfd:0017 Kvaser Memorator
> Professional HS/HS FW 2.0.50.
>
> Fixes: 71873a9b38d1 ("can: kvaser_usb: Add support for more Kvaser Leaf v2 devices")
> Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>

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

> ---
> .../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 31 ++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> 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 742626e69dd8..4125074c7066 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> @@ -401,6 +401,9 @@ struct kvaser_usb_net_leaf_priv {
> struct kvaser_cmd_busparams params_response;
>
> struct delayed_work chip_state_req_work;
> +
> + /* started but not reported as bus-on yet */
> + bool joining_bus;
> };
>
> static const struct can_bittiming_const kvaser_usb_leaf_bittiming_const = {
> @@ -800,6 +803,7 @@ kvaser_usb_leaf_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
> const struct kvaser_usb_err_summary *es,
> struct can_frame *cf)
> {
> + struct kvaser_usb_net_leaf_priv *leaf = priv->sub_priv;
> struct kvaser_usb *dev = priv->dev;
> struct net_device_stats *stats = &priv->netdev->stats;
> enum can_state cur_state, new_state, tx_state, rx_state;
> @@ -824,6 +828,22 @@ kvaser_usb_leaf_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
> new_state = CAN_STATE_ERROR_ACTIVE;
> }
>
> + /* 0bfd:0124 FW 4.18.778 was observed to send the initial
> + * CMD_CHIP_STATE_EVENT after CMD_START_CHIP with M16C_STATE_BUS_OFF
> + * bit set if the channel was bus-off when it was last stopped (even
> + * across chip resets). This bit will clear shortly afterwards, without
> + * triggering a second unsolicited chip state event.
> + * Ignore this initial bus-off.
> + */
> + if (leaf->joining_bus) {
> + if (new_state == CAN_STATE_BUS_OFF) {
> + netdev_dbg(priv->netdev, "ignoring bus-off during startup");
> + new_state = cur_state;
> + } else {
> + leaf->joining_bus = false;
> + }
> + }
> +
> if (new_state != cur_state) {
> tx_state = (es->txerr >= es->rxerr) ? new_state : 0;
> rx_state = (es->txerr <= es->rxerr) ? new_state : 0;
> @@ -899,9 +919,12 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
>
> /* If there are errors, request status updates periodically as we do
> * not get automatic notifications of improved state.
> + * Also request updates if we saw a stale BUS_OFF during startup
> + * (joining_bus).
> */
> if (new_state < CAN_STATE_BUS_OFF &&
> - (es->rxerr || es->txerr || new_state == CAN_STATE_ERROR_PASSIVE))
> + (es->rxerr || es->txerr || new_state == CAN_STATE_ERROR_PASSIVE ||
> + leaf->joining_bus))
> schedule_delayed_work(&leaf->chip_state_req_work,
> msecs_to_jiffies(500));
>
> @@ -1392,8 +1415,11 @@ static int kvaser_usb_leaf_set_opt_mode(const struct kvaser_usb_net_priv *priv)
>
> static int kvaser_usb_leaf_start_chip(struct kvaser_usb_net_priv *priv)
> {
> + struct kvaser_usb_net_leaf_priv *leaf = priv->sub_priv;
> int err;
>
> + leaf->joining_bus = true;
> +
> reinit_completion(&priv->start_comp);
>
> err = kvaser_usb_leaf_send_simple_cmd(priv->dev, CMD_START_CHIP,
> @@ -1566,6 +1592,7 @@ static int kvaser_usb_leaf_set_mode(struct net_device *netdev,
> enum can_mode mode)
> {
> struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> + struct kvaser_usb_net_leaf_priv *leaf = priv->sub_priv;
> int err;
>
> switch (mode) {
> @@ -1576,6 +1603,8 @@ static int kvaser_usb_leaf_set_mode(struct net_device *netdev,
>
> kvaser_usb_unlink_tx_urbs(priv);
>
> + leaf->joining_bus = true;
> +
> err = kvaser_usb_leaf_simple_cmd_async(priv, CMD_START_CHIP);
> if (err)
> return err;

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