[PATCH net v8 02/10] can: rockchip_canfd: serialize TX state and command writes
Cunhao Lu via B4 Relay
devnull+1579567540.qq.com at kernel.org
Sun Nov 8 03:33:29 PST 2026
From: Cunhao Lu <1579567540 at qq.com>
rkcanfd_get_effective_tx_free() inspects the tail echo entry while the RX
completion path removes it and advances tx_tail. Concurrent TX submission
can observe inconsistent queue state or dereference an skb already queued
for NAPI, where another CPU can free it. An inconsistent free-space check
can also leave the TX queue stopped without a completion to wake it.
TX submission and RX-side retry also share the MODE/CMD/MODE sequence
used for erratum 12. Interleaving these sequences can restore the default
mode before the other caller writes CMD, bypassing the workaround.
Serialize TX submission with RX completion and retry using tx_lock. Hold
it across the TX space and RX pending checks, echo handling, head/tail
updates, BQL accounting, queue stop/wake and command writes. Require
caller-held locking in the shared helpers and keep TX diagnostics in
the same critical section.
Release the original skb retained by the preceding echo-failure fix after
unlocking and restoring IRQs, so its socket destructor cannot run under
tx_lock. Backports need that retained-reference handling and the CAN skb
ownership fix, or equivalent handling.
This lock covers running TX, completion and retry. Synchronization with
bus-off and controller reinitialization is provided by the later
lifecycle fix.
Fixes: ae002cc32ec4 ("can: rockchip_canfd: prepare to use full TX-FIFO depth")
Fixes: 83f9bd6bf39d ("can: rockchip_canfd: implement workaround for erratum 12")
Cc: stable at vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540 at qq.com>
---
Changes in v8:
- Extend the TX/RX critical sections to cover space and pending checks,
queue updates and diagnostics; require caller-held helper locking and
assert it for command writes.
- Release the retained original skb after unlocking.
---
drivers/net/can/rockchip/rockchip_canfd-core.c | 1 +
drivers/net/can/rockchip/rockchip_canfd-rx.c | 23 ++++++++++++++---------
drivers/net/can/rockchip/rockchip_canfd-tx.c | 26 +++++++++++++++++++++++---
drivers/net/can/rockchip/rockchip_canfd.h | 3 +++
4 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-core.c b/drivers/net/can/rockchip/rockchip_canfd-core.c
index 37c1c22c40c9..1cae86973da5 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-core.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-core.c
@@ -921,6 +921,7 @@ static int rkcanfd_probe(struct platform_device *pdev)
priv->can.do_set_mode = rkcanfd_set_mode;
priv->can.do_get_berr_counter = rkcanfd_get_berr_counter;
priv->ndev = ndev;
+ spin_lock_init(&priv->tx_lock);
match = device_get_match_data(&pdev->dev);
if (match) {
diff --git a/drivers/net/can/rockchip/rockchip_canfd-rx.c b/drivers/net/can/rockchip/rockchip_canfd-rx.c
index 24e87daa1df0..23476a334d6c 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-rx.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-rx.c
@@ -91,6 +91,7 @@ rkcanfd_fifo_header_to_cfd_header(const struct rkcanfd_priv *priv,
return len + cfd->len;
}
+/* Called with tx_lock held. */
static int rkcanfd_rxstx_filter(struct rkcanfd_priv *priv,
const struct canfd_frame *cfd_rx, const u32 ts,
bool *tx_done)
@@ -101,6 +102,8 @@ static int rkcanfd_rxstx_filter(struct rkcanfd_priv *priv,
const struct sk_buff *skb;
unsigned int tx_tail;
+ lockdep_assert_held(&priv->tx_lock);
+
tx_tail = rkcanfd_get_tx_tail(priv);
skb = priv->can.echo_skb[tx_tail];
if (!skb) {
@@ -201,9 +204,11 @@ rkcanfd_fifo_header_empty(const struct rkcanfd_fifo_header *header)
static int rkcanfd_handle_rx_int_one(struct rkcanfd_priv *priv)
{
struct net_device_stats *stats = &priv->ndev->stats;
- struct canfd_frame cfd[1] = { }, *skb_cfd;
struct rkcanfd_fifo_header header[1] = { };
+ struct canfd_frame cfd[1] = { }, *skb_cfd;
+ bool tx_done = false;
struct sk_buff *skb;
+ unsigned long flags;
unsigned int len;
int err;
@@ -237,15 +242,15 @@ static int rkcanfd_handle_rx_int_one(struct rkcanfd_priv *priv)
return 0;
}
- if (rkcanfd_get_tx_pending(priv)) {
- bool tx_done = false;
-
+ spin_lock_irqsave(&priv->tx_lock, flags);
+ err = 0;
+ if (rkcanfd_get_tx_pending(priv))
err = rkcanfd_rxstx_filter(priv, cfd, header->ts, &tx_done);
- if (err)
- return err;
- if (tx_done && !(priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK))
- return 0;
- }
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+ if (err)
+ return err;
+ if (tx_done && !(priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK))
+ return 0;
/* Manual handling of CAN Bus Error counters. See
* rkcanfd_get_corrected_berr_counter() for detailed
diff --git a/drivers/net/can/rockchip/rockchip_canfd-tx.c b/drivers/net/can/rockchip/rockchip_canfd-tx.c
index 420e59b41059..caa71cf8e39a 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-tx.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-tx.c
@@ -14,6 +14,8 @@ static bool rkcanfd_tx_tail_is_eff(const struct rkcanfd_priv *priv)
const struct sk_buff *skb;
unsigned int tx_tail;
+ lockdep_assert_held(&priv->tx_lock);
+
if (!rkcanfd_get_tx_pending(priv))
return false;
@@ -35,6 +37,8 @@ static bool rkcanfd_tx_tail_is_eff(const struct rkcanfd_priv *priv)
unsigned int rkcanfd_get_effective_tx_free(const struct rkcanfd_priv *priv)
{
+ lockdep_assert_held(&priv->tx_lock);
+
if (priv->devtype_data.quirks & RKCANFD_QUIRK_RK3568_ERRATUM_6 &&
rkcanfd_tx_tail_is_eff(priv))
return 0;
@@ -45,6 +49,8 @@ unsigned int rkcanfd_get_effective_tx_free(const struct rkcanfd_priv *priv)
static void rkcanfd_start_xmit_write_cmd(const struct rkcanfd_priv *priv,
const u32 reg_cmd)
{
+ lockdep_assert_held(&priv->tx_lock);
+
if (priv->devtype_data.quirks & RKCANFD_QUIRK_RK3568_ERRATUM_12)
rkcanfd_write(priv, RKCANFD_REG_MODE, priv->reg_mode_default |
RKCANFD_REG_MODE_SPACE_RX_MODE);
@@ -60,6 +66,8 @@ void rkcanfd_xmit_retry(struct rkcanfd_priv *priv)
const unsigned int tx_head = rkcanfd_get_tx_head(priv);
const u32 reg_cmd = RKCANFD_REG_CMD_TX_REQ(tx_head);
+ lockdep_assert_held(&priv->tx_lock);
+
rkcanfd_start_xmit_write_cmd(priv, reg_cmd);
}
@@ -69,12 +77,15 @@ netdev_tx_t rkcanfd_start_xmit(struct sk_buff *skb, struct net_device *ndev)
u32 reg_frameinfo, reg_id, reg_cmd;
unsigned int tx_head, frame_len;
const struct canfd_frame *cfd;
+ unsigned long flags;
int err;
u8 i;
if (can_dev_dropped_skb(ndev, skb))
return NETDEV_TX_OK;
+ spin_lock_irqsave(&priv->tx_lock, flags);
+
if (!netif_subqueue_maybe_stop(priv->ndev, 0,
rkcanfd_get_effective_tx_free(priv),
RKCANFD_TX_STOP_THRESHOLD,
@@ -85,7 +96,7 @@ netdev_tx_t rkcanfd_start_xmit(struct sk_buff *skb, struct net_device *ndev)
priv->tx_head, priv->tx_tail,
rkcanfd_get_tx_pending(priv));
- return NETDEV_TX_BUSY;
+ goto out_busy;
}
cfd = (struct canfd_frame *)skb->data;
@@ -117,8 +128,8 @@ netdev_tx_t rkcanfd_start_xmit(struct sk_buff *skb, struct net_device *ndev)
tx_head = rkcanfd_get_tx_head(priv);
reg_cmd = RKCANFD_REG_CMD_TX_REQ(tx_head);
- /* Retain the original skb as the frame-data source even if the
- * echo skb is removed before submission finishes.
+ /* Keep the original skb alive until IRQs are restored so echo
+ * preparation cannot invoke its socket destructor under tx_lock.
*/
frame_len = can_skb_get_frame_len(skb);
err = can_put_echo_skb(skb_get(skb), ndev, tx_head, frame_len);
@@ -142,15 +153,22 @@ netdev_tx_t rkcanfd_start_xmit(struct sk_buff *skb, struct net_device *ndev)
RKCANFD_TX_STOP_THRESHOLD,
RKCANFD_TX_START_THRESHOLD);
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
dev_consume_skb_any(skb);
return NETDEV_TX_OK;
out_drop:
ndev->stats.tx_dropped++;
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
dev_kfree_skb_any(skb);
return NETDEV_TX_OK;
+
+out_busy:
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+
+ return NETDEV_TX_BUSY;
}
void rkcanfd_handle_tx_done_one(struct rkcanfd_priv *priv, const u32 ts,
@@ -160,6 +178,8 @@ void rkcanfd_handle_tx_done_one(struct rkcanfd_priv *priv, const u32 ts,
unsigned int tx_tail;
struct sk_buff *skb;
+ lockdep_assert_held(&priv->tx_lock);
+
tx_tail = rkcanfd_get_tx_tail(priv);
skb = priv->can.echo_skb[tx_tail];
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index 95bea9bfd8a2..077dd57d42de 100644
--- a/drivers/net/can/rockchip/rockchip_canfd.h
+++ b/drivers/net/can/rockchip/rockchip_canfd.h
@@ -15,6 +15,7 @@
#include <linux/netdevice.h>
#include <linux/reset.h>
#include <linux/skbuff.h>
+#include <linux/spinlock.h>
#include <linux/timecounter.h>
#include <linux/types.h>
#include <linux/u64_stats_sync.h>
@@ -474,6 +475,8 @@ struct rkcanfd_priv {
struct can_rx_offload offload;
struct net_device *ndev;
+ /* Protect running TX/RX echo state, queue updates and TX commands. */
+ spinlock_t tx_lock;
void __iomem *regs;
unsigned int tx_head;
unsigned int tx_tail;
--
2.34.1
More information about the Linux-rockchip
mailing list