Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

platform: intel: ace: remove all unnecessary clk.h definitions #9683

Merged

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Nov 26, 2024

Strip all unnecessary definitions from Intel ACE clk.h platform code. Only expose the main DSP clock via the SOF clk.h layer, as required by cpu-clk-manager.c.

/** \brief Total number of clocks */
#define NUM_CLOCKS (CLK_SSP + 1)
#define NUM_CLOCKS NUM_CPU_FREQ
Copy link
Collaborator

@lyakh lyakh Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a bit confusing. It's used in

static SHARED_DATA struct clock_info platform_clocks_info[NUM_CLOCKS];
as a size of array, but then lines

sof/zephyr/lib/clk.c

Lines 29 to 32 in 5389b25

sof->clocks = platform_clocks_info;
for (i = 0; i < CONFIG_CORE_COUNT; i++) {
sof->clocks[i] = (struct clock_info) {
iterate over that array, but using CONFIG_CORE_COUNT as the iteration number? So, e.g. on MTL NUM_CPU_FREQ is set to 2, but it has 3 cores... Same with LNL?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lyakh , this was a good catch. This is mixing number of clocks and number of frequencies (each CPU clock has). Fixed version uploaded now.

Strip all unnecessary definitions from Intel ACE clk.h
platform code. Only expose the main DSP clock via the SOF clk.h
layer, as required by cpu-clk-manager.c.

Signed-off-by: Kai Vehmanen <[email protected]>
@kv2019i kv2019i force-pushed the 202411-clk-prune-ace-plat-code branch from 5389b25 to d721f76 Compare November 27, 2024 15:01
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one question.

Add a build assert to ensure platform NUM_CLOCKS is set to at least
CONFIG_CORE_COUNT. This is assumed to hold in the implementation of
platform_clock_init().

Signed-off-by: Kai Vehmanen <[email protected]>
@kv2019i kv2019i force-pushed the 202411-clk-prune-ace-plat-code branch from d721f76 to 92a3c29 Compare November 28, 2024 15:46
@lgirdwood
Copy link
Member

SOFCI TEST

@lgirdwood lgirdwood merged commit 4fde218 into thesofproject:main Dec 5, 2024
45 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants