cpufreq.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafał Bilski" <rafalbilski@interia.pl>
To: Ben Hutchings <ben@decadent.org.uk>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Viresh Kumar <viresh.kumar@linaro.org>
Cc: cpufreq@vger.kernel.org, Dave Jones <davej@redhat.com>
Subject: Re: [PATCH 2/2] e_powersaver: Require setting a module parameter to enable it
Date: Mon, 09 Jun 2014 19:15:22 +0100	[thread overview]
Message-ID: <5395F9BA.40703@interia.pl> (raw)
In-Reply-To: <1402269741.23860.50.camel@deadeye.wl.decadent.org.uk>


> The Kconfig text and comment at the top of the file say this is
> DANGEROUS.  According to Rafał Bilski it is not so bad as it used to
> be, but he still didn't think it should be auto-loaded.
>
> Rafał already made a similar change to longhaul.
>
> References: http://www.spinics.net/lists/cpufreq/msg02919.html
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
>   drivers/cpufreq/e_powersaver.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/drivers/cpufreq/e_powersaver.c b/drivers/cpufreq/e_powersaver.c
> index a0d2a42..880be39 100644
> --- a/drivers/cpufreq/e_powersaver.c
> +++ b/drivers/cpufreq/e_powersaver.c
> @@ -45,6 +45,7 @@ static struct eps_cpu_data *eps_cpu[NR_CPUS];
>   static int freq_failsafe_off;
>   static int voltage_failsafe_off;
>   static int set_max_voltage;
> +static int enable;
>   
>   #if defined CONFIG_ACPI_PROCESSOR || defined CONFIG_ACPI_PROCESSOR_MODULE
>   static int ignore_acpi_limit;
> @@ -410,6 +411,10 @@ static int __init eps_init(void)
>   {
>   	if (!x86_match_cpu(eps_cpu_id) || boot_cpu_data.x86_model < 10)
>   		return -ENODEV;
> +	if (!enable) {
> +		printk(KERN_ERR "eps: Option \"enable\" not set. Aborting.\n");
> +		return -ENODEV;
> +	}
>   	if (cpufreq_register_driver(&eps_driver))
>   		return -EINVAL;
>   	return 0;
> @@ -432,6 +437,10 @@ MODULE_PARM_DESC(ignore_acpi_limit, "Don't check ACPI's processor speed limit");
>   #endif
>   module_param(set_max_voltage, int, 0644);
>   MODULE_PARM_DESC(set_max_voltage, "Set maximum CPU voltage (mV) C7-M only");
> +/* By default driver is disabled to prevent incompatible
> + * system freeze. */
> +module_param(enable, int, 0644);
> +MODULE_PARM_DESC(enable, "Enable driver");
>   
>   MODULE_AUTHOR("Rafal Bilski <rafalbilski@interia.pl>");
>   MODULE_DESCRIPTION("Enhanced PowerSaver driver for VIA C7 CPU's.");
>
Actually that's quite good solution for conflict between ACPI P-states driver and this
driver. In this way ACPI will always be loaded first unless user will choose otherwise.
I would like to point out that this text and one in Kconfig is a bit incorrect. Processor
won't allow any frequency outside it's limits by design. Processor may get overheated
if system isn't designed for CPU to run at full frequency and BIOS sets limits by means
of P-states tables. Otherwise only difference is ACPI P-states driver seems to be using
SMI to change frequency while e_powersaver uses MSRs.

Rafał Bilski

      parent reply	other threads:[~2014-06-09 18:15 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-08 23:22 [PATCH 2/2] e_powersaver: Require setting a module parameter to enable it Ben Hutchings
2014-06-09  4:43 ` Viresh Kumar
2014-06-09 18:15 ` Rafał Bilski [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5395F9BA.40703@interia.pl \
    --to=rafalbilski@interia.pl \
    --cc=ben@decadent.org.uk \
    --cc=cpufreq@vger.kernel.org \
    --cc=davej@redhat.com \
    --cc=rjw@rjwysocki.net \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).