applications: nrf_desktop: Use clock api for DVFS #17562
Conversation
github-actions
bot
added
the
changelog-entry-required
zycz
requested review from
kapi-no,
MarekPieta,
alstrzebonski and
mkapala-nordic
zycz
force-pushed
the
issue29351_clock_api
branch
from
f3f4914
to
36c0bb6
Compare
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: dbf7537c8cd1198a654ec40bf7e474c07b2e447c more detailssdk-nrf:
Github labels
List of changed files detected by CI (3)Outputs:ToolchainVersion: 3dd8985b56 Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
|
You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds. Note: This comment is automatically posted by the Documentation Publishing GitHub Action. |
zycz
force-pushed
the
issue29351_clock_api
branch
from
36c0bb6
to
9937560
Compare
zycz
removed
the
changelog-entry-required
applications/nrf_desktop/configuration/nrf54h20dk_nrf54h20_cpuapp/app.overlay
Outdated
Show resolved
Hide resolved
zycz
force-pushed
the
issue29351_clock_api
branch
from
9937560
to
3f4c90d
Compare
github-actions
bot
added
the
changelog-entry-required
zycz
force-pushed
the
issue29351_clock_api
branch
from
3f4c90d
to
df0b116
Compare
zycz
force-pushed
the
issue29351_clock_api
branch
2 times, most recently
from
4f1fcd7
to
56d247f
Compare
zycz
removed
the
changelog-entry-required
zycz
force-pushed
the
issue29351_clock_api
branch
from
56d247f
to
c443b29
Compare
|
rebased |
zycz
force-pushed
the
issue29351_clock_api
branch
from
8463c22
to
148b1e7
Compare
zycz
force-pushed
the
issue29351_clock_api
branch
from
148b1e7
to
cbcb264
Compare
| @@ -59,6 +59,10 @@ | |||
| in-polling-period-us = <125>; | |||
| in-report-size = <64>; | |||
| }; | |||
|
|
|||
| aliases { | |||
| nrfdesktop-dvfs-clock = &cpuapp_hsfll; | |||
There was a problem hiding this comment.
I wonder if we could refer to cpuapp's clock here to avoid introducing the DT alias.
zephyr/dts/common/nordic/nrf54h20.dtsi defines cpuapp's clock nicely:
cpuapp: cpu@2 {
compatible = "arm,cortex-m33";
reg = <2>;
device_type = "cpu";
clocks = <&cpuapp_hsfll>;
clock-frequency = <DT_FREQ_M(320)>;
cpu-power-states = <&idle_cache_disabled &s2ram>;
};
There was a problem hiding this comment.
Furthermore, zephyr/dts/arm/nordic/nrf54h20_cpuapp.dtsi assigns cpu label to the cpuapp.
There was a problem hiding this comment.
The change would allow us to explicitly show that we want to control frequency of the cpuapp here. It could also simplify maintenance and configuration (we will remove risk that user enables the module without configuring the used DT alias too).
There was a problem hiding this comment.
Then we would be able to call the module e.g. cpu_freq_control later on (FYI: @kapi-no - you asked for more generic name)
There was a problem hiding this comment.
Changes may be applied under follow-up tickets, but if we plan to change the name or configuration scheme after the NCS 2.8, we should mark the module as experimental for NCS 2.8 (breaking APIs)
There was a problem hiding this comment.
zycz
force-pushed
the
issue29351_clock_api
branch
from
cbcb264
to
85cd07e
Compare
zycz
force-pushed
the
issue29351_clock_api
branch
from
85cd07e
to
542b772
Compare
| } | ||
| if (requested_freq != current_freq) { | ||
| /* | ||
| * In current solution it is assumed tht no other module |
There was a problem hiding this comment.
| * In current solution it is assumed tht no other module | |
| * In current solution it is assumed that no other module |
zycz
force-pushed
the
issue29351_clock_api
branch
from
542b772
to
2552d42
Compare
zycz
force-pushed
the
issue29351_clock_api
branch
from
2552d42
to
401ccf3
Compare
zycz
removed
the
changelog-entry-required
Use clock api in nrf_desktop dvfs module. Use notification handler to notify correct completion of DVFS. JIRA: NCSDK-29351 Signed-off-by: Jan Zyczkowski <jan.zyczkowski@nordicsemi.no>
zycz
force-pushed
the
issue29351_clock_api
branch
from
401ccf3
to
dbf7537
Compare
github-actions
bot
added
the
changelog-entry-required
| config DESKTOP_DVFS_FREQ_HIGH | ||
| int | ||
| default 320000000 | ||
|
|
||
| config DESKTOP_DVFS_FREQ_MED | ||
| int | ||
| default 128000000 | ||
|
|
||
| config DESKTOP_DVFS_FREQ_LOW | ||
| int | ||
| default 64000000 |
There was a problem hiding this comment.
@gmarull these should come from dts?
There was a problem hiding this comment.
But we don't have those frequencies in dts
There was a problem hiding this comment.
The range of valid frequencies should definitely be in DT, this is a HW capability thing. The software choices are fine in Kconfig. However, adjusting frequencies directly in applications doesn't seem like a great design. This should be governed by some framework.
There was a problem hiding this comment.
Ok, but valid frequencies should be added globally in dts at board level. Till it is not added shouldn't we base on frequencies from Kconfig?
However, adjusting frequencies directly in applications doesn't seem like a great design.
Object of this task is to move responsibility of changing frequencies to clock_api instead directly from application (module has been written prior to clock driver)
Use clock api in nrf_desktop dvfs module.
Use notification handler to notify correct
completion of DVFS.
JIRA: NCSDK-29351