| Message ID | 20240519-qemu-xtensa-v1-6-8fff0cb11c19@flygoat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | xtensa: Enable qemu-xtensa board | expand |
On Sun, May 19, 2024 at 1:53 PM Jiaxun Yang <jiaxun.yang@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? > Signed-off-by: Jiaxun Yang <jiaxun.yang@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@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? > + > + 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. > + 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 >
在2024年5月21日五月 上午3:31，Max Filippov写道： > On Sun, May 19, 2024 at 1:53 PM Jiaxun Yang <jiaxun.yang@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@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@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
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@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}; + + 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; + 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
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. Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> --- drivers/serial/Kconfig | 18 ++++++- drivers/serial/Makefile | 1 + drivers/serial/serial_xtensa_semihosting.c | 79 ++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 1 deletion(-)