Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UART console input does not work on SAM E5x #23281

Closed
stephanosio opened this issue Mar 5, 2020 · 16 comments · Fixed by #23289
Closed

UART console input does not work on SAM E5x #23281

stephanosio opened this issue Mar 5, 2020 · 16 comments · Fixed by #23289
Assignees
Labels
area: Drivers area: UART Universal Asynchronous Receiver-Transmitter bug The issue is a bug, or the PR is fixing a bug platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) priority: medium Medium impact/importance bug

Comments

@stephanosio
Copy link
Member

stephanosio commented Mar 5, 2020

Describe the bug
UART (serial) console input does not work on the SAM E5x series devices (tested on atsame54_xpro).

To Reproduce
Steps to reproduce the behavior:

  1. mkdir build; cd build
  2. cmake -DBOARD=atsame54_xpro ..
  3. make
  4. Flash and run

Expected behavior
The serial console should echo the typed character and the test should succeed once the enter key is pressed.

Impact
Nothing that requires serial console input works (notably, the shell).

Screenshots or console output

*** Booting Zephyr OS build v2*** Booting Zephyr OS build v2.2.0-rc3-45-gd48c2c4e969f  ***
Running test suite uart_basic_test
===================================================================
starting test - test_uart_fifo_fill
This is a FIFO test.
PASS - test_uart_fifo_fill
===================================================================
starting test - test_uart_fifo_read
Please send characters to serial console

Environment (please complete the following information):

  • OS: Ubuntu 18.04
  • Toolchain: Zephyr SDK 0.11.2
  • Commit SHA: f756432

Additional context

  • In summary, UART RX does not seem to be working properly for some reason. This problem applies to any test/sample that expects the serial console user input (e.g. shell).
  • This issue might not be specific to SAM D and E5x, and might also be applicable to all SAM0 family devices (e.g. SAM D21).
@stephanosio stephanosio added bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug area: Drivers area: UART Universal Asynchronous Receiver-Transmitter platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) labels Mar 5, 2020
@stephanosio stephanosio self-assigned this Mar 5, 2020
@stephanosio
Copy link
Member Author

cc @benpicco

Could you check if this issue is reproducible on your side?

@stephanosio
Copy link
Member Author

To anyone with access to a SAM D2x development board:

Could you check if this issue is reproducible on the board?

@carlescufi
Copy link
Member

@nandojve do you have this platform to test?

@benpicco
Copy link
Collaborator

benpicco commented Mar 5, 2020

I can confirm the issue on atsame54_xpro with the shell example.
With atsamr21_xpro it's working.

Looks like this check to detect the additional UART interrupts on same5x broke.

@benpicco
Copy link
Collaborator

benpicco commented Mar 5, 2020

I supposed checking for the existence of DT_INST_0_ATMEL_SAM0_UART_IRQ_3 would work - or is there a better way to get the number of UART IRQs from the device tree?

@stephanosio
Copy link
Member Author

I supposed checking for the existence of DT_INST_0_ATMEL_SAM0_UART_IRQ_3 would work - or is there a better way to get the number of UART IRQs from the device tree?

I can see that is indeed the problem.

Checking for the existence of DT_INST_0_ATMEL_SAM0_UART_IRQ_3 would work, but the code checks for DT_ATMEL_SAM0_UART_0_IRQ_3 which has been phased out and replaced by DT_INST_.

I have opened a PR that fixes this problem #23289.

@stephanosio
Copy link
Member Author

stephanosio commented Mar 5, 2020

Is there a better way to get the number of UART IRQs from the device tree?

Unless we add a new kind of device tree symbol that specifies the number of interrupts connected to a device, I think the following would be the best we can do:

  1. Define a Kconfig helper symbol that resolves SoC-specific IRQ count (AFAIK, only SAM D/E5x has 4 IRQs per SERCOM).
  2. Use that Kconfig symbol to connect IRQ(s).

cc @galak

@nandojve
Copy link
Member

nandojve commented Mar 5, 2020

@carlescufi Yes I had a SAMD20_XPRO and can test tonight.
I've been addressing all DT_INST changes on #23107.

@benpicco
Copy link
Collaborator

benpicco commented Mar 5, 2020

Looks like i2c has the same issue.

Searching for DT_ATMEL_SAM0_ in drivers/ gives plenty results - is that a problem?

@nandojve
Copy link
Member

nandojve commented Mar 5, 2020

@benpicco #23107

@stephanosio
Copy link
Member Author

stephanosio commented Mar 5, 2020

Looks like i2c has the same issue.

Searching for DT_ATMEL_SAM0_ in drivers/ gives plenty results - is that a problem?

Thanks for pointing it out. DT_ATMEL_SAM0_I2C_0_IRQ_3 does need to be fixed as it uses the obsolete DT names. => fixed by #23296.

DT_ATMEL_SAM0 in itself is not a problem; DT_<COMPAT>_<INSTANCE>_<PROP> is.

See 01e54a5

@stephanosio
Copy link
Member Author

In regards to #23107, I believe the DT_<COMPAT>_<INSTANCE>_<PROP> issue is a little different from the on-going DT_INST conversions, as these symbols were completely removed and should have already been renamed earlier when the mass renaming was done (I guess they missed a few in the process).

Since this issue inhibits proper operation of SERCOMs on the SAM D/E5x, IMO, this should be treated as a bug, rather than an enhancement.

@nandojve
Copy link
Member

nandojve commented Mar 6, 2020

To anyone with access to a SAM D2x development board:

Could you check if this issue is reproducible on the board?

west build -b atsamd20_xpro tests/drivers/uart/uart_basic_api
west flash

Running test suite uart_basic_*** Booting Zephyr OS build v2.2.0-rc3-49-g2835da8194b3  ***
Running test suite uart_basic_test
===================================================================
starting test - test_uart_fifo_fill
This is a FIFO test.
PASS - test_uart_fifo_fill
===================================================================
starting test - test_uart_fifo_read
Please send characters to serial console
PASS - test_uart_fifo_read
===================================================================
starting test - test_uart_poll_in
Please send characters to serial console
PASS - test_uart_poll_in
===================================================================
starting test - test_uart_poll_out
This is a POLL test.
PASS - test_uart_poll_out
===================================================================
Test suite uart_basic_test succeeded
===================================================================
PROJECT EXECUTION SUCCESSFUL

@stephanosio
Copy link
Member Author

@nandojve Thanks for checking. Noting that the SAM D2x only has 1 interrupt per SERCOM, that is indeed the expected result.

@stephanosio
Copy link
Member Author

@nandojve This is a little off-topic, but could you also check uart_async_api on the SAM D2x? That also seems to be broken on the SAM E5x even with the IRQ fix.

@nandojve
Copy link
Member

nandojve commented Mar 6, 2020

Ok two things on uart_async_api:

1- SAMD20 is the only 32-bit device on all SAM family without DMA, so build error.
2- SAMR21 build but execution on HW fail. Maybe I forgot to do something like short circuit RX/TX? Execution logs below:

*** Booting Zephyr OS build v2.2.0-rc3-49-g2835da8194b3  ***
Running test suite uart_async_test
===================================================================
starting test - test_single_read_setup
PASS - test_single_read_setup
===================================================================
starting test - test_single_read

    Assertion failed at ZEPHYR_BASE/tests/drivers/uart/uart_async_api/src/test_uart_async.c:74: test_single_read: (k_sem_take(&tx_done, K_MSEC(100)) not equal to 0)
TX_DONE timeout
FAIL - test_single_read
===================================================================
starting test - test_chained_read_setup
PASS - test_chained_read_setup
===================================================================
starting test - test_chained_read

    Assertion failed at ZEPHYR_BASE/tests/drivers/uart/uart_async_api/src/test_uart_async.c:154: test_chained_read: (k_sem_take(&tx_done, K_MSEC(100)) not equal to 0)
TX_DONE timeout
FAIL - test_chained_read
===================================================================
starting test - test_double_buffer_setup
PASS - test_double_buffer_setup
===================================================================
starting test - test_double_buffer

    Assertion failed at ZEPHYR_BASE/tests/drivers/uart/uart_async_api/src/test_uart_async.c:213: test_double_buffer: (uart_rx_enable(uart_dev, double_buffer[0], sizeof(double_buffer[0]), 50)
Failed to enable receiving
FAIL - test_double_buffer
===================================================================
starting test - test_read_abort_setup
PASS - test_read_abort_setup
===================================================================
starting test - test_read_abort

    Assertion failed at ZEPHYR_BASE/tests/drivers/uart/uart_async_api/src/test_uart_async.c:276: test_read_abort: (k_sem_take(&tx_done, K_MSEC(100)) not equal to 0)
TX_DONE timeout
FAIL - test_read_abort
===================================================================
starting test - test_chained_write_setup
PASS - test_chained_write_setup
===================================================================
starting test - test_chained_write

    Assertion failed at ZEPHYR_BASE/tests/drivers/uart/uart_async_api/src/test_uart_async.c:488: test_chained_write: (k_sem_take(&tx_done, K_MSEC(100)) not equal to 0)
TX_DONE timeout
FAIL - test_chained_write
===================================================================
starting test - test_long_buffers_setup
PASS - test_long_buffers_setup
===================================================================
starting test - test_long_buffers

    Assertion failed at ZEPHYR_BASE/tests/drivers/uart/uart_async_api/src/test_uart_async.c:566: test_long_buffers: (k_sem_take(&tx_done, K_MSEC(200)) not equal to 0)
TX_DONE timeout
FAIL - test_long_buffers
===================================================================
starting test - test_write_abort_setup
PASS - test_write_abort_setup
===================================================================
starting test - test_write_abort

    Assertion failed at ZEPHYR_BASE/tests/drivers/uart/uart_async_api/src/test_uart_async.c:342: test_write_abort: (k_sem_take(&tx_done, K_MSEC(100)) not equal to 0)
TX_DONE timeout
FAIL - test_write_abort
===================================================================
starting test - test_forever_timeout_setup
PASS - test_forever_timeout_setup
===================================================================
starting test - test_forever_timeout

    Assertion failed at ZEPHYR_BASE/tests/drivers/uart/uart_async_api/src/test_uart_async.c:411: test_forever_timeout: (k_sem_take(&tx_done, K_MSEC(100)) not equal to 0)
TX_DONE timeout
FAIL - test_forever_timeout
===================================================================
Test suite uart_async_test failed.
===================================================================
PROJECT EXECUTION FAILED

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Drivers area: UART Universal Asynchronous Receiver-Transmitter bug The issue is a bug, or the PR is fixing a bug platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants