Re: [PATCH v2 1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h

[Bin Meng]

On 2022/12/8 14:08:33, "Jiri Slaby" <jirislaby@xxxxxxxxxx> wrote:

> On 07. 12. 22, 14:53, Bin Meng wrote:
> > Move smh_putc() variants in respective arch/*/include/asm/semihost.h,
> > in preparation to add RISC-V support.
> >
> > Signed-off-by: Bin Meng <bmeng@xxxxxxxxxxx>
> ...
> > --- /dev/null
> > +++ b/arch/arm/include/asm/semihost.h
> > @@ -0,0 +1,23 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2012 ARM Ltd.
> > + * Author: Marc Zyngier <marc.zyngier@xxxxxxx>
> > + *
> > + * Adapted for ARM and earlycon:
> > + * Copyright (C) 2014 Linaro Ltd.
> > + * Author: Rob Herring <robh@xxxxxxxxxx>
> > + */
>
> Much better. There are three minor issues:
> 1) protection against multiple #include-s is missing here.

Oops, will add in v3.

> > +#ifdef CONFIG_THUMB2_KERNEL
> > +#define SEMIHOST_SWI	"0xab"
> > +#else
> > +#define SEMIHOST_SWI	"0x123456"
> > +#endif
> > +
> > +static inline void smh_putc(struct uart_port *port, unsigned char c)
>
> 2) port is unused in all implementations. So it should be dropped.
> 3) can you make "c" an explicit u8?

The smh_putc function signature is defined by the uart_console_write 
helper. I don't think we can change it.

> > +{
> > +	asm volatile("mov  r1, %0\n"
> > +		     "mov  r0, #3\n"
> > +		     "svc  " SEMIHOST_SWI "\n"
> > +		     : : "r" (&c) : "r0", "r1", "memory");
> > +}

Regards,
Bin