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.

Sign up for GitHub

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

drivers: serial: nrfx_uarte: Improve pin retention #81273

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

drivers: serial: nrfx_uarte: Improve pin retention #81273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cdc0d10
dts: bindings: serial: nrfx_uarte: Add pin-retention property
nordic-krch Nov 12, 2024
c31750c
drivers: serial: nrfx_uarte: Retain pins only when needed
nordic-krch Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions drivers/serial/uart_nrfx_uarte.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,9 @@ struct uarte_nrfx_data {
/* If enabled then UARTE peripheral is using memory which is cacheable. */
#define UARTE_CFG_FLAG_CACHEABLE BIT(3)

/* If enabled then pins must be retained when UARTE is disabled. */
#define UARTE_CFG_FLAG_RETAIN_PINS BIT(4)

/* Macro for converting numerical baudrate to register value. It is convenient
* to use this approach because for constant input it can calculate nrf setting
* at compile time.
Expand Down Expand Up @@ -593,7 +596,9 @@ static void uarte_periph_enable(const struct device *dev)
(void)data;
nrf_uarte_enable(uarte);
#ifdef CONFIG_SOC_NRF54H20_GPD
nrf_gpd_retain_pins_set(config->pcfg, false);
if (config->flags & UARTE_CFG_FLAG_RETAIN_PINS) {
nrf_gpd_retain_pins_set(config->pcfg, false);
}
#endif
#if UARTE_BAUDRATE_RETENTION_WORKAROUND
nrf_uarte_baudrate_set(uarte,
Expand Down Expand Up @@ -710,7 +715,9 @@ static void uarte_disable_locked(const struct device *dev, uint32_t dis_mask)
#ifdef CONFIG_SOC_NRF54H20_GPD
const struct uarte_nrfx_config *cfg = dev->config;

nrf_gpd_retain_pins_set(cfg->pcfg, true);
if (cfg->flags & UARTE_CFG_FLAG_RETAIN_PINS) {
nrf_gpd_retain_pins_set(cfg->pcfg, true);
}
#endif
nrf_uarte_disable(get_uarte_instance(dev));
}
Expand Down Expand Up @@ -2181,7 +2188,9 @@ static void uarte_pm_suspend(const struct device *dev)
}

#ifdef CONFIG_SOC_NRF54H20_GPD
nrf_gpd_retain_pins_set(cfg->pcfg, true);
if (cfg->flags & UARTE_CFG_FLAG_RETAIN_PINS) {
nrf_gpd_retain_pins_set(cfg->pcfg, true);
}
#endif

nrf_uarte_disable(uarte);
Expand Down Expand Up @@ -2395,6 +2404,8 @@ static int uarte_instance_init(const struct device *dev,
(!IS_ENABLED(CONFIG_HAS_NORDIC_DMM) ? 0 : \
(UARTE_IS_CACHEABLE(idx) ? \
UARTE_CFG_FLAG_CACHEABLE : 0)) | \
(UARTE_PROP(idx, pin_retention) ? \
UARTE_CFG_FLAG_RETAIN_PINS : 0) | \
USE_LOW_POWER(idx), \
UARTE_DISABLE_RX_INIT(UARTE(idx)), \
.poll_out_byte = &uarte##idx##_poll_out_byte, \
Expand Down
6 changes: 6 additions & 0 deletions dts/bindings/serial/nordic,nrf-uarte.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,9 @@ properties:
type: boolean
description: |
UARTE has RX frame timeout HW feature.
pin-retention:
type: boolean
description: |
UARTE pin states must be retained when peripheral is disabled.
It is required when CTRLSEL is used to route UARTE pins. It is always the
case for fast peripheral and in special cases for slow peripherals.
Comment on lines +17 to +22
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be inferred from the power domain UART resides?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but there are special cases where for example UARTE130 can use P9.5 for TX and that configuration goes through CTRLSEL so I added this property so that there is a possibility to add pin-retention to uart130 node.

Copy link
Collaborator

@masz-nordic masz-nordic Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried that expecting users to remember to add this in special cases may lead to bug reports.
Is there any alternative to automate this? Or at least throw an error when property is missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing that comes to my mind is to add build assert in code where there are fixed location checked but we try to avoid putting soc specific stuff in the code.

Maybe we could come up with a way of describing that in DT. e.g. create a description of pool of available pins with some specific properties that are required for a given pool. This could be useful not only in this case but also in case of Lumos where peripherals are fixed to a single gpio port.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should likely review the pinctrl model we have, as new socs have changed it. For example, 54H20 has a mix of PSEL (peripheral-level) and CTRLSEL (classical multiplexer).

1 change: 1 addition & 0 deletions dts/common/nordic/nrf54h20.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,7 @@
power-domains = <&gpd NRF_GPD_FAST_ACTIVE1>;
endtx-stoptx-supported;
frame-timeout-supported;
pin-retention;
};

spi121: spi@8e7000 {
Expand Down
1 change: 1 addition & 0 deletions dts/common/nordic/nrf9280.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@
interrupts = <230 NRF_DEFAULT_IRQ_PRIORITY>;
endtx-stoptx-supported;
frame-timeout-supported;
pin-retention;
};

spi121: spi@8e7000 {
Expand Down
Loading