Linux CAN drivers development
 help / color / mirror / Atom feed
From: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>,
	Vincent Mailhol <mailhol@kernel.org>
Cc: Vadim Fedorenko <vadim.fedorenko@linux.dev>,
	Celeste Liu <uwu@coelacanthus.name>, Kees Cook <kees@kernel.org>,
	linux-can@vger.kernel.org, linux-kernel@vger.kernel.org,
	Shuangpeng Bai <shuangpeng.kernel@gmail.com>,
	stable@vger.kernel.org
Subject: [PATCH can v2] can: gs_usb: fix hardware timestamp state for mixed channels
Date: Sat, 18 Jul 2026 23:00:00 -0400	[thread overview]
Message-ID: <20260719030000.3667449-1-shuangpeng.kernel@gmail.com> (raw)
In-Reply-To: <20260718052815.2507496-1-shuangpeng.kernel@gmail.com>

The hardware timestamp state is shared by struct gs_usb, but gs_can_open()
and gs_can_close() tie its initialization and teardown to active_channels
and to the feature bits of the channel being opened or closed.

This is wrong for mixed-channel devices in both directions. If a
non-timestamp channel opens first, a later timestamp-capable channel does
not initialize the shared cyclecounter/timecounter because active_channels
is already non-zero. Timestamp RX then calls timecounter_cyc2time() with
parent->tc.cc unset.

Conversely, if a timestamp-capable channel opens first and starts the
shared delayed work, then closes while a non-timestamp channel remains
active, disconnect may close the non-timestamp channel last. The old
teardown check skips gs_usb_timestamp_stop() in that case and frees
struct gs_usb while the delayed work timer is still queued.

Count the number of active timestamp-capable channels. Start the shared
timestamp worker when the first such channel opens, stop it when the last
such channel closes, and unwind the count if open fails.

Fixes: 45dfa45f52e6 ("can: gs_usb: add RX and TX hardware timestamp support")
Cc: stable@vger.kernel.org
Suggested-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
---
Changes in v2:
- Count active timestamp-capable channels instead of tracking only whether
  the shared timestamp worker has been started.
- Stop the shared timestamp worker when the last timestamp-capable channel
  closes, even if non-timestamp channels remain open.
- Unwind the timestamp-capable channel count on gs_can_open() failures.

 drivers/net/can/usb/gs_usb.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index ec9a7cbbbc69..9cc197803e0d 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -337,6 +337,7 @@ struct gs_usb {
 
 	unsigned int hf_size_rx;
 	u8 active_channels;
+	u8 active_timestamp_channels;
 	u8 channel_cnt;
 
 	unsigned int pipe_in;
@@ -980,10 +981,13 @@ static int gs_can_open(struct net_device *netdev)
 
 	can_rx_offload_enable(&dev->offload);
 
-	if (!parent->active_channels) {
-		if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)
+	if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP) {
+		if (!parent->active_timestamp_channels)
 			gs_usb_timestamp_init(parent);
+		parent->active_timestamp_channels++;
+	}
 
+	if (!parent->active_channels) {
 		for (i = 0; i < GS_MAX_RX_URBS; i++) {
 			u8 *buf;
 
@@ -1094,13 +1098,15 @@ static int gs_can_open(struct net_device *netdev)
 out_usb_free_urb:
 	usb_free_urb(urb);
 out_usb_kill_anchored_urbs:
-	if (!parent->active_channels) {
-		usb_kill_anchored_urbs(&parent->rx_submitted);
-
-		if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)
+	if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP) {
+		parent->active_timestamp_channels--;
+		if (!parent->active_timestamp_channels)
 			gs_usb_timestamp_stop(parent);
 	}
 
+	if (!parent->active_channels)
+		usb_kill_anchored_urbs(&parent->rx_submitted);
+
 	can_rx_offload_disable(&dev->offload);
 	close_candev(netdev);
 
@@ -1152,13 +1158,15 @@ static int gs_can_close(struct net_device *netdev)
 
 	/* Stop polling */
 	parent->active_channels--;
-	if (!parent->active_channels) {
-		usb_kill_anchored_urbs(&parent->rx_submitted);
-
-		if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)
+	if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP) {
+		parent->active_timestamp_channels--;
+		if (!parent->active_timestamp_channels)
 			gs_usb_timestamp_stop(parent);
 	}
 
+	if (!parent->active_channels)
+		usb_kill_anchored_urbs(&parent->rx_submitted);
+
 	/* Stop sending URBs */
 	usb_kill_anchored_urbs(&dev->tx_submitted);
 	atomic_set(&dev->active_tx_urbs, 0);
-- 
2.43.0

  parent reply	other threads:[~2026-07-19  3:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-18  5:28 [PATCH can] can: gs_usb: fix hardware timestamp state for mixed channels Shuangpeng Bai
2026-07-18  5:42 ` sashiko-bot
2026-07-18 21:53 ` Vadim Fedorenko
2026-07-19  3:00 ` Shuangpeng Bai [this message]
2026-07-19  3:13   ` [PATCH can v2] " sashiko-bot
2026-07-19  9:25   ` Vadim Fedorenko

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=20260719030000.3667449-1-shuangpeng.kernel@gmail.com \
    --to=shuangpeng.kernel@gmail.com \
    --cc=kees@kernel.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mailhol@kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=stable@vger.kernel.org \
    --cc=uwu@coelacanthus.name \
    --cc=vadim.fedorenko@linux.dev \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox