zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.83k stars 6.6k forks source link

nRF5340 - frequency set API #21826

Closed koffes closed 2 years ago

koffes commented 4 years ago

There should be an API to set the APP core freq of the nRF5340.

Currently, the method of changing the core freq is direct register writes.

This API should:

@ioannisg

mmajchrzycki commented 2 years ago

You can do it by using the following function (from nRF Connect SDK):

#include <nrfx_clock.h>

nrfx_clock_divider_set(NRF_CLOCK_DOMAIN_HFCLK, NRF_CLOCK_HFCLK_DIV_1);
koffes commented 2 years ago

Thanks, we are already using that one. Though, this may cause timer issues as Zephyr is not aware that the CPU has changed its base frequency. This may impact logging, scheduling, etc. This should be a Zephyr level API.

anangl commented 2 years ago

nrfx_clock_divider_set() calls SystemCoreClockUpdate(), so Zephyr is actually aware of the change. And I am not sure what "timer issues" you mean. The TIMER peripherals are driven by a peripheral clock that is not affected by the change of the core clock divider and the kernel timers are driven by the LFCLK clock that Zephyr uses as its system clock, which is not affected as well (so I don't see any impact on logging or scheduling). Only the SysTick timer is affected by the core clock frequency change.

koffes commented 2 years ago

I see now that just 14 days after I opened this issue, you added the SystemCoreClockUpdate() @anangl :)

So, I take it as Zephyr is aware of the change, or rather does not need to be aware of the change, we can alter between 64 and 128 MHz whenever we want? The "timer issues" was a concern from my side, as we originally did a call directly to nrfx without Zephyr knowing that any change had been made. This could potentially lead to a number of issues, depending on how the system is constructed.

nordic-krch commented 2 years ago

@anangl i think that k_busy_wait will be affected.

anangl commented 2 years ago

@anangl i think that k_busy_wait will be affected.

Yes, but it calls nrfx_coredep_delay_us() that takes SystemCoreClock into account, so the delay time will be almost the same (only the overhead of calling the function will be a bit lower for the core working with the higher clock).

nordic-krch commented 2 years ago

@anangl right, i missed that it takes SystemCoreClock for delay calculation.

koffes commented 2 years ago

FYI: @alexsven.

I will close this issue as no one seems to see any issue with this.