Discussion:
[PATCH 4/8] can: flexcan: correctly initialize mailboxes
Marc Kleine-Budde
2014-09-18 10:40:58 UTC
Permalink
From: David Jander <***@protonic.nl>

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.

This patch initializes all mailboxes after the FIFO as RX_INACTIVE.

Signed-off-by: David Jander <***@protonic.nl>
Cc: linux-stable <***@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/flexcan.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 76dcbca..fc076952 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -812,6 +812,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);
@@ -878,6 +879,12 @@ 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);

+ /* clear and invalidate all mailboxes first */
+ for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) {
+ flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE,
+ &regs->cantxfg[i].can_ctrl);
+ }
+
/* mark TX mailbox as INACTIVE */
flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
&regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
--
2.1.0
Marc Kleine-Budde
2014-09-18 10:40:55 UTC
Permalink
From: Roger Quadros <***@ti.com>

Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). They
seem to have been swapped in the usage instances.

Reported-by: Jay Schroeder <***@garmin.com>
Signed-off-by: Roger Quadros <***@ti.com>
Cc: linux-stable <***@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/c_can/c_can_platform.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 109cb44..fb279d6 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -97,14 +97,14 @@ static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
writel(ctrl, priv->raminit_ctrlreg);
ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance);
- c_can_hw_raminit_wait_ti(priv, ctrl, mask);
+ c_can_hw_raminit_wait_ti(priv, mask, ctrl);

if (enable) {
/* Set start bit and wait for the done bit. */
ctrl |= CAN_RAMINIT_START_MASK(priv->instance);
writel(ctrl, priv->raminit_ctrlreg);
ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
- c_can_hw_raminit_wait_ti(priv, ctrl, mask);
+ c_can_hw_raminit_wait_ti(priv, mask, ctrl);
}
spin_unlock(&raminit_lock);
}
--
2.1.0
Marc Kleine-Budde
2014-09-18 10:40:57 UTC
Permalink
This patch fixes the initialization of the TX mailbox. It is now correctly
initialized as TX_INACTIVE not RX_EMPTY.

Cc: linux-stable <***@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/flexcan.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 630c7bf..76dcbca 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -136,6 +136,17 @@

/* FLEXCAN message buffers */
#define FLEXCAN_MB_CNT_CODE(x) (((x) & 0xf) << 24)
+#define FLEXCAN_MB_CODE_RX_INACTIVE (0x0 << 24)
+#define FLEXCAN_MB_CODE_RX_EMPTY (0x4 << 24)
+#define FLEXCAN_MB_CODE_RX_FULL (0x2 << 24)
+#define FLEXCAN_MB_CODE_RX_OVERRRUN (0x6 << 24)
+#define FLEXCAN_MB_CODE_RX_RANSWER (0xa << 24)
+
+#define FLEXCAN_MB_CODE_TX_INACTIVE (0x8 << 24)
+#define FLEXCAN_MB_CODE_TX_ABORT (0x9 << 24)
+#define FLEXCAN_MB_CODE_TX_DATA (0xc << 24)
+#define FLEXCAN_MB_CODE_TX_TANSWER (0xe << 24)
+
#define FLEXCAN_MB_CNT_SRR BIT(22)
#define FLEXCAN_MB_CNT_IDE BIT(21)
#define FLEXCAN_MB_CNT_RTR BIT(20)
@@ -867,8 +878,8 @@ 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),
+ /* mark TX mailbox as INACTIVE */
+ flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
&regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);

/* acceptance mask/acceptance code (accept everything) */
--
2.1.0
Marc Kleine-Budde
2014-09-18 10:41:01 UTC
Permalink
This patch increases the mask in the FLEXCAN_MCR_MAXMB() to 7 bits as in the
newer flexcan cores the MAXMB field is 7 bits wide.

Reported-by: David Jander <***@protonic.nl>
Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/flexcan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index c17ae9e..6586309 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) & 0x1f)
+#define FLEXCAN_MCR_MAXMB(x) ((x) & 0x7f)
#define FLEXCAN_MCR_IDAM_A (0 << 8)
#define FLEXCAN_MCR_IDAM_B (1 << 8)
#define FLEXCAN_MCR_IDAM_C (2 << 8)
--
2.1.0
Marc Kleine-Budde
2014-09-18 10:40:59 UTC
Permalink
From: David Jander <***@protonic.nl>

This patch implements the workaround mentioned in ERR005829:

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
message for transmission:

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.

Signed-off-by: David Jander <***@protonic.nl>
Cc: linux-stable <***@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/flexcan.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index fc076952..54061c4 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_TX_BUF_RESERVED 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)
@@ -439,6 +441,14 @@ 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_CODE_TX_INACTIVE,
+ &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+ flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+ &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+
return NETDEV_TX_OK;
}

@@ -885,6 +895,10 @@ static int flexcan_chip_start(struct net_device *dev)
&regs->cantxfg[i].can_ctrl);
}

+ /* Errata ERR005829: mark first TX mailbox as INACTIVE */
+ flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+ &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+
/* mark TX mailbox as INACTIVE */
flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
&regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-can" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Marc Kleine-Budde
2014-09-18 10:41:02 UTC
Permalink
From: David Dueck <***@googlemail.com>

In order to make the driver work with the common clock framework, this patch
converts the clk_enable()/clk_disable() to
clk_prepare_enable()/clk_disable_unprepare(). While there, add the missing
error handling.

Signed-off-by: David Dueck <***@googlemail.com>
Signed-off-by: Anthony Harivel <***@emtrion.de>
Acked-by: Boris Brezillon <***@free-electrons.com>
Cc: linux-stable <***@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/at91_can.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index f07fa89..05e1aa0 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -1123,7 +1123,9 @@ static int at91_open(struct net_device *dev)
struct at91_priv *priv = netdev_priv(dev);
int err;

- clk_enable(priv->clk);
+ err = clk_prepare_enable(priv->clk);
+ if (err)
+ return err;

/* check or determine and set bittime */
err = open_candev(dev);
@@ -1149,7 +1151,7 @@ static int at91_open(struct net_device *dev)
out_close:
close_candev(dev);
out:
- clk_disable(priv->clk);
+ clk_disable_unprepare(priv->clk);

return err;
}
@@ -1166,7 +1168,7 @@ static int at91_close(struct net_device *dev)
at91_chip_stop(dev, CAN_STATE_STOPPED);

free_irq(dev->irq, dev);
- clk_disable(priv->clk);
+ clk_disable_unprepare(priv->clk);

close_candev(dev);
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-can" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Marc Kleine-Budde
2014-09-18 10:41:00 UTC
Permalink
After sending a RTR frame the TX mailbox becomes a RX_EMPTY mailbox. To avoid
side effects when the RX-FIFO is full, this patch puts the TX mailbox into
TX_INACTIVE mode in the transmission complete interrupt handler. This, of
course, leaves a race window between the actual completion of the transmission
and the handling of tx-complete interrupt. However this is the best we can do
without busy polling the tx complete interrupt.

Cc: linux-stable <***@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/flexcan.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 54061c4..c17ae9e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -765,6 +765,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
stats->tx_bytes += can_get_echo_skb(dev, 0);
stats->tx_packets++;
can_led_event(dev, CAN_LED_EVENT_TX);
+ /* after sending a RTR frame mailbox is in RX mode */
+ flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+ &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
netif_wake_queue(dev);
}
--
2.1.0
Marc Kleine-Budde
2014-09-18 10:40:56 UTC
Permalink
From: Oliver Hartkopp <***@hartkopp.net>

Add PCI ID definition for the single channel PCAN ExpressCard 34 adapter. Due
to the subsystem id evaluation the correct number of channels (here 1) is
created at initialization time. Tested including the LED functionality.

Signed-off-by: Oliver Hartkopp <***@hartkopp.net>
Acked-by: Stephane Grosjean <***@peak-system.com>
Signed-off-by: Marc Kleine-Budde <***@pengutronix.de>
---
drivers/net/can/sja1000/peak_pci.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/sja1000/peak_pci.c b/drivers/net/can/sja1000/peak_pci.c
index 7a85590..e5fac36 100644
--- a/drivers/net/can/sja1000/peak_pci.c
+++ b/drivers/net/can/sja1000/peak_pci.c
@@ -70,6 +70,8 @@ struct peak_pci_chan {
#define PEAK_PC_104P_DEVICE_ID 0x0006 /* PCAN-PC/104+ cards */
#define PEAK_PCI_104E_DEVICE_ID 0x0007 /* PCAN-PCI/104 Express cards */
#define PEAK_MPCIE_DEVICE_ID 0x0008 /* The miniPCIe slot cards */
+#define PEAK_PCIE_OEM_ID 0x0009 /* PCAN-PCI Express OEM */
+#define PEAK_PCIEC34_DEVICE_ID 0x000A /* PCAN-PCI Express 34 (one channel) */

#define PEAK_PCI_CHAN_MAX 4

@@ -87,6 +89,7 @@ static const struct pci_device_id peak_pci_tbl[] = {
{PEAK_PCI_VENDOR_ID, PEAK_CPCI_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
#ifdef CONFIG_CAN_PEAK_PCIEC
{PEAK_PCI_VENDOR_ID, PEAK_PCIEC_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
+ {PEAK_PCI_VENDOR_ID, PEAK_PCIEC34_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
#endif
{0,}
};
@@ -653,7 +656,8 @@ static int peak_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
* This must be done *before* register_sja1000dev() but
* *after* devices linkage
*/
- if (pdev->device == PEAK_PCIEC_DEVICE_ID) {
+ if (pdev->device == PEAK_PCIEC_DEVICE_ID ||
+ pdev->device == PEAK_PCIEC34_DEVICE_ID) {
err = peak_pciec_probe(pdev, dev);
if (err) {
dev_err(&pdev->dev,
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-can" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller
2014-09-19 21:34:48 UTC
Permalink
From: Marc Kleine-Budde <***@pengutronix.de>
Date: Thu, 18 Sep 2014 12:40:54 +0200
this is a pull request of 8 patches for current net.
A patch by Roger Quadros for the c_can driver fixes the swapped parameters of
the c_can_hw_raminit_ti() function. Oliver Hartkopp adds the missing PCI ids to
the peak_pci driver to support the single channel PCAN ExpressCard 34 adapter.
David Dueck converts the at91_can driver to use proper clock handling
functions. Then there are 5 patches by David Jander and me which fix several
mailbox related problems in the flexcan driver.
Pulled, thanks Marc.
--
To unsubscribe from this list: send the line "unsubscribe linux-can" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...