drivers: serial: nrfx_uarte: UART shim based on nrfx_uarte driver #65155

nordic-krch · 2023-11-14T07:36:43Z

PR adds new Nordic UART shim which is based on the nrfx_uarte driver. Legacy shim is kept for now and can be used after CONFIG_UART_NRFX_UARTE_LEGACY_SHIM=y.

New shim implements only polling and asynchronous API. Interrupt driven API is implemented via generic asynchronous to interrupt driven adaptation layer. New shim is using different approach to RX bytes counting (legacy shim was using RXDRDY interrupt or dedicated TIMER peripheral).

New approach changes the behavior of the RX while being compliant with the API. When there is no RX activity for the specified amount of time UART RX is restarted (stopped with ENDRX_STARTRX short being set). With this approach there is no need for byte counting since DMA transfer ends on timeout and AMOUNT register is updated. However, it means that every UART_RX_RDY event comes with the new buffer (offset in uart_rx_event is always 0).

PR contains asynchronous to interrupt driven adaptation layer which is using uart_async_rx helper added by #63967.

PR is based on #65113 and #63967.

zephyrbot · 2023-11-15T14:31:29Z

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff

Note: This message is automatically posted and updated by the Manifest GitHub Action.

drivers/serial/Kconfig

anangl · 2024-01-17T10:59:18Z

drivers/serial/uart_nrfx_uarte2.c

+		if ((IS_INT_DRIVEN_API(dev) || IS_POLLING_API(dev)) &&
+		     !(cfg->flags & UARTE_CFG_FLAG_NO_RX)) {


For consistency with PM_DEVICE_ACTION_RESUME:

Suggested change

if ((IS_INT_DRIVEN_API(dev) || IS_POLLING_API(dev)) &&

!(cfg->flags & UARTE_CFG_FLAG_NO_RX)) {

if (!IS_ASYNC_API(dev) && !(cfg->flags & UARTE_CFG_FLAG_NO_RX)) {

anangl · 2024-01-17T11:15:46Z

drivers/serial/uart_async_to_irq.c

+
+static const struct uart_async_to_irq_config *get_config(const struct device *dev)
+{
+	const struct uart_async_to_irq_config **config = dev->config;


Sorry, I missed that one const in my previous comment.

Suggested change

const struct uart_async_to_irq_config **config = dev->config;

const struct uart_async_to_irq_config *const *config = dev->config;

Add adaptation layer which allows to provide interrupt driven API for drivers which exposes only asynchronous API. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

Since it takes 400 bytes of code and it is rarely used disable by default this feature. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

Add Kconfig options to new configuration flags for nrfx_uarte. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

gmarull

IMHO this should be fixed better https://github.com/zephyrproject-rtos/zephyr/pull/65155/files#r1450353386, but let's move on.

aescolar · 2024-01-17T16:38:38Z

drivers/serial/uart_nrfx_uarte2.c

+			k_msleep(1);
+		}
+		Z_SPIN_DELAY(3);
+	} while (err != NRFX_ERROR_BUSY);


Isn't this a bug? shouldn't it be
} while (err != NRFX_SUCCESS); ?
Otherwise the same byte would be enqueued again if nrfx_uarte_tx() enqueued it right away, or?

Yes, it should be while (err == NRFX_ERROR_BUSY).

anangl

See #65155 (comment).

Add new shim which is based on nrfx driver. Legacy shim is kept for transition period. It can be used after setting CONFIG_UART_NRFX_UARTE_LEGACY_SHIM. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

Align test configurations to the new uart driver based on nrfx_uarte. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

Add configuration which tests legacy nrf uart driver. Prevent tests on nrf52_bsim of the new driver (it is requiring adjustments for the simulation). It will be fixed by another patch. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

New nrf uart shim requires addiotional work around PM. Use legacy version for those tests until it is fixed. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

New UART shim must be adapted to work in the simulated environment. Use legacy version until it is fixed. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

Add adaptation layer which allows to provide interrupt driven API for drivers which exposes only asynchronous API. Cherry-picked from zephyrproject-rtos#65155. This commit should not be reviewed in this PR. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

Since it takes 400 bytes of code and it is rarely used disable by default this feature. Cherry picked from zephyrproject-rtos#65155. This commit should not be reviewed in this PR. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

Add new shim which is based on nrfx driver. Legacy shim is kept for transition period. It can be used after setting CONFIG_UART_NRFX_UARTE_LEGACY_SHIM. Cherry-picked from zephyrproject-rtos#65155. This commit should not be reviewd in this PR. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

Align test configurations to the new uart driver based on nrfx_uarte. Cherry picked from zephyrproject-rtos#65155. This commit should not be review in this PR. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

Check was complaining when Kconfig flag was build using concatenation via a macro (it was covering explicit token pasting ##). Added negative lookbehind for CAT( which should cover exisiting concatenation macros (UTIL_CAT, CONCAT, CAT). Cherry picked from zephyrproject-rtos#65155. This commit should not be review in this PR. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

nordic-krch force-pushed the new_nrfx_uarte branch 2 times, most recently from b027a7a to 83e124b Compare November 15, 2023 14:30

zephyrbot added manifest manifest-hal_nordic DNM This PR should not be merged (Do Not Merge) labels Nov 15, 2023

nordic-krch force-pushed the new_nrfx_uarte branch 3 times, most recently from a6e25db to c92d171 Compare November 20, 2023 12:28

nordic-krch force-pushed the new_nrfx_uarte branch 4 times, most recently from 946e8e6 to d58bbf3 Compare December 13, 2023 15:39

nordic-krch force-pushed the new_nrfx_uarte branch from d58bbf3 to a273de8 Compare January 3, 2024 14:19

nordic-krch marked this pull request as ready for review January 3, 2024 14:19

nordic-krch requested a review from anangl as a code owner January 3, 2024 14:19

zephyrbot added area: Base OS Base OS Library (lib/os) platform: nRF Nordic nRFx area: UART Universal Asynchronous Receiver-Transmitter labels Jan 3, 2024

zephyrbot requested review from andyross, dcpleung, hubertmis, nashif and peter-mitsis January 3, 2024 14:20

zephyrbot assigned dcpleung Jan 3, 2024

dcpleung reviewed Jan 3, 2024

View reviewed changes

drivers/serial/Kconfig Outdated Show resolved Hide resolved

nordic-krch force-pushed the new_nrfx_uarte branch from a273de8 to 33cffd8 Compare January 4, 2024 10:17

zephyrbot added area: Coding Guidelines Coding guidelines and style area: Continuous Integration labels Jan 4, 2024

zephyrbot requested a review from carlescufi January 4, 2024 10:17

anangl previously approved these changes Jan 17, 2024

View reviewed changes

anangl reviewed Jan 17, 2024

View reviewed changes

nordic-krch dismissed anangl’s stale review via 560afe1 January 17, 2024 11:25

nordic-krch force-pushed the new_nrfx_uarte branch 2 times, most recently from 560afe1 to d882b8e Compare January 17, 2024 11:56

zephyrbot added the platform: nRF BSIM Nordic Semiconductors, nRF BabbleSim label Jan 17, 2024

zephyrbot requested a review from aescolar January 17, 2024 11:57

carlescufi requested a review from anangl January 17, 2024 13:41

anangl approved these changes Jan 17, 2024

View reviewed changes

nordic-krch added 3 commits January 17, 2024 14:58

drivers: serial: Add async to interrupt driven adaptation

efb1b3e

Add adaptation layer which allows to provide interrupt driven API for drivers which exposes only asynchronous API. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

soc: arm: nordic_nrf: Disable UART runtime configuration

5d657ab

Since it takes 400 bytes of code and it is rarely used disable by default this feature. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

modules: hal_nordic: nrfx: Add new UARTE options to Kconfig

5596455

Add Kconfig options to new configuration flags for nrfx_uarte. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

gmarull previously approved these changes Jan 17, 2024

View reviewed changes

aescolar reviewed Jan 17, 2024

View reviewed changes

anangl requested changes Jan 17, 2024

View reviewed changes

nordic-krch dismissed gmarull’s stale review via cfa4d58 January 17, 2024 18:41

nordic-krch force-pushed the new_nrfx_uarte branch from d882b8e to cfa4d58 Compare January 17, 2024 18:41

nordic-krch added 5 commits January 17, 2024 20:41

drivers: serial: nrfx: Add new shim based on nrfx_uarte

d38e688

Add new shim which is based on nrfx driver. Legacy shim is kept for transition period. It can be used after setting CONFIG_UART_NRFX_UARTE_LEGACY_SHIM. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

tests: drivers: uart: uart_mix_fifo_poll: Align the the new nrfx driver

a64a5f0

Align test configurations to the new uart driver based on nrfx_uarte. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

tests: drivers: uart: uart_pm: Use legacy nrf driver

7972e4f

New nrf uart shim requires addiotional work around PM. Use legacy version for those tests until it is fixed. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

boards: posix: nrf_bsim: Use legacy UART shim

cfa4d58

New UART shim must be adapted to work in the simulated environment. Use legacy version until it is fixed. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

anangl approved these changes Jan 17, 2024

View reviewed changes

gmarull approved these changes Jan 18, 2024

View reviewed changes

carlescufi merged commit e2dccab into zephyrproject-rtos:main Jan 18, 2024
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: serial: nrfx_uarte: UART shim based on nrfx_uarte driver #65155

drivers: serial: nrfx_uarte: UART shim based on nrfx_uarte driver #65155

nordic-krch commented Nov 14, 2023

zephyrbot commented Nov 15, 2023 •

edited

anangl Jan 17, 2024

anangl Jan 17, 2024

gmarull left a comment

aescolar Jan 17, 2024 •

edited

anangl Jan 17, 2024

anangl left a comment

		if ((IS_INT_DRIVEN_API(dev) \|\| IS_POLLING_API(dev)) &&
		!(cfg->flags & UARTE_CFG_FLAG_NO_RX)) {

	if ((IS_INT_DRIVEN_API(dev) \|\| IS_POLLING_API(dev)) &&
	!(cfg->flags & UARTE_CFG_FLAG_NO_RX)) {
	if (!IS_ASYNC_API(dev) && !(cfg->flags & UARTE_CFG_FLAG_NO_RX)) {

	const struct uart_async_to_irq_config **config = dev->config;
	const struct uart_async_to_irq_config const config = dev->config;

drivers: serial: nrfx_uarte: UART shim based on nrfx_uarte driver #65155

drivers: serial: nrfx_uarte: UART shim based on nrfx_uarte driver #65155

Conversation

nordic-krch commented Nov 14, 2023

zephyrbot commented Nov 15, 2023 • edited

anangl Jan 17, 2024

Choose a reason for hiding this comment

anangl Jan 17, 2024

Choose a reason for hiding this comment

gmarull left a comment

Choose a reason for hiding this comment

aescolar Jan 17, 2024 • edited

Choose a reason for hiding this comment

anangl Jan 17, 2024

Choose a reason for hiding this comment

anangl left a comment

Choose a reason for hiding this comment

zephyrbot commented Nov 15, 2023 •

edited

aescolar Jan 17, 2024 •

edited