Re: [PATCH] cpufreq: acpi: Add comment under ACPI_ADR_SPACE_SYSTEM_IO case

[Viresh Kumar]

On 12/10/2013, Rafael J. Wysocki <rjw@xxxxxxx> wrote:
> Well, please merge it with the existing comment and use the usual format
> for comments that are longer than two lines.

I thought these are separate comments and so keeping them separate
might be better, so that this one doesn't get deleted in case somebody
is removing the other one.. and vice versa..

Anyway its fixed in attached commit now :)

commit ecb9ef81b50eb5e8559f7d132ef46803c8272091
Author: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
Date:   Sat Oct 12 07:00:01 2013 +0530

    cpufreq: acpi: Add comment under ACPI_ADR_SPACE_SYSTEM_IO case

    policy->cur is now set by cpufreq core when cpufreq_driver->get()
is defined and
    so drivers aren't required to set it. When space_id is
ACPI_ADR_SPACE_SYSTEM_IO
    for acpi cpufreq driver it doesn't set ->get to a valid function
pointer and so
    policy->cur is required to be set by driver.

    This is already followed in acpi-cpufreq driver. This patch adds a comment
    describing why we need to set policy->cur from driver.

    Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
    Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
---
 drivers/cpufreq/acpi-cpufreq.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index a8dac7b..d2df543 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -837,7 +837,12 @@ static int acpi_cpufreq_cpu_init(struct
cpufreq_policy *policy)

        switch (perf->control_register.space_id) {
        case ACPI_ADR_SPACE_SYSTEM_IO:
-               /* Current speed is unknown and not detectable by IO port */
+               /*
+                * Current speed is unknown and not detectable by IO port.
+                * policy->cur wouldn't be set by core as cpufreq_driver->get()
+                * is NULL for this space_id and so we need to set policy->cur
+                * here.
+                */
                policy->cur = acpi_cpufreq_guess_freq(data, policy->cpu);
                break;
        case ACPI_ADR_SPACE_FIXED_HARDWARE:
From ecb9ef81b50eb5e8559f7d132ef46803c8272091 Mon Sep 17 00:00:00 2001
Message-Id: <ecb9ef81b50eb5e8559f7d132ef46803c8272091.1381591429.git.viresh.kumar@xxxxxxxxxx>
From: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
Date: Sat, 12 Oct 2013 07:00:01 +0530
Subject: [PATCH] cpufreq: acpi: Add comment under ACPI_ADR_SPACE_SYSTEM_IO
 case

policy->cur is now set by cpufreq core when cpufreq_driver->get() is defined and
so drivers aren't required to set it. When space_id is ACPI_ADR_SPACE_SYSTEM_IO
for acpi cpufreq driver it doesn't set ->get to a valid function pointer and so
policy->cur is required to be set by driver.

This is already followed in acpi-cpufreq driver. This patch adds a comment
describing why we need to set policy->cur from driver.

Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
---
 drivers/cpufreq/acpi-cpufreq.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index a8dac7b..d2df543 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -837,7 +837,12 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
 
 	switch (perf->control_register.space_id) {
 	case ACPI_ADR_SPACE_SYSTEM_IO:
-		/* Current speed is unknown and not detectable by IO port */
+		/*
+		 * Current speed is unknown and not detectable by IO port.
+		 * policy->cur wouldn't be set by core as cpufreq_driver->get()
+		 * is NULL for this space_id and so we need to set policy->cur
+		 * here.
+		 */
 		policy->cur = acpi_cpufreq_guess_freq(data, policy->cpu);
 		break;
 	case ACPI_ADR_SPACE_FIXED_HARDWARE:
-- 
1.7.12.rc2.18.g61b472e