Linux USB
 help / color / mirror / Atom feed
* [PATCH 0/6] usb: serial: ch341: small cleanups and fixes
@ 2026-08-06  5:02 Vegom Doster
  2026-08-06  5:29 ` Greg KH
  0 siblings, 1 reply; 2+ messages in thread
From: Vegom Doster @ 2026-08-06  5:02 UTC (permalink / raw)
  To: linux-usb; +Cc: gregkh, johan


[-- Attachment #1.1: Type: text/plain, Size: 884 bytes --]

This series makes a few small cleanups and fixes to the CH341 serial
driver:

  - migrate the control-out path to usb_control_msg_send() so both
    directions use the modern interface
  - check the return value of ch341_configure() in reset_resume()
  - encode the inverted bit-7 hardware difference as a quirk flag set
    at configure time instead of a version comparison in the baud-rate
    setup path
  - replace the obsolete speed_t typedef with unsigned int
  - only update the cached baud rate when the register write succeeds,
    keeping the software state consistent with the hardware
  - annotate the break_end accesses with READ/WRITE_ONCE

All changes were tested on real hardware (1a86:7523): TX/RX echo at
9600 and 115200 baud, DTR/RTS toggling, and the divisor calculation was
verified for a range of line speeds.

Signed-off-by: Mark Ivanov <vegomdoster@gmail.com>

[-- Attachment #1.2: Type: text/html, Size: 1023 bytes --]

[-- Attachment #2: 0000-cover-letter.patch --]
[-- Type: text/x-patch, Size: 1559 bytes --]

From 7a0ddb9a49ad152efe8ae9171a92249343f68c7c Mon Sep 17 00:00:00 2001
From: Mark Ivanov <vegomdoster@gmail.com>
Date: Thu, 6 Aug 2026 09:57:46 +0500
Subject: [PATCH 0/6] usb: serial: ch341: small cleanups and fixes

This series makes a few small cleanups and fixes to the CH341 serial
driver:

  - migrate the control-out path to usb_control_msg_send() so both
    directions use the modern interface
  - check the return value of ch341_configure() in reset_resume()
  - encode the inverted bit-7 hardware difference as a quirk flag set
    at configure time instead of a version comparison in the baud-rate
    setup path
  - replace the obsolete speed_t typedef with unsigned int
  - only update the cached baud rate when the register write succeeds,
    keeping the software state consistent with the hardware
  - annotate the break_end accesses with READ/WRITE_ONCE

All changes were tested on real hardware (1a86:7523): TX/RX echo at
9600 and 115200 baud, DTR/RTS toggling, and the divisor calculation was
verified for a range of line speeds.

Mark Ivanov (6):
  usb: serial: ch341: use usb_control_msg_send()
  usb: serial: ch341: check ch341_configure() in reset_resume
  usb: serial: ch341: encode inverted bit-7 as a quirk flag
  usb: serial: ch341: use unsigned int instead of speed_t
  usb: serial: ch341: only update cached baud rate on success
  usb: serial: ch341: mark break_end accesses with READ/WRITE_ONCE

 ch341.c | 54 ++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 34 insertions(+), 20 deletions(-)

-- 
2.55.0


[-- Attachment #3: 0002-usb-serial-ch341-check-ch341_configure-in-reset_resu.patch --]
[-- Type: text/x-patch, Size: 1082 bytes --]

From 5c6b978bdc8aff977935e310093bc6f495ec00d9 Mon Sep 17 00:00:00 2001
From: Mark Ivanov <vegomdoster@gmail.com>
Date: Thu, 6 Aug 2026 09:55:44 +0500
Subject: [PATCH 2/6] usb: serial: ch341: check ch341_configure() in
 reset_resume

ch341_configure() can fail, but its return value was ignored during
reset_resume(), leaving the port possibly unconfigured after a bus
reset. Propagate any error to the caller.

Signed-off-by: Mark Ivanov <vegomdoster@gmail.com>
---
 ch341.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ch341.c b/ch341.c
index d3a91f7..2d5e398 100644
--- a/ch341.c
+++ b/ch341.c
@@ -843,7 +843,12 @@ static int ch341_reset_resume(struct usb_serial *serial)
 		return 0;
 
 	/* reconfigure ch341 serial port after bus-reset */
-	ch341_configure(serial->dev, priv);
+	ret = ch341_configure(serial->dev, priv);
+	if (ret) {
+		dev_err(&port->dev, "failed to reconfigure after bus reset: %d\n",
+			ret);
+		return ret;
+	}
 
 	if (tty_port_initialized(&port->port)) {
 		ret = usb_submit_urb(port->interrupt_in_urb, GFP_NOIO);
-- 
2.55.0


[-- Attachment #4: 0003-usb-serial-ch341-encode-inverted-bit-7-as-a-quirk-fl.patch --]
[-- Type: text/x-patch, Size: 2214 bytes --]

From 988fdf85547cd1a4f1ec368ca380e1903d6cee00 Mon Sep 17 00:00:00 2001
From: Mark Ivanov <vegomdoster@gmail.com>
Date: Thu, 6 Aug 2026 09:56:50 +0500
Subject: [PATCH 3/6] usb: serial: ch341: encode inverted bit-7 as a quirk flag

The decision to set bit 7 of the divisor register was made at baud-rate
setup time based on the chip version. Move it into a quirk flag that is
computed once during ch341_configure(), alongside the other device
quirks, so the hardware difference is captured in one place and the
version field only tracks what was read from the device.

Signed-off-by: Mark Ivanov <vegomdoster@gmail.com>
---
 ch341.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/ch341.c b/ch341.c
index 2d5e398..cbd7433 100644
--- a/ch341.c
+++ b/ch341.c
@@ -83,6 +83,7 @@
 
 #define CH341_QUIRK_LIMITED_PRESCALER	BIT(0)
 #define CH341_QUIRK_SIMULATE_BREAK	BIT(1)
+#define CH341_QUIRK_INVERTED_BIT7	BIT(2)
 
 static const struct usb_device_id id_table[] = {
 	{ USB_DEVICE(0x1a86, 0x5523) },
@@ -257,12 +258,10 @@ static int ch341_set_baudrate_lcr(struct usb_device *dev,
 
 	/*
 	 * CH341A buffers data until a full endpoint-size packet (32 bytes)
-	 * has been received unless bit 7 is set.
-	 *
-	 * At least one device with version 0x27 appears to have this bit
-	 * inverted.
+	 * has been received unless bit 7 is set. At least one device with
+	 * version 0x27 appears to have this bit inverted.
 	 */
-	if (priv->version > 0x27)
+	if (!(priv->quirks & CH341_QUIRK_INVERTED_BIT7))
 		val |= BIT(7);
 
 	r = ch341_control_out(dev, CH341_REQ_WRITE_REG,
@@ -327,6 +326,15 @@ static int ch341_configure(struct usb_device *dev, struct ch341_private *priv)
 	priv->version = buffer[0];
 	dev_dbg(&dev->dev, "Chip version: 0x%02x\n", priv->version);
 
+	/*
+	 * CH341A buffers data until a full endpoint-size packet (32 bytes)
+	 * has been received unless bit 7 is set. At least one device with
+	 * version 0x27 appears to have this bit inverted, so it must not be
+	 * set on those devices.
+	 */
+	if (priv->version <= 0x27)
+		priv->quirks |= CH341_QUIRK_INVERTED_BIT7;
+
 	r = ch341_control_out(dev, CH341_REQ_SERIAL_INIT, 0, 0);
 	if (r < 0)
 		return r;
-- 
2.55.0


[-- Attachment #5: 0004-usb-serial-ch341-use-unsigned-int-instead-of-speed_t.patch --]
[-- Type: text/x-patch, Size: 1602 bytes --]

From 9949b4b7eada0eea92bee3e8e2e584dca2d67934 Mon Sep 17 00:00:00 2001
From: Mark Ivanov <vegomdoster@gmail.com>
Date: Thu, 6 Aug 2026 09:57:07 +0500
Subject: [PATCH 4/6] usb: serial: ch341: use unsigned int instead of speed_t

speed_t is an obsolete typedef for unsigned int. Use unsigned int
directly for the baud-rate parameters and the table of minimum rates.

Signed-off-by: Mark Ivanov <vegomdoster@gmail.com>
---
 ch341.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ch341.c b/ch341.c
index cbd7433..af5d412 100644
--- a/ch341.c
+++ b/ch341.c
@@ -157,7 +157,7 @@ static int ch341_control_in(struct usb_device *dev,
 #define CH341_CLK_DIV(ps, fact)	(1 << (12 - 3 * (ps) - (fact)))
 #define CH341_MIN_RATE(ps)	(CH341_CLKRATE / (CH341_CLK_DIV((ps), 1) * 512))
 
-static const speed_t ch341_min_rates[] = {
+static const unsigned int ch341_min_rates[] = {
 	CH341_MIN_RATE(0),
 	CH341_MIN_RATE(1),
 	CH341_MIN_RATE(2),
@@ -178,7 +178,7 @@ static const speed_t ch341_min_rates[] = {
  *		2 <= div <= 256 if fact = 0, or
  *		9 <= div <= 256 if fact = 1
  */
-static int ch341_get_divisor(struct ch341_private *priv, speed_t speed)
+static int ch341_get_divisor(struct ch341_private *priv, unsigned int speed)
 {
 	unsigned int fact, div, clk_div;
 	bool force_fact0 = false;
@@ -244,7 +244,7 @@ static int ch341_get_divisor(struct ch341_private *priv, speed_t speed)
 
 static int ch341_set_baudrate_lcr(struct usb_device *dev,
 				  struct ch341_private *priv,
-				  speed_t baud_rate, u8 lcr)
+				  unsigned int baud_rate, u8 lcr)
 {
 	int val;
 	int r;
-- 
2.55.0


[-- Attachment #6: 0001-usb-serial-ch341-use-usb_control_msg_send.patch --]
[-- Type: text/x-patch, Size: 1263 bytes --]

From c14a1786ed5c60804cc90d17ee0b3b13e2d219aa Mon Sep 17 00:00:00 2001
From: Mark Ivanov <vegomdoster@gmail.com>
Date: Thu, 6 Aug 2026 09:55:37 +0500
Subject: [PATCH 1/6] usb: serial: ch341: use usb_control_msg_send()

The ch341_control_out() function still uses the older usb_control_msg()
interface while the read side already uses usb_control_msg_recv().
Migrate the write path to usb_control_msg_send() for consistency.

Signed-off-by: Mark Ivanov <vegomdoster@gmail.com>
---
 ch341.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/ch341.c b/ch341.c
index 569f4ae..d3a91f7 100644
--- a/ch341.c
+++ b/ch341.c
@@ -120,10 +120,11 @@ static int ch341_control_out(struct usb_device *dev, u8 request,
 	dev_dbg(&dev->dev, "%s - (%02x,%04x,%04x)\n", __func__,
 		request, value, index);
 
-	r = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request,
-			    USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
-			    value, index, NULL, 0, DEFAULT_TIMEOUT);
-	if (r < 0)
+	r = usb_control_msg_send(dev, 0, request,
+				 USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+				 value, index, NULL, 0, DEFAULT_TIMEOUT,
+				 GFP_KERNEL);
+	if (r)
 		dev_err(&dev->dev, "failed to send control message: %d\n", r);
 
 	return r;
-- 
2.55.0


[-- Attachment #7: 0005-usb-serial-ch341-only-update-cached-baud-rate-on-suc.patch --]
[-- Type: text/x-patch, Size: 1273 bytes --]

From ffc93748483576c89c6c013a9f32313d62b55230 Mon Sep 17 00:00:00 2001
From: Mark Ivanov <vegomdoster@gmail.com>
Date: Thu, 6 Aug 2026 09:57:18 +0500
Subject: [PATCH 5/6] usb: serial: ch341: only update cached baud rate on
 success

priv->baud_rate was updated before the register write, so a failed
write left the software state inconsistent with the hardware (the
cached rate no longer matching what is programmed). Update the cached
value only when the write succeeds; on failure the termios is restored
from old_termios when available.

Signed-off-by: Mark Ivanov <vegomdoster@gmail.com>
---
 ch341.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ch341.c b/ch341.c
index af5d412..c419061 100644
--- a/ch341.c
+++ b/ch341.c
@@ -561,14 +561,12 @@ static void ch341_set_termios(struct tty_struct *tty,
 		lcr |= CH341_LCR_STOP_BITS_2;
 
 	if (baud_rate) {
-		priv->baud_rate = baud_rate;
-
 		r = ch341_set_baudrate_lcr(port->serial->dev, priv,
-					   priv->baud_rate, lcr);
+					   baud_rate, lcr);
 		if (r < 0 && old_termios) {
-			priv->baud_rate = tty_termios_baud_rate(old_termios);
 			tty_termios_copy_hw(&tty->termios, old_termios);
 		} else if (r == 0) {
+			priv->baud_rate = baud_rate;
 			priv->lcr = lcr;
 		}
 	}
-- 
2.55.0


[-- Attachment #8: 0006-usb-serial-ch341-mark-break_end-accesses-with-READ-W.patch --]
[-- Type: text/x-patch, Size: 1372 bytes --]

From 7a0ddb9a49ad152efe8ae9171a92249343f68c7c Mon Sep 17 00:00:00 2001
From: Mark Ivanov <vegomdoster@gmail.com>
Date: Thu, 6 Aug 2026 09:57:29 +0500
Subject: [PATCH 6/6] usb: serial: ch341: mark break_end accesses with
 READ/WRITE_ONCE

Annotate the break_end field with WRITE_ONCE/READ_ONCE so the accesses
are well-defined for static analysis and any future use from another
context.

Signed-off-by: Mark Ivanov <vegomdoster@gmail.com>
---
 ch341.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ch341.c b/ch341.c
index c419061..fb60e8d 100644
--- a/ch341.c
+++ b/ch341.c
@@ -632,7 +632,8 @@ static int ch341_simulate_break(struct tty_struct *tty, int break_state)
 		 *
 		 * 11 bits = 1 start, 8 data, 1 stop, 1 margin
 		 */
-		priv->break_end = jiffies + (11 * HZ / CH341_MIN_BPS);
+		WRITE_ONCE(priv->break_end,
+			   jiffies + (11 * HZ / CH341_MIN_BPS));
 
 		return 0;
 	}
@@ -641,9 +642,10 @@ static int ch341_simulate_break(struct tty_struct *tty, int break_state)
 
 	now = jiffies;
 
-	if (time_before(now, priv->break_end)) {
+	delay = READ_ONCE(priv->break_end);
+	if (time_before(now, delay)) {
 		/* Wait until NUL byte is written */
-		delay = priv->break_end - now;
+		delay -= now;
 		dev_dbg(&port->dev,
 			"wait %d ms while transmitting NUL byte at %u baud\n",
 			jiffies_to_msecs(delay), CH341_MIN_BPS);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH 0/6] usb: serial: ch341: small cleanups and fixes
  2026-08-06  5:02 [PATCH 0/6] usb: serial: ch341: small cleanups and fixes Vegom Doster
@ 2026-08-06  5:29 ` Greg KH
  0 siblings, 0 replies; 2+ messages in thread
From: Greg KH @ 2026-08-06  5:29 UTC (permalink / raw)
  To: Vegom Doster; +Cc: linux-usb, johan

On Thu, Aug 06, 2026 at 10:02:25AM +0500, Vegom Doster wrote:
> This series makes a few small cleanups and fixes to the CH341 serial
> driver:
> 
>   - migrate the control-out path to usb_control_msg_send() so both
>     directions use the modern interface
>   - check the return value of ch341_configure() in reset_resume()
>   - encode the inverted bit-7 hardware difference as a quirk flag set
>     at configure time instead of a version comparison in the baud-rate
>     setup path
>   - replace the obsolete speed_t typedef with unsigned int
>   - only update the cached baud rate when the register write succeeds,
>     keeping the software state consistent with the hardware
>   - annotate the break_end accesses with READ/WRITE_ONCE
> 
> All changes were tested on real hardware (1a86:7523): TX/RX echo at
> 9600 and 115200 baud, DTR/RTS toggling, and the divisor calculation was
> verified for a range of line speeds.
> 
> Signed-off-by: Mark Ivanov <vegomdoster@gmail.com>

Please slow down.  This needs to be sent as a patch series, not all in
one email, as the documentation asks.  Also, don't send patches more
than once a day at the most.

And finally, these are not in the correct style, and can not be applied.
Please take a look at the kernel documentation for how to do this in the
proper format.  If you have any questions, please let us know.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-06  5:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06  5:02 [PATCH 0/6] usb: serial: ch341: small cleanups and fixes Vegom Doster
2026-08-06  5:29 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox