drivers: spi_nrfx_spim: Add clock requests for fast SPIM instances #82994
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zephyrbot
added
area: UART
zephyrbot
requested review from
bjarki-andreasen,
ceolin,
dcpleung,
hubertmis,
jaz1-nordic,
JordanYates,
kl-cruz,
magp-nordic,
masz-nordic,
nashif,
nika-nordic,
nordic-krch,
tbursztyka,
teburd and
tmleman
|
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
nika-nordic
previously approved these changes
Dec 16, 2024
| }; | ||
|
|
||
| static void event_handler(const nrfx_spim_evt_t *p_event, void *p_context); | ||
|
|
||
| static inline int request_clock(const struct device *dev) |
There was a problem hiding this comment.
won't the compiler complain about unused dev parameter when USE_CLOCK_REQUESTS is undefined? same for release_clock
There was a problem hiding this comment.
Yes, I think it may complain, depending on the enabled warnings. I'll add ARG_UNUSED() macro calls.
anangl
force-pushed
the
req_hsfll_spim
branch
2 times, most recently
from
ecf7509 to
7b94b4d
Compare
bjarki-andreasen
previously approved these changes
Dec 17, 2024
This is a follow-up to commit fe0e2db. If `nrf_clock_control_request_sync()` ends up with a timeout, before returning it must cancel the request that was not fulfilled on time. Otherwise, the request may actually finish successfully a bit later, but the caller will not be aware that the clock needs to be released (since the call resulted in an error). And actually even more serious problem is that because the `req` structure is placed on stack, after the function returns, the contents of this structure will be probably overwritten with some other data, so if the request finishes at that point, an attempt to execute the callback function pointed by this structure will most likely cause a crash. Signed-off-by: Andrzej Głąbek <[email protected]>
Fast SPIM instances in nRF54H20 (SPIM120 and SPIM121) are driven by the global HSFLL (HSFLL120). Add `clocks` property in these nodes to reflect this. Signed-off-by: Andrzej Głąbek <[email protected]>
anangl
force-pushed
the
req_hsfll_spim
branch
from
7b94b4d to
ec91a73
Compare
|
Rebased. |
masz-nordic
reviewed
Dec 18, 2024
bjarki-andreasen
previously approved these changes
Dec 18, 2024
Fast SPIM instances (SPIM120 and SPIM121) for correct operation require the highest frequency from the global HSFLL. This commit adds needed clock controller requests to the driver. When the runtime device power management is enabled, the frequency is requested as long as the SPIM is resumed, otherwise it is requested for the duration of transfers. This commit also adds a missing call to `pm_device_runtime_put()` when SPIM reconfiguration fails. Signed-off-by: Andrzej Głąbek <[email protected]>
anangl
force-pushed
the
req_hsfll_spim
branch
from
ec91a73 to
4e01a59
Compare
bjarki-andreasen
approved these changes
Dec 19, 2024
masz-nordic
approved these changes
Dec 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #75715 and #81735. Only the last three commits are actually added by this PR.drivers: clock_control_nrf2: Add missing cancelation of request
This is a follow-up to commit fe0e2db.
If
nrf_clock_control_request_sync()ends up with a timeout, before returning it must cancel the request that was not fulfilled on time. Otherwise, the request may actually finish successfully a bit later, but the caller will not be aware that the clock needs to be released (since the call resulted in an error). And actually even more serious problem is that because thereqstructure is placed on stack, after the function returns, the contents of this structure will be probably overwritten with some other data, so if the request finishes at that point, an attempt to execute the callback function pointed by this structure will most likely cause a crash.dts: nrf54h20: Add clocks property in fast SPIM nodes
Fast SPIM instances in nRF54H20 (SPIM120 and SPIM121) are driven by the global HSFLL (HSFLL120). Add
clocksproperty in these nodes to reflect this.drivers: spi_nrfx_spim: Add clock requests for fast SPIM instances
Fast SPIM instances (SPIM120 and SPIM121) for correct operation require the highest frequency from the global HSFLL. This commit adds needed clock controller requests to the driver. When the runtime device power management is enabled, the frequency is requested as long as the SPIM is resumed, otherwise it is requested for the duration of transfers.