serial: Fix support for UPF_SPD_* flags

PATCH In review Pali Rohár 3 participants 17 messages Mar 21, 2022 – Jul 8, 2022

Subsystems (18)

Arm Qualcomm mailing list Arm i.MX and Layerscape Arm MediaTek TTY layer and serial drivers +14 more subsystems

lore

Share & tools

lore

Pali Rohár

author maintainer

2022-03-21 16:31 UTC #0 lore raw

cover serial: Fix support for UPF_SPD_* flags

Support for UPF_SPD_* flags is currently broken in more drivers for two reasons. First one is that uart_update_timeout() function does not calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that userspace termios structre is modified by most drivers after each TIOCSSERIAL or TCSETS call to discard activation of UPF_SPD_* flags.

Reproducer for these issues:

  #include <termios.h>
  #include <sys/ioctl.h>
  #include <asm/ioctls.h>
  #include <linux/serial.h>

  static unsigned cdiv(unsigned a, unsigned b) {
    if (!b) return b;
    return (a + (b/2)) / b;
  }

  void set_active_spd_cust_baud(int fd, unsigned baudrate) {
    struct serial_struct serial;
    struct termios termios;
    tcgetattr(fd, &termios);
    ioctl(fd, TIOCGSERIAL, &serial);
    serial.flags &= ~ASYNC_SPD_MASK;
    serial.flags |= ASYNC_SPD_CUST;
    serial.custom_divisor = cdiv(serial.baud_base, baudrate);
    ioctl(fd, TIOCSSERIAL, &serial);
    cfsetspeed(&termios, B38400);
    tcsetattr(fd, TCSANOW, &termios);
  }

  int is_spd_cust_active(int fd) {
    struct serial_struct serial;
    struct termios termios;
    tcgetattr(fd, &termios);
    ioctl(fd, TIOCGSERIAL, &serial);
    return
      (serial.flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST &&
      cfgetospeed(&termios) == B38400;
  }

(error handling was ommited for simplification)

After calling set_active_spd_cust_baud() function SPD custom divisor should be active and therefore is_spd_cust_active() should return true.

But it is not active (cfgetospeed does not return B38400) and this patch series should fix it. I have tested it with 8250 driver.

Originally Johan Hovold reported that there may be issue with these ASYNC_SPD_FLAGS in email:
https://lore.kernel.org/linux-serial/[email protected]/

Johan, Greg, could you please test these patches if there is not any regression?

Pali Rohár (3):
serial: core: Document why UPF_SPD_CUST is not handled in
uart_get_baud_rate()
serial: core: Fix function uart_update_timeout() to handle
UPF_SPD_CUST flag
serial: Fix support for UPF_SPD_* flags in serial drivers

 drivers/tty/serial/21285.c          |  2 +-
 drivers/tty/serial/8250/8250_mtk.c  |  2 +-
 drivers/tty/serial/8250/8250_omap.c |  2 +-
 drivers/tty/serial/8250/8250_port.c |  2 +-
 drivers/tty/serial/altera_uart.c    |  2 +-
 drivers/tty/serial/ar933x_uart.c    |  2 +-
 drivers/tty/serial/arc_uart.c       |  2 +-
 drivers/tty/serial/dz.c             |  2 +-
 drivers/tty/serial/imx.c            |  3 +-
 drivers/tty/serial/lantiq.c         |  2 +-
 drivers/tty/serial/lpc32xx_hs.c     |  2 +-
 drivers/tty/serial/men_z135_uart.c  |  2 +-
 drivers/tty/serial/mps2-uart.c      |  2 +-
 drivers/tty/serial/msm_serial.c     |  2 +-
 drivers/tty/serial/mvebu-uart.c     |  2 +-
 drivers/tty/serial/owl-uart.c       |  2 +-
 drivers/tty/serial/pch_uart.c       |  2 +-
 drivers/tty/serial/pic32_uart.c     |  2 +-
 drivers/tty/serial/rda-uart.c       |  2 +-
 drivers/tty/serial/rp2.c            |  2 +-
 drivers/tty/serial/sccnxp.c         |  2 +-
 drivers/tty/serial/serial-tegra.c   |  2 +-
 drivers/tty/serial/serial_core.c    | 76 ++++++++++++++++++++++++++++-
 drivers/tty/serial/sprd_serial.c    |  2 +-
 drivers/tty/serial/timbuart.c       |  2 +-
 drivers/tty/serial/vt8500_serial.c  |  2 +-
 drivers/tty/serial/xilinx_uartps.c  |  2 +-
 include/linux/serial_core.h         |  2 +
 28 files changed, 103 insertions(+), 28 deletions(-)

Signature · 1 line

2.20.1

Pali Rohár

author maintainer

2022-03-21 16:31 UTC #1 lore raw

PATCH 1/3 serial: core: Document why UPF_SPD_CUST is not handled in uart_get_baud_rate()

Switch in uart_get_baud_rate() function is missing case for UPF_SPD_CUST flag. It is not obvious why it is missing here, so add comments explaining how deprecated UPF_SPD_CUST flag is handled and how drivers should call uart_get_baud_rate() and uart_get_divisor() functions.

Signed-off-by Pali Rohár <[email protected]>

---
 drivers/tty/serial/serial_core.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

drivers/tty/serial/serial_core.c +14 -1

index 1e738f265eea..d8fc2616d62b 100644
@@ -364,7 +364,17 @@ EXPORT_SYMBOL(uart_update_timeout);
  *
  *	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 baud.
+ *	flags, except cust), and mapping the %B0 rate to 9600 baud.
+ *
+ *	UPF_SPD_CUST flag is not handled in this function as it applies
+ *	to the custom divisor. When UPF_SPD_CUST flag is active and in
+ *	use then this function returns value 38400 and not the correct
+ *	baud rate.
+ *
+ *	Drivers should call uart_get_divisor() function with baud rate
+ *	returned from this function to calculate clock divisor. Function
+ *	uart_get_divisor() then handles UPF_SPD_CUST flag with magic
+ *	baud rate value 38400 and returns the correct custom divisor.
  *
  *	If the new baud rate is invalid, try the old termios setting.
  *	If it's still invalid, we try 9600 baud.

@@ -396,6 +406,7 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
 	case UPF_SPD_WARP:
 		altbaud = 460800;
 		break;
+	/* Flag UPF_SPD_CUST is not handed here, see description why. */
 	default:
 		altbaud = 38400;
 		break;

@@ -462,6 +473,8 @@ EXPORT_SYMBOL(uart_get_baud_rate);
  *	@baud: desired baud rate
  *
  *	Calculate the uart clock divisor for the port.
+ *
+ *	Handles also special case when UPF_SPD_CUST flag is in use.
  */
 unsigned int
 uart_get_divisor(struct uart_port *port, unsigned int baud)

Signature · 1 line

2.20.1

Pali Rohár

author maintainer

2022-03-21 16:31 UTC #2 lore raw

PATCH 2/3 serial: core: Fix function uart_update_timeout() to handle UPF_SPD_CUST flag

In reply to Pali Rohár

Function uart_update_timeout() currently handles UPF_SPD_HI, UPF_SPD_VHI, UPF_SPD_SHI and UPF_SPD_WARP flags thanks to how uart_get_baud_rate() works and how should be used. uart_get_baud_rate() already translates these 4 special flags to real baud rate value and therefore uart_update_timeout() is called with the correct baud rate argument.

What is not handled in this function is the last remaining flag UPF_SPD_CUST. It is because uart_get_baud_rate() returns hardcoded value 38400 when UPF_SPD_CUST is set and in use.

Implement support for UPF_SPD_CUST in uart_update_timeout(), so port->timeout is calculated also when UPF_SPD_CUST flag is set.

For calculation use base uart clock and custom divisor. This calculation does not have to be precise for all UART hardware but this the best approximation which can be done. It is for sure better than hardcoded baud rate value 38400.

Signed-off-by Pali Rohár <[email protected]>

---
 drivers/tty/serial/serial_core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

drivers/tty/serial/serial_core.c +10 -0

index d8fc2616d62b..34e085a038fe 100644
@@ -343,6 +343,16 @@ uart_update_timeout(struct uart_port *port, unsigned int cflag,
 {
 	unsigned int size;
 
+	/*
+	 * Old custom speed handling. See function uart_get_divisor()
+	 * and description in uart_get_baud_rate() function.
+	 * This calculation does not have to be precise for all UART
+	 * hardware but it is the best approximation which we can do here
+	 * as we do not know the exact baud rate.
+	 */
+	if (baud == 38400 && (port->flags & UPF_SPD_MASK) == UPF_SPD_CUST)
+		baud = DIV_ROUND_CLOSEST(port->uartclk, 16 * port->custom_divisor);
+
 	size = tty_get_frame_size(cflag) * port->fifosize;
 
 	/*

Signature · 1 line

2.20.1

Pali Rohár

author maintainer

2022-03-21 16:31 UTC #3 lore raw

PATCH 3/3 serial: Fix support for UPF_SPD_* flags in serial drivers

In reply to Pali Rohár

Most serial drivers do not handle UPF_SPD_* flags correctly. They use uart_get_baud_rate() and uart_get_divisor() functions for retrieving baud rate and divisor which correctly handle UPF_SPD_* flags and set correct value to HW. But drivers do not propagate correct value to termios structure as they call just tty_termios_encode_baud_rate() function which completely ignores these UPF_SPD_* flags. So termios structure reported to userspace does not match to UPF_SPD_* flags which were used for configuring HW and also does not match ASYNC_SDP_* flags stored in serial_struct which are reported to userspace.

Fix this issue by introducing a new function uart_set_baud_rate() which is wrapper around tty_termios_encode_baud_rate() and which handles those UPF_SPD_* flags correctly.

Replace all calls of tty_termios_encode_baud_rate() function which take an argument from uart_get_baud_rate() function by this new function uart_set_baud_rate().

This ensures that serial drivers which are using uart_get_baud_rate() will correctly handle UPF_SPD_* flags.

Signed-off-by Pali Rohár <[email protected]>

---
 drivers/tty/serial/21285.c          |  2 +-
 drivers/tty/serial/8250/8250_mtk.c  |  2 +-
 drivers/tty/serial/8250/8250_omap.c |  2 +-
 drivers/tty/serial/8250/8250_port.c |  2 +-
 drivers/tty/serial/altera_uart.c    |  2 +-
 drivers/tty/serial/ar933x_uart.c    |  2 +-
 drivers/tty/serial/arc_uart.c       |  2 +-
 drivers/tty/serial/dz.c             |  2 +-
 drivers/tty/serial/imx.c            |  3 +-
 drivers/tty/serial/lantiq.c         |  2 +-
 drivers/tty/serial/lpc32xx_hs.c     |  2 +-
 drivers/tty/serial/men_z135_uart.c  |  2 +-
 drivers/tty/serial/mps2-uart.c      |  2 +-
 drivers/tty/serial/msm_serial.c     |  2 +-
 drivers/tty/serial/mvebu-uart.c     |  2 +-
 drivers/tty/serial/owl-uart.c       |  2 +-
 drivers/tty/serial/pch_uart.c       |  2 +-
 drivers/tty/serial/pic32_uart.c     |  2 +-
 drivers/tty/serial/rda-uart.c       |  2 +-
 drivers/tty/serial/rp2.c            |  2 +-
 drivers/tty/serial/sccnxp.c         |  2 +-
 drivers/tty/serial/serial-tegra.c   |  2 +-
 drivers/tty/serial/serial_core.c    | 51 +++++++++++++++++++++++++++++
 drivers/tty/serial/sprd_serial.c    |  2 +-
 drivers/tty/serial/timbuart.c       |  2 +-
 drivers/tty/serial/vt8500_serial.c  |  2 +-
 drivers/tty/serial/xilinx_uartps.c  |  2 +-
 include/linux/serial_core.h         |  2 ++
 28 files changed, 79 insertions(+), 27 deletions(-)

drivers/tty/serial/21285.c +1 -1

index 09baef4ccc39..4e4dbd58c581 100644
@@ -265,7 +265,7 @@ serial21285_set_termios(struct uart_port *port, struct ktermios *termios,
 	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/16); 
 	quot = uart_get_divisor(port, baud);
 	b = port->uartclk / (16 * quot);
-	tty_termios_encode_baud_rate(termios, b, b);
+	uart_set_baud_rate(port, termios, b);
 
 	switch (termios->c_cflag & CSIZE) {
 	case CS5:

drivers/tty/serial/8250/8250_mtk.c +1 -1

index fb65dc601b23..4a46a7f039d0 100644
@@ -406,7 +406,7 @@ mtk8250_set_termios(struct uart_port *port, struct ktermios *termios,
 	spin_unlock_irqrestore(&port->lock, flags);
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 }
 
 static int __maybe_unused mtk8250_runtime_suspend(struct device *dev)

drivers/tty/serial/8250/8250_omap.c +1 -1

index 73e5f1dbd075..96c1df9ffd25 100644
@@ -510,7 +510,7 @@ static void omap_8250_set_termios(struct uart_port *port,
 
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 }
 
 /* same as 8250 except that we may have extra flow bits set in EFR */

drivers/tty/serial/8250/8250_port.c +1 -1

index 5775cbff8f6e..05ca0152b932 100644
@@ -2878,7 +2878,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 }
 EXPORT_SYMBOL(serial8250_do_set_termios);

drivers/tty/serial/altera_uart.c +1 -1

index 7c5f4e966b59..ec352d53662a 100644
@@ -185,7 +185,7 @@ static void altera_uart_set_termios(struct uart_port *port,
 
 	if (old)
 		tty_termios_copy_hw(termios, old);
-	tty_termios_encode_baud_rate(termios, baud, baud);
+	uart_set_baud_rate(port, termios, baud);
 
 	spin_lock_irqsave(&port->lock, flags);
 	uart_update_timeout(port, termios->c_cflag, baud);

drivers/tty/serial/ar933x_uart.c +1 -1

index 4379ca4842ae..48e8127d0e9d 100644
@@ -355,7 +355,7 @@ static void ar933x_uart_set_termios(struct uart_port *port,
 	spin_unlock_irqrestore(&up->port.lock, flags);
 
 	if (tty_termios_baud_rate(new))
-		tty_termios_encode_baud_rate(new, baud, baud);
+		uart_set_baud_rate(port, new, baud);
 }
 
 static void ar933x_uart_rx_chars(struct ar933x_uart_port *up)

drivers/tty/serial/arc_uart.c +1 -1

index 596217d10d5c..cc2a06832699 100644
@@ -391,7 +391,7 @@ arc_serial_set_termios(struct uart_port *port, struct ktermios *new,
 
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(new))
-		tty_termios_encode_baud_rate(new, baud, baud);
+		uart_set_baud_rate(port, new, baud);
 
 	uart_update_timeout(port, new->c_cflag, baud);

drivers/tty/serial/dz.c +1 -1

index e9edabc5a211..8084966ba9f9 100644
@@ -599,7 +599,7 @@ static void dz_set_termios(struct uart_port *uport, struct ktermios *termios,
 			baud = 9600;
 			bflag = DZ_B9600;
 		}
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(uport, termios, baud);
 	}
 	cflag |= bflag;

drivers/tty/serial/imx.c +1 -2

index 90f82e6c54e4..b56344b46459 100644
@@ -1729,8 +1729,7 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	tdiv64 = sport->port.uartclk;
 	tdiv64 *= num;
 	do_div(tdiv64, denom * 16 * div);
-	tty_termios_encode_baud_rate(termios,
-				(speed_t)tdiv64, (speed_t)tdiv64);
+	uart_set_baud_rate(port, termios, (speed_t)tdiv64);
 
 	num -= 1;
 	denom -= 1;

drivers/tty/serial/lantiq.c +1 -1

index 497b334bc845..38b2f7fa80ff 100644
@@ -502,7 +502,7 @@ lqasc_set_termios(struct uart_port *port,
 
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(new))
-		tty_termios_encode_baud_rate(new, baud, baud);
+		uart_set_baud_rate(port, new, baud);
 
 	uart_update_timeout(port, cflag, baud);
 }

drivers/tty/serial/lpc32xx_hs.c +1 -1

index b199d7859961..e6f9166be0c7 100644
@@ -530,7 +530,7 @@ static void serial_lpc32xx_set_termios(struct uart_port *port,
 
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 }
 
 static const char *serial_lpc32xx_type(struct uart_port *port)

drivers/tty/serial/men_z135_uart.c +1 -1

index 9acae5f8fc32..e3a83bca9991 100644
@@ -713,7 +713,7 @@ static void men_z135_set_termios(struct uart_port *port,
 
 	spin_lock_irq(&port->lock);
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 
 	bd_reg = uart_freq / (4 * baud);
 	iowrite32(bd_reg, port->membase + MEN_Z135_BAUD_REG);

drivers/tty/serial/mps2-uart.c +1 -1

index 587b42f754cb..117a7afcd088 100644
@@ -383,7 +383,7 @@ mps2_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	spin_unlock_irqrestore(&port->lock, flags);
 
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 }
 
 static const char *mps2_uart_type(struct uart_port *port)

drivers/tty/serial/msm_serial.c +1 -1

index fcef7a961430..789dfeb286bf 100644
@@ -1262,7 +1262,7 @@ static void msm_set_termios(struct uart_port *port, struct ktermios *termios,
 	baud = uart_get_baud_rate(port, termios, old, 300, 4000000);
 	baud = msm_set_baud_rate(port, baud, &flags);
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 
 	/* calculate parity */
 	mr = msm_read(port, UART_MR2);

drivers/tty/serial/mvebu-uart.c +1 -1

index ab226da75f7b..7bd9579855fa 100644
@@ -515,7 +515,7 @@ static void mvebu_uart_set_termios(struct uart_port *port,
 			baud = uart_get_baud_rate(port, old, NULL,
 						  min_baud, max_baud);
 	} else {
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 		uart_update_timeout(port, termios->c_cflag, baud);
 	}

drivers/tty/serial/owl-uart.c +1 -1

index 91f1eb0058d7..8d70cdbd4a73 100644
@@ -387,7 +387,7 @@ static void owl_uart_set_termios(struct uart_port *port,
 
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 
 	port->read_status_mask |= OWL_UART_STAT_RXER;
 	if (termios->c_iflag & INPCK)

drivers/tty/serial/pch_uart.c +1 -1

index f0351e6f0ef6..53f2e9c09d8d 100644
@@ -1412,7 +1412,7 @@ static void pch_uart_set_termios(struct uart_port *port,
 	pch_uart_set_mctrl(&priv->port, priv->port.mctrl);
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 
 out:
 	spin_unlock(&port->lock);

drivers/tty/serial/pic32_uart.c +1 -1

index 0a12fb11e698..286045456a1c 100644
@@ -596,7 +596,7 @@ static void pic32_uart_set_termios(struct uart_port *port,
 	uart_update_timeout(port, new->c_cflag, baud);
 
 	if (tty_termios_baud_rate(new))
-		tty_termios_encode_baud_rate(new, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 
 	/* enable uart */
 	pic32_uart_en_and_unmask(port);

drivers/tty/serial/rda-uart.c +1 -1

index d550d8fa2fab..c08c597cda86 100644
@@ -318,7 +318,7 @@ static void rda_uart_set_termios(struct uart_port *port,
 
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 
 	/* update the per-port timeout */
 	uart_update_timeout(port, termios->c_cflag, baud);

drivers/tty/serial/rp2.c +1 -1

index 6689d8add8f7..a12e9a97905b 100644
@@ -382,7 +382,7 @@ static void rp2_uart_set_termios(struct uart_port *port,
 	baud_div = uart_get_divisor(port, baud);
 
 	if (tty_termios_baud_rate(new))
-		tty_termios_encode_baud_rate(new, baud, baud);
+		uart_set_baud_rate(port, new, baud);
 
 	spin_lock_irqsave(&port->lock, flags);

drivers/tty/serial/sccnxp.c +1 -1

index 10cc16a71f26..141dc0ebb688 100644
@@ -714,7 +714,7 @@ static void sccnxp_set_termios(struct uart_port *port,
 
 	/* Report actual baudrate back to core */
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 
 	/* Enable RX & TX */
 	sccnxp_port_write(port, SCCNXP_CR_REG, CR_RX_ENABLE | CR_TX_ENABLE);

drivers/tty/serial/serial-tegra.c +1 -1

index 45e2e4109acd..876aa57dbcd0 100644
@@ -1360,7 +1360,7 @@ static void tegra_uart_set_termios(struct uart_port *u,
 		return;
 	}
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(u, termios, baud);
 	spin_lock_irqsave(&u->lock, flags);
 
 	/* Flow control */

drivers/tty/serial/serial_core.c +51 -0

57 lines not on this page · raw message

drivers/tty/serial/sprd_serial.c +1 -1

index 9a7ae6384edf..17b0cadd31e6 100644
@@ -867,7 +867,7 @@ static void sprd_set_termios(struct uart_port *port,
 
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 }
 
 static const char *sprd_type(struct uart_port *port)

drivers/tty/serial/timbuart.c +1 -1

index 08941eabe7b1..c41377d218e7 100644
@@ -294,7 +294,7 @@ static void timbuart_set_termios(struct uart_port *port,
 	   up initially */
 	if (old)
 		tty_termios_copy_hw(termios, old);
-	tty_termios_encode_baud_rate(termios, baud, baud);
+	uart_set_baud_rate(port, termios, baud);
 
 	spin_lock_irqsave(&port->lock, flags);
 	iowrite8((u8)bindex, port->membase + TIMBUART_BAUDRATE);

drivers/tty/serial/vt8500_serial.c +1 -1

index e15b2bf69904..2e5d9b52e478 100644
@@ -369,7 +369,7 @@ static void vt8500_set_termios(struct uart_port *port,
 	baud = uart_get_baud_rate(port, termios, old, 900, 921600);
 	baud = vt8500_set_baud_rate(port, baud);
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 
 	/* calculate parity */
 	lcr = vt8500_read(&vt8500_port->uart, VT8500_URLCR);

drivers/tty/serial/xilinx_uartps.c +1 -1

index d5e243908d9f..300108c9ba7e 100644
@@ -714,7 +714,7 @@ static void cdns_uart_set_termios(struct uart_port *port,
 	baud = uart_get_baud_rate(port, termios, old, minbaud, maxbaud);
 	baud = cdns_uart_set_baud_rate(port, baud);
 	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_set_baud_rate(port, termios, baud);
 
 	/* Update the per-port timeout. */
 	uart_update_timeout(port, termios->c_cflag, baud);

include/linux/serial_core.h +2 -0

index c58cc142d23f..a0f736be5645 100644
@@ -330,6 +330,8 @@ void uart_update_timeout(struct uart_port *port, unsigned int cflag,
 unsigned int uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
 				struct ktermios *old, unsigned int min,
 				unsigned int max);
+void uart_set_baud_rate(struct uart_port *port, struct ktermios *termios,
+			unsigned int baud);
 unsigned int uart_get_divisor(struct uart_port *port, unsigned int baud);
 
 /* Base timer interval for polling */

Signature · 1 line

2.20.1

Andy Shevchenko

2022-03-22 14:30 UTC #4 lore raw

In reply to Pali Rohár

Quoted 3 lines · Pali Rohár

Support for UPF_SPD_* flags is currently broken in more drivers for two reasons. First one is that uart_update_timeout() function does not

the uart_update_timeout()

Quoted 2 lines

calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that userspace termios structre is modified by most drivers after each

structure

...

Quoted 1 line

(error handling was ommited for simplification)

omitted

Quoted 5 lines

After calling set_active_spd_cust_baud() function SPD custom divisor should be active and therefore is_spd_cust_active() should return true.
But it is not active (cfgetospeed does not return B38400) and this patch series should fix it. I have tested it with 8250 driver.

drivers

Quoted 7 lines

Originally Johan Hovold reported that there may be issue with these ASYNC_SPD_FLAGS in email:
https://lore.kernel.org/linux-serial/[email protected]/
Johan, Greg, could you please test these patches if there is not any regression?

I'm wondering why we are still supporting this ugly hack? Doesn't BOTHER work for you?

I would rather expect to have this removed completely.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Pali Rohár

author maintainer

2022-03-22 18:54 UTC #5 lore raw

Quoted 31 lines · On Tuesday 22 March 2022 16:29:08 Andy Shevchenko wrote:

Quoted 3 lines · Pali Rohár
Support for UPF_SPD_* flags is currently broken in more drivers for two reasons. First one is that uart_update_timeout() function does not
the uart_update_timeout()
Quoted 2 lines
calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that userspace termios structre is modified by most drivers after each
structure
...
Quoted 1 line
(error handling was ommited for simplification)
omitted
Quoted 5 lines
After calling set_active_spd_cust_baud() function SPD custom divisor should be active and therefore is_spd_cust_active() should return true.
But it is not active (cfgetospeed does not return B38400) and this patch series should fix it. I have tested it with 8250 driver.
drivers
Quoted 5 lines
Originally Johan Hovold reported that there may be issue with these ASYNC_SPD_FLAGS in email:
https://lore.kernel.org/linux-serial/[email protected]/

Quoted 5 lines

Quoted 2 lines
Johan, Greg, could you please test these patches if there is not any regression?
I'm wondering why we are still supporting this ugly hack? Doesn't BOTHER work for you?

Johan pointed in above mentioned patch that it would break ASYNC_SPD_FLAGS. So I have looked at how are ASYNC_SPD_FLAGS implemented to ensure that they would work correctly...

Quoted 1 line

I would rather expect to have this removed completely.

Well, if somebody is going to remove it, I have no objections. But I understood that ASYNC_SPD_FLAGS should be still supported...

Greg Kroah-Hartman

maintainer

2022-04-22 14:28 UTC #6 lore raw

In reply to Andy Shevchenko

Quoted 31 lines · Andy Shevchenko

Quoted 3 lines · Pali Rohár
Support for UPF_SPD_* flags is currently broken in more drivers for two reasons. First one is that uart_update_timeout() function does not
the uart_update_timeout()
Quoted 2 lines
calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that userspace termios structre is modified by most drivers after each
structure
...
Quoted 1 line
(error handling was ommited for simplification)
omitted
Quoted 5 lines
After calling set_active_spd_cust_baud() function SPD custom divisor should be active and therefore is_spd_cust_active() should return true.
But it is not active (cfgetospeed does not return B38400) and this patch series should fix it. I have tested it with 8250 driver.
drivers
Quoted 5 lines
Originally Johan Hovold reported that there may be issue with these ASYNC_SPD_FLAGS in email:
https://lore.kernel.org/linux-serial/[email protected]/

Quoted 5 lines

Quoted 2 lines
Johan, Greg, could you please test these patches if there is not any regression?
I'm wondering why we are still supporting this ugly hack? Doesn't BOTHER work for you?

Yes, I too do not want to add more support for these old flags. If they have not been working, let's not add support for them as obviously no one is using them. Let's try to remove them if at all possible.

thanks,

greg k-h

Pali Rohár

author maintainer

2022-07-07 08:48 UTC #7 lore raw

Quoted 35 lines · On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote:

Quoted 34 lines · Andy Shevchenko
Quoted 3 lines · Pali Rohár
Support for UPF_SPD_* flags is currently broken in more drivers for two reasons. First one is that uart_update_timeout() function does not
the uart_update_timeout()
Quoted 2 lines
calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that userspace termios structre is modified by most drivers after each
structure
...
Quoted 1 line
(error handling was ommited for simplification)
omitted
Quoted 5 lines
After calling set_active_spd_cust_baud() function SPD custom divisor should be active and therefore is_spd_cust_active() should return true.
But it is not active (cfgetospeed does not return B38400) and this patch series should fix it. I have tested it with 8250 driver.
drivers
Quoted 7 lines
Originally Johan Hovold reported that there may be issue with these ASYNC_SPD_FLAGS in email:
https://lore.kernel.org/linux-serial/[email protected]/
Johan, Greg, could you please test these patches if there is not any regression?

Quoted 6 lines

Quoted 2 lines
I'm wondering why we are still supporting this ugly hack? Doesn't BOTHER work for you?
Yes, I too do not want to add more support for these old flags. If they have not been working, let's not add support for them as obviously no one is using them. Let's try to remove them if at all possible.

Well, it works partially. For more drivers SET method is working, but GET method returns incorrect value. If your userspace application is written in a way that does not retrieve from kernel current settings then it has big probability that application works.

So, do you really want to remove support for these old flags completely? That would of course break above applications.

Note that usage of BOTHER is problematic and in most cases highly impossible if you are using glibc libc.so. BOTHER is incompatible with glibc header files and so you can either include BOTHER/linux termios file (exclusive) OR glibc header files.

New version of tcsetattr and ioctl_tty manpages would have documented how to use BOTHER (it is currently in the manpages git).

Currently the only known option how to use BOTHER is to completely reimplement all functions from "termios.h", provide custom "termios.h" header file (and not use glibc termios.h nor any file which it includes) and statically link this reimplementation into final application.

So in most cases BOTHER is not alternative to those old SPD flags even for modern applications.

Quoted 3 lines

thanks,
greg k-h

Greg Kroah-Hartman

maintainer

2022-07-08 13:07 UTC #8 lore raw

Quoted 39 lines · Pali Rohár

Quoted 38 lines · On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote:
Quoted 36 lines · Andy Shevchenko
Quoted 3 lines · Pali Rohár
Support for UPF_SPD_* flags is currently broken in more drivers for two reasons. First one is that uart_update_timeout() function does not
the uart_update_timeout()
Quoted 2 lines
calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that userspace termios structre is modified by most drivers after each
structure
...
Quoted 1 line
(error handling was ommited for simplification)
omitted
Quoted 5 lines
After calling set_active_spd_cust_baud() function SPD custom divisor should be active and therefore is_spd_cust_active() should return true.
But it is not active (cfgetospeed does not return B38400) and this patch series should fix it. I have tested it with 8250 driver.
drivers
Quoted 7 lines
Originally Johan Hovold reported that there may be issue with these ASYNC_SPD_FLAGS in email:
https://lore.kernel.org/linux-serial/[email protected]/
Johan, Greg, could you please test these patches if there is not any regression?
I'm wondering why we are still supporting this ugly hack? Doesn't BOTHER work for you?

Quoted 8 lines

Quoted 3 lines
Yes, I too do not want to add more support for these old flags. If they have not been working, let's not add support for them as obviously no one is using them. Let's try to remove them if at all possible.
Well, it works partially. For more drivers SET method is working, but GET method returns incorrect value. If your userspace application is written in a way that does not retrieve from kernel current settings then it has big probability that application works.

I do not understand, sorry, what do you mean by this?

And as you are responding to a months-old thread, I am totally lost, and don't even know what the patch here was...

Quoted 2 lines

So, do you really want to remove support for these old flags completely? That would of course break above applications.

I'm not saying remove them, I'm saying let us not add any more dependancies on them in order to keep new applications from ever wanting to use them.

Quoted 4 lines

Note that usage of BOTHER is problematic and in most cases highly impossible if you are using glibc libc.so. BOTHER is incompatible with glibc header files and so you can either include BOTHER/linux termios file (exclusive) OR glibc header files.

I thought that was all fixed up in newer versions of glibc. Is that really still the case in the latest release?

Quoted 10 lines

New version of tcsetattr and ioctl_tty manpages would have documented how to use BOTHER (it is currently in the manpages git).
Currently the only known option how to use BOTHER is to completely reimplement all functions from "termios.h", provide custom "termios.h" header file (and not use glibc termios.h nor any file which it includes) and statically link this reimplementation into final application.
So in most cases BOTHER is not alternative to those old SPD flags even for modern applications.

Using the kernel's .h file should be easy enough here for them, right? And again, I thought glibc was fixed, what about other libc versions?

thanks,

greg k-h

Pali Rohár

author maintainer

2022-07-08 13:26 UTC #9 lore raw

Quoted 44 lines · On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote:

Quoted 43 lines · Pali Rohár
Quoted 41 lines · On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote:
Quoted 31 lines · Andy Shevchenko
Quoted 3 lines · Pali Rohár
Support for UPF_SPD_* flags is currently broken in more drivers for two reasons. First one is that uart_update_timeout() function does not
the uart_update_timeout()
Quoted 2 lines
calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that userspace termios structre is modified by most drivers after each
structure
...
Quoted 1 line
(error handling was ommited for simplification)
omitted
Quoted 5 lines
After calling set_active_spd_cust_baud() function SPD custom divisor should be active and therefore is_spd_cust_active() should return true.
But it is not active (cfgetospeed does not return B38400) and this patch series should fix it. I have tested it with 8250 driver.
drivers
Quoted 5 lines
Originally Johan Hovold reported that there may be issue with these ASYNC_SPD_FLAGS in email:
https://lore.kernel.org/linux-serial/[email protected]/
Quoted 5 lines
Quoted 2 lines
Johan, Greg, could you please test these patches if there is not any regression?
I'm wondering why we are still supporting this ugly hack? Doesn't BOTHER work for you?
Yes, I too do not want to add more support for these old flags. If they have not been working, let's not add support for them as obviously no one is using them. Let's try to remove them if at all possible.

Quoted 6 lines

Quoted 4 lines
Well, it works partially. For more drivers SET method is working, but GET method returns incorrect value. If your userspace application is written in a way that does not retrieve from kernel current settings then it has big probability that application works.
I do not understand, sorry, what do you mean by this?

I mean that SET methods are working, GET methods not. In this case SET done via ioctl(TIOCSSERIAL) and GET via ioctl(TIOCGSERIAL).

Quoted 9 lines

And as you are responding to a months-old thread, I am totally lost, and don't even know what the patch here was...
Quoted 2 lines
So, do you really want to remove support for these old flags completely? That would of course break above applications.
I'm not saying remove them, I'm saying let us not add any more dependancies on them in order to keep new applications from ever wanting to use them.

Last time you wrote to remove them. Now saying not to remove them. So I do not understand you now.

Quoted 7 lines

Quoted 4 lines
Note that usage of BOTHER is problematic and in most cases highly impossible if you are using glibc libc.so. BOTHER is incompatible with glibc header files and so you can either include BOTHER/linux termios file (exclusive) OR glibc header files.
I thought that was all fixed up in newer versions of glibc. Is that really still the case in the latest release?

In which version you though that it was fixed? Last time when I checked it (half year ago) all these issues were there still present. Now I briefly grepped glib git source code and seems that nothing was changed, no support for BOTHER and neither ABI changes. I also looked into changelong of last 3 released glibc versions there is neither no information about it.

Quoted 8 lines

Quoted 8 lines
New version of tcsetattr and ioctl_tty manpages would have documented how to use BOTHER (it is currently in the manpages git).
Currently the only known option how to use BOTHER is to completely reimplement all functions from "termios.h", provide custom "termios.h" header file (and not use glibc termios.h nor any file which it includes) and statically link this reimplementation into final application.

Quoted 4 lines

Quoted 2 lines
So in most cases BOTHER is not alternative to those old SPD flags even for modern applications.
Using the kernel's .h file should be easy enough here for them, right?

No. Because structures and defines conflicts with glibc structures and defines. That is why (ABI incompatible) reimplementation is needed.

Quoted 1 line

And again, I thought glibc was fixed, what about other libc versions?

musl does not support BOTHER too. And I do not know other libc versions which are used in production.

Quoted 3 lines

thanks,
greg k-h

Greg Kroah-Hartman

maintainer

2022-07-08 13:51 UTC #10 lore raw

Quoted 61 lines · Pali Rohár

Quoted 44 lines · On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote:
Quoted 43 lines · Pali Rohár
Quoted 41 lines · On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote:
Quoted 31 lines · Andy Shevchenko
Quoted 3 lines · Pali Rohár
Support for UPF_SPD_* flags is currently broken in more drivers for two reasons. First one is that uart_update_timeout() function does not
the uart_update_timeout()
Quoted 2 lines
calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that userspace termios structre is modified by most drivers after each
structure
...
Quoted 1 line
(error handling was ommited for simplification)
omitted
Quoted 5 lines
After calling set_active_spd_cust_baud() function SPD custom divisor should be active and therefore is_spd_cust_active() should return true.
But it is not active (cfgetospeed does not return B38400) and this patch series should fix it. I have tested it with 8250 driver.
drivers
Quoted 5 lines
Originally Johan Hovold reported that there may be issue with these ASYNC_SPD_FLAGS in email:
https://lore.kernel.org/linux-serial/[email protected]/
Quoted 5 lines
Quoted 2 lines
Johan, Greg, could you please test these patches if there is not any regression?
I'm wondering why we are still supporting this ugly hack? Doesn't BOTHER work for you?
Yes, I too do not want to add more support for these old flags. If they have not been working, let's not add support for them as obviously no one is using them. Let's try to remove them if at all possible.
Quoted 6 lines
Quoted 4 lines
Well, it works partially. For more drivers SET method is working, but GET method returns incorrect value. If your userspace application is written in a way that does not retrieve from kernel current settings then it has big probability that application works.
I do not understand, sorry, what do you mean by this?
I mean that SET methods are working, GET methods not. In this case SET done via ioctl(TIOCSSERIAL) and GET via ioctl(TIOCGSERIAL).
Quoted 6 lines
And as you are responding to a months-old thread, I am totally lost, and don't even know what the patch here was...
Quoted 2 lines
So, do you really want to remove support for these old flags completely? That would of course break above applications.

Quoted 6 lines

Quoted 3 lines
I'm not saying remove them, I'm saying let us not add any more dependancies on them in order to keep new applications from ever wanting to use them.
Last time you wrote to remove them. Now saying not to remove them. So I do not understand you now.

I'm sorry, I am totally lost. How about starting over and resubmitting the changes you want and we can go from there.

greg k-h

Pali Rohár

author maintainer

2022-07-08 14:20 UTC #11 lore raw

Quoted 62 lines · On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote:

Quoted 61 lines · Pali Rohár
Quoted 44 lines · On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote:
Quoted 43 lines · Pali Rohár
Quoted 41 lines · On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote:
Quoted 31 lines · Andy Shevchenko
Quoted 3 lines · Pali Rohár
Support for UPF_SPD_* flags is currently broken in more drivers for two reasons. First one is that uart_update_timeout() function does not
the uart_update_timeout()
Quoted 2 lines
calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that userspace termios structre is modified by most drivers after each
structure
...
Quoted 1 line
(error handling was ommited for simplification)
omitted
Quoted 5 lines
After calling set_active_spd_cust_baud() function SPD custom divisor should be active and therefore is_spd_cust_active() should return true.
But it is not active (cfgetospeed does not return B38400) and this patch series should fix it. I have tested it with 8250 driver.
drivers
Quoted 5 lines
Originally Johan Hovold reported that there may be issue with these ASYNC_SPD_FLAGS in email:
https://lore.kernel.org/linux-serial/[email protected]/
Quoted 5 lines
Quoted 2 lines
Johan, Greg, could you please test these patches if there is not any regression?
I'm wondering why we are still supporting this ugly hack? Doesn't BOTHER work for you?
Yes, I too do not want to add more support for these old flags. If they have not been working, let's not add support for them as obviously no one is using them. Let's try to remove them if at all possible.
Quoted 6 lines
Quoted 4 lines
Well, it works partially. For more drivers SET method is working, but GET method returns incorrect value. If your userspace application is written in a way that does not retrieve from kernel current settings then it has big probability that application works.
I do not understand, sorry, what do you mean by this?
I mean that SET methods are working, GET methods not. In this case SET done via ioctl(TIOCSSERIAL) and GET via ioctl(TIOCGSERIAL).
Quoted 6 lines
And as you are responding to a months-old thread, I am totally lost, and don't even know what the patch here was...
Quoted 2 lines
So, do you really want to remove support for these old flags completely? That would of course break above applications.

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
I'm not saying remove them, I'm saying let us not add any more dependancies on them in order to keep new applications from ever wanting to use them.
Last time you wrote to remove them. Now saying not to remove them. So I do not understand you now.
I'm sorry, I am totally lost.

So look what you have wrote? Who is lost here is me.

Quoted 4 lines

How about starting over and resubmitting
the changes you want and we can go from there.
greg k-h

What to resubmit? I do not understand you. In case you lost emails or accidentally removed them, you can look at them in archive, not? I hope that you do not want me to copy+paste all existing patches with all your quotes on them which you wrote into new emails.

Andy Shevchenko

2022-07-08 15:42 UTC #12 lore raw

Quoted 5 lines · Pali Rohár

Quoted 4 lines · On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote:
Quoted 3 lines · Pali Rohár
Quoted 2 lines · On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote:
Quoted 1 line · Pali Rohár
On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote:

...

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
I'm not saying remove them, I'm saying let us not add any more dependancies on them in order to keep new applications from ever wanting to use them.
Last time you wrote to remove them. Now saying not to remove them. So I do not understand you now.

There was a _new_ addition of the ugly SPD_CUST, that's what I believe Greg opposes to. And I support that.

Quoted 11 lines

Quoted 1 line
I'm sorry, I am totally lost.
So look what you have wrote? Who is lost here is me.
Quoted 2 lines
How about starting over and resubmitting
the changes you want and we can go from there.
What to resubmit? I do not understand you. In case you lost emails or accidentally removed them, you can look at them in archive, not? I hope that you do not want me to copy+paste all existing patches with all your quotes on them which you wrote into new emails.

That change that adds the new user of SPD_CUST?

In any case the best summary about BOTHER I ever read is this [1] (and an initial steps in picocom [2]). And I believe that instead of SPD_CUST we should get rid (or at least minimize) the problems with BOTHER in user space.

[1]: https://github.com/npat-efault/picocom/blob/master/termios2.txt
[2]: https://github.com/jmesmon/picocom/issues/2

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Pali Rohár

author maintainer

2022-07-08 15:54 UTC #13 lore raw

Quoted 13 lines · On Friday 08 July 2022 17:42:03 Andy Shevchenko wrote:

Quoted 5 lines · Pali Rohár
Quoted 4 lines · On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote:
Quoted 3 lines · Pali Rohár
Quoted 2 lines · On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote:
Quoted 1 line · Pali Rohár
On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote:
...
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
I'm not saying remove them, I'm saying let us not add any more dependancies on them in order to keep new applications from ever wanting to use them.

Quoted 5 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Last time you wrote to remove them. Now saying not to remove them. So I do not understand you now.
There was a _new_ addition of the ugly SPD_CUST, that's what I believe Greg opposes to. And I support that.

Which addition? I do not understand you. There was not any new driver with introduction of SPD support.

Quoted 7 lines

Quoted 7 lines
Quoted 1 line
I'm sorry, I am totally lost.
So look what you have wrote? Who is lost here is me.
Quoted 2 lines
How about starting over and resubmitting
the changes you want and we can go from there.

Quoted 6 lines

Quoted 4 lines
What to resubmit? I do not understand you. In case you lost emails or accidentally removed them, you can look at them in archive, not? I hope that you do not want me to copy+paste all existing patches with all your quotes on them which you wrote into new emails.
That change that adds the new user of SPD_CUST?

What you are talking about? Which user?

Quoted 2 lines

In any case the best summary about BOTHER I ever read is this [1] (and an initial steps in picocom [2]).

Is not that example in manpage enough?

Quoted 3 lines

And I believe that instead of
SPD_CUST we should get rid (or at least minimize) the problems with BOTHER in user space.

I looked into archives and seems that glibc people are not interested in this area. And I'm not going to spend time on another project which seems to be useless.

Quoted 8 lines

[1]: https://github.com/npat-efault/picocom/blob/master/termios2.txt
[2]: https://github.com/jmesmon/picocom/issues/2
Signature · 2 lines
With Best Regards,
Andy Shevchenko

Andy Shevchenko

2022-07-08 16:09 UTC #14 lore raw

Quoted 14 lines · Pali Rohár

Quoted 13 lines · On Friday 08 July 2022 17:42:03 Andy Shevchenko wrote:
Quoted 5 lines · Pali Rohár
Quoted 4 lines · On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote:
Quoted 3 lines · Pali Rohár
Quoted 2 lines · On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote:
Quoted 1 line · Pali Rohár
On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote:
...
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
I'm not saying remove them, I'm saying let us not add any more dependancies on them in order to keep new applications from ever wanting to use them.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Last time you wrote to remove them. Now saying not to remove them. So I do not understand you now.
There was a _new_ addition of the ugly SPD_CUST, that's what I believe Greg opposes to. And I support that.
Which addition? I do not understand you. There was not any new driver with introduction of SPD support.

You stated that SPD_CUST is broken in some drivers, so you are trying to fix a broken ugly hack. Why? Instead of making it rot and be removed eventually, you pump life into frankenstein.

Quoted 7 lines

Quoted 7 lines
Quoted 7 lines
Quoted 1 line
I'm sorry, I am totally lost.
So look what you have wrote? Who is lost here is me.
Quoted 2 lines
How about starting over and resubmitting
the changes you want and we can go from there.

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
What to resubmit? I do not understand you. In case you lost emails or accidentally removed them, you can look at them in archive, not? I hope that you do not want me to copy+paste all existing patches with all your quotes on them which you wrote into new emails.
That change that adds the new user of SPD_CUST?
What you are talking about? Which user?

This I missed, I was thinking that you are talking about a new user, now I read briefly and it seems that it's about an existing user. Anyway, that change I suppose.

Quoted 4 lines

Quoted 2 lines
In any case the best summary about BOTHER I ever read is this [1] (and an initial steps in picocom [2]).
Is not that example in manpage enough?

Dunno.
Can you point it out to me? I can't find it quickly.

Quoted 7 lines

Quoted 3 lines
And I believe that instead of
SPD_CUST we should get rid (or at least minimize) the problems with BOTHER in user space.
I looked into archives and seems that glibc people are not interested in this area. And I'm not going to spend time on another project which seems to be useless.

So why should the kernel suffer if it already provides something good for the user and user space ignores that?

Quoted 2 lines

Quoted 2 lines
[1]: https://github.com/npat-efault/picocom/blob/master/termios2.txt
[2]: https://github.com/jmesmon/picocom/issues/2

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Pali Rohár

author maintainer

2022-07-08 16:26 UTC #15 lore raw

Quoted 21 lines · On Friday 08 July 2022 18:09:07 Andy Shevchenko wrote:

Quoted 20 lines · Pali Rohár
Quoted 18 lines · On Friday 08 July 2022 17:42:03 Andy Shevchenko wrote:
Quoted 5 lines · Pali Rohár
Quoted 4 lines · On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote:
Quoted 3 lines · Pali Rohár
Quoted 2 lines · On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote:
Quoted 1 line · Pali Rohár
On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote:
...
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
I'm not saying remove them, I'm saying let us not add any more dependancies on them in order to keep new applications from ever wanting to use them.
Last time you wrote to remove them. Now saying not to remove them. So I do not understand you now.
There was a _new_ addition of the ugly SPD_CUST, that's what I believe Greg opposes to. And I support that.

Quoted 6 lines

Quoted 2 lines
Which addition? I do not understand you. There was not any new driver with introduction of SPD support.
You stated that SPD_CUST is broken in some drivers, so you are trying to fix a broken ugly hack. Why? Instead of making it rot and be removed eventually, you pump life into frankenstein.

Firstly I got rejection of my other patches because they does not handle SDP_CUST correctly. So I decided to look at those issues and fix it via helper function which can be easily reused in all drivers. So helper function wrap all "ugly" hacks. Then I got reaction that SDP should be removed. Then I got another reaction that that "I'm not saying to remove them" and another another reaction why to be removed eventually.

So how should I interpret this? I'm feeling that you are just trying to annoy people with this "do this", "do opposite", "do again it", "do again opposite"...

Quoted 20 lines

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
I'm sorry, I am totally lost.
So look what you have wrote? Who is lost here is me.
Quoted 2 lines
How about starting over and resubmitting
the changes you want and we can go from there.
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
What to resubmit? I do not understand you. In case you lost emails or accidentally removed them, you can look at them in archive, not? I hope that you do not want me to copy+paste all existing patches with all your quotes on them which you wrote into new emails.
That change that adds the new user of SPD_CUST?
What you are talking about? Which user?
This I missed, I was thinking that you are talking about a new user, now I read briefly and it seems that it's about an existing user. Anyway, that change I suppose.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
In any case the best summary about BOTHER I ever read is this [1] (and an initial steps in picocom [2]).
Is not that example in manpage enough?
Dunno.
Can you point it out to me? I can't find it quickly.

Argh... Have you read emails to which you wrote reply? So copy+paste relevant part from my previous email just for you:

"New version of tcsetattr and ioctl_tty manpages would have documented
how to use BOTHER (it is currently in the manpages git)."

Plus in past I also pointed to the extended version of that example from manpage which is currently in my repo on github:
https://github.com/pali/linux-baudrate.git

Quoted 10 lines

Quoted 7 lines
Quoted 3 lines
And I believe that instead of
SPD_CUST we should get rid (or at least minimize) the problems with BOTHER in user space.
I looked into archives and seems that glibc people are not interested in this area. And I'm not going to spend time on another project which seems to be useless.
So why should the kernel suffer if it already provides something good for the user and user space ignores that?

Because it is unusable? API which standard linux userspace applications cannot use is useless. And for application develop it does not matter if issue is in kernel part of API or userspace part of API. At the end would be use used.

With whole this discussion I have feeling that there correct way is just to use SDP flags in userspace as there is no interest in fixing BOTHER's c_ospeed and c_ispeed in kernel drivers and it was rejected just because of not handling SDP flags correctly.

Quoted 6 lines

Quoted 2 lines
Quoted 2 lines
[1]: https://github.com/npat-efault/picocom/blob/master/termios2.txt
[2]: https://github.com/jmesmon/picocom/issues/2
Signature · 2 lines
With Best Regards,
Andy Shevchenko

Andy Shevchenko

2022-07-08 17:42 UTC #16 lore raw

Quoted 37 lines · Pali Rohár

Quoted 21 lines · On Friday 08 July 2022 18:09:07 Andy Shevchenko wrote:
Quoted 20 lines · Pali Rohár
Quoted 18 lines · On Friday 08 July 2022 17:42:03 Andy Shevchenko wrote:
Quoted 5 lines · Pali Rohár
Quoted 4 lines · On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote:
Quoted 3 lines · Pali Rohár
Quoted 2 lines · On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote:
Quoted 1 line · Pali Rohár
On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote:
...
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
I'm not saying remove them, I'm saying let us not add any more dependancies on them in order to keep new applications from ever wanting to use them.
Last time you wrote to remove them. Now saying not to remove them. So I do not understand you now.
There was a _new_ addition of the ugly SPD_CUST, that's what I believe Greg opposes to. And I support that.
Quoted 6 lines
Quoted 2 lines
Which addition? I do not understand you. There was not any new driver with introduction of SPD support.
You stated that SPD_CUST is broken in some drivers, so you are trying to fix a broken ugly hack. Why? Instead of making it rot and be removed eventually, you pump life into frankenstein.
Firstly I got rejection of my other patches because they does not handle SDP_CUST correctly. So I decided to look at those issues and fix it via helper function which can be easily reused in all drivers. So helper function wrap all "ugly" hacks. Then I got reaction that SDP should be removed. Then I got another reaction that that "I'm not saying to remove them" and another another reaction why to be removed eventually.

Quoted 3 lines

So how should I interpret this? I'm feeling that you are just trying to annoy people with this "do this", "do opposite", "do again it", "do again opposite"...

Ask someone who makes a decision. I wrote just my p.o.v. on the "problem". I think there is no problem with SPD_CUST, it should be oblivionized.

Quoted 25 lines

Quoted 25 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
I'm sorry, I am totally lost.
So look what you have wrote? Who is lost here is me.
Quoted 2 lines
How about starting over and resubmitting
the changes you want and we can go from there.
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
What to resubmit? I do not understand you. In case you lost emails or accidentally removed them, you can look at them in archive, not? I hope that you do not want me to copy+paste all existing patches with all your quotes on them which you wrote into new emails.
That change that adds the new user of SPD_CUST?
What you are talking about? Which user?
This I missed, I was thinking that you are talking about a new user, now I read briefly and it seems that it's about an existing user. Anyway, that change I suppose.
Quoted 4 lines
Quoted 2 lines
In any case the best summary about BOTHER I ever read is this [1] (and an initial steps in picocom [2]).
Is not that example in manpage enough?

Quoted 8 lines

Quoted 2 lines
Dunno.
Can you point it out to me? I can't find it quickly.
Argh... Have you read emails to which you wrote reply? So copy+paste relevant part from my previous email just for you:
"New version of tcsetattr and ioctl_tty manpages would have documented
how to use BOTHER (it is currently in the manpages git)."

I do not know the "manpages git" URL. Neither its hosting. kernel.org? And then? It took time for you to just write something instead of helping me. Whatever. I found the commits.

Quoted 12 lines

Plus in past I also pointed to the extended version of that example from manpage which is currently in my repo on github:
https://github.com/pali/linux-baudrate.git
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
And I believe that instead of
SPD_CUST we should get rid (or at least minimize) the problems with BOTHER in user space.
I looked into archives and seems that glibc people are not interested in this area. And I'm not going to spend time on another project which seems to be useless.

Quoted 7 lines

Quoted 2 lines
So why should the kernel suffer if it already provides something good for the user and user space ignores that?
Because it is unusable? API which standard linux userspace applications cannot use is useless. And for application develop it does not matter if issue is in kernel part of API or userspace part of API. At the end would be use used.

Then help make it happen?

Quoted 4 lines

With whole this discussion I have feeling that there correct way is just to use SDP flags in userspace as there is no interest in fixing BOTHER's c_ospeed and c_ispeed in kernel drivers and it was rejected just because of not handling SDP flags correctly.

I'm puzzled who asked you about SPD_CUST implementation... It.is.an.ugly.hack.

Signature · 2 lines

With Best Regards,
Andy Shevchenko