USB: serial: ch341: fix simulated-break comment

On devices which do not support break signalling a break condition is simulated by sending a NUL byte at the lowest possible speed. The break condition will be 9 bit periods long (start bit and eight data bits), but the transmission itself also includes the stop bit.

---

Hi Michael,

I reread the break-end comment and found it a bit confusing still. The below seems more correct to me. I'm assuming you did not intend to add an additional bit period as margin?

Johan

 drivers/usb/serial/ch341.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

index 011d7953f087..27a2a62777c9 100644
@@ -604,9 +604,8 @@ static void ch341_simulate_break(struct tty_struct *tty, int break_state)
 		}
 
 		/*
-		 * Compute expected transmission duration and add a single bit
-		 * of safety margin (the actual NUL byte transmission is 8 bits
-		 * plus one stop bit).
+		 * Compute expected transmission duration (including stop
+		 * bit).
 		 */
 		priv->break_end = jiffies + (10 * HZ / CH341_MIN_BPS);
 

2.26.2

On devices which do not support break signalling a break condition is simulated by sending a NUL byte at the lowest possible speed. The break condition will be 9 bit periods long (start bit and eight data bits), but the transmission itself also includes the stop bit. The safety margin of one bit is kept to account for timing differences.

---

Hi Johan

The additional bit was intentional, but I missed the start bit and was off by one. As such your fix indeed addresses the inconsistency between the comment and code. Considering the general quality of the ch341 chips needing the simulation and to account for timing differences I'd instead prefer to increase the delay from 10 to 11 bits (1 start, 8 data, 1 stop, 1 margin).

Michael

 drivers/usb/serial/ch341.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

index 0cb02d1bde02..580aa5833091 100644
@@ -603,11 +603,13 @@ static void ch341_simulate_break(struct tty_struct *tty, int break_state)
 		}
 
 		/*
-		 * Compute expected transmission duration and add a single bit
-		 * of safety margin (the actual NUL byte transmission is 8 bits
-		 * plus one stop bit).
+		 * Compute expected transmission duration including safety
+		 * margin. The original baud rate is only restored after the
+		 * computed point in time.
+		 *
+		 * 11 bits = 1 start, 8 data, 1 stop, 1 margin
 		 */
-		priv->break_end = jiffies + (10 * HZ / CH341_MIN_BPS);
+		priv->break_end = jiffies + (11 * HZ / CH341_MIN_BPS);
 
 		return;
 	}

2.20.1

Thanks for confirming. I've applied this patch with a slightly modified commit message:

https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-next

Johan