RE: mcp25xxfd: missing cerrif

[<Thomas.Kopp@microchip.com>]

> Hmmm. I think the question is, is there a dedicated warning state and
> if, how are the transitions from and to it?
I think the main confusion comes from the fact that error warning is not a state as defined in the ISO. The ISO defines three states: error-active, error-passive and bus-off.
The error warning is an interrupt that (most?) CAN IPs issue when the error counters exceeds 96. AFAIK this is only a convention though, not a spec (I know that Kvaser and the Bosch MCAN handle it like this).

> ACK. The documentation talks about error passive to error active, which
> means the diagram looks like this:
Not quite, that's how it should look like:
                     -- E > 95 --         -- E > 127 --               -- E > 255 --
                      /            \          /             \                    /             \
                    /              V         /               V                /               V
 --> error active  (error warning)      error passive    bus off
           ^              /                      ^         /
            \            /                         \       /
             -- E < 96 --                   -- E < 128 --

The enum in netlink.h mixes the error states plus some other states. To fulfill the ISO an interrupt from Error warning to Error active isn't really useful. I suppose that's why it's not implemented in the MCP2518FD.

enum can_state {
	CAN_STATE_ERROR_ACTIVE = 0,	/* RX/TX error count < 96 */
	CAN_STATE_ERROR_WARNING,	/* RX/TX error count < 128 */
	CAN_STATE_ERROR_PASSIVE,	/* RX/TX error count < 256 */
	CAN_STATE_BUS_OFF,		/* RX/TX error count >= 256 */
	CAN_STATE_STOPPED,		/* Device is stopped */
	CAN_STATE_SLEEPING,		/* Device is sleeping */
	CAN_STATE_MAX
};

Quickly looking at different CAN drivers in the kernel I don't really see a common pattern how this counter is used.
Taking the M_CAN driver as an example (drivers/net/can/m_can/m_can.c#L671) I see that a state change from error passive to error active (with the error warning bit being set) leads to the error warning counter being incremented and the state being updated to CAN_STATE_ERROR_WARNING.
I did not find a documentation on what this error counter is supposed to show - I suppose I didn't look in the right place. But while the number of times the device was in error warning state has technically gone up by one I would have expected that this counts the switches. Again, this is not based on documentation (I'm not really familiar with the SocketCAN so it's likely I missed something) but rather on what I would have expected intuitively.

One more thing I noticed in the mcan driver is that there doesn't seem to be a way to go from error warning to error active without ifdown/ifup. So the driver remains in in error warning state for a significant time.

Another example in the kvaser_usb_hydra.c:
 * Known issues:
 *  - Transition from CAN_STATE_ERROR_WARNING to CAN_STATE_ERROR_ACTIVE is only
 *    reported after a call to do_get_berr_counter(), since firmware does not
 *    distinguish between ERROR_WARNING and ERROR_ACTIVE.

> I think we have to discuss how the state diagram on socketcan should
>look like.

I agree. How would that discussion happen?

Best Regards,
Thomas