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

21 May 2024

在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
-- 
- Jiaxun

    