Hello Wolfgang,

From: Wolfgang Grandegger [mailto:wg@grandegger.com]
Sent: Wednesday, August 30, 2017 2:26 PM

Am 30.08.2017 um 06:22 schrieb ZHU Yi (ST-FIR/ENG1-Zhu):

quoted

quoted

From: Wolfgang Grandegger [mailto:wg@grandegger.com]
Sent: Tuesday, August 29, 2017 7:17 PM

Am 29.08.2017 um 10:49 schrieb ZHU Yi (ST-FIR/ENG1-Zhu):
[...]

For the legacy FLEXCAN_QUIRK_BROKEN_ERR_STATE we do not want to
disable
bus error interrupts as well. To handle that properly, I think we need

I'm not sure why we do not want to disable error interrupt for legacy broken core,
I think if this approach works for both (with/without warning irq) core, then
disable the interrupts helps avoid flooding.

How does it work if no cable is connected?  On a legacy device (with
broken [TR]WRN_INT), no interrupt will arrive other than the bus error
interrupt. Maybe I have missed something. Unfortunately, I do not have
any Flexcan device at hand to check myself.

The bus error interrupt means error counter changed, and we can derive correct
state from that. Ah, the node needs to try to send something, otherwise,
the state will not change, but this is valid for all can controllers.

quoted

quoted

quoted

One more thing regarding patch 2 is, it doesn't rely on error warning
can be reported by [TR]WRN_INT, because this state can also be reported
upon the _any_ interrupt. Only bus-off state needs to have dedicated
interrupt line (is this true for all flexcan core?)

I don't think so. How should it work if no cable is connected? Anyway,
the patch seems not to be to lenghty.

I disabled the [TR]WRN_INT on i.MX6 by change
(http://elixir.free-electrons.com/linux/latest/source/drivers/net/can/flexcan.c#L88)
to:
#define FLEXCAN_CTRL_ERR_STATE FLEXCAN_CTRL_BOFF_MSK
and tested with the patch, seems the state transitions still works fine.

With no cable connected?

Yes, 
1. Unplug the cable will change to error passive
2. plug back the cable will eventually change to error active
3. unplug the cable while the bus is recovering to error warning / error active,
it will report error passive again
4. repeat plug/unplug the cable and watched the state change as expected

quoted

As we only have i.MX6 at hand, so we cannot verify this on other flexcan core.
If we want to limit the patch to only take effect for i.MX6, then we should

introduce

quoted

the FLEXCAN_QUIRK_BROKEN_P(W)ERR_STATE, thus the legacy core will not

be

quoted

affected.

As you/I cannot test on legacy and other cores, I would go for a
separate quirk for the mx6q.

OK.

quoted

Shall I send separate e-mail for each patch? Due to our mail client is outlook, I'm
not sure whether the inline patch can work smoothly, if not, shall I attach the

patch?

Separate patches please, especially for the first one.

OK.

Best regards
Yi