|SUCCESS| power: fix duplicated typedef for setting uncore freq
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Thu May 7 14:20:27 CEST 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163784
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-07
Commit: (unable to determine git sha from patch)
Author: Huisong Li <lihuisong at huawei.com>
Subject: [PATCH] power: fix duplicated typedef for setting uncore freq
This patch removes a duplicated typedef declaration and updates the
associated documentation comment to more accurately describe the function
pointer's purpose.
> diff --git a/lib/power/power_uncore_ops.h b/lib/power/power_uncore_ops.h
> index 783860ee5b..025a55cb97 100644
> --- a/lib/power/power_uncore_ops.h
> +++ b/lib/power/power_uncore_ops.h
> @@ -76,8 +76,7 @@ typedef int (*rte_power_uncore_exit_t)(unsigned int pkg, unsigned int die);
> typedef uint32_t (*rte_power_get_uncore_freq_t)(unsigned int pkg, unsigned int die);
>
> /**
> - * Set minimum and maximum uncore frequency for specified die on a package
> - * to specified index value.
> + * Set uncore frequency for specified die on a package to specified index value.
Does this documentation change accurately reflect the function's behavior?
The original comment suggested the function sets both minimum and maximum
frequencies, but the new comment implies it sets only a single frequency.
Should the documentation clarify which frequency is being set (current,
minimum, or maximum)?
> * It should be protected outside of this function for threadsafe.
> *
> * This function should NOT be called in the fast path.
> @@ -98,27 +97,6 @@ typedef uint32_t (*rte_power_get_uncore_freq_t)(unsigned int pkg, unsigned int d
> */
> typedef int (*rte_power_set_uncore_freq_t)(unsigned int pkg, unsigned int die, uint32_t index);
>
> -/**
> - * Return the list length of available frequencies in the index array.
The removed comment describes "Return the list length of available
frequencies" but this doesn't match the function name
rte_power_set_uncore_freq_t. Was this documentation copied from a different
function? The duplication suggests the second typedef declaration may have
been added with incorrect documentation.
> - *
> - * This function should NOT be called in the fast path.
> - *
> - * @param pkg
> - * Package number.
> - * Each physical CPU in a system is referred to as a package.
> - * @param die
> - * Die number.
> - * Each package can have several dies connected together via the uncore mesh.
> - * @param index
> - * The index of available frequencies.
> - *
> - * @return
> - * - 1 on success with frequency changed.
> - * - 0 on success without frequency changed.
> - * - Negative on error.
> - */
> -typedef int (*rte_power_set_uncore_freq_t)(unsigned int pkg, unsigned int die, uint32_t index);
> -
> /**
> * Return the list length of available frequencies in the index array.
> *
More information about the test-report
mailing list