RFC: [PATCH v2] can: flexcan: Implement errata ERR005829

[David Jander]

Signed-off-by: David Jander <***@protonic.nl>
---

changed from v1:
- Implemented complete workaround and based on linux-can/testing

drivers/net/can/flexcan.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 0bcbb96..d9d0ecb 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -125,7 +125,9 @@
FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)

/* FLEXCAN interrupt flag register (IFLAG) bits */
-#define FLEXCAN_TX_BUF_ID 8
+/* Errata ERR005829 step7: Reserve first valid MB */
+#define FLEXCAN_FIRST_VALID_MB 8
+#define FLEXCAN_TX_BUF_ID 9
#define FLEXCAN_IFLAG_BUF(x) BIT(x)
#define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW BIT(7)
#define FLEXCAN_IFLAG_RX_FIFO_WARN BIT(6)
@@ -428,6 +430,12 @@ 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(0x8) code to first MB */
+ flexcan_write(FLEXCAN_MB_CNT_CODE(0x8),
+ &regs->cantxfg[FLEXCAN_FIRST_VALID_MB].can_ctrl);
+ flexcan_write(FLEXCAN_MB_CNT_CODE(0x8),
+ &regs->cantxfg[FLEXCAN_FIRST_VALID_MB].can_ctrl);
+
return NETDEV_TX_OK;
}

[Marc Kleine-Budde]

Looks good. Can you please prepare a more detailed commit message.

Ok, I will briefly describe the workaround. I first thought that mentioning the
errata number would be sufficient, since it is freely available documentation
from Freescale.
ERR005829: FlexCAN: FlexCAN does not transmit a message that is enabled to be
transmitted in a specific moment during the arbitration process.
Workaround: The workaround consists of two extra steps after setting up a
Step 8: Reserve the first valid mailbox as an inactive mailbox (CODE=0b1000).
If RX FIFO is disabled, this mailbox must be message buffer 0. Otherwise, the
first valid mailbox can be found using the "RX FIFO filters" table in the
FlexCAN chapter of the chip reference manual.
Step 9: Write twice INACTIVE code (0b1000) into the first valid mailbox.

Thanks, added the description to the patch.

Post by David Jander
drivers/net/can/flexcan.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 0bcbb96..d9d0ecb 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -125,7 +125,9 @@
FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
/* FLEXCAN interrupt flag register (IFLAG) bits */
-#define FLEXCAN_TX_BUF_ID 8
+/* Errata ERR005829 step7: Reserve first valid MB */
+#define FLEXCAN_FIRST_VALID_MB 8
+#define FLEXCAN_TX_BUF_ID 9
#define FLEXCAN_IFLAG_BUF(x) BIT(x)
#define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW BIT(7)
#define FLEXCAN_IFLAG_RX_FIFO_WARN BIT(6)
@@ -428,6 +430,12 @@ 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);

However, I don't want to write unconditionally two times here, so I've
added a runtime decision with a quirk for this. I'll send an updated patch.

Please note that if the silicon bug didn't exist, none of the two writes would
be necessary.
Once you create a quirk for this, how are we supposed to know which versions
need this quirk, and which don't? Can we trust the existence of erratas for
the different i.MX Soc's, and should we just go and check them all?

I had a patch with the quirk, but I removed it, as you suggested, just
in case.

Marc

[David Jander]

On Tue, 16 Sep 2014 13:48:27 +0200

Post by Marc Kleine-Budde

Looks good. Can you please prepare a more detailed commit message.

Ok, I will briefly describe the workaround. I first thought that
mentioning the errata number would be sufficient, since it is freely
available documentation from Freescale.
ERR005829: FlexCAN: FlexCAN does not transmit a message that is enabled to
be transmitted in a specific moment during the arbitration process.
Workaround: The workaround consists of two extra steps after setting up a
Step 8: Reserve the first valid mailbox as an inactive mailbox
(CODE=0b1000). If RX FIFO is disabled, this mailbox must be message buffer
0. Otherwise, the first valid mailbox can be found using the "RX FIFO
filters" table in the FlexCAN chapter of the chip reference manual.
Step 9: Write twice INACTIVE code (0b1000) into the first valid mailbox.

Thanks, added the description to the patch.

Post by David Jander
drivers/net/can/flexcan.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 0bcbb96..d9d0ecb 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -125,7 +125,9 @@
FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
/* FLEXCAN interrupt flag register (IFLAG) bits */
-#define FLEXCAN_TX_BUF_ID 8
+/* Errata ERR005829 step7: Reserve first valid MB */
+#define FLEXCAN_FIRST_VALID_MB 8
+#define FLEXCAN_TX_BUF_ID 9
#define FLEXCAN_IFLAG_BUF(x) BIT(x)
#define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW BIT(7)
#define FLEXCAN_IFLAG_RX_FIFO_WARN BIT(6)
@@ -428,6 +430,12 @@ 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);

However, I don't want to write unconditionally two times here, so I've
added a runtime decision with a quirk for this. I'll send an updated patch.

Please note that if the silicon bug didn't exist, none of the two writes
would be necessary.
Once you create a quirk for this, how are we supposed to know which
versions need this quirk, and which don't? Can we trust the existence of
erratas for the different i.MX Soc's, and should we just go and check them
all?

I had a patch with the quirk, but I removed it, as you suggested, just
in case.

I did not directly suggest to NOT use a quirk, I only had some concerns. In
the meantime I have checked a few of the SoCs for erratas, and found this
errata only for i.MX6 and Vybrid VF6xx, which makes me suspect this problem is
unique to IP version 10 (if the IP version table at the beginning of the
source-code is to be trusted).

OTOH, if you decided to leave the unconditional writes in there, I am fine
with that. I don't think they do much harm.

Is it time to revisit my other patch(es) yet? If so, from where should I pull
the base?

Best regards,

[Marc Kleine-Budde]

Post by David Jander

Post by Marc Kleine-Budde

Please note that if the silicon bug didn't exist, none of the two writes
would be necessary.
Once you create a quirk for this, how are we supposed to know which
versions need this quirk, and which don't? Can we trust the existence of
erratas for the different i.MX Soc's, and should we just go and check them
all?

I had a patch with the quirk, but I removed it, as you suggested, just
in case.

I did not directly suggest to NOT use a quirk, I only had some concerns. In

Not directly, but between the lines :D

Post by David Jander
the meantime I have checked a few of the SoCs for erratas, and found this
errata only for i.MX6 and Vybrid VF6xx, which makes me suspect this problem is
unique to IP version 10 (if the IP version table at the beginning of the
source-code is to be trusted).

We don't even know the IP version of the flexcan core on the vybrid, it
has different features than the imx6 one.

Post by David Jander
OTOH, if you decided to leave the unconditional writes in there, I am fine
with that. I don't think they do much harm.
Is it time to revisit my other patch(es) yet? If so, from where should I pull
the base?

Sorry, no time yet, but I've found another (luckily only a minor)
problem with the mailboxes. After sending a RTR frame the TX mailbox
gets automatically converted into a RX one.

Post by David Jander
Transmit remote request frame unconditionally once. After
transmission, the MB automatically becomes an Rx Empty MB
with the same ID.

I'll prepare a patch.

Marc

[Marc Kleine-Budde]

Post by David Jander
Is it time to revisit my other patch(es) yet? If so, from where should I pull
the base?

Please review the flexcan series (v4) I just sent around. That will go
into net/master then.

For new the new flexcan features please use this as your base:

git://gitorious.org/linux-can/linux-can-next.git flexcan-next

It's net-next merged to the latest can patches.

Marc

[David Jander]

On Tue, 16 Sep 2014 16:29:25 +0200

Post by Marc Kleine-Budde

Post by David Jander
Is it time to revisit my other patch(es) yet? If so, from where should I
pull the base?

Please review the flexcan series (v4) I just sent around. That will go
into net/master then.

Looks good.

Post by Marc Kleine-Budde
git://gitorious.org/linux-can/linux-can-next.git flexcan-next

Thanks. See my new patch v4 I just sent. I rebased on top of that and tested
it briefly. Seems to work fine.

Best regards,