Add global HSFLL clock control device driver #81735

bjarki-andreasen · 2024-11-21T20:57:10Z

Draft for now since testing is blocked by missing firmware for the nRF54H20

bjarki-andreasen · 2024-11-21T20:58:05Z

zephyrbot · 2024-11-21T20:58:13Z

The following west manifest projects have changed revision in this Pull Request:

Name	Old Revision	New Revision	Diff
hal_nordic	zephyrproject-rtos/hal_nordic@`e0e48c4`	zephyrproject-rtos/hal_nordic@`fae1542` (`master`)	zephyrproject-rtos/[email protected]

✅ All manifest checks OK

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

bjarki-andreasen · 2024-11-21T21:00:35Z

I need the to update the frequencies to match the opaque GDFS_FREQ_MEDHIGH in devicetree, and verify that the initial frequency of the clock is the lowest one :) Also need to find appropriate timeout for gdfs requests.

west.yml

masz-nordic · 2024-11-29T17:24:19Z

drivers/clock_control/CMakeLists.txt

@@ -37,6 +37,7 @@ zephyr_library_sources_ifdef(CONFIG_CLOCK_CONTROL_RENESAS_RA_CGC      clock_cont
 zephyr_library_sources_ifdef(CONFIG_CLOCK_CONTROL_AMBIQ               clock_control_ambiq.c)
 zephyr_library_sources_ifdef(CONFIG_CLOCK_CONTROL_PWM                 clock_control_pwm.c)
 zephyr_library_sources_ifdef(CONFIG_CLOCK_CONTROL_RPI_PICO            clock_control_rpi_pico.c)
+zephyr_library_sources_ifdef(CONFIG_CLOCK_CONTROL_NRF2_GLOBAL_HSFLL   clock_control_nrf2_global_hsfll.c)


Shouldn't this be below, after if(CONFIG_CLOCK_CONTROL_NRF2)? I see that Kconfig is under if CLOCK_CONTROL_NRF2

anangl · 2024-12-12T14:23:41Z

modules/hal_nordic/nrfs/Kconfig

@@ -117,6 +120,11 @@ config NRFS_GDPWR_SERVICE_ENABLED
 	depends on NRFS_HAS_GDPWR_SERVICE
 	default y

+config NRFS_GDFS_SERVICE_ENABLED
+	bool "Global domain frequeny scaling service"


typo: "frequeny"

anangl · 2024-12-12T14:33:01Z

dts/bindings/clock/nordic,nrf-local-hsfll.yaml

It would be better to have the name matching the actual compatible, so nordic,nrf-hsfll-local.yaml.

updated name of file to match compat

anangl · 2024-12-12T14:35:48Z

drivers/clock_control/clock_control_nrf2_hsfll.c

@@ -48,7 +47,7 @@ static const struct clock_options {
 };

 struct hsfll_dev_data {
-	STRUCT_CLOCK_CONFIG(hsfll, ARRAY_SIZE(clock_options)) clk_cfg;
+	STRUCT_CLOCK_CONFIG(global_hsfll, ARRAY_SIZE(clock_options)) clk_cfg;


Why? Isn't this local HSFLL?

rogue mistake, changed back

anangl · 2024-12-12T15:24:37Z

tests/drivers/clock_control/nrf_clock_control/src/main.c

+		.frequency = MHZ(320),
+		.accuracy = 0,
+		.precision = NRF_CLOCK_CONTROL_PRECISION_DEFAULT,


Why not just:

Suggested change

.frequency = MHZ(320),

.accuracy = 0,

.precision = NRF_CLOCK_CONTROL_PRECISION_DEFAULT,

.frequency = MHZ(320),

anangl · 2024-12-13T07:48:51Z

drivers/clock_control/clock_control_nrf2_hsfll.c

@@ -221,7 +208,6 @@ static DEVICE_API(nrf_clock_control, hsfll_drv_api) = {
 	.std_api = {
 		.on = api_nosys_on_off,
 		.off = api_nosys_on_off,
-		.get_rate = api_get_rate_hsfll,


What was wrong with this?

Its not useful when the frequency is dynamic, it can change at any time, the only way to guarantee any frequency is to request what you need, and wait for confirmation, so there is no value in checking the freq like this at any time. Its also not possible with the global hsfll (not this clock I know) but it proves the point :) there we can't even get the freq if we wanted :)

Agreed. It could be mentioned in the commit message so that the motivation is clear.

anangl · 2024-12-13T07:50:52Z

drivers/clock_control/clock_control_nrf2_global_hsfll.c

+	return -EINVAL;
+}
+
+static struct nrf_clock_control_driver_api driver_api = {


Suggested change

static struct nrf_clock_control_driver_api driver_api = {

static DEVICE_API(nrf_clock_control, driver_api) = {

this is not done for other nrf2 drivers, I think we should update them together

drivers/clock_control/clock_control_nrf2_global_hsfll.c

Adding the implementation for the GDFS service Signed-off-by: Paweł Pelikan <[email protected]>

Add specific device model for global hsfll clock and update dts tree to use specific model. The clock is not fixed, and configurable at runtime to predefined frequencies specified by the platform. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

The nrf-hsfll was previously the only supported HSFLL clock, hence it was not namespaced fully. Since we added nrf-hsfll-global, we should add the namespace to nrf-hsfll as well. Updates drivers and devicetree uses of HSFLL as well. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

Add device driver support for global hsfll clock. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

Extend sample to support global hsfll clock control. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

Extend test suite to test global HSFLL clock. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

The assert BUILD_ASSERT(NRF_GPD_FAST_ACTIVE1 == 0); is not correct given that NRF_GPD_FAST_ACTIVE1 is defined as 1U, and is not used in the file anyway. Remove the build assert. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

carlescufi

LGTM, thanks @bjarki-andreasen

masz-nordic · 2024-12-17T12:31:34Z

drivers/clock_control/CMakeLists.txt

@@ -37,6 +37,7 @@ zephyr_library_sources_ifdef(CONFIG_CLOCK_CONTROL_RENESAS_RA_CGC      clock_cont
 zephyr_library_sources_ifdef(CONFIG_CLOCK_CONTROL_AMBIQ               clock_control_ambiq.c)
 zephyr_library_sources_ifdef(CONFIG_CLOCK_CONTROL_PWM                 clock_control_pwm.c)
 zephyr_library_sources_ifdef(CONFIG_CLOCK_CONTROL_RPI_PICO            clock_control_rpi_pico.c)
+zephyr_library_sources_ifdef(CONFIG_CLOCK_CONTROL_NRF2_GLOBAL_HSFLL   clock_control_nrf2_global_hsfll.c)


masz-nordic · 2024-12-17T12:42:15Z

drivers/clock_control/clock_control_nrf2_global_hsfll.c

+{
+	const struct global_hsfll_dev_config *dev_config = dev->config;
+
+	return dev_config->clock_frequencies[ARRAY_SIZE(dev_config->clock_frequencies) - 1];


This could be returned build-time since there are build asserts checking for supported clock frequencies?

this driver does have some strangeness regarding single instances / multi instance code, I wanted to make it as easy as possible to extend it if we choose to support more HSFLLs in the future

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

bjarki-andreasen force-pushed the nordic-global-hsfll branch from cbea899 to 197adff Compare November 21, 2024 21:02

ppelikan-nordic reviewed Nov 26, 2024

View reviewed changes

west.yml Outdated Show resolved Hide resolved

bjarki-andreasen force-pushed the nordic-global-hsfll branch from 197adff to c71d0d0 Compare November 26, 2024 15:35

bjarki-andreasen marked this pull request as ready for review November 26, 2024 15:35

bjarki-andreasen requested review from anangl and nordic-krch as code owners November 26, 2024 15:35

zephyrbot added platform: nRF Nordic nRFx area: Samples Samples area: Clock Control labels Nov 26, 2024

zephyrbot requested review from hubertmis, jaz1-nordic, kartben, kl-cruz, krish2718, magp-nordic, masz-nordic, nashif, nika-nordic and sachinthegreen November 26, 2024 15:36

zephyrbot assigned nordic-krch Nov 26, 2024

bjarki-andreasen mentioned this pull request Nov 26, 2024

nrfs: add GDFS service zephyrproject-rtos/hal_nordic#263

Open

bjarki-andreasen force-pushed the nordic-global-hsfll branch from c71d0d0 to 49f2189 Compare November 26, 2024 22:46

nordic-krch mentioned this pull request Nov 27, 2024

drivers: serial: nrfx_uarte: Add requesting of HSFLL GD clock when fast UARTE (uart120) is used #82103

Merged

bjarki-andreasen force-pushed the nordic-global-hsfll branch from 49f2189 to 6b3e913 Compare November 27, 2024 21:01

bjarki-andreasen force-pushed the nordic-global-hsfll branch from 35cd442 to 5a17aca Compare December 12, 2024 14:30

masz-nordic reviewed Dec 12, 2024

View reviewed changes

bjarki-andreasen force-pushed the nordic-global-hsfll branch from 5a17aca to 5f6afbf Compare December 12, 2024 15:23

nordic-krch previously approved these changes Dec 13, 2024

View reviewed changes

anangl reviewed Dec 13, 2024

View reviewed changes

bjarki-andreasen dismissed nordic-krch’s stale review via 5a4bd73 December 13, 2024 08:56

bjarki-andreasen force-pushed the nordic-global-hsfll branch 2 times, most recently from 5a4bd73 to 30b9d0d Compare December 13, 2024 13:20

ppelikan-nordic and others added 3 commits December 13, 2024 14:38

modules: hal_nordic: nrfs: add GDFS Service

fc34b36

Adding the implementation for the GDFS service Signed-off-by: Paweł Pelikan <[email protected]>

bjarki-andreasen force-pushed the nordic-global-hsfll branch from 30b9d0d to 710d4db Compare December 13, 2024 13:38

bjarki-andreasen added 4 commits December 13, 2024 14:41

drivers: clock_control: nrf2: add support for global hfsll clock

7f61724

Add device driver support for global hsfll clock. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

samples: boards: nordic: clock_control: support global hsfll

413216c

Extend sample to support global hsfll clock control. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

tests: drivers: clock_control: nrf: add global hsfll clock

daff550

Extend test suite to test global HSFLL clock. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

bjarki-andreasen force-pushed the nordic-global-hsfll branch from 710d4db to 8e63354 Compare December 13, 2024 13:42

anangl mentioned this pull request Dec 14, 2024

drivers: spi_nrfx_spim: Add clock requests for fast SPIM instances #82994

Merged

bjarki-andreasen requested review from masz-nordic, anangl, karstenkoenig and nordic-krch December 17, 2024 12:26

carlescufi approved these changes Dec 17, 2024

View reviewed changes

anangl approved these changes Dec 17, 2024

View reviewed changes

masz-nordic reviewed Dec 17, 2024

View reviewed changes

gmarull approved these changes Dec 17, 2024

View reviewed changes

carlescufi assigned anangl and unassigned nordic-krch Dec 17, 2024

kartben merged commit 7487eab into zephyrproject-rtos:main Dec 17, 2024
26 checks passed

bjarki-andreasen deleted the nordic-global-hsfll branch December 17, 2024 14:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add global HSFLL clock control device driver #81735

Add global HSFLL clock control device driver #81735

bjarki-andreasen commented Nov 21, 2024

bjarki-andreasen commented Nov 21, 2024

zephyrbot commented Nov 21, 2024 •

edited

Loading

bjarki-andreasen commented Nov 21, 2024 •

edited

Loading

masz-nordic Nov 29, 2024 •

edited

Loading

masz-nordic Dec 17, 2024

anangl Dec 12, 2024

bjarki-andreasen Dec 13, 2024

anangl Dec 12, 2024

bjarki-andreasen Dec 13, 2024

anangl Dec 12, 2024

bjarki-andreasen Dec 13, 2024

anangl Dec 12, 2024

bjarki-andreasen Dec 13, 2024

anangl Dec 13, 2024

bjarki-andreasen Dec 13, 2024

anangl Dec 13, 2024

anangl Dec 13, 2024

bjarki-andreasen Dec 13, 2024

carlescufi left a comment

masz-nordic Dec 17, 2024

masz-nordic Dec 17, 2024

bjarki-andreasen Dec 17, 2024

	static struct nrf_clock_control_driver_api driver_api = {
	static DEVICE_API(nrf_clock_control, driver_api) = {

Add global HSFLL clock control device driver #81735

Add global HSFLL clock control device driver #81735

Conversation

bjarki-andreasen commented Nov 21, 2024

bjarki-andreasen commented Nov 21, 2024

zephyrbot commented Nov 21, 2024 • edited Loading

bjarki-andreasen commented Nov 21, 2024 • edited Loading

masz-nordic Nov 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlescufi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zephyrbot commented Nov 21, 2024 •

edited

Loading

bjarki-andreasen commented Nov 21, 2024 •

edited

Loading

masz-nordic Nov 29, 2024 •

edited

Loading