|SUCCESS| power/intel_uncore: reduce log level for dependency missing

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163923

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-12

Below is my review of the patch:

commit (from patch file)
Author: Huisong Li <lihuisong at huawei.com>
Subject: [PATCH] power/intel_uncore: reduce log level for dependency missing

The patch reduces the log level from ERR to DEBUG for messages indicating
that Intel uncore frequency management is not supported, to avoid noisy
output when running on non-x86 platforms with automatic detection mode.

> diff --git a/drivers/power/intel_uncore/intel_uncore.c b/drivers/power/intel_uncore/intel_uncore.c
> index 6759ea1445..ebca395075 100644
> --- a/drivers/power/intel_uncore/intel_uncore.c
> +++ b/drivers/power/intel_uncore/intel_uncore.c

[ ... ]

> @@ -419,7 +419,7 @@ power_intel_uncore_get_num_pkgs(void)
>  
>  	d = opendir(INTEL_UNCORE_FREQUENCY_DIR);
>  	if (d == NULL) {
> -		POWER_LOG(ERR,
> +		POWER_LOG(DEBUG,
>  		"Uncore frequency management not supported/enabled on this kernel. "
>  		"Please enable CONFIG_INTEL_UNCORE_FREQ_CONTROL if on Intel x86 with linux kernel"
>  		" >= 5.6");

Could this diagnostic message loss cause issues for users on Intel x86
platforms who genuinely have a misconfigured kernel?  When uncore frequency
management genuinely fails on Intel x86 due to missing kernel configuration,
wouldn't users benefit from an ERR level message rather than having it
hidden at DEBUG level?

The commit message mentions automatic detection mode triggering this on
non-x86 platforms.  Would it be more appropriate to check the platform or
architecture early in power_intel_uncore_init() and return a specific error
code without logging, rather than suppressing an error that may be
legitimate on Intel platforms?

> @@ -457,7 +457,7 @@ power_intel_uncore_get_num_dies(unsigned int pkg)
>  
>  	d = opendir(INTEL_UNCORE_FREQUENCY_DIR);
>  	if (d == NULL) {
> -		POWER_LOG(ERR,
> +		POWER_LOG(DEBUG,
>  		"Uncore frequency management not supported/enabled on this kernel. "
>  		"Please enable CONFIG_INTEL_UNCORE_FREQ_CONTROL if on Intel x86 with linux kernel"
>  		" >= 5.6");

Same concern applies here in power_intel_uncore_get_num_dies().  Does
downgrading this to DEBUG level risk hiding legitimate configuration
problems on Intel systems?

Also note that both POWER_LOG() calls have the string literals split across
lines without proper indentation continuation.  According to the coding
style guide section on line breaking, continuation lines should be indented
to align or use appropriate indentation.  The second line "Please enable
CONFIG_INTEL_UNCORE_FREQ_CONTROL..." should align with the opening quote or
be properly indented.  However, this is a pre-existing style issue, not
introduced by this patch.