On-demand start of uart shell and dynamic pin control.

[kat829]

Hi!

We have three different uart applications (two for production use, and the uart shell backend) and three different hardware output paths, but nRF52840 features just two uart controllers. So we would like to be able to pick any two out of the three applications, and run them on the two controllers using dynamic pin control. We would like to be able to do so dynamically during boot.

So far I can re-route our uarts using the pinctrl_update_states and pinctrl_apply_states.

A problem comes with the uart shell, as we need to dynamically either start it or not, where by not starting I mean to not make it even claim the uart controller.

The minimum config that allows me to compile seems to be:
```
CONFIG_SHELL=y
CONFIG_SHELL_AUTOSTART=n
CONFIG_SHELL_BACKEND_SERIAL=y
CONFIG_UART_CONSOLE=n
```

Now when I set `CONFIG_SHELL_BACKEND_SERIAL=y` and reassign the pins using the pinctrl_* functions then: 

-  I can start the shell on demand with `shell_start(shell_backend_uart_get_ptr());`
-  but when I hook a logic analyzer, I don't see any traffic on the line where I had redirected the production app traffic to. This line and controller is also specified as the dts "chosen" zephyr,shell-uart. 

When I set `CONFIG_SHELL_BACKEND_SERIAL=n` and remove all code that relies on it:

-  then redirecting the traffic to the other set of lines works fine - I see there the traffic using a logic analyzer
-  but I loose the ability to start the shell on uart, since `shell_backend_uart_get_ptr()` is not compiled in without CONFIG_SHELL_BACKEND_SERIAL

I assume that it is because while the shell didn't autostart, the shell backend was automatically initialized at startup and claimed the pins.

How do we compile zephyr in such way that uart shell support is compiled in, but doesn't claim any controller unless told to?

Thanks!
Karel

[kat829]

Well, it seems that the trouble is incompatibility of the interrupt uart driver (that is default for uart shell) with the async uart driver (that is used by our app). 
When I enable: 
```
CONFIG_UART_0_ASYNC=y
CONFIG_UART_0_INTERRUPT_DRIVEN=n
```
the uart application starts magically working even despite CONFIG_SHELL_BACKEND_SERIAL=y. Of course at this point shell doesn't work when enabled instead of our app.

It looks like a solution might be adding:
```
CONFIG_SHELL_BACKEND_SERIAL_API_ASYNC=y
```
Current state of affairs: an attempt to call to pm_device_action_run(&uart0, PM_DEVICE_ACTION_RESUME) results in a crash.

I will continue digging.

[Vidar Berg]

It sounds like you have made good progress. Regarding the crash, do you have logging over UART (another instance) or RTT so you can get a crashlog?

[kat829]

Not at the moment. The changes have broken both our logging backends. Will dig later. Just wanted to let you know about the progress so that you don't burn time on it ;-). 

[kat829]

Crashlog is here

```
ASSERTION FAIL @ WEST_TOPDIR/zephyr/drivers/serial/uart_nrfx_uarte.c:2124
[00:00:07.173,767] <err> os: r0/a1:  0x00000004  r1/a2:  0x0000084c  r2/a3:  0x20012030
[00:00:07.173,797] <err> os: r3/a4:  0x00000004 r12/ip:  0x2000f568 r14/lr:  0x00066dcf
[00:00:07.173,797] <err> os:  xpsr:  0x01000000
[00:00:07.173,828] <err> os: r4/v1:  0x40002000  r5/v2:  0x000823fc  r6/v3:  0x00088308
[00:00:07.173,858] <err> os: r7/v4:  0x20002f70  r8/v5:  0x00000000  r9/v6:  0x00000000
[00:00:07.173,858] <err> os: r10/v7: 0x00000000  r11/v8: 0x00000000    psp:  0x200376b8
[00:00:07.173,889] <err> os: EXC_RETURN: 0x0
[00:00:07.173,889] <err> os: Faulting instruction address (r15/pc): 0x000781e0
[00:00:07.173,919] <err> os: >>> ZEPHYR FATAL ERROR 4: Kernel panic on CPU 0
[00:00:07.173,980] <err> os: Current thread: 0x20012030 (main)
```

Or: 

```
static void uarte_pm_suspend(const struct device *dev)
{
    NRF_UARTE_Type *uarte = get_uarte_instance(dev);
    const struct uarte_nrfx_config *cfg = dev->config;
    struct uarte_nrfx_data *data = dev->data;

    (void)data;
#ifdef UARTE_ANY_ASYNC
    if (data->async) {
        /* Entering inactive state requires device to be no
         * active asynchronous calls.
         */
        __ASSERT_NO_MSG(!data->async->rx.enabled); // <- here
```

I am not explicitly enabling uart anywhere before this call site (not that I know of anyways).

The related configs that come to mind are:

```
CONFIG_CONSOLE=y
CONFIG_UART_CONSOLE=y

CONFIG_SERIAL=y
CONFIG_SHELL=y
CONFIG_SHELL_BACKEND_SERIAL=y
CONFIG_SHELL_AUTOSTART=n

# Also tried without success, while also commenting out the zephyr,console in the dts chosen section:
#CONFIG_UART_CONSOLE=n 
#CONFIG_CONSOLE=n
```

Any ideas?

[Vidar Berg]

So as you pointed out, the assert is raised because UART reception is enabled while you try to suspend the driver. 

kat829 said:

I am not explicitly enabling uart anywhere before this call site (not that I know of anyways).

It is enabled by the shell. Is the shell initialized at this point and have you tried calling uart_rx_disable() first?

[kat829]

Ah, right. I was calling uart_irq_rx_disable, but not uart_rx_disable .

Well. When I call the uart_rx_disable, it returns -14, but indeed, the suspend doesn't fail anymore. 

But:
- If I suspend, call pinctrls, resume; then the resumes crash with various errors, all related to the drivers not being initialized.
- If I suspend, call pinctrls, initialize the drivers, then call resume; then the devices remain dead.
- If I suspend, call pinctrls, initialize the drivers; then the devices remain dead.


If I don't suspend or resume the devices at all then everything seems to work fine.

[Vidar Berg]

I think I would need to reproduce this on my end to understand why it's failing. But do you need the ability to change the pinout more than once after reset? If not, the approach used in the dynamic pin control sample (https://docs.nordicsemi.com/bundle/ncs-3.1.1/page/zephyr/samples/boards/nordic/dynamic_pinctrl/README.html) will likely be better suited for you app. 

[kat829]

> I think I would need to reproduce this on my end to understand why it's failing. 

Well, the code is public. You can see it at https://bit.ly/4n33Vd6 . (The indirection is for privacy reasons, since we usually discuss future products on these forums.)

The relevant files are device/src/{main.c,pin_wiring.c}. 

> But do you need the ability to change the pinout more than once after reset?

No. 

But I think we would like to do so a bit later during boot, somewhere in the main function. 

Maybe the question is just whether changing the pinctrl without suspending the devices is actually dangerous in our situation. I.e., the shell has claimed the uart, but with `CONFIG_SHELL_AUTOSTART=n` technically shouldn't be actually used at the time. 

[Vidar Berg]

After reading your problem description again and reviewing the implementation a bit more, I understand the problem better, but I'm not sure I see a clean solution to this. As you pointed out, the shell uart backend is initilized on startup regardless of the CONFIG_SHELL_AUTOSTART setting. This means the UART receiver will already be enabled when you reach main(). You could try to re-initialize he shell backend as done in this sample: https://github.com/nrfconnect/sdk-zephyr/blob/29feb191e381db3f73eb2443f0bec047837d5086/samples/subsys/shell/shell_module/src/uart_reinit.c#L11 to see if this enables you to properly re-enable the receiver.

[kat829]

Thanks for pointers Vidar! 

I am afraid I am still missing something. 

I have reduced the code to a one function that just tries to uninitialize the shell, stop and resume the device, and reinitialize it again: 

void PinWiring_Test() {
    // ### Uninitialize the shell
    {
        // uninitialize the shell
        int err;
        const struct shell *sh = NULL;

        LogS("Uninitializing shell...\n");

        sh = shell_backend_uart_get_ptr();
        shell_uninit(sh, shell_uninit_cb);

        k_sleep(K_MSEC(500));

        //disable uart rx
        err = uart_rx_disable(uart0.device);
        if (err != 0) {
            LogS("Failed to disable UART RX: %d\n", err);
        }
    }

    // ### Suspend uart0
    {
        int ret = pm_device_action_run(uart0.device, PM_DEVICE_ACTION_SUSPEND);
        if (ret != 0) {
            LogS("Failed to suspend device: %d\n", ret);
        }
        LogS("Suspended device %s\n", uart0.name);
    }
    // ### Resume uart0
    {
        int ret = pm_device_action_run(uart0.device, PM_DEVICE_ACTION_RESUME);
        if (ret != 0) {
            LogS("Failed to resume device: %d\n", ret);
        }
        LogS("Resumed device %s\n", uart0.name);
    }
    // ### (Re)start the shell
    {
        int ret;
        const struct shell *sh = NULL;

        sh = shell_backend_uart_get_ptr();

        if (!sh) {
            LogS("Shell backend not found\n");
            return -ENODEV;
        }

        LogS("A\n");

        // (Re)initialize the shell
        bool log_backend = CONFIG_SHELL_BACKEND_SERIAL_LOG_LEVEL > 0;
        uint32_t level = (CONFIG_SHELL_BACKEND_SERIAL_LOG_LEVEL > LOG_LEVEL_DBG) ? CONFIG_LOG_MAX_LEVEL : CONFIG_SHELL_BACKEND_SERIAL_LOG_LEVEL;
        k_sleep(K_MSEC(500));

        // ### I am getting the error inside this call:
        ret = shell_init(sh, uart0.device, sh->ctx->cfg.flags, log_backend, level);
        if (ret < 0) {
            LogS("Shell init failed: %d\n", ret);
            return ret;
        }

        LogS("A1\n");

        // Start the shell
        ret = shell_start(sh);
        if (ret < 0) {
            LogS("Shell start failed: %d\n", ret);
            return ret;
        }
        LogS("A2\n");

        // init our custom commands
        InitShell();
    }
}

But this fails with Busy when the shell tries to uart_rx_enable again.

fatal_error: Resetting system
async RX enable: 0
Uninitializing shell...
Shell uninitialized with result: 0
Suspended device uart0
Resumed device uart0
A
async RX enable: -16
ASSERTION FAIL @ WEST_TOPDIR/zephyr/subsys/shell/backends/shell_uart.c:261 
os: r0/a1:  0x00000004  r1/a2:  0x00000105  r2/a3:  0x20012048
os: r3/a4:  0x00000004 r12/ip:  0x2000f580 r14/lr:  0x0004d929
os:  xpsr:  0x01000000
os: r4/v1:  0xfffffff0  r5/v2:  0x00082468  r6/v3:  0x20014745
os: r7/v4:  0x00000001  r8/v5:  0x00082468  r9/v6:  0x0000030c
os: r10/v7: 0x00000000  r11/v8: 0x00000000    psp:  0x200376e8
os: EXC_RETURN: 0x0
os: Faulting instruction address (r15/pc): 0x00078290
os: >>> ZEPHYR FATAL ERROR 4: Kernel panic on CPU 0
os: Current thread: 0x20012048 (main)
===== PANIC ==

I.e., calling shell_init goes into shell_uart.c, function async_init. There it calls uart_rx_enable which returns -16 (busy) then (inside the async_init) assert (err == 0) is encountered.

When trying to trace uart_rx_enable, I got thoroughly lost.

[Vidar Berg]

Please try to comment this return at this line https://github.com/nrfconnect/sdk-zephyr/blob/37b9a01aa8037dc373465760ee3c64cba2179318/drivers/serial/uart_nrfx_uarte.c#L1956 and see if it works then. This isn't a solution or workaround, but it would help us understand whether the only remaining problem is that the shell backend fails to re-enable reception.