soc: intel_adsp/ace: use functions to do CPU power control #59799
Conversation
zephyrbot
requested review from
aborisovich,
ceolin,
jxstelter,
kv2019i,
lgirdwood,
lyakh,
marc-hb,
marcinszkudlinski,
MaureenHelm,
softwarecki and
tmleman
| ACE_PWRCTL->wpdsphpxpg &= ~BIT(cpu_num); | ||
| } | ||
|
|
||
| static ALWAYS_INLINE bool soc_cpu_is_powered(int cpu_num) |
There was a problem hiding this comment.
Aren't these names a bit too generic? I mean no ace_ prefix/namespace or anything.
There was a problem hiding this comment.
I could change these if you insist. I made these generic so we don't need to rename stuff if we ever need to share code with another audio DSP architecture in the future.
There was a problem hiding this comment.
I could change these if you insist. I made these generic so we don't need to rename stuff if we ever need to share code with another audio DSP architecture in the future.
Thanks, I was wondering if this was by design. If this is generic by design then great but then leave a short comment that it's meant to be. There is zero comment right now.
There was a problem hiding this comment.
Added the function description (doxygen). Hopefully this would be enough.
Unless we have a porting guide, it's hard to convey the idea of "generic function name to ease future platform addition" in the comment without it sounding out of place.
There was a problem hiding this comment.
Thanks, the doxygen looks useful but that's not what I had in mind.
it's hard to convey the idea of "generic function name to ease future platform addition" in the comment
I think you just did, brilliantly :-)
without it sounding out of place.
I don't see why it would sound out of place.
It's not important, nevermind.
There was a problem hiding this comment.
I opt for including this in the commit message.
dcpleung
force-pushed
the
intel_adsp/soc_cpu_power_funcs
branch
from
22ca3e5
to
8a3a2b7
Compare
Instead of relying on direct memory access via structs to control CPU power and status, using inline functions instead to hide the details. This makes reading the common code a bit cleaner. The function names are generic and not architecture or platform specific, in an attempt to ease future arch or platform additions with code reuse. Or else we would need to rename these. Signed-off-by: Daniel Leung <[email protected]>
dcpleung
force-pushed
the
intel_adsp/soc_cpu_power_funcs
branch
from
8a3a2b7
to
aab4f7d
Compare
Instead of relying on direct memory access via structs to control CPU power and status, using inline functions instead to hide the details. This makes reading the common code a bit cleaner.
The function names are generic and not architecture or platform specific, in an attempt to ease future arch or platform additions with code reuse. Or else we would need to rename these.