Hello,
There was heavy system load once upon and it was possible that userspace
tool did not drain backlog on time. This case ended up with massive dump
of call stack from skb_release_head_state() and bottom of the call stack
was flexcan_irq() calling can_get_echo_skb(). I can post the log if
someone wants to see it.
I created and roughly tested on my imx6ull platform attached patch. the
can_get_echo_skb() is being called from napi's callback now and I
decided to use 24 message boxes for tx purpose to reduce wakeup number
of napi's callback.
I consider this patch as engineering version and look for advice or
discussion about the idea.
--
Pozdrawiam / Best Regards / Freundliche Grüße
Krzysztof Błaszkowski
Projektant / Elektronik
Ekoenergetyka-Polska Sp. z o.o.
ul.Nowy Kisielin - Wysockiego 8,
66-002 Zielona Góra
Tel./Fax +48 68 328 20 68
/
/www.ekoenergetyka.com.pl <http://www.ekoenergetyka.com.pl/>
_Dane do faktury*:*_
Ekoenergetyka - Polska sp. z o.o.
ul. Nowy Kisielin - A. Wysockiego 8
66-002 Zielona Góra
NIP: 973-101-39-38
Regon: 081115852
BDO: 000014716
Sąd Rejonowy w Zielonej Górze, VIII Wydział Gospodarczy KRS
KRS: 0000452958
Kapitał zakładowy: 133.300,00 zł
>From 86879f7da1f4075547de6e880fdd187e03580031 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krzysztof=20B=C5=82aszkowski?=
<krzysztof.blaszkowski@xxxxxxxxxxxxxxxxxxxx>
Date: Wed, 26 Sep 2018 09:47:38 +0200
Subject: [PATCH 2/2] This change is supposed to avoid warn_on(in_irq()) from
skb_release_head_state() and the call flow was: flexcan_irq() ->
can_get_echo_skb() -> .. enqueue_to_backlog() -> kfree_skb()
24 tx MBs support and napi poll for tx, MBs #40 .. 63 are used for transmission,
MB #39 is reserved for the errata
can_get_echo_skb() moved to tx part of flexcan_poll(),
clk_set_rate(clk_get("per"), 60M instead of default 30M) due to usage of all hw MBs
---
drivers/net/can/flexcan.c | 219 ++++++++++++++++++++++++++++++++++++----------
1 file changed, 173 insertions(+), 46 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 353b4a3..07dfe86 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -151,16 +151,16 @@
FLEXCAN_ESR_WAK_INT)
/* FLEXCAN interrupt flag register (IFLAG) bits */
-/* Errata ERR005829 step7: Reserve first valid MB */
-#define FLEXCAN_TX_BUF_RESERVED 8
-#define FLEXCAN_TX_BUF_ID 9
+/* Errata ERR005829 step7: Reserve first valid MB, ie. TX_MB0 - 1 */
+#define FLEXCAN_TX_MB0 40
#define FLEXCAN_IFLAG_BUF(x) BIT(x)
#define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW BIT(7)
#define FLEXCAN_IFLAG_RX_FIFO_WARN BIT(6)
#define FLEXCAN_IFLAG_RX_FIFO_AVAILABLE BIT(5)
#define FLEXCAN_IFLAG_DEFAULT \
(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW | FLEXCAN_IFLAG_RX_FIFO_AVAILABLE | \
- FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))
+ 0)
+// FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))
/* FLEXCAN message buffers */
#define FLEXCAN_MB_CODE_RX_INACTIVE (0x0 << 24)
@@ -200,6 +200,10 @@
#define FLEXCAN_QUIRK_DISABLE_RXFG BIT(2) /* Disable RX FIFO Global mask */
#define FLEXCAN_QUIRK_DISABLE_MECR BIT(3) /* Disble Memory error detection */
+
+#define FLEXCAN_PROCESS_PEND_TXMB 31
+#define FXCAN_0_DBG 0
+
/* Structure of the message buffer */
struct flexcan_mb {
u32 can_ctrl;
@@ -276,8 +280,12 @@ struct flexcan_priv {
struct flexcan_platform_data *pdata;
const struct flexcan_devtype_data *devtype_data;
struct regulator *reg_xceiver;
- int id;
struct flexcan_stop_mode stm;
+ int id;
+ int txMBs;
+ unsigned long txMB_idle_set;
+ unsigned long txMB_pending_set;
+ unsigned long txMB_poll_last;
};
static struct flexcan_devtype_data fsl_p1010_devtype_data = {
@@ -509,20 +517,17 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
return err;
}
-static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
+
+static void flexcan_txmb_cp(struct sk_buff *skb, struct net_device *dev, int imb)
{
- const struct flexcan_priv *priv = netdev_priv(dev);
+ struct flexcan_priv *priv = netdev_priv(dev);
struct flexcan_regs __iomem *regs = priv->regs;
struct can_frame *cf = (struct can_frame *)skb->data;
+ struct flexcan_mb *fxmb = ®s->mb[FLEXCAN_TX_MB0 + imb];
u32 can_id;
u32 data;
u32 ctrl = FLEXCAN_MB_CODE_TX_DATA | (cf->can_dlc << 16);
- if (can_dropped_invalid_skb(dev, skb))
- return NETDEV_TX_OK;
-
- netif_stop_queue(dev);
-
if (cf->can_id & CAN_EFF_FLAG) {
can_id = cf->can_id & CAN_EFF_MASK;
ctrl |= FLEXCAN_MB_CNT_IDE | FLEXCAN_MB_CNT_SRR;
@@ -535,29 +540,62 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
if (cf->can_dlc > 0) {
data = be32_to_cpup((__be32 *)&cf->data[0]);
- flexcan_write(data, ®s->mb[FLEXCAN_TX_BUF_ID].data[0]);
+ flexcan_write(data, &fxmb->data[0]);
}
if (cf->can_dlc > 4) {
data = be32_to_cpup((__be32 *)&cf->data[4]);
- flexcan_write(data, ®s->mb[FLEXCAN_TX_BUF_ID].data[1]);
+ flexcan_write(data, &fxmb->data[1]);
}
- can_put_echo_skb(skb, dev, 0);
- flexcan_write(can_id, ®s->mb[FLEXCAN_TX_BUF_ID].can_id);
- flexcan_write(ctrl, ®s->mb[FLEXCAN_TX_BUF_ID].can_ctrl);
+#if FXCAN_0_DBG
+printk("%s:%d mb idx %d, is %08lx, ps %08lx\n", __FUNCTION__, __LINE__, imb,
+ priv->txMB_idle_set, priv->txMB_pending_set);
+#endif
+ can_put_echo_skb(skb, dev, imb);
+
+ flexcan_write(can_id, &fxmb->can_id);
+ flexcan_write(ctrl, &fxmb->can_ctrl);
+}
+
+
+static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ struct flexcan_priv *priv = netdev_priv(dev);
+ struct flexcan_regs __iomem *regs = priv->regs;
+ int j, idle_mbs;
+
+ if (can_dropped_invalid_skb(dev, skb))
+ return NETDEV_TX_OK;
+
+
+ for (j = 0, idle_mbs = 0; j < priv->txMBs; j++) {
+ if (skb) {
+ if (test_and_clear_bit(j, &priv->txMB_idle_set)) {
+ flexcan_txmb_cp(skb, dev, j);
+ skb = NULL;
+ }
+ } else if (priv->txMB_idle_set & (1 << j))
+ idle_mbs++;
+ }
+
+ if (idle_mbs < 1 && skb) {
+ netif_stop_queue(dev);
+ return NETDEV_TX_BUSY;
+ }
/* Errata ERR005829 step8:
* Write twice INACTIVE(0x8) code to first MB.
*/
flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
- ®s->mb[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+ ®s->mb[FLEXCAN_TX_MB0 - 1].can_ctrl);
flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
- ®s->mb[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
+ ®s->mb[FLEXCAN_TX_MB0 - 1].can_ctrl);
return NETDEV_TX_OK;
}
+
static void do_bus_err(struct net_device *dev,
struct can_frame *cf, u32 reg_esr)
{
@@ -718,13 +756,32 @@ static int flexcan_read_frame(struct net_device *dev)
return 1;
}
+static int flexcan_poll_txmbs(struct net_device *dev, int quota, int work_done)
+{
+ struct flexcan_priv *priv = netdev_priv(dev);
+ struct net_device_stats *stats = &dev->stats;
+ int j;
+
+ for (j = 0; j < priv->txMBs && work_done < quota; j++) {
+ if (test_and_clear_bit(j, &priv->txMB_pending_set)) {
+ stats->tx_bytes += can_get_echo_skb(dev, j);
+ stats->tx_packets++;
+ set_bit(j, &priv->txMB_idle_set);
+ work_done++;
+ }
+ }
+
+ return work_done;
+}
+
static int flexcan_poll(struct napi_struct *napi, int quota)
{
struct net_device *dev = napi->dev;
- const struct flexcan_priv *priv = netdev_priv(dev);
+ struct flexcan_priv *priv = netdev_priv(dev);
struct flexcan_regs __iomem *regs = priv->regs;
u32 reg_iflag1, reg_esr;
int work_done = 0;
+ int txq_stopped, j, tx_idle;
/* The error bits are cleared on read,
* use saved value from irq handler.
@@ -734,6 +791,35 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
/* handle state changes */
work_done += flexcan_poll_state(dev, reg_esr);
+ for (j = 0, tx_idle = 0; j < priv->txMBs; j++) {
+ if (priv->txMB_idle_set & (1 << j))
+ tx_idle++;
+ }
+
+ if ((txq_stopped = netif_queue_stopped(dev)) ||
+ tx_idle < priv->txMBs / 2 ||
+ test_and_clear_bit(FLEXCAN_PROCESS_PEND_TXMB, &priv->txMB_pending_set)) {
+ int old_work = work_done;
+
+#if FXCAN_0_DBG
+ printk("%s:%d tx_idle %d, txq stp %d, is %08lx, ps %08lx\n",
+ __FUNCTION__, __LINE__, tx_idle, txq_stopped,
+ priv->txMB_idle_set, priv->txMB_pending_set);
+#endif
+ work_done = flexcan_poll_txmbs(dev, quota, work_done);
+
+#if FXCAN_0_DBG
+ printk("%s:%d w %d, q %d, is %08lx, ps %08lx\n",
+ __FUNCTION__, __LINE__, work_done, quota,
+ priv->txMB_idle_set, priv->txMB_pending_set);
+#endif
+ if (work_done != old_work)
+ can_led_event(dev, CAN_LED_EVENT_TX);
+
+ if (txq_stopped)
+ netif_wake_queue(dev);
+ }
+
/* handle RX-FIFO */
reg_iflag1 = flexcan_read(®s->iflag1);
while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
@@ -759,12 +845,14 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
static irqreturn_t flexcan_irq(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
- struct net_device_stats *stats = &dev->stats;
struct flexcan_priv *priv = netdev_priv(dev);
struct flexcan_regs __iomem *regs = priv->regs;
u32 reg_iflag1, reg_esr;
+ u32 reg_iflag2;
+ int rc = IRQ_HANDLED;
reg_iflag1 = flexcan_read(®s->iflag1);
+ reg_iflag2 = flexcan_read(®s->iflag2);
reg_esr = flexcan_read(®s->esr);
/* ACK all bus error and state change IRQ sources */
@@ -778,6 +866,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
* - rx IRQ
* - state change IRQ
* - bus error IRQ and bus error reporting is activated
+ * - tx irq when number of idle tx MBs is less than all tx MBs /2
*/
if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
(reg_esr & FLEXCAN_ESR_ERR_STATE) ||
@@ -801,19 +890,40 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
}
/* transmission complete interrupt */
- if (reg_iflag1 & (1 << FLEXCAN_TX_BUF_ID)) {
- stats->tx_bytes += can_get_echo_skb(dev, 0);
- stats->tx_packets++;
- can_led_event(dev, CAN_LED_EVENT_TX);
+ if (reg_iflag2 & ((1 << (FLEXCAN_TX_MB0 - 32 + priv->txMBs)) - 1)) {
+ int j, idle_mbs;
- /* after sending a RTR frame MB is in RX mode */
- flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
- ®s->mb[FLEXCAN_TX_BUF_ID].can_ctrl);
- flexcan_write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1);
- netif_wake_queue(dev);
+ for (j = 0, idle_mbs = 0 ; j < priv->txMBs; j++) {
+ int MB_bit = FLEXCAN_TX_MB0 - 32 + j;
+
+ if (reg_iflag2 & (1 << MB_bit) ) {
+ /* after sending a RTR frame MB is in RX mode */
+ flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+ ®s->mb[FLEXCAN_TX_MB0 + j].can_ctrl);
+
+ flexcan_write((1 << MB_bit), ®s->iflag2);
+ set_bit(j, &priv->txMB_pending_set);
+ }
+
+ if (priv->txMB_idle_set & (1 << j))
+ idle_mbs++;
+ }
+
+ if (idle_mbs < priv->txMBs / 2 ||
+ (j = time_after(jiffies, priv->txMB_poll_last + HZ))) {
+
+ if (j) {
+ /* if there was long enough time between last frame sent
+ * and current let flexcan_poll() update tx stats anyway.
+ */
+ set_bit(FLEXCAN_PROCESS_PEND_TXMB, &priv->txMB_pending_set);
+ }
+ priv->txMB_poll_last = jiffies;
+ napi_schedule(&priv->napi);
+ }
}
- return IRQ_HANDLED;
+ return rc;
}
static void flexcan_set_bittiming(struct net_device *dev)
@@ -896,7 +1006,7 @@ static int flexcan_chip_start(struct net_device *dev)
FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
FLEXCAN_MCR_WAK_MSK | FLEXCAN_MCR_SLF_WAK |
- FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
+ FLEXCAN_MCR_MAXMB(FLEXCAN_TX_MB0 + priv->txMBs);
netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
flexcan_write(reg_mcr, ®s->mcr);
@@ -934,18 +1044,18 @@ static int flexcan_chip_start(struct net_device *dev)
flexcan_write(reg_ctrl, ®s->ctrl);
/* clear and invalidate all mailboxes first */
- for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->mb); i++) {
+ /* +Errata ERR005829: mark first TX mailbox as INACTIVE */
+ for (i = 8; i < FLEXCAN_TX_MB0 - 1; i++) {
flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE,
®s->mb[i].can_ctrl);
}
+ for (i = FLEXCAN_TX_MB0 - 1; i < ARRAY_SIZE(regs->mb); i++) {
+ flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+ ®s->mb[i].can_ctrl);
+ }
+ priv->txMB_idle_set = ((u32)1 << priv->txMBs) - 1;
+ priv->txMB_pending_set = 0;
- /* Errata ERR005829: mark first TX mailbox as INACTIVE */
- flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
- ®s->mb[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
-
- /* mark TX mailbox as INACTIVE */
- flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
- ®s->mb[FLEXCAN_TX_BUF_ID].can_ctrl);
/* acceptance mask/acceptance code (accept everything) */
flexcan_write(0x0, ®s->rxgmask);
@@ -989,10 +1099,12 @@ static int flexcan_chip_start(struct net_device *dev)
priv->can.state = CAN_STATE_ERROR_ACTIVE;
+ priv->txMB_poll_last = jiffies;
/* enable interrupts atomically */
disable_irq(dev->irq);
flexcan_write(priv->reg_ctrl_default, ®s->ctrl);
flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1);
+ flexcan_write((((u32)1 << priv->txMBs) - 1) << (FLEXCAN_TX_MB0 - 32), ®s->imask2);
enable_irq(dev->irq);
/* print chip status */
@@ -1023,11 +1135,14 @@ static void flexcan_chip_stop(struct net_device *dev)
/* Disable all interrupts */
flexcan_write(0, ®s->imask1);
+ flexcan_write(0, ®s->imask2);
flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
®s->ctrl);
flexcan_transceiver_disable(priv);
priv->can.state = CAN_STATE_STOPPED;
+
+ flexcan_poll_txmbs(dev, priv->txMBs, 0);
}
static int flexcan_open(struct net_device *dev)
@@ -1047,7 +1162,7 @@ static int flexcan_open(struct net_device *dev)
if (err)
goto out_disable_per;
- err = request_irq(dev->irq, flexcan_irq, IRQF_SHARED, dev->name, dev);
+ err = request_threaded_irq(dev->irq, flexcan_irq, NULL, IRQF_SHARED, dev->name, dev);
if (err)
goto out_close;
@@ -1260,6 +1375,7 @@ static int flexcan_probe(struct platform_device *pdev)
int err, irq;
u32 clock_freq = 0;
int wakeup = 1;
+ int txMBs = 24;
reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER)
@@ -1285,9 +1401,19 @@ static int flexcan_probe(struct platform_device *pdev)
return PTR_ERR(clk_per);
}
- if (!clock_freq)
+ if (!clock_freq) {
clock_freq = clk_get_rate(clk_per);
- else {
+ if (txMBs > 1) {
+ /*
+ * MCR::MAXMB [6:0] will be set to all available message boxes
+ * due to that and section 26.6.9.5 of imx6 rm let's double
+ * default clock (30M)
+ */
+
+ clk_set_rate(clk_per, 60000000);
+ }
+ clock_freq = clk_get_rate(clk_per);
+ } else {
u32 tmp_clk_frq = clock_freq;
clk_set_rate(clk_per, clock_freq);
@@ -1319,7 +1445,7 @@ static int flexcan_probe(struct platform_device *pdev)
return -ENODEV;
}
- dev = alloc_candev(sizeof(struct flexcan_priv), 1);
+ dev = alloc_candev(sizeof(struct flexcan_priv), txMBs);
if (!dev)
return -ENOMEM;
@@ -1341,8 +1467,10 @@ static int flexcan_probe(struct platform_device *pdev)
priv->pdata = dev_get_platdata(&pdev->dev);
priv->devtype_data = devtype_data;
priv->reg_xceiver = reg_xceiver;
+ priv->txMBs = txMBs;
- netif_napi_add(dev, &priv->napi, flexcan_poll, FLEXCAN_NAPI_WEIGHT);
+ netif_napi_add(dev, &priv->napi, flexcan_poll,
+ FLEXCAN_NAPI_WEIGHT + priv->txMBs);
platform_set_drvdata(pdev, dev);
SET_NETDEV_DEV(dev, &pdev->dev);
@@ -1361,7 +1489,6 @@ static int flexcan_probe(struct platform_device *pdev)
wakeup = 0;
dev_dbg(&pdev->dev, "failed to parse stop-mode\n");
}
-
}
device_set_wakeup_capable(&pdev->dev, wakeup);
--
2.11.0
