[PATCH 3/3] can: flexcan.c: Implement last step of workaround for errata ERR005829

[David Jander]

It is not clear if this errata only applies to i.MX6, but the impact of
the workaround should be minimal. Basically it comes down to reserving
mailbox 0 as a permanently inactive MB and writing twice to its C/S field
on every message transmit.

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/flexcan.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index bf444ff..3998d4c 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -434,6 +434,10 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);

+ /* Errata ERR005829 step8: Write twice INACTIVE(0b1000) code to MB 0 */
+ flexcan_write(FLEXCAN_MB_CNT_CODE(0x8), &regs->cantxfg[0].can_ctrl);
+ flexcan_write(FLEXCAN_MB_CNT_CODE(0x8), &regs->cantxfg[0].can_ctrl);
+
return NETDEV_TX_OK;
}

[David Jander]

This post might be inappropriate. Click to display it.

[Marc Kleine-Budde]

Post by David Jander
The FlexCAN controller has a RX FIFO that is only 6 messages deep, and a
mailbox space capable of holding up to 63 messages.
This space was largely unused, limiting the permissible latency from
interrupt to NAPI to only 6 messages. This patch uses all available MBs
for message reception and frees the MBs in the IRQ handler to greatly
decrease the likelihood of receive overruns.

What about the order of the incoming CAN frames? Is it still preserved?

You make use of the CTRL2 register, which is not present on some older
(but supported) flexcan IP cores. You increase FLEXCAN_MCR_MAXMB to
0x40, which is not supported on older IPs. The register rximr, is also
not present on older cores. Don't break support for the older CAN cores.

Please make this patch based on linux-can-next/master (which holds some
updates to the regs structure).

Marc

[David Jander]

On Tue, 02 Sep 2014 13:30:24 +0200

Post by Marc Kleine-Budde

Post by David Jander
The FlexCAN controller has a RX FIFO that is only 6 messages deep, and a
mailbox space capable of holding up to 63 messages.
This space was largely unused, limiting the permissible latency from
interrupt to NAPI to only 6 messages. This patch uses all available MBs
for message reception and frees the MBs in the IRQ handler to greatly
decrease the likelihood of receive overruns.

What about the order of the incoming CAN frames? Is it still preserved?

Yes, it is preserved as long as latency doesn't exceed 30 frames, which is
way more than the original driver could take. The algorithm is not trivial,
therefor I included an explanatory comment to flexcan_copy_rxmbs().

Post by Marc Kleine-Budde
You make use of the CTRL2 register, which is not present on some older
(but supported) flexcan IP cores. You increase FLEXCAN_MCR_MAXMB to
0x40, which is not supported on older IPs. The register rximr, is also
not present on older cores. Don't break support for the older CAN cores.

Oops. Thanks for pointing that out. I will check the reference manual of the
i.MX53 (which should have be the oldest supported version of this IP core,
right?). Of course I do not want to break older CAN cores.
I will check correctness testing the code on an i.MX28 board which I have.

Indeed the MAXMB field in IP-version 3 seems to be 6 bits wide instead of 7,
which doesn't make sense, since there are still 64 MB's. If one would want all
MB's to take part in the arbitration process, one would need to write 0x40 to
this register, which doesn't fit. Could it be that this is just a
documentation error? I would bet so. Who knows?
We can settle for 0x3f to be safe, can't we?

Post by Marc Kleine-Budde
Please make this patch based on linux-can-next/master (which holds some
updates to the regs structure).

Thanks for pointing out. I will use that as base for the next version.

Best regards,

[Marc Kleine-Budde]

Post by David Jander
On Tue, 02 Sep 2014 13:30:24 +0200

Post by Marc Kleine-Budde

Post by David Jander
The FlexCAN controller has a RX FIFO that is only 6 messages deep, and a
mailbox space capable of holding up to 63 messages.
This space was largely unused, limiting the permissible latency from
interrupt to NAPI to only 6 messages. This patch uses all available MBs
for message reception and frees the MBs in the IRQ handler to greatly
decrease the likelihood of receive overruns.

What about the order of the incoming CAN frames? Is it still preserved?

Yes, it is preserved as long as latency doesn't exceed 30 frames, which is
way more than the original driver could take. The algorithm is not trivial,
therefor I included an explanatory comment to flexcan_copy_rxmbs().

To be honest, I haven't looked at the algorithm in detail. I think we
first have to fix both bugs (the errata and the mailbox initialization
issue).

Post by David Jander

Post by Marc Kleine-Budde
You make use of the CTRL2 register, which is not present on some older
(but supported) flexcan IP cores. You increase FLEXCAN_MCR_MAXMB to
0x40, which is not supported on older IPs. The register rximr, is also
not present on older cores. Don't break support for the older CAN cores.

Oops. Thanks for pointing that out. I will check the reference manual of the
i.MX53 (which should have be the oldest supported version of this IP core,
right?). Of course I do not want to break older CAN cores.
I will check correctness testing the code on an i.MX28 board which I have.

Regarding the versions of the IP cores, we should not trust them, as
they are not public available..

Marc

[David Jander]

On Tue, 02 Sep 2014 16:53:46 +0200

Post by Marc Kleine-Budde

Post by David Jander
On Tue, 02 Sep 2014 13:30:24 +0200

Post by Marc Kleine-Budde

Post by David Jander
The FlexCAN controller has a RX FIFO that is only 6 messages deep, and a
mailbox space capable of holding up to 63 messages.
This space was largely unused, limiting the permissible latency from
interrupt to NAPI to only 6 messages. This patch uses all available MBs
for message reception and frees the MBs in the IRQ handler to greatly
decrease the likelihood of receive overruns.

What about the order of the incoming CAN frames? Is it still preserved?

Yes, it is preserved as long as latency doesn't exceed 30 frames, which is
way more than the original driver could take. The algorithm is not trivial,
therefor I included an explanatory comment to flexcan_copy_rxmbs().

To be honest, I haven't looked at the algorithm in detail. I think we
first have to fix both bugs (the errata and the mailbox initialization
issue).

Ok, I agree.

Post by Marc Kleine-Budde

Post by David Jander

Post by Marc Kleine-Budde
You make use of the CTRL2 register, which is not present on some older
(but supported) flexcan IP cores. You increase FLEXCAN_MCR_MAXMB to
0x40, which is not supported on older IPs. The register rximr, is also
not present on older cores. Don't break support for the older CAN cores.

Oops. Thanks for pointing that out. I will check the reference manual of
the i.MX53 (which should have be the oldest supported version of this IP
core, right?). Of course I do not want to break older CAN cores.
I will check correctness testing the code on an i.MX28 board which I have.

Regarding the versions of the IP cores, we should not trust them, as
they are not public available..

Ok. I fear that for this last patch we will need some run-time check for the
version of the flexcan we have. I suppose the typical table with OF ids and
flags will do?

Best regards,

[Marc Kleine-Budde]

Post by David Jander

Post by Marc Kleine-Budde

Post by David Jander

Post by Marc Kleine-Budde
You make use of the CTRL2 register, which is not present on some older
(but supported) flexcan IP cores. You increase FLEXCAN_MCR_MAXMB to
0x40, which is not supported on older IPs. The register rximr, is also
not present on older cores. Don't break support for the older CAN cores.

Oops. Thanks for pointing that out. I will check the reference manual of
the i.MX53 (which should have be the oldest supported version of this IP
core, right?). Of course I do not want to break older CAN cores.
I will check correctness testing the code on an i.MX28 board which I have.

Regarding the versions of the IP cores, we should not trust them, as
they are not public available..

Ok. I fear that for this last patch we will need some run-time check for the
version of the flexcan we have. I suppose the typical table with OF ids and
flags will do?

Yes, we already track this information in

Post by David Jander
struct flexcan_devtype_data {
u32 features; /* hardware controller features */
};
#define FLEXCAN_HAS_V10_FEATURES BIT(1) /* For core version >= 10 */

which is probably what you need. Feel free to add another bit, if you
need to.

Marc

[David Jander]

On Tue, 02 Sep 2014 13:30:24 +0200

Post by Marc Kleine-Budde

Post by David Jander
The FlexCAN controller has a RX FIFO that is only 6 messages deep, and a
mailbox space capable of holding up to 63 messages.
This space was largely unused, limiting the permissible latency from
interrupt to NAPI to only 6 messages. This patch uses all available MBs
for message reception and frees the MBs in the IRQ handler to greatly
decrease the likelihood of receive overruns.

What about the order of the incoming CAN frames? Is it still preserved?
You make use of the CTRL2 register, which is not present on some older
(but supported) flexcan IP cores. You increase FLEXCAN_MCR_MAXMB to
0x40, which is not supported on older IPs. The register rximr, is also
not present on older cores. Don't break support for the older CAN cores.

I've checked the RM of i.MX53, i.MX28 and i.MX25, and they all seem to support
the rximr registers. Which IP version doesn't support them (that is still
supported by this driver anyway)?

AFAICS, the only unsupported feature on older SoC's is the CTRL2 register, and
the fact that the documentation says the MCR_MAXMB field is only 6 bits wide,
right?

Best regards,

[David Jander]

Apparently mailboxes may contain random data at startup, causing some of
them being prepared for message reception. This causes overruns being
missed or even confusing the IRQ check for trasmitted messages, increasing
the transmit counter instead of the error counter.

Signed-off-by: David Jander <***@protonic.nl>
---
drivers/net/can/flexcan.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 944aa5d..a9700f3 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -801,6 +801,7 @@ static int flexcan_chip_start(struct net_device *dev)
struct flexcan_regs __iomem *regs = priv->base;
int err;
u32 reg_mcr, reg_ctrl;
+ int i;

/* enable module */
err = flexcan_chip_enable(priv);
@@ -867,9 +868,11 @@ static int flexcan_chip_start(struct net_device *dev)
netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
flexcan_write(reg_ctrl, &regs->ctrl);

- /* Abort any pending TX, mark Mailbox as INACTIVE */
- flexcan_write(FLEXCAN_MB_CNT_CODE(0x4),
- &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
+ /* Clear and invalidate all mailboxes first */
+ for (i = 0; i < 64; i++) {
+ flexcan_write(FLEXCAN_MB_CNT_CODE(0),
+ &regs->cantxfg[i].can_ctrl);
+ }

/* acceptance mask/acceptance code (accept everything) */
flexcan_write(0x0, &regs->rxgmask);

[Marc Kleine-Budde]

Post by David Jander
Apparently mailboxes may contain random data at startup, causing some of
them being prepared for message reception. This causes overruns being
missed or even confusing the IRQ check for trasmitted messages, increasing
the transmit counter instead of the error counter.

Before patch

0d1862e can: flexcan: fix flexcan_chip_start() on imx6

there was a loop clearing the whole cantxfg register space. But this
turned out to be bogus, as message buffers 1...7 are reserved by the
FIFO engine and we're not allowed to tough them. This lead to some kind
of abort on imx6.

You may need this patch once you don't make use of the FIFO engine any more.

Marc

[David Jander]

On Tue, 02 Sep 2014 12:24:28 +0200

Post by Marc Kleine-Budde

Post by David Jander
Apparently mailboxes may contain random data at startup, causing some of
them being prepared for message reception. This causes overruns being
missed or even confusing the IRQ check for trasmitted messages, increasing
the transmit counter instead of the error counter.

Before patch
0d1862e can: flexcan: fix flexcan_chip_start() on imx6
there was a loop clearing the whole cantxfg register space. But this
turned out to be bogus, as message buffers 1...7 are reserved by the
FIFO engine and we're not allowed to tough them. This lead to some kind
of abort on imx6.
You may need this patch once you don't make use of the FIFO engine any more.

You will need this patch in either case, but indeed, if you use the FIFO, you
should skip the MB's that are shadowed by the FIFO.
If you don't clear the rest of the MB's they may still contain random data and
the problem remains.
IMHO 0d1862e is wrong, since buffers are not in reset default values. There is
no indication of that in the reference manual, and I have observed that they
are indeed not cleared after reset.

Best regards,

[Marc Kleine-Budde]

Post by David Jander
On Tue, 02 Sep 2014 12:24:28 +0200

Post by Marc Kleine-Budde

Post by David Jander
Apparently mailboxes may contain random data at startup, causing some of
them being prepared for message reception. This causes overruns being
missed or even confusing the IRQ check for trasmitted messages, increasing
the transmit counter instead of the error counter.

Before patch
0d1862e can: flexcan: fix flexcan_chip_start() on imx6
there was a loop clearing the whole cantxfg register space. But this
turned out to be bogus, as message buffers 1...7 are reserved by the
FIFO engine and we're not allowed to tough them. This lead to some kind
of abort on imx6.
You may need this patch once you don't make use of the FIFO engine any more.

You will need this patch in either case, but indeed, if you use the FIFO, you
should skip the MB's that are shadowed by the FIFO.

ACK

Post by David Jander
If you don't clear the rest of the MB's they may still contain random data and
the problem remains.
IMHO 0d1862e is wrong, since buffers are not in reset default values. There is
no indication of that in the reference manual, and I have observed that they
are indeed not cleared after reset.

Yes, 0d1862e was not complete, the initialisation was fixes with:

d5a7b40 can: flexcan: flexcan_chip_start: fix regression,
mark one MB for TX and abort pending TX

Which sets FLEXCAN_MCR_MAXMB to 8, which is the only mailbox used for tx
and the code of the tx mailbox is set to 0x4 == tx, inactive.

This should be enough in FIFO mode, correct?

Marc

[David Jander]

On Tue, 02 Sep 2014 12:59:42 +0200

Post by Marc Kleine-Budde

Post by David Jander
On Tue, 02 Sep 2014 12:24:28 +0200

Post by Marc Kleine-Budde

Post by David Jander
Apparently mailboxes may contain random data at startup, causing some of
them being prepared for message reception. This causes overruns being
missed or even confusing the IRQ check for trasmitted messages,
increasing the transmit counter instead of the error counter.

Before patch
0d1862e can: flexcan: fix flexcan_chip_start() on imx6
there was a loop clearing the whole cantxfg register space. But this
turned out to be bogus, as message buffers 1...7 are reserved by the
FIFO engine and we're not allowed to tough them. This lead to some kind
of abort on imx6.
You may need this patch once you don't make use of the FIFO engine any more.

You will need this patch in either case, but indeed, if you use the FIFO,
you should skip the MB's that are shadowed by the FIFO.

ACK

Post by David Jander
If you don't clear the rest of the MB's they may still contain random data
and the problem remains.
IMHO 0d1862e is wrong, since buffers are not in reset default values.
There is no indication of that in the reference manual, and I have
observed that they are indeed not cleared after reset.

d5a7b40 can: flexcan: flexcan_chip_start: fix regression,
mark one MB for TX and abort pending TX
Which sets FLEXCAN_MCR_MAXMB to 8, which is the only mailbox used for tx
and the code of the tx mailbox is set to 0x4 == tx, inactive.
This should be enough in FIFO mode, correct?

AFAICS not. There could still be other MB's with reception or transmission
enabled (randomly) causing potential data loss, extra frames sent and/or errors
in the statistics.
If the FIFO is full for example it should overflow with the next message, but
if the next message instead hits an (randomly) empty and readied RX MB
somewhere, the overflow is undetected and one (or more) frame(s) is lost.

Best regards,

[Marc Kleine-Budde]

Post by David Jander

Post by Marc Kleine-Budde
d5a7b40 can: flexcan: flexcan_chip_start: fix regression,
mark one MB for TX and abort pending TX
Which sets FLEXCAN_MCR_MAXMB to 8, which is the only mailbox used for tx
and the code of the tx mailbox is set to 0x4 == tx, inactive.
This should be enough in FIFO mode, correct?

AFAICS not. There could still be other MB's with reception or transmission
enabled (randomly) causing potential data loss, extra frames sent and/or errors
in the statistics.
If the FIFO is full for example it should overflow with the next message, but
if the next message instead hits an (randomly) empty and readied RX MB
somewhere, the overflow is undetected and one (or more) frame(s) is lost.

Is FIFO to normal mailbox overflow a new feature of the imx6 flexcan

Post by David Jander
If the FIFO is full and more frames continue to be received, an
OVERFLOW interrupt is issued to the ARM and subsequent frames are not
accepted until the ARM creates space in the FIFO by reading one or
more frames.
Note that the flag will not be asserted when the Rx FIFO is
full and the message was captured by a Mailbox.
In the event that the FIFO is full, the matching algorithm always
looks for a matching message buffer outside the FIFO region.

This probably means even on the mx35 the _FIFO_ does not accept any more
message if it's full, but the other mailboxes may....

Marc

[David Jander]

On Tue, 02 Sep 2014 15:54:06 +0200

Post by Marc Kleine-Budde

Post by David Jander

Post by Marc Kleine-Budde
d5a7b40 can: flexcan: flexcan_chip_start: fix regression,
mark one MB for TX and abort pending TX
Which sets FLEXCAN_MCR_MAXMB to 8, which is the only mailbox used for tx
and the code of the tx mailbox is set to 0x4 == tx, inactive.
This should be enough in FIFO mode, correct?

AFAICS not. There could still be other MB's with reception or transmission
enabled (randomly) causing potential data loss, extra frames sent and/or
errors in the statistics.
If the FIFO is full for example it should overflow with the next message,
but if the next message instead hits an (randomly) empty and readied RX MB
somewhere, the overflow is undetected and one (or more) frame(s) is lost.

Is FIFO to normal mailbox overflow a new feature of the imx6 flexcan

Post by David Jander
If the FIFO is full and more frames continue to be received, an
OVERFLOW interrupt is issued to the ARM and subsequent frames are not
accepted until the ARM creates space in the FIFO by reading one or
more frames.
Note that the flag will not be asserted when the Rx FIFO is
full and the message was captured by a Mailbox.
In the event that the FIFO is full, the matching algorithm always
looks for a matching message buffer outside the FIFO region.

This probably means even on the mx35 the _FIFO_ does not accept any more
message if it's full, but the other mailboxes may....
[...]
If the FIFO is enabled, the priority of scanning can be selected between
Mailboxes and FIFO filters. In any case, the matching starts from the lowest
number Message Buffer toward the higher ones. If no match is found within
the first structure then the other is scanned subsequently. In the event
that the FIFO is full, the matching algorithm will always look for a
matching MB outside the FIFO region.

What's not quite clear to me right now, is the function of MCR_MAXMB after
reading this. As far as I have observed, MCR_MAXMB is of no influence for
this. I needed to explicitely invalidate all remaining MB's to avoid flexcan
putting overflow messages there.

Best regards,

[David Jander]

On Tue, 02 Sep 2014 12:59:42 +0200

Post by Marc Kleine-Budde

Post by David Jander
On Tue, 02 Sep 2014 12:24:28 +0200

Post by Marc Kleine-Budde

Post by David Jander
Apparently mailboxes may contain random data at startup, causing some of
them being prepared for message reception. This causes overruns being
missed or even confusing the IRQ check for trasmitted messages,
increasing the transmit counter instead of the error counter.

Before patch
0d1862e can: flexcan: fix flexcan_chip_start() on imx6
there was a loop clearing the whole cantxfg register space. But this
turned out to be bogus, as message buffers 1...7 are reserved by the
FIFO engine and we're not allowed to tough them. This lead to some kind
of abort on imx6.
You may need this patch once you don't make use of the FIFO engine any more.

You will need this patch in either case, but indeed, if you use the FIFO,
you should skip the MB's that are shadowed by the FIFO.

ACK

Post by David Jander
If you don't clear the rest of the MB's they may still contain random data
and the problem remains.
IMHO 0d1862e is wrong, since buffers are not in reset default values.
There is no indication of that in the reference manual, and I have
observed that they are indeed not cleared after reset.

d5a7b40 can: flexcan: flexcan_chip_start: fix regression,
mark one MB for TX and abort pending TX
Which sets FLEXCAN_MCR_MAXMB to 8, which is the only mailbox used for tx
and the code of the tx mailbox is set to 0x4 == tx, inactive.

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 3f21142..f028c5d 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -62,7 +62,7 @@
#define FLEXCAN_MCR_BCC BIT(16)
#define FLEXCAN_MCR_LPRIO_EN BIT(13)
#define FLEXCAN_MCR_AEN BIT(12)
-#define FLEXCAN_MCR_MAXMB(x) ((x) & 0xf)
+#define FLEXCAN_MCR_MAXMB(x) ((x) & 0x1f)
#define FLEXCAN_MCR_IDAM_A (0 << 8)
#define FLEXCAN_MCR_IDAM_B (1 << 8)
#define FLEXCAN_MCR_IDAM_C (2 << 8)
@@ -735,9 +735,11 @@ static int flexcan_chip_start(struct net_device *dev)
*
*/
reg_mcr = flexcan_read(&regs->mcr);
+ reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
- FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS;
+ FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
+ FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
flexcan_write(reg_mcr, &regs->mcr);

Eh! This looks wrong! The MAXMB field is 7 bits wide according to the
reference manual (bits 0-6)... but the reset default value is supposed to be
0x0f, so the mask is still enough to clear the reset default.
What I don't understand is why the CAN controller is still able to put
messages into MB's after the FIFO is full. At least that is what I observed.

Best regards,

[Marc Kleine-Budde]

Post by David Jander
On Tue, 02 Sep 2014 12:59:42 +0200

Post by Marc Kleine-Budde

Post by David Jander
On Tue, 02 Sep 2014 12:24:28 +0200

Post by Marc Kleine-Budde

Post by David Jander
Apparently mailboxes may contain random data at startup, causing some of
them being prepared for message reception. This causes overruns being
missed or even confusing the IRQ check for trasmitted messages,
increasing the transmit counter instead of the error counter.

Before patch
0d1862e can: flexcan: fix flexcan_chip_start() on imx6
there was a loop clearing the whole cantxfg register space. But this
turned out to be bogus, as message buffers 1...7 are reserved by the
FIFO engine and we're not allowed to tough them. This lead to some kind
of abort on imx6.
You may need this patch once you don't make use of the FIFO engine any more.

You will need this patch in either case, but indeed, if you use the FIFO,
you should skip the MB's that are shadowed by the FIFO.

ACK

Post by David Jander
If you don't clear the rest of the MB's they may still contain random data
and the problem remains.
IMHO 0d1862e is wrong, since buffers are not in reset default values.
There is no indication of that in the reference manual, and I have
observed that they are indeed not cleared after reset.

d5a7b40 can: flexcan: flexcan_chip_start: fix regression,
mark one MB for TX and abort pending TX
Which sets FLEXCAN_MCR_MAXMB to 8, which is the only mailbox used for tx
and the code of the tx mailbox is set to 0x4 == tx, inactive.

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 3f21142..f028c5d 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -62,7 +62,7 @@
#define FLEXCAN_MCR_BCC BIT(16)
#define FLEXCAN_MCR_LPRIO_EN BIT(13)
#define FLEXCAN_MCR_AEN BIT(12)
-#define FLEXCAN_MCR_MAXMB(x) ((x) & 0xf)
+#define FLEXCAN_MCR_MAXMB(x) ((x) & 0x1f)
#define FLEXCAN_MCR_IDAM_A (0 << 8)
#define FLEXCAN_MCR_IDAM_B (1 << 8)
#define FLEXCAN_MCR_IDAM_C (2 << 8)
@@ -735,9 +735,11 @@ static int flexcan_chip_start(struct net_device *dev)
*
*/
reg_mcr = flexcan_read(&regs->mcr);
+ reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
- FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS;
+ FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
+ FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
flexcan_write(reg_mcr, &regs->mcr);
Eh! This looks wrong! The MAXMB field is 7 bits wide according to the
reference manual (bits 0-6)... but the reset default value is supposed to be

...according to the imx6 reference manual. The size of the MAXMB field
depends on the actual IP version.

Post by David Jander
0x0f, so the mask is still enough to clear the reset default.
What I don't understand is why the CAN controller is still able to put
messages into MB's after the FIFO is full. At least that is what I observed.

It says to in the imx6 manual, but not in the older ones (see other
mail, that I'm still writing).

Marc

[Marc Kleine-Budde]

Post by Marc Kleine-Budde

Post by David Jander
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 3f21142..f028c5d 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -62,7 +62,7 @@
#define FLEXCAN_MCR_BCC BIT(16)
#define FLEXCAN_MCR_LPRIO_EN BIT(13)
#define FLEXCAN_MCR_AEN BIT(12)
-#define FLEXCAN_MCR_MAXMB(x) ((x) & 0xf)
+#define FLEXCAN_MCR_MAXMB(x) ((x) & 0x1f)
#define FLEXCAN_MCR_IDAM_A (0 << 8)
#define FLEXCAN_MCR_IDAM_B (1 << 8)
#define FLEXCAN_MCR_IDAM_C (2 << 8)
@@ -735,9 +735,11 @@ static int flexcan_chip_start(struct net_device *dev)
*
*/
reg_mcr = flexcan_read(&regs->mcr);
+ reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
- FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS;
+ FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
+ FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
flexcan_write(reg_mcr, &regs->mcr);
Eh! This looks wrong! The MAXMB field is 7 bits wide according to the
reference manual (bits 0-6)... but the reset default value is supposed to be

...according to the imx6 reference manual. The size of the MAXMB field
depends on the actual IP version.

On mcf5213 (some old coldfire, not supported by this driver)
it's 4 bit wide.
On mx25, mx28, mx53 it's 6 bits wide.
On mx6{q,sdl}, vf610 it's 7 bits wide.
I'm not sure about the powerpc based cores as I don't have the data sheets.

I cannot find a data sheet, where it has a width of 5 bit.

I'll prepare a patch to fix it.

Marc

[Marc Kleine-Budde]

Post by David Jander
It is not clear if this errata only applies to i.MX6, but the impact of
the workaround should be minimal. Basically it comes down to reserving
mailbox 0 as a permanently inactive MB and writing twice to its C/S field
on every message transmit.

Please fix this errata in one patch exactly. Please fix the errata
before introducing a new feature, i.e. rewriting of the RX path.

Marc

[David Jander]

On Tue, 02 Sep 2014 13:28:55 +0200

Post by Marc Kleine-Budde

Post by David Jander
It is not clear if this errata only applies to i.MX6, but the impact of
the workaround should be minimal. Basically it comes down to reserving
mailbox 0 as a permanently inactive MB and writing twice to its C/S field
on every message transmit.

Please fix this errata in one patch exactly. Please fix the errata
before introducing a new feature, i.e. rewriting of the RX path.

Ok, I will try to split this out in the next version.

Best regards,