Re: [Xen-devel] [RFC PATCH v5 06/12] cpufreq: make cpufreq driver more generalizable

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.11.14 at 14:17, <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx> wrote:
> @@ -207,6 +207,18 @@ int cpufreq_add_cpu(unsigned int cpu)
>                  );
>              return -EINVAL;
>          }
> +#else /* CONFIG_ACPI */
> +        if ((perf->domain_info.num_processors !=
> +            processor_pminfo[firstcpu]->perf.domain_info.num_processors)) {
> +
> +            printk(KERN_WARNING "cpufreq fail to add CPU%d:"
> +                   "incorrect num processors (%"PRIu64"), 
> expect(%"PRIu64")\n",
> +                   cpu, perf->domain_info.num_processors,
> +                   
> processor_pminfo[firstcpu]->perf.domain_info.num_processors
> +                );

Please don't copy formatting breakage.

> @@ -401,13 +427,53 @@ static void print_PPC(unsigned int platform_limit)
>      printk("\t_PPC: %d\n", platform_limit);
>  }
>  
> +static inline uint32_t is_pss_data(struct xen_processor_performance *px)
> +{
> +#ifdef CONFIG_ACPI
> +    return px->flags & XEN_PX_PSS;
> +#else
> +    return px->flags == XEN_PX_DATA;

Why's that == instead of &. Also you appear to mean this function
to return a boolean value, in which case its return type ought to be
bool_t.

> +static inline uint32_t is_all_data(struct xen_processor_performance *px)
> +{
> +#ifdef CONFIG_ACPI
> +    return px->flags == ( XEN_PX_PCT | XEN_PX_PSS | XEN_PX_PSD | XEN_PX_PPC 
> );

No spaces immediately inside the parentheses please.

>  int set_px_pminfo(uint32_t acpi_id, struct xen_processor_performance 
> *dom0_px_info)
>  {
>      int ret=0, cpuid;
>      struct processor_pminfo *pmpt;
>      struct processor_performance *pxpt;
>  
> +#ifdef CONFIG_ACPI
>      cpuid = get_cpu_id(acpi_id);
> +#else
> +    cpuid = acpi_id;

This just can't be right when !CONFIG_ACPI, but I think I commented
on the naming of such variables/fields already for an earlier version.

> --- a/xen/include/public/platform.h
> +++ b/xen/include/public/platform.h
> @@ -358,6 +358,7 @@ DEFINE_XEN_GUEST_HANDLE(xenpf_getidletime_t);
>  #define XEN_PX_PSS   2
>  #define XEN_PX_PPC   4
>  #define XEN_PX_PSD   8
> +#define XEN_PX_DATA  16

This is too generic a name to go without comment.

> --- a/xen/include/xen/processor_perf.h
> +++ b/xen/include/xen/processor_perf.h
> @@ -3,7 +3,10 @@
>  
>  #include <public/platform.h>
>  #include <public/sysctl.h>
> +
> +#ifdef CONFIG_ACPI
>  #include <xen/acpi.h>
> +#endif

This might better be done inside xen/acpi.h.

Jan


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