From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@suse.com>
Subject: Re: [PATCH v6 4/6] x86/intel_pstate: add a booting
 param to select the driver to load
Date: Fri, 20 Nov 2015 07:22:36 -0700
Message-ID: <564F3ABC02000078000B72B2@prv-mh.provo.novell.com>
References: <1446002478-16105-1-git-send-email-wei.w.wang@intel.com>
	<1446002478-16105-5-git-send-email-wei.w.wang@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1446002478-16105-5-git-send-email-wei.w.wang@intel.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Wei Wang <wei.w.wang@intel.com>
Cc: andrew.cooper3@citrix.com, wei.liu2@citrix.com, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

>>> On 28.10.15 at 04:21, <wei.w.wang@intel.com> wrote:
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with two remarks:

> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -858,6 +858,13 @@ debug hypervisor only).
>  ### idle\_latency\_factor
>  > `= <integer>`
>  
> +### intel\_pstate
> +> `= <boolean>`
> +
> +> Default: `false`
> +
> +Enable the loading of the intel pstate driver.

"Loading" is probably not the right term, as it gets always loaded.
Perhaps just drop "the loading of"?

> --- a/xen/include/asm-x86/cpufreq.h
> +++ b/xen/include/asm-x86/cpufreq.h
> @@ -22,6 +22,8 @@
>   * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   */
>  
> +extern int intel_pstate_init(void);

Considering that I questioned the creation of this header in the
previous patch, the addition of this declaration should not result
in it now being created here. In fact if any new header was
needed for this declaration, it would probably better live in
xen/arch/x86/acpi/cpufreq/.

Moving this to a _suitable_ existing header does not void the ack
given.

Jan