2022-03-03 08:32:31

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v4] serial: make uart_console_write->putchar()'s character an unsigned char

On 03/03/2022 09:08, Jiri Slaby wrote:
> Currently, uart_console_write->putchar's second parameter (the
> character) is of type int. It makes little sense, provided uart_console_write()
> accepts the input string as "const char *s" and passes its content -- the
> characters -- to putchar(). So switch the character's type to unsigned
> char.
>
> We don't use char as that is signed on some platforms. That would cause
> troubles for drivers which (implicitly) cast the char to u16 when
> writing to the device. Sign extension would happen in that case and the
> value written would be completely different to the provided char. DZ is
> an example of such a driver -- on MIPS, it uses u16 for dz_out in
> dz_console_putchar().
>
> Note we do the char -> uchar conversion implicitly in
> uart_console_write(). Provided we do not change size of the data type,
> sign extension does not happen there, so the problem is void.
>
> This makes the types consistent and unified with the rest of the uart
> layer, which uses unsigned char in most places already. One exception is
> xmit_buf, but that is going to be converted later.
>
> Signed-off-by: Jiri Slaby <[email protected]>
> Acked-by: Richard Genoud <[email protected]> [atmel_serial]
> Acked-by: Uwe Kleine-König <[email protected]>
> Cc: Paul Cercueil <[email protected]>
> Cc: Tobias Klauser <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Vineet Gupta <[email protected]>
> Cc: Nicolas Ferre <[email protected]>
> Cc: Alexandre Belloni <[email protected]>
> Cc: Ludovic Desroches <[email protected]>
> Cc: Florian Fainelli <[email protected]>
> Cc: [email protected]
> Cc: Alexander Shiyan <[email protected]>
> Cc: Baruch Siach <[email protected]>
> Cc: "Maciej W. Rozycki" <[email protected]>
> Cc: Paul Walmsley <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Albert Ou <[email protected]>
> Cc: Shawn Guo <[email protected]>
> Cc: Sascha Hauer <[email protected]>
> Cc: Pengutronix Kernel Team <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: NXP Linux Team <[email protected]>
> Cc: Karol Gugala <[email protected]>
> Cc: Mateusz Holenko <[email protected]>
> Cc: Vladimir Zapolskiy <[email protected]>
> Cc: Neil Armstrong <[email protected]>
> Cc: Kevin Hilman <[email protected]>
> Cc: Jerome Brunet <[email protected]>
> Cc: Martin Blumenstingl <[email protected]>
> Cc: Taichi Sugaya <[email protected]>
> Cc: Takao Orito <[email protected]>
> Cc: Liviu Dudau <[email protected]>
> Cc: Sudeep Holla <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: "Andreas Färber" <[email protected]>
> Cc: Manivannan Sadhasivam <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Andy Gross <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Orson Zhai <[email protected]>
> Cc: Baolin Wang <[email protected]>
> Cc: Chunyan Zhang <[email protected]>
> Cc: Patrice Chotard <[email protected]>
> Cc: Maxime Coquelin <[email protected]>
> Cc: Alexandre Torgue <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Peter Korsgaard <[email protected]>
> Cc: Michal Simek <[email protected]>
> ---
> [v2] make it unsigned
> [v3] fix also sunsab
> [v4] fix also sunplus (added quite recently)
>
> drivers/tty/goldfish.c | 2 +-
> drivers/tty/hvc/hvc_dcc.c | 2 +-
> drivers/tty/serial/21285.c | 2 +-
> drivers/tty/serial/8250/8250_early.c | 2 +-
> drivers/tty/serial/8250/8250_ingenic.c | 2 +-
> drivers/tty/serial/8250/8250_port.c | 2 +-
> drivers/tty/serial/altera_jtaguart.c | 4 ++--
> drivers/tty/serial/altera_uart.c | 2 +-
> drivers/tty/serial/amba-pl010.c | 2 +-
> drivers/tty/serial/amba-pl011.c | 6 +++---
> drivers/tty/serial/apbuart.c | 2 +-
> drivers/tty/serial/ar933x_uart.c | 2 +-
> drivers/tty/serial/arc_uart.c | 2 +-
> drivers/tty/serial/atmel_serial.c | 2 +-
> drivers/tty/serial/bcm63xx_uart.c | 2 +-
> drivers/tty/serial/clps711x.c | 2 +-
> drivers/tty/serial/digicolor-usart.c | 2 +-
> drivers/tty/serial/dz.c | 2 +-
> drivers/tty/serial/earlycon-arm-semihost.c | 2 +-
> drivers/tty/serial/earlycon-riscv-sbi.c | 2 +-
> drivers/tty/serial/fsl_linflexuart.c | 4 ++--
> drivers/tty/serial/fsl_lpuart.c | 4 ++--
> drivers/tty/serial/imx.c | 2 +-
> drivers/tty/serial/imx_earlycon.c | 2 +-
> drivers/tty/serial/ip22zilog.c | 2 +-
> drivers/tty/serial/lantiq.c | 2 +-
> drivers/tty/serial/liteuart.c | 2 +-
> drivers/tty/serial/lpc32xx_hs.c | 2 +-
> drivers/tty/serial/meson_uart.c | 2 +-
> drivers/tty/serial/milbeaut_usio.c | 2 +-
> drivers/tty/serial/mps2-uart.c | 4 ++--
> drivers/tty/serial/mvebu-uart.c | 4 ++--
> drivers/tty/serial/mxs-auart.c | 2 +-
> drivers/tty/serial/omap-serial.c | 4 ++--
> drivers/tty/serial/owl-uart.c | 2 +-
> drivers/tty/serial/pch_uart.c | 2 +-
> drivers/tty/serial/pic32_uart.c | 2 +-
> drivers/tty/serial/pmac_zilog.c | 2 +-
> drivers/tty/serial/pxa.c | 2 +-
> drivers/tty/serial/qcom_geni_serial.c | 2 +-
> drivers/tty/serial/rda-uart.c | 2 +-
> drivers/tty/serial/sa1100.c | 2 +-
> drivers/tty/serial/samsung_tty.c | 4 ++--
> drivers/tty/serial/sb1250-duart.c | 2 +-
> drivers/tty/serial/sccnxp.c | 2 +-
> drivers/tty/serial/serial_core.c | 2 +-
> drivers/tty/serial/serial_txx9.c | 2 +-
> drivers/tty/serial/sh-sci.c | 2 +-
> drivers/tty/serial/sifive.c | 4 ++--
> drivers/tty/serial/sprd_serial.c | 4 ++--
> drivers/tty/serial/st-asc.c | 2 +-
> drivers/tty/serial/stm32-usart.c | 2 +-
> drivers/tty/serial/sunplus-uart.c | 5 +++--
> drivers/tty/serial/sunsab.c | 2 +-
> drivers/tty/serial/sunsu.c | 2 +-
> drivers/tty/serial/sunzilog.c | 4 ++--
> drivers/tty/serial/uartlite.c | 4 ++--
> drivers/tty/serial/vr41xx_siu.c | 2 +-
> drivers/tty/serial/vt8500_serial.c | 2 +-
> drivers/tty/serial/xilinx_uartps.c | 2 +-
> drivers/tty/serial/zs.c | 2 +-
> include/linux/serial_core.h | 2 +-
> 62 files changed, 77 insertions(+), 76 deletions(-)
>

[...]

> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -516,7 +516,7 @@ static void meson_uart_enable_tx_engine(struct uart_port *port)
> writel(val, port->membase + AML_UART_CONTROL);
> }
>
> -static void meson_console_putchar(struct uart_port *port, int ch)
> +static void meson_console_putchar(struct uart_port *port, unsigned char ch)
> {
> if (!port->membase)
> return;

[...]
Acked-by: Neil Armstrong <[email protected]> # meson_serial