casaroli commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r4441536138


##########
arch/arm64/src/qemu/qemu_serial.c:
##########
@@ -37,11 +37,111 @@
 #include <nuttx/serial/uart_pl011.h>
 #include "arm64_internal.h"
 
-#ifdef USE_SERIALDRIVER
+/***************************************************************************
+ * Pre-processor definitions
+ ***************************************************************************/
+
+#if defined(CONFIG_UART0_SERIAL_CONSOLE)
+  #define CONSOLE_DEV g_pl011_port0
+#elif defined(CONFIG_UART1_SERIAL_CONSOLE)
+  #define CONSOLE_DEV g_pl011_port1
+#endif
+
+/* QEMU-specific configuration parameters */
+
+#define UART0_BASEADDR (0x9000000)
+#define UART0_CLK_FREQ (24000000)
+#define UART0_IRQ (33)
+
+#define UART1_BASEADDR (0x9040000)
+#define UART1_CLK_FREQ (24000000)
+#define UART1_IRQ (40)
+
+/***************************************************************************
+ * Private data
+ ***************************************************************************/
+
+#ifdef CONFIG_UART0_SERIALDRIVER
+static char g_uart0_rx_buf[CONFIG_UART0_RXBUFSIZE];
+static char g_uart0_tx_buf[CONFIG_UART0_TXBUFSIZE];
+
+static struct pl011_uart_port_s g_pl011_port0 =
+{
+  .config =
+    {
+      .baseaddr = (void *)UART0_BASEADDR,
+      .baud_rate = CONFIG_UART0_BAUD,
+      .irq_num = UART0_IRQ,
+      .sbsa = false,
+      .sys_clk_freq = UART0_CLK_FREQ,
+    },
+
+  .uart =
+    {
+      .recv =
+        {
+          .buffer = g_uart0_rx_buf,
+          .size = CONFIG_UART0_RXBUFSIZE,
+        },
+      .xmit =
+        {
+          .buffer = g_uart0_tx_buf,
+          .size = CONFIG_UART0_TXBUFSIZE,
+        },
+    },
+};
+#endif
+
+#ifdef CONFIG_UART1_SERIALDRIVER
+static char g_uart1_rx_buf[CONFIG_UART1_RXBUFSIZE];
+static char g_uart1_tx_buf[CONFIG_UART1_TXBUFSIZE];
+
+static struct pl011_uart_port_s g_pl011_port1 =
+{
+  .config =
+    {
+      .baseaddr = (void *)UART1_BASEADDR,
+      .baud_rate = CONFIG_UART1_BAUD,
+      .irq_num = UART1_IRQ,
+      .sbsa = false,
+      .sys_clk_freq = UART1_CLK_FREQ,
+    },
+
+  .uart =
+    {
+      .recv =
+        {
+          .buffer = g_uart1_rx_buf,
+          .size = CONFIG_UART1_RXBUFSIZE,
+        },
+      .xmit =
+        {
+          .buffer = g_uart1_tx_buf,
+          .size = CONFIG_UART1_TXBUFSIZE,
+        },
+    },
+};
+#endif
+
 /***************************************************************************
  * Public Functions
  ***************************************************************************/
 
+/***************************************************************************
+ * Name: up_putc
+ *
+ * Description:
+ *   Provide priority, low-level access to support OS debug writes
+ *
+ ***************************************************************************/
+
+#ifdef CONSOLE_DEV
+void up_putc(int ch)

Review Comment:
   `up_putc()` passes the console to `pl011_putc()`, which uses `dev->priv`, 
but `dev->priv` is set only by `pl011_dev_init()` in `arm64_earlyserialinit()`. 
`arm64_chip_boot()` calls `arm64_psci_init()` before that, and its `sinfo()` 
goes to `up_putc()`: with `CONFIG_DEBUG_SCHED_INFO=y` (set in 
`qemu-armv8a:nsh_fiq`), `qemu-armv8a:nsh` stops after "Boot to C runtime for OS 
Initialize" and never reaches the prompt; master boots.
   
   Fix: initialize `.uart.priv` and `.uart.ops` statically in the port 
definitions (or make `up_putc()` call `pl011_dev_init()` first), here and in 
the fvp and arm qemu serial files.



##########
include/nuttx/serial/uart_pl011.h:
##########
@@ -29,28 +29,48 @@
 
 #include <nuttx/config.h>
 
-#ifdef CONFIG_UART_PL011
+#include <nuttx/serial/serial.h>>

Review Comment:
   `#include <nuttx/serial/serial.h>>` has an extra `>`.
   
   Fix: `#include <nuttx/serial/serial.h>`.



##########
boards/arm64/qemu/qemu-armv8a/configs/nsh/defconfig:
##########
@@ -5,6 +5,7 @@
 # You can then do "make savedefconfig" to generate a new defconfig file that 
includes your
 # modifications.
 #
+# CONFIG_STANDARD_SERIAL is not set

Review Comment:
   `# CONFIG_STANDARD_SERIAL is not set` disables the standard serial driver 
only in `qemu-armv8a:nsh`; it is `=y` on master and in the other configs.
   
   Fix: remove the line.



##########
arch/arm64/src/qemu/Kconfig:
##########
@@ -47,4 +47,30 @@ config ARCH_CHIP_QEMU_WITH_HV
 
 endmenu # "Qemu Chip Selection"
 
+#####################################################################
+#  UART Configuration
+#####################################################################
+
+menu "QEMU ARMv8A Peripheral Selection"
+
+config QEMU_UART0
+       bool "UART0"
+       default y
+       select UART_PL011
+       select UART0_SERIALDRIVER
+       select ARCH_HAVE_SERIAL_TERMIOS

Review Comment:
   The new UART options select `ARCH_HAVE_SERIAL_TERMIOS`, but `pl011_ioctl()` 
still returns `-ENOTTY` for every command, so `CONFIG_SERIAL_TERMIOS` can be 
enabled while the driver cannot set the baud rate or the format.
   
   Fix: do not select `ARCH_HAVE_SERIAL_TERMIOS` here and in the qemu (arm) and 
fvp Kconfig files until the driver handles `TCGETS`/`TCSETS`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to