[PATCH 06/10] drivers: serial: Add xtensa semihosting driver

[Jiaxun Yang]

在2024年5月21日五月 上午3:31，Max Filippov写道：
> On Sun, May 19, 2024 at 1:53 PM Jiaxun Yang <jiaxun.yang at flygoat.com> wrote:
>>
>> Add xtensa semihosting driver.
>>
>> It can't use regular semihosting driver as Xtensa's has it's own
>> semihosting ABI.
>>
>> Note that semihosting supports puts in serial but I never managed to
>> get it work, so it's putc only for now.
>
> I wonder, what were the issues that you experienced?

Never mind, I got it work, it was an off-by-one error in length calculation.
>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang at flygoat.com>
>> ---
>>  drivers/serial/Kconfig                     | 18 ++++++-
>>  drivers/serial/Makefile                    |  1 +
>>  drivers/serial/serial_xtensa_semihosting.c | 79 ++++++++++++++++++++++++++++++
>>  3 files changed, 97 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>> index 1fe4607598eb..3a1e5a6f2877 100644
>> --- a/drivers/serial/Kconfig
>> +++ b/drivers/serial/Kconfig
>> @@ -501,6 +501,15 @@ config DEBUG_UART_MT7620
>>           driver will be available until the real driver model serial is
>>           running.
>>
>> +config DEBUG_UART_XTENSA_SEMIHOSTING
>> +       bool "Xtensa semihosting"
>> +       depends on XTENSA_SEMIHOSTING_SERIAL
>> +       help
>> +         Select this to enable the debug UART using the Xtensa semihosting driver.
>> +         This provides basic serial output from the console without needing to
>> +         start up driver model. The driver will be available until the real
>> +         driver model serial is running.
>> +
>>  endchoice
>>
>>  config DEBUG_UART_BASE
>> @@ -936,7 +945,6 @@ config SH_SCIF_CLK_FREQ
>>  config SEMIHOSTING_SERIAL
>>         bool "Semihosting UART support"
>>         depends on SEMIHOSTING && !SERIAL_RX_BUFFER
>> -       imply SERIAL_PUTS
>>         help
>>           Select this to enable a serial UART using semihosting. Special halt
>>           instructions will be issued which an external debugger (such as a
>> @@ -1115,6 +1123,14 @@ config XEN_SERIAL
>>           If built without DM support, then requires Xen
>>           to be built with CONFIG_VERBOSE_DEBUG.
>>
>> +config XTENSA_SEMIHOSTING_SERIAL
>> +       bool "Xtensa Semihosting UART support"
>> +       depends on DM_SERIAL
>> +       depends on XTENSA_SEMIHOSTING
>> +       imply SERIAL_PUTS
>> +       help
>> +         Select this to enable a serial UART using Xtensa semihosting.
>> +
>>  choice
>>         prompt "Console port"
>>         default 8xx_CONS_SMC1
>> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
>> index dbe598b74064..78810f98367c 100644
>> --- a/drivers/serial/Makefile
>> +++ b/drivers/serial/Makefile
>> @@ -60,6 +60,7 @@ obj-$(CONFIG_MT7620_SERIAL) += serial_mt7620.o
>>  obj-$(CONFIG_HTIF_CONSOLE) += serial_htif.o
>>  obj-$(CONFIG_SIFIVE_SERIAL) += serial_sifive.o
>>  obj-$(CONFIG_XEN_SERIAL) += serial_xen.o
>> +obj-$(CONFIG_XTENSA_SEMIHOSTING_SERIAL) += serial_xtensa_semihosting.o
>>  obj-$(CONFIG_S5P4418_PL011_SERIAL) += serial_s5p4418_pl011.o
>>
>>  ifndef CONFIG_SPL_BUILD
>> diff --git a/drivers/serial/serial_xtensa_semihosting.c b/drivers/serial/serial_xtensa_semihosting.c
>> new file mode 100644
>> index 000000000000..1bf3c18537e7
>> --- /dev/null
>> +++ b/drivers/serial/serial_xtensa_semihosting.c
>> @@ -0,0 +1,79 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2024 Jiaxun Yang <jiaxun.yang at flygoat.com>
>> + */
>> +
>> +#include <dm.h>
>> +#include <malloc.h>
>> +#include <serial.h>
>> +
>> +#include <asm/platform/simcall.h>
>> +
>> +/**
>> + * struct simc_serial_priv - Semihosting serial private data
>> + * @counter: Counter used to fake pending every other call
>> + */
>> +struct simc_serial_priv {
>> +       unsigned int counter;
>> +};
>> +
>> +static int simc_serial_getc(struct udevice *dev)
>> +{
>> +       char ch = 0;
>> +
>> +       simc_read(0, &ch, sizeof(ch));
>> +
>> +       return ch;
>> +}
>> +
>> +static int simc_serial_putc(struct udevice *dev, const char ch)
>> +{
>> +       char str[2] = {0};
>
> Why does it need to be an array if only the first element is ever used?

I want to ensure it's null terminated but it turns out to be unnecessay, will
drop in next version.

>
>> +
>> +       str[0] = ch;
>> +       simc_write(1, str, 1);
>> +
>> +       return 0;
>> +}
>> +
>> +static int simc_serial_pending(struct udevice *dev, bool input)
>> +{
>> +       struct simc_serial_priv *priv = dev_get_priv(dev);
>> +
>> +       if (input)
>> +               return priv->counter++ & 1;
>
> This can result in blocking the whole CPU thread of the emulator when
> used with simcall-based semihosting and there's no input actually available.
> Not sure how it would behave with the gdbio. I think something like
> the following:
>
> if (input) {
>     int res = simc_poll(0);
>     return res < 0 ? priv->counter++ & 1 : res;
> }
>
> could be used here to avoid that.

That worked out, thanks.

>
>> +       return false;
>> +}
>> +
>> +static const struct dm_serial_ops simc_serial_ops = {
>> +       .putc = simc_serial_putc,
>> +       .getc = simc_serial_getc,
>> +       .pending = simc_serial_pending,
>> +};
>> +
>> +U_BOOT_DRIVER(simc_serial) = {
>> +       .name   = "serial_xtensa_semihosting",
>> +       .id     = UCLASS_SERIAL,
>> +       .priv_auto = sizeof(struct simc_serial_priv),
>> +       .ops    = &simc_serial_ops,
>> +       .flags  = DM_FLAG_PRE_RELOC,
>> +};
>> +
>> +U_BOOT_DRVINFO(simc_serial) = {
>> +       .name = "serial_xtensa_semihosting",
>> +};
>> +
>> +#if CONFIG_IS_ENABLED(DEBUG_UART_XTENSA_SEMIHOSTING)
>> +#include <debug_uart.h>
>> +
>> +static inline void _debug_uart_init(void)
>> +{
>> +}
>> +
>> +static inline void _debug_uart_putc(int c)
>> +{
>> +       simc_serial_putc(NULL, c);
>> +}
>> +
>> +DEBUG_UART_FUNCS
>> +#endif
>>
>> --
>> 2.43.0
>>
>
>
> -- 
> Thanks.
> -- Max

-- 
- Jiaxun