Re: Bug? -- NEVER getting controller-problem{back-to-error-active}

[Wolfgang Grandegger <wg@grandegger.com>]

Hello Stephane,

Am 21.06.2016 um 10:29 schrieb Stephane Grosjean:
> Hi,
>
> The reason of this:
>
>          } else {
>                  /* no error bit (so, no error skb, back to active
> state) */
>                  dev->can.state = CAN_STATE_ERROR_ACTIVE;
>                  pdev->bec.txerr = 0;
>                  pdev->bec.rxerr = 0;
>                  return 0;
>          }
>
> is given by the comment: AFAIR in these times, going (back) to
> ACTIVE_STATE was made like above, by the device driver itself.

Well, in the past we didn't care about *decreasing* state changes but 
new driver should, and this does include the "back-to-error-active" 
message, which is automatically generated by "change_state()". So far, 
only a few driver are using that function:

   $ grep -rl can_change_state .
   ./sja1000/sja1000.c
   ./usb/peak_usb/pcan_usb_fd.c
   ./usb/kvaser_usb.c
   ./flexcan.c
   ./mscan/mscan.c

> The lower condition:
>
>          /* allocate an skb to store the error frame */
>          skb = alloc_can_err_skb(netdev, &cf);
>          if (skb)
>                  can_change_state(netdev, cf, tx_state, rx_state);
>
> uses can_change_state() if and only if "skb" is not NULL, while there
> was no need of any skb to go (back) to ACTIVE_STATE.
>
> So, I suppose that the proposed change should also include something
> like the following one:
>
>           /* allocate an skb to store the error frame */
>           skb = alloc_can_err_skb(netdev, &cf);
> -        if (skb)
> +        if (skb || (new_state == CAN_STATE_ERROR_ACTIVE))
>                   can_change_state(netdev, cf, tx_state, rx_state);

can_change_state() should not be called if "skb" is NULL (because cf is 
not valid).

Wolfgang.