You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "linguini1 (via GitHub)" <gi...@apache.org> on 2026/05/02 19:44:11 UTC
[PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
linguini1 opened a new pull request, #18837:
URL: https://github.com/apache/nuttx/pull/18837
## Summary
This commit refactors the PL011 UART driver so that it can be re-used for any number of UART interfaces depending on the board/chip. This commit also hooks the UART interface configuration/selection for PL011 UART interfaces into the same Kconfig used for regular UART interfaces. Now UART interfaces are configured in a standard, extensible way.
## Impact
Impacts only the 4 (simulator) boards that used PL011 UART.
## Testing
For each of the QEMU boards, I tested their `gdbstub` demos since those configurations use both UART interfaces, not just a console.
QEMU ARMv8A
<details>
```console
$ qemu-system-aarch64 -cpu cortex-a53 -nographic -machine virt,virtualization=on,gic-version=3 -net none -kernel ./nuttx -serial mon:stdio -serial pty
char device redirected to /dev/pts/1 (label serial1)
- Ready to Boot Primary CPU
- Boot from EL2
- Boot from EL1
- Boot to C runtime for OS Initialize
NuttShell (NSH) NuttX-12.13.0
nsh> ps
TID PID PPID PRI POLICY TYPE NPX STATE EVENT SIGMASK STACK USED FILLED COMMAND
0 0 0 0 FIFO Kthread - Ready 0000000000000000 0008144 0001664 20.4% Idle_Task
1 0 0 192 RR Kthread - Waiting Semaphore 0000000000000000 0008080 0001152 14.2% hpwork 0x403cf6d0 0x403cf758
3 3 0 100 RR Task - Running 0000000000000000 0008112 0003840 47.3% nsh_main
nsh> ls
/:
dev/
proc/
tmp/
```
```console
$ gdb nuttx -ex "target remote /dev/pts/1"
GNU gdb (GDB) 17.1
Copyright (C) 2025 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from nuttx...
Remote debugging using /dev/pts/1
0x0000000040297528 in up_idle () at common/arm64_idle.c:65
65 }
(gdb) exit
```
</details>
QEMU ARMv7A
<details>
```console
$ qemu-system-arm -cpu cortex-a7 -nographic -machine virt,virtualization=off,gic-version=2 -net none -kernel ./nuttx -serial mon:stdio -serial pty
char device redirected to /dev/pts/2 (label serial1)
nx_start: Entry
uart_register: Registering /dev/console
uart_register: Registering /dev/ttyS0
work_start_highpri: Starting high-priority kernel worker thread(s)
nxtask_activate: hpwork pid=1,TCB=0x4020d5c0
nxtask_activate: AppBringUp pid=2,TCB=0x4020e690
nx_start_application: Starting init thread
task_spawn: name=nsh_main entry=0x60ccac file_actions=0 attr=0x4020f724 argv=0x4020f720
nxtask_activate: nsh_main pid=3,TCB=0x4020f790
nxtask_exit: AppBringUp pid=2,TCB=0x4020e690
lib_cxx_initialize: _sinit: 0x631110 _einit: 0x631110
NuttShell (NSH) NuttX-12.13.0
nsh> nx_start: CPU0: Beginning Idle Loop
nsh> ps
TID PID PPID PRI POLICY TYPE NPX STATE EVENT SIGMASK STACK USED FILLED COMMAND
0 0 0 0 FIFO Kthread - Ready 0000000000000000 0004072 0000704 17.2% Idle_Task
1 0 0 192 RR Kthread - Waiting Semaphore 0000000000000000 0004024 0000456 11.3% hpwork 0x40200150 0x40200198
3 3 0 100 RR Task - Running 0000000000000000 0004048 0001552 38.3% nsh_main
nsh>
```
```console
$ gdb nuttx -ex "target remote /dev/pts/2"
GNU gdb (GDB) 17.1
Copyright (C) 2025 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from nuttx...
Remote debugging using /dev/pts/2
0x0060c488 in up_idle () at chip/qemu_idle.c:63
63 }
(gdb) exit
```
</details>
For the FVP boards, I tested compilation only.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3191476887
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
I'm not sure there's a way around that except by maintaining two interfaces for PL011 registration and all the old Kconfig options. Plus, I will have to add some refactoring for more granular interrupt management.
Let me brainstorm and get back to you, maybe I can keep the init function and still remove the PL011-specific Kconfig functions. The main challenge is selection of the base addresses and IRQ numbers.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3567621758
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
> But defining in defconfig causes problems for some targets. The RPi4B has 5 interfaces, which is beyond what is defined in Kconfig.
we can add a new set of Kconfig.
> It also doesn't have an IRQ for each interface, it shares one single IRQ. So to reuse the PL011 driver code, it needs some access to the internals so it can register for multiple interfaces without modifying the PL011 Kconfig as a bandaid solution (my next PR will handle the multiple vs single IRQ issue).
>
For the special case, let add a new struct/function, so the chip/board could do some customization. But please keep the origin usage.
> I see it as we're choosing between this definition of PL011 interfaces in code which requires similar definitions for each interface, or continually patching Kconfig options to modify the driver options to fit with every target. I think duplication in C is more expressive and worth the minimal duplication. I understand why you don't like the duplication, but if we keep the Kconfig option there will then be two different ways of handling PL011 interfaces which I think is more confusing/unclean.
>
> Any thoughts?
let's keep the old method and your new method:
Make simple things simple, and complex things possible.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3530427601
##########
arch/arm/src/cxd32xx/cxd32_serial_pl011.c:
##########
@@ -59,7 +59,7 @@ void arm_earlyserialinit(void)
* when they are first opened.
*/
- pl011_earlyserialinit();
+ #error "Not implemented"
Review Comment:
From what I can tell, there are no boards using cxd32 in the tree. Which means I can't look at a defconfig file to see what PL011 interfaces to put in code and use as the serial console (there is no `CONSOLE_DEV` definition). I added the `#error` in this case because anyone making out of board trees will be alerted to implement the functions for the PL011 interfaces they are using.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4546266885
@lupyuen thanks for the testing help! I did not try running with the cibuild script, I only targeted the failing `qemu-armv8a:citest` build. I can try it that way instead. I've fixed the two errors you found though; I'm keeping all those changes on a branch called `pl011-uart-test` so I avoid re-triggering the CI build here every time I upstream.
I will try raiden's suggestion of building in CI with Make instead of CMake so I can download the artifact. Hopefully that reveals something!
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4969555143
> * Lets just sort out if/how to keep the Kconfig part or at least most general-purpose of it if really necessary (maybe only number of PL011 UARTS and the rest is done by the board logic as it knows what is underneath?) :-)
That is the current setup; the Kconfig chooses to enable UART1, UART2, UART3, etc. and the chip logic registers the selected ones.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3566726123
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
But this makes it more complex to maintain and keeps all the Kconfig options. I think it should remain up to the board to define which PL011 interfaces exist; now it is easier to extend. The only replicated code is the definition of the PL011 interfaces, which is what we do for other UART interfaces.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4536964804
> Did you run this on your host or from nuttx-ci docker? I also checked it on my host, and this change pass ntfc tests. So I think there might be something wrong with image produced by docker itself, or maybe qemu version that is installed on this image
This was run on my host, not from the `nuttx-ci` docker. Darn, what a pesky bug to track down!
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4544186634
> @linguini1 I ran docker CI for this PR on my host and all tests pass... I have no idea where the problem could be.
>
> Next thing I would try is to export nuttx elf which is built on github CI, I would download CI artifacts with it and see if it works locally on the host. You could change the build for failing citest config from cmake to make and see if it works. If it doesn't, at least you'll be able to easily download the nuttx elf that doesn't work and run it locally.
Maybe we could create SBOM for the CI Docker image so we know what are exact dependencies? :-)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "raiden00pl (via GitHub)" <gi...@apache.org>.
raiden00pl commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4885266553
@linguini1 before I dig into the problem myself, I run Claude and see if it finds the issue
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3533089254
##########
arch/arm64/src/fvp-v8r/fvp_serial.c:
##########
@@ -37,12 +37,187 @@
#include <nuttx/serial/uart_pl011.h>
#include "arm64_internal.h"
+/***************************************************************************
+ * 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
+#elif defined(CONFIG_UART2_SERIAL_CONSOLE)
+ #define CONSOLE_DEV g_pl011_port2
+#elif defined(CONFIG_UART3_SERIAL_CONSOLE)
+ #define CONSOLE_DEV g_pl011_port3
+#endif
+
+/* QEMU-specific configuration parameters */
+
+#define UART0_BASEADDR (0x9c090000)
+#define UART0_CLK_FREQ (24000000)
+#define UART0_IRQ (37)
+
+#define UART1_BASEADDR (0x9c0a0000)
+#define UART1_CLK_FREQ (24000000)
+#define UART1_IRQ (38)
+
+#define UART2_BASEADDR (0x9c0b0000)
+#define UART2_CLK_FREQ (24000000)
+#define UART2_IRQ (39)
+
+#define UART3_BASEADDR (0x9c0c0000)
+#define UART3_CLK_FREQ (24000000)
+#define UART3_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
+
+#ifdef CONFIG_UART2_SERIALDRIVER
+static char g_uart2_rx_buf[CONFIG_UART2_RXBUFSIZE];
+static char g_uart2_tx_buf[CONFIG_UART2_TXBUFSIZE];
+
+static struct pl011_uart_port_s g_pl011_port2 =
+{
+ .config =
+ {
+ .baseaddr = (void *)UART2_BASEADDR,
+ .baud_rate = CONFIG_UART2_BAUD,
+ .irq_num = UART2_IRQ,
+ .sbsa = false,
+ .sys_clk_freq = UART2_CLK_FREQ,
+ },
+
+ .uart =
+ {
+ .recv =
+ {
+ .buffer = g_uart2_rx_buf,
+ .size = CONFIG_UART2_RXBUFSIZE,
+ },
+ .xmit =
+ {
+ .buffer = g_uart2_tx_buf,
+ .size = CONFIG_UART2_TXBUFSIZE,
+ },
+ },
+};
+#endif
+
+#ifdef CONFIG_UART3_SERIALDRIVER
+static char g_uart3_rx_buf[CONFIG_UART3_RXBUFSIZE];
Review Comment:
but why do we dup the setting in all chip?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3533098245
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
the solution is simple:
1. add the new function for dynamically register
2. allow the preconfigured instance to zero
so, the chip which doesn't require the special handling could reuse the simple configuration.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "raiden00pl (via GitHub)" <gi...@apache.org>.
raiden00pl commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4366549094
no idea. NTFC should export nuttx.elf in CI artifacts, but for some reason these are not present for qemu-armv8a:citest and qemu-armv8a:citest_smp. What's funny - it works for all other targets citest targets.
Now I see that this target uses cmake, so that might be the reason for the missing elf in the artifacts.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3190499371
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
but the similar config duplicated in the many places now. I would prefer to keep the origin global definition, so the common usage could be simplified.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "raiden00pl (via GitHub)" <gi...@apache.org>.
raiden00pl commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4885364220
@linguini1 you can try this:
```
The fix
1. Port 85337c0efc to boards/arm64/qemu/qemu-armv8a/scripts/dramboot.ld: move .kasan.shadows to
after .rodata (fixes convergence) and give .kasan.unused/.kasan.global a concrete in-image location
before .data (stops the 0x40000000 relocation). This is really an arm64 counterpart to the armv7a
fix — arguably a standalone infra commit that unblocks the PR.
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4963101928
I am moving discussion about PL011 UART init between @linguini1 and @xiaoxiang781216 here, as in code comments will disappear as soon as code is changed. Here we will preserve discussion history :-)
This PL011 is kind of special case where more complex configuration is necessary and goes beyond current Kconfig design because it requires underlying hw knowledge that only Arch/Board has.. and as more hw shows up the more Kconfig gets complicated.. thus new approach proposed by @linguini1 as more versatile, less confusing, cleaner, better in maintenance, thus more elegant?
I understand that @xiaoxiang781216 proposes to keep the old way for compatibility.. but each hw has its own parameters that are board specific rather than system specific this why it needs a switch to a new design as proposed by @linguini1? When keeping the Kconfig it should rather go Arch/Board wide rather than System wide? Or just init the PL011 in the Arch/Board code? And it may be a good time for a switch while there are still not many boards/users impacted?
How about we just select how many PL011 uarts we want in the Kconfig (the "old" generic system wide way) and the rest would be done by the board setup code (the "new" way)? If we want no uart then Kconfig selects `0`, if we want one uart then Kconfig selects `1`, if we want five then Kconfig selects `5`. Board code would then perform build-time-check if requested uart count is valid (i.e. for rPI-4B selecting `6` would cause build error because only 5 is possible). Also the board would initialize underlying hardware in the way that is the best for that specific hardware. Would that create a compromise for board-aware-initialization as proposed by @linguini1 and Kconfig selectable PL011 UART count as proposed by @xiaoxiang781216 ? :-)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4544178350
Will take a look today, thank you @linguini1 :-)
Looking at the @xiaoxiang781216 comment, maybe it is possible to add parameter to existing functions to tell which port we want to use to be least invasive?
If current approach is blocking, on the other hand, it may need an update. Will take a look into details in few hours, sorry for the delay :-)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "lupyuen (via GitHub)" <gi...@apache.org>.
lupyuen commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4543934467
@linguini1 You might wanna check these Build Errors:
(1) https://github.com/lupyuen260526/nuttx/actions/runs/26436834321/job/77821696938#step:10:898
```
Configuration/Tool: fvp-armv8r-aarch32/nsh,CONFIG_ARM_TOOLCHAIN_GNU_EABI
[1/1] Normalize fvp-armv8r-aarch32/nsh
27,29d26
< CONFIG_FVP_ARMV8R_UART0=y
< CONFIG_FVP_ARMV8R_UART2=y
< CONFIG_FVP_ARMV8R_UART3=y
32a30,32
> CONFIG_FVP_ARMV8R_UART0=y
> CONFIG_FVP_ARMV8R_UART2=y
> CONFIG_FVP_ARMV8R_UART3=y
```
(2) https://github.com/lupyuen260526/nuttx/actions/runs/26436834321/job/77821696977#step:10:1704
```
Configuration/Tool: qemu-armv7r/pnsh,CONFIG_ARM_TOOLCHAIN_GNU_EABI
arm-none-eabi-ld: /github/workspace/sources/nuttx/staging/libdrivers.a(syslog_channel.o): in function `syslog_default_putc':
/github/workspace/sources/nuttx/drivers/syslog/syslog_channel.c:295:(.text.syslog_default_putc+0x30): undefined reference to `up_putc'
arm-none-eabi-ld: /github/workspace/sources/nuttx/drivers/syslog/syslog_channel.c:300:(.text.syslog_default_putc+0x44): undefined reference to `up_putc'
arm-none-eabi-ld: /github/workspace/sources/nuttx/staging/libkarch.a(arm_nputs.o): in function `up_nputs':
/github/workspace/sources/nuttx/arch/arm/src/common/arm_nputs.c:47:(.text.up_nputs+0x20): undefined reference to `up_putc'
[1/1] Normalize qemu-armv7r/pnsh
57,61d56
< CONFIG_UART1_BASE=0x9000000
< CONFIG_UART1_IRQ=33
< CONFIG_UART1_PL011=y
< CONFIG_UART1_SERIAL_CONSOLE=y
< CONFIG_UART_PL011=y
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4364942729
I am able to run the `qemu-armv8a:citest` binary locally with the same invocation used by NTFC:
```console
$ qemu-system-aarch64 -cpu cortex-a53 -nographic -machine virt,virtualization=on,gic-version=3 -net none -chardev stdio,id=con,mux=on -serial chardev:con -mon chardev=con,mode=readline -kernel ./nuttx
NuttShell (NSH) NuttX-12.13.0
nsh> ps
TID PID PPID PRI POLICY TYPE NPX STATE EVENT SIGMASK STACK USED FILLED COMMAND
0 0 0 0 FIFO Kthread - Ready 0000000000000000 0008144 0001616 19.8% Idle_Task
1 0 0 192 RR Kthread - Waiting Semaphore 0000000000000000 0008080 0001200 14.8% hpwork 0x404196a0 0x40419728
3 3 0 100 RR Task - Running 0000000000000000 0008112 0003728 45.9% nsh_main
nsh> ls
/:
dev/
proc/
tmp/
nsh>
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-5093398790
> Can we move forward here please @xiaoxiang781216 ? :-)
no, I don't agree the design.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4536748843
Okay so I built `qemu-armv8a:citest` locally using CMake:
```console
$ cmake -B build -DBOARD_CONFIG=qemu-armv8a:citest -GNinja
$ cmake --build build
```
Then I downloaded the `nuttx-ntfc` and `nuttx-ntfc-testing` repositories to run the CI test-suites locally:
```
ntfc test --testpath=/home/linguini/coding/nuttx-space/nuttx-ntfc/external/nuttx-testing/ --confpath=/home/linguini/coding/nuttx-space/nuttx/boards/arm64/qemu/qemu-armv8a/configs/citest_smp/config.yaml --jsonconf=/home/linguini/coding/nuttx-space/nuttx/boards/arm64/qemu/qemu-armv8a/configs/citest_smp/session.json
--------------------------------------------------------------------------------
NTFC PID: 3602936
--------------------------------------------------------------------------------
YAML config:
{'config': {'cwd': './', 'kv': [], 'loops': 1},
'product': {'cores': {'core0': {'conf_path': './build/.config',
'device': 'qemu',
'elf_path': './build/nuttx',
'exec_args': '-cpu cortex-a53 -nographic -smp '
'4 -machine '
'virt,virtualization=on,gic-version=3 '
'-net none -chardev '
'stdio,id=con,mux=on -serial '
'chardev:con -mon '
'chardev=con,mode=readline',
'exec_path': 'qemu-system-aarch64',
'name': 'main'}},
'name': 'qemu-armv8a-smp'}}
JSON config:
{'args': {'kv': []},
'module': {'exclude_module': ['Nuttx_System_Fs_Fs'],
'include_module': [],
'order': []}}
CPU information header not found in output
==================================================================== test session starts =====================================================================
platform linux -- Python 3.14.5, pytest-9.0.3, pluggy-1.6.0
rootdir: /home/linguini/coding/nuttx-space/nuttx-ntfc
configfile: tox.ini
plugins: html-4.2.0, repeat-0.9.4, ordering-0.6, dependency-0.6.1, metadata-3.1.1, timeout-2.4.0
collected 932 items
=================================================================== no tests ran in 0.39s ====================================================================
====================================================================================================
🚀 RUNNING 63 SELECTED TEST(S)
====================================================================================================
┼─────┼──────────────────────────────────┼──────────────────────────────┼
│ Idx │ Module │ Test Case │
┼─────┼──────────────────────────────────┼──────────────────────────────┼
│ 1 │ Nuttx_System_Arch_Os_Integration │ test_os[main] │
│ 2 │ Nuttx_System_Arch_Example │ test_hello_integration[main] │
│ 3 │ Nuttx_System_Arch_Example │ test_getprime │
│ 4 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat001[main] │
│ 5 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat002[main] │
│ 6 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat003[main] │
│ 7 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat004[main] │
│ 8 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat005[main] │
│ 9 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat006[main] │
│ 10 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat007[main] │
│ 11 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat008[main] │
│ 12 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat009[main] │
│ 13 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat010[main] │
│ 14 │ Nuttx_System_Arch_Nsh │ TestCd.test_cd001 │
│ 15 │ Nuttx_System_Arch_Nsh │ TestCd.test_cd002 │
│ 16 │ Nuttx_System_Arch_Nsh │ TestCd.test_cd003 │
│ 17 │ Nuttx_System_Arch_Nsh │ TestCd.test_cd004 │
│ 18 │ Nuttx_System_Arch_Nsh │ TestCd.test_cd005 │
│ 19 │ Nuttx_System_Arch_Nsh │ TestEcho.test_echo001 │
│ 20 │ Nuttx_System_Arch_Nsh │ TestEcho.test_echo002 │
│ 21 │ Nuttx_System_Arch_Nsh │ TestEcho.test_echo003 │
│ 22 │ Nuttx_System_Arch_Nsh │ TestHelp.test_help01 │
│ 23 │ Nuttx_System_Arch_Nsh │ TestHelp.test_help02 │
│ 24 │ Nuttx_System_Arch_Nsh │ TestHelp.test_help_v │
│ 25 │ Nuttx_System_Arch_Nsh │ TestHelp.test_help_cat │
│ 26 │ Nuttx_System_Arch_Nsh │ TestLs.test_ls_l │
│ 27 │ Nuttx_System_Arch_Nsh │ TestLs.test_ls_s │
│ 28 │ Nuttx_System_Arch_Nsh │ TestLs.test_ls_R │
│ 29 │ Nuttx_System_Arch_Nsh │ TestRm.test_rm001 │
│ 30 │ Nuttx_System_Arch_Nsh │ TestRm.test_rm002 │
│ 31 │ Nuttx_System_Arch_Nsh │ TestMkdir.test_mkdir001 │
│ 32 │ Nuttx_System_Arch_Nsh │ TestRmdir.test_rmdir001 │
│ 33 │ Nuttx_System_Arch_Nsh │ TestRmdir.test_rmdir002 │
│ 34 │ Nuttx_System_Arch_Nsh │ TestCp.test_cp001 │
│ 35 │ Nuttx_System_Arch_Nsh │ TestCp.test_cp002 │
│ 36 │ Nuttx_System_Arch_Nsh │ TestCp.test_cp003 │
│ 37 │ Nuttx_System_Arch_Nsh │ TestPs.test_ps │
│ 38 │ Nuttx_System_Arch_Nsh │ TestTime.test_sleep │
│ 39 │ Nuttx_System_Arch_Nsh │ TestTime.test_usleep │
│ 40 │ Nuttx_System_Arch_Nsh │ TestSet.test_set │
│ 41 │ Nuttx_System_Arch_Nsh │ TestSet.test_unset │
│ 42 │ Nuttx_System_Arch_Nsh │ TestTest.test_test │
│ 43 │ Nuttx_System_Arch_Nsh │ TestDirname.test_dirname │
│ 44 │ Nuttx_System_Arch_Nsh │ TestBasename.test_basename │
│ 45 │ Nuttx_System_Arch_Nsh │ TestDf.test_df │
│ 46 │ Nuttx_System_Arch_Nsh │ TestDf.test_df_h │
│ 47 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname │
│ 48 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_a │
│ 49 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_s │
│ 50 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_r │
│ 51 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_v │
│ 52 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_m │
│ 53 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_i │
│ 54 │ Nuttx_System_Arch_Nsh │ TestEnv.test_env │
│ 55 │ Nuttx_System_Arch_Nsh │ TestMv.test_mv │
│ 56 │ Nuttx_System_Arch_Nsh │ TestPwd.test_pwd │
│ 57 │ Nuttx_System_Arch_Nsh │ TestHexdump.test_hexdump │
│ 58 │ Nuttx_System_Arch_Nsh │ TestMkrd.test_mkrd │
│ 59 │ Nuttx_System_Arch_Nsh │ TestMw.test_mw │
│ 60 │ Nuttx_System_Arch_Nsh │ TestExec.test_exec │
│ 61 │ Nuttx_System_Arch_Space │ test_free[main] │
│ 62 │ Nuttx_System_Arch_Space │ test_df[main] │
│ 63 │ Nuttx_System_Tools │ test_system_mmleak │
┼─────┼──────────────────────────────────┼──────────────────────────────┼
====================================================================================================
CPU information header not found in output
==================================================================== test session starts =====================================================================
platform linux -- Python 3.14.5, pytest-9.0.3, pluggy-1.6.0
rootdir: /home/linguini/coding/nuttx-space/nuttx-ntfc
configfile: tox.ini
plugins: html-4.2.0, repeat-0.9.4, ordering-0.6, dependency-0.6.1, metadata-3.1.1, timeout-2.4.0
timeout: 800.0s
timeout method: signal
timeout func_only: False
session timeout: 3600.0s
collected 64 items
../nuttx-ntfc/external/nuttx-testing/arch/os/integration/test_arch_os_integration.py::test_os[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/example/test_arch_example_integration.py::test_hello_integration[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/example/test_arch_example_integration.py::test_getprime PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat001[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat002[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat003[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat004[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat005[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat006[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat007[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat008[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat009[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat010[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCd::test_cd001 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCd::test_cd002 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCd::test_cd003 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCd::test_cd004 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCd::test_cd005 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestEcho::test_echo001 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestEcho::test_echo002 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestEcho::test_echo003 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestHelp::test_help01 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestHelp::test_help02 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestHelp::test_help_v PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestHelp::test_help_cat PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestLs::test_ls_l PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestLs::test_ls_s SKIPPED (No Ready)
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestLs::test_ls_R PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestRm::test_rm001
```
It works fine. However, running the CI through the manual CI testing repository, I can see it fail again for this specific configuration: https://github.com/linguini1/manual-nuttx-ci/actions/runs/26415001939/job/77757543595
I'm genuinely at a loss for what's going on. Can you spot anything @raiden00pl?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4895115467
Just added @raiden00pl's fix, let's see how it goes!
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4547899567
Okay so I downloaded the build artifact from CI here:
https://github.com/linguini1/manual-nuttx-ci/actions/runs/26415001939
And I both launched the image with `qemu-system-aarch64` and ran `ntfc` on it, and both tests succeeded. So either:
- Something is wrong with the CMake build of this (since the archive was generated from Make) in the CI only
- There is something else specifically about NTFC running in CI that is causing this build to fail, as the image itself is fine
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4537213861
Even more hair pulling. I downloaded the latest CI docker image and ran it
```console
$ docker pull ghcr.io/apache/nuttx/apache-nuttx-ci-linux:latest
```
Then I cloned my branch with these changes, nuttx-apps, nuttx-ntfc, nuttx-ntfc-testing, installed NTFC, built `qemu-armv8a:citest` with CMake, etc. And...... it works fine:
```console
$ ntfc test --testpath=../nuttx-ntfc/external/nuttx-testing/ --confpath=boards/arm64/qemu/qemu-armv8a/configs/citest/config.yaml --jsonconf=boards/arm64/qemu/qemu-armv8a/configs/citest/session.json
--------------------------------------------------------------------------------
NTFC PID: 11313
--------------------------------------------------------------------------------
YAML config:
{'config': {'cwd': './', 'kv': [], 'loops': 1},
'product': {'cores': {'core0': {'conf_path': './build/.config',
'device': 'qemu',
'elf_path': './build/nuttx',
'exec_args': '-cpu cortex-a53 -nographic '
'-machine '
'virt,virtualization=on,gic-version=3 '
'-net none -chardev '
'stdio,id=con,mux=on -serial '
'chardev:con -mon '
'chardev=con,mode=readline',
'exec_path': 'qemu-system-aarch64',
'name': 'main'}},
'name': 'qemu-armv8a'}}
JSON config:
{'args': {'kv': []},
'module': {'exclude_module': ['Nuttx_System_Fs_Fs'],
'include_module': [],
'order': []}}
CPU information header not found in output
================================================================== test session starts ===================================================================
platform linux -- Python 3.10.12, pytest-9.0.3, pluggy-1.6.0
rootdir: /tools/nuttx-space/nuttx-ntfc
configfile: tox.ini
plugins: html-4.2.0, dependency-0.6.1, metadata-3.1.1, timeout-2.4.0, repeat-0.9.1, ordering-0.6, json-0.4.0
collected 932 items
================================================================= no tests ran in 0.41s ==================================================================
====================================================================================================
🚀 RUNNING 63 SELECTED TEST(S)
====================================================================================================
┼─────┼──────────────────────────────────┼──────────────────────────────┼
│ Idx │ Module │ Test Case │
┼─────┼──────────────────────────────────┼──────────────────────────────┼
│ 1 │ Nuttx_System_Arch_Os_Integration │ test_os[main] │
│ 2 │ Nuttx_System_Arch_Example │ test_hello_integration[main] │
│ 3 │ Nuttx_System_Arch_Example │ test_getprime │
│ 4 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat001[main] │
│ 5 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat002[main] │
│ 6 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat003[main] │
│ 7 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat004[main] │
│ 8 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat005[main] │
│ 9 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat006[main] │
│ 10 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat007[main] │
│ 11 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat008[main] │
│ 12 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat009[main] │
│ 13 │ Nuttx_System_Arch_Nsh │ TestCat.test_cat010[main] │
│ 14 │ Nuttx_System_Arch_Nsh │ TestCd.test_cd001 │
│ 15 │ Nuttx_System_Arch_Nsh │ TestCd.test_cd002 │
│ 16 │ Nuttx_System_Arch_Nsh │ TestCd.test_cd003 │
│ 17 │ Nuttx_System_Arch_Nsh │ TestCd.test_cd004 │
│ 18 │ Nuttx_System_Arch_Nsh │ TestCd.test_cd005 │
│ 19 │ Nuttx_System_Arch_Nsh │ TestEcho.test_echo001 │
│ 20 │ Nuttx_System_Arch_Nsh │ TestEcho.test_echo002 │
│ 21 │ Nuttx_System_Arch_Nsh │ TestEcho.test_echo003 │
│ 22 │ Nuttx_System_Arch_Nsh │ TestHelp.test_help01 │
│ 23 │ Nuttx_System_Arch_Nsh │ TestHelp.test_help02 │
│ 24 │ Nuttx_System_Arch_Nsh │ TestHelp.test_help_v │
│ 25 │ Nuttx_System_Arch_Nsh │ TestHelp.test_help_cat │
│ 26 │ Nuttx_System_Arch_Nsh │ TestLs.test_ls_l │
│ 27 │ Nuttx_System_Arch_Nsh │ TestLs.test_ls_s │
│ 28 │ Nuttx_System_Arch_Nsh │ TestLs.test_ls_R │
│ 29 │ Nuttx_System_Arch_Nsh │ TestRm.test_rm001 │
│ 30 │ Nuttx_System_Arch_Nsh │ TestRm.test_rm002 │
│ 31 │ Nuttx_System_Arch_Nsh │ TestMkdir.test_mkdir001 │
│ 32 │ Nuttx_System_Arch_Nsh │ TestRmdir.test_rmdir001 │
│ 33 │ Nuttx_System_Arch_Nsh │ TestRmdir.test_rmdir002 │
│ 34 │ Nuttx_System_Arch_Nsh │ TestCp.test_cp001 │
│ 35 │ Nuttx_System_Arch_Nsh │ TestCp.test_cp002 │
│ 36 │ Nuttx_System_Arch_Nsh │ TestCp.test_cp003 │
│ 37 │ Nuttx_System_Arch_Nsh │ TestPs.test_ps │
│ 38 │ Nuttx_System_Arch_Nsh │ TestTime.test_sleep │
│ 39 │ Nuttx_System_Arch_Nsh │ TestTime.test_usleep │
│ 40 │ Nuttx_System_Arch_Nsh │ TestSet.test_set │
│ 41 │ Nuttx_System_Arch_Nsh │ TestSet.test_unset │
│ 42 │ Nuttx_System_Arch_Nsh │ TestTest.test_test │
│ 43 │ Nuttx_System_Arch_Nsh │ TestDirname.test_dirname │
│ 44 │ Nuttx_System_Arch_Nsh │ TestBasename.test_basename │
│ 45 │ Nuttx_System_Arch_Nsh │ TestDf.test_df │
│ 46 │ Nuttx_System_Arch_Nsh │ TestDf.test_df_h │
│ 47 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname │
│ 48 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_a │
│ 49 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_s │
│ 50 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_r │
│ 51 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_v │
│ 52 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_m │
│ 53 │ Nuttx_System_Arch_Nsh │ TestUname.test_uname_i │
│ 54 │ Nuttx_System_Arch_Nsh │ TestEnv.test_env │
│ 55 │ Nuttx_System_Arch_Nsh │ TestMv.test_mv │
│ 56 │ Nuttx_System_Arch_Nsh │ TestPwd.test_pwd │
│ 57 │ Nuttx_System_Arch_Nsh │ TestHexdump.test_hexdump │
│ 58 │ Nuttx_System_Arch_Nsh │ TestMkrd.test_mkrd │
│ 59 │ Nuttx_System_Arch_Nsh │ TestMw.test_mw │
│ 60 │ Nuttx_System_Arch_Nsh │ TestExec.test_exec │
│ 61 │ Nuttx_System_Arch_Space │ test_free[main] │
│ 62 │ Nuttx_System_Arch_Space │ test_df[main] │
│ 63 │ Nuttx_System_Tools │ test_system_mmleak │
┼─────┼──────────────────────────────────┼──────────────────────────────┼
====================================================================================================
CPU information header not found in output
================================================================== test session starts ===================================================================
platform linux -- Python 3.10.12, pytest-9.0.3, pluggy-1.6.0
rootdir: /tools/nuttx-space/nuttx-ntfc
configfile: tox.ini
plugins: html-4.2.0, dependency-0.6.1, metadata-3.1.1, timeout-2.4.0, repeat-0.9.1, ordering-0.6, json-0.4.0
timeout: 800.0s
timeout method: signal
timeout func_only: False
session timeout: 3600.0s
collected 64 items
../nuttx-ntfc/external/nuttx-testing/arch/os/integration/test_arch_os_integration.py::test_os[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/example/test_arch_example_integration.py::test_hello_integration[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/example/test_arch_example_integration.py::test_getprime PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat001[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat002[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat003[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat004[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat005[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat006[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat007[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat008[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat009[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat010[main] PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCd::test_cd001 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCd::test_cd002 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCd::test_cd003 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCd::test_cd004 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestCd::test_cd005 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestEcho::test_echo001 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestEcho::test_echo002 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestEcho::test_echo003 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestHelp::test_help01 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestHelp::test_help02 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestHelp::test_help_v PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestHelp::test_help_cat PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestLs::test_ls_l PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestLs::test_ls_s SKIPPED (No Ready)
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestLs::test_ls_R PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestRm::test_rm001 PASSED
../nuttx-ntfc/external/nuttx-testing/arch/nsh/test_arch_nsh_integration.py::TestRm::test_rm002 PASSED
```
@simbit18 or @lupyuen , do you have any idea what might be going on here? Any advice for getting closer to reproducing the CI conditions locally?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4366444512
@raiden00pl Do you have any idea why this change might cause NTFC to fail with a boot timeout on `qemu-armv8a:citest` but not `qemu-armv8a:citest_smp`?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "raiden00pl (via GitHub)" <gi...@apache.org>.
raiden00pl commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4885367716
here full report:
> Your PL011 refactor is fine. The failure is a latent KASAN linker-script bug in the arm64 board that your refactor happens to trigger.
>
> The chain of reasoning
>
> 1. Only one config fails: qemu-armv8a:citest — and it's the only arm64 config with CONFIG_MM_KASAN_GLOBAL=y. citest_smp and every nsh* config (no KASAN) pass. That's the whole tell.
> 2. The failure is NTFC device boot timeout (~9 s window) — the guest never emits nsh>.
> 3. I reproduced the exact CI build+run locally (CMake + your CI QEMU command line, aarch64-gcc 14.2): it boots to nsh> in 0.09 s, 8/8, KASAN table converges, sections sane. So — like you and the maintainers — I can't make it fail. That proves it's
> layout/toolchain-sensitive, not a code bug.
> 4. boards/arm64/qemu/qemu-armv8a/scripts/dramboot.ld places the writable .kasan.unused/.kasan.global and .kasan.shadows sections before .text with no memory region.
> 5. I found upstream commit 85337c0efc (merged 2026-07-03) that fixes exactly this — but only for armv7a. Its message spells out the mechanism: writable KASAN sections before .text get silently relocated by the linker to 0x40000000, making an extra LOAD segment that
> collides with QEMU virt's RAM/DTB → boot failure; and .kasan.shadows before .text breaks multi-pass link convergence.
> 6. The arm64 script was never fixed, and your PR (dated 2026-05-24) predates the commit, so it also lacks the matching CMake strip step.
>
> Why it's intermittent / CI-only / Make-works
>
> The linker's silent relocation to 0x40000000 depends on toolchain and on the size/content of the writable KASAN sections. Your refactor moved the PL011 global buffers/structs, changing .kasan.global enough to flip GitHub's toolchain into the "bad" placement. Modern local
> toolchains keep the sections contiguous (my build), and the Makefile path already strips them — so nothing reproduces locally.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3528180899
##########
arch/arm/src/cxd32xx/cxd32_serial_pl011.c:
##########
@@ -59,7 +59,7 @@ void arm_earlyserialinit(void)
* when they are first opened.
*/
- pl011_earlyserialinit();
+ #error "Not implemented"
Review Comment:
Do we want here something like:
```
#ifdef CONSOLE_DEV
pl011_dev_init(&CONSOLE_DEV);
CONSOLE_DEV.uart.isconsole = true;
CONSOLE_DEV.uart.ops->setup(&CONSOLE_DEV.uart); /* Early set up */
#endif
```
and in other similar places where `#error "Not implemented"` shows up?
Or this setup is provided by the arch below and not necessary here anymore? If so do we want error here?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3533089254
##########
arch/arm64/src/fvp-v8r/fvp_serial.c:
##########
@@ -37,12 +37,187 @@
#include <nuttx/serial/uart_pl011.h>
#include "arm64_internal.h"
+/***************************************************************************
+ * 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
+#elif defined(CONFIG_UART2_SERIAL_CONSOLE)
+ #define CONSOLE_DEV g_pl011_port2
+#elif defined(CONFIG_UART3_SERIAL_CONSOLE)
+ #define CONSOLE_DEV g_pl011_port3
+#endif
+
+/* QEMU-specific configuration parameters */
+
+#define UART0_BASEADDR (0x9c090000)
+#define UART0_CLK_FREQ (24000000)
+#define UART0_IRQ (37)
+
+#define UART1_BASEADDR (0x9c0a0000)
+#define UART1_CLK_FREQ (24000000)
+#define UART1_IRQ (38)
+
+#define UART2_BASEADDR (0x9c0b0000)
+#define UART2_CLK_FREQ (24000000)
+#define UART2_IRQ (39)
+
+#define UART3_BASEADDR (0x9c0c0000)
+#define UART3_CLK_FREQ (24000000)
+#define UART3_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
+
+#ifdef CONFIG_UART2_SERIALDRIVER
+static char g_uart2_rx_buf[CONFIG_UART2_RXBUFSIZE];
+static char g_uart2_tx_buf[CONFIG_UART2_TXBUFSIZE];
+
+static struct pl011_uart_port_s g_pl011_port2 =
+{
+ .config =
+ {
+ .baseaddr = (void *)UART2_BASEADDR,
+ .baud_rate = CONFIG_UART2_BAUD,
+ .irq_num = UART2_IRQ,
+ .sbsa = false,
+ .sys_clk_freq = UART2_CLK_FREQ,
+ },
+
+ .uart =
+ {
+ .recv =
+ {
+ .buffer = g_uart2_rx_buf,
+ .size = CONFIG_UART2_RXBUFSIZE,
+ },
+ .xmit =
+ {
+ .buffer = g_uart2_tx_buf,
+ .size = CONFIG_UART2_TXBUFSIZE,
+ },
+ },
+};
+#endif
+
+#ifdef CONFIG_UART3_SERIALDRIVER
+static char g_uart3_rx_buf[CONFIG_UART3_RXBUFSIZE];
Review Comment:
but why do we dup the setting in all chip?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-5093602715
I think PL011 IP itself is really crap! It is not possible to use it with NuttX modbus without a nasty hack. Shame on ARM for doing it instead of creating a good standard UART
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4537125599
Okay it looks the CI uses QEMU version 6.2.0, while I have 11.0.0 (for `aarch64`). Not sure if this is necessarily the culprit but I'll investigate.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "raiden00pl (via GitHub)" <gi...@apache.org>.
raiden00pl commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4536933015
Did you run this on your host or from nuttx-ci docker? I also checked it on my host, and this change pass ntfc tests. So I think there might be something wrong with image produced by docker itself, or maybe qemu version that is installed on this image
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-5092191300
Can we move forward here please @xiaoxiang781216 ? :-)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3566781586
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
I don't think so. it's more simple to define the option in defconfig, and it duplicates the definition in each board/chip file.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3539740949
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
It seems like the boards using the common definition before needed to change things like the base address and IRQ number. So, if I do it that way, I will need to keep the PL011 Kconfig which allows menu configuration of IRQ numbers and base addresses, which means it keeps the PL011 duplication of common UART features. I think that makes it more confusing to use PL011 on new boards. Any suggestions about that?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3548193723
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
we can hide the option if user set PL011 instance to zero.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4781645098
I'm completely at a loss here with my testing as to what the issue is.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4364938564
Not entirely sure why NTFC is failing. I used the same command it invokes and was able to start the `qemu-armv8a:citest` configuration just fine:
```console
$ qemu-system-aarch64 -cpu cortex-a53 -nographic -machine virt,virtualization=on,gic-version=3 -net none -chardev stdio,id=con,mux=on -serial chardev:con -mon chardev=con,mode=readline -kernel ./nuttx
NuttShell (NSH) NuttX-12.13.0
nsh> ls
/:
dev/
proc/
tmp/
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3178407654
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
why drop the default implementation which could avoid the code duplication in the common case. it's better that:
1. keep pl011_serialinit/pl011_earlyserialinit if one of CONFIG_UARTx_PL011 is enabled
2. add pl011_dev_init, so the special case could call the new function directly.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4895110517
<!-- membrowse-pr-comment -->
## <img src="https://membrowse.com/membrowse-logo.svg" height="24" align="top"> MemBrowse Memory Report
### [qemu-armv8a](https://membrowse.com/public/apache/nuttx/target/qemu-armv8a/compare?head=c08a71c9d4083b551507e5c2c6ea9c1fc8b8aa37&base=08414762396f82e4c4d3c18d08b67235cb8ead2f)
- **Code**: .rodata +8 B, .text.arm64_earlyserialinit +44 B, .text.arm64_serialinit +68 B, .text.pl011_irq_handler +12 B, .text.pl011_setup +4 B, .text.up_putc -112 B (+0.1%, 315,510 B)
*No memory changes detected for:*
- [arduino-mega2560](https://membrowse.com/public/apache/nuttx/target/arduino-mega2560/compare?head=c08a71c9d4083b551507e5c2c6ea9c1fc8b8aa37&base=08414762396f82e4c4d3c18d08b67235cb8ead2f)
- [esp32-devkitc](https://membrowse.com/public/apache/nuttx/target/esp32-devkitc/compare?head=c08a71c9d4083b551507e5c2c6ea9c1fc8b8aa37&base=08414762396f82e4c4d3c18d08b67235cb8ead2f)
- [hifive1-revb](https://membrowse.com/public/apache/nuttx/target/hifive1-revb/compare?head=c08a71c9d4083b551507e5c2c6ea9c1fc8b8aa37&base=08414762396f82e4c4d3c18d08b67235cb8ead2f)
- [mirtoo](https://membrowse.com/public/apache/nuttx/target/mirtoo/compare?head=c08a71c9d4083b551507e5c2c6ea9c1fc8b8aa37&base=08414762396f82e4c4d3c18d08b67235cb8ead2f)
- [qemu-intel64](https://membrowse.com/public/apache/nuttx/target/qemu-intel64/compare?head=c08a71c9d4083b551507e5c2c6ea9c1fc8b8aa37&base=08414762396f82e4c4d3c18d08b67235cb8ead2f)
- [rx65n-rsk2mb](https://membrowse.com/public/apache/nuttx/target/rx65n-rsk2mb/compare?head=c08a71c9d4083b551507e5c2c6ea9c1fc8b8aa37&base=08414762396f82e4c4d3c18d08b67235cb8ead2f)
- [s698pm-dkit](https://membrowse.com/public/apache/nuttx/target/s698pm-dkit/compare?head=c08a71c9d4083b551507e5c2c6ea9c1fc8b8aa37&base=08414762396f82e4c4d3c18d08b67235cb8ead2f)
- [stm32-nucleo-f103rb](https://membrowse.com/public/apache/nuttx/target/stm32-nucleo-f103rb/compare?head=c08a71c9d4083b551507e5c2c6ea9c1fc8b8aa37&base=08414762396f82e4c4d3c18d08b67235cb8ead2f)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3533098245
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
the solution is simple:
1. add the new function for dynamically register
2. allow the preconfigured instance to zero
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3566842479
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
But defining in defconfig causes problems for some targets. The RPi4B has 5 interfaces, which is beyond what is defined in Kconfig. It also doesn't have an IRQ for each interface, it shares one single IRQ. So to reuse the PL011 driver code, it needs some access to the internals so it can register for multiple interfaces without modifying the PL011 Kconfig as a bandaid solution (my next PR will handle the multiple vs single IRQ issue).
I see it as we're choosing between this definition of PL011 interfaces in code which requires similar definitions for each interface, or continually patching Kconfig options to modify the driver options to fit with every target. I think duplication in C is more expressive and worth the minimal duplication. I understand why you don't like the duplication, but if we keep the Kconfig option there will then be two different ways of handling PL011 interfaces which I think is more confusing/unclean.
Any thoughts?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "raiden00pl (via GitHub)" <gi...@apache.org>.
raiden00pl commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4885276741
no problem, good decision :)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "raiden00pl (via GitHub)" <gi...@apache.org>.
raiden00pl commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4885700132
here is fix: https://github.com/raiden00pl/nuttx/commit/6994677e060fc8c57d28ba7a357667509266850b
here is CI run with fix: https://github.com/raiden00pl/manual-nuttx-ci/actions/runs/28737600179/job/85214536574#step:9:233
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4886718927
Nice work @raiden00pl ! What a strange fix, and even stranger that it only seemed to have an effect in CI.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "lupyuen (via GitHub)" <gi...@apache.org>.
lupyuen commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4540696731
@linguini1 [Following these Docker Steps](https://lupyuen.org/articles/ci2#build-nuttx-for-one-target-group): I'm hitting some errors with cibuild.sh at `qemu-armv8a:citest_smp`. Are you able to reproduce this? https://gist.github.com/lupyuen/42df35f7c96ef79f3402353c0065af5c
```bash
## Based on https://lupyuen.org/articles/ci2#build-nuttx-for-one-target-group
## Tested on Ubuntu 24.04.2 LTS x86_64
$ sudo docker run \
-it \
ghcr.io/apache/nuttx/apache-nuttx-ci-linux:latest \
/bin/bash
$ pip install ntfc
$ mkdir $HOME/nuttx-ntfc
$ mkdir $HOME/nuttx-ntfc/external
$ cd $HOME/nuttx-ntfc/external
$ git clone -b release-0.0.1 https://github.com/apache/nuttx-ntfc-testing
$ mv nuttx-ntfc-testing nuttx-testing
$ export NTFCDIR=$HOME/nuttx-ntfc
$ cd
$ git clone https://github.com/linguini1/nuttx --branch pl011-uart
HEAD is now at 0fd76d2a1c drivers/serial: Refactor PL011 to be general-purpose
NuttX Source: https://github.com/apache/nuttx/tree/0fd76d2a1c9a6f281138b4df2ba8dda736e72575
$ git clone https://github.com/apache/nuttx-apps apps
HEAD is now at 939ed0ca2 apps/system/microros: add rcutils strcasecmp strings.h include patch.
NuttX Apps: https://github.com/apache/nuttx-apps/tree/939ed0ca2a7b593949973be5d09aefea1b659418
$ cd ~/nuttx/tools/ci
$ ./cibuild.sh -c -A -N -R testlist/arm64-01.dat
.../arch/os/integration/test_arch_os_integration.py::test_os[main] PASSED
.../arch/example/test_arch_example_integration.py::test_hello_integration[main] PASSED
.../arch/example/test_arch_example_integration.py::test_getprime PASSED
.../arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat001[main] FAILED
.../arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat002[main] FAILED
.../arch/nsh/test_arch_nsh_integration.py::TestCat::test_cat003[main] FAILED
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "raiden00pl (via GitHub)" <gi...@apache.org>.
raiden00pl commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4543963756
@linguini1 I ran docker CI for this PR on my host and all tests pass... I have no idea where the problem could be.
Next thing I would try is to export nuttx elf which is built on github CI, I would download CI artifacts with it and see if it works locally on the host.
You could change the build for failing citest config from cmake to make and see if it works. If it doesn't, at least you'll be able to easily download the nuttx elf that doesn't work and run it locally.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4885271464
Sorry I haven't had more time @raiden00pl, I decided to pivot to not delay my GSoC milestones more. Let me know if I can help in any way if you find something!
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3530427601
##########
arch/arm/src/cxd32xx/cxd32_serial_pl011.c:
##########
@@ -59,7 +59,7 @@ void arm_earlyserialinit(void)
* when they are first opened.
*/
- pl011_earlyserialinit();
+ #error "Not implemented"
Review Comment:
From what I can tell, there are no boards using cxd32 in the tree. Which means I can't look at a defconfig file to see what PL011 interfaces to put in code and use as the serial console (there is no `CONSOLE_DEV` definition). I added the `#error` in this case because anyone making out of tree boards will be alerted to implement the functions for the PL011 interfaces they are using.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3530961809
##########
arch/arm/src/cxd32xx/cxd32_serial_pl011.c:
##########
@@ -59,7 +59,7 @@ void arm_earlyserialinit(void)
* when they are first opened.
*/
- pl011_earlyserialinit();
+ #error "Not implemented"
Review Comment:
Roger! Thank you @linguini1 :-)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3528180899
##########
arch/arm/src/cxd32xx/cxd32_serial_pl011.c:
##########
@@ -59,7 +59,7 @@ void arm_earlyserialinit(void)
* when they are first opened.
*/
- pl011_earlyserialinit();
+ #error "Not implemented"
Review Comment:
Do we want here something like:
```
#ifdef CONSOLE_DEV
pl011_dev_init(&CONSOLE_DEV);
CONSOLE_DEV.uart.isconsole = true;
CONSOLE_DEV.uart.ops->setup(&CONSOLE_DEV.uart); /* Early set up */
#endif
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3178426587
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
Because this function is no longer useful. It needs access to the device structs for the PL011 interfaces in order to register them, but these are defined outside this module now.
If we keep this function, then we can no longer:
- Define an arbitrary number of interfaces
- Use the standard UART config options for configuring PL011 UART
It should now be up to the chip logic to register the UART interfaces instead, as with all other UART drivers. Otherwise we don't have the benefits and we must also maintain two separate ways of doing things. There are only 3 boards that use PL011 right now, so it's not a super common case.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4366646287
> Now I see that this target uses cmake, so that might be the reason for the missing elf in the artifacts.
I'll try building with CMake in case that reveals what the issue might be.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] drivers/serial: Refactor PL011 to be general-purpose [nuttx]
Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#issuecomment-4972192834
perfect! lets go :-)
--
CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
> Message ID: ***@***.***>
>
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org