Re: [PATCH 10/11] serial: make uart_console_write->putchar()'s character a char

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 26. 01. 22, 18:57, Maciej W. Rozycki wrote:
On Mon, 24 Jan 2022, Jiri Slaby wrote:

diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index e9edabc5a211..3493e201d67f 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -802,7 +802,7 @@ static void __init dz_init_ports(void)
   * restored.  Welcome to the world of PDP-11!
   * -------------------------------------------------------------------
   */
-static void dz_console_putchar(struct uart_port *uport, int ch)
+static void dz_console_putchar(struct uart_port *uport, char ch)
  {
  	struct dz_port *dport = to_dport(uport);
  	unsigned long flags;

  Hmm, this is unsafe, because on the MIPS target the lone `char' type is
signed and therefore a call to `->putchar' will see `ch' sign-extended
from bit #7 to the width of the argument register used.  Which means that
if a character is sent to the console that has its bit #7 set, then the
call to:

		dz_out(dport, DZ_TDR, ch);

i.e.:

static void dz_out(struct dz_port *dport, unsigned offset, u16 value)

will send a value to DZ_TDR with bits #15:8 set to all-ones.  And bits
#11:8 there are the BREAK control bits, active high, for serial lines #3:0
respectively.

  We could handle this with a preparatory change by calling:

		dz_out(dport, DZ_TDR, ch & 0xffu);

instead, but perhaps `->putchar' should simply take `unsigned char' or
maybe even `u8' as its third argument?

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index c58cc142d23f..68e62703eaa6 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -399,7 +399,7 @@ int uart_set_options(struct uart_port *port, struct console
*co, int baud,
  struct tty_driver *uart_console_device(struct console *co, int *index);
  void uart_console_write(struct uart_port *port, const char *s,
  			unsigned int count,
-			void (*putchar)(struct uart_port *, int));
+			void (*putchar)(struct uart_port *, char));
/*
   * Port/driver registration/removal

  I.e.:

			void (*putchar)(struct uart_port *, unsigned char));

I can see we get it right already with:

	unsigned char		x_char;			/* xon/xoff char */

and for `dz_transmit_chars' we have:

	unsigned char tmp;
[...]
	tmp = xmit->buf[xmit->tail];
	xmit->tail = (xmit->tail + 1) & (DZ_XMIT_SIZE - 1);
	dz_out(dport, DZ_TDR, tmp);

(because `struct circ_buf' is generic and not limited to unsigned buffer
contents interpretation; it's not clear to me if that has been intended
though).

Thanks, good point. I was considering unsigned char and concluded not go that path right now as the rest of the console world uses char -- starting from the printk code. OTOH the whole uart world uses 'unsigned char' except that circ_buf. But I am phasing circ_buf out in favor of kfifo+unsigned-char anyway.

So let me switch the whole uart (the console code in particular) to an unsigned world. Meaning printk will pass char, uart will receive it as unsigned char and will keep it unsigned in all cases.

thanks,
--
js
suse labs



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux