06/10/2022 11:38, Tadhg Kearney:
> +API Overview for Intel Uncore
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Overview of each function in the Intel Uncore API, with explanation of what
> +they do. Each function should not be called in the fast path.
> +
> +* **Uncore Power Init**
> + Initialize uncore power, populate frequency array and record
> + original min & max for die on pkg.
> +
> +* **Uncore Power Exit**
> + Exit uncore power, restoring original min & max for die on pkg.
> +
> +* **Get Uncore Power Freq**
> + Get current uncore freq index for die on pkg.
> +
> +* **Set Uncore Power Freq**
> + Set min & max uncore freq index for die on pkg to specified index value
> + (min and max will be the same).
> +
> +* **Uncore Power Max**
> + Set min & max uncore freq to maximum frequency index for die on pkg
> + (min and max will be the same).
> +
> +* **Uncore Power Min**
> + Set min & max uncore freq to minimum frequency index for die on pkg
> + (min and max will be the same).
> +
> +* **Get Num Freqs**
> + Get the number of frequencies in the index array.
> +
> +* **Get Num Pkgs**
> + Get the number of packages (CPU's) on the system.
> +
> +* **Get Num Dies**
> + Get the number of die's on a given package.
This is the role of doxygen documentation to explain API.
I don't understand why it is there.
Anyway I've converted it into a definition list,
which the proper RST syntax for what you do.
> 'rte_power.c',
> 'rte_power_empty_poll.c',
> 'rte_power_pmd_mgmt.c',
> + 'rte_power_intel_uncore.c',
In general, we keep such list in alphabetical order.
[...]
> +struct uncore_power_info {
> + unsigned int die; /** Core die id */
> + unsigned int pkg; /** Package id */
> + uint32_t freqs[MAX_UNCORE_FREQS];/** Frequency array */
> + uint32_t nb_freqs; /** Number of available freqs */
> + FILE *f_cur_min; /** FD of scaling_min */
> + FILE *f_cur_max; /** FD of scaling_max */
> + uint32_t curr_idx; /** Freq index in freqs array */
> + uint32_t org_min_freq; /** Original min freq of uncore */
> + uint32_t org_max_freq; /** Original max freq of uncore */
> + uint32_t init_max_freq; /** System max uncore freq */
> + uint32_t init_min_freq; /** System min uncore freq */
> +} __rte_cache_aligned;
No need of doxygen syntax in a .c file.
And an alignment is wrong.
[...]
> + RTE_LOG(DEBUG, POWER, "Invalid uncore frequency index %u, which
> "
> + "should be less than %u\n", idx, ui->nb_freqs);
When you have time, it would be good to switch to dynamic logging.
See RTE_LOG_REGISTER_DEFAULT
[...]
> +#ifndef _RTE_POWER_INTEL_UNCORE_H
> +#define _RTE_POWER_INTEL_UNCORE_H
underscore prefix is reserved for reserved keywords
[...]
> +/**
> + * Exit uncore frequency management on a specific die on a package. It will
> restore uncore min and
Screen width is limited, while scrolling bar is infinite,
so don't hesitate to break your lines shorter when possible,
like at the end of a sentence.
