[PATCH 01/20] can: raw: check for CAN FD capable netdev in raw_sendmsg()

[PATCH 01/20] can: raw: check for CAN FD capable netdev in raw_sendmsg()

[Marc Kleine-Budde]

From: Oliver Hartkopp <socketcan@hartkopp.net>

When the socket is CAN FD enabled it can handle CAN FD frame
transmissions.  Add an additional check in raw_sendmsg() as a CAN2.0 CAN
driver (non CAN FD) should never see a CAN FD frame. Due to the commonly
used can_dropped_invalid_skb() function the CAN 2.0 driver would drop
that CAN FD frame anyway - but with this patch the user gets a proper
-EINVAL return code.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/raw.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/net/can/raw.c b/net/can/raw.c
index 1051eee82581..3aab7664933f 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -745,18 +745,19 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	} else
 		ifindex = ro->ifindex;
 
-	if (ro->fd_frames) {
+	dev = dev_get_by_index(sock_net(sk), ifindex);
+	if (!dev)
+		return -ENXIO;
+
+	err = -EINVAL;
+	if (ro->fd_frames && dev->mtu == CANFD_MTU) {
 		if (unlikely(size != CANFD_MTU && size != CAN_MTU))
-			return -EINVAL;
+			goto put_dev;
 	} else {
 		if (unlikely(size != CAN_MTU))
-			return -EINVAL;
+			goto put_dev;
 	}
 
-	dev = dev_get_by_index(sock_net(sk), ifindex);
-	if (!dev)
-		return -ENXIO;
-
 	skb = sock_alloc_send_skb(sk, size + sizeof(struct can_skb_priv),
 				  msg->msg_flags & MSG_DONTWAIT, &err);
 	if (!skb)
-- 
2.19.1

[PATCH 02/20] can: kvaser_usb: Fix potential uninitialized variable use

[Marc Kleine-Budde]

From: Jimmy Assarsson <jimmyassarsson@gmail.com>

If alloc_can_err_skb() fails, cf is never initialized.
Move assignment of cf inside check.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jimmy Assarsson <jimmyassarsson@gmail.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
index c084bae5ec0a..5fc0be564274 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -1019,6 +1019,11 @@ kvaser_usb_hydra_error_frame(struct kvaser_usb_net_priv *priv,
 					new_state : CAN_STATE_ERROR_ACTIVE;
 
 			can_change_state(netdev, cf, tx_state, rx_state);
+
+			if (priv->can.restart_ms &&
+			    old_state >= CAN_STATE_BUS_OFF &&
+			    new_state < CAN_STATE_BUS_OFF)
+				cf->can_id |= CAN_ERR_RESTARTED;
 		}
 
 		if (new_state == CAN_STATE_BUS_OFF) {
@@ -1028,11 +1033,6 @@ kvaser_usb_hydra_error_frame(struct kvaser_usb_net_priv *priv,
 
 			can_bus_off(netdev);
 		}
-
-		if (priv->can.restart_ms &&
-		    old_state >= CAN_STATE_BUS_OFF &&
-		    new_state < CAN_STATE_BUS_OFF)
-			cf->can_id |= CAN_ERR_RESTARTED;
 	}
 
 	if (!skb) {
-- 
2.19.1

[PATCH 03/20] can: kvaser_usb: Fix accessing freed memory in kvaser_usb_start_xmit()

[Marc Kleine-Budde]

From: Jimmy Assarsson <jimmyassarsson@gmail.com>

The call to can_put_echo_skb() may result in the skb being freed. The skb
is later used in the call to dev->ops->dev_frame_to_cmd().

This is avoided by moving the call to can_put_echo_skb() after
dev->ops->dev_frame_to_cmd().

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jimmy Assarsson <jimmyassarsson@gmail.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
index b939a4c10b84..c89c7d4900d7 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -528,7 +528,6 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 			context = &priv->tx_contexts[i];
 
 			context->echo_index = i;
-			can_put_echo_skb(skb, netdev, context->echo_index);
 			++priv->active_tx_contexts;
 			if (priv->active_tx_contexts >= (int)dev->max_tx_urbs)
 				netif_stop_queue(netdev);
@@ -553,7 +552,6 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 		dev_kfree_skb(skb);
 		spin_lock_irqsave(&priv->tx_contexts_lock, flags);
 
-		can_free_echo_skb(netdev, context->echo_index);
 		context->echo_index = dev->max_tx_urbs;
 		--priv->active_tx_contexts;
 		netif_wake_queue(netdev);
@@ -564,6 +562,8 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 
 	context->priv = priv;
 
+	can_put_echo_skb(skb, netdev, context->echo_index);
+
 	usb_fill_bulk_urb(urb, dev->udev,
 			  usb_sndbulkpipe(dev->udev,
 					  dev->bulk_out->bEndpointAddress),
-- 
2.19.1

[PATCH 09/20] can: hi311x: Use level-triggered interrupt

[Marc Kleine-Budde]

From: Lukas Wunner <lukas@wunner.de>

If the hi3110 shares the SPI bus with another traffic-intensive device
and packets are received in high volume (by a separate machine sending
with "cangen -g 0 -i -x"), reception stops after a few minutes and the
counter in /proc/interrupts stops incrementing.  Bus state is "active".
Bringing the interface down and back up reconvenes the reception.  The
issue is not observed when the hi3110 is the sole device on the SPI bus.

Using a level-triggered interrupt makes the issue go away and lets the
hi3110 successfully receive 2 GByte over the course of 5 days while a
ks8851 Ethernet chip on the same SPI bus handles 6 GByte of traffic.

Unfortunately the hi3110 datasheet is mum on the trigger type.  The pin
description on page 3 only specifies the polarity (active high):
http://www.holtic.com/documents/371-hi-3110_v-rev-kpdf.do

Cc: Mathias Duckeck <m.duckeck@kunbus.de>
Cc: Akshay Bhat <akshay.bhat@timesys.com>
Cc: Casey Fitzpatrick <casey.fitzpatrick@timesys.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 Documentation/devicetree/bindings/net/can/holt_hi311x.txt | 2 +-
 drivers/net/can/spi/hi311x.c                              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/holt_hi311x.txt b/Documentation/devicetree/bindings/net/can/holt_hi311x.txt
index 903a78da65be..3a9926f99937 100644
--- a/Documentation/devicetree/bindings/net/can/holt_hi311x.txt
+++ b/Documentation/devicetree/bindings/net/can/holt_hi311x.txt
@@ -17,7 +17,7 @@ Example:
 		reg = <1>;
 		clocks = <&clk32m>;
 		interrupt-parent = <&gpio4>;
-		interrupts = <13 IRQ_TYPE_EDGE_RISING>;
+		interrupts = <13 IRQ_TYPE_LEVEL_HIGH>;
 		vdd-supply = <&reg5v0>;
 		xceiver-supply = <&reg5v0>;
 	};
diff --git a/drivers/net/can/spi/hi311x.c b/drivers/net/can/spi/hi311x.c
index 53e320c92a8b..ddaf46239e39 100644
--- a/drivers/net/can/spi/hi311x.c
+++ b/drivers/net/can/spi/hi311x.c
@@ -760,7 +760,7 @@ static int hi3110_open(struct net_device *net)
 {
 	struct hi3110_priv *priv = netdev_priv(net);
 	struct spi_device *spi = priv->spi;
-	unsigned long flags = IRQF_ONESHOT | IRQF_TRIGGER_RISING;
+	unsigned long flags = IRQF_ONESHOT | IRQF_TRIGGER_HIGH;
 	int ret;
 
 	ret = open_candev(net);
-- 
2.19.1

[PATCH 10/20] can: flexcan: Unlock the MB unconditionally

[Marc Kleine-Budde]

From: Pankaj Bansal <pankaj.bansal@nxp.com>

Unlock the MB irrespective of reception method being FIFO or timestamp
based. It is optional but recommended to unlock Mailbox as soon as
possible and make it available for reception.

Reported-by: Alexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 8e972ef08637..0431f8d05518 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -720,9 +720,14 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload,
 			priv->write(BIT(n - 32), &regs->iflag2);
 	} else {
 		priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
-		priv->read(&regs->timer);
 	}
 
+	/* Read the Free Running Timer. It is optional but recommended
+	 * to unlock Mailbox as soon as possible and make it available
+	 * for reception.
+	 */
+	priv->read(&regs->timer);
+
 	return 1;
 }
 
-- 
2.19.1

[PATCH 11/20] can: flexcan: Always use last mailbox for TX

[Marc Kleine-Budde]

From: Alexander Stein <alexander.stein@systec-electronic.com>

Essentially this patch moves the TX mailbox to position 63, regardless
of timestamp based offloading or RX FIFO. So mainly the iflag register
usage regarding TX has changed. The rest is consolidating RX FIFO and
timestamp offloading as they now use both the same TX mailbox.

The reason is a very annoying behavior regarding sending RTR frames when
_not_ using RX FIFO:

If a TX mailbox sent a RTR frame it becomes a RX mailbox. For that
reason flexcan_irq disables the TX mailbox again. But if during the time
the RTR was sent and the TX mailbox is disabled a new CAN frames is
received, it is lost without notice. The reason is that so-called
"Move-in" process starts from the lowest mailbox which happen to be a TX
mailbox set to EMPTY.

Steps to reproduce (I used an imx7d):
1. generate regular bursts of messages
2. send a RTR from flexcan with higher priority than burst messages every
   1ms, e.g. cangen -I 0x100 -L 0 -g 1 -R can0
3. notice a lost message without notification after some seconds

When running an iperf in parallel this problem is occurring even more
frequently. Using filters is not possible as at least one single CAN-ID
is allowed. Handling the TX MB during RX is also not possible as there
is no race-free disable of RX MB.

There is still a slight window when the described problem can occur. But
for that all RX MB must be in use which is essentially next to an
overrun. Still there will be no indication if it ever occurs.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c | 67 +++++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 0431f8d05518..677c41701cf3 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -135,13 +135,12 @@
 
 /* FLEXCAN interrupt flag register (IFLAG) bits */
 /* Errata ERR005829 step7: Reserve first valid MB */
-#define FLEXCAN_TX_MB_RESERVED_OFF_FIFO	8
-#define FLEXCAN_TX_MB_OFF_FIFO		9
+#define FLEXCAN_TX_MB_RESERVED_OFF_FIFO		8
 #define FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP	0
-#define FLEXCAN_TX_MB_OFF_TIMESTAMP		1
-#define FLEXCAN_RX_MB_OFF_TIMESTAMP_FIRST	(FLEXCAN_TX_MB_OFF_TIMESTAMP + 1)
-#define FLEXCAN_RX_MB_OFF_TIMESTAMP_LAST	63
-#define FLEXCAN_IFLAG_MB(x)		BIT(x)
+#define FLEXCAN_TX_MB				63
+#define FLEXCAN_RX_MB_OFF_TIMESTAMP_FIRST	(FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP + 1)
+#define FLEXCAN_RX_MB_OFF_TIMESTAMP_LAST	(FLEXCAN_TX_MB - 1)
+#define FLEXCAN_IFLAG_MB(x)		BIT(x & 0x1f)
 #define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW	BIT(7)
 #define FLEXCAN_IFLAG_RX_FIFO_WARN	BIT(6)
 #define FLEXCAN_IFLAG_RX_FIFO_AVAILABLE	BIT(5)
@@ -737,9 +736,9 @@ static inline u64 flexcan_read_reg_iflag_rx(struct flexcan_priv *priv)
 	struct flexcan_regs __iomem *regs = priv->regs;
 	u32 iflag1, iflag2;
 
-	iflag2 = priv->read(&regs->iflag2) & priv->reg_imask2_default;
-	iflag1 = priv->read(&regs->iflag1) & priv->reg_imask1_default &
+	iflag2 = priv->read(&regs->iflag2) & priv->reg_imask2_default &
 		~FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
+	iflag1 = priv->read(&regs->iflag1) & priv->reg_imask1_default;
 
 	return (u64)iflag2 << 32 | iflag1;
 }
@@ -751,11 +750,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->regs;
 	irqreturn_t handled = IRQ_NONE;
-	u32 reg_iflag1, reg_esr;
+	u32 reg_iflag2, reg_esr;
 	enum can_state last_state = priv->can.state;
 
-	reg_iflag1 = priv->read(&regs->iflag1);
-
 	/* reception interrupt */
 	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
 		u64 reg_iflag;
@@ -769,6 +766,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 				break;
 		}
 	} else {
+		u32 reg_iflag1;
+
+		reg_iflag1 = priv->read(&regs->iflag1);
 		if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) {
 			handled = IRQ_HANDLED;
 			can_rx_offload_irq_offload_fifo(&priv->offload);
@@ -784,8 +784,10 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		}
 	}
 
+	reg_iflag2 = priv->read(&regs->iflag2);
+
 	/* transmission complete interrupt */
-	if (reg_iflag1 & FLEXCAN_IFLAG_MB(priv->tx_mb_idx)) {
+	if (reg_iflag2 & FLEXCAN_IFLAG_MB(priv->tx_mb_idx)) {
 		handled = IRQ_HANDLED;
 		stats->tx_bytes += can_get_echo_skb(dev, 0);
 		stats->tx_packets++;
@@ -794,7 +796,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		/* after sending a RTR frame MB is in RX mode */
 		priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
 			    &priv->tx_mb->can_ctrl);
-		priv->write(FLEXCAN_IFLAG_MB(priv->tx_mb_idx), &regs->iflag1);
+		priv->write(FLEXCAN_IFLAG_MB(priv->tx_mb_idx), &regs->iflag2);
 		netif_wake_queue(dev);
 	}
 
@@ -936,15 +938,13 @@ static int flexcan_chip_start(struct net_device *dev)
 	reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
 	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV |
 		FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_SRX_DIS | FLEXCAN_MCR_IRMQ |
-		FLEXCAN_MCR_IDAM_C;
+		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
 
-	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
 		reg_mcr &= ~FLEXCAN_MCR_FEN;
-		reg_mcr |= FLEXCAN_MCR_MAXMB(priv->offload.mb_last);
-	} else {
-		reg_mcr |= FLEXCAN_MCR_FEN |
-			FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
-	}
+	else
+		reg_mcr |= FLEXCAN_MCR_FEN;
+
 	netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
 	priv->write(reg_mcr, &regs->mcr);
 
@@ -987,16 +987,17 @@ static int flexcan_chip_start(struct net_device *dev)
 		priv->write(reg_ctrl2, &regs->ctrl2);
 	}
 
-	/* clear and invalidate all mailboxes first */
-	for (i = priv->tx_mb_idx; i < ARRAY_SIZE(regs->mb); i++) {
-		priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
-			    &regs->mb[i].can_ctrl);
-	}
-
 	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
-		for (i = priv->offload.mb_first; i <= priv->offload.mb_last; i++)
+		for (i = priv->offload.mb_first; i <= priv->offload.mb_last; i++) {
 			priv->write(FLEXCAN_MB_CODE_RX_EMPTY,
 				    &regs->mb[i].can_ctrl);
+		}
+	} else {
+		/* clear and invalidate unused mailboxes first */
+		for (i = FLEXCAN_TX_MB_RESERVED_OFF_FIFO; i <= ARRAY_SIZE(regs->mb); i++) {
+			priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
+				    &regs->mb[i].can_ctrl);
+		}
 	}
 
 	/* Errata ERR005829: mark first TX mailbox as INACTIVE */
@@ -1360,17 +1361,15 @@ static int flexcan_probe(struct platform_device *pdev)
 	priv->devtype_data = devtype_data;
 	priv->reg_xceiver = reg_xceiver;
 
-	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
-		priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_TIMESTAMP;
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
 		priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP];
-	} else {
-		priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_FIFO;
+	else
 		priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_FIFO];
-	}
+	priv->tx_mb_idx = FLEXCAN_TX_MB;
 	priv->tx_mb = &regs->mb[priv->tx_mb_idx];
 
-	priv->reg_imask1_default = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
-	priv->reg_imask2_default = 0;
+	priv->reg_imask1_default = 0;
+	priv->reg_imask2_default = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
 
 	priv->offload.mailbox_read = flexcan_mailbox_read;
 
-- 
2.19.1

Re: [PATCH 11/20] can: flexcan: Always use last mailbox for TX

[Uwe Kleine-König]

hello,

On Mon, Nov 12, 2018 at 12:57:19PM +0100, Marc Kleine-Budde wrote:
> From: Alexander Stein <alexander.stein@systec-electronic.com>
> 
> Essentially this patch moves the TX mailbox to position 63, regardless
> of timestamp based offloading or RX FIFO. So mainly the iflag register
> usage regarding TX has changed. The rest is consolidating RX FIFO and
> timestamp offloading as they now use both the same TX mailbox.
> 
> The reason is a very annoying behavior regarding sending RTR frames when
> _not_ using RX FIFO:
> 
> If a TX mailbox sent a RTR frame it becomes a RX mailbox. For that
> reason flexcan_irq disables the TX mailbox again. But if during the time
> the RTR was sent and the TX mailbox is disabled a new CAN frames is
> received, it is lost without notice. The reason is that so-called
> "Move-in" process starts from the lowest mailbox which happen to be a TX
> mailbox set to EMPTY.
> 
> Steps to reproduce (I used an imx7d):
> 1. generate regular bursts of messages
> 2. send a RTR from flexcan with higher priority than burst messages every
>    1ms, e.g. cangen -I 0x100 -L 0 -g 1 -R can0
> 3. notice a lost message without notification after some seconds
> 
> When running an iperf in parallel this problem is occurring even more
> frequently. Using filters is not possible as at least one single CAN-ID
> is allowed. Handling the TX MB during RX is also not possible as there
> is no race-free disable of RX MB.
> 
> There is still a slight window when the described problem can occur. But
> for that all RX MB must be in use which is essentially next to an
> overrun. Still there will be no indication if it ever occurs.
> 
> Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> Cc: linux-stable <stable@vger.kernel.org>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

The backport of this patch (24e5589791d09214e31335ae2b581c668d4803b7) in
4.19.x makes the driver trigger an abort on an i.MX25 based machine.

If I revert

	d5a9ba4324d5 ("can: flexcan: remove not needed struct flexcan_priv::tx_mb and struct flexcan_priv::tx_mb_idx")
	24e5589791d0 ("can: flexcan: Always use last mailbox for TX")

on top of v4.19.13 the oops is gone. (I have to add #define
FLEXCAN_TX_MB 63 to the driver though as the next older flexcan patch
(04f985776f04 ("can: flexcan: handle tx-complete CAN frames via
rx-offload infrastructure")) makes use of this symbol but it is only
defined as part of 24e5589791d0. In mainline the order of the commits is
different such that this problem doesn't occur.)

The output with no changes to flexcan on top of 4.19.13 looks as follows:

[   10.446142] Unhandled fault: external abort on non-linefetch (0x808) at 0xc89f4480
[   10.453774] pgd = (ptrval)
[   10.456510] [c89f4480] *pgd=87885811, *pte=43f88653, *ppte=43f88552
[   10.462856] Internal error: : 808 [#1] PREEMPT ARM
[   10.467678] Modules linked in:
[   10.470786] CPU: 0 PID: 685 Comm: ip Not tainted 4.19.13-20180926-1-00011-ga0dd04ff511f #4
[   10.479071] Hardware name: Freescale i.MX25 (Device Tree Support)
[   10.485232] PC is at flexcan_write_le+0x0/0x8
[   10.489632] LR is at flexcan_chip_start+0x450/0x474
[   10.494538] pc : [<c0436f18>]    lr : [<c0437680>]    psr: 20000013
[   10.500826] sp : c6ed1900  ip : 00000000  fp : c7a73000
[   10.506071] r10: c0789a30  r9 : c89f4004  r8 : c89f4490
[   10.511317] r7 : 00000000  r6 : c7a733e0  r5 : c89f4000  r4 : c89f4490
[   10.517866] r3 : c0436f10  r2 : c0436f18  r1 : c89f4480  r0 : 00000000
[   10.524416] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   10.531574] Control: 0005317f  Table: 86f28000  DAC: 00000051
[   10.537345] Process ip (pid: 685, stack limit = 0x(ptrval))
[   10.542942] Stack: (0xc6ed1900 to 0xc6ed2000)
[   10.547342] 1900: 00000080 c7a73000 00000000 c70d0780 00000001 00040080 c06a4760 c6ca8010
[   10.555563] 1920: 00000000 c0438534 c7a73000 c7a73000 c7a73000 c084d028 c06a4760 c0504924
[   10.563782] 1940: c054cacc 0137be89 c6c88e50 c7a73000 00000000 0137be89 c7a73000 c084d028
[   10.572002] 1960: 00040081 c0504d0c 000000be 00000000 c06a485c c6ed19fc c7a73000 0137be89
[   10.580220] 1980: c084d028 c7a73000 00000000 c7a73138 00040080 c6ed1ca0 c06a4760 c0504d94
[   10.588442] 19a0: c7a73000 c6ed1bd0 c084d028 c7249260 c6ed1ca0 c051b1cc 000000a1 0000000b
[   10.596663] 19c0: 00000002 c6ed1d6c 00053de0 c7a73000 c6ed1b0c c6ed1b0c c084d028 c6ed19fc
[   10.604883] 19e0: c06a485c c7a733fc c7a73000 c04364a4 00000000 c6ca8040 00000001 0000c350
[   10.613100] 1a00: 00000359 00000594 00000005 0137be89 00000002 c7a73000 00000000 00000000
[   10.621322] 1a20: c6ca8000 c6ed1b0c c084d7f8 c6ed1d6c 00000000 c051c0cc c6ed1bd0 c6ed1ca0
[   10.629539] 1a40: 00000003 c004951c c08b1940 00000000 c7249260 c6ca8010 00000000 00000000
[   10.637758] 1a60: 00000000 c6ca802c c6ca8034 00000000 00000000 00000000 00000000 c0856980
[   10.645977] 1a80: 00000005 c0049e98 c78fa4f0 c0049e98 c06644c0 c0856980 c78fa4c0 00000000
[   10.654196] 1aa0: 00000000 c08c8968 c0854aa8 c0042e14 c78fa4c0 a0000093 c6ed1acc c0042ed4
[   10.662415] 1ac0: c084d028 a0000093 c6ed1afc c00341f8 fe7ed638 c004951c c79d4110 0137be89
[   10.670635] 1ae0: c7a6e75c c781bd00 c08c0554 00000000 00000000 c08c8968 c7a36860 c00341f8
[   10.678854] 1b00: c084d028 c08569b0 c79d4110 00000000 c6ca8040 00000000 00000000 00000000
[   10.687069] 1b20: 00000000 c6ca8038 00000000 00000000 00000000 00000000 00000000 00000000
[   10.695286] 1b40: 00000000 00000000 00000000 c6c98200 c6ed1b84 c025a9bc 00000000 00000000
[   10.703506] 1b60: c7a6e738 c025a9bc 00000000 c0335c90 c6e94900 c00341f8 c7a6e858 c7a6e8a0
[   10.711725] 1b80: c71bd380 0137be89 c7a86dfc c781bd00 c08c0554 00000000 00000000 c08c8968
[   10.719945] 1ba0: c7a36860 c00341f8 c6ed1cd0 00000000 c6ed1cd0 c067849c c6cf0900 20000013
[   10.728164] 1bc0: ffffffe1 c6ed1c2c c084d028 c7a86f40 00000000 00000000 00000000 00000000
[   10.736379] 1be0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.744595] 1c00: 00000000 c6ca8020 00000000 00000000 00000000 00000000 c6ca8028 00000000
[   10.752810] 1c20: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.761029] 1c40: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.769244] 1c60: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.777459] 1c80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.785677] 1ca0: c771f600 c08569b0 c79d4110 00000009 006e6163 00000000 00000000 00000000
[   10.793892] 1cc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.802111] 1ce0: 00000000 00000000 00000000 0137be89 c08c8010 c78f4440 00000000 c6ca8000
[   10.810330] 1d00: c084d028 c6ed1d6c c7249260 c091bbc4 00000000 c0517800 c6ed1d64 00000000
[   10.818548] 1d20: 006200ca ffffe000 c771f6e4 c6ed1db4 c6ed1db4 c065d938 0000000f 0137be89
[   10.826768] 1d40: 00000000 c7249260 c05176e0 c084d028 c6ca8000 00000064 00000000 c084d028
[   10.834987] 1d60: 00000000 c053c710 00000000 00000000 00000000 00000000 00000000 00000000
[   10.843204] 1d80: 00000000 00000000 00000000 0137be89 c790cc00 c6db6000 00000064 c7249260
[   10.851422] 1da0: c6ed1da8 c053bed0 7fffffff 0137be89 c6ed1f5c c084d028 c6db6000 c7249260
[   10.859641] 1dc0: 006000c0 00000064 00000000 c053c348 0000000c c6ed1f64 c084d028 00000000
[   10.867861] 1de0: c6ed1e38 00000008 c6cf0e40 00000000 000002ad 00000000 00000000 0137be89
[   10.876079] 1e00: bef24778 c6ed1f5c 00000000 c084d028 00000000 c773ee00 00000000 c6ed1efc
[   10.884297] 1e20: 00000000 c04e14d4 c6ed1f5c c04e1a40 c6ed1e54 00041000 00000000 bef24798
[   10.892515] 1e40: 00000064 c012d73c 00000000 00000000 00000000 c6ed3de0 00000070 006200ca
[   10.900733] 1e60: 00000031 00041000 c6f28000 c6f28000 00000000 00000000 00000000 00000010
[   10.908950] 1e80: 00000000 00000000 00000000 0137be89 c6ed1fb0 c6ed1fb0 c79d40e0 c7810500
[   10.917168] 1ea0: 80000005 00041350 00000070 c00117f0 c6ed0000 00000000 bef23f44 c015733c
[   10.925386] 1ec0: c789a030 00000000 00000000 00000000 00000000 c01306f0 00000000 c08517b8
[   10.933608] 1ee0: 00000005 c084d028 c0011a74 00041350 c6ed1fb0 00066328 bef24f30 c0011c64
[   10.941831] 1f00: 017ce000 c6ed3eb0 c6ed3ea0 c0166f58 c7810500 00000000 00000021 0137be89
[   10.950050] 1f20: c6ed3eb0 c084d028 bef2472c 00000000 c773ee00 c00091e4 c6ed0000 00000000
[   10.958270] 1f40: 0008c000 c04e2a20 00000000 00000000 bef23f44 00000000 fffffff7 c6ed1e7c
[   10.966485] 1f60: 0000000c 00000001 00000000 00000000 c6ed1e44 00000000 c00091e4 00000000
[   10.974703] 1f80: 00000000 00000000 00000000 0137be89 c00091e4 00000000 bef24e48 00041350
[   10.982922] 1fa0: 00000128 c0009000 00000000 bef24e48 00000003 bef2472c 00000000 00000000
[   10.991140] 1fc0: 00000000 bef24e48 00041350 00000128 5c387178 00000000 00066328 0008c000
[   10.999356] 1fe0: b6f49000 bef246d8 00062aa0 b6ed9918 60000010 00000003 00000000 00000000
[   11.007630] [<c0436f18>] (flexcan_write_le) from [<c0437680>] (flexcan_chip_start+0x450/0x474)
[   11.016307] [<c0437680>] (flexcan_chip_start) from [<c0438534>] (flexcan_open+0xf8/0x144)
[   11.024545] [<c0438534>] (flexcan_open) from [<c0504924>] (__dev_open+0xe8/0x174)
[   11.032084] [<c0504924>] (__dev_open) from [<c0504d0c>] (__dev_change_flags+0x160/0x1c8)
[   11.040227] [<c0504d0c>] (__dev_change_flags) from [<c0504d94>] (dev_change_flags+0x20/0x50)
[   11.048723] [<c0504d94>] (dev_change_flags) from [<c051b1cc>] (do_setlink+0x360/0xa88)
[   11.056695] [<c051b1cc>] (do_setlink) from [<c051c0cc>] (rtnl_newlink+0x4a4/0x6f4)
[   11.064317] [<c051c0cc>] (rtnl_newlink) from [<c0517800>] (rtnetlink_rcv_msg+0x120/0x2f4)
[   11.072566] [<c0517800>] (rtnetlink_rcv_msg) from [<c053c710>] (netlink_rcv_skb+0xbc/0x118)
[   11.080981] [<c053c710>] (netlink_rcv_skb) from [<c053bed0>] (netlink_unicast+0x184/0x1fc)
[   11.089304] [<c053bed0>] (netlink_unicast) from [<c053c348>] (netlink_sendmsg+0x338/0x38c)
[   11.097639] [<c053c348>] (netlink_sendmsg) from [<c04e14d4>] (sock_sendmsg+0x1c/0x2c)
[   11.105532] [<c04e14d4>] (sock_sendmsg) from [<c04e1a40>] (___sys_sendmsg+0x210/0x22c)
[   11.113503] [<c04e1a40>] (___sys_sendmsg) from [<c04e2a20>] (__sys_sendmsg+0x54/0x94)
[   11.121383] [<c04e2a20>] (__sys_sendmsg) from [<c0009000>] (ret_fast_syscall+0x0/0x50)
[   11.129327] Exception stack(0xc6ed1fa8 to 0xc6ed1ff0)
[   11.134416] 1fa0:                   00000000 bef24e48 00000003 bef2472c 00000000 00000000
[   11.142634] 1fc0: 00000000 bef24e48 00041350 00000128 5c387178 00000000 00066328 0008c000
[   11.150842] 1fe0: b6f49000 bef246d8 00062aa0 b6ed9918
[   11.155941] Code: e5813000 e12fff1e e5900000 e12fff1e (e5810000) 
[   11.162068] ---[ end trace aaa0ccd32b1426b3 ]---

I have a fix for 4.19.14 that doesn't fit as is on current mainline. I
didn't check if mainline is still affected, but will do so later today.
I will send the fix for 4.19.x as a reply to this mail.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH v4.19.x] can: flexcan: fix out-of-bounds array access

[Uwe Kleine-König]

The loop body uses regs->mb[i], so i should not ensured to be smaller
than ARRAY_SIZE(regs->mb).

This change fixes a backtrace during boot on an i.MX25 based machine:

[   10.093464] Unhandled fault: external abort on non-linefetch (0x808) at 0xc89f4480
[   10.101096] pgd = (ptrval)
[   10.103830] [c89f4480] *pgd=87885811, *pte=43f88653, *ppte=43f88552
[   10.110174] Internal error: : 808 [#1] PREEMPT ARM
[   10.114988] Modules linked in:
[   10.118096] CPU: 0 PID: 680 Comm: ip Not tainted 4.19.13-20180926-1-00011-ga0dd04ff511f-dirty #7
[   10.126904] Hardware name: Freescale i.MX25 (Device Tree Support)
[   10.133066] PC is at flexcan_write_le+0x0/0x8
[   10.137469] LR is at flexcan_chip_start+0x450/0x474
[   10.142373] pc : [<c0436f18>]    lr : [<c0437680>]    psr: 20000013
[   10.148658] sp : c6e0f900  ip : 00000000  fp : c708b000
[   10.153903] r10: c0789a38  r9 : c89f4004  r8 : c89f4490
[   10.159150] r7 : 00000000  r6 : c708b3e0  r5 : c89f4000  r4 : c89f4490
[   10.165697] r3 : c0436f10  r2 : c0436f18  r1 : c89f4480  r0 : 00000000
[   10.172249] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   10.179407] Control: 0005317f  Table: 86e40000  DAC: 00000051
[   10.185173] Process ip (pid: 680, stack limit = 0x(ptrval))
[   10.190768] Stack: (0xc6e0f900 to 0xc6e10000)
[   10.195168] f900: 00000080 c708b000 00000000 c70c0780 00000001 00040080 c06a4760 c6d15e10
[   10.203388] f920: 00000000 c0438534 c708b000 c708b000 c708b000 c084d028 c06a4760 c0504924
[   10.211603] f940: 00000000 0000003c ffffe000 c708b000 00000000 cf29b3e2 c708b000 c084d028
[   10.219824] f960: 00040081 c0504d0c 000000be 00000000 c06a485c c6e0f9fc c708b000 cf29b3e2
[   10.228040] f980: c084d028 c708b000 00000000 c708b138 00040080 c6e0fca0 c06a4760 c0504d94
[   10.236258] f9a0: c708b000 c6e0fbd0 c084d028 c72346c0 c6e0fca0 c051b1cc 000000a1 0000000b
[   10.244479] f9c0: 00000002 c6e0fd6c c793b3b8 c708b000 c6e0fb0c c6e0fb0c c084d028 c6e0f9fc
[   10.252698] f9e0: c06a485c c708b3fc c708b000 c04364a4 00000000 c6d15e40 00000001 0000c350
[   10.260915] fa00: 00000359 00000594 00000005 cf29b3e2 00000002 c708b000 00000000 00000000
[   10.269132] fa20: c6d15e00 c6e0fb0c c084d7f8 c6e0fd6c 00000000 c051c0cc c6e0fbd0 c6e0fca0
[   10.277348] fa40: 00000003 c032efa8 c08b1940 00000000 c72346c0 c6d15e10 0000007c c055c32c
[   10.285566] fa60: 00000000 c6d15e2c c6d15e34 00000000 00000000 00000000 0000fe88 c0856980
[   10.293779] fa80: c6c8e920 0000007c 000005a8 00000000 0000007c 00000000 00000000 00000000
[   10.301996] faa0: 00000000 0000fe88 00000000 c0042e14 00000000 00000000 00000000 00000000
[   10.310213] fac0: c084d028 c00494b8 c7aa2150 c00341f8 f9b002f7 c004951c c7aa2150 cf29b3e2
[   10.318429] fae0: c6d7497c c781bd00 c08c0554 00000000 00000000 c08c8968 c7a2b860 c00341f8
[   10.326648] fb00: c084d028 c08569b0 c7aa2150 00000000 c6d15e40 00000000 00000000 00000000
[   10.334861] fb20: 00000000 c6d15e38 00000000 00000000 00000000 00000000 00000000 00000000
[   10.343078] fb40: 00000000 00000000 00000000 c6c7e200 c6e0fb84 c025a9bc 00000000 00000000
[   10.351294] fb60: c6d74958 c025a9bc 00000000 c0335c90 c72f7e00 c00341f8 c6d74a78 c6d74ac0
[   10.359513] fb80: c71af3e0 cf29b3e2 c6d164fc c781bd00 c08c0554 00000000 00000000 c08c8968
[   10.367731] fba0: c7a2b860 c00341f8 c6e0fcd0 00000000 c6e0fcd0 c067849c c6cd6bc0 20000013
[   10.375949] fbc0: ffffffe1 c6e0fc2c c084d028 c6d16640 00000000 00000000 00000000 00000000
[   10.384164] fbe0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.392378] fc00: 00000000 c6d15e20 00000000 00000000 00000000 00000000 c6d15e28 00000000
[   10.400595] fc20: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.408808] fc40: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.417024] fc60: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.425237] fc80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.433454] fca0: c7711300 c08569b0 c7aa2150 00000009 006e6163 00000000 00000000 00000000
[   10.441668] fcc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   10.449885] fce0: 00000000 00000000 00000000 cf29b3e2 c08c8010 c78f4440 00000000 c6d15e00
[   10.458101] fd00: c084d028 c6e0fd6c c72346c0 c091bbc4 00000000 c0517800 c6e0fd64 00000000
[   10.466318] fd20: 006200ca ffffe000 c77114b4 c6e0fdb4 c6e0fdb4 c065d938 0000000f cf29b3e2
[   10.474537] fd40: 00000000 c72346c0 c05176e0 c084d028 c6d15e00 00000064 00000000 c084d028
[   10.482750] fd60: 00000000 c053c710 00000000 00000000 00000000 00000000 00000000 00000000
[   10.490966] fd80: 00000000 00000000 00000000 cf29b3e2 c790cc00 c6d82c00 00000064 c72346c0
[   10.499184] fda0: c6e0fda8 c053bed0 7fffffff cf29b3e2 c6e0ff5c c084d028 c6d82c00 c72346c0
[   10.507402] fdc0: 006000c0 00000064 00000000 c053c348 0000000c c6e0ff64 c084d028 00000000
[   10.515616] fde0: c6e0fe38 00000008 c6cd6b40 00000000 000002a8 00000000 00000000 cf29b3e2
[   10.523832] fe00: be84d778 c6e0ff5c 00000000 c084d028 00000000 c76ec060 00000000 c6e0fefc
[   10.532052] fe20: 00000000 c04e14d4 c6e0ff5c c04e1a40 c6e0fe54 00041000 00000000 be84d798
[   10.540266] fe40: 00000064 c012d73c 00000000 004c4b40 00000000 c6e2a560 00000070 006200ca
[   10.548482] fe60: 00000031 00041000 c6e40000 c6e40000 00000000 00000000 00000000 00000010
[   10.556697] fe80: 00000000 00000000 00000000 cf29b3e2 c6e0ffb0 c6e0ffb0 c7aa2120 c6d55ba0
[   10.564915] fea0: 80000005 00041350 00000070 c00117f0 c6e0e000 00000000 be84cf44 c015733c
[   10.573133] fec0: c789a030 00000000 00000000 00000000 00000000 c01306f0 00000000 c08517b8
[   10.581350] fee0: 00000005 c084d028 c0011a74 00041350 c6e0ffb0 00066328 be84df30 c0011c64
[   10.589568] ff00: 0042a000 c6e2a630 c6e2a620 c0166f58 c6d55ba0 00000000 00000021 cf29b3e2
[   10.597786] ff20: c6e2a630 c084d028 be84d72c 00000000 c76ec060 c00091e4 c6e0e000 00000000
[   10.606004] ff40: 0008c000 c04e2a20 00000000 00000000 be84cf44 00000000 fffffff7 c6e0fe7c
[   10.614221] ff60: 0000000c 00000001 00000000 00000000 c6e0fe44 00000000 c00091e4 00000000
[   10.622437] ff80: 00000000 00000000 00000000 cf29b3e2 c00091e4 00000000 be84de48 00041350
[   10.630657] ffa0: 00000128 c0009000 00000000 be84de48 00000003 be84d72c 00000000 00000000
[   10.638873] ffc0: 00000000 be84de48 00041350 00000128 5c3875b0 00000000 00066328 0008c000
[   10.647091] ffe0: b6ee0000 be84d6d8 00062aa0 b6e70918 60000010 00000003 00000000 00000000
[   10.655357] [<c0436f18>] (flexcan_write_le) from [<c0437680>] (flexcan_chip_start+0x450/0x474)
[   10.664035] [<c0437680>] (flexcan_chip_start) from [<c0438534>] (flexcan_open+0xf8/0x144)
[   10.672278] [<c0438534>] (flexcan_open) from [<c0504924>] (__dev_open+0xe8/0x174)
[   10.679814] [<c0504924>] (__dev_open) from [<c0504d0c>] (__dev_change_flags+0x160/0x1c8)
[   10.687956] [<c0504d0c>] (__dev_change_flags) from [<c0504d94>] (dev_change_flags+0x20/0x50)
[   10.696452] [<c0504d94>] (dev_change_flags) from [<c051b1cc>] (do_setlink+0x360/0xa88)
[   10.704424] [<c051b1cc>] (do_setlink) from [<c051c0cc>] (rtnl_newlink+0x4a4/0x6f4)
[   10.712046] [<c051c0cc>] (rtnl_newlink) from [<c0517800>] (rtnetlink_rcv_msg+0x120/0x2f4)
[   10.720294] [<c0517800>] (rtnetlink_rcv_msg) from [<c053c710>] (netlink_rcv_skb+0xbc/0x118)
[   10.728707] [<c053c710>] (netlink_rcv_skb) from [<c053bed0>] (netlink_unicast+0x184/0x1fc)
[   10.737028] [<c053bed0>] (netlink_unicast) from [<c053c348>] (netlink_sendmsg+0x338/0x38c)
[   10.745363] [<c053c348>] (netlink_sendmsg) from [<c04e14d4>] (sock_sendmsg+0x1c/0x2c)
[   10.753254] [<c04e14d4>] (sock_sendmsg) from [<c04e1a40>] (___sys_sendmsg+0x210/0x22c)
[   10.761226] [<c04e1a40>] (___sys_sendmsg) from [<c04e2a20>] (__sys_sendmsg+0x54/0x94)
[   10.769103] [<c04e2a20>] (__sys_sendmsg) from [<c0009000>] (ret_fast_syscall+0x0/0x50)
[   10.777047] Exception stack(0xc6e0ffa8 to 0xc6e0fff0)
[   10.782132] ffa0:                   00000000 be84de48 00000003 be84d72c 00000000 00000000
[   10.790348] ffc0: 00000000 be84de48 00041350 00000128 5c3875b0 00000000 00066328 0008c000
[   10.798551] ffe0: b6ee0000 be84d6d8 00062aa0 b6e70918
[   10.803643] Code: e5813000 e12fff1e e5900000 e12fff1e (e5810000)
[   10.809769] ---[ end trace 1a4586e3b7840d04 ]---

Fixes: 24e5589791d0 ("can: flexcan: Always use last mailbox for TX")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@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 75ce11395ee8..ae219b8a7754 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -1004,7 +1004,7 @@ static int flexcan_chip_start(struct net_device *dev)
 		}
 	} else {
 		/* clear and invalidate unused mailboxes first */
-		for (i = FLEXCAN_TX_MB_RESERVED_OFF_FIFO; i <= ARRAY_SIZE(regs->mb); i++) {
+		for (i = FLEXCAN_TX_MB_RESERVED_OFF_FIFO; i < ARRAY_SIZE(regs->mb); i++) {
 			priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
 				    &regs->mb[i].can_ctrl);
 		}
-- 
2.20.1

[PATCH] can: flexcan: fix NULL pointer exception during bringup

[Uwe Kleine-König]

Commit cbffaf7aa09e ("can: flexcan: Always use last mailbox for TX")
introduced a loop letting i run up to (including) ARRAY_SIZE(regs->mb)
and in the body accessed regs->mb[i] which is an out-of-bounds array
access that then resulted in an access to an reserved register area.

Later this was changed by commit 0517961ccdf1 ("can: flexcan: Add
provision for variable payload size") to iterate a bit differently but
still runs one iteration too much resulting to call

	flexcan_get_mb(priv, priv->mb_count)

which results in a WARN_ON and then a NULL pointer exception. This
only affects devices compatible with "fsl,p1010-flexcan",
"fsl,imx53-flexcan", "fsl,imx35-flexcan", "fsl,imx25-flexcan",
"fsl,imx28-flexcan", so newer i.MX SoCs are not affected.

Fixes: cbffaf7aa09e ("can: flexcan: Always use last mailbox for TX")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@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 0f36eafe3ac1..4c0173d2aff1 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -1106,7 +1106,7 @@ static int flexcan_chip_start(struct net_device *dev)
 		}
 	} else {
 		/* clear and invalidate unused mailboxes first */
-		for (i = FLEXCAN_TX_MB_RESERVED_OFF_FIFO; i <= priv->mb_count; i++) {
+		for (i = FLEXCAN_TX_MB_RESERVED_OFF_FIFO; i < priv->mb_count; i++) {
 			mb = flexcan_get_mb(priv, i);
 			priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
 				    &mb->can_ctrl);
-- 
2.20.1

Re: [PATCH] can: flexcan: fix NULL pointer exception during bringup

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1306 bytes --]

On 1/11/19 12:20 PM, Uwe Kleine-König wrote:
> Commit cbffaf7aa09e ("can: flexcan: Always use last mailbox for TX")
> introduced a loop letting i run up to (including) ARRAY_SIZE(regs->mb)
> and in the body accessed regs->mb[i] which is an out-of-bounds array
> access that then resulted in an access to an reserved register area.
> 
> Later this was changed by commit 0517961ccdf1 ("can: flexcan: Add
> provision for variable payload size") to iterate a bit differently but
> still runs one iteration too much resulting to call
> 
> 	flexcan_get_mb(priv, priv->mb_count)
> 
> which results in a WARN_ON and then a NULL pointer exception. This
> only affects devices compatible with "fsl,p1010-flexcan",
> "fsl,imx53-flexcan", "fsl,imx35-flexcan", "fsl,imx25-flexcan",
> "fsl,imx28-flexcan", so newer i.MX SoCs are not affected.
> 
> Fixes: cbffaf7aa09e ("can: flexcan: Always use last mailbox for TX")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied to linux-can

Tnx,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 12/20] can: flexcan: remove not needed struct flexcan_priv::tx_mb and struct flexcan_priv::tx_mb_idx

[Marc Kleine-Budde]

The previous patch changes the TX path to always use the last mailbox
regardless of the used offload scheme (rx-fifo or timestamp based). This
means members "tx_mb" and "tx_mb_idx" of the struct flexcan_priv don't
depend on the offload scheme, so replace them by compile time constants.

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

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 677c41701cf3..68b46395c580 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -258,9 +258,7 @@ struct flexcan_priv {
 	struct can_rx_offload offload;
 
 	struct flexcan_regs __iomem *regs;
-	struct flexcan_mb __iomem *tx_mb;
 	struct flexcan_mb __iomem *tx_mb_reserved;
-	u8 tx_mb_idx;
 	u32 reg_ctrl_default;
 	u32 reg_imask1_default;
 	u32 reg_imask2_default;
@@ -514,6 +512,7 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
 static netdev_tx_t flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->regs;
 	struct can_frame *cf = (struct can_frame *)skb->data;
 	u32 can_id;
 	u32 data;
@@ -536,17 +535,17 @@ static netdev_tx_t flexcan_start_xmit(struct sk_buff *skb, struct net_device *de
 
 	if (cf->can_dlc > 0) {
 		data = be32_to_cpup((__be32 *)&cf->data[0]);
-		priv->write(data, &priv->tx_mb->data[0]);
+		priv->write(data, &regs->mb[FLEXCAN_TX_MB].data[0]);
 	}
 	if (cf->can_dlc > 4) {
 		data = be32_to_cpup((__be32 *)&cf->data[4]);
-		priv->write(data, &priv->tx_mb->data[1]);
+		priv->write(data, &regs->mb[FLEXCAN_TX_MB].data[1]);
 	}
 
 	can_put_echo_skb(skb, dev, 0);
 
-	priv->write(can_id, &priv->tx_mb->can_id);
-	priv->write(ctrl, &priv->tx_mb->can_ctrl);
+	priv->write(can_id, &regs->mb[FLEXCAN_TX_MB].can_id);
+	priv->write(ctrl, &regs->mb[FLEXCAN_TX_MB].can_ctrl);
 
 	/* Errata ERR005829 step8:
 	 * Write twice INACTIVE(0x8) code to first MB.
@@ -737,7 +736,7 @@ static inline u64 flexcan_read_reg_iflag_rx(struct flexcan_priv *priv)
 	u32 iflag1, iflag2;
 
 	iflag2 = priv->read(&regs->iflag2) & priv->reg_imask2_default &
-		~FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
+		~FLEXCAN_IFLAG_MB(FLEXCAN_TX_MB);
 	iflag1 = priv->read(&regs->iflag1) & priv->reg_imask1_default;
 
 	return (u64)iflag2 << 32 | iflag1;
@@ -787,7 +786,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	reg_iflag2 = priv->read(&regs->iflag2);
 
 	/* transmission complete interrupt */
-	if (reg_iflag2 & FLEXCAN_IFLAG_MB(priv->tx_mb_idx)) {
+	if (reg_iflag2 & FLEXCAN_IFLAG_MB(FLEXCAN_TX_MB)) {
 		handled = IRQ_HANDLED;
 		stats->tx_bytes += can_get_echo_skb(dev, 0);
 		stats->tx_packets++;
@@ -795,8 +794,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 
 		/* after sending a RTR frame MB is in RX mode */
 		priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
-			    &priv->tx_mb->can_ctrl);
-		priv->write(FLEXCAN_IFLAG_MB(priv->tx_mb_idx), &regs->iflag2);
+			    &regs->mb[FLEXCAN_TX_MB].can_ctrl);
+		priv->write(FLEXCAN_IFLAG_MB(FLEXCAN_TX_MB), &regs->iflag2);
 		netif_wake_queue(dev);
 	}
 
@@ -938,7 +937,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
 	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV |
 		FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_SRX_DIS | FLEXCAN_MCR_IRMQ |
-		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
+		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_MAXMB(FLEXCAN_TX_MB);
 
 	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
 		reg_mcr &= ~FLEXCAN_MCR_FEN;
@@ -1006,7 +1005,7 @@ static int flexcan_chip_start(struct net_device *dev)
 
 	/* mark TX mailbox as INACTIVE */
 	priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
-		    &priv->tx_mb->can_ctrl);
+		    &regs->mb[FLEXCAN_TX_MB].can_ctrl);
 
 	/* acceptance mask/acceptance code (accept everything) */
 	priv->write(0x0, &regs->rxgmask);
@@ -1365,11 +1364,9 @@ static int flexcan_probe(struct platform_device *pdev)
 		priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP];
 	else
 		priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_FIFO];
-	priv->tx_mb_idx = FLEXCAN_TX_MB;
-	priv->tx_mb = &regs->mb[priv->tx_mb_idx];
 
 	priv->reg_imask1_default = 0;
-	priv->reg_imask2_default = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
+	priv->reg_imask2_default = FLEXCAN_IFLAG_MB(FLEXCAN_TX_MB);
 
 	priv->offload.mailbox_read = flexcan_mailbox_read;
 
-- 
2.19.1

[PATCH 13/20] can: dev: can_get_echo_skb(): factor out non sending code to __can_get_echo_skb()

[Marc Kleine-Budde]

This patch factors out all non sending parts of can_get_echo_skb() into
a seperate function __can_get_echo_skb(), so that it can be re-used in
an upcoming patch.

Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/dev.c   | 36 +++++++++++++++++++++++++-----------
 include/linux/can/dev.h |  1 +
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 49163570a63a..80530ab37b1e 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -477,14 +477,7 @@ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(can_put_echo_skb);
 
-/*
- * Get the skb from the stack and loop it back locally
- *
- * The function is typically called when the TX done interrupt
- * is handled in the device driver. The driver must protect
- * access to priv->echo_skb, if necessary.
- */
-unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx)
+struct sk_buff *__can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr)
 {
 	struct can_priv *priv = netdev_priv(dev);
 
@@ -495,13 +488,34 @@ unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx)
 		struct can_frame *cf = (struct can_frame *)skb->data;
 		u8 dlc = cf->can_dlc;
 
-		netif_rx(priv->echo_skb[idx]);
+		*len_ptr = dlc;
 		priv->echo_skb[idx] = NULL;
 
-		return dlc;
+		return skb;
 	}
 
-	return 0;
+	return NULL;
+}
+
+/*
+ * Get the skb from the stack and loop it back locally
+ *
+ * The function is typically called when the TX done interrupt
+ * is handled in the device driver. The driver must protect
+ * access to priv->echo_skb, if necessary.
+ */
+unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx)
+{
+	struct sk_buff *skb;
+	u8 len;
+
+	skb = __can_get_echo_skb(dev, idx, &len);
+	if (!skb)
+		return 0;
+
+	netif_rx(skb);
+
+	return len;
 }
 EXPORT_SYMBOL_GPL(can_get_echo_skb);
 
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index a83e1f632eb7..f01623aef2f7 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -169,6 +169,7 @@ void can_change_state(struct net_device *dev, struct can_frame *cf,
 
 void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
 		      unsigned int idx);
+struct sk_buff *__can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr);
 unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx);
 void can_free_echo_skb(struct net_device *dev, unsigned int idx);
 
-- 
2.19.1

[PATCH 14/20] can: dev: __can_get_echo_skb(): replace struct can_frame by canfd_frame to access frame length

[Marc Kleine-Budde]

This patch replaces the use of "struct can_frame::can_dlc" by "struct
canfd_frame::len" to access the frame's length. As it is ensured that
both structures have a compatible memory layout for this member this is
no functional change. Futher, this compatibility is documented in a
comment.

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

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 80530ab37b1e..46cc5fec4043 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -484,11 +484,14 @@ struct sk_buff *__can_get_echo_skb(struct net_device *dev, unsigned int idx, u8
 	BUG_ON(idx >= priv->echo_skb_max);
 
 	if (priv->echo_skb[idx]) {
+		/* Using "struct canfd_frame::len" for the frame
+		 * length is supported on both CAN and CANFD frames.
+		 */
 		struct sk_buff *skb = priv->echo_skb[idx];
-		struct can_frame *cf = (struct can_frame *)skb->data;
-		u8 dlc = cf->can_dlc;
+		struct canfd_frame *cf = (struct canfd_frame *)skb->data;
+		u8 len = cf->len;
 
-		*len_ptr = dlc;
+		*len_ptr = len;
 		priv->echo_skb[idx] = NULL;
 
 		return skb;
-- 
2.19.1

[PATCH 15/20] can: dev: __can_get_echo_skb(): Don't crash the kernel if can_priv::echo_skb is accessed out of bounds

[Marc Kleine-Budde]

If the "struct can_priv::echo_skb" is accessed out of bounds would lead
to a kernel crash. Better print a sensible warning message instead and
try to recover.

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

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 46cc5fec4043..c05e4d50d43d 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -481,7 +481,11 @@ struct sk_buff *__can_get_echo_skb(struct net_device *dev, unsigned int idx, u8
 {
 	struct can_priv *priv = netdev_priv(dev);
 
-	BUG_ON(idx >= priv->echo_skb_max);
+	if (idx >= priv->echo_skb_max) {
+		netdev_err(dev, "%s: BUG! Trying to access can_priv::echo_skb out of bounds (%u/max %u)\n",
+			   __func__, idx, priv->echo_skb_max);
+		return NULL;
+	}
 
 	if (priv->echo_skb[idx]) {
 		/* Using "struct canfd_frame::len" for the frame
-- 
2.19.1

[PATCH 16/20] can: dev: __can_get_echo_skb(): print error message, if trying to echo non existing skb

[Marc Kleine-Budde]

Prior to echoing a successfully transmitted CAN frame (by calling
can_get_echo_skb()), CAN drivers have to put the CAN frame (by calling
can_put_echo_skb() in the transmit function). These put and get function
take an index as parameter, which is used to identify the CAN frame.

A driver calling can_get_echo_skb() with a index not pointing to a skb
is a BUG, so add an appropriate error message.

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

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index c05e4d50d43d..3b3f88ffab53 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -480,6 +480,8 @@ EXPORT_SYMBOL_GPL(can_put_echo_skb);
 struct sk_buff *__can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr)
 {
 	struct can_priv *priv = netdev_priv(dev);
+	struct sk_buff *skb = priv->echo_skb[idx];
+	struct canfd_frame *cf;
 
 	if (idx >= priv->echo_skb_max) {
 		netdev_err(dev, "%s: BUG! Trying to access can_priv::echo_skb out of bounds (%u/max %u)\n",
@@ -487,21 +489,20 @@ struct sk_buff *__can_get_echo_skb(struct net_device *dev, unsigned int idx, u8
 		return NULL;
 	}
 
-	if (priv->echo_skb[idx]) {
-		/* Using "struct canfd_frame::len" for the frame
-		 * length is supported on both CAN and CANFD frames.
-		 */
-		struct sk_buff *skb = priv->echo_skb[idx];
-		struct canfd_frame *cf = (struct canfd_frame *)skb->data;
-		u8 len = cf->len;
-
-		*len_ptr = len;
-		priv->echo_skb[idx] = NULL;
-
-		return skb;
+	if (!skb) {
+		netdev_err(dev, "%s: BUG! Trying to echo non existing skb: can_priv::echo_skb[%u]\n",
+			   __func__, idx);
+		return NULL;
 	}
 
-	return NULL;
+	/* Using "struct canfd_frame::len" for the frame
+	 * length is supported on both CAN and CANFD frames.
+	 */
+	cf = (struct canfd_frame *)skb->data;
+	*len_ptr = cf->len;
+	priv->echo_skb[idx] = NULL;
+
+	return skb;
 }
 
 /*
-- 
2.19.1

[PATCH 17/20] can: rx-offload: introduce can_rx_offload_get_echo_skb() and can_rx_offload_queue_sorted() functions

[Marc Kleine-Budde]

From: Oleksij Rempel <o.rempel@pengutronix.de>

Current CAN framework can't guarantee proper/chronological order
of RX and TX-ECHO messages. To make this possible, drivers should use
this functions instead of can_get_echo_skb().

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/rx-offload.c   | 46 ++++++++++++++++++++++++++++++++++
 include/linux/can/rx-offload.h |  4 +++
 2 files changed, 50 insertions(+)

diff --git a/drivers/net/can/rx-offload.c b/drivers/net/can/rx-offload.c
index c7d05027a7a0..c368686e2164 100644
--- a/drivers/net/can/rx-offload.c
+++ b/drivers/net/can/rx-offload.c
@@ -211,6 +211,52 @@ int can_rx_offload_irq_offload_fifo(struct can_rx_offload *offload)
 }
 EXPORT_SYMBOL_GPL(can_rx_offload_irq_offload_fifo);
 
+int can_rx_offload_queue_sorted(struct can_rx_offload *offload,
+				struct sk_buff *skb, u32 timestamp)
+{
+	struct can_rx_offload_cb *cb;
+	unsigned long flags;
+
+	if (skb_queue_len(&offload->skb_queue) >
+	    offload->skb_queue_len_max)
+		return -ENOMEM;
+
+	cb = can_rx_offload_get_cb(skb);
+	cb->timestamp = timestamp;
+
+	spin_lock_irqsave(&offload->skb_queue.lock, flags);
+	__skb_queue_add_sort(&offload->skb_queue, skb, can_rx_offload_compare);
+	spin_unlock_irqrestore(&offload->skb_queue.lock, flags);
+
+	can_rx_offload_schedule(offload);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(can_rx_offload_queue_sorted);
+
+unsigned int can_rx_offload_get_echo_skb(struct can_rx_offload *offload,
+					 unsigned int idx, u32 timestamp)
+{
+	struct net_device *dev = offload->dev;
+	struct net_device_stats *stats = &dev->stats;
+	struct sk_buff *skb;
+	u8 len;
+	int err;
+
+	skb = __can_get_echo_skb(dev, idx, &len);
+	if (!skb)
+		return 0;
+
+	err = can_rx_offload_queue_sorted(offload, skb, timestamp);
+	if (err) {
+		stats->rx_errors++;
+		stats->tx_fifo_errors++;
+	}
+
+	return len;
+}
+EXPORT_SYMBOL_GPL(can_rx_offload_get_echo_skb);
+
 int can_rx_offload_irq_queue_err_skb(struct can_rx_offload *offload, struct sk_buff *skb)
 {
 	if (skb_queue_len(&offload->skb_queue) >
diff --git a/include/linux/can/rx-offload.h b/include/linux/can/rx-offload.h
index cb31683bbe15..01a7c9e5d8d8 100644
--- a/include/linux/can/rx-offload.h
+++ b/include/linux/can/rx-offload.h
@@ -41,6 +41,10 @@ int can_rx_offload_add_timestamp(struct net_device *dev, struct can_rx_offload *
 int can_rx_offload_add_fifo(struct net_device *dev, struct can_rx_offload *offload, unsigned int weight);
 int can_rx_offload_irq_offload_timestamp(struct can_rx_offload *offload, u64 reg);
 int can_rx_offload_irq_offload_fifo(struct can_rx_offload *offload);
+int can_rx_offload_queue_sorted(struct can_rx_offload *offload,
+				struct sk_buff *skb, u32 timestamp);
+unsigned int can_rx_offload_get_echo_skb(struct can_rx_offload *offload,
+					 unsigned int idx, u32 timestamp);
 int can_rx_offload_irq_queue_err_skb(struct can_rx_offload *offload, struct sk_buff *skb);
 void can_rx_offload_reset(struct can_rx_offload *offload);
 void can_rx_offload_del(struct can_rx_offload *offload);
-- 
2.19.1

[PATCH 18/20] can: flexcan: handle tx-complete CAN frames via rx-offload infrastructure

[Marc Kleine-Budde]

From: Oleksij Rempel <o.rempel@pengutronix.de>

Current flexcan driver will put TX-ECHO in regular unsorted way, in
this case TX-ECHO can come after the response to the same TXed message.
In some cases, for example for J1939 stack, things will break.
This patch is using new rx-offload API to put the messages just in the
right place.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 68b46395c580..41a175f80c4b 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -787,8 +787,11 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 
 	/* transmission complete interrupt */
 	if (reg_iflag2 & FLEXCAN_IFLAG_MB(FLEXCAN_TX_MB)) {
+		u32 reg_ctrl = priv->read(&regs->mb[FLEXCAN_TX_MB].can_ctrl);
+
 		handled = IRQ_HANDLED;
-		stats->tx_bytes += can_get_echo_skb(dev, 0);
+		stats->tx_bytes += can_rx_offload_get_echo_skb(&priv->offload,
+							       0, reg_ctrl << 16);
 		stats->tx_packets++;
 		can_led_event(dev, CAN_LED_EVENT_TX);
 
-- 
2.19.1

[PATCH 19/20] can: rx-offload: rename can_rx_offload_irq_queue_err_skb() to can_rx_offload_queue_tail()

[Marc Kleine-Budde]

From: Oleksij Rempel <o.rempel@pengutronix.de>

This function has nothing todo with error.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c      | 4 ++--
 drivers/net/can/rx-offload.c   | 5 +++--
 include/linux/can/rx-offload.h | 3 ++-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 41a175f80c4b..5923bd0ec118 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -610,7 +610,7 @@ static void flexcan_irq_bus_err(struct net_device *dev, u32 reg_esr)
 	if (tx_errors)
 		dev->stats.tx_errors++;
 
-	can_rx_offload_irq_queue_err_skb(&priv->offload, skb);
+	can_rx_offload_queue_tail(&priv->offload, skb);
 }
 
 static void flexcan_irq_state(struct net_device *dev, u32 reg_esr)
@@ -650,7 +650,7 @@ static void flexcan_irq_state(struct net_device *dev, u32 reg_esr)
 	if (unlikely(new_state == CAN_STATE_BUS_OFF))
 		can_bus_off(dev);
 
-	can_rx_offload_irq_queue_err_skb(&priv->offload, skb);
+	can_rx_offload_queue_tail(&priv->offload, skb);
 }
 
 static inline struct flexcan_priv *rx_offload_to_priv(struct can_rx_offload *offload)
diff --git a/drivers/net/can/rx-offload.c b/drivers/net/can/rx-offload.c
index c368686e2164..2ce4fa8698c7 100644
--- a/drivers/net/can/rx-offload.c
+++ b/drivers/net/can/rx-offload.c
@@ -257,7 +257,8 @@ unsigned int can_rx_offload_get_echo_skb(struct can_rx_offload *offload,
 }
 EXPORT_SYMBOL_GPL(can_rx_offload_get_echo_skb);
 
-int can_rx_offload_irq_queue_err_skb(struct can_rx_offload *offload, struct sk_buff *skb)
+int can_rx_offload_queue_tail(struct can_rx_offload *offload,
+			      struct sk_buff *skb)
 {
 	if (skb_queue_len(&offload->skb_queue) >
 	    offload->skb_queue_len_max)
@@ -268,7 +269,7 @@ int can_rx_offload_irq_queue_err_skb(struct can_rx_offload *offload, struct sk_b
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(can_rx_offload_irq_queue_err_skb);
+EXPORT_SYMBOL_GPL(can_rx_offload_queue_tail);
 
 static int can_rx_offload_init_queue(struct net_device *dev, struct can_rx_offload *offload, unsigned int weight)
 {
diff --git a/include/linux/can/rx-offload.h b/include/linux/can/rx-offload.h
index 01a7c9e5d8d8..8268811a697e 100644
--- a/include/linux/can/rx-offload.h
+++ b/include/linux/can/rx-offload.h
@@ -45,7 +45,8 @@ int can_rx_offload_queue_sorted(struct can_rx_offload *offload,
 				struct sk_buff *skb, u32 timestamp);
 unsigned int can_rx_offload_get_echo_skb(struct can_rx_offload *offload,
 					 unsigned int idx, u32 timestamp);
-int can_rx_offload_irq_queue_err_skb(struct can_rx_offload *offload, struct sk_buff *skb);
+int can_rx_offload_queue_tail(struct can_rx_offload *offload,
+			      struct sk_buff *skb);
 void can_rx_offload_reset(struct can_rx_offload *offload);
 void can_rx_offload_del(struct can_rx_offload *offload);
 void can_rx_offload_enable(struct can_rx_offload *offload);
-- 
2.19.1

[PATCH 20/20] can: flexcan: use can_rx_offload_queue_sorted() for flexcan_irq_bus_*()

[Marc Kleine-Budde]

From: Oleksij Rempel <o.rempel@pengutronix.de>

Currently, in case of bus error, driver will generate error message and put
in the tail of the message queue. To avoid confusions, this change should
place the bus related messages in proper order.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 5923bd0ec118..75ce11395ee8 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -561,9 +561,13 @@ static netdev_tx_t flexcan_start_xmit(struct sk_buff *skb, struct net_device *de
 static void flexcan_irq_bus_err(struct net_device *dev, u32 reg_esr)
 {
 	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->regs;
 	struct sk_buff *skb;
 	struct can_frame *cf;
 	bool rx_errors = false, tx_errors = false;
+	u32 timestamp;
+
+	timestamp = priv->read(&regs->timer) << 16;
 
 	skb = alloc_can_err_skb(dev, &cf);
 	if (unlikely(!skb))
@@ -610,17 +614,21 @@ static void flexcan_irq_bus_err(struct net_device *dev, u32 reg_esr)
 	if (tx_errors)
 		dev->stats.tx_errors++;
 
-	can_rx_offload_queue_tail(&priv->offload, skb);
+	can_rx_offload_queue_sorted(&priv->offload, skb, timestamp);
 }
 
 static void flexcan_irq_state(struct net_device *dev, u32 reg_esr)
 {
 	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->regs;
 	struct sk_buff *skb;
 	struct can_frame *cf;
 	enum can_state new_state, rx_state, tx_state;
 	int flt;
 	struct can_berr_counter bec;
+	u32 timestamp;
+
+	timestamp = priv->read(&regs->timer) << 16;
 
 	flt = reg_esr & FLEXCAN_ESR_FLT_CONF_MASK;
 	if (likely(flt == FLEXCAN_ESR_FLT_CONF_ACTIVE)) {
@@ -650,7 +658,7 @@ static void flexcan_irq_state(struct net_device *dev, u32 reg_esr)
 	if (unlikely(new_state == CAN_STATE_BUS_OFF))
 		can_bus_off(dev);
 
-	can_rx_offload_queue_tail(&priv->offload, skb);
+	can_rx_offload_queue_sorted(&priv->offload, skb, timestamp);
 }
 
 static inline struct flexcan_priv *rx_offload_to_priv(struct can_rx_offload *offload)
-- 
2.19.1