Zephyr: UART_INTERRUPT_DRIVEN broken on SOC_SERIES_IMX7_M4 and SOC_SERIES_IMX_6X_M4

[diegosueiro]

Shell module sample app was working without any issues on v.1.13 and after testing it on master no characters are not shown in the console.

After bisecting it I found that the issue came up with the 4962618 commit.

Using the debugger I could track that the uart_imx_isr is always called even if there is no received or data to transmit.

Disabling the CONFIG_UART_INTERRUPT_DRIVEN the shell prints the prompt but it doesn't process the user entry. The sys_dlist_get is always returning NULL when called from _impl_k_poll_signal_raise which was called from shell_transport_evt_handler.

Because of memory limitations I'm using the prj_minimal.conf:
cmake -DBOARD=colibri_imx7d_m4 -DCONF_FILE=prj_minimal.conf ..

@stanislav-poboril, is it possible to test this in the UDOO Neo since it uses the same uart driver?

Is issue is possibly related to #11281, #11202 #8869.

[diegosueiro]

Some additional information.

When compiling with CONFIG_UART_INTERRUPT_DRIVEN=y the UART_SetIntCmd appears this way in the zephyr.lst (attached) file:

1fff90b2 <UART_SetIntCmd>:
1fff90b2:       f7ff bff0       b.w     1fff9096 <UART_SetDmaCmd>

As far as I understand the UART_SetIntCmd is calling the UART_SetDmaCmd and it shouldn't.

I don't have any idea how to investigate this. Is this a compiler/liker issue? The function declaration and protopyte looks ok to me.

zephyr.lst.txt

[stanislav-poboril]

Hi @diegosueiro, I have verified that the UDOO Neo has the same problem on master while not having it on 1.13.

As for the UART_SetIntCmd() and UART_SetDmaCmd, the functions looks the same except for the name of one parameter. So compiler probably optimized it this way and it should be ok.

[pfalcon]

@diegosueiro : Thanks for ticketing this issue.

[pfalcon]

Using the debugger I could track that the uart_imx_isr is always called even if there is no received or data to transmit.

Do you want to say that running with shell enabled, your board is effectively locked up, always spinning in calls to ISR?

[pfalcon]

@diegosueiro : Can you try samples/subsys/console/echo, and post the exact output you get (plus description of what happens if you type chars of course). Thanks.

[diegosueiro]

Using the debugger I could track that the uart_imx_isr is always called even if there is no received or data to transmit.

Do you want to say that running with shell enabled, your board is effectively locked up, always spinning in calls to ISR?

Yep.

[diegosueiro]

@diegosueiro : Can you try samples/subsys/console/echo, and post the exact output you get (plus description of what happens if you type chars of course). Thanks.

This is the output for the console echo sample app when I type one key (to send one character in the uart to the board):

***** Booting Zephyr OS zephyr-v1.13.0-1905-gf2d8d93 *****
Start typing characters to see them echoed back
***** USAGE FAULT *****
  Attempt to execute undefined instruction
***** Hardware exception *****
Current thread ID = 0x200002b8
Faulting instruction address = 0x90f8d504
Fatal fault in ISR! Spinning...

Running the addr2line didn't help:

addr2line -a 0x90f8d504 -e zephyr/zephyr.elf 
0x90f8d504
??:0

[diegosueiro]

Hi @diegosueiro, I have verified that the UDOO Neo has the same problem on master while not having it on 1.13.

As for the UART_SetIntCmd() and UART_SetDmaCmd, the functions looks the same except for the name of one parameter. So compiler probably optimized it this way and it should be ok.

@stanislav-poboril , I used the step-by-step execution with j-link and it went through the UART_SetDmaCmd function.

[diegosueiro]

@pfalcon ,

The console echo is broken on v1.13.0. I think I should change the issue subject.

If I checkout at the commit ecc891b2 the console echo works fine.

[stanislav-poboril]

Hi @diegosueiro, I have verified that the UDOO Neo has the same problem on master while not having it on 1.13.

As for the UART_SetIntCmd() and UART_SetDmaCmd, the functions looks the same except for the name of one parameter. So compiler probably optimized it this way and it should be ok.

@stanislav-poboril , I used the step-by-step execution with j-link and it went through the UART_SetDmaCmd function.

@diegosueiro Yes and I think it is OK. The two functions are identical, so it is enough to branch the execution from UART_SetIntCmd() into UART_SetDmaCmd() function without preparing the function parameters and adjusting a call stack. The compiler probably does this to minimize the code size.

[diegosueiro]

@pfalcon and @stanislav-poboril ,

I don't have enough time to investigate this issue at the moment because I'm in the middle of another work for Zephyr.

It should be great if one of you could help to look at this in the UDOO Neo board.

[pfalcon]

@diegosueiro: Same stuff here. (All the time, but lately even worse than usual ;-) ). At the least, I need to finish with MPS2 issue before looking into something else. But we'll get to it - ticket is here, so it won't be lost.

Note that I don't any device with that, so my review will be only speculative.

[stanislav-poboril]

@diegosueiro , @pfalcon , I can investigate it on UDOO Neo board once I will get to it.

[diegosueiro]

@pfalcon and @stanislav-poboril ,

I found the issue with the console echo sample app:

diff --git a/drivers/serial/uart_imx.c b/drivers/serial/uart_imx.c
index 9bc5f3b..5a24ab9 100644
--- a/drivers/serial/uart_imx.c
+++ b/drivers/serial/uart_imx.c
@@ -254,7 +254,7 @@ void uart_imx_isr(void *arg)
        struct imx_uart_data *data = dev->driver_data;

        if (data->callback) {
-               data->callback(dev);
+               data->callback(data->cb_data);
        }
 }
 #endif /* CONFIG_UART_INTERRUPT_DRIVEN */

Later I'll test on the shell module sample app.

[pfalcon]

Oh damn, my fault. Thanks for investigating it!

[diegosueiro]

@nashif,

Should I close this issue since the PR #11778 fixed it?

[pfalcon]

@diegosueiro : You're the author of this bugreport, you would be among first to know if it should be closed or not. If you'd have added:
~
Fixes: #11465
~

To the commit message of #11778, it would be closed automatically, and that's recommended practice, as we have really high ticket flow indeed to track/perform manual closures.

[diegosueiro]

@diegosueiro : You're the author of this bugreport, you would be among first to know if it should be closed or not. If you'd have added:

Fixes: #11465

To the commit message of #11778, it would be closed automatically, and that's recommended practice, as we have really high ticket flow indeed to track/perform manual closures.

Thanks Paul. I'll close it manually.