* [PATCH v9 4/6] serial: take termios_rwsem for ->rs485_config() & pass termios as param
[not found] <20220624204210.11112-1-ilpo.jarvinen@linux.intel.com>
@ 2022-06-24 20:42 ` Ilpo Järvinen
2022-07-04 6:51 ` Jiri Slaby
0 siblings, 1 reply; 3+ messages in thread
From: Ilpo Järvinen @ 2022-06-24 20:42 UTC (permalink / raw)
To: linux-serial, Greg KH, Jiri Slaby, Andy Shevchenko,
Vladimir Zapolskiy, Russell King, Richard Genoud, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Maxime Coquelin, Alexandre Torgue, linux-kernel, linux-arm-kernel,
linux-stm32
Cc: Lukas Wunner, Uwe Kleine-König, Lino Sanfilippo,
Ilpo Järvinen
To be able to alter ADDRB within ->rs485_config(), take termios_rwsem
before calling ->rs485_config() and pass termios.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/tty/serial/8250/8250.h | 3 ++-
drivers/tty/serial/8250/8250_dwlib.c | 3 ++-
drivers/tty/serial/8250/8250_exar.c | 9 +++++----
drivers/tty/serial/8250/8250_fintek.c | 2 +-
drivers/tty/serial/8250/8250_lpc18xx.c | 2 +-
drivers/tty/serial/8250/8250_pci.c | 2 +-
drivers/tty/serial/8250/8250_port.c | 3 ++-
drivers/tty/serial/amba-pl011.c | 2 +-
drivers/tty/serial/ar933x_uart.c | 2 +-
drivers/tty/serial/atmel_serial.c | 2 +-
drivers/tty/serial/fsl_lpuart.c | 4 ++--
drivers/tty/serial/imx.c | 2 +-
drivers/tty/serial/max310x.c | 2 +-
drivers/tty/serial/mcf.c | 3 ++-
drivers/tty/serial/omap-serial.c | 3 ++-
drivers/tty/serial/sc16is7xx.c | 2 +-
drivers/tty/serial/serial_core.c | 14 ++++++++++----
drivers/tty/serial/stm32-usart.c | 2 +-
include/linux/serial_core.h | 1 +
19 files changed, 38 insertions(+), 25 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 5cc967fe3b59..287153d32536 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -203,7 +203,8 @@ void serial8250_rpm_put(struct uart_8250_port *p);
void serial8250_rpm_get_tx(struct uart_8250_port *p);
void serial8250_rpm_put_tx(struct uart_8250_port *p);
-int serial8250_em485_config(struct uart_port *port, struct serial_rs485 *rs485);
+int serial8250_em485_config(struct uart_port *port, struct ktermios *termios,
+ struct serial_rs485 *rs485);
void serial8250_em485_start_tx(struct uart_8250_port *p);
void serial8250_em485_stop_tx(struct uart_8250_port *p);
void serial8250_em485_destroy(struct uart_8250_port *p);
diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index c83e7eaf3877..d1ff3daeb0ba 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -85,7 +85,8 @@ void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, struct
}
EXPORT_SYMBOL_GPL(dw8250_do_set_termios);
-static int dw8250_rs485_config(struct uart_port *p, struct serial_rs485 *rs485)
+static int dw8250_rs485_config(struct uart_port *p, struct ktermios *termios,
+ struct serial_rs485 *rs485)
{
u32 tcr;
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 3d999eec4087..f5344cfe303c 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -112,7 +112,8 @@
struct exar8250;
struct exar8250_platform {
- int (*rs485_config)(struct uart_port *, struct serial_rs485 *);
+ int (*rs485_config)(struct uart_port *port, struct ktermios *termios,
+ struct serial_rs485 *rs485);
const struct serial_rs485 *rs485_supported;
int (*register_gpio)(struct pci_dev *, struct uart_8250_port *);
void (*unregister_gpio)(struct uart_8250_port *);
@@ -409,7 +410,7 @@ static void xr17v35x_unregister_gpio(struct uart_8250_port *port)
port->port.private_data = NULL;
}
-static int generic_rs485_config(struct uart_port *port,
+static int generic_rs485_config(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485)
{
bool is_rs485 = !!(rs485->flags & SER_RS485_ENABLED);
@@ -441,7 +442,7 @@ static const struct exar8250_platform exar8250_default_platform = {
.rs485_supported = &generic_rs485_supported,
};
-static int iot2040_rs485_config(struct uart_port *port,
+static int iot2040_rs485_config(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485)
{
bool is_rs485 = !!(rs485->flags & SER_RS485_ENABLED);
@@ -471,7 +472,7 @@ static int iot2040_rs485_config(struct uart_port *port,
value |= mode;
writeb(value, p + UART_EXAR_MPIOLVL_7_0);
- return generic_rs485_config(port, rs485);
+ return generic_rs485_config(port, termios, rs485);
}
static const struct serial_rs485 iot2040_rs485_supported = {
diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index 1fb86c73786c..eea693f5b577 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -191,7 +191,7 @@ static int fintek_8250_get_ldn_range(struct fintek_8250 *pdata, int *min,
return -ENODEV;
}
-static int fintek_8250_rs485_config(struct uart_port *port,
+static int fintek_8250_rs485_config(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485)
{
uint8_t config = 0;
diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index 3a1cb51cbc91..d7cb3bb52069 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -32,7 +32,7 @@ struct lpc18xx_uart_data {
int line;
};
-static int lpc18xx_rs485_config(struct uart_port *port,
+static int lpc18xx_rs485_config(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485)
{
struct uart_8250_port *up = up_to_u8250p(port);
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index b6d71268aa7d..d31d2350a9db 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1553,7 +1553,7 @@ pci_brcm_trumanage_setup(struct serial_private *priv,
#define FINTEK_RTS_INVERT BIT(5)
/* We should do proper H/W transceiver setting before change to RS485 mode */
-static int pci_fintek_rs485_config(struct uart_port *port,
+static int pci_fintek_rs485_config(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485)
{
struct pci_dev *pci_dev = to_pci_dev(port->dev);
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 19c612d732cf..5cb3673f9ee5 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -664,7 +664,8 @@ EXPORT_SYMBOL_GPL(serial8250_em485_supported);
* if the uart is incapable of driving RTS as a Transmit Enable signal in
* hardware, relying on software emulation instead.
*/
-int serial8250_em485_config(struct uart_port *port, struct serial_rs485 *rs485)
+int serial8250_em485_config(struct uart_port *port, struct ktermios *termios,
+ struct serial_rs485 *rs485)
{
struct uart_8250_port *up = up_to_u8250p(port);
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index eccd66625d25..c8f52945a4aa 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2197,7 +2197,7 @@ static int pl011_verify_port(struct uart_port *port, struct serial_struct *ser)
return ret;
}
-static int pl011_rs485_config(struct uart_port *port,
+static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485)
{
struct uart_amba_port *uap =
diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c
index ab2c5b2a1ce8..b73ce13683db 100644
--- a/drivers/tty/serial/ar933x_uart.c
+++ b/drivers/tty/serial/ar933x_uart.c
@@ -580,7 +580,7 @@ static const struct uart_ops ar933x_uart_ops = {
.verify_port = ar933x_uart_verify_port,
};
-static int ar933x_config_rs485(struct uart_port *port,
+static int ar933x_config_rs485(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485conf)
{
struct ar933x_uart_port *up =
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 74dd1d3ac46f..64dd6fea0a30 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -285,7 +285,7 @@ static void atmel_tasklet_schedule(struct atmel_uart_port *atmel_port,
}
/* Enable or disable the rs485 support */
-static int atmel_config_rs485(struct uart_port *port,
+static int atmel_config_rs485(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485conf)
{
struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index d35414cb3e4e..8fe0494d4057 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1355,7 +1355,7 @@ static void lpuart_dma_rx_free(struct uart_port *port)
sport->dma_rx_cookie = -EINVAL;
}
-static int lpuart_config_rs485(struct uart_port *port,
+static int lpuart_config_rs485(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485)
{
struct lpuart_port *sport = container_of(port,
@@ -1385,7 +1385,7 @@ static int lpuart_config_rs485(struct uart_port *port,
return 0;
}
-static int lpuart32_config_rs485(struct uart_port *port,
+static int lpuart32_config_rs485(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485)
{
struct lpuart_port *sport = container_of(port,
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index f4edde54175f..3457006cea3f 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1907,7 +1907,7 @@ static void imx_uart_poll_put_char(struct uart_port *port, unsigned char c)
#endif
/* called with port.lock taken and irqs off or from .probe without locking */
-static int imx_uart_rs485_config(struct uart_port *port,
+static int imx_uart_rs485_config(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485conf)
{
struct imx_port *sport = (struct imx_port *)port;
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index d7d1791fcb57..5a8521fb9237 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1026,7 +1026,7 @@ static void max310x_rs_proc(struct work_struct *ws)
MAX310X_MODE2_ECHOSUPR_BIT, mode2);
}
-static int max310x_rs485_config(struct uart_port *port,
+static int max310x_rs485_config(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485)
{
struct max310x_one *one = to_max310x_port(port);
diff --git a/drivers/tty/serial/mcf.c b/drivers/tty/serial/mcf.c
index 036f178e3d66..73c5287b8e5e 100644
--- a/drivers/tty/serial/mcf.c
+++ b/drivers/tty/serial/mcf.c
@@ -431,7 +431,8 @@ static int mcf_verify_port(struct uart_port *port, struct serial_struct *ser)
/****************************************************************************/
/* Enable or disable the RS485 support */
-static int mcf_config_rs485(struct uart_port *port, struct serial_rs485 *rs485)
+static int mcf_config_rs485(struct uart_port *port, struct ktermios *termios,
+ struct serial_rs485 *rs485)
{
unsigned char mr1, mr2;
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 98622c35d896..1d1e48ff33d8 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1325,7 +1325,8 @@ static inline void serial_omap_add_console_port(struct uart_omap_port *up)
/* Enable or disable the rs485 support */
static int
-serial_omap_config_rs485(struct uart_port *port, struct serial_rs485 *rs485)
+serial_omap_config_rs485(struct uart_port *port, struct ktermios *termios,
+ struct serial_rs485 *rs485)
{
struct uart_omap_port *up = to_uart_omap_port(port);
unsigned int mode;
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 2ceecaa4a478..8cb92a3b3fb8 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1127,7 +1127,7 @@ static void sc16is7xx_set_termios(struct uart_port *port,
spin_unlock_irqrestore(&port->lock, flags);
}
-static int sc16is7xx_config_rs485(struct uart_port *port,
+static int sc16is7xx_config_rs485(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485)
{
struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 621fc15e2e54..44c3785445e3 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1359,7 +1359,7 @@ int uart_rs485_config(struct uart_port *port)
uart_sanitize_serial_rs485(port, rs485);
- ret = port->rs485_config(port, rs485);
+ ret = port->rs485_config(port, NULL, rs485);
if (ret)
memset(rs485, 0, sizeof(*rs485));
@@ -1383,7 +1383,7 @@ static int uart_get_rs485_config(struct uart_port *port,
return 0;
}
-static int uart_set_rs485_config(struct uart_port *port,
+static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
struct serial_rs485 __user *rs485_user)
{
struct serial_rs485 rs485;
@@ -1402,7 +1402,7 @@ static int uart_set_rs485_config(struct uart_port *port,
uart_sanitize_serial_rs485(port, &rs485);
spin_lock_irqsave(&port->lock, flags);
- ret = port->rs485_config(port, &rs485);
+ ret = port->rs485_config(port, &tty->termios, &rs485);
if (!ret)
port->rs485 = rs485;
spin_unlock_irqrestore(&port->lock, flags);
@@ -1511,6 +1511,10 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
if (ret != -ENOIOCTLCMD)
goto out;
+ /* rs485_config requires more locking than others */
+ if (cmd == TIOCGRS485)
+ down_write(&tty->termios_rwsem);
+
mutex_lock(&port->mutex);
uport = uart_port_check(state);
@@ -1534,7 +1538,7 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
break;
case TIOCSRS485:
- ret = uart_set_rs485_config(uport, uarg);
+ ret = uart_set_rs485_config(tty, uport, uarg);
break;
case TIOCSISO7816:
@@ -1551,6 +1555,8 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
}
out_up:
mutex_unlock(&port->mutex);
+ if (cmd == TIOCGRS485)
+ up_write(&tty->termios_rwsem);
out:
return ret;
}
diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index db3dd9731ee1..13992e64a7df 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -97,7 +97,7 @@ static void stm32_usart_config_reg_rs485(u32 *cr1, u32 *cr3, u32 delay_ADE,
*cr1 |= rs485_deat_dedt;
}
-static int stm32_usart_config_rs485(struct uart_port *port,
+static int stm32_usart_config_rs485(struct uart_port *port, struct ktermios *termios,
struct serial_rs485 *rs485conf)
{
struct stm32_port *stm32_port = to_stm32_port(port);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 5518b70177b3..17bbd0412a23 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -132,6 +132,7 @@ struct uart_port {
unsigned int old);
void (*handle_break)(struct uart_port *);
int (*rs485_config)(struct uart_port *,
+ struct ktermios *termios,
struct serial_rs485 *rs485);
int (*iso7816_config)(struct uart_port *,
struct serial_iso7816 *iso7816);
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v9 4/6] serial: take termios_rwsem for ->rs485_config() & pass termios as param
2022-06-24 20:42 ` [PATCH v9 4/6] serial: take termios_rwsem for ->rs485_config() & pass termios as param Ilpo Järvinen
@ 2022-07-04 6:51 ` Jiri Slaby
2022-07-04 6:56 ` Jiri Slaby
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Slaby @ 2022-07-04 6:51 UTC (permalink / raw)
To: Ilpo Järvinen, linux-serial, Greg KH, Andy Shevchenko,
Vladimir Zapolskiy, Russell King, Richard Genoud, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Maxime Coquelin, Alexandre Torgue, linux-kernel, linux-arm-kernel,
linux-stm32
Cc: Lukas Wunner, Uwe Kleine-König, Lino Sanfilippo
On 24. 06. 22, 22:42, Ilpo Järvinen wrote:
> To be able to alter ADDRB within ->rs485_config(), take termios_rwsem
> before calling ->rs485_config() and pass termios.
OK, FTR, worth noting the tty->termios_rwsem -> port->mutex lock chain
is preexisting.
Anyway, I'm not sure I buy the above. Why is termios_rwsem needed to
alter ADDRB?
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 621fc15e2e54..44c3785445e3 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
...
> @@ -1511,6 +1511,10 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
> if (ret != -ENOIOCTLCMD)
> goto out;
>
> + /* rs485_config requires more locking than others */
> + if (cmd == TIOCGRS485)
> + down_write(&tty->termios_rwsem);
> +
> mutex_lock(&port->mutex);
> uport = uart_port_check(state);
>
...
> @@ -1551,6 +1555,8 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
> }
> out_up:
> mutex_unlock(&port->mutex);
> + if (cmd == TIOCGRS485)
> + up_write(&tty->termios_rwsem);
> out:
> return ret;
> }
thanks,
--
js
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v9 4/6] serial: take termios_rwsem for ->rs485_config() & pass termios as param
2022-07-04 6:51 ` Jiri Slaby
@ 2022-07-04 6:56 ` Jiri Slaby
0 siblings, 0 replies; 3+ messages in thread
From: Jiri Slaby @ 2022-07-04 6:56 UTC (permalink / raw)
To: Ilpo Järvinen, linux-serial, Greg KH, Andy Shevchenko,
Vladimir Zapolskiy, Russell King, Richard Genoud, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Maxime Coquelin, Alexandre Torgue, linux-kernel, linux-arm-kernel,
linux-stm32
Cc: Lukas Wunner, Uwe Kleine-König, Lino Sanfilippo
On 04. 07. 22, 8:51, Jiri Slaby wrote:
> Anyway, I'm not sure I buy the above. Why is termios_rwsem needed to
> alter ADDRB?
Nevermind, reading patch 5/6 gives a clue.
--
js
suse labs
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-07-04 6:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220624204210.11112-1-ilpo.jarvinen@linux.intel.com>
2022-06-24 20:42 ` [PATCH v9 4/6] serial: take termios_rwsem for ->rs485_config() & pass termios as param Ilpo Järvinen
2022-07-04 6:51 ` Jiri Slaby
2022-07-04 6:56 ` Jiri Slaby
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).