2023-07-12 08:44:56

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 00/10] tty: u8 conversion preparation

This is another round of preparations for conversions of chars and flags
to u8. This series converts sysrq handlers and serial_core + serial
drivers. The tty proper will be posted separately later.

Jiri Slaby (SUSE) (10):
tty: sysrq: rename and re-type i in sysrq_handle_loglevel()
tty: sysrq: switch sysrq handlers from int to u8
tty: sysrq: switch the rest of keys to u8
tty: sysrq: use switch in sysrq_key_table_key2index()
serial: convert uart sysrq handling to u8
serial: make uart_insert_char() accept u8s
serial: pass state to __uart_start() directly
serial: arc_uart: simplify flags handling in arc_serial_rx_chars()
serial: omap-serial: remove flag from serial_omap_rdi()
serial: drivers: switch ch and flag to u8

arch/alpha/kernel/setup.c | 2 +-
arch/loongarch/kernel/sysrq.c | 2 +-
arch/mips/kernel/sysrq.c | 2 +-
arch/powerpc/xmon/xmon.c | 2 +-
arch/sparc/kernel/process_64.c | 4 +-
drivers/gpu/drm/drm_fb_helper.c | 2 +-
drivers/tty/serial/21285.c | 3 +-
drivers/tty/serial/8250/8250_port.c | 3 +-
drivers/tty/serial/altera_jtaguart.c | 2 +-
drivers/tty/serial/altera_uart.c | 2 +-
drivers/tty/serial/amba-pl010.c | 3 +-
drivers/tty/serial/amba-pl011.c | 3 +-
drivers/tty/serial/apbuart.c | 3 +-
drivers/tty/serial/arc_uart.c | 29 +++++-----
drivers/tty/serial/atmel_serial.c | 2 +-
drivers/tty/serial/clps711x.c | 3 +-
drivers/tty/serial/digicolor-usart.c | 3 +-
drivers/tty/serial/dz.c | 2 +-
drivers/tty/serial/ip22zilog.c | 2 +-
drivers/tty/serial/max3100.c | 3 +-
drivers/tty/serial/max310x.c | 3 +-
drivers/tty/serial/mcf.c | 2 +-
drivers/tty/serial/milbeaut_usio.c | 3 +-
drivers/tty/serial/mxs-auart.c | 3 +-
drivers/tty/serial/omap-serial.c | 8 +--
drivers/tty/serial/pxa.c | 2 +-
drivers/tty/serial/rp2.c | 4 +-
drivers/tty/serial/sa1100.c | 3 +-
drivers/tty/serial/samsung_tty.c | 3 +-
drivers/tty/serial/sb1250-duart.c | 3 +-
drivers/tty/serial/sc16is7xx.c | 3 +-
drivers/tty/serial/sccnxp.c | 3 +-
drivers/tty/serial/serial-tegra.c | 7 +--
drivers/tty/serial/serial_core.c | 15 +++--
drivers/tty/serial/serial_txx9.c | 3 +-
drivers/tty/serial/sifive.c | 2 +-
drivers/tty/serial/sprd_serial.c | 5 +-
drivers/tty/serial/st-asc.c | 2 +-
drivers/tty/serial/stm32-usart.c | 5 +-
drivers/tty/serial/sunplus-uart.c | 2 +-
drivers/tty/serial/zs.c | 3 +-
drivers/tty/sysrq.c | 84 ++++++++++++++--------------
include/linux/serial_core.h | 18 +++---
include/linux/sysrq.h | 18 +++---
kernel/debug/debug_core.c | 2 +-
kernel/power/poweroff.c | 2 +-
kernel/rcu/tree_stall.h | 2 +-
47 files changed, 142 insertions(+), 145 deletions(-)

--
2.41.0



2023-07-12 08:47:28

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 08/10] serial: arc_uart: simplify flags handling in arc_serial_rx_chars()

* move the declaration of flg (with the initializer) to the loop, so
there is no need to reset it to TTY_NORMAL by an 'else' branch.
* use TTY_NORMAL as initializer above, not a magic zero constant
* remove the outer 'if' from this construct:
if (S & (A | B)) {
if (S & A)
X;
if (S & B)
Y;
}
* drop unlikely() as I doubt it has any benefits here. If it does,
provide numbers.

All four make the code easier to read.

Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
Cc: Vineet Gupta <[email protected]>
---
drivers/tty/serial/arc_uart.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/serial/arc_uart.c b/drivers/tty/serial/arc_uart.c
index 4b2512eef577..835903488acb 100644
--- a/drivers/tty/serial/arc_uart.c
+++ b/drivers/tty/serial/arc_uart.c
@@ -195,8 +195,6 @@ static void arc_serial_start_tx(struct uart_port *port)

static void arc_serial_rx_chars(struct uart_port *port, unsigned int status)
{
- unsigned int ch, flg = 0;
-
/*
* UART has 4 deep RX-FIFO. Driver's recongnition of this fact
* is very subtle. Here's how ...
@@ -207,24 +205,23 @@ static void arc_serial_rx_chars(struct uart_port *port, unsigned int status)
* controller, which is indeed the Rx-FIFO.
*/
do {
+ unsigned int ch, flg = TTY_NORMAL;
+
/*
* This could be an Rx Intr for err (no data),
* so check err and clear that Intr first
*/
- if (unlikely(status & (RXOERR | RXFERR))) {
- if (status & RXOERR) {
- port->icount.overrun++;
- flg = TTY_OVERRUN;
- UART_CLR_STATUS(port, RXOERR);
- }
-
- if (status & RXFERR) {
- port->icount.frame++;
- flg = TTY_FRAME;
- UART_CLR_STATUS(port, RXFERR);
- }
- } else
- flg = TTY_NORMAL;
+ if (status & RXOERR) {
+ port->icount.overrun++;
+ flg = TTY_OVERRUN;
+ UART_CLR_STATUS(port, RXOERR);
+ }
+
+ if (status & RXFERR) {
+ port->icount.frame++;
+ flg = TTY_FRAME;
+ UART_CLR_STATUS(port, RXFERR);
+ }

if (status & RXEMPTY)
continue;
--
2.41.0


2023-07-12 08:53:27

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 04/10] tty: sysrq: use switch in sysrq_key_table_key2index()

Using switch with range cases makes the code more aligned and readable.
Expand also that 36 as explicit addition of 10 + 26 to make the source
of the constant more obvious.

Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/sysrq.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index ccf429ba348e..23198e3f1461 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -532,17 +532,16 @@ static const struct sysrq_key_op *sysrq_key_table[62] = {
/* key2index calculation, -1 on invalid index */
static int sysrq_key_table_key2index(u8 key)
{
- int retval;
-
- if ((key >= '0') && (key <= '9'))
- retval = key - '0';
- else if ((key >= 'a') && (key <= 'z'))
- retval = key + 10 - 'a';
- else if ((key >= 'A') && (key <= 'Z'))
- retval = key + 36 - 'A';
- else
- retval = -1;
- return retval;
+ switch (key) {
+ case '0' ... '9':
+ return key - '0';
+ case 'a' ... 'z':
+ return key - 'a' + 10;
+ case 'A' ... 'Z':
+ return key - 'A' + 10 + 26;
+ default:
+ return -1;
+ }
}

/*
--
2.41.0


2023-07-12 08:54:05

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 05/10] serial: convert uart sysrq handling to u8

Propagate u8 from the sysrq code further up to serial's
uart_handle_sysrq_char() and friends.

Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/serial/serial_core.c | 4 ++--
include/linux/serial_core.h | 16 ++++++++--------
2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 831d033611e6..7e37db9adbd4 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3505,7 +3505,7 @@ void uart_insert_char(struct uart_port *port, unsigned int status,
EXPORT_SYMBOL_GPL(uart_insert_char);

#ifdef CONFIG_MAGIC_SYSRQ_SERIAL
-static const char sysrq_toggle_seq[] = CONFIG_MAGIC_SYSRQ_SERIAL_SEQUENCE;
+static const u8 sysrq_toggle_seq[] = CONFIG_MAGIC_SYSRQ_SERIAL_SEQUENCE;

static void uart_sysrq_on(struct work_struct *w)
{
@@ -3528,7 +3528,7 @@ static DECLARE_WORK(sysrq_enable_work, uart_sysrq_on);
* Returns: %false if @ch is out of enabling sequence and should be
* handled some other way, %true if @ch was consumed.
*/
-bool uart_try_toggle_sysrq(struct uart_port *port, unsigned int ch)
+bool uart_try_toggle_sysrq(struct uart_port *port, u8 ch)
{
int sysrq_toggle_seq_len = strlen(sysrq_toggle_seq);

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 6d58c57acdaa..14dd85ee849e 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -569,7 +569,7 @@ struct uart_port {
struct serial_port_device *port_dev; /* serial core port device */

unsigned long sysrq; /* sysrq timeout */
- unsigned int sysrq_ch; /* char for sysrq */
+ u8 sysrq_ch; /* char for sysrq */
unsigned char has_sysrq;
unsigned char sysrq_seq; /* index in sysrq_toggle_seq */

@@ -910,9 +910,9 @@ void uart_xchar_out(struct uart_port *uport, int offset);
#ifdef CONFIG_MAGIC_SYSRQ_SERIAL
#define SYSRQ_TIMEOUT (HZ * 5)

-bool uart_try_toggle_sysrq(struct uart_port *port, unsigned int ch);
+bool uart_try_toggle_sysrq(struct uart_port *port, u8 ch);

-static inline int uart_handle_sysrq_char(struct uart_port *port, unsigned int ch)
+static inline int uart_handle_sysrq_char(struct uart_port *port, u8 ch)
{
if (!port->sysrq)
return 0;
@@ -931,7 +931,7 @@ static inline int uart_handle_sysrq_char(struct uart_port *port, unsigned int ch
return 0;
}

-static inline int uart_prepare_sysrq_char(struct uart_port *port, unsigned int ch)
+static inline int uart_prepare_sysrq_char(struct uart_port *port, u8 ch)
{
if (!port->sysrq)
return 0;
@@ -952,7 +952,7 @@ static inline int uart_prepare_sysrq_char(struct uart_port *port, unsigned int c

static inline void uart_unlock_and_check_sysrq(struct uart_port *port)
{
- int sysrq_ch;
+ u8 sysrq_ch;

if (!port->has_sysrq) {
spin_unlock(&port->lock);
@@ -971,7 +971,7 @@ static inline void uart_unlock_and_check_sysrq(struct uart_port *port)
static inline void uart_unlock_and_check_sysrq_irqrestore(struct uart_port *port,
unsigned long flags)
{
- int sysrq_ch;
+ u8 sysrq_ch;

if (!port->has_sysrq) {
spin_unlock_irqrestore(&port->lock, flags);
@@ -987,11 +987,11 @@ static inline void uart_unlock_and_check_sysrq_irqrestore(struct uart_port *port
handle_sysrq(sysrq_ch);
}
#else /* CONFIG_MAGIC_SYSRQ_SERIAL */
-static inline int uart_handle_sysrq_char(struct uart_port *port, unsigned int ch)
+static inline int uart_handle_sysrq_char(struct uart_port *port, u8 ch)
{
return 0;
}
-static inline int uart_prepare_sysrq_char(struct uart_port *port, unsigned int ch)
+static inline int uart_prepare_sysrq_char(struct uart_port *port, u8 ch)
{
return 0;
}
--
2.41.0


2023-07-12 08:56:13

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 01/10] tty: sysrq: rename and re-type i in sysrq_handle_loglevel()

'i' is a too generic name for something which carries a 'loglevel'. Name
it as such and make it 'u8', the same as key will become in the next
patches.

Note that we are not stripping any high bits away, 'key' is given only
8bit values.

Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/sysrq.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index e1df63a88aac..13465e4cca9b 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -100,12 +100,11 @@ __setup("sysrq_always_enabled", sysrq_always_enabled_setup);

static void sysrq_handle_loglevel(int key)
{
- int i;
+ u8 loglevel = key - '0';

- i = key - '0';
console_loglevel = CONSOLE_LOGLEVEL_DEFAULT;
- pr_info("Loglevel set to %d\n", i);
- console_loglevel = i;
+ pr_info("Loglevel set to %u\n", loglevel);
+ console_loglevel = loglevel;
}
static const struct sysrq_key_op sysrq_loglevel_op = {
.handler = sysrq_handle_loglevel,
--
2.41.0


2023-07-12 08:56:47

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 07/10] serial: pass state to __uart_start() directly

__uart_start() does not need a tty struct. It works only with
uart_state. So pass the latter directly.

Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/serial/serial_core.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index bef507cb804c..306ea1a560e6 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -133,9 +133,8 @@ static void uart_stop(struct tty_struct *tty)
uart_port_unlock(port, flags);
}

-static void __uart_start(struct tty_struct *tty)
+static void __uart_start(struct uart_state *state)
{
- struct uart_state *state = tty->driver_data;
struct uart_port *port = state->uart_port;
struct serial_port_device *port_dev;
int err;
@@ -170,7 +169,7 @@ static void uart_start(struct tty_struct *tty)
unsigned long flags;

port = uart_port_lock(state, flags);
- __uart_start(tty);
+ __uart_start(state);
uart_port_unlock(port, flags);
}

@@ -239,7 +238,7 @@ static void uart_change_line_settings(struct tty_struct *tty, struct uart_state
if (!old_hw_stopped)
uport->ops->stop_tx(uport);
else
- __uart_start(tty);
+ __uart_start(state);
}
spin_unlock_irq(&uport->lock);
}
@@ -619,7 +618,7 @@ static int uart_write(struct tty_struct *tty,
ret += c;
}

- __uart_start(tty);
+ __uart_start(state);
uart_port_unlock(port, flags);
return ret;
}
--
2.41.0


2023-07-12 08:59:47

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 03/10] tty: sysrq: switch the rest of keys to u8

Propagate u8 more from the bottom to the interface, so that sysrq
callers (usually drivers) see that u8 is expected.

Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/tty/sysrq.c | 16 ++++++++--------
include/linux/sysrq.h | 16 ++++++++--------
2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 1271a82c0887..ccf429ba348e 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -530,7 +530,7 @@ static const struct sysrq_key_op *sysrq_key_table[62] = {
};

/* key2index calculation, -1 on invalid index */
-static int sysrq_key_table_key2index(int key)
+static int sysrq_key_table_key2index(u8 key)
{
int retval;

@@ -548,7 +548,7 @@ static int sysrq_key_table_key2index(int key)
/*
* get and put functions for the table, exposed to modules.
*/
-static const struct sysrq_key_op *__sysrq_get_key_op(int key)
+static const struct sysrq_key_op *__sysrq_get_key_op(u8 key)
{
const struct sysrq_key_op *op_p = NULL;
int i;
@@ -560,7 +560,7 @@ static const struct sysrq_key_op *__sysrq_get_key_op(int key)
return op_p;
}

-static void __sysrq_put_key_op(int key, const struct sysrq_key_op *op_p)
+static void __sysrq_put_key_op(u8 key, const struct sysrq_key_op *op_p)
{
int i = sysrq_key_table_key2index(key);

@@ -568,7 +568,7 @@ static void __sysrq_put_key_op(int key, const struct sysrq_key_op *op_p)
sysrq_key_table[i] = op_p;
}

-void __handle_sysrq(int key, bool check_mask)
+void __handle_sysrq(u8 key, bool check_mask)
{
const struct sysrq_key_op *op_p;
int orig_log_level;
@@ -627,7 +627,7 @@ void __handle_sysrq(int key, bool check_mask)
suppress_printk = orig_suppress_printk;
}

-void handle_sysrq(int key)
+void handle_sysrq(u8 key)
{
if (sysrq_on())
__handle_sysrq(key, true);
@@ -1111,7 +1111,7 @@ int sysrq_toggle_support(int enable_mask)
}
EXPORT_SYMBOL_GPL(sysrq_toggle_support);

-static int __sysrq_swap_key_ops(int key, const struct sysrq_key_op *insert_op_p,
+static int __sysrq_swap_key_ops(u8 key, const struct sysrq_key_op *insert_op_p,
const struct sysrq_key_op *remove_op_p)
{
int retval;
@@ -1135,13 +1135,13 @@ static int __sysrq_swap_key_ops(int key, const struct sysrq_key_op *insert_op_p,
return retval;
}

-int register_sysrq_key(int key, const struct sysrq_key_op *op_p)
+int register_sysrq_key(u8 key, const struct sysrq_key_op *op_p)
{
return __sysrq_swap_key_ops(key, op_p, NULL);
}
EXPORT_SYMBOL(register_sysrq_key);

-int unregister_sysrq_key(int key, const struct sysrq_key_op *op_p)
+int unregister_sysrq_key(u8 key, const struct sysrq_key_op *op_p)
{
return __sysrq_swap_key_ops(key, NULL, op_p);
}
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index bb8d07814b0e..bdca467ebb77 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -43,10 +43,10 @@ struct sysrq_key_op {
* are available -- else NULL's).
*/

-void handle_sysrq(int key);
-void __handle_sysrq(int key, bool check_mask);
-int register_sysrq_key(int key, const struct sysrq_key_op *op);
-int unregister_sysrq_key(int key, const struct sysrq_key_op *op);
+void handle_sysrq(u8 key);
+void __handle_sysrq(u8 key, bool check_mask);
+int register_sysrq_key(u8 key, const struct sysrq_key_op *op);
+int unregister_sysrq_key(u8 key, const struct sysrq_key_op *op);
extern const struct sysrq_key_op *__sysrq_reboot_op;

int sysrq_toggle_support(int enable_mask);
@@ -54,20 +54,20 @@ int sysrq_mask(void);

#else

-static inline void handle_sysrq(int key)
+static inline void handle_sysrq(u8 key)
{
}

-static inline void __handle_sysrq(int key, bool check_mask)
+static inline void __handle_sysrq(u8 key, bool check_mask)
{
}

-static inline int register_sysrq_key(int key, const struct sysrq_key_op *op)
+static inline int register_sysrq_key(u8 key, const struct sysrq_key_op *op)
{
return -EINVAL;
}

-static inline int unregister_sysrq_key(int key, const struct sysrq_key_op *op)
+static inline int unregister_sysrq_key(u8 key, const struct sysrq_key_op *op)
{
return -EINVAL;
}
--
2.41.0


2023-07-13 01:36:10

by Vineet Gupta

[permalink] [raw]
Subject: Re: [PATCH 08/10] serial: arc_uart: simplify flags handling in arc_serial_rx_chars()



On 7/12/23 01:18, Jiri Slaby (SUSE) wrote:
> * move the declaration of flg (with the initializer) to the loop, so
> there is no need to reset it to TTY_NORMAL by an 'else' branch.
> * use TTY_NORMAL as initializer above, not a magic zero constant
> * remove the outer 'if' from this construct:
> if (S & (A | B)) {
> if (S & A)
> X;
> if (S & B)
> Y;
> }
> * drop unlikely() as I doubt it has any benefits here. If it does,
> provide numbers.
>
> All four make the code easier to read.
>
> Signed-off-by: Jiri Slaby (SUSE)<[email protected]>
> Cc: Vineet Gupta<[email protected]>

Thanks for the cleanup.

Acked-by: Vineet Gupta <[email protected]>

Th,
-Vineet