Re: [Xen-devel] [PATCH v4 11/11] tools: enable xenpm to control the intel_pstate driver

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Jul 07, 2015 at 01:05:21PM +0100, Jan Beulich wrote:
> >>> On 07.07.15 at 10:55, <wei.liu2@xxxxxxxxxx> wrote:
> > On Thu, Jun 25, 2015 at 07:19:05PM +0800, Wei Wang wrote:
> >> --- a/tools/libxc/include/xenctrl.h
> >> +++ b/tools/libxc/include/xenctrl.h
> >> @@ -2266,8 +2266,18 @@ struct xc_get_cpufreq_para {
> >>      uint32_t scaling_cur_freq;
> >>  
> >>      char scaling_governor[CPUFREQ_NAME_LEN];
> >> -    uint32_t scaling_max_freq;
> >> -    uint32_t scaling_min_freq;
> >> +
> >> +    union {
> >> +        uint32_t freq;
> >> +        uint32_t pct;
> >> +    } scaling_max;
> >> +
> >> +    union {
> >> +        uint32_t freq;
> >> +        uint32_t  pct;
> >> +    } scaling_min;
> >> +
> > 
> > Don't you need struct? I don't see how union would work for you, you
> > clearly need bot freq and pct at the same time.
> 
> Why? The current driver uses freq; intel_pstate uses pct. What looks
> wrong is the code below using both fields at once.
> 

I only looked at this single patch.

I got that impression from here:

+        user_para->scaling_max.freq             = sys_para->scaling_max.freq;
+        user_para->scaling_min.freq             = sys_para->scaling_min.freq;
+        user_para->scaling_max.pct              = sys_para->scaling_max.pct;
+        user_para->scaling_min.pct              = sys_para->scaling_min.pct;

So using union is OK, just the code is confusing.

Wei.

> Jan

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