* [PATCH net v5 0/6] can: fix bus-off restart and device close races
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
This work started from a real failure on a production robot using a
LubanCat-5IO with an RK3588. RX continued, but the TX queue remained
stopped and subsequent sends returned ENOBUFS. The interface still
reported ERROR-ACTIVE, and the hardware indicated neither a bus-off
condition nor a pending TX request.
While testing the echo-loss fix with fault injection, I discovered a
separate race between bus-off restart and device close. This series fixes
that race; the fix for the original TX stall will follow separately.
Rockchip currently cancels restart_work in close_candev(), after freeing
the IRQ, disabling RX offload and stopping the controller. A concurrent
restart can re-enable the controller and interrupts after IRQ teardown,
and requeue timestamp work that accesses registers after runtime suspend
has disabled the controller clocks.
The CAN core checks netif_running() before scheduling or starting
recovery. A new close_candev_cancel_restart_sync() helper lets Rockchip
wait for any restart already in progress before tearing down resources.
The final close_candev() call retains cancellation and echo cleanup.
Rockchip restart masks the device's interrupts and drains active IRQ
handlers without disabling the shared interrupt line. It also drains
timestamp work before reinitializing the controller and timecounter.
Shared IRQ handling stays suspended until initialization is complete.
Timestamp locking prevents workers from rearming after deactivation and
keeps timestamp conversions consistent with timecounter updates.
The bus-off path drains TX submission and RX-side echo users before
publishing BUS_OFF, protecting the CAN core's echo flush during manual
recovery. Consumed RX FIFO entries are dropped while TX is disabled to
prevent self-transmitted frames from reaching ordinary RX. Startup
serializes carrier restoration, IRQ resumption and queue wake with a new
bus-off. The series also fixes RX offload cleanup when request_irq() fails
during open.
The following issues were also identified during this work:
- Races in the Rockchip bus error counter cache.
- Incomplete runtime PM clock cleanup in Rockchip during removal or
failed registration.
These two issues are outside this series. When I have time,
I will send separate patches to fix them one by one.
Hardware testing used a LubanCat-5IO running a Linux 6.18 PREEMPT_RT
kernel with this series backported. Tests covered 2,700 ranging
request/response exchanges and 630 seconds of continuous traffic,
including three naturally scheduled timestamp worker runs. RX, TX echo
and TX completion hardware timestamps were present throughout, with no
backward or abnormal jumps.
Test-only instrumentation captured 20 close/IRQ interleavings, including
15 with multiple FIFO entries. In every case, consumed self-transmitted
frames were dropped after TX was disabled, the FIFO was drained, and
traffic resumed after reopening.
All 100 classic CAN loopback tests, 20 automatic and 20 manual restarts
using synthetic bus-off injection, and 20 down/up cycles passed. No
timestamp worker ran during about 300 seconds after close, and no
deadlocks or WARN/Oops were observed.
---
Changes in v5:
- Drop consumed RX FIFO entries under tx_lock when TX is disabled
instead of only skipping echo filtering.
- Link to v4:
https://patch.msgid.link/20261015-rockchip-canfd-restart-close-v4-0-ad149ff79450@qq.com
Changes in v4:
- Serialize timecounter_cyc2time() with the worker's timecounter_read()
and startup's timecounter_init() using timestamp_lock.
- Update rkcanfd_skb_set_timestamp() to take a non-const private pointer
and document the expanded scope of timestamp_lock.
- Link to v3:
https://patch.msgid.link/20261014-rockchip-canfd-restart-close-v3-0-f0eb41090242@qq.com
Changes in v3:
- Replace disable_irq()/enable_irq() in Rockchip restart with device
interrupt masking, mask readback, irq_suspended and synchronize_irq().
- Use release/acquire ordering when resuming the IRQ handler.
- Initialize timestamp delayed work once at probe and synchronously
cancel it before restart reinitializes the timecounter.
- Balance RX offload enablement when request_irq() fails during open.
- Prevent timestamp work from reading the counter or rearming after
deactivation, with cancellation outside the lock.
- Drain TX submission and RX-side echo users before publishing BUS_OFF,
protecting the core's pre-restart echo flush.
- Serialize carrier restoration, IRQ enablement and queue wake with a new
bus-off; suspend shared IRQ handling during open until startup finishes.
- Link to v2:
https://lore.kernel.org/linux-can/tencent_82C93561D200C84C24608C80049C23EEF80A@qq.com/
Changes in v2:
- Split the series into the common checks, the cancellation helper, and
its use in Rockchip.
- Check netif_running() after the do_set_mode capability check and before
the restart notification, echo cleanup, and carrier update.
- Only queue automatic recovery for a running interface; keep reporting
bus-off and make the log message match the scheduling decision.
- Add close_candev_cancel_restart_sync() in linux/can/dev.h and use it in
close_candev() without changing the existing cleanup order.
- Replace the Rockchip mutex and closing flag with an early call to the
helper. Drop the extra close-path IRQ synchronization and mask readback.
- Retain IRQ exclusion around hardware reinitialization in the restart
callback.
- Link to v1:
https://lore.kernel.org/linux-can/tencent_5802EE88434010229EDF7A53084D994C6106@qq.com/
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Vincent Mailhol <mailhol@kernel.org>
To: Heiko Stuebner <heiko@sntech.de>
To: "David S. Miller" <davem@davemloft.net>
To: Wolfgang Grandegger <wg@grandegger.com>
To: Oliver Hartkopp <socketcan@hartkopp.net>
To: kernel@pengutronix.de
Cc: linux-can@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-rockchip@lists.infradead.org
---
Cunhao Lu (6):
can: dev: prevent bus-off recovery when the interface is not running
can: dev: add close_candev_cancel_restart_sync()
can: rockchip_canfd: cancel bus-off restart before closing the device
can: rockchip_canfd: prevent timestamp work from rearming after bus-off
can: rockchip_canfd: serialize TX with bus-off and restart
can: rockchip_canfd: serialize timecounter timestamp conversion
drivers/net/can/dev/dev.c | 12 ++--
drivers/net/can/rockchip/rockchip_canfd-core.c | 67 ++++++++++++++++++++--
drivers/net/can/rockchip/rockchip_canfd-rx.c | 26 ++++++---
.../net/can/rockchip/rockchip_canfd-timestamp.c | 43 ++++++++++++--
drivers/net/can/rockchip/rockchip_canfd-tx.c | 16 +++++-
drivers/net/can/rockchip/rockchip_canfd.h | 11 +++-
include/linux/can/dev.h | 12 ++++
7 files changed, 162 insertions(+), 25 deletions(-)
---
base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
change-id: 20261013-rockchip-canfd-restart-close-ebde5954f1bf
Best regards,
--
Cunhao Lu <1579567540@qq.com>
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH net v5 0/6] can: fix bus-off restart and device close races
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
This work started from a real failure on a production robot using a
LubanCat-5IO with an RK3588. RX continued, but the TX queue remained
stopped and subsequent sends returned ENOBUFS. The interface still
reported ERROR-ACTIVE, and the hardware indicated neither a bus-off
condition nor a pending TX request.
While testing the echo-loss fix with fault injection, I discovered a
separate race between bus-off restart and device close. This series fixes
that race; the fix for the original TX stall will follow separately.
Rockchip currently cancels restart_work in close_candev(), after freeing
the IRQ, disabling RX offload and stopping the controller. A concurrent
restart can re-enable the controller and interrupts after IRQ teardown,
and requeue timestamp work that accesses registers after runtime suspend
has disabled the controller clocks.
The CAN core checks netif_running() before scheduling or starting
recovery. A new close_candev_cancel_restart_sync() helper lets Rockchip
wait for any restart already in progress before tearing down resources.
The final close_candev() call retains cancellation and echo cleanup.
Rockchip restart masks the device's interrupts and drains active IRQ
handlers without disabling the shared interrupt line. It also drains
timestamp work before reinitializing the controller and timecounter.
Shared IRQ handling stays suspended until initialization is complete.
Timestamp locking prevents workers from rearming after deactivation and
keeps timestamp conversions consistent with timecounter updates.
The bus-off path drains TX submission and RX-side echo users before
publishing BUS_OFF, protecting the CAN core's echo flush during manual
recovery. Consumed RX FIFO entries are dropped while TX is disabled to
prevent self-transmitted frames from reaching ordinary RX. Startup
serializes carrier restoration, IRQ resumption and queue wake with a new
bus-off. The series also fixes RX offload cleanup when request_irq() fails
during open.
The following issues were also identified during this work:
- Races in the Rockchip bus error counter cache.
- Incomplete runtime PM clock cleanup in Rockchip during removal or
failed registration.
These two issues are outside this series. When I have time,
I will send separate patches to fix them one by one.
Hardware testing used a LubanCat-5IO running a Linux 6.18 PREEMPT_RT
kernel with this series backported. Tests covered 2,700 ranging
request/response exchanges and 630 seconds of continuous traffic,
including three naturally scheduled timestamp worker runs. RX, TX echo
and TX completion hardware timestamps were present throughout, with no
backward or abnormal jumps.
Test-only instrumentation captured 20 close/IRQ interleavings, including
15 with multiple FIFO entries. In every case, consumed self-transmitted
frames were dropped after TX was disabled, the FIFO was drained, and
traffic resumed after reopening.
All 100 classic CAN loopback tests, 20 automatic and 20 manual restarts
using synthetic bus-off injection, and 20 down/up cycles passed. No
timestamp worker ran during about 300 seconds after close, and no
deadlocks or WARN/Oops were observed.
---
Changes in v5:
- Drop consumed RX FIFO entries under tx_lock when TX is disabled
instead of only skipping echo filtering.
- Link to v4:
https://patch.msgid.link/20261015-rockchip-canfd-restart-close-v4-0-ad149ff79450@qq.com
Changes in v4:
- Serialize timecounter_cyc2time() with the worker's timecounter_read()
and startup's timecounter_init() using timestamp_lock.
- Update rkcanfd_skb_set_timestamp() to take a non-const private pointer
and document the expanded scope of timestamp_lock.
- Link to v3:
https://patch.msgid.link/20261014-rockchip-canfd-restart-close-v3-0-f0eb41090242@qq.com
Changes in v3:
- Replace disable_irq()/enable_irq() in Rockchip restart with device
interrupt masking, mask readback, irq_suspended and synchronize_irq().
- Use release/acquire ordering when resuming the IRQ handler.
- Initialize timestamp delayed work once at probe and synchronously
cancel it before restart reinitializes the timecounter.
- Balance RX offload enablement when request_irq() fails during open.
- Prevent timestamp work from reading the counter or rearming after
deactivation, with cancellation outside the lock.
- Drain TX submission and RX-side echo users before publishing BUS_OFF,
protecting the core's pre-restart echo flush.
- Serialize carrier restoration, IRQ enablement and queue wake with a new
bus-off; suspend shared IRQ handling during open until startup finishes.
- Link to v2:
https://lore.kernel.org/linux-can/tencent_82C93561D200C84C24608C80049C23EEF80A@qq.com/
Changes in v2:
- Split the series into the common checks, the cancellation helper, and
its use in Rockchip.
- Check netif_running() after the do_set_mode capability check and before
the restart notification, echo cleanup, and carrier update.
- Only queue automatic recovery for a running interface; keep reporting
bus-off and make the log message match the scheduling decision.
- Add close_candev_cancel_restart_sync() in linux/can/dev.h and use it in
close_candev() without changing the existing cleanup order.
- Replace the Rockchip mutex and closing flag with an early call to the
helper. Drop the extra close-path IRQ synchronization and mask readback.
- Retain IRQ exclusion around hardware reinitialization in the restart
callback.
- Link to v1:
https://lore.kernel.org/linux-can/tencent_5802EE88434010229EDF7A53084D994C6106@qq.com/
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Vincent Mailhol <mailhol@kernel.org>
To: Heiko Stuebner <heiko@sntech.de>
To: "David S. Miller" <davem@davemloft.net>
To: Wolfgang Grandegger <wg@grandegger.com>
To: Oliver Hartkopp <socketcan@hartkopp.net>
To: kernel@pengutronix.de
Cc: linux-can@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-rockchip@lists.infradead.org
---
Cunhao Lu (6):
can: dev: prevent bus-off recovery when the interface is not running
can: dev: add close_candev_cancel_restart_sync()
can: rockchip_canfd: cancel bus-off restart before closing the device
can: rockchip_canfd: prevent timestamp work from rearming after bus-off
can: rockchip_canfd: serialize TX with bus-off and restart
can: rockchip_canfd: serialize timecounter timestamp conversion
drivers/net/can/dev/dev.c | 12 ++--
drivers/net/can/rockchip/rockchip_canfd-core.c | 67 ++++++++++++++++++++--
drivers/net/can/rockchip/rockchip_canfd-rx.c | 26 ++++++---
.../net/can/rockchip/rockchip_canfd-timestamp.c | 43 ++++++++++++--
drivers/net/can/rockchip/rockchip_canfd-tx.c | 16 +++++-
drivers/net/can/rockchip/rockchip_canfd.h | 11 +++-
include/linux/can/dev.h | 12 ++++
7 files changed, 162 insertions(+), 25 deletions(-)
---
base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
change-id: 20261013-rockchip-canfd-restart-close-ebde5954f1bf
Best regards,
--
Cunhao Lu <1579567540@qq.com>
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH net v5 1/6] can: dev: prevent bus-off recovery when the interface is not running
2026-10-15 9:45 ` Cunhao Lu via B4 Relay
(?)
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
-1 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
The networking core clears __LINK_STATE_START before calling ndo_stop(),
but CAN bus-off recovery neither checks this state before queuing work
nor before calling the driver's restart callback. Recovery can therefore
be queued or started after the interface has begun closing, potentially
re-enabling hardware while the driver is tearing it down.
Check netif_running() in can_restart() before flushing echo skbs, sending
the restart notification or enabling the carrier. Keep the existing
unsupported-operation check first. In can_bus_off(), only schedule
recovery for a running interface and make the log message reflect that
decision. Keep reporting bus-off and disabling the carrier.
These checks do not wait for recovery that has already passed the check,
nor serialize the check with work submission. Drivers still need to
cancel restart work before tearing down resources and quiesce its
producers before their final cleanup.
Fixes: 39549eef3587 ("can: CAN Network device driver and Netlink interface")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
drivers/net/can/dev/dev.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 769745e22a3c7..92492c503446e 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -198,6 +198,9 @@ static int can_restart(struct net_device *dev)
if (!priv->do_set_mode)
return -EOPNOTSUPP;
+ if (!netif_running(dev))
+ return -ENETDOWN;
+
if (netif_carrier_ok(dev))
netdev_err(dev, "Attempt to restart for bus-off recovery, but carrier is OK?\n");
@@ -264,8 +267,9 @@ int can_restart_now(struct net_device *dev)
void can_bus_off(struct net_device *dev)
{
struct can_priv *priv = netdev_priv(dev);
+ const bool restart = priv->restart_ms && netif_running(dev);
- if (priv->restart_ms)
+ if (restart)
netdev_info(dev, "bus-off, scheduling restart in %d ms\n",
priv->restart_ms);
else
@@ -273,7 +277,7 @@ void can_bus_off(struct net_device *dev)
netif_carrier_off(dev);
- if (priv->restart_ms)
+ if (restart)
schedule_delayed_work(&priv->restart_work,
msecs_to_jiffies(priv->restart_ms));
}
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 1/6] can: dev: prevent bus-off recovery when the interface is not running
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
The networking core clears __LINK_STATE_START before calling ndo_stop(),
but CAN bus-off recovery neither checks this state before queuing work
nor before calling the driver's restart callback. Recovery can therefore
be queued or started after the interface has begun closing, potentially
re-enabling hardware while the driver is tearing it down.
Check netif_running() in can_restart() before flushing echo skbs, sending
the restart notification or enabling the carrier. Keep the existing
unsupported-operation check first. In can_bus_off(), only schedule
recovery for a running interface and make the log message reflect that
decision. Keep reporting bus-off and disabling the carrier.
These checks do not wait for recovery that has already passed the check,
nor serialize the check with work submission. Drivers still need to
cancel restart work before tearing down resources and quiesce its
producers before their final cleanup.
Fixes: 39549eef3587 ("can: CAN Network device driver and Netlink interface")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
drivers/net/can/dev/dev.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 769745e22a3c7..92492c503446e 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -198,6 +198,9 @@ static int can_restart(struct net_device *dev)
if (!priv->do_set_mode)
return -EOPNOTSUPP;
+ if (!netif_running(dev))
+ return -ENETDOWN;
+
if (netif_carrier_ok(dev))
netdev_err(dev, "Attempt to restart for bus-off recovery, but carrier is OK?\n");
@@ -264,8 +267,9 @@ int can_restart_now(struct net_device *dev)
void can_bus_off(struct net_device *dev)
{
struct can_priv *priv = netdev_priv(dev);
+ const bool restart = priv->restart_ms && netif_running(dev);
- if (priv->restart_ms)
+ if (restart)
netdev_info(dev, "bus-off, scheduling restart in %d ms\n",
priv->restart_ms);
else
@@ -273,7 +277,7 @@ void can_bus_off(struct net_device *dev)
netif_carrier_off(dev);
- if (priv->restart_ms)
+ if (restart)
schedule_delayed_work(&priv->restart_work,
msecs_to_jiffies(priv->restart_ms));
}
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 1/6] can: dev: prevent bus-off recovery when the interface is not running
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
The networking core clears __LINK_STATE_START before calling ndo_stop(),
but CAN bus-off recovery neither checks this state before queuing work
nor before calling the driver's restart callback. Recovery can therefore
be queued or started after the interface has begun closing, potentially
re-enabling hardware while the driver is tearing it down.
Check netif_running() in can_restart() before flushing echo skbs, sending
the restart notification or enabling the carrier. Keep the existing
unsupported-operation check first. In can_bus_off(), only schedule
recovery for a running interface and make the log message reflect that
decision. Keep reporting bus-off and disabling the carrier.
These checks do not wait for recovery that has already passed the check,
nor serialize the check with work submission. Drivers still need to
cancel restart work before tearing down resources and quiesce its
producers before their final cleanup.
Fixes: 39549eef3587 ("can: CAN Network device driver and Netlink interface")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
drivers/net/can/dev/dev.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 769745e22a3c7..92492c503446e 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -198,6 +198,9 @@ static int can_restart(struct net_device *dev)
if (!priv->do_set_mode)
return -EOPNOTSUPP;
+ if (!netif_running(dev))
+ return -ENETDOWN;
+
if (netif_carrier_ok(dev))
netdev_err(dev, "Attempt to restart for bus-off recovery, but carrier is OK?\n");
@@ -264,8 +267,9 @@ int can_restart_now(struct net_device *dev)
void can_bus_off(struct net_device *dev)
{
struct can_priv *priv = netdev_priv(dev);
+ const bool restart = priv->restart_ms && netif_running(dev);
- if (priv->restart_ms)
+ if (restart)
netdev_info(dev, "bus-off, scheduling restart in %d ms\n",
priv->restart_ms);
else
@@ -273,7 +277,7 @@ void can_bus_off(struct net_device *dev)
netif_carrier_off(dev);
- if (priv->restart_ms)
+ if (restart)
schedule_delayed_work(&priv->restart_work,
msecs_to_jiffies(priv->restart_ms));
}
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH net v5 1/6] can: dev: prevent bus-off recovery when the interface is not running
2026-10-15 9:45 ` Cunhao Lu via B4 Relay
(?)
(?)
@ 2026-10-15 9:56 ` sashiko-bot
2026-10-15 10:22 ` Cunhao Lu
-1 siblings, 1 reply; 25+ messages in thread
From: sashiko-bot @ 2026-10-15 9:56 UTC (permalink / raw)
To: Cunhao Lu
Cc: linux-can, Marc Kleine-Budde, Oliver Hartkopp, Vincent Mailhol,
Oleksij Rempel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] can: dev: TOCTOU race in can_restart() allows accessing hardware with disabled clocks
--
commit 4e524e53df8c23d434660cf0e7c4868293520f98
Author: Cunhao Lu <1579567540@qq.com>
can: dev: prevent bus-off recovery when the interface is not running
This commit adds a check for netif_running() in can_restart() and
can_bus_off() to avoid executing bus-off recovery on an interface that
is in the process of closing.
> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
> index 769745e22a3c7..92492c503446e 100644
> --- a/drivers/net/can/dev/dev.c
> +++ b/drivers/net/can/dev/dev.c
> @@ -198,6 +198,9 @@ static int can_restart(struct net_device *dev)
> if (!priv->do_set_mode)
> return -EOPNOTSUPP;
>
> + if (!netif_running(dev))
> + return -ENETDOWN;
> +
[Severity: High]
Can this lockless check create a time-of-check to time-of-use (TOCTOU)
race that results in accessing hardware with disabled clocks?
If a bus-off event schedules the delayed restart work, can_restart()
could check netif_running() and evaluate to true. If the workqueue thread
is preempted immediately after this check, and concurrently the interface
is brought down:
dev_close() clears __LINK_STATE_START and calls ndo_stop().
In drivers like at91_can and rcar_can, ndo_stop() disables hardware
clocks before calling close_candev() to cancel the restart work:
drivers/net/can/at91_can.c:at91_close() {
...
clk_disable_unprepare(priv->clk);
phy_power_off(priv->transceiver);
close_candev(dev);
...
}
Since close_candev() blocks on cancel_delayed_work_sync(), it waits for
can_restart() to finish. When the preempted workqueue thread resumes,
it has already passed the check and will proceed to call
priv->do_set_mode().
Will this attempt to access hardware registers while their backing clocks
are disabled, potentially causing a synchronous external abort or system
panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-rockchip-canfd-restart-close-v5-0-ae77327037c9@qq.com?part=1
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH net v5 1/6] can: dev: prevent bus-off recovery when the interface is not running
2026-10-15 9:56 ` sashiko-bot
@ 2026-10-15 10:22 ` Cunhao Lu
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu @ 2026-10-15 10:22 UTC (permalink / raw)
To: sashiko-reviews
Cc: linux-can, Marc Kleine-Budde, Oliver Hartkopp, Vincent Mailhol,
Oleksij Rempel
Hi,
> [Severity: High]
> Can this lockless check create a time-of-check to time-of-use (TOCTOU)
> race that results in accessing hardware with disabled clocks?
>
> If a bus-off event schedules the delayed restart work, can_restart()
> could check netif_running() and evaluate to true. If the workqueue thread
> is preempted immediately after this check, and concurrently the interface
> is brought down:
>
> dev_close() clears __LINK_STATE_START and calls ndo_stop().
> In drivers like at91_can and rcar_can, ndo_stop() disables hardware
> clocks before calling close_candev() to cancel the restart work:
>
> drivers/net/can/at91_can.c:at91_close() {
> ...
> clk_disable_unprepare(priv->clk);
> phy_power_off(priv->transceiver);
>
> close_candev(dev);
> ...
> }
>
> Since close_candev() blocks on cancel_delayed_work_sync(), it waits for
> can_restart() to finish. When the preempted workqueue thread resumes,
> it has already passed the check and will proceed to call
> priv->do_set_mode().
>
> Will this attempt to access hardware registers while their backing clocks
> are disabled, potentially causing a synchronous external abort or system
> panic?
This is the same missing interrupt-mask readback issue reported in:
https://lore.kernel.org/all/sashiko-outbox-172613@kernel.org/
The sequence of masking the device interrupts and then calling free_irq()
without a readback predates this patch. It is independent of the
restart/close race addressed here and is not introduced by this change.
I will leave this issue out of the current series. When I have time,
I will follow up with a separate patch to fix it.
Regards,
Cunhao Lu
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH net v5 2/6] can: dev: add close_candev_cancel_restart_sync()
2026-10-15 9:45 ` Cunhao Lu via B4 Relay
(?)
@ 2026-10-15 9:45 ` Cunhao Lu
-1 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
Drivers may need to wait for bus-off recovery before tearing down the
IRQ and hardware resources used by their restart callback. Calling
close_candev() at that point also flushes echo skbs, which belongs after
the driver has quiesced the users of those buffers.
Add close_candev_cancel_restart_sync() in linux/can/dev.h so drivers can
cancel the core restart work separately, without accessing can_priv's
work item directly. The helper only cancels restart work; callers must
still stop its producers and call close_candev() for the final cleanup.
Use the helper in close_candev(), preserving its existing cancellation
and echo cleanup order.
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
drivers/net/can/dev/dev.c | 4 +---
include/linux/can/dev.h | 12 ++++++++++++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 92492c503446e..52e8c4854ae0d 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -534,9 +534,7 @@ EXPORT_SYMBOL_GPL(of_can_transceiver);
*/
void close_candev(struct net_device *dev)
{
- struct can_priv *priv = netdev_priv(dev);
-
- cancel_delayed_work_sync(&priv->restart_work);
+ close_candev_cancel_restart_sync(dev);
can_flush_echo_skb(dev);
}
EXPORT_SYMBOL_GPL(close_candev);
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 6d0710d6f571d..a277d12332f15 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -22,6 +22,7 @@
#include <linux/can/skb.h>
#include <linux/ethtool.h>
#include <linux/netdevice.h>
+#include <linux/workqueue.h>
/*
* CAN mode
@@ -115,6 +116,17 @@ struct can_priv *safe_candev_priv(struct net_device *dev);
int open_candev(struct net_device *dev);
void close_candev(struct net_device *dev);
+
+/* Cancel bus-off restart before tearing down resources used by the restart
+ * callback. The driver must still call close_candev() for the final cleanup.
+ */
+static inline void close_candev_cancel_restart_sync(struct net_device *dev)
+{
+ struct can_priv *priv = netdev_priv(dev);
+
+ cancel_delayed_work_sync(&priv->restart_work);
+}
+
void can_set_default_mtu(struct net_device *dev);
void can_set_cap_info(struct net_device *dev);
int __must_check can_set_static_ctrlmode(struct net_device *dev,
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 2/6] can: dev: add close_candev_cancel_restart_sync()
@ 2026-10-15 9:45 ` Cunhao Lu
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
Drivers may need to wait for bus-off recovery before tearing down the
IRQ and hardware resources used by their restart callback. Calling
close_candev() at that point also flushes echo skbs, which belongs after
the driver has quiesced the users of those buffers.
Add close_candev_cancel_restart_sync() in linux/can/dev.h so drivers can
cancel the core restart work separately, without accessing can_priv's
work item directly. The helper only cancels restart work; callers must
still stop its producers and call close_candev() for the final cleanup.
Use the helper in close_candev(), preserving its existing cancellation
and echo cleanup order.
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
drivers/net/can/dev/dev.c | 4 +---
include/linux/can/dev.h | 12 ++++++++++++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 92492c503446e..52e8c4854ae0d 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -534,9 +534,7 @@ EXPORT_SYMBOL_GPL(of_can_transceiver);
*/
void close_candev(struct net_device *dev)
{
- struct can_priv *priv = netdev_priv(dev);
-
- cancel_delayed_work_sync(&priv->restart_work);
+ close_candev_cancel_restart_sync(dev);
can_flush_echo_skb(dev);
}
EXPORT_SYMBOL_GPL(close_candev);
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 6d0710d6f571d..a277d12332f15 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -22,6 +22,7 @@
#include <linux/can/skb.h>
#include <linux/ethtool.h>
#include <linux/netdevice.h>
+#include <linux/workqueue.h>
/*
* CAN mode
@@ -115,6 +116,17 @@ struct can_priv *safe_candev_priv(struct net_device *dev);
int open_candev(struct net_device *dev);
void close_candev(struct net_device *dev);
+
+/* Cancel bus-off restart before tearing down resources used by the restart
+ * callback. The driver must still call close_candev() for the final cleanup.
+ */
+static inline void close_candev_cancel_restart_sync(struct net_device *dev)
+{
+ struct can_priv *priv = netdev_priv(dev);
+
+ cancel_delayed_work_sync(&priv->restart_work);
+}
+
void can_set_default_mtu(struct net_device *dev);
void can_set_cap_info(struct net_device *dev);
int __must_check can_set_static_ctrlmode(struct net_device *dev,
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 2/6] can: dev: add close_candev_cancel_restart_sync()
@ 2026-10-15 9:45 ` Cunhao Lu
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
Drivers may need to wait for bus-off recovery before tearing down the
IRQ and hardware resources used by their restart callback. Calling
close_candev() at that point also flushes echo skbs, which belongs after
the driver has quiesced the users of those buffers.
Add close_candev_cancel_restart_sync() in linux/can/dev.h so drivers can
cancel the core restart work separately, without accessing can_priv's
work item directly. The helper only cancels restart work; callers must
still stop its producers and call close_candev() for the final cleanup.
Use the helper in close_candev(), preserving its existing cancellation
and echo cleanup order.
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
drivers/net/can/dev/dev.c | 4 +---
include/linux/can/dev.h | 12 ++++++++++++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 92492c503446e..52e8c4854ae0d 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -534,9 +534,7 @@ EXPORT_SYMBOL_GPL(of_can_transceiver);
*/
void close_candev(struct net_device *dev)
{
- struct can_priv *priv = netdev_priv(dev);
-
- cancel_delayed_work_sync(&priv->restart_work);
+ close_candev_cancel_restart_sync(dev);
can_flush_echo_skb(dev);
}
EXPORT_SYMBOL_GPL(close_candev);
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 6d0710d6f571d..a277d12332f15 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -22,6 +22,7 @@
#include <linux/can/skb.h>
#include <linux/ethtool.h>
#include <linux/netdevice.h>
+#include <linux/workqueue.h>
/*
* CAN mode
@@ -115,6 +116,17 @@ struct can_priv *safe_candev_priv(struct net_device *dev);
int open_candev(struct net_device *dev);
void close_candev(struct net_device *dev);
+
+/* Cancel bus-off restart before tearing down resources used by the restart
+ * callback. The driver must still call close_candev() for the final cleanup.
+ */
+static inline void close_candev_cancel_restart_sync(struct net_device *dev)
+{
+ struct can_priv *priv = netdev_priv(dev);
+
+ cancel_delayed_work_sync(&priv->restart_work);
+}
+
void can_set_default_mtu(struct net_device *dev);
void can_set_cap_info(struct net_device *dev);
int __must_check can_set_static_ctrlmode(struct net_device *dev,
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH net v5 3/6] can: rockchip_canfd: cancel bus-off restart before closing the device
2026-10-15 9:45 ` Cunhao Lu via B4 Relay
(?)
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
-1 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
The close path cancels restart_work in close_candev() only after freeing
the IRQ, disabling RX offload and stopping the controller. A concurrent
restart can re-enable the controller and interrupts after IRQ teardown,
or requeue timestamp work that accesses registers after runtime suspend
has disabled the controller clocks.
Call close_candev_cancel_restart_sync() at the start of ndo_stop(), while
the resources needed by a running restart are still available. Keep
close_candev() after IRQ teardown for final cancellation and echo cleanup.
During restart, mask the device's interrupts and drain active handlers
without disabling the shared IRQ line. Suspend handler access to the
controller until reinitialization is complete, and publish the initialized
state before resuming IRQ handling.
Drain timestamp work before reinitializing the timecounter, and initialize
the delayed work once at probe. Keep asynchronous cancellation in the
bus-off path. Also disable RX offload if request_irq() fails during open.
Fixes: ff60bfbaf67f ("can: rockchip_canfd: add driver for Rockchip CAN-FD controller")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
Changes in v3:
- Replace disable_irq()/enable_irq() with device interrupt masking,
mask readback, an irq_suspended flag and synchronize_irq().
- Pair release/acquire accesses to publish reinitialized state before
the IRQ handler resumes, without disabling other shared IRQ users.
- Initialize timestamp delayed work once at probe and synchronously
cancel it before reinitializing the timestamp state during restart.
- Disable RX offload when request_irq() fails during open.
---
drivers/net/can/rockchip/rockchip_canfd-core.c | 20 ++++++++++++++++++++
drivers/net/can/rockchip/rockchip_canfd-timestamp.c | 6 +++++-
drivers/net/can/rockchip/rockchip_canfd.h | 2 ++
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-core.c b/drivers/net/can/rockchip/rockchip_canfd-core.c
index 37c1c22c40c9f..4b3db07242395 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-core.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-core.c
@@ -20,6 +20,7 @@
#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/smp.h>
#include <linux/string.h>
#include "rockchip_canfd.h"
@@ -356,7 +357,18 @@ static int rkcanfd_set_mode(struct net_device *ndev,
switch (mode) {
case CAN_MODE_START:
+ rkcanfd_chip_interrupts_disable(priv);
+ /* Flush the interrupt mask write before suspending the handler. */
+ rkcanfd_read(priv, RKCANFD_REG_INT_MASK);
+ /* Keep shared IRQs from accessing the device during restart. */
+ WRITE_ONCE(priv->irq_suspended, true);
+ /* Wait for handlers that already passed the suspended check. */
+ synchronize_irq(ndev->irq);
+ rkcanfd_timestamp_stop_sync(priv);
+
rkcanfd_chip_start(priv);
+ /* Publish the initialized state before resuming IRQ handling. */
+ smp_store_release(&priv->irq_suspended, false);
rkcanfd_chip_interrupts_enable(priv);
netif_wake_queue(ndev);
break;
@@ -666,6 +678,10 @@ static irqreturn_t rkcanfd_irq(int irq, void *dev_id)
struct rkcanfd_priv *priv = dev_id;
u32 reg_int_unmasked, reg_int;
+ /* Pairs with the release after restart has initialized the device. */
+ if (smp_load_acquire(&priv->irq_suspended))
+ return IRQ_NONE;
+
reg_int_unmasked = rkcanfd_read(priv, RKCANFD_REG_INT);
reg_int = reg_int_unmasked & ~priv->reg_int_mask_default;
@@ -747,6 +763,7 @@ static int rkcanfd_open(struct net_device *ndev)
return 0;
out_rkcanfd_chip_stop:
+ can_rx_offload_disable(&priv->offload);
rkcanfd_chip_stop_sync(priv, CAN_STATE_STOPPED);
pm_runtime_put(ndev->dev.parent);
out_close_candev:
@@ -758,6 +775,7 @@ static int rkcanfd_stop(struct net_device *ndev)
{
struct rkcanfd_priv *priv = netdev_priv(ndev);
+ close_candev_cancel_restart_sync(ndev);
netif_stop_queue(ndev);
rkcanfd_chip_interrupts_disable(priv);
@@ -934,6 +952,8 @@ static int rkcanfd_probe(struct platform_device *pdev)
if (err)
goto out_free_candev;
+ rkcanfd_timestamp_work_init(priv);
+
err = rkcanfd_register(priv);
if (err)
goto out_can_rx_offload_del;
diff --git a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
index 72774cd2f94b7..ad582afbbd682 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
@@ -37,6 +37,11 @@ static void rkcanfd_timestamp_work(struct work_struct *work)
schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
}
+void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv)
+{
+ INIT_DELAYED_WORK(&priv->timestamp, rkcanfd_timestamp_work);
+}
+
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
{
const struct can_bittiming *dbt = &priv->can.fd.data_bittiming;
@@ -73,7 +78,6 @@ void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
max_cycles = min(max_cycles, cc->mask);
work_delay_ns = clocksource_cyc2ns(max_cycles, cc->mult, cc->shift);
priv->work_delay_jiffies = div_u64(work_delay_ns, 3u * NSEC_PER_SEC / HZ);
- INIT_DELAYED_WORK(&priv->timestamp, rkcanfd_timestamp_work);
netdev_dbg(priv->ndev, "clock=%lu.%02luMHz bitrate=%lu.%02luMBit/s div=%u rate=%lu.%02luMHz mult=%u shift=%u delay=%lus\n",
priv->can.clock.freq / MEGA,
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index 95bea9bfd8a2f..e1debab54534e 100644
--- a/drivers/net/can/rockchip/rockchip_canfd.h
+++ b/drivers/net/can/rockchip/rockchip_canfd.h
@@ -480,6 +480,7 @@ struct rkcanfd_priv {
u32 reg_mode_default;
u32 reg_int_mask_default;
+ bool irq_suspended;
struct rkcanfd_devtype_data devtype_data;
struct cyclecounter cc;
@@ -551,6 +552,7 @@ int rkcanfd_handle_rx_int(struct rkcanfd_priv *priv);
void rkcanfd_skb_set_timestamp(const struct rkcanfd_priv *priv,
struct sk_buff *skb, const u32 timestamp);
+void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_start(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_stop(struct rkcanfd_priv *priv);
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 3/6] can: rockchip_canfd: cancel bus-off restart before closing the device
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
The close path cancels restart_work in close_candev() only after freeing
the IRQ, disabling RX offload and stopping the controller. A concurrent
restart can re-enable the controller and interrupts after IRQ teardown,
or requeue timestamp work that accesses registers after runtime suspend
has disabled the controller clocks.
Call close_candev_cancel_restart_sync() at the start of ndo_stop(), while
the resources needed by a running restart are still available. Keep
close_candev() after IRQ teardown for final cancellation and echo cleanup.
During restart, mask the device's interrupts and drain active handlers
without disabling the shared IRQ line. Suspend handler access to the
controller until reinitialization is complete, and publish the initialized
state before resuming IRQ handling.
Drain timestamp work before reinitializing the timecounter, and initialize
the delayed work once at probe. Keep asynchronous cancellation in the
bus-off path. Also disable RX offload if request_irq() fails during open.
Fixes: ff60bfbaf67f ("can: rockchip_canfd: add driver for Rockchip CAN-FD controller")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
Changes in v3:
- Replace disable_irq()/enable_irq() with device interrupt masking,
mask readback, an irq_suspended flag and synchronize_irq().
- Pair release/acquire accesses to publish reinitialized state before
the IRQ handler resumes, without disabling other shared IRQ users.
- Initialize timestamp delayed work once at probe and synchronously
cancel it before reinitializing the timestamp state during restart.
- Disable RX offload when request_irq() fails during open.
---
drivers/net/can/rockchip/rockchip_canfd-core.c | 20 ++++++++++++++++++++
drivers/net/can/rockchip/rockchip_canfd-timestamp.c | 6 +++++-
drivers/net/can/rockchip/rockchip_canfd.h | 2 ++
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-core.c b/drivers/net/can/rockchip/rockchip_canfd-core.c
index 37c1c22c40c9f..4b3db07242395 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-core.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-core.c
@@ -20,6 +20,7 @@
#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/smp.h>
#include <linux/string.h>
#include "rockchip_canfd.h"
@@ -356,7 +357,18 @@ static int rkcanfd_set_mode(struct net_device *ndev,
switch (mode) {
case CAN_MODE_START:
+ rkcanfd_chip_interrupts_disable(priv);
+ /* Flush the interrupt mask write before suspending the handler. */
+ rkcanfd_read(priv, RKCANFD_REG_INT_MASK);
+ /* Keep shared IRQs from accessing the device during restart. */
+ WRITE_ONCE(priv->irq_suspended, true);
+ /* Wait for handlers that already passed the suspended check. */
+ synchronize_irq(ndev->irq);
+ rkcanfd_timestamp_stop_sync(priv);
+
rkcanfd_chip_start(priv);
+ /* Publish the initialized state before resuming IRQ handling. */
+ smp_store_release(&priv->irq_suspended, false);
rkcanfd_chip_interrupts_enable(priv);
netif_wake_queue(ndev);
break;
@@ -666,6 +678,10 @@ static irqreturn_t rkcanfd_irq(int irq, void *dev_id)
struct rkcanfd_priv *priv = dev_id;
u32 reg_int_unmasked, reg_int;
+ /* Pairs with the release after restart has initialized the device. */
+ if (smp_load_acquire(&priv->irq_suspended))
+ return IRQ_NONE;
+
reg_int_unmasked = rkcanfd_read(priv, RKCANFD_REG_INT);
reg_int = reg_int_unmasked & ~priv->reg_int_mask_default;
@@ -747,6 +763,7 @@ static int rkcanfd_open(struct net_device *ndev)
return 0;
out_rkcanfd_chip_stop:
+ can_rx_offload_disable(&priv->offload);
rkcanfd_chip_stop_sync(priv, CAN_STATE_STOPPED);
pm_runtime_put(ndev->dev.parent);
out_close_candev:
@@ -758,6 +775,7 @@ static int rkcanfd_stop(struct net_device *ndev)
{
struct rkcanfd_priv *priv = netdev_priv(ndev);
+ close_candev_cancel_restart_sync(ndev);
netif_stop_queue(ndev);
rkcanfd_chip_interrupts_disable(priv);
@@ -934,6 +952,8 @@ static int rkcanfd_probe(struct platform_device *pdev)
if (err)
goto out_free_candev;
+ rkcanfd_timestamp_work_init(priv);
+
err = rkcanfd_register(priv);
if (err)
goto out_can_rx_offload_del;
diff --git a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
index 72774cd2f94b7..ad582afbbd682 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
@@ -37,6 +37,11 @@ static void rkcanfd_timestamp_work(struct work_struct *work)
schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
}
+void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv)
+{
+ INIT_DELAYED_WORK(&priv->timestamp, rkcanfd_timestamp_work);
+}
+
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
{
const struct can_bittiming *dbt = &priv->can.fd.data_bittiming;
@@ -73,7 +78,6 @@ void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
max_cycles = min(max_cycles, cc->mask);
work_delay_ns = clocksource_cyc2ns(max_cycles, cc->mult, cc->shift);
priv->work_delay_jiffies = div_u64(work_delay_ns, 3u * NSEC_PER_SEC / HZ);
- INIT_DELAYED_WORK(&priv->timestamp, rkcanfd_timestamp_work);
netdev_dbg(priv->ndev, "clock=%lu.%02luMHz bitrate=%lu.%02luMBit/s div=%u rate=%lu.%02luMHz mult=%u shift=%u delay=%lus\n",
priv->can.clock.freq / MEGA,
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index 95bea9bfd8a2f..e1debab54534e 100644
--- a/drivers/net/can/rockchip/rockchip_canfd.h
+++ b/drivers/net/can/rockchip/rockchip_canfd.h
@@ -480,6 +480,7 @@ struct rkcanfd_priv {
u32 reg_mode_default;
u32 reg_int_mask_default;
+ bool irq_suspended;
struct rkcanfd_devtype_data devtype_data;
struct cyclecounter cc;
@@ -551,6 +552,7 @@ int rkcanfd_handle_rx_int(struct rkcanfd_priv *priv);
void rkcanfd_skb_set_timestamp(const struct rkcanfd_priv *priv,
struct sk_buff *skb, const u32 timestamp);
+void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_start(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_stop(struct rkcanfd_priv *priv);
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 3/6] can: rockchip_canfd: cancel bus-off restart before closing the device
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
The close path cancels restart_work in close_candev() only after freeing
the IRQ, disabling RX offload and stopping the controller. A concurrent
restart can re-enable the controller and interrupts after IRQ teardown,
or requeue timestamp work that accesses registers after runtime suspend
has disabled the controller clocks.
Call close_candev_cancel_restart_sync() at the start of ndo_stop(), while
the resources needed by a running restart are still available. Keep
close_candev() after IRQ teardown for final cancellation and echo cleanup.
During restart, mask the device's interrupts and drain active handlers
without disabling the shared IRQ line. Suspend handler access to the
controller until reinitialization is complete, and publish the initialized
state before resuming IRQ handling.
Drain timestamp work before reinitializing the timecounter, and initialize
the delayed work once at probe. Keep asynchronous cancellation in the
bus-off path. Also disable RX offload if request_irq() fails during open.
Fixes: ff60bfbaf67f ("can: rockchip_canfd: add driver for Rockchip CAN-FD controller")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
Changes in v3:
- Replace disable_irq()/enable_irq() with device interrupt masking,
mask readback, an irq_suspended flag and synchronize_irq().
- Pair release/acquire accesses to publish reinitialized state before
the IRQ handler resumes, without disabling other shared IRQ users.
- Initialize timestamp delayed work once at probe and synchronously
cancel it before reinitializing the timestamp state during restart.
- Disable RX offload when request_irq() fails during open.
---
drivers/net/can/rockchip/rockchip_canfd-core.c | 20 ++++++++++++++++++++
drivers/net/can/rockchip/rockchip_canfd-timestamp.c | 6 +++++-
drivers/net/can/rockchip/rockchip_canfd.h | 2 ++
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-core.c b/drivers/net/can/rockchip/rockchip_canfd-core.c
index 37c1c22c40c9f..4b3db07242395 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-core.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-core.c
@@ -20,6 +20,7 @@
#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/smp.h>
#include <linux/string.h>
#include "rockchip_canfd.h"
@@ -356,7 +357,18 @@ static int rkcanfd_set_mode(struct net_device *ndev,
switch (mode) {
case CAN_MODE_START:
+ rkcanfd_chip_interrupts_disable(priv);
+ /* Flush the interrupt mask write before suspending the handler. */
+ rkcanfd_read(priv, RKCANFD_REG_INT_MASK);
+ /* Keep shared IRQs from accessing the device during restart. */
+ WRITE_ONCE(priv->irq_suspended, true);
+ /* Wait for handlers that already passed the suspended check. */
+ synchronize_irq(ndev->irq);
+ rkcanfd_timestamp_stop_sync(priv);
+
rkcanfd_chip_start(priv);
+ /* Publish the initialized state before resuming IRQ handling. */
+ smp_store_release(&priv->irq_suspended, false);
rkcanfd_chip_interrupts_enable(priv);
netif_wake_queue(ndev);
break;
@@ -666,6 +678,10 @@ static irqreturn_t rkcanfd_irq(int irq, void *dev_id)
struct rkcanfd_priv *priv = dev_id;
u32 reg_int_unmasked, reg_int;
+ /* Pairs with the release after restart has initialized the device. */
+ if (smp_load_acquire(&priv->irq_suspended))
+ return IRQ_NONE;
+
reg_int_unmasked = rkcanfd_read(priv, RKCANFD_REG_INT);
reg_int = reg_int_unmasked & ~priv->reg_int_mask_default;
@@ -747,6 +763,7 @@ static int rkcanfd_open(struct net_device *ndev)
return 0;
out_rkcanfd_chip_stop:
+ can_rx_offload_disable(&priv->offload);
rkcanfd_chip_stop_sync(priv, CAN_STATE_STOPPED);
pm_runtime_put(ndev->dev.parent);
out_close_candev:
@@ -758,6 +775,7 @@ static int rkcanfd_stop(struct net_device *ndev)
{
struct rkcanfd_priv *priv = netdev_priv(ndev);
+ close_candev_cancel_restart_sync(ndev);
netif_stop_queue(ndev);
rkcanfd_chip_interrupts_disable(priv);
@@ -934,6 +952,8 @@ static int rkcanfd_probe(struct platform_device *pdev)
if (err)
goto out_free_candev;
+ rkcanfd_timestamp_work_init(priv);
+
err = rkcanfd_register(priv);
if (err)
goto out_can_rx_offload_del;
diff --git a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
index 72774cd2f94b7..ad582afbbd682 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
@@ -37,6 +37,11 @@ static void rkcanfd_timestamp_work(struct work_struct *work)
schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
}
+void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv)
+{
+ INIT_DELAYED_WORK(&priv->timestamp, rkcanfd_timestamp_work);
+}
+
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
{
const struct can_bittiming *dbt = &priv->can.fd.data_bittiming;
@@ -73,7 +78,6 @@ void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
max_cycles = min(max_cycles, cc->mask);
work_delay_ns = clocksource_cyc2ns(max_cycles, cc->mult, cc->shift);
priv->work_delay_jiffies = div_u64(work_delay_ns, 3u * NSEC_PER_SEC / HZ);
- INIT_DELAYED_WORK(&priv->timestamp, rkcanfd_timestamp_work);
netdev_dbg(priv->ndev, "clock=%lu.%02luMHz bitrate=%lu.%02luMBit/s div=%u rate=%lu.%02luMHz mult=%u shift=%u delay=%lus\n",
priv->can.clock.freq / MEGA,
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index 95bea9bfd8a2f..e1debab54534e 100644
--- a/drivers/net/can/rockchip/rockchip_canfd.h
+++ b/drivers/net/can/rockchip/rockchip_canfd.h
@@ -480,6 +480,7 @@ struct rkcanfd_priv {
u32 reg_mode_default;
u32 reg_int_mask_default;
+ bool irq_suspended;
struct rkcanfd_devtype_data devtype_data;
struct cyclecounter cc;
@@ -551,6 +552,7 @@ int rkcanfd_handle_rx_int(struct rkcanfd_priv *priv);
void rkcanfd_skb_set_timestamp(const struct rkcanfd_priv *priv,
struct sk_buff *skb, const u32 timestamp);
+void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_start(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_stop(struct rkcanfd_priv *priv);
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH net v5 3/6] can: rockchip_canfd: cancel bus-off restart before closing the device
2026-10-15 9:45 ` Cunhao Lu via B4 Relay
(?)
(?)
@ 2026-10-15 9:57 ` sashiko-bot
2026-10-15 10:13 ` Cunhao Lu
-1 siblings, 1 reply; 25+ messages in thread
From: sashiko-bot @ 2026-10-15 9:57 UTC (permalink / raw)
To: Cunhao Lu
Cc: linux-can, Marc Kleine-Budde, Oliver Hartkopp, Vincent Mailhol,
Oleksij Rempel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] can: rockchip_canfd: missing mask flush before free_irq() risks storm
--
commit 761946c971cc50dc5c7a42016ab27b61a872001e
Author: Cunhao Lu <1579567540@qq.com>
can: rockchip_canfd: cancel bus-off restart before closing the device
This patch fixes issues in the driver's close and restart paths by reordering
operations in ndo_stop(). It synchronously cancels restart work earlier, masks
device interrupts, and synchronizes IRQs without disabling the shared IRQ line.
> diff --git a/drivers/net/can/rockchip/rockchip_canfd-core.c b/drivers/net/can/rockchip/rockchip_canfd-core.c
> index 37c1c22c40c9f..4b3db07242395 100644
> --- a/drivers/net/can/rockchip/rockchip_canfd-core.c
> +++ b/drivers/net/can/rockchip/rockchip_canfd-core.c
[ ... ]
> @@ -356,7 +357,18 @@ static int rkcanfd_set_mode(struct net_device *ndev,
>
> switch (mode) {
> case CAN_MODE_START:
> + rkcanfd_chip_interrupts_disable(priv);
> + /* Flush the interrupt mask write before suspending the handler. */
> + rkcanfd_read(priv, RKCANFD_REG_INT_MASK);
> + /* Keep shared IRQs from accessing the device during restart. */
> + WRITE_ONCE(priv->irq_suspended, true);
> + /* Wait for handlers that already passed the suspended check. */
> + synchronize_irq(ndev->irq);
> + rkcanfd_timestamp_stop_sync(priv);
> +
> rkcanfd_chip_start(priv);
[ ... ]
> @@ -758,6 +775,7 @@ static int rkcanfd_stop(struct net_device *ndev)
> {
> struct rkcanfd_priv *priv = netdev_priv(ndev);
>
> + close_candev_cancel_restart_sync(ndev);
> netif_stop_queue(ndev);
>
> rkcanfd_chip_interrupts_disable(priv);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Does rkcanfd_stop() also need a readback flush after disabling hardware
interrupts, similar to the one added above in rkcanfd_set_mode()?
Without a readback flush before the subsequent free_irq() call, if the
shared IRQ line receives an interrupt before the posted write reaches
the bus, the CPU could receive the interrupt after the handler has been
deregistered. Could this cause an unhandled interrupt storm that disables
the shared IRQ line entirely?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-rockchip-canfd-restart-close-v5-0-ae77327037c9@qq.com?part=3
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH net v5 3/6] can: rockchip_canfd: cancel bus-off restart before closing the device
2026-10-15 9:57 ` sashiko-bot
@ 2026-10-15 10:13 ` Cunhao Lu
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu @ 2026-10-15 10:13 UTC (permalink / raw)
To: sashiko-reviews
Cc: linux-can, Marc Kleine-Budde, Oliver Hartkopp, Vincent Mailhol,
Oleksij Rempel
Hi,
> [Severity: High]
> Does rkcanfd_stop() also need a readback flush after disabling hardware
> interrupts, similar to the one added above in rkcanfd_set_mode()?
>
> Without a readback flush before the subsequent free_irq() call, if the
> shared IRQ line receives an interrupt before the posted write reaches
> the bus, the CPU could receive the interrupt after the handler has been
> deregistered. Could this cause an unhandled interrupt storm that disables
> the shared IRQ line entirely?
The missing readback in rkcanfd_stop() is a pre-existing issue,
independent of the restart/close race addressed by this patch. The
interrupt-mask write followed by free_irq(), without a readback, has
been present since ff60bfbaf67f ("can: rockchip_canfd: add driver for
Rockchip CAN-FD controller"). This patch does not change that sequence.
I will leave this issue out of the current series. When I have time,
I will follow up with a separate patch to fix it.
Regards,
Cunhao Lu
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH net v5 4/6] can: rockchip_canfd: prevent timestamp work from rearming after bus-off
2026-10-15 9:45 ` Cunhao Lu via B4 Relay
(?)
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
-1 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
The bus-off IRQ cancels timestamp work asynchronously. If the worker
is already running, it can requeue itself after cancellation and keep
polling while the controller remains bus-off.
Add a dedicated lock and an active flag, initialized once at probe.
Serialize the worker's active check, counter read and self-requeue with
start and stop. Clear the flag under the lock before cancelling work
so no worker can read the counter or rearm after deactivation. Enable
the flag and schedule the initial work under the same lock at start.
Keep both cancellation calls outside the lock so synchronous stop can
wait for the worker to finish.
Fixes: 4e1a18bab124 ("can: rockchip_canfd: add hardware timestamping support")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
.../net/can/rockchip/rockchip_canfd-timestamp.c | 34 +++++++++++++++++++---
drivers/net/can/rockchip/rockchip_canfd.h | 4 +++
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
index ad582afbbd68..0fa791849c95 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
@@ -30,18 +30,27 @@ static void rkcanfd_timestamp_work(struct work_struct *work)
{
const struct delayed_work *delayed_work = to_delayed_work(work);
struct rkcanfd_priv *priv;
+ unsigned long flags;
priv = container_of(delayed_work, struct rkcanfd_priv, timestamp);
- timecounter_read(&priv->tc);
-
- schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ if (priv->timestamp_active) {
+ timecounter_read(&priv->tc);
+
+ /* Serialize requeueing with timestamp_stop(). */
+ schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
+ }
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
}
void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv)
{
+ spin_lock_init(&priv->timestamp_lock);
+ priv->timestamp_active = false;
INIT_DELAYED_WORK(&priv->timestamp, rkcanfd_timestamp_work);
}
+/* The caller must quiesce IRQs and timestamp work before reconfiguration. */
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
{
const struct can_bittiming *dbt = &priv->can.fd.data_bittiming;
@@ -93,17 +102,34 @@ void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
void rkcanfd_timestamp_start(struct rkcanfd_priv *priv)
{
- timecounter_init(&priv->tc, &priv->cc, ktime_get_real_ns());
+ unsigned long flags;
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ timecounter_init(&priv->tc, &priv->cc, ktime_get_real_ns());
+ priv->timestamp_active = true;
schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
}
void rkcanfd_timestamp_stop(struct rkcanfd_priv *priv)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ priv->timestamp_active = false;
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
+
cancel_delayed_work(&priv->timestamp);
}
void rkcanfd_timestamp_stop_sync(struct rkcanfd_priv *priv)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ priv->timestamp_active = false;
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
+
+ /* The worker needs timestamp_lock to finish. */
cancel_delayed_work_sync(&priv->timestamp);
}
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index e1debab54534..2743c96ed42f 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>
@@ -485,8 +486,11 @@ struct rkcanfd_priv {
struct cyclecounter cc;
struct timecounter tc;
+ /* Protects timestamp_active and worker reads/requeueing. */
+ spinlock_t timestamp_lock;
struct delayed_work timestamp;
unsigned long work_delay_jiffies;
+ bool timestamp_active;
struct can_berr_counter bec;
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 4/6] can: rockchip_canfd: prevent timestamp work from rearming after bus-off
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
The bus-off IRQ cancels timestamp work asynchronously. If the worker
is already running, it can requeue itself after cancellation and keep
polling while the controller remains bus-off.
Add a dedicated lock and an active flag, initialized once at probe.
Serialize the worker's active check, counter read and self-requeue with
start and stop. Clear the flag under the lock before cancelling work
so no worker can read the counter or rearm after deactivation. Enable
the flag and schedule the initial work under the same lock at start.
Keep both cancellation calls outside the lock so synchronous stop can
wait for the worker to finish.
Fixes: 4e1a18bab124 ("can: rockchip_canfd: add hardware timestamping support")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
.../net/can/rockchip/rockchip_canfd-timestamp.c | 34 +++++++++++++++++++---
drivers/net/can/rockchip/rockchip_canfd.h | 4 +++
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
index ad582afbbd68..0fa791849c95 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
@@ -30,18 +30,27 @@ static void rkcanfd_timestamp_work(struct work_struct *work)
{
const struct delayed_work *delayed_work = to_delayed_work(work);
struct rkcanfd_priv *priv;
+ unsigned long flags;
priv = container_of(delayed_work, struct rkcanfd_priv, timestamp);
- timecounter_read(&priv->tc);
-
- schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ if (priv->timestamp_active) {
+ timecounter_read(&priv->tc);
+
+ /* Serialize requeueing with timestamp_stop(). */
+ schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
+ }
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
}
void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv)
{
+ spin_lock_init(&priv->timestamp_lock);
+ priv->timestamp_active = false;
INIT_DELAYED_WORK(&priv->timestamp, rkcanfd_timestamp_work);
}
+/* The caller must quiesce IRQs and timestamp work before reconfiguration. */
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
{
const struct can_bittiming *dbt = &priv->can.fd.data_bittiming;
@@ -93,17 +102,34 @@ void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
void rkcanfd_timestamp_start(struct rkcanfd_priv *priv)
{
- timecounter_init(&priv->tc, &priv->cc, ktime_get_real_ns());
+ unsigned long flags;
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ timecounter_init(&priv->tc, &priv->cc, ktime_get_real_ns());
+ priv->timestamp_active = true;
schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
}
void rkcanfd_timestamp_stop(struct rkcanfd_priv *priv)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ priv->timestamp_active = false;
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
+
cancel_delayed_work(&priv->timestamp);
}
void rkcanfd_timestamp_stop_sync(struct rkcanfd_priv *priv)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ priv->timestamp_active = false;
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
+
+ /* The worker needs timestamp_lock to finish. */
cancel_delayed_work_sync(&priv->timestamp);
}
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index e1debab54534..2743c96ed42f 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>
@@ -485,8 +486,11 @@ struct rkcanfd_priv {
struct cyclecounter cc;
struct timecounter tc;
+ /* Protects timestamp_active and worker reads/requeueing. */
+ spinlock_t timestamp_lock;
struct delayed_work timestamp;
unsigned long work_delay_jiffies;
+ bool timestamp_active;
struct can_berr_counter bec;
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 4/6] can: rockchip_canfd: prevent timestamp work from rearming after bus-off
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
The bus-off IRQ cancels timestamp work asynchronously. If the worker
is already running, it can requeue itself after cancellation and keep
polling while the controller remains bus-off.
Add a dedicated lock and an active flag, initialized once at probe.
Serialize the worker's active check, counter read and self-requeue with
start and stop. Clear the flag under the lock before cancelling work
so no worker can read the counter or rearm after deactivation. Enable
the flag and schedule the initial work under the same lock at start.
Keep both cancellation calls outside the lock so synchronous stop can
wait for the worker to finish.
Fixes: 4e1a18bab124 ("can: rockchip_canfd: add hardware timestamping support")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
.../net/can/rockchip/rockchip_canfd-timestamp.c | 34 +++++++++++++++++++---
drivers/net/can/rockchip/rockchip_canfd.h | 4 +++
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
index ad582afbbd68..0fa791849c95 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
@@ -30,18 +30,27 @@ static void rkcanfd_timestamp_work(struct work_struct *work)
{
const struct delayed_work *delayed_work = to_delayed_work(work);
struct rkcanfd_priv *priv;
+ unsigned long flags;
priv = container_of(delayed_work, struct rkcanfd_priv, timestamp);
- timecounter_read(&priv->tc);
-
- schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ if (priv->timestamp_active) {
+ timecounter_read(&priv->tc);
+
+ /* Serialize requeueing with timestamp_stop(). */
+ schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
+ }
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
}
void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv)
{
+ spin_lock_init(&priv->timestamp_lock);
+ priv->timestamp_active = false;
INIT_DELAYED_WORK(&priv->timestamp, rkcanfd_timestamp_work);
}
+/* The caller must quiesce IRQs and timestamp work before reconfiguration. */
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
{
const struct can_bittiming *dbt = &priv->can.fd.data_bittiming;
@@ -93,17 +102,34 @@ void rkcanfd_timestamp_init(struct rkcanfd_priv *priv)
void rkcanfd_timestamp_start(struct rkcanfd_priv *priv)
{
- timecounter_init(&priv->tc, &priv->cc, ktime_get_real_ns());
+ unsigned long flags;
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ timecounter_init(&priv->tc, &priv->cc, ktime_get_real_ns());
+ priv->timestamp_active = true;
schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies);
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
}
void rkcanfd_timestamp_stop(struct rkcanfd_priv *priv)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ priv->timestamp_active = false;
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
+
cancel_delayed_work(&priv->timestamp);
}
void rkcanfd_timestamp_stop_sync(struct rkcanfd_priv *priv)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
+ priv->timestamp_active = false;
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
+
+ /* The worker needs timestamp_lock to finish. */
cancel_delayed_work_sync(&priv->timestamp);
}
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index e1debab54534..2743c96ed42f 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>
@@ -485,8 +486,11 @@ struct rkcanfd_priv {
struct cyclecounter cc;
struct timecounter tc;
+ /* Protects timestamp_active and worker reads/requeueing. */
+ spinlock_t timestamp_lock;
struct delayed_work timestamp;
unsigned long work_delay_jiffies;
+ bool timestamp_active;
struct can_berr_counter bec;
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH net v5 5/6] can: rockchip_canfd: serialize TX with bus-off and restart
2026-10-15 9:45 ` Cunhao Lu via B4 Relay
(?)
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
-1 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
The bus-off path can reset the controller while ndo_start_xmit() is
active. CAN restart also flushes the echo ring before calling set_mode(),
so disabling TX in that callback is too late to protect echo slots.
Serialize TX submission, RX-side completion and retry with a private
lock and enabled flag. Disable TX and drain these users before publishing
CAN_STATE_BUS_OFF, protecting the core's echo flush during automatic and
manual recovery. Reject further submissions with NETDEV_TX_BUSY.
If close disables TX after an RX IRQ has consumed a self-transmitted
frame, skipping echo filtering alone would deliver it as ordinary RX.
Drop consumed FIFO entries while TX is disabled, checking under the same
lock, and continue draining the FIFO.
After draining the old IRQ and reinitializing the controller, restore
carrier, enable TX, resume IRQ handling and wake the queue under the
lock. This prevents the old bus-off IRQ from leaving manual recovery
with carrier off, while allowing a new bus-off to take precedence.
Keep shared IRQ handling suspended during open until initialization is
complete, and leave TX disabled on close or failed open. Keep resets and
synchronous IRQ/work draining outside the lock.
Fixes: b6661d73290c ("can: rockchip_canfd: add TX PATH")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
Changes in v5:
- Drop consumed RX FIFO entries when tx_enabled is false under tx_lock,
rather than skipping only echo filtering and delivering them as RX.
---
drivers/net/can/rockchip/rockchip_canfd-core.c | 51 ++++++++++++++++++++++----
drivers/net/can/rockchip/rockchip_canfd-rx.c | 26 +++++++++----
drivers/net/can/rockchip/rockchip_canfd-tx.c | 16 +++++++-
drivers/net/can/rockchip/rockchip_canfd.h | 3 ++
4 files changed, 80 insertions(+), 16 deletions(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-core.c b/drivers/net/can/rockchip/rockchip_canfd-core.c
index 4b3db07242395..150748c0b6845 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-core.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-core.c
@@ -326,6 +326,35 @@ static void rkcanfd_chip_start(struct rkcanfd_priv *priv)
rkcanfd_read(priv, RKCANFD_REG_MODE));
}
+static void rkcanfd_tx_disable(struct rkcanfd_priv *priv)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->tx_lock, flags);
+ priv->tx_enabled = false;
+ netif_stop_queue(priv->ndev);
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+}
+
+static void rkcanfd_chip_start_finish(struct rkcanfd_priv *priv)
+{
+ unsigned long flags;
+
+ /* Serialize carrier, IRQ enable and queue wake with a new bus-off. */
+ spin_lock_irqsave(&priv->tx_lock, flags);
+ /* A manual restart may have raced with the old bus-off IRQ's
+ * netif_carrier_off(). That IRQ is now drained; restore carrier
+ * before resuming IRQ handling so a new bus-off takes precedence.
+ */
+ netif_carrier_on(priv->ndev);
+ priv->tx_enabled = true;
+ /* Publish the initialized state before resuming IRQ handling. */
+ smp_store_release(&priv->irq_suspended, false);
+ rkcanfd_chip_interrupts_enable(priv);
+ netif_wake_queue(priv->ndev);
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+}
+
static void __rkcanfd_chip_stop(struct rkcanfd_priv *priv, const enum can_state state)
{
priv->can.state = state;
@@ -357,6 +386,7 @@ static int rkcanfd_set_mode(struct net_device *ndev,
switch (mode) {
case CAN_MODE_START:
+ rkcanfd_tx_disable(priv);
rkcanfd_chip_interrupts_disable(priv);
/* Flush the interrupt mask write before suspending the handler. */
rkcanfd_read(priv, RKCANFD_REG_INT_MASK);
@@ -367,10 +397,7 @@ static int rkcanfd_set_mode(struct net_device *ndev,
rkcanfd_timestamp_stop_sync(priv);
rkcanfd_chip_start(priv);
- /* Publish the initialized state before resuming IRQ handling. */
- smp_store_release(&priv->irq_suspended, false);
- rkcanfd_chip_interrupts_enable(priv);
- netif_wake_queue(ndev);
+ rkcanfd_chip_start_finish(priv);
break;
default:
@@ -604,6 +631,12 @@ static int rkcanfd_handle_state_error_int(struct rkcanfd_priv *priv)
* handles cf == NULL.
*/
skb = rkcanfd_alloc_can_err_skb(priv, &cf, ×tamp);
+ if (new_state == CAN_STATE_BUS_OFF) {
+ /* Drain TX before publishing BUS_OFF, which permits a manual
+ * restart to flush echo SKBs before calling our set_mode().
+ */
+ rkcanfd_tx_disable(priv);
+ }
can_change_state(ndev, cf, tx_state, rx_state);
if (new_state == CAN_STATE_BUS_OFF) {
@@ -749,6 +782,8 @@ static int rkcanfd_open(struct net_device *ndev)
if (err)
goto out_close_candev;
+ /* A shared IRQ may run as soon as request_irq() succeeds. */
+ WRITE_ONCE(priv->irq_suspended, true);
rkcanfd_chip_start(priv);
can_rx_offload_enable(&priv->offload);
@@ -756,13 +791,12 @@ static int rkcanfd_open(struct net_device *ndev)
if (err)
goto out_rkcanfd_chip_stop;
- rkcanfd_chip_interrupts_enable(priv);
-
- netif_start_queue(ndev);
+ rkcanfd_chip_start_finish(priv);
return 0;
out_rkcanfd_chip_stop:
+ rkcanfd_tx_disable(priv);
can_rx_offload_disable(&priv->offload);
rkcanfd_chip_stop_sync(priv, CAN_STATE_STOPPED);
pm_runtime_put(ndev->dev.parent);
@@ -776,7 +810,7 @@ static int rkcanfd_stop(struct net_device *ndev)
struct rkcanfd_priv *priv = netdev_priv(ndev);
close_candev_cancel_restart_sync(ndev);
- netif_stop_queue(ndev);
+ rkcanfd_tx_disable(priv);
rkcanfd_chip_interrupts_disable(priv);
free_irq(ndev->irq, priv);
@@ -939,6 +973,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 24e87daa1df00..5e64317d4eee6 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 and TX enabled. */
static int rkcanfd_rxstx_filter(struct rkcanfd_priv *priv,
const struct canfd_frame *cfd_rx, const u32 ts,
bool *tx_done)
@@ -204,7 +205,9 @@ static int rkcanfd_handle_rx_int_one(struct rkcanfd_priv *priv)
struct canfd_frame cfd[1] = { }, *skb_cfd;
struct rkcanfd_fifo_header header[1] = { };
struct sk_buff *skb;
+ unsigned long flags;
unsigned int len;
+ bool tx_done = false;
int err;
/* read header into separate struct and convert it later */
@@ -237,15 +240,24 @@ static int rkcanfd_handle_rx_int_one(struct rkcanfd_priv *priv)
return 0;
}
- if (rkcanfd_get_tx_pending(priv)) {
- bool tx_done = false;
+ /* Drop FIFO entries while TX is disabled, as they may be self-
+ * received frames. Do not access the echo ring, retry TX or wake the
+ * queue while restart owns the ring. The entry is already consumed.
+ */
+ spin_lock_irqsave(&priv->tx_lock, flags);
+ if (unlikely(!priv->tx_enabled)) {
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+ return 0;
+ }
+ 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 12200dcfd3389..f49ed6d2266d9 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-tx.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-tx.c
@@ -69,12 +69,19 @@ 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 (unlikely(!priv->tx_enabled)) {
+ netif_stop_queue(ndev);
+ goto out_busy;
+ }
+
if (!netif_subqueue_maybe_stop(priv->ndev, 0,
rkcanfd_get_effective_tx_free(priv),
RKCANFD_TX_STOP_THRESHOLD,
@@ -85,7 +92,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;
@@ -137,7 +144,14 @@ 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);
+
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,
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index 2743c96ed42f0..4a2bdc205296e 100644
--- a/drivers/net/can/rockchip/rockchip_canfd.h
+++ b/drivers/net/can/rockchip/rockchip_canfd.h
@@ -476,6 +476,9 @@ struct rkcanfd_priv {
struct net_device *ndev;
void __iomem *regs;
+ /* Serialize TX submission/completion with bus-off and restart. */
+ spinlock_t tx_lock;
+ bool tx_enabled;
unsigned int tx_head;
unsigned int tx_tail;
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 5/6] can: rockchip_canfd: serialize TX with bus-off and restart
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
The bus-off path can reset the controller while ndo_start_xmit() is
active. CAN restart also flushes the echo ring before calling set_mode(),
so disabling TX in that callback is too late to protect echo slots.
Serialize TX submission, RX-side completion and retry with a private
lock and enabled flag. Disable TX and drain these users before publishing
CAN_STATE_BUS_OFF, protecting the core's echo flush during automatic and
manual recovery. Reject further submissions with NETDEV_TX_BUSY.
If close disables TX after an RX IRQ has consumed a self-transmitted
frame, skipping echo filtering alone would deliver it as ordinary RX.
Drop consumed FIFO entries while TX is disabled, checking under the same
lock, and continue draining the FIFO.
After draining the old IRQ and reinitializing the controller, restore
carrier, enable TX, resume IRQ handling and wake the queue under the
lock. This prevents the old bus-off IRQ from leaving manual recovery
with carrier off, while allowing a new bus-off to take precedence.
Keep shared IRQ handling suspended during open until initialization is
complete, and leave TX disabled on close or failed open. Keep resets and
synchronous IRQ/work draining outside the lock.
Fixes: b6661d73290c ("can: rockchip_canfd: add TX PATH")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
Changes in v5:
- Drop consumed RX FIFO entries when tx_enabled is false under tx_lock,
rather than skipping only echo filtering and delivering them as RX.
---
drivers/net/can/rockchip/rockchip_canfd-core.c | 51 ++++++++++++++++++++++----
drivers/net/can/rockchip/rockchip_canfd-rx.c | 26 +++++++++----
drivers/net/can/rockchip/rockchip_canfd-tx.c | 16 +++++++-
drivers/net/can/rockchip/rockchip_canfd.h | 3 ++
4 files changed, 80 insertions(+), 16 deletions(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-core.c b/drivers/net/can/rockchip/rockchip_canfd-core.c
index 4b3db07242395..150748c0b6845 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-core.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-core.c
@@ -326,6 +326,35 @@ static void rkcanfd_chip_start(struct rkcanfd_priv *priv)
rkcanfd_read(priv, RKCANFD_REG_MODE));
}
+static void rkcanfd_tx_disable(struct rkcanfd_priv *priv)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->tx_lock, flags);
+ priv->tx_enabled = false;
+ netif_stop_queue(priv->ndev);
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+}
+
+static void rkcanfd_chip_start_finish(struct rkcanfd_priv *priv)
+{
+ unsigned long flags;
+
+ /* Serialize carrier, IRQ enable and queue wake with a new bus-off. */
+ spin_lock_irqsave(&priv->tx_lock, flags);
+ /* A manual restart may have raced with the old bus-off IRQ's
+ * netif_carrier_off(). That IRQ is now drained; restore carrier
+ * before resuming IRQ handling so a new bus-off takes precedence.
+ */
+ netif_carrier_on(priv->ndev);
+ priv->tx_enabled = true;
+ /* Publish the initialized state before resuming IRQ handling. */
+ smp_store_release(&priv->irq_suspended, false);
+ rkcanfd_chip_interrupts_enable(priv);
+ netif_wake_queue(priv->ndev);
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+}
+
static void __rkcanfd_chip_stop(struct rkcanfd_priv *priv, const enum can_state state)
{
priv->can.state = state;
@@ -357,6 +386,7 @@ static int rkcanfd_set_mode(struct net_device *ndev,
switch (mode) {
case CAN_MODE_START:
+ rkcanfd_tx_disable(priv);
rkcanfd_chip_interrupts_disable(priv);
/* Flush the interrupt mask write before suspending the handler. */
rkcanfd_read(priv, RKCANFD_REG_INT_MASK);
@@ -367,10 +397,7 @@ static int rkcanfd_set_mode(struct net_device *ndev,
rkcanfd_timestamp_stop_sync(priv);
rkcanfd_chip_start(priv);
- /* Publish the initialized state before resuming IRQ handling. */
- smp_store_release(&priv->irq_suspended, false);
- rkcanfd_chip_interrupts_enable(priv);
- netif_wake_queue(ndev);
+ rkcanfd_chip_start_finish(priv);
break;
default:
@@ -604,6 +631,12 @@ static int rkcanfd_handle_state_error_int(struct rkcanfd_priv *priv)
* handles cf == NULL.
*/
skb = rkcanfd_alloc_can_err_skb(priv, &cf, ×tamp);
+ if (new_state == CAN_STATE_BUS_OFF) {
+ /* Drain TX before publishing BUS_OFF, which permits a manual
+ * restart to flush echo SKBs before calling our set_mode().
+ */
+ rkcanfd_tx_disable(priv);
+ }
can_change_state(ndev, cf, tx_state, rx_state);
if (new_state == CAN_STATE_BUS_OFF) {
@@ -749,6 +782,8 @@ static int rkcanfd_open(struct net_device *ndev)
if (err)
goto out_close_candev;
+ /* A shared IRQ may run as soon as request_irq() succeeds. */
+ WRITE_ONCE(priv->irq_suspended, true);
rkcanfd_chip_start(priv);
can_rx_offload_enable(&priv->offload);
@@ -756,13 +791,12 @@ static int rkcanfd_open(struct net_device *ndev)
if (err)
goto out_rkcanfd_chip_stop;
- rkcanfd_chip_interrupts_enable(priv);
-
- netif_start_queue(ndev);
+ rkcanfd_chip_start_finish(priv);
return 0;
out_rkcanfd_chip_stop:
+ rkcanfd_tx_disable(priv);
can_rx_offload_disable(&priv->offload);
rkcanfd_chip_stop_sync(priv, CAN_STATE_STOPPED);
pm_runtime_put(ndev->dev.parent);
@@ -776,7 +810,7 @@ static int rkcanfd_stop(struct net_device *ndev)
struct rkcanfd_priv *priv = netdev_priv(ndev);
close_candev_cancel_restart_sync(ndev);
- netif_stop_queue(ndev);
+ rkcanfd_tx_disable(priv);
rkcanfd_chip_interrupts_disable(priv);
free_irq(ndev->irq, priv);
@@ -939,6 +973,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 24e87daa1df00..5e64317d4eee6 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 and TX enabled. */
static int rkcanfd_rxstx_filter(struct rkcanfd_priv *priv,
const struct canfd_frame *cfd_rx, const u32 ts,
bool *tx_done)
@@ -204,7 +205,9 @@ static int rkcanfd_handle_rx_int_one(struct rkcanfd_priv *priv)
struct canfd_frame cfd[1] = { }, *skb_cfd;
struct rkcanfd_fifo_header header[1] = { };
struct sk_buff *skb;
+ unsigned long flags;
unsigned int len;
+ bool tx_done = false;
int err;
/* read header into separate struct and convert it later */
@@ -237,15 +240,24 @@ static int rkcanfd_handle_rx_int_one(struct rkcanfd_priv *priv)
return 0;
}
- if (rkcanfd_get_tx_pending(priv)) {
- bool tx_done = false;
+ /* Drop FIFO entries while TX is disabled, as they may be self-
+ * received frames. Do not access the echo ring, retry TX or wake the
+ * queue while restart owns the ring. The entry is already consumed.
+ */
+ spin_lock_irqsave(&priv->tx_lock, flags);
+ if (unlikely(!priv->tx_enabled)) {
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+ return 0;
+ }
+ 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 12200dcfd3389..f49ed6d2266d9 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-tx.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-tx.c
@@ -69,12 +69,19 @@ 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 (unlikely(!priv->tx_enabled)) {
+ netif_stop_queue(ndev);
+ goto out_busy;
+ }
+
if (!netif_subqueue_maybe_stop(priv->ndev, 0,
rkcanfd_get_effective_tx_free(priv),
RKCANFD_TX_STOP_THRESHOLD,
@@ -85,7 +92,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;
@@ -137,7 +144,14 @@ 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);
+
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,
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index 2743c96ed42f0..4a2bdc205296e 100644
--- a/drivers/net/can/rockchip/rockchip_canfd.h
+++ b/drivers/net/can/rockchip/rockchip_canfd.h
@@ -476,6 +476,9 @@ struct rkcanfd_priv {
struct net_device *ndev;
void __iomem *regs;
+ /* Serialize TX submission/completion with bus-off and restart. */
+ spinlock_t tx_lock;
+ bool tx_enabled;
unsigned int tx_head;
unsigned int tx_tail;
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 5/6] can: rockchip_canfd: serialize TX with bus-off and restart
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
The bus-off path can reset the controller while ndo_start_xmit() is
active. CAN restart also flushes the echo ring before calling set_mode(),
so disabling TX in that callback is too late to protect echo slots.
Serialize TX submission, RX-side completion and retry with a private
lock and enabled flag. Disable TX and drain these users before publishing
CAN_STATE_BUS_OFF, protecting the core's echo flush during automatic and
manual recovery. Reject further submissions with NETDEV_TX_BUSY.
If close disables TX after an RX IRQ has consumed a self-transmitted
frame, skipping echo filtering alone would deliver it as ordinary RX.
Drop consumed FIFO entries while TX is disabled, checking under the same
lock, and continue draining the FIFO.
After draining the old IRQ and reinitializing the controller, restore
carrier, enable TX, resume IRQ handling and wake the queue under the
lock. This prevents the old bus-off IRQ from leaving manual recovery
with carrier off, while allowing a new bus-off to take precedence.
Keep shared IRQ handling suspended during open until initialization is
complete, and leave TX disabled on close or failed open. Keep resets and
synchronous IRQ/work draining outside the lock.
Fixes: b6661d73290c ("can: rockchip_canfd: add TX PATH")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
Changes in v5:
- Drop consumed RX FIFO entries when tx_enabled is false under tx_lock,
rather than skipping only echo filtering and delivering them as RX.
---
drivers/net/can/rockchip/rockchip_canfd-core.c | 51 ++++++++++++++++++++++----
drivers/net/can/rockchip/rockchip_canfd-rx.c | 26 +++++++++----
drivers/net/can/rockchip/rockchip_canfd-tx.c | 16 +++++++-
drivers/net/can/rockchip/rockchip_canfd.h | 3 ++
4 files changed, 80 insertions(+), 16 deletions(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-core.c b/drivers/net/can/rockchip/rockchip_canfd-core.c
index 4b3db07242395..150748c0b6845 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-core.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-core.c
@@ -326,6 +326,35 @@ static void rkcanfd_chip_start(struct rkcanfd_priv *priv)
rkcanfd_read(priv, RKCANFD_REG_MODE));
}
+static void rkcanfd_tx_disable(struct rkcanfd_priv *priv)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->tx_lock, flags);
+ priv->tx_enabled = false;
+ netif_stop_queue(priv->ndev);
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+}
+
+static void rkcanfd_chip_start_finish(struct rkcanfd_priv *priv)
+{
+ unsigned long flags;
+
+ /* Serialize carrier, IRQ enable and queue wake with a new bus-off. */
+ spin_lock_irqsave(&priv->tx_lock, flags);
+ /* A manual restart may have raced with the old bus-off IRQ's
+ * netif_carrier_off(). That IRQ is now drained; restore carrier
+ * before resuming IRQ handling so a new bus-off takes precedence.
+ */
+ netif_carrier_on(priv->ndev);
+ priv->tx_enabled = true;
+ /* Publish the initialized state before resuming IRQ handling. */
+ smp_store_release(&priv->irq_suspended, false);
+ rkcanfd_chip_interrupts_enable(priv);
+ netif_wake_queue(priv->ndev);
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+}
+
static void __rkcanfd_chip_stop(struct rkcanfd_priv *priv, const enum can_state state)
{
priv->can.state = state;
@@ -357,6 +386,7 @@ static int rkcanfd_set_mode(struct net_device *ndev,
switch (mode) {
case CAN_MODE_START:
+ rkcanfd_tx_disable(priv);
rkcanfd_chip_interrupts_disable(priv);
/* Flush the interrupt mask write before suspending the handler. */
rkcanfd_read(priv, RKCANFD_REG_INT_MASK);
@@ -367,10 +397,7 @@ static int rkcanfd_set_mode(struct net_device *ndev,
rkcanfd_timestamp_stop_sync(priv);
rkcanfd_chip_start(priv);
- /* Publish the initialized state before resuming IRQ handling. */
- smp_store_release(&priv->irq_suspended, false);
- rkcanfd_chip_interrupts_enable(priv);
- netif_wake_queue(ndev);
+ rkcanfd_chip_start_finish(priv);
break;
default:
@@ -604,6 +631,12 @@ static int rkcanfd_handle_state_error_int(struct rkcanfd_priv *priv)
* handles cf == NULL.
*/
skb = rkcanfd_alloc_can_err_skb(priv, &cf, ×tamp);
+ if (new_state == CAN_STATE_BUS_OFF) {
+ /* Drain TX before publishing BUS_OFF, which permits a manual
+ * restart to flush echo SKBs before calling our set_mode().
+ */
+ rkcanfd_tx_disable(priv);
+ }
can_change_state(ndev, cf, tx_state, rx_state);
if (new_state == CAN_STATE_BUS_OFF) {
@@ -749,6 +782,8 @@ static int rkcanfd_open(struct net_device *ndev)
if (err)
goto out_close_candev;
+ /* A shared IRQ may run as soon as request_irq() succeeds. */
+ WRITE_ONCE(priv->irq_suspended, true);
rkcanfd_chip_start(priv);
can_rx_offload_enable(&priv->offload);
@@ -756,13 +791,12 @@ static int rkcanfd_open(struct net_device *ndev)
if (err)
goto out_rkcanfd_chip_stop;
- rkcanfd_chip_interrupts_enable(priv);
-
- netif_start_queue(ndev);
+ rkcanfd_chip_start_finish(priv);
return 0;
out_rkcanfd_chip_stop:
+ rkcanfd_tx_disable(priv);
can_rx_offload_disable(&priv->offload);
rkcanfd_chip_stop_sync(priv, CAN_STATE_STOPPED);
pm_runtime_put(ndev->dev.parent);
@@ -776,7 +810,7 @@ static int rkcanfd_stop(struct net_device *ndev)
struct rkcanfd_priv *priv = netdev_priv(ndev);
close_candev_cancel_restart_sync(ndev);
- netif_stop_queue(ndev);
+ rkcanfd_tx_disable(priv);
rkcanfd_chip_interrupts_disable(priv);
free_irq(ndev->irq, priv);
@@ -939,6 +973,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 24e87daa1df00..5e64317d4eee6 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 and TX enabled. */
static int rkcanfd_rxstx_filter(struct rkcanfd_priv *priv,
const struct canfd_frame *cfd_rx, const u32 ts,
bool *tx_done)
@@ -204,7 +205,9 @@ static int rkcanfd_handle_rx_int_one(struct rkcanfd_priv *priv)
struct canfd_frame cfd[1] = { }, *skb_cfd;
struct rkcanfd_fifo_header header[1] = { };
struct sk_buff *skb;
+ unsigned long flags;
unsigned int len;
+ bool tx_done = false;
int err;
/* read header into separate struct and convert it later */
@@ -237,15 +240,24 @@ static int rkcanfd_handle_rx_int_one(struct rkcanfd_priv *priv)
return 0;
}
- if (rkcanfd_get_tx_pending(priv)) {
- bool tx_done = false;
+ /* Drop FIFO entries while TX is disabled, as they may be self-
+ * received frames. Do not access the echo ring, retry TX or wake the
+ * queue while restart owns the ring. The entry is already consumed.
+ */
+ spin_lock_irqsave(&priv->tx_lock, flags);
+ if (unlikely(!priv->tx_enabled)) {
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+ return 0;
+ }
+ 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 12200dcfd3389..f49ed6d2266d9 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-tx.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-tx.c
@@ -69,12 +69,19 @@ 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 (unlikely(!priv->tx_enabled)) {
+ netif_stop_queue(ndev);
+ goto out_busy;
+ }
+
if (!netif_subqueue_maybe_stop(priv->ndev, 0,
rkcanfd_get_effective_tx_free(priv),
RKCANFD_TX_STOP_THRESHOLD,
@@ -85,7 +92,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;
@@ -137,7 +144,14 @@ 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);
+
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,
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index 2743c96ed42f0..4a2bdc205296e 100644
--- a/drivers/net/can/rockchip/rockchip_canfd.h
+++ b/drivers/net/can/rockchip/rockchip_canfd.h
@@ -476,6 +476,9 @@ struct rkcanfd_priv {
struct net_device *ndev;
void __iomem *regs;
+ /* Serialize TX submission/completion with bus-off and restart. */
+ spinlock_t tx_lock;
+ bool tx_enabled;
unsigned int tx_head;
unsigned int tx_tail;
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH net v5 6/6] can: rockchip_canfd: serialize timecounter timestamp conversion
2026-10-15 9:45 ` Cunhao Lu via B4 Relay
(?)
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
-1 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
The timestamp worker advances the timecounter with timecounter_read()
while RX, TX completion and error-frame paths use timecounter_cyc2time()
to convert hardware timestamps without holding timestamp_lock.
timecounter_read() updates frac, cycle_last and nsec in separate steps.
A concurrent conversion can observe the new cycle_last with the old
nsec and produce an incorrect timestamp.
Hold timestamp_lock across timecounter_cyc2time(), using the same lock
as timecounter_read() and timecounter_init(). Keep the SKB timestamp
assignment outside the critical section.
TX completion nests timestamp_lock inside tx_lock; no timestamp-lock
holder acquires tx_lock.
Fixes: 4e1a18bab124 ("can: rockchip_canfd: add hardware timestamping support")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
drivers/net/can/rockchip/rockchip_canfd-timestamp.c | 5 ++++-
drivers/net/can/rockchip/rockchip_canfd.h | 4 ++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
index 0fa791849c95..05f1005ee506 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
@@ -15,13 +15,16 @@ static u64 rkcanfd_timestamp_read(struct cyclecounter *cc)
return rkcanfd_get_timestamp(priv);
}
-void rkcanfd_skb_set_timestamp(const struct rkcanfd_priv *priv,
+void rkcanfd_skb_set_timestamp(struct rkcanfd_priv *priv,
struct sk_buff *skb, const u32 timestamp)
{
struct skb_shared_hwtstamps *hwtstamps = skb_hwtstamps(skb);
+ unsigned long flags;
u64 ns;
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
ns = timecounter_cyc2time(&priv->tc, timestamp);
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
hwtstamps->hwtstamp = ns_to_ktime(ns);
}
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index 4a2bdc205296..63f146ff8e6b 100644
--- a/drivers/net/can/rockchip/rockchip_canfd.h
+++ b/drivers/net/can/rockchip/rockchip_canfd.h
@@ -489,7 +489,7 @@ struct rkcanfd_priv {
struct cyclecounter cc;
struct timecounter tc;
- /* Protects timestamp_active and worker reads/requeueing. */
+ /* Protects tc, timestamp_active and worker requeueing. */
spinlock_t timestamp_lock;
struct delayed_work timestamp;
unsigned long work_delay_jiffies;
@@ -557,7 +557,7 @@ void rkcanfd_ethtool_init(struct rkcanfd_priv *priv);
int rkcanfd_handle_rx_int(struct rkcanfd_priv *priv);
-void rkcanfd_skb_set_timestamp(const struct rkcanfd_priv *priv,
+void rkcanfd_skb_set_timestamp(struct rkcanfd_priv *priv,
struct sk_buff *skb, const u32 timestamp);
void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv);
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 6/6] can: rockchip_canfd: serialize timecounter timestamp conversion
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
The timestamp worker advances the timecounter with timecounter_read()
while RX, TX completion and error-frame paths use timecounter_cyc2time()
to convert hardware timestamps without holding timestamp_lock.
timecounter_read() updates frac, cycle_last and nsec in separate steps.
A concurrent conversion can observe the new cycle_last with the old
nsec and produce an incorrect timestamp.
Hold timestamp_lock across timecounter_cyc2time(), using the same lock
as timecounter_read() and timecounter_init(). Keep the SKB timestamp
assignment outside the critical section.
TX completion nests timestamp_lock inside tx_lock; no timestamp-lock
holder acquires tx_lock.
Fixes: 4e1a18bab124 ("can: rockchip_canfd: add hardware timestamping support")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
drivers/net/can/rockchip/rockchip_canfd-timestamp.c | 5 ++++-
drivers/net/can/rockchip/rockchip_canfd.h | 4 ++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
index 0fa791849c95..05f1005ee506 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
@@ -15,13 +15,16 @@ static u64 rkcanfd_timestamp_read(struct cyclecounter *cc)
return rkcanfd_get_timestamp(priv);
}
-void rkcanfd_skb_set_timestamp(const struct rkcanfd_priv *priv,
+void rkcanfd_skb_set_timestamp(struct rkcanfd_priv *priv,
struct sk_buff *skb, const u32 timestamp)
{
struct skb_shared_hwtstamps *hwtstamps = skb_hwtstamps(skb);
+ unsigned long flags;
u64 ns;
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
ns = timecounter_cyc2time(&priv->tc, timestamp);
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
hwtstamps->hwtstamp = ns_to_ktime(ns);
}
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index 4a2bdc205296..63f146ff8e6b 100644
--- a/drivers/net/can/rockchip/rockchip_canfd.h
+++ b/drivers/net/can/rockchip/rockchip_canfd.h
@@ -489,7 +489,7 @@ struct rkcanfd_priv {
struct cyclecounter cc;
struct timecounter tc;
- /* Protects timestamp_active and worker reads/requeueing. */
+ /* Protects tc, timestamp_active and worker requeueing. */
spinlock_t timestamp_lock;
struct delayed_work timestamp;
unsigned long work_delay_jiffies;
@@ -557,7 +557,7 @@ void rkcanfd_ethtool_init(struct rkcanfd_priv *priv);
int rkcanfd_handle_rx_int(struct rkcanfd_priv *priv);
-void rkcanfd_skb_set_timestamp(const struct rkcanfd_priv *priv,
+void rkcanfd_skb_set_timestamp(struct rkcanfd_priv *priv,
struct sk_buff *skb, const u32 timestamp);
void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv);
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net v5 6/6] can: rockchip_canfd: serialize timecounter timestamp conversion
@ 2026-10-15 9:45 ` Cunhao Lu via B4 Relay
0 siblings, 0 replies; 25+ messages in thread
From: Cunhao Lu via B4 Relay @ 2026-10-15 9:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Heiko Stuebner,
David S. Miller, Wolfgang Grandegger, Oliver Hartkopp, kernel
Cc: linux-can, linux-kernel, linux-arm-kernel, linux-rockchip, stable,
Cunhao Lu
From: Cunhao Lu <1579567540@qq.com>
The timestamp worker advances the timecounter with timecounter_read()
while RX, TX completion and error-frame paths use timecounter_cyc2time()
to convert hardware timestamps without holding timestamp_lock.
timecounter_read() updates frac, cycle_last and nsec in separate steps.
A concurrent conversion can observe the new cycle_last with the old
nsec and produce an incorrect timestamp.
Hold timestamp_lock across timecounter_cyc2time(), using the same lock
as timecounter_read() and timecounter_init(). Keep the SKB timestamp
assignment outside the critical section.
TX completion nests timestamp_lock inside tx_lock; no timestamp-lock
holder acquires tx_lock.
Fixes: 4e1a18bab124 ("can: rockchip_canfd: add hardware timestamping support")
Cc: stable@vger.kernel.org
Signed-off-by: Cunhao Lu <1579567540@qq.com>
---
drivers/net/can/rockchip/rockchip_canfd-timestamp.c | 5 ++++-
drivers/net/can/rockchip/rockchip_canfd.h | 4 ++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
index 0fa791849c95..05f1005ee506 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-timestamp.c
@@ -15,13 +15,16 @@ static u64 rkcanfd_timestamp_read(struct cyclecounter *cc)
return rkcanfd_get_timestamp(priv);
}
-void rkcanfd_skb_set_timestamp(const struct rkcanfd_priv *priv,
+void rkcanfd_skb_set_timestamp(struct rkcanfd_priv *priv,
struct sk_buff *skb, const u32 timestamp)
{
struct skb_shared_hwtstamps *hwtstamps = skb_hwtstamps(skb);
+ unsigned long flags;
u64 ns;
+ spin_lock_irqsave(&priv->timestamp_lock, flags);
ns = timecounter_cyc2time(&priv->tc, timestamp);
+ spin_unlock_irqrestore(&priv->timestamp_lock, flags);
hwtstamps->hwtstamp = ns_to_ktime(ns);
}
diff --git a/drivers/net/can/rockchip/rockchip_canfd.h b/drivers/net/can/rockchip/rockchip_canfd.h
index 4a2bdc205296..63f146ff8e6b 100644
--- a/drivers/net/can/rockchip/rockchip_canfd.h
+++ b/drivers/net/can/rockchip/rockchip_canfd.h
@@ -489,7 +489,7 @@ struct rkcanfd_priv {
struct cyclecounter cc;
struct timecounter tc;
- /* Protects timestamp_active and worker reads/requeueing. */
+ /* Protects tc, timestamp_active and worker requeueing. */
spinlock_t timestamp_lock;
struct delayed_work timestamp;
unsigned long work_delay_jiffies;
@@ -557,7 +557,7 @@ void rkcanfd_ethtool_init(struct rkcanfd_priv *priv);
int rkcanfd_handle_rx_int(struct rkcanfd_priv *priv);
-void rkcanfd_skb_set_timestamp(const struct rkcanfd_priv *priv,
+void rkcanfd_skb_set_timestamp(struct rkcanfd_priv *priv,
struct sk_buff *skb, const u32 timestamp);
void rkcanfd_timestamp_work_init(struct rkcanfd_priv *priv);
void rkcanfd_timestamp_init(struct rkcanfd_priv *priv);
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread