* [PATCH 0/3] serial: Fix support for UPF_SPD_* flags
@ 2022-03-21 16:30 Pali Rohár
2022-03-21 16:30 ` [PATCH 1/3] serial: core: Document why UPF_SPD_CUST is not handled in uart_get_baud_rate() Pali Rohár
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Pali Rohár @ 2022-03-21 16:30 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Marek Behún
Cc: linux-serial, linux-kernel
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/20211007133146.28949-1-johan@kernel.org/
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(-)
--
2.20.1
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/3] serial: core: Document why UPF_SPD_CUST is not handled in uart_get_baud_rate() 2022-03-21 16:30 [PATCH 0/3] serial: Fix support for UPF_SPD_* flags Pali Rohár @ 2022-03-21 16:30 ` Pali Rohár 2022-03-21 16:30 ` [PATCH 2/3] serial: core: Fix function uart_update_timeout() to handle UPF_SPD_CUST flag Pali Rohár ` (2 subsequent siblings) 3 siblings, 0 replies; 17+ messages in thread From: Pali Rohár @ 2022-03-21 16:30 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Marek Behún Cc: linux-serial, linux-kernel 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 <pali@kernel.org> --- drivers/tty/serial/serial_core.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 1e738f265eea..d8fc2616d62b 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -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) -- 2.20.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] serial: core: Fix function uart_update_timeout() to handle UPF_SPD_CUST flag 2022-03-21 16:30 [PATCH 0/3] serial: Fix support for UPF_SPD_* flags Pali Rohár 2022-03-21 16:30 ` [PATCH 1/3] serial: core: Document why UPF_SPD_CUST is not handled in uart_get_baud_rate() Pali Rohár @ 2022-03-21 16:30 ` Pali Rohár 2022-03-21 16:30 ` [PATCH 3/3] serial: Fix support for UPF_SPD_* flags in serial drivers Pali Rohár 2022-03-22 14:29 ` [PATCH 0/3] serial: Fix support for UPF_SPD_* flags Andy Shevchenko 3 siblings, 0 replies; 17+ messages in thread From: Pali Rohár @ 2022-03-21 16:30 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Marek Behún Cc: linux-serial, linux-kernel 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 <pali@kernel.org> --- drivers/tty/serial/serial_core.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index d8fc2616d62b..34e085a038fe 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -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; /* -- 2.20.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] serial: Fix support for UPF_SPD_* flags in serial drivers 2022-03-21 16:30 [PATCH 0/3] serial: Fix support for UPF_SPD_* flags Pali Rohár 2022-03-21 16:30 ` [PATCH 1/3] serial: core: Document why UPF_SPD_CUST is not handled in uart_get_baud_rate() Pali Rohár 2022-03-21 16:30 ` [PATCH 2/3] serial: core: Fix function uart_update_timeout() to handle UPF_SPD_CUST flag Pali Rohár @ 2022-03-21 16:30 ` Pali Rohár 2022-03-22 14:29 ` [PATCH 0/3] serial: Fix support for UPF_SPD_* flags Andy Shevchenko 3 siblings, 0 replies; 17+ messages in thread From: Pali Rohár @ 2022-03-21 16:30 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Marek Behún Cc: linux-serial, linux-kernel 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 <pali@kernel.org> --- 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(-) diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c index 09baef4ccc39..4e4dbd58c581 100644 --- a/drivers/tty/serial/21285.c +++ b/drivers/tty/serial/21285.c @@ -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: diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c index fb65dc601b23..4a46a7f039d0 100644 --- a/drivers/tty/serial/8250/8250_mtk.c +++ b/drivers/tty/serial/8250/8250_mtk.c @@ -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) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 73e5f1dbd075..96c1df9ffd25 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -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 */ diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 5775cbff8f6e..05ca0152b932 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -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); diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c index 7c5f4e966b59..ec352d53662a 100644 --- a/drivers/tty/serial/altera_uart.c +++ b/drivers/tty/serial/altera_uart.c @@ -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); diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c index 4379ca4842ae..48e8127d0e9d 100644 --- a/drivers/tty/serial/ar933x_uart.c +++ b/drivers/tty/serial/ar933x_uart.c @@ -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) diff --git a/drivers/tty/serial/arc_uart.c b/drivers/tty/serial/arc_uart.c index 596217d10d5c..cc2a06832699 100644 --- a/drivers/tty/serial/arc_uart.c +++ b/drivers/tty/serial/arc_uart.c @@ -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); diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c index e9edabc5a211..8084966ba9f9 100644 --- a/drivers/tty/serial/dz.c +++ b/drivers/tty/serial/dz.c @@ -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; diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 90f82e6c54e4..b56344b46459 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -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; diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c index 497b334bc845..38b2f7fa80ff 100644 --- a/drivers/tty/serial/lantiq.c +++ b/drivers/tty/serial/lantiq.c @@ -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); } diff --git a/drivers/tty/serial/lpc32xx_hs.c b/drivers/tty/serial/lpc32xx_hs.c index b199d7859961..e6f9166be0c7 100644 --- a/drivers/tty/serial/lpc32xx_hs.c +++ b/drivers/tty/serial/lpc32xx_hs.c @@ -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) diff --git a/drivers/tty/serial/men_z135_uart.c b/drivers/tty/serial/men_z135_uart.c index 9acae5f8fc32..e3a83bca9991 100644 --- a/drivers/tty/serial/men_z135_uart.c +++ b/drivers/tty/serial/men_z135_uart.c @@ -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); diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c index 587b42f754cb..117a7afcd088 100644 --- a/drivers/tty/serial/mps2-uart.c +++ b/drivers/tty/serial/mps2-uart.c @@ -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) diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c index fcef7a961430..789dfeb286bf 100644 --- a/drivers/tty/serial/msm_serial.c +++ b/drivers/tty/serial/msm_serial.c @@ -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); diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c index ab226da75f7b..7bd9579855fa 100644 --- a/drivers/tty/serial/mvebu-uart.c +++ b/drivers/tty/serial/mvebu-uart.c @@ -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); } diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c index 91f1eb0058d7..8d70cdbd4a73 100644 --- a/drivers/tty/serial/owl-uart.c +++ b/drivers/tty/serial/owl-uart.c @@ -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) diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c index f0351e6f0ef6..53f2e9c09d8d 100644 --- a/drivers/tty/serial/pch_uart.c +++ b/drivers/tty/serial/pch_uart.c @@ -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); diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c index 0a12fb11e698..286045456a1c 100644 --- a/drivers/tty/serial/pic32_uart.c +++ b/drivers/tty/serial/pic32_uart.c @@ -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); diff --git a/drivers/tty/serial/rda-uart.c b/drivers/tty/serial/rda-uart.c index d550d8fa2fab..c08c597cda86 100644 --- a/drivers/tty/serial/rda-uart.c +++ b/drivers/tty/serial/rda-uart.c @@ -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); diff --git a/drivers/tty/serial/rp2.c b/drivers/tty/serial/rp2.c index 6689d8add8f7..a12e9a97905b 100644 --- a/drivers/tty/serial/rp2.c +++ b/drivers/tty/serial/rp2.c @@ -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); diff --git a/drivers/tty/serial/sccnxp.c b/drivers/tty/serial/sccnxp.c index 10cc16a71f26..141dc0ebb688 100644 --- a/drivers/tty/serial/sccnxp.c +++ b/drivers/tty/serial/sccnxp.c @@ -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); diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c index 45e2e4109acd..876aa57dbcd0 100644 --- a/drivers/tty/serial/serial-tegra.c +++ b/drivers/tty/serial/serial-tegra.c @@ -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 */ diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 34e085a038fe..113db02be87e 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -477,6 +477,57 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios, EXPORT_SYMBOL(uart_get_baud_rate); +void +uart_set_baud_rate(struct uart_port *port, struct ktermios *termios, unsigned int baud) +{ + upf_t flags = port->flags & UPF_SPD_MASK; + unsigned int close = baud / 50; + unsigned int altbaud = 0; + + switch (flags) { + case UPF_SPD_HI: + altbaud = 57600; + break; + case UPF_SPD_VHI: + altbaud = 115200; + break; + case UPF_SPD_SHI: + altbaud = 230400; + break; + case UPF_SPD_WARP: + altbaud = 460800; + break; + } + + /* + * UPF_SPD_* port flags are in use when B38400 is set in termios. + * Let termios baudrate set to B38400 value when new baudrate is + * in 2% tolerance (same tolerance as in tty_termios_encode_baud_rate). + * For UPF_SPD_CUST flag it is required to be this function called with + * baud = 38400 and then real baudrate depends only on custom_divisor. + */ + if (tty_termios_baud_rate(termios) == 38400) { + if (altbaud && baud - close >= altbaud && baud + close <= altbaud) { + altbaud = baud; + baud = 38400; + } else if (flags == UPF_SPD_CUST && baud == 38400) { + /* See uart_update_timeout() about this calculation */ + altbaud = DIV_ROUND_CLOSEST(port->uartclk, 16 * port->custom_divisor); + } + } + + tty_termios_encode_baud_rate(termios, baud, baud); + + /* + * If UPF_SPD_* port flags are active and in use then store into + * TCGETS2 c_*speed fields real baudrate. + */ + if (baud == 38400 && altbaud) + termios->c_ispeed = termios->c_ospeed = altbaud; +} + +EXPORT_SYMBOL(uart_set_baud_rate); + /** * uart_get_divisor - return uart clock divisor * @port: uart_port structure describing the port. diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c index 9a7ae6384edf..17b0cadd31e6 100644 --- a/drivers/tty/serial/sprd_serial.c +++ b/drivers/tty/serial/sprd_serial.c @@ -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) diff --git a/drivers/tty/serial/timbuart.c b/drivers/tty/serial/timbuart.c index 08941eabe7b1..c41377d218e7 100644 --- a/drivers/tty/serial/timbuart.c +++ b/drivers/tty/serial/timbuart.c @@ -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); diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c index e15b2bf69904..2e5d9b52e478 100644 --- a/drivers/tty/serial/vt8500_serial.c +++ b/drivers/tty/serial/vt8500_serial.c @@ -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); diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index d5e243908d9f..300108c9ba7e 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -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); diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h index c58cc142d23f..a0f736be5645 100644 --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -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 */ -- 2.20.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-03-21 16:30 [PATCH 0/3] serial: Fix support for UPF_SPD_* flags Pali Rohár ` (2 preceding siblings ...) 2022-03-21 16:30 ` [PATCH 3/3] serial: Fix support for UPF_SPD_* flags in serial drivers Pali Rohár @ 2022-03-22 14:29 ` Andy Shevchenko 2022-03-22 18:53 ` Pali Rohár 2022-04-22 14:28 ` Greg Kroah-Hartman 3 siblings, 2 replies; 17+ messages in thread From: Andy Shevchenko @ 2022-03-22 14:29 UTC (permalink / raw) To: Pali Rohár Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Mon, Mar 21, 2022 at 11:07 PM Pali Rohár <pali@kernel.org> wrote: > > 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() > calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that > userspace termios structre is modified by most drivers after each structure ... > (error handling was ommited for simplification) omitted > 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 > Originally Johan Hovold reported that there may be issue with these > ASYNC_SPD_FLAGS in email: > https://lore.kernel.org/linux-serial/20211007133146.28949-1-johan@kernel.org/ > > > 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. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-03-22 14:29 ` [PATCH 0/3] serial: Fix support for UPF_SPD_* flags Andy Shevchenko @ 2022-03-22 18:53 ` Pali Rohár 2022-04-22 14:28 ` Greg Kroah-Hartman 1 sibling, 0 replies; 17+ messages in thread From: Pali Rohár @ 2022-03-22 18:53 UTC (permalink / raw) To: Andy Shevchenko Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Tuesday 22 March 2022 16:29:08 Andy Shevchenko wrote: > On Mon, Mar 21, 2022 at 11:07 PM Pali Rohár <pali@kernel.org> wrote: > > > > 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() > > > calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that > > userspace termios structre is modified by most drivers after each > > structure > > ... > > > (error handling was ommited for simplification) > > omitted > > > 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 > > > Originally Johan Hovold reported that there may be issue with these > > ASYNC_SPD_FLAGS in email: > > https://lore.kernel.org/linux-serial/20211007133146.28949-1-johan@kernel.org/ > > > > > > 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... > 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... ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-03-22 14:29 ` [PATCH 0/3] serial: Fix support for UPF_SPD_* flags Andy Shevchenko 2022-03-22 18:53 ` Pali Rohár @ 2022-04-22 14:28 ` Greg Kroah-Hartman 2022-07-07 8:48 ` Pali Rohár 1 sibling, 1 reply; 17+ messages in thread From: Greg Kroah-Hartman @ 2022-04-22 14:28 UTC (permalink / raw) To: Andy Shevchenko Cc: Pali Rohár, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Tue, Mar 22, 2022 at 04:29:08PM +0200, Andy Shevchenko wrote: > On Mon, Mar 21, 2022 at 11:07 PM Pali Rohár <pali@kernel.org> wrote: > > > > 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() > > > calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that > > userspace termios structre is modified by most drivers after each > > structure > > ... > > > (error handling was ommited for simplification) > > omitted > > > 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 > > > Originally Johan Hovold reported that there may be issue with these > > ASYNC_SPD_FLAGS in email: > > https://lore.kernel.org/linux-serial/20211007133146.28949-1-johan@kernel.org/ > > > > > > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-04-22 14:28 ` Greg Kroah-Hartman @ 2022-07-07 8:48 ` Pali Rohár 2022-07-08 13:07 ` Greg Kroah-Hartman 0 siblings, 1 reply; 17+ messages in thread From: Pali Rohár @ 2022-07-07 8:48 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Andy Shevchenko, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote: > On Tue, Mar 22, 2022 at 04:29:08PM +0200, Andy Shevchenko wrote: > > On Mon, Mar 21, 2022 at 11:07 PM Pali Rohár <pali@kernel.org> wrote: > > > > > > 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() > > > > > calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that > > > userspace termios structre is modified by most drivers after each > > > > structure > > > > ... > > > > > (error handling was ommited for simplification) > > > > omitted > > > > > 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 > > > > > Originally Johan Hovold reported that there may be issue with these > > > ASYNC_SPD_FLAGS in email: > > > https://lore.kernel.org/linux-serial/20211007133146.28949-1-johan@kernel.org/ > > > > > > > > > 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. 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. > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-07-07 8:48 ` Pali Rohár @ 2022-07-08 13:07 ` Greg Kroah-Hartman 2022-07-08 13:26 ` Pali Rohár 0 siblings, 1 reply; 17+ messages in thread From: Greg Kroah-Hartman @ 2022-07-08 13:07 UTC (permalink / raw) To: Pali Rohár Cc: Andy Shevchenko, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Thu, Jul 07, 2022 at 10:48:40AM +0200, Pali Rohár wrote: > On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote: > > On Tue, Mar 22, 2022 at 04:29:08PM +0200, Andy Shevchenko wrote: > > > On Mon, Mar 21, 2022 at 11:07 PM Pali Rohár <pali@kernel.org> wrote: > > > > > > > > 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() > > > > > > > calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that > > > > userspace termios structre is modified by most drivers after each > > > > > > structure > > > > > > ... > > > > > > > (error handling was ommited for simplification) > > > > > > omitted > > > > > > > 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 > > > > > > > Originally Johan Hovold reported that there may be issue with these > > > > ASYNC_SPD_FLAGS in email: > > > > https://lore.kernel.org/linux-serial/20211007133146.28949-1-johan@kernel.org/ > > > > > > > > > > > > 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. > > 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... > 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. > 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? > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-07-08 13:07 ` Greg Kroah-Hartman @ 2022-07-08 13:26 ` Pali Rohár 2022-07-08 13:51 ` Greg Kroah-Hartman 0 siblings, 1 reply; 17+ messages in thread From: Pali Rohár @ 2022-07-08 13:26 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Andy Shevchenko, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote: > On Thu, Jul 07, 2022 at 10:48:40AM +0200, Pali Rohár wrote: > > On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote: > > > On Tue, Mar 22, 2022 at 04:29:08PM +0200, Andy Shevchenko wrote: > > > > On Mon, Mar 21, 2022 at 11:07 PM Pali Rohár <pali@kernel.org> wrote: > > > > > > > > > > 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() > > > > > > > > > calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that > > > > > userspace termios structre is modified by most drivers after each > > > > > > > > structure > > > > > > > > ... > > > > > > > > > (error handling was ommited for simplification) > > > > > > > > omitted > > > > > > > > > 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 > > > > > > > > > Originally Johan Hovold reported that there may be issue with these > > > > > ASYNC_SPD_FLAGS in email: > > > > > https://lore.kernel.org/linux-serial/20211007133146.28949-1-johan@kernel.org/ > > > > > > > > > > > > > > > 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. > > > > 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). > And as you are responding to a months-old thread, I am totally lost, and > don't even know what the patch here was... > > > 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. > > 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. > > 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? No. Because structures and defines conflicts with glibc structures and defines. That is why (ABI incompatible) reimplementation is needed. > 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. > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-07-08 13:26 ` Pali Rohár @ 2022-07-08 13:51 ` Greg Kroah-Hartman 2022-07-08 14:20 ` Pali Rohár 0 siblings, 1 reply; 17+ messages in thread From: Greg Kroah-Hartman @ 2022-07-08 13:51 UTC (permalink / raw) To: Pali Rohár Cc: Andy Shevchenko, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Fri, Jul 08, 2022 at 03:26:21PM +0200, Pali Rohár wrote: > On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote: > > On Thu, Jul 07, 2022 at 10:48:40AM +0200, Pali Rohár wrote: > > > On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote: > > > > On Tue, Mar 22, 2022 at 04:29:08PM +0200, Andy Shevchenko wrote: > > > > > On Mon, Mar 21, 2022 at 11:07 PM Pali Rohár <pali@kernel.org> wrote: > > > > > > > > > > > > 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() > > > > > > > > > > > calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that > > > > > > userspace termios structre is modified by most drivers after each > > > > > > > > > > structure > > > > > > > > > > ... > > > > > > > > > > > (error handling was ommited for simplification) > > > > > > > > > > omitted > > > > > > > > > > > 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 > > > > > > > > > > > Originally Johan Hovold reported that there may be issue with these > > > > > > ASYNC_SPD_FLAGS in email: > > > > > > https://lore.kernel.org/linux-serial/20211007133146.28949-1-johan@kernel.org/ > > > > > > > > > > > > > > > > > > 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. > > > > > > 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). > > > And as you are responding to a months-old thread, I am totally lost, and > > don't even know what the patch here was... > > > > > 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. 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-07-08 13:51 ` Greg Kroah-Hartman @ 2022-07-08 14:20 ` Pali Rohár 2022-07-08 15:42 ` Andy Shevchenko 0 siblings, 1 reply; 17+ messages in thread From: Pali Rohár @ 2022-07-08 14:20 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Andy Shevchenko, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote: > On Fri, Jul 08, 2022 at 03:26:21PM +0200, Pali Rohár wrote: > > On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote: > > > On Thu, Jul 07, 2022 at 10:48:40AM +0200, Pali Rohár wrote: > > > > On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote: > > > > > On Tue, Mar 22, 2022 at 04:29:08PM +0200, Andy Shevchenko wrote: > > > > > > On Mon, Mar 21, 2022 at 11:07 PM Pali Rohár <pali@kernel.org> wrote: > > > > > > > > > > > > > > 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() > > > > > > > > > > > > > calculate timeout for UPF_SPD_CUST flag correctly. Second reason is that > > > > > > > userspace termios structre is modified by most drivers after each > > > > > > > > > > > > structure > > > > > > > > > > > > ... > > > > > > > > > > > > > (error handling was ommited for simplification) > > > > > > > > > > > > omitted > > > > > > > > > > > > > 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 > > > > > > > > > > > > > Originally Johan Hovold reported that there may be issue with these > > > > > > > ASYNC_SPD_FLAGS in email: > > > > > > > https://lore.kernel.org/linux-serial/20211007133146.28949-1-johan@kernel.org/ > > > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > 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). > > > > > And as you are responding to a months-old thread, I am totally lost, and > > > don't even know what the patch here was... > > > > > > > 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. > > I'm sorry, I am totally lost. So look what you have wrote? Who is lost here is me. > 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-07-08 14:20 ` Pali Rohár @ 2022-07-08 15:42 ` Andy Shevchenko 2022-07-08 15:54 ` Pali Rohár 0 siblings, 1 reply; 17+ messages in thread From: Andy Shevchenko @ 2022-07-08 15:42 UTC (permalink / raw) To: Pali Rohár Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Fri, Jul 8, 2022 at 4:20 PM Pali Rohár <pali@kernel.org> wrote: > On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote: > > On Fri, Jul 08, 2022 at 03:26:21PM +0200, Pali Rohár wrote: > > > On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote: > > > > On Thu, Jul 07, 2022 at 10:48:40AM +0200, Pali Rohár wrote: > > > > > On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote: ... > > > > 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. > > I'm sorry, I am totally lost. > > So look what you have wrote? Who is lost here is me. > > > 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 -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-07-08 15:42 ` Andy Shevchenko @ 2022-07-08 15:54 ` Pali Rohár 2022-07-08 16:09 ` Andy Shevchenko 0 siblings, 1 reply; 17+ messages in thread From: Pali Rohár @ 2022-07-08 15:54 UTC (permalink / raw) To: Andy Shevchenko Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Friday 08 July 2022 17:42:03 Andy Shevchenko wrote: > On Fri, Jul 8, 2022 at 4:20 PM Pali Rohár <pali@kernel.org> wrote: > > On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote: > > > On Fri, Jul 08, 2022 at 03:26:21PM +0200, Pali Rohár wrote: > > > > On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote: > > > > > On Thu, Jul 07, 2022 at 10:48:40AM +0200, Pali Rohár wrote: > > > > > > On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote: > > ... > > > > > > 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. Which addition? I do not understand you. There was not any new driver with introduction of SPD support. > > > I'm sorry, I am totally lost. > > > > So look what you have wrote? Who is lost here is me. > > > > > 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? What you are talking about? Which user? > 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? > 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. > [1]: https://github.com/npat-efault/picocom/blob/master/termios2.txt > [2]: https://github.com/jmesmon/picocom/issues/2 > > > > -- > With Best Regards, > Andy Shevchenko ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-07-08 15:54 ` Pali Rohár @ 2022-07-08 16:09 ` Andy Shevchenko 2022-07-08 16:25 ` Pali Rohár 0 siblings, 1 reply; 17+ messages in thread From: Andy Shevchenko @ 2022-07-08 16:09 UTC (permalink / raw) To: Pali Rohár Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Fri, Jul 8, 2022 at 5:54 PM Pali Rohár <pali@kernel.org> wrote: > On Friday 08 July 2022 17:42:03 Andy Shevchenko wrote: > > On Fri, Jul 8, 2022 at 4:20 PM Pali Rohár <pali@kernel.org> wrote: > > > On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote: > > > > On Fri, Jul 08, 2022 at 03:26:21PM +0200, Pali Rohár wrote: > > > > > On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote: > > > > > > On Thu, Jul 07, 2022 at 10:48:40AM +0200, Pali Rohár wrote: > > > > > > > On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote: > > > > ... > > > > > > > > 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. > > 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. > > > > I'm sorry, I am totally lost. > > > > > > So look what you have wrote? Who is lost here is me. > > > > > > > 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? > > 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. > > 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. > > 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? > > [1]: https://github.com/npat-efault/picocom/blob/master/termios2.txt > > [2]: https://github.com/jmesmon/picocom/issues/2 -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-07-08 16:09 ` Andy Shevchenko @ 2022-07-08 16:25 ` Pali Rohár 2022-07-08 17:41 ` Andy Shevchenko 0 siblings, 1 reply; 17+ messages in thread From: Pali Rohár @ 2022-07-08 16:25 UTC (permalink / raw) To: Andy Shevchenko Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Friday 08 July 2022 18:09:07 Andy Shevchenko wrote: > On Fri, Jul 8, 2022 at 5:54 PM Pali Rohár <pali@kernel.org> wrote: > > On Friday 08 July 2022 17:42:03 Andy Shevchenko wrote: > > > On Fri, Jul 8, 2022 at 4:20 PM Pali Rohár <pali@kernel.org> wrote: > > > > On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote: > > > > > On Fri, Jul 08, 2022 at 03:26:21PM +0200, Pali Rohár wrote: > > > > > > On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote: > > > > > > > On Thu, Jul 07, 2022 at 10:48:40AM +0200, Pali Rohár wrote: > > > > > > > > On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote: > > > > > > ... > > > > > > > > > > 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. > > > > 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"... > > > > > I'm sorry, I am totally lost. > > > > > > > > So look what you have wrote? Who is lost here is me. > > > > > > > > > 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? > > > > 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. > > > > 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 > > > 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. > > > [1]: https://github.com/npat-efault/picocom/blob/master/termios2.txt > > > [2]: https://github.com/jmesmon/picocom/issues/2 > > -- > With Best Regards, > Andy Shevchenko ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] serial: Fix support for UPF_SPD_* flags 2022-07-08 16:25 ` Pali Rohár @ 2022-07-08 17:41 ` Andy Shevchenko 0 siblings, 0 replies; 17+ messages in thread From: Andy Shevchenko @ 2022-07-08 17:41 UTC (permalink / raw) To: Pali Rohár Cc: Greg Kroah-Hartman, Jiri Slaby, Johan Hovold, Marek Behún, open list:SERIAL DRIVERS, Linux Kernel Mailing List On Fri, Jul 8, 2022 at 6:25 PM Pali Rohár <pali@kernel.org> wrote: > > On Friday 08 July 2022 18:09:07 Andy Shevchenko wrote: > > On Fri, Jul 8, 2022 at 5:54 PM Pali Rohár <pali@kernel.org> wrote: > > > On Friday 08 July 2022 17:42:03 Andy Shevchenko wrote: > > > > On Fri, Jul 8, 2022 at 4:20 PM Pali Rohár <pali@kernel.org> wrote: > > > > > On Friday 08 July 2022 15:51:01 Greg Kroah-Hartman wrote: > > > > > > On Fri, Jul 08, 2022 at 03:26:21PM +0200, Pali Rohár wrote: > > > > > > > On Friday 08 July 2022 15:07:43 Greg Kroah-Hartman wrote: > > > > > > > > On Thu, Jul 07, 2022 at 10:48:40AM +0200, Pali Rohár wrote: > > > > > > > > > On Friday 22 April 2022 16:28:06 Greg Kroah-Hartman wrote: > > > > > > > > ... > > > > > > > > > > > > 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. > > > > > > 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"... 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. > > > > > > I'm sorry, I am totally lost. > > > > > > > > > > So look what you have wrote? Who is lost here is me. > > > > > > > > > > > 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? > > > > > > 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. > > > > > > 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)." 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. > 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 > > > > > 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. Then help make it happen? > 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. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-07-08 17:42 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-21 16:30 [PATCH 0/3] serial: Fix support for UPF_SPD_* flags Pali Rohár 2022-03-21 16:30 ` [PATCH 1/3] serial: core: Document why UPF_SPD_CUST is not handled in uart_get_baud_rate() Pali Rohár 2022-03-21 16:30 ` [PATCH 2/3] serial: core: Fix function uart_update_timeout() to handle UPF_SPD_CUST flag Pali Rohár 2022-03-21 16:30 ` [PATCH 3/3] serial: Fix support for UPF_SPD_* flags in serial drivers Pali Rohár 2022-03-22 14:29 ` [PATCH 0/3] serial: Fix support for UPF_SPD_* flags Andy Shevchenko 2022-03-22 18:53 ` Pali Rohár 2022-04-22 14:28 ` Greg Kroah-Hartman 2022-07-07 8:48 ` Pali Rohár 2022-07-08 13:07 ` Greg Kroah-Hartman 2022-07-08 13:26 ` Pali Rohár 2022-07-08 13:51 ` Greg Kroah-Hartman 2022-07-08 14:20 ` Pali Rohár 2022-07-08 15:42 ` Andy Shevchenko 2022-07-08 15:54 ` Pali Rohár 2022-07-08 16:09 ` Andy Shevchenko 2022-07-08 16:25 ` Pali Rohár 2022-07-08 17:41 ` Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).