Re: [PATCH v3] serial: make uart_console_write->putchar()'s character an unsigned char

[Jiri Slaby]

On 03. 03. 22, 0:57, David Laight wrote:
> From: Uwe Kleine-König
> > Sent: 02 March 2022 17:53
> >
> > On Wed, Mar 02, 2022 at 08:27:32AM +0100, 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().
> >
> > I always thought this was bigger than 8bit for hardware that supports
> > wider characters. But if that's the case that's completely unsupported,
> > there isn't even CS9.
>
> The real problem is that using char (or short) for a function parameter
> or result is very likely to require the compile add code to mask
> the value to 8 (or 16) bits.
>
> Remember that almost every time you do anything with a signed or unsigned
> char/short variable the compiler has to use the integer promotion rules
> to convert the value to int.
>
> You'll almost certainly get better code if the value is left in an
> int (or unsigned int) variable until the low 8 bits get written to
> a buffer (or hardware register).

So should we use int/uint instead of more appropriate shorter types 
everywhere now? The answer is: definitely not. The assembly on x86 looks 
good (it uses movz, no ands), RISC architectures have to do what they 
chose to.

Note the patch unifies the type with preexistent uchar use in the whole 
uart layer. I.e. before the patch, char was upcast to int, and later, it 
was downcast to u8 again when talking to HW.

thanks,
--
js
suse labs