Skip to content

soc: nordic: nrf53: assign pin xl1,xl2 to app core if lfxo disabled #92664

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+10 −78
Open

soc: nordic: nrf53: assign pin xl1,xl2 to app core if lfxo disabled #92664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
63 changes: 0 additions & 63 deletions soc/nordic/nrf53/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,69 +210,6 @@ config BOARD_ENABLE_CPUNET

if !TRUSTED_EXECUTION_NONSECURE || BUILD_WITH_TFM

config SOC_ENABLE_LFXO
bool "LFXO"
select DEPRECATED
help
This option is deprecated, use DT instead. For this option to apply,
make sure to select either "internal" or "external" in the
load-capacitors property.

Enable the low-frequency oscillator (LFXO) functionality on XL1 and
XL2 pins.
This option must be enabled if either application or network core is
to use the LFXO. Otherwise, XL1 and XL2 pins will behave as regular
GPIOs.

choice SOC_LFXO_LOAD_CAPACITANCE
prompt "LFXO load capacitance"
depends on SOC_ENABLE_LFXO

config SOC_LFXO_CAP_EXTERNAL
bool "Use external load capacitors"
select DEPRECATED
help
This option is deprecated, use DT instead. Example configuration:

&lfxo {
load-capacitors = "external";
};

config SOC_LFXO_CAP_INT_6PF
bool "6 pF internal load capacitance"
select DEPRECATED
help
This option is deprecated, use DT instead. Example configuration:

&lfxo {
load-capacitors = "internal";
load-capacitance-picofarad = <6>;
};

config SOC_LFXO_CAP_INT_7PF
bool "7 pF internal load capacitance"
select DEPRECATED
help
This option is deprecated, use DT instead. Example configuration:

&lfxo {
load-capacitors = "internal";
load-capacitance-picofarad = <7>;
};

config SOC_LFXO_CAP_INT_9PF
bool "9 pF internal load capacitance"
select DEPRECATED
help
This option is deprecated, use DT instead. Example configuration:

&lfxo {
load-capacitors = "internal";
load-capacitance-picofarad = <9>;
};

endchoice

choice SOC_HFXO_LOAD_CAPACITANCE
prompt "HFXO load capacitance"
default SOC_HFXO_CAP_DEFAULT
Expand Down
25 changes: 10 additions & 15 deletions soc/nordic/nrf53/soc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,17 @@
#define HFXO_NODE DT_NODELABEL(hfxo)

/* LFXO config from DT */
#if DT_NODE_HAS_STATUS_OKAY(LFXO_NODE)
BUILD_ASSERT(DT_NODE_HAS_PROP(LFXO_NODE, load_capacitors));
Copy link
Member

Choose a reason for hiding this comment

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

It's probably better to mark this property as required in the devicetree binding. But anyway, as long as the legacy Kconfig options are available, we cannot require this property to be present.

Copy link
Contributor

@bjarki-andreasen bjarki-andreasen Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Marking it as required is an issue since the lfxo is not neccesarily present, which I believe is noted by the lfxo node having status = "disabled". In this case, if the prop is required, some random value has to be set even though it can not be correct given there is no lfxo on the board :) The same issue is present for the comparator where some properties are required only if the comp is actually enabled :)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think I understand the issue. Properties with required: true are required by dtc only for nodes with status "okay", so for disabled nodes you don't need to provide any value for them.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, I though they where always required, given what "disabled" means according to devicetree spec, seems we have our own rule which matches zephyrs bindings better :)

#define LFXO_PIN_SEL NRF_GPIO_PIN_SEL_PERIPHERAL
#if DT_ENUM_HAS_VALUE(LFXO_NODE, load_capacitors, external)
#define LFXO_CAP NRF_OSCILLATORS_LFXO_CAP_EXTERNAL
#elif DT_ENUM_HAS_VALUE(LFXO_NODE, load_capacitors, internal)
#define LFXO_CAP (DT_ENUM_IDX(LFXO_NODE, load_capacitance_picofarad) + 1U)
#endif /*DT_ENUM_HAS_VALUE(LFXO_NODE, load_capacitors, external) */
#else
/* LFXO config from legacy Kconfig */
#if defined(CONFIG_SOC_LFXO_CAP_INT_6PF)
#define LFXO_CAP NRF_OSCILLATORS_LFXO_CAP_6PF
#elif defined(CONFIG_SOC_LFXO_CAP_INT_7PF)
#define LFXO_CAP NRF_OSCILLATORS_LFXO_CAP_7PF
#elif defined(CONFIG_SOC_LFXO_CAP_INT_9PF)
#define LFXO_CAP NRF_OSCILLATORS_LFXO_CAP_9PF
#else
#define LFXO_CAP NRF_OSCILLATORS_LFXO_CAP_EXTERNAL
#endif
#endif
#define LFXO_PIN_SEL NRF_GPIO_PIN_SEL_APP
#endif /* DT_NODE_HAS_STATUS_OKAY(LFXO_NODE) */

/* HFXO config from DT */
#if DT_ENUM_HAS_VALUE(HFXO_NODE, load_capacitors, internal)
Expand Down Expand Up @@ -496,17 +491,17 @@ void soc_early_init_hook(void)
#endif

#ifdef CONFIG_SOC_NRF5340_CPUAPP
#if defined(LFXO_CAP)
#if DT_NODE_HAS_STATUS_OKAY(LFXO_NODE)
nrf_oscillators_lfxo_cap_set(NRF_OSCILLATORS, LFXO_CAP);
#if !defined(CONFIG_BUILD_WITH_TFM)
/* This can only be done from secure code.
* This is handled by the TF-M platform so we skip it when TF-M is
* enabled.
*/
nrf_gpio_pin_control_select(PIN_XL1, NRF_GPIO_PIN_SEL_PERIPHERAL);
nrf_gpio_pin_control_select(PIN_XL2, NRF_GPIO_PIN_SEL_PERIPHERAL);
nrf_gpio_pin_control_select(PIN_XL1, LFXO_PIN_SEL);
nrf_gpio_pin_control_select(PIN_XL2, LFXO_PIN_SEL);
#endif /* !defined(CONFIG_BUILD_WITH_TFM) */
#endif /* defined(LFXO_CAP) */
#endif /* DT_NODE_HAS_STATUS_OKAY(LFXO_NODE) */
Copy link
Member

Choose a reason for hiding this comment

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

How about defining something like LFXO_PIN_SEL depending on the lfxo node status and using here just:

#if defined(LFXO_CAP)
	nrf_oscillators_lfxo_cap_set(NRF_OSCILLATORS, LFXO_CAP);
#endif
#if !defined(CONFIG_BUILD_WITH_TFM)
	/* This can only be done from secure code.
	 * This is handled by the TF-M platform so we skip it when TF-M is
	 * enabled.
	 */
	nrf_gpio_pin_control_select(PIN_XL1, LFXO_PIN_SEL);
	nrf_gpio_pin_control_select(PIN_XL2, LFXO_PIN_SEL);
#endif /* !defined(CONFIG_BUILD_WITH_TFM) */

Wouldn't that be clearer?

#if defined(HFXO_CAP_VAL_X2)
/* This register is only accessible from secure code. */
uint32_t xosc32mtrim = soc_secure_read_xosc32mtrim();
Expand Down
Loading