Re: AW: AW: flexcan missing error state transitions

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

Hello Mark,

Quoting Jonas Mark (ST-FIR/ENG1) (2017-08-22 13:50:35)
> > > Which polling interval do you think would be appropriate? It somehow has to be
> > a reasonable trade-off between latency and CPU usage.
> > That's a good question. I don't think that there is a really good answer to it.
> > It would be nice to see intermittent state-transitions happening, so I would say as
> > fast as possible without affecting CPU usage to a significant degree. The polling
> > timer should not be a high resolution timer or anything that would preempt an rt
> > task.
> > 
> > This value could also depend on the bitrate as that will affect the rate at which the
> > error-counter can change. E.g. on 125 k rate, you will receive at most one frame
> > per 1 ms. This means that the error state will change at at least
> > 64 ms intervals, so we would have to sample at 32 ms intervals to catch all the
> > transitions. For 1 M this would be around 2 ms.
> > 
> > Note that this is all somewhat speculative as I do not know exactly how fast the
> > error counters are incremented/decremented.
> 
> We agree that there is a sensible lower boundary for the polling interval which depends on the bitrate. And there is a upper boundary which depends on the application.
> 
> We are currently thinking about whether it would be possible to do the polling only in case the controller is actually experiencing trouble.
> 
> For the i.MX6 there is an interrupt when the controller changes from Error Active to Error Warning. This shall enable the polling.
> 
> For the i.MX53 there is no Active -> Warning interrupt. But for all kinds of error events on the bus an Error interrupt will be created. So the polling shall be enabled then. This is a little bit less efficient compared to i.MX6 but the polling will be disabled during the next interrupt if the state is still Error Active (see below).
> 
> For both, the polling can be disabled when the status is read and it is Error Active. The polling shall be disabled as well when a Bus-Off interrupt occurs.
> 
> What we still need to resolve is the concurrency between the timer and the interrupt when accessing internal data structures and sending SocketCAN error frames. Once we have that sorted out we will send a patch. None the less, early feedback is welcome.
The last time we discussed this issue, I suggested something similar and
Wolfgang's response was "Puh, that's far too sophisticated."

What you suggest does not sound overly complicated, so I would say it's a good
idea. It seems like it's the best that we can do with what we have.

Wolfgang, Marc, what do you think?
> 
> > > Would it make sense to make the polling interval configurable via device tree?
> > Also a good questions. I tried compiling a C program that runs usleep(2000) in a
> > loop for the i.MX6. It runs at around 1% CPU usage in top. I would call that
> > significant. Perhaps it would run better in kernel space?
> > 
> > If the polling interval can be configured, users who care about it can set it to a
> > value that matches their bitrate. Others could set it to zero to turn it off.
> 
> We also think that the polling interval should be configurable. It strongly depends on the application which latency between state change and notification is acceptable and how much computing resources may be invested to achieve that.
Good.
> 
> > As the useful sampling frequency depends on the bitrate, perhaps it should even
> > be a part of the netlink interface? This would also make it easier for the user to find
> > a good sampling frequency based on trial and error.
> 
> Right now we are not convinced that adding to the netlink interface is justified to work around deficiencies of a single CAN peripheral.
Fair enough.

Regards,
Andri