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

[Maciej W. Rozycki]

On Thu, 3 Mar 2022, Paul Cercueil wrote:

> >  We do have an issue, because we still have this:
> > 
> > void uart_console_write(struct uart_port *port, const char *s,
> > 			unsigned int count,
> > 			void (*putchar)(struct uart_port *, int))
> > 
> > and then:
> > 
> > 		putchar(port, *s);
> > 
> > there.  Consequently on targets where plain `char' type is signed the
> > value retrieved from `*s' has to be truncated in the call to `putchar'.
> > And indeed it happens with the MIPS target:
> > 
> > 803ae47c:	82050000 	lb	a1,0(s0)
> > 803ae480:	26100001 	addiu	s0,s0,1
> > 803ae484:	02402025 	move	a0,s2
> > 803ae488:	0220f809 	jalr	s1
> > 803ae48c:	30a500ff 	andi	a1,a1,0xff
> > 
> > vs current code:
> > 
> > 803ae47c:	82050000 	lb	a1,0(s0)
> > 803ae480:	26100001 	addiu	s0,s0,1
> > 803ae484:	0220f809 	jalr	s1
> > 803ae488:	02402025 	move	a0,s2
> 
> And how is that at all a problem?

 It wastes an instruction.  An instruction wasted here, an instruction 
wasted there, and suddenly we have grown bloatware. :(

> >  So I'd recommend changing `s' here to `const unsigned char *' or, as I
> > previously suggested, maybe to `const u8 *' even.
> 
> Just cast the string to "const u8 *" within the function, while keeping a
> "const char *s" argument. The compiler will then most likely generate LBUs.

 It does, but, oh dear, it's a "solution" to a problem we have created in 
the first place.  Why do we ever want to have signed characters in the TTY 
layer, and then to vary between platforms?  It's asking for portability 
issues.

  Maciej