2023-03-09 08:23:01

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 0/8] tty: Cleanups

Small cleanups to tty.

Ilpo Järvinen (8):
n_tty: Convert no_space_left to space_left boolean
tty_ioctl: Use BIT() for internal flags
Bluetooth: hci_ldisc: Fix tty_set_termios() return value assumptions
n_tty: Sort includes alphabetically
n_tty: Use DIV_ROUND_UP() in room calculation
n_tty: Cleanup includes
n_tty: Reindent if condition
tty: Convert hw_stopped in tty_struct to bool

drivers/bluetooth/hci_ldisc.c | 8 +++---
drivers/char/pcmcia/synclink_cs.c | 6 ++---
drivers/mmc/core/sdio_uart.c | 10 +++----
drivers/tty/amiserial.c | 6 ++---
drivers/tty/mxser.c | 6 ++---
drivers/tty/n_tty.c | 43 +++++++++++++++----------------
drivers/tty/synclink_gt.c | 6 ++---
drivers/tty/tty_ioctl.c | 9 ++++---
include/linux/tty.h | 2 +-
9 files changed, 48 insertions(+), 48 deletions(-)

--
2.30.2



2023-03-09 08:23:04

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 1/8] n_tty: Convert no_space_left to space_left boolean

The no_space_left variable is only assigned with 0 and 1.

Change its type to boolean and move negation from its name into the
check.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/tty/n_tty.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index c8f56c9b1a1c..4fc5bd166e56 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -625,7 +625,7 @@ static size_t __process_echoes(struct tty_struct *tty)
c = echo_buf(ldata, tail);
if (c == ECHO_OP_START) {
unsigned char op;
- int no_space_left = 0;
+ bool space_left = true;

/*
* Since add_echo_byte() is called without holding
@@ -664,7 +664,7 @@ static size_t __process_echoes(struct tty_struct *tty)
num_bs = 8 - (num_chars & 7);

if (num_bs > space) {
- no_space_left = 1;
+ space_left = false;
break;
}
space -= num_bs;
@@ -690,7 +690,7 @@ static size_t __process_echoes(struct tty_struct *tty)
case ECHO_OP_START:
/* This is an escaped echo op start code */
if (!space) {
- no_space_left = 1;
+ space_left = false;
break;
}
tty_put_char(tty, ECHO_OP_START);
@@ -710,7 +710,7 @@ static size_t __process_echoes(struct tty_struct *tty)
*
*/
if (space < 2) {
- no_space_left = 1;
+ space_left = false;
break;
}
tty_put_char(tty, '^');
@@ -720,7 +720,7 @@ static size_t __process_echoes(struct tty_struct *tty)
tail += 2;
}

- if (no_space_left)
+ if (!space_left)
break;
} else {
if (O_OPOST(tty)) {
--
2.30.2


2023-03-09 08:23:11

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 2/8] tty_ioctl: Use BIT() for internal flags

Convert internal flags to use BIT().

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/tty/tty_ioctl.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 12983ce4e43e..32ff9959b565 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -7,6 +7,7 @@
* discipline handling modules (like SLIP).
*/

+#include <linux/bits.h>
#include <linux/types.h>
#include <linux/termios.h>
#include <linux/errno.h>
@@ -40,10 +41,10 @@
/*
* Internal flag options for termios setting behavior
*/
-#define TERMIOS_FLUSH 1
-#define TERMIOS_WAIT 2
-#define TERMIOS_TERMIO 4
-#define TERMIOS_OLD 8
+#define TERMIOS_FLUSH BIT(0)
+#define TERMIOS_WAIT BIT(1)
+#define TERMIOS_TERMIO BIT(2)
+#define TERMIOS_OLD BIT(3)


/**
--
2.30.2


2023-03-09 08:23:15

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 3/8] Bluetooth: hci_ldisc: Fix tty_set_termios() return value assumptions

tty_set_termios() never returns anything else than 0. Make the debug
prints to look directly into the new termios instead to check CRTSCTS
state.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/bluetooth/hci_ldisc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 865112e96ff9..efdda2c3fce8 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -323,9 +323,9 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
/* Disable hardware flow control */
ktermios = tty->termios;
ktermios.c_cflag &= ~CRTSCTS;
- status = tty_set_termios(tty, &ktermios);
+ tty_set_termios(tty, &ktermios);
BT_DBG("Disabling hardware flow control: %s",
- status ? "failed" : "success");
+ (tty->termios.c_cflag & CRTSCTS) ? "failed" : "success");

/* Clear RTS to prevent the device from sending */
/* Most UARTs need OUT2 to enable interrupts */
@@ -357,9 +357,9 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
/* Re-enable hardware flow control */
ktermios = tty->termios;
ktermios.c_cflag |= CRTSCTS;
- status = tty_set_termios(tty, &ktermios);
+ tty_set_termios(tty, &ktermios);
BT_DBG("Enabling hardware flow control: %s",
- status ? "failed" : "success");
+ !(tty->termios.c_cflag & CRTSCTS) ? "failed" : "success");
}
}

--
2.30.2


2023-03-09 08:23:33

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 4/8] n_tty: Sort includes alphabetically

Sort includes in n_tty alphabetically to make it easier to see if an
include is among the list or not.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/tty/n_tty.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 4fc5bd166e56..2cf263de1366 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -28,27 +28,28 @@
* EAGAIN
*/

-#include <linux/types.h>
-#include <linux/major.h>
+#include <linux/audit.h>
+#include <linux/bitops.h>
+#include <linux/ctype.h>
#include <linux/errno.h>
-#include <linux/signal.h>
#include <linux/fcntl.h>
-#include <linux/sched.h>
+#include <linux/file.h>
#include <linux/interrupt.h>
-#include <linux/tty.h>
-#include <linux/timer.h>
-#include <linux/ctype.h>
+#include <linux/major.h>
#include <linux/mm.h>
-#include <linux/string.h>
-#include <linux/slab.h>
-#include <linux/poll.h>
-#include <linux/bitops.h>
-#include <linux/audit.h>
-#include <linux/file.h>
-#include <linux/uaccess.h>
#include <linux/module.h>
+#include <linux/poll.h>
#include <linux/ratelimit.h>
+#include <linux/sched.h>
+#include <linux/signal.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/timer.h>
+#include <linux/tty.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
#include <linux/vmalloc.h>
+
#include "tty.h"

/*
--
2.30.2


2023-03-09 08:24:04

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 7/8] n_tty: Reindent if condition

Align if condition to make it easier to read.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/tty/n_tty.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 0481e57077f1..1c9e5d2ea7de 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1176,7 +1176,7 @@ static void n_tty_receive_overrun(struct tty_struct *tty)

ldata->num_overrun++;
if (time_after(jiffies, ldata->overrun_time + HZ) ||
- time_after(ldata->overrun_time, jiffies)) {
+ time_after(ldata->overrun_time, jiffies)) {
tty_warn(tty, "%d input overrun(s)\n", ldata->num_overrun);
ldata->overrun_time = jiffies;
ldata->num_overrun = 0;
--
2.30.2


2023-03-09 08:24:05

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 5/8] n_tty: Use DIV_ROUND_UP() in room calculation

When PARMRK is set, a character can result in up to 3 chars in the read
buffer. Receive code calculates for how many characters there (at
least) is room. Convert an opencoded rounding in the calculation to use
DIV_ROUND_UP().

Note: the room variable is decremented afterwards by one which ensures
the characters will fit into the buffer for real so the code is okay
despite rounding upwards.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/tty/n_tty.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 2cf263de1366..6d754fc35dce 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -36,6 +36,7 @@
#include <linux/file.h>
#include <linux/interrupt.h>
#include <linux/major.h>
+#include <linux/math.h>
#include <linux/mm.h>
#include <linux/module.h>
#include <linux/poll.h>
@@ -1692,7 +1693,7 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,

room = N_TTY_BUF_SIZE - (ldata->read_head - tail);
if (I_PARMRK(tty))
- room = (room + 2) / 3;
+ room = DIV_ROUND_UP(room, 3);
room--;
if (room <= 0) {
overflow = ldata->icanon && ldata->canon_head == tail;
--
2.30.2


2023-03-09 08:24:05

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 6/8] n_tty: Cleanup includes

n_tty uses:
- bitmap_zero() from linux/bitmap.h
- EXPORT_SYMBOL_GPL() from linux/export.h
- jiffies, time_after() from linux/jiffies.h

Add includes.

n_tty uses nothing from:
- linux/audit.h
- linux/interrupt.h
- linux/major.h
- linux/mm.h
- linux/module.h
- linux/timer.h

Remove those includes.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/tty/n_tty.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 6d754fc35dce..0481e57077f1 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -28,24 +28,21 @@
* EAGAIN
*/

-#include <linux/audit.h>
+#include <linux/bitmap.h>
#include <linux/bitops.h>
#include <linux/ctype.h>
#include <linux/errno.h>
+#include <linux/export.h>
#include <linux/fcntl.h>
#include <linux/file.h>
-#include <linux/interrupt.h>
-#include <linux/major.h>
+#include <linux/jiffies.h>
#include <linux/math.h>
-#include <linux/mm.h>
-#include <linux/module.h>
#include <linux/poll.h>
#include <linux/ratelimit.h>
#include <linux/sched.h>
#include <linux/signal.h>
#include <linux/slab.h>
#include <linux/string.h>
-#include <linux/timer.h>
#include <linux/tty.h>
#include <linux/types.h>
#include <linux/uaccess.h>
--
2.30.2


2023-03-09 08:24:36

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 8/8] tty: Convert hw_stopped in tty_struct to bool

hw_stopped in tty_struct is used like bool, convert the variable type
to bool.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/char/pcmcia/synclink_cs.c | 6 +++---
drivers/mmc/core/sdio_uart.c | 10 +++++-----
drivers/tty/amiserial.c | 6 +++---
drivers/tty/mxser.c | 6 +++---
drivers/tty/synclink_gt.c | 6 +++---
include/linux/tty.h | 2 +-
6 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c
index 6ddfeb2fe98f..97c5bfb9d58a 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -1060,7 +1060,7 @@ static void cts_change(MGSLPC_INFO *info, struct tty_struct *tty)
if (info->serial_signals & SerialSignal_CTS) {
if (debug_level >= DEBUG_LEVEL_ISR)
printk("CTS tx start...");
- tty->hw_stopped = 0;
+ tty->hw_stopped = false;
tx_start(info, tty);
info->pending_bh |= BH_TRANSMIT;
return;
@@ -1069,7 +1069,7 @@ static void cts_change(MGSLPC_INFO *info, struct tty_struct *tty)
if (!(info->serial_signals & SerialSignal_CTS)) {
if (debug_level >= DEBUG_LEVEL_ISR)
printk("CTS tx stop...");
- tty->hw_stopped = 1;
+ tty->hw_stopped = true;
tx_stop(info);
}
}
@@ -2312,7 +2312,7 @@ static void mgslpc_set_termios(struct tty_struct *tty,

/* Handle turning off CRTSCTS */
if (old_termios->c_cflag & CRTSCTS && !C_CRTSCTS(tty)) {
- tty->hw_stopped = 0;
+ tty->hw_stopped = false;
tx_release(tty);
}
}
diff --git a/drivers/mmc/core/sdio_uart.c b/drivers/mmc/core/sdio_uart.c
index 50536fe59f1a..aa659758563f 100644
--- a/drivers/mmc/core/sdio_uart.c
+++ b/drivers/mmc/core/sdio_uart.c
@@ -478,13 +478,13 @@ static void sdio_uart_check_modem_status(struct sdio_uart_port *port)
int cts = (status & UART_MSR_CTS);
if (tty->hw_stopped) {
if (cts) {
- tty->hw_stopped = 0;
+ tty->hw_stopped = false;
sdio_uart_start_tx(port);
tty_wakeup(tty);
}
} else {
if (!cts) {
- tty->hw_stopped = 1;
+ tty->hw_stopped = true;
sdio_uart_stop_tx(port);
}
}
@@ -633,7 +633,7 @@ static int sdio_uart_activate(struct tty_port *tport, struct tty_struct *tty)

if (C_CRTSCTS(tty))
if (!(sdio_uart_get_mctrl(port) & TIOCM_CTS))
- tty->hw_stopped = 1;
+ tty->hw_stopped = true;

clear_bit(TTY_IO_ERROR, &tty->flags);

@@ -882,14 +882,14 @@ static void sdio_uart_set_termios(struct tty_struct *tty,

/* Handle turning off CRTSCTS */
if ((old_termios->c_cflag & CRTSCTS) && !(cflag & CRTSCTS)) {
- tty->hw_stopped = 0;
+ tty->hw_stopped = false;
sdio_uart_start_tx(port);
}

/* Handle turning on CRTSCTS */
if (!(old_termios->c_cflag & CRTSCTS) && (cflag & CRTSCTS)) {
if (!(sdio_uart_get_mctrl(port) & TIOCM_CTS)) {
- tty->hw_stopped = 1;
+ tty->hw_stopped = true;
sdio_uart_stop_tx(port);
}
}
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index d7515d61659e..c06ad0a0744b 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -347,7 +347,7 @@ static void check_modem_status(struct serial_state *info)
#if (defined(SERIAL_DEBUG_INTR) || defined(SERIAL_DEBUG_FLOW))
printk("CTS tx start...");
#endif
- port->tty->hw_stopped = 0;
+ port->tty->hw_stopped = false;
info->IER |= UART_IER_THRI;
amiga_custom.intena = IF_SETCLR | IF_TBE;
mb();
@@ -362,7 +362,7 @@ static void check_modem_status(struct serial_state *info)
#if (defined(SERIAL_DEBUG_INTR) || defined(SERIAL_DEBUG_FLOW))
printk("CTS tx stop...");
#endif
- port->tty->hw_stopped = 1;
+ port->tty->hw_stopped = true;
info->IER &= ~UART_IER_THRI;
/* disable Tx interrupt and remove any pending interrupts */
amiga_custom.intena = IF_TBE;
@@ -1197,7 +1197,7 @@ static void rs_set_termios(struct tty_struct *tty, const struct ktermios *old_te

/* Handle turning off CRTSCTS */
if ((old_termios->c_cflag & CRTSCTS) && !C_CRTSCTS(tty)) {
- tty->hw_stopped = 0;
+ tty->hw_stopped = false;
rs_start(tty);
}

diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index ef3116e87975..10855e66fda1 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -553,7 +553,7 @@ static void mxser_handle_cts(struct tty_struct *tty, struct mxser_port *info,

if (tty->hw_stopped) {
if (cts) {
- tty->hw_stopped = 0;
+ tty->hw_stopped = false;

if (!mxser_16550A_or_MUST(info))
__mxser_start_tx(info);
@@ -563,7 +563,7 @@ static void mxser_handle_cts(struct tty_struct *tty, struct mxser_port *info,
} else if (cts)
return;

- tty->hw_stopped = 1;
+ tty->hw_stopped = true;
if (!mxser_16550A_or_MUST(info))
__mxser_stop_tx(info);
}
@@ -1361,7 +1361,7 @@ static void mxser_set_termios(struct tty_struct *tty,
spin_unlock_irqrestore(&info->slock, flags);

if ((old_termios->c_cflag & CRTSCTS) && !C_CRTSCTS(tty)) {
- tty->hw_stopped = 0;
+ tty->hw_stopped = false;
mxser_start(tty);
}

diff --git a/drivers/tty/synclink_gt.c b/drivers/tty/synclink_gt.c
index 33f258d6fef9..543b3224dce9 100644
--- a/drivers/tty/synclink_gt.c
+++ b/drivers/tty/synclink_gt.c
@@ -730,7 +730,7 @@ static void set_termios(struct tty_struct *tty,

/* Handle turning off CRTSCTS */
if ((old_termios->c_cflag & CRTSCTS) && !C_CRTSCTS(tty)) {
- tty->hw_stopped = 0;
+ tty->hw_stopped = false;
tx_release(tty);
}
}
@@ -1953,13 +1953,13 @@ static void cts_change(struct slgt_info *info, unsigned short status)
if (info->port.tty) {
if (info->port.tty->hw_stopped) {
if (info->signals & SerialSignal_CTS) {
- info->port.tty->hw_stopped = 0;
+ info->port.tty->hw_stopped = false;
info->pending_bh |= BH_TRANSMIT;
return;
}
} else {
if (!(info->signals & SerialSignal_CTS))
- info->port.tty->hw_stopped = 1;
+ info->port.tty->hw_stopped = true;
}
}
}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 093935e97f42..60871a9d3212 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -227,7 +227,7 @@ struct tty_struct {
unsigned long unused[0];
} __aligned(sizeof(unsigned long)) ctrl;

- int hw_stopped;
+ bool hw_stopped;
unsigned int receive_room;
int flow_change;

--
2.30.2


2023-03-09 08:37:05

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 8/8] tty: Convert hw_stopped in tty_struct to bool

On Thu, Mar 9, 2023, at 09:20, Ilpo Järvinen wrote:
> hw_stopped in tty_struct is used like bool, convert the variable type
> to bool.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
> drivers/char/pcmcia/synclink_cs.c | 6 +++---
> drivers/mmc/core/sdio_uart.c | 10 +++++-----
> drivers/tty/amiserial.c | 6 +++---
> drivers/tty/mxser.c | 6 +++---
> drivers/tty/synclink_gt.c | 6 +++---
> include/linux/tty.h | 2 +-
> 6 files changed, 18 insertions(+), 18 deletions(-)

The patch looks good to me, but it will (trivially) conflict with the
synclink_cs removal.

Arnd

2023-03-09 13:45:53

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 7/8] n_tty: Reindent if condition

On 09. 03. 23, 9:20, Ilpo Järvinen wrote:
> Align if condition to make it easier to read.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
> drivers/tty/n_tty.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index 0481e57077f1..1c9e5d2ea7de 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1176,7 +1176,7 @@ static void n_tty_receive_overrun(struct tty_struct *tty)
>
> ldata->num_overrun++;
> if (time_after(jiffies, ldata->overrun_time + HZ) ||
> - time_after(ldata->overrun_time, jiffies)) {
> + time_after(ldata->overrun_time, jiffies)) {

Staring at this, what the second time_after() does in the first place?

> tty_warn(tty, "%d input overrun(s)\n", ldata->num_overrun);
> ldata->overrun_time = jiffies;
> ldata->num_overrun = 0;

--
js
suse labs


2023-03-09 14:03:01

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH 7/8] n_tty: Reindent if condition

On Thu, 9 Mar 2023, Jiri Slaby wrote:

> On 09. 03. 23, 9:20, Ilpo Järvinen wrote:
> > Align if condition to make it easier to read.
> >
> > Signed-off-by: Ilpo Järvinen <[email protected]>
> > ---
> > drivers/tty/n_tty.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> > index 0481e57077f1..1c9e5d2ea7de 100644
> > --- a/drivers/tty/n_tty.c
> > +++ b/drivers/tty/n_tty.c
> > @@ -1176,7 +1176,7 @@ static void n_tty_receive_overrun(struct tty_struct
> > *tty)
> > ldata->num_overrun++;
> > if (time_after(jiffies, ldata->overrun_time + HZ) ||
> > - time_after(ldata->overrun_time, jiffies)) {
> > + time_after(ldata->overrun_time, jiffies)) {
>
> Staring at this, what the second time_after() does in the first place?
>
> > tty_warn(tty, "%d input overrun(s)\n", ldata->num_overrun);
> > ldata->overrun_time = jiffies;
> > ldata->num_overrun = 0;

That's a very good question ... I first thought it was checking whether
the jiffies is between two times but obviously that was wrong intuition
now when taking a closer look.

But then, looking more into it, this whole thing looks an opencoded
*_ratelimited print. So perhaps overrun_time could be removed
completely... ? I can see it kinda changes priority of which messages
would get filtered out but I don't know if that's a problem or not.

--
i.

2023-03-09 15:02:57

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 8/8] tty: Convert hw_stopped in tty_struct to bool

On Thu, 9 Mar 2023 at 09:22, Ilpo Järvinen
<[email protected]> wrote:
>
> hw_stopped in tty_struct is used like bool, convert the variable type
> to bool.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>

Acked-by: Ulf Hansson <[email protected]> # For MMC

Kind regards
Uffe

> ---
> drivers/char/pcmcia/synclink_cs.c | 6 +++---
> drivers/mmc/core/sdio_uart.c | 10 +++++-----
> drivers/tty/amiserial.c | 6 +++---
> drivers/tty/mxser.c | 6 +++---
> drivers/tty/synclink_gt.c | 6 +++---
> include/linux/tty.h | 2 +-
> 6 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c
> index 6ddfeb2fe98f..97c5bfb9d58a 100644
> --- a/drivers/char/pcmcia/synclink_cs.c
> +++ b/drivers/char/pcmcia/synclink_cs.c
> @@ -1060,7 +1060,7 @@ static void cts_change(MGSLPC_INFO *info, struct tty_struct *tty)
> if (info->serial_signals & SerialSignal_CTS) {
> if (debug_level >= DEBUG_LEVEL_ISR)
> printk("CTS tx start...");
> - tty->hw_stopped = 0;
> + tty->hw_stopped = false;
> tx_start(info, tty);
> info->pending_bh |= BH_TRANSMIT;
> return;
> @@ -1069,7 +1069,7 @@ static void cts_change(MGSLPC_INFO *info, struct tty_struct *tty)
> if (!(info->serial_signals & SerialSignal_CTS)) {
> if (debug_level >= DEBUG_LEVEL_ISR)
> printk("CTS tx stop...");
> - tty->hw_stopped = 1;
> + tty->hw_stopped = true;
> tx_stop(info);
> }
> }
> @@ -2312,7 +2312,7 @@ static void mgslpc_set_termios(struct tty_struct *tty,
>
> /* Handle turning off CRTSCTS */
> if (old_termios->c_cflag & CRTSCTS && !C_CRTSCTS(tty)) {
> - tty->hw_stopped = 0;
> + tty->hw_stopped = false;
> tx_release(tty);
> }
> }
> diff --git a/drivers/mmc/core/sdio_uart.c b/drivers/mmc/core/sdio_uart.c
> index 50536fe59f1a..aa659758563f 100644
> --- a/drivers/mmc/core/sdio_uart.c
> +++ b/drivers/mmc/core/sdio_uart.c
> @@ -478,13 +478,13 @@ static void sdio_uart_check_modem_status(struct sdio_uart_port *port)
> int cts = (status & UART_MSR_CTS);
> if (tty->hw_stopped) {
> if (cts) {
> - tty->hw_stopped = 0;
> + tty->hw_stopped = false;
> sdio_uart_start_tx(port);
> tty_wakeup(tty);
> }
> } else {
> if (!cts) {
> - tty->hw_stopped = 1;
> + tty->hw_stopped = true;
> sdio_uart_stop_tx(port);
> }
> }
> @@ -633,7 +633,7 @@ static int sdio_uart_activate(struct tty_port *tport, struct tty_struct *tty)
>
> if (C_CRTSCTS(tty))
> if (!(sdio_uart_get_mctrl(port) & TIOCM_CTS))
> - tty->hw_stopped = 1;
> + tty->hw_stopped = true;
>
> clear_bit(TTY_IO_ERROR, &tty->flags);
>
> @@ -882,14 +882,14 @@ static void sdio_uart_set_termios(struct tty_struct *tty,
>
> /* Handle turning off CRTSCTS */
> if ((old_termios->c_cflag & CRTSCTS) && !(cflag & CRTSCTS)) {
> - tty->hw_stopped = 0;
> + tty->hw_stopped = false;
> sdio_uart_start_tx(port);
> }
>
> /* Handle turning on CRTSCTS */
> if (!(old_termios->c_cflag & CRTSCTS) && (cflag & CRTSCTS)) {
> if (!(sdio_uart_get_mctrl(port) & TIOCM_CTS)) {
> - tty->hw_stopped = 1;
> + tty->hw_stopped = true;
> sdio_uart_stop_tx(port);
> }
> }
> diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
> index d7515d61659e..c06ad0a0744b 100644
> --- a/drivers/tty/amiserial.c
> +++ b/drivers/tty/amiserial.c
> @@ -347,7 +347,7 @@ static void check_modem_status(struct serial_state *info)
> #if (defined(SERIAL_DEBUG_INTR) || defined(SERIAL_DEBUG_FLOW))
> printk("CTS tx start...");
> #endif
> - port->tty->hw_stopped = 0;
> + port->tty->hw_stopped = false;
> info->IER |= UART_IER_THRI;
> amiga_custom.intena = IF_SETCLR | IF_TBE;
> mb();
> @@ -362,7 +362,7 @@ static void check_modem_status(struct serial_state *info)
> #if (defined(SERIAL_DEBUG_INTR) || defined(SERIAL_DEBUG_FLOW))
> printk("CTS tx stop...");
> #endif
> - port->tty->hw_stopped = 1;
> + port->tty->hw_stopped = true;
> info->IER &= ~UART_IER_THRI;
> /* disable Tx interrupt and remove any pending interrupts */
> amiga_custom.intena = IF_TBE;
> @@ -1197,7 +1197,7 @@ static void rs_set_termios(struct tty_struct *tty, const struct ktermios *old_te
>
> /* Handle turning off CRTSCTS */
> if ((old_termios->c_cflag & CRTSCTS) && !C_CRTSCTS(tty)) {
> - tty->hw_stopped = 0;
> + tty->hw_stopped = false;
> rs_start(tty);
> }
>
> diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
> index ef3116e87975..10855e66fda1 100644
> --- a/drivers/tty/mxser.c
> +++ b/drivers/tty/mxser.c
> @@ -553,7 +553,7 @@ static void mxser_handle_cts(struct tty_struct *tty, struct mxser_port *info,
>
> if (tty->hw_stopped) {
> if (cts) {
> - tty->hw_stopped = 0;
> + tty->hw_stopped = false;
>
> if (!mxser_16550A_or_MUST(info))
> __mxser_start_tx(info);
> @@ -563,7 +563,7 @@ static void mxser_handle_cts(struct tty_struct *tty, struct mxser_port *info,
> } else if (cts)
> return;
>
> - tty->hw_stopped = 1;
> + tty->hw_stopped = true;
> if (!mxser_16550A_or_MUST(info))
> __mxser_stop_tx(info);
> }
> @@ -1361,7 +1361,7 @@ static void mxser_set_termios(struct tty_struct *tty,
> spin_unlock_irqrestore(&info->slock, flags);
>
> if ((old_termios->c_cflag & CRTSCTS) && !C_CRTSCTS(tty)) {
> - tty->hw_stopped = 0;
> + tty->hw_stopped = false;
> mxser_start(tty);
> }
>
> diff --git a/drivers/tty/synclink_gt.c b/drivers/tty/synclink_gt.c
> index 33f258d6fef9..543b3224dce9 100644
> --- a/drivers/tty/synclink_gt.c
> +++ b/drivers/tty/synclink_gt.c
> @@ -730,7 +730,7 @@ static void set_termios(struct tty_struct *tty,
>
> /* Handle turning off CRTSCTS */
> if ((old_termios->c_cflag & CRTSCTS) && !C_CRTSCTS(tty)) {
> - tty->hw_stopped = 0;
> + tty->hw_stopped = false;
> tx_release(tty);
> }
> }
> @@ -1953,13 +1953,13 @@ static void cts_change(struct slgt_info *info, unsigned short status)
> if (info->port.tty) {
> if (info->port.tty->hw_stopped) {
> if (info->signals & SerialSignal_CTS) {
> - info->port.tty->hw_stopped = 0;
> + info->port.tty->hw_stopped = false;
> info->pending_bh |= BH_TRANSMIT;
> return;
> }
> } else {
> if (!(info->signals & SerialSignal_CTS))
> - info->port.tty->hw_stopped = 1;
> + info->port.tty->hw_stopped = true;
> }
> }
> }
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 093935e97f42..60871a9d3212 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -227,7 +227,7 @@ struct tty_struct {
> unsigned long unused[0];
> } __aligned(sizeof(unsigned long)) ctrl;
>
> - int hw_stopped;
> + bool hw_stopped;
> unsigned int receive_room;
> int flow_change;
>
> --
> 2.30.2
>

2023-03-14 23:10:37

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH 3/8] Bluetooth: hci_ldisc: Fix tty_set_termios() return value assumptions

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Thu, 9 Mar 2023 10:20:30 +0200 you wrote:
> tty_set_termios() never returns anything else than 0. Make the debug
> prints to look directly into the new termios instead to check CRTSCTS
> state.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
> drivers/bluetooth/hci_ldisc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)

Here is the summary with links:
- [3/8] Bluetooth: hci_ldisc: Fix tty_set_termios() return value assumptions
https://git.kernel.org/bluetooth/bluetooth-next/c/dd41882582a9

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html