[PATCH v3 0/2] serial: core: Don't clobber the baud rate on hangup via B0

[PATCH v3 0/2] serial: core: Don't clobber the baud rate on hangup via B0

[Maciej W. Rozycki]

[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]

Greg,

 This is v3 of the change for device baud rate preservation where the B0 
rate has been requested for hangup that we discussed at LPC.

 Following the observations regarding v2 I have now excluded the 8250 
console restoration path from rewriting port->hup_speed; this path only 
takes the most recent baud rate previously set and reinstates it with 
hardware without changing the setting chosen.  See the individual patches 
for further details of the changes made.

 I have verified it with the system and driver originally affected, that 
is a Broadcom BCM91250A "SWARM" system and sb1250-duart driver, but now 
also an MTI Malta system and the 8250 driver, with both showing the same 
symptoms on real hardware (worth mentioning since Malta is commonly used 
emulated in QEMU); cf. SIBYTE_SWARM and MIPS_MALTA in arch/mips/Kconfig.  
I have retained the console output logs previously attached for the bad vs 
fixed case; output is virtually the same across both systems.

 All that needed to be done to reproduce it was to keep killing getty(8)s 
repeatedly until init(8) decided something was wrong and chose to put 
their inittab(5) entries into a snooze, leaving the line(s) at the wrong 
speed setting.  I have done further experimentation with actual kernel 
console output by issuing:

# echo h >/proc/sysrq-trigger

once the console port has been hung up and that obviously also produced 
corrupt output without the fix and correct output with the fix applied.

 I have no means to verify the serial8250_console_restore() path; the fix 
is expected to make no functional change there.

 Since the return path from uart_get_baud_rate() has become more complex 
now than just supplying the baud rate chosen to the caller I went back to 
the original idea of having this as a small patch series, where 1/2 merely 
merges the two successful return paths into one.

 NB this fixes the missing %B0 literal markup as a part of the doc update.  
I've concluded it's not worth splitting off into a separate patch.  Let me 
know if you think otherwise.

 Previous iterations:

- v2 at: <https://lore.kernel.org/r/alpine.DEB.2.21.2610120150230.55767@angie.orcam.me.uk/>.

- v1 at: <https://lore.kernel.org/r/alpine.DEB.2.21.2605242242330.1450@angie.orcam.me.uk/>.

  Maciej

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/plain; CHARSET=US-ASCII; NAME=swarm-bad.log, Size: 1526 bytes --]

^[[H^[[J\rswarm.orcam.me.uk login: \0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0ð\0þ\0\0ð\0\0ð\0þ\0þ\0\0ð\0\0ð\0þ\0\0\0À\0þ\0þ\0þ\0\0ð\0þ\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0þ\0\0ð\0\0ð\0þ\0\0\0À\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0ð\0\0\0À\0\0ð\0þ\0\0ð\0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0ð\0\0\0À\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0þ\0þ\0þ\0\0\0\0\0\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0\0ð\0\0ð\0\0\0À\0þ\0þ\0þ\0þ\0þ\0þ\0þ\0þ\0\0\0\0\0\0\0ð\0\0ð\0\0ð\0\0ð\0þ\0þ\0\0\0\0\0\0þ\0þ\0\0ð\0þ\0\0\0À\0þ\0þ\0\0ð\0þ\0\0ð\0\0\0\0\0\0\0ð\0\0ð\0\0\0À\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0\0À\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0\0\0À\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0þ\0\0ð\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0þ\0þ\0þ\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0þ\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0þ\0þ\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0\0\0\0\0\0ð\0þ\0\0\0\0\0\0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0ð\0þ\0\0ð\0\0ð\0þ\0þ\0\0ð\0\0ð\0þ\0\0\0À\0þ\0þ\0þ\0\0ð\0þ\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0þ\0\0ð\0\0ð\0þ\0\0\0À\0\0ð\0þ\0\0\0\0\0\0\0ð\0\0ð\0\0ð\0\0\0À\0\0ð\0þ\0\0ð\0þ\0þ\0þ\0\0\0À\0\0ð\0\0ð\0\0\0À\0\0ð\0\0\0\0\0\0\0ð\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0þ\0þ\0þ\0\0\0\0\0\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0\0À\0þ\0þ\0þ\0þ\0þ\0þ\0þ\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0ð\0\0ð\0þ\0þ\0\0\0\0\0\0þ\0þ\0\0ð\0þ\0\0\0À\0þ\0þ\0\0ð\0þ\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0\0\0À\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0\0À\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0\0\0À\0\0ð\0þ\0\0\0\0\0\0\0ð\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0þ\0\0ð\0\0ð\0þ\0\0\0\0\0\0\0ð\0\0ð\0þ\0þ\0þ\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0þ\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0þ\0þ\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0\0\0\0\0\0ð\0þ\0\0\0\0\0\0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0ð\0þ\0\0ð\0\0ð\0þ\0þ\0\0ð\0\0ð\0þ\0\0\0À\0þ\0þ\0þ\0\0ð\0þ\0\0ð\0\0\0\0\0\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0þ\0þ\0\0\0\0\0\0\0ø\0\0ð\0þ\0þ\0þ\0þ\0þ\0þ\0þ\0\0ð\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0\0\0\0\0þ\0þ\0\0ð\0\0ð\0þ\0þ\0þ\0þ\0þ\0\0\0À\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0\0ð\0\0ð\0þ\0\0\0À\0þ\0þ\0\0\0\0\0\0\0ð\0\0ð\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0\0\0\0\0\0\0ð\0\0ð\0\0\0À\0þ\0þ\0\0\0\0\0\0þ\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0\0ð\0þ\0þ\0þ\0þ\0\0ð\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0\0\0\0\0\0ð\0þ\0\0\0\0\0^[[H^[[J\rswarm.orcam.me.uk login: 

[-- Attachment #3: Type: text/plain, Size: 229 bytes --]

^[[H^[[Jswarm.orcam.me.uk login: INIT: Id "S0" respawning too fast: disabled for 5 minutes
INIT: Id "S1" respawning too fast: disabled for 5 minutes
INIT: no more processes left in this runlevel
^[[H^[[Jswarm.orcam.me.uk login: 

[PATCH v3 1/2] serial: core: Unify uart_get_baud_rate() success return paths

[Maciej W. Rozycki]

In preparation to adding extra return actions merge the two success 
return paths in uart_get_baud_rate() into one.

Since as from commit 091ea8e5d34e ("serial: core: prevent division by 
zero by always returning non-zero baud rate") it is now guaranteed that 
the B0 baud rate is within [min; max], deferring to the regular return 
path in the B0 case causes no functional change.

That is as long as max >= max(min, 9600), which I believe is always the 
case and none of UART hardware our drivers provide support for has its 
highest baud rate below 9600.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
New change in v3.
---
 drivers/tty/serial/serial_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

linux-serial-uart-get-baud-rate-return.diff
Index: linux-macro/drivers/tty/serial/serial_core.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/serial_core.c
+++ linux-macro/drivers/tty/serial/serial_core.c
@@ -515,7 +515,7 @@ uart_get_baud_rate(struct uart_port *por
 		 * Special case: B0 rate.
 		 */
 		if (baud == 0)
-			return max(min, 9600);
+			baud = max(min, 9600);
 
 		if (baud >= min && baud <= max)
 			return baud;

[PATCH v3 2/2] serial: core: Don't clobber the baud rate on hangup via B0

[Maciej W. Rozycki]

Requesting hangup via the B0 baud rate has this nasty side effect of 
clobbering the previously set rate for the port unless it happens to be 
9600bps or the port minimum rate if higher.  Consequently if there's a 
consumer still active at the other end, then it receives corrupt data 
from any other producers outputting messages to the port, such as 
init(8) or the kernel console.

There doesn't appear to be any particular reason for this clobbering 
other than that we have been doing it since 2.1.131, so take a saner 
approach and keep track of the most recently set baud rate and then use 
it whenever hangup has been requested.

There is one issue however with serial8250_console_restore(), possibly 
called from the NMI context on some platforms, in which case there is a 
concurrency issue owing to `port->mutex' not being held by the function.  
Consequently `port->hup_speed' could be written midway through an update 
being made by a termios call, causing the value saved to clobber the 
baud rate programmed into hardware with a later B0 rate selection.

Handle it by renaming uart_get_baud_rate() to uart_get_baud_rate_save() 
and giving the function an extra `save' parameter to control whether or 
not to have the baud rate saved in `port->hup_speed' that is returned by 
the function.  Then use that parameter to suppress saving when called by 
serial8250_console_restore(), observing that the call is only ever made 
to restore the baud rate most recently set.  Any concurrent interrupted 
termios call, once it has been passed control again, will then program 
hardware consistently with the value it may have written already or will 
otherwise write to `port->hup_speed'.

In the unlikely case `port->hup_speed' is zero where the B0 rate has 
been requested as the first one since the system boot for a non-console 
device, resort to the current default of max(min, 9600).

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
Changes from v2, 
<https://lore.kernel.org/r/alpine.DEB.2.21.2610120231550.55767@angie.orcam.me.uk/>:

- Add a `save' parameter to uart_get_baud_rate() and rename the function 
  to uart_get_baud_rate_save().

- Add a uart_get_baud_rate() static inline wrapper for regular cases in 
  drivers such as handling termios calls.

- Use the `save' parameter appropriately in serial8250_get_baud_rate() to 
  handle serial8250_console_restore() calls.

Changes from v1, 
<https://lore.kernel.org/r/alpine.DEB.2.21.2605242242330.1450@angie.orcam.me.uk/>:

- Cache the most recently set actual baud rate in port->hup_speed and use 
  it whenever the B0 rate has been requested since the old termios may be 
  NULL or have the B0 rate recorded.
---
 drivers/tty/serial/8250/8250_port.c |    9 +++++----
 drivers/tty/serial/serial_core.c    |   25 ++++++++++++++++---------
 include/linux/serial_core.h         |   28 +++++++++++++++++++++++++---
 3 files changed, 46 insertions(+), 16 deletions(-)

linux-serial-uart-get-baud-rate-hup.diff
Index: linux-macro/drivers/tty/serial/8250/8250_port.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/8250/8250_port.c
+++ linux-macro/drivers/tty/serial/8250/8250_port.c
@@ -2626,7 +2626,8 @@ static void serial8250_set_divisor(struc
 
 static unsigned int serial8250_get_baud_rate(struct uart_port *port,
 					     struct ktermios *termios,
-					     const struct ktermios *old)
+					     const struct ktermios *old,
+					     bool save)
 {
 	unsigned int tolerance = port->uartclk / 100;
 	unsigned int min;
@@ -2651,7 +2652,7 @@ static unsigned int serial8250_get_baud_
 	 * slower than nominal still match standard baud rates without
 	 * causing transmission errors.
 	 */
-	return uart_get_baud_rate(port, termios, old, min, max);
+	return uart_get_baud_rate_save(port, termios, old, min, max, save);
 }
 
 /*
@@ -2842,7 +2843,7 @@ serial8250_do_set_termios(struct uart_po
 
 	serial8250_set_mini(port, termios);
 	lcr = serial8250_compute_lcr(up, termios->c_cflag);
-	baud = serial8250_get_baud_rate(port, termios, old);
+	baud = serial8250_get_baud_rate(port, termios, old, true);
 	quot = serial8250_get_divisor(port, baud, &frac);
 
 	/*
@@ -3339,7 +3340,7 @@ static void serial8250_console_restore(s
 		termios.c_ospeed = port->state->port.tty->termios.c_ospeed;
 	}
 
-	baud = serial8250_get_baud_rate(port, &termios, NULL);
+	baud = serial8250_get_baud_rate(port, &termios, NULL, false);
 	quot = serial8250_get_divisor(port, baud, &frac);
 
 	serial8250_set_divisor(port, baud, quot, frac);
Index: linux-macro/drivers/tty/serial/serial_core.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/serial_core.c
+++ linux-macro/drivers/tty/serial/serial_core.c
@@ -454,29 +454,32 @@ uart_update_timeout(struct uart_port *po
 EXPORT_SYMBOL(uart_update_timeout);
 
 /**
- * uart_get_baud_rate - return baud rate for a particular port
+ * uart_get_baud_rate_save - return baud rate for a particular port
  * @port: uart_port structure describing the port in question.
  * @termios: desired termios settings
  * @old: old termios (or %NULL)
  * @min: minimum acceptable baud rate
  * @max: maximum acceptable baud rate
+ * @save: whether to save the baud rate chosen
  *
  * Decode the termios structure into a numeric baud rate, taking account of the
- * magic 38400 baud rate (with spd_* flags), and mapping the %B0 rate to 9600
+ * magic 38400 baud rate (with spd_* flags), and mapping the %B0 rate to the
+ * previously saved rate provided by @port, or if this is unset, then 9600
  * baud or min argument, whichever is greater.
  *
  * If the new baud rate is invalid, try the @old termios setting. If it's still
  * invalid, clip to the nearest chip supported rate.
  *
  * The @termios structure is updated to reflect the baud rate we're actually
- * going to be using. Don't do this for the case where B0 is requested ("hang
- * up").
+ * going to be using, except where %B0 is requested ("hang up"). Likewise the
+ * @port structure, except where @save is %false.
  *
  * Locking: caller dependent
  */
 unsigned int
-uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
-		   const struct ktermios *old, unsigned int min, unsigned int max)
+uart_get_baud_rate_save(struct uart_port *port,
+			struct ktermios *termios, const struct ktermios *old,
+			unsigned int min, unsigned int max, bool save)
 {
 	unsigned int try;
 	unsigned int baud;
@@ -515,10 +518,14 @@ uart_get_baud_rate(struct uart_port *por
 		 * Special case: B0 rate.
 		 */
 		if (baud == 0)
-			baud = max(min, 9600);
+			baud = (port->hup_speed ?
+				port->hup_speed : max(min, 9600));
 
-		if (baud >= min && baud <= max)
+		if (baud >= min && baud <= max) {
+			if (save)
+				port->hup_speed = baud;
 			return baud;
+		}
 
 		/*
 		 * If the range cannot be met then try again with
@@ -548,7 +555,7 @@ uart_get_baud_rate(struct uart_port *por
 	WARN_ON(1);
 	return 0;
 }
-EXPORT_SYMBOL(uart_get_baud_rate);
+EXPORT_SYMBOL(uart_get_baud_rate_save);
 
 /**
  * uart_get_divisor - return uart clock divisor
Index: linux-macro/include/linux/serial_core.h
===================================================================
--- linux-macro.orig/include/linux/serial_core.h
+++ linux-macro/include/linux/serial_core.h
@@ -573,6 +573,7 @@ struct uart_port {
 
 	bool			hw_stopped;		/* sw-assisted CTS flow state */
 	bool			cons_flow;		/* user specified console flow control */
+	speed_t			hup_speed;		/* speed to use for B0 rate (hang-up) */
 	unsigned int		mctrl;			/* current modem ctrl settings */
 	unsigned int		frame_time;		/* frame timing in ns */
 	unsigned int		type;			/* port type */
@@ -1036,11 +1037,32 @@ enum UART_TX_FLAGS {
  */
 void uart_update_timeout(struct uart_port *port, unsigned int cflag,
 			 unsigned int baud);
-unsigned int uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
-				const struct ktermios *old, unsigned int min,
-				unsigned int max);
+unsigned int uart_get_baud_rate_save(struct uart_port *port,
+				     struct ktermios *termios,
+				     const struct ktermios *old,
+				     unsigned int min, unsigned int max,
+				     bool save);
 unsigned int uart_get_divisor(struct uart_port *port, unsigned int baud);
 
+/**
+ * uart_get_baud_rate - return baud rate for a particular port
+ * @port: uart_port structure describing the port in question.
+ * @termios: desired termios settings
+ * @old: old termios (or %NULL)
+ * @min: minimum acceptable baud rate
+ * @max: maximum acceptable baud rate
+ *
+ * See uart_get_baud_rate_save() for more details.
+ */
+static inline unsigned int uart_get_baud_rate(struct uart_port *port,
+					      struct ktermios *termios,
+					      const struct ktermios *old,
+					      unsigned int min,
+					      unsigned int max)
+{
+	return uart_get_baud_rate_save(port, termios, old, min, max, true);
+}
+
 /*
  * Calculates FIFO drain time.
  */

Re: [PATCH v3 2/2] serial: core: Don't clobber the baud rate on hangup via B0

[Andy Shevchenko]

On Tue, Nov 03, 2026 at 10:07:58PM +0000, Maciej W. Rozycki wrote:
> Requesting hangup via the B0 baud rate has this nasty side effect of 
> clobbering the previously set rate for the port unless it happens to be 
> 9600bps or the port minimum rate if higher.  Consequently if there's a 
> consumer still active at the other end, then it receives corrupt data 
> from any other producers outputting messages to the port, such as 
> init(8) or the kernel console.
> 
> There doesn't appear to be any particular reason for this clobbering 
> other than that we have been doing it since 2.1.131, so take a saner 
> approach and keep track of the most recently set baud rate and then use 
> it whenever hangup has been requested.
> 
> There is one issue however with serial8250_console_restore(), possibly 
> called from the NMI context on some platforms, in which case there is a 
> concurrency issue owing to `port->mutex' not being held by the function.  
> Consequently `port->hup_speed' could be written midway through an update 
> being made by a termios call, causing the value saved to clobber the 
> baud rate programmed into hardware with a later B0 rate selection.
> 
> Handle it by renaming uart_get_baud_rate() to uart_get_baud_rate_save() 
> and giving the function an extra `save' parameter to control whether or 
> not to have the baud rate saved in `port->hup_speed' that is returned by 
> the function.  Then use that parameter to suppress saving when called by 
> serial8250_console_restore(), observing that the call is only ever made 
> to restore the baud rate most recently set.  Any concurrent interrupted 
> termios call, once it has been passed control again, will then program 
> hardware consistently with the value it may have written already or will 
> otherwise write to `port->hup_speed'.
> 
> In the unlikely case `port->hup_speed' is zero where the B0 rate has 
> been requested as the first one since the system boot for a non-console 
> device, resort to the current default of max(min, 9600).

All lines above (except the last ones in the paragraphs) have a single trailing
white space. Usually checkpatch warns at this.

>  /**
> - * uart_get_baud_rate - return baud rate for a particular port
> + * uart_get_baud_rate_save - return baud rate for a particular port

Now it also saves it, so

 * uart_get_baud_rate_save - return and save baud rate for a particular port

>   * @port: uart_port structure describing the port in question.
>   * @termios: desired termios settings
>   * @old: old termios (or %NULL)
>   * @min: minimum acceptable baud rate
>   * @max: maximum acceptable baud rate
> + * @save: whether to save the baud rate chosen
>   *
>   * Decode the termios structure into a numeric baud rate, taking account of the
> - * magic 38400 baud rate (with spd_* flags), and mapping the %B0 rate to 9600
> + * magic 38400 baud rate (with spd_* flags), and mapping the %B0 rate to the
> + * previously saved rate provided by @port, or if this is unset, then 9600
>   * baud or min argument, whichever is greater.
>   *
>   * If the new baud rate is invalid, try the @old termios setting. If it's still
>   * invalid, clip to the nearest chip supported rate.
>   *
>   * The @termios structure is updated to reflect the baud rate we're actually
> - * going to be using. Don't do this for the case where B0 is requested ("hang
> - * up").
> + * going to be using, except where %B0 is requested ("hang up"). Likewise the
> + * @port structure, except where @save is %false.
>   *
>   * Locking: caller dependent
>   */

...

>  		if (baud == 0)
> -			baud = max(min, 9600);
> +			baud = (port->hup_speed ?
> +				port->hup_speed : max(min, 9600));

Outer parentheses are redundant. Also use Elvis.

			baud = port->hup_speed ?: max(min, 9600);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/2] serial: core: Unify uart_get_baud_rate() success return paths

[Andy Shevchenko]

On Tue, Nov 03, 2026 at 10:07:44PM +0000, Maciej W. Rozycki wrote:
> In preparation to adding extra return actions merge the two success 
> return paths in uart_get_baud_rate() into one.
> 
> Since as from commit 091ea8e5d34e ("serial: core: prevent division by 
> zero by always returning non-zero baud rate") it is now guaranteed that 
> the B0 baud rate is within [min; max], deferring to the regular return 
> path in the B0 case causes no functional change.
> 
> That is as long as max >= max(min, 9600), which I believe is always the 
> case and none of UART hardware our drivers provide support for has its 
> highest baud rate below 9600.

Same issue with the commit message — too many trailing white spaces.
After fixing that, feel free to add
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] serial: core: Don't clobber the baud rate on hangup via B0

[Johan Hovold]

On Wed, Nov 04, 2026 at 01:10:18PM +0200, Andy Shevchenko wrote:
> On Tue, Nov 03, 2026 at 10:07:58PM +0000, Maciej W. Rozycki wrote:
 
> >  		if (baud == 0)
> > -			baud = max(min, 9600);
> > +			baud = (port->hup_speed ?
> > +				port->hup_speed : max(min, 9600));
> 
> Outer parentheses are redundant. Also use Elvis.
> 
> 			baud = port->hup_speed ?: max(min, 9600);

No, don't.

Don't use the ternary operator at all as it tends to make code less
readable.

Johan

Re: [PATCH v3 2/2] serial: core: Don't clobber the baud rate on hangup via B0

[Greg Kroah-Hartman]

On Wed, Nov 04, 2026 at 12:28:22PM +0100, Johan Hovold wrote:
> On Wed, Nov 04, 2026 at 01:10:18PM +0200, Andy Shevchenko wrote:
> > On Tue, Nov 03, 2026 at 10:07:58PM +0000, Maciej W. Rozycki wrote:
>  
> > >  		if (baud == 0)
> > > -			baud = max(min, 9600);
> > > +			baud = (port->hup_speed ?
> > > +				port->hup_speed : max(min, 9600));
> > 
> > Outer parentheses are redundant. Also use Elvis.
> > 
> > 			baud = port->hup_speed ?: max(min, 9600);
> 
> No, don't.
> 
> Don't use the ternary operator at all as it tends to make code less
> readable.

I strongly endorse this.  Write code for people first, compilers second.
? : is not friendly for people.

thanks,

greg k-h

Re: [PATCH v3 2/2] serial: core: Don't clobber the baud rate on hangup via B0

[Andy Shevchenko]

On Wed, Nov 04, 2026 at 12:28:22PM +0100, Johan Hovold wrote:
> On Wed, Nov 04, 2026 at 01:10:18PM +0200, Andy Shevchenko wrote:
> > On Tue, Nov 03, 2026 at 10:07:58PM +0000, Maciej W. Rozycki wrote:

...

> > > -			baud = max(min, 9600);
> > > +			baud = (port->hup_speed ?
> > > +				port->hup_speed : max(min, 9600));
> > 
> > Outer parentheses are redundant. Also use Elvis.
> > 
> > 			baud = port->hup_speed ?: max(min, 9600);
> 
> No, don't.
> 
> Don't use the ternary operator at all as it tends to make code less
> readable.

I disagree. Elvis is readable as in Python, for example, or a simple English.
"Assign foo to baz OR bar" <——> "foo = baz ?: bar".

-- 
With Best Regards,
Andy Shevchenko