Re: [Xen-devel] [PATCH v4 03/11] x86/intel_pstate: add new policy fields and a new driver interface

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.06.15 at 13:15, <wei.w.wang@xxxxxxxxx> wrote:
> --- a/xen/include/acpi/cpufreq/cpufreq.h
> +++ b/xen/include/acpi/cpufreq/cpufreq.h
> @@ -41,6 +41,24 @@ struct cpufreq_cpuinfo {
>      unsigned int        transition_latency; /* in 10^(-9) s = nanoseconds */
>  };
>  
> +struct perf_limits {
> +    bool_t no_turbo;
> +    bool_t turbo_disabled;

Considering that struct cpufreq_policy already has a turbo field,
and without seeing how the fields get initialized or used, I can't
really judge whether they're rightfully being added here. If you
want to keep them, please defer their addition to this structure
until the patch actually using them. (I wonder whether that
wouldn't be better for most/all of the other fields too.)

> +struct internal_governor {
> +    char *avail_gov;
> +    uint32_t gov_num;
> +    uint32_t cur_gov;

Same here - how should a reviewer of this patch judge whether
these fields make sens when they aren't being used anywhere?
I can somehow guess the meaning of the last field...

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel