Re: [PATCH V5 1/1] can: Add support for esd CAN PCIe/402 card

[Andri Yngvason <andri.yngvason@marel.com>]

Quoting Ahmed S. Darwish (2015-03-17 07:26:39)
> Hi Thomas,
> 
> On Tue, Mar 17, 2015 at 07:30:02AM +0100, Thomas Körper wrote:
> > Hi Andri,
> > 
> > I've looked at the sources you mentioned, but I'm a little bit unsure now /
> > the handling seems not perfectly consistent to me. (flexcan calls
> > can_change_state() with tx/rx_state of 0 in the bus off path.
> >
> 
> There're discrpeancies indeed on the BUSOFF path:
> 
> 1) flexcan calls can_change_state(dev, cf, 0, 0). kvaser and others
>    do can_change_state(dev, cf, tx, rx) where either tx or rx =
>    CAN_STATE_BUS_OFF. In this case, kvaser and the other drivers
>    are the _correct_ behavior. This is validated by can-dev
>    can_chage_state() code:
> 
>         if (unlikely(new_state == CAN_STATE_BUS_OFF)) {
>                 cf->can_id |= CAN_ERR_BUSOFF;
>                 return;
>         }
Indeed, I'll have to fix that. Well spotted Thomas!
> 
> 2) flexcan calls can_bus_off(dev), while kvaser goes a layer deeper
>    and directly calls netif_carrier_off(netdev). In this case,
>    flexcan is the more correct behavior.
> 
IMO, that's fine as long as it doesn't add to inconsistency between outwards
behaviour of the system; which it doesn't.

Best regards,
Andri