All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Cunhao Lu" <1579567540@qq.com>
Cc: linux-can@vger.kernel.org,
	"Marc Kleine-Budde" <mkl@pengutronix.de>,
	"Oliver Hartkopp" <socketcan@hartkopp.net>,
	"Vincent Mailhol" <mailhol@kernel.org>,
	"Oleksij Rempel" <o.rempel@pengutronix.de>
Subject: Re: [PATCH net v3 3/5] can: rockchip_canfd: cancel bus-off restart before closing the device
Date: Wed, 14 Oct 2026 13:35:42 +0000	[thread overview]
Message-ID: <sashiko-outbox-171362@kernel.org> (raw)
In-Reply-To: <20261014-rockchip-canfd-restart-close-v3-3-f0eb41090242@qq.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pm: async pm_runtime_put() before pm_runtime_disable() leaves hardware active
--

commit 614c95a9bb5ead8b95b8b6a430415575bc664f43
Author: Cunhao Lu <1579567540@qq.com>

can: rockchip_canfd: cancel bus-off restart before closing the device

This commit fixes a race condition during device close by cancelling the
bus-off restart work early. It adds synchronous cancellation to ndo_stop()
while resources are still available, preventing late restarts. The patch
also improves interrupt handling and RX offload enablement.

> 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

[ ... ]

> @@ -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: Medium]
Later in this function, pm_runtime_put(ndev->dev.parent) is called.

Since pm_runtime_put() queues an asynchronous idle request, it can leave the
hardware active.

When rmmod is executed, the removal path calls rkcanfd_remove() which calls
rkcanfd_unregister():

static inline void rkcanfd_unregister(struct rkcanfd_priv *priv)
{
	struct net_device *ndev	= priv->ndev;

	unregister_candev(ndev);
	pm_runtime_disable(ndev->dev.parent);
}

unregister_candev() synchronously brings the interface down by calling
rkcanfd_stop(), followed immediately by pm_runtime_disable() in
rkcanfd_unregister().

Because pm_runtime_disable() detects and cancels the pending async
idle/suspend work queued by pm_runtime_put() in rkcanfd_stop(), the device is
never suspended, leaving the hardware clocks enabled and powered on
indefinitely after the driver is removed.

Could this be a regression? Should pm_runtime_put_sync() be used in
rkcanfd_stop() instead of pm_runtime_put() to ensure the hardware is suspended
before pm_runtime_disable() cancels the work?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-rockchip-canfd-restart-close-v3-0-f0eb41090242@qq.com?part=3

  reply	other threads:[~2026-10-14 13:35 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-14 13:24 [PATCH net v3 0/5] can: fix bus-off restart and device close races Cunhao Lu
2026-10-14 13:24 ` Cunhao Lu via B4 Relay
2026-10-14 13:24 ` Cunhao Lu via B4 Relay
2026-10-14 13:24 ` [PATCH net v3 1/5] can: dev: prevent bus-off recovery when the interface is not running Cunhao Lu
2026-10-14 13:24   ` Cunhao Lu via B4 Relay
2026-10-14 13:24   ` Cunhao Lu via B4 Relay
2026-10-14 13:24 ` [PATCH net v3 2/5] can: dev: add close_candev_cancel_restart_sync() Cunhao Lu
2026-10-14 13:24   ` Cunhao Lu via B4 Relay
2026-10-14 13:24   ` Cunhao Lu via B4 Relay
2026-10-14 13:24 ` [PATCH net v3 3/5] can: rockchip_canfd: cancel bus-off restart before closing the device Cunhao Lu
2026-10-14 13:24   ` Cunhao Lu via B4 Relay
2026-10-14 13:24   ` Cunhao Lu via B4 Relay
2026-10-14 13:35   ` sashiko-bot [this message]
2026-10-14 13:24 ` [PATCH net v3 4/5] can: rockchip_canfd: prevent timestamp work from rearming after bus-off Cunhao Lu
2026-10-14 13:24   ` Cunhao Lu via B4 Relay
2026-10-14 13:24   ` Cunhao Lu via B4 Relay
2026-10-14 13:35   ` sashiko-bot
2026-10-14 13:24 ` [PATCH net v3 5/5] can: rockchip_canfd: serialize TX with bus-off and restart Cunhao Lu
2026-10-14 13:24   ` Cunhao Lu via B4 Relay
2026-10-14 13:24   ` Cunhao Lu via B4 Relay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-171362@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=1579567540@qq.com \
    --cc=linux-can@vger.kernel.org \
    --cc=mailhol@kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=o.rempel@pengutronix.de \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=socketcan@hartkopp.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.