removal of cpufreq_set_policy()

Quoted 9 lines · Thomas Renninger

set_policy is buggy:
It calls verify_within_limits (where max_freq might get reduced) then sets
data->user_policy.max = data->max;
which is used to restore to highest freq with update_policy, but an already lowered freq might get assigned (e.g. if initialized on battery with limited freq, you will never ever reach higher freqs again).

Quoted 2 lines

I grepped the whole kernel and couldn't find cpufreq_set_policy to be used anywhere else.

Looks like cbe_cpufreq driver was merged at the same time as you were removing set_polcy :-)

Quoted 5 lines

The cell cpufreq driver also does not match?:
linux-2.6.21_tests/arch/powerpc/platforms/cell/cbe_cpufreq.c

Can you give me a pointer to the code, is this an external project?

It should be in that cbe_cpufreq.c file in current -git. I'm not the author of the driver though, I'm just reporting the build failure :-)

Arnd and Christian, can you check what's up ?

I think that driver uses set_policy to enforce new limits upon request from the system management.

I know I did something similar, though slightly different, for powermac, in drivers/macintosh/windfarm_cpufreq_clamp.c which is a module in my thermal control infrastructure that clamps down the cpufreq when an overtemp condition is detected.

Quoted 11 lines · Benjamin Herrenschmidt

Quoted 9 lines · Thomas Renninger

set_policy is buggy:
It calls verify_within_limits (where max_freq might get reduced) then sets
data->user_policy.max = data->max;
which is used to restore to highest freq with update_policy, but an already lowered freq might get assigned (e.g. if initialized on battery with limited freq, you will never ever reach higher freqs again).

Quoted 5 lines

Quoted 2 lines

I grepped the whole kernel and couldn't find cpufreq_set_policy to be used anywhere else.

Looks like cbe_cpufreq driver was merged at the same time as you were removing set_polcy :-)

Yes.

Quoted 11 lines

Quoted 5 lines

The cell cpufreq driver also does not match?:
linux-2.6.21_tests/arch/powerpc/platforms/cell/cbe_cpufreq.c

Can you give me a pointer to the code, is this an external project?

It should be in that cbe_cpufreq.c file in current -git. I'm not the author of the driver though, I'm just reporting the build failure :-)

Arnd and Christian, can you check what's up ?

Quoted 7 lines

I think that driver uses set_policy to enforce new limits upon request from the system management.

I know I did something similar, though slightly different, for powermac, in drivers/macintosh/windfarm_cpufreq_clamp.c which is a module in my thermal control infrastructure that clamps down the cpufreq when an overtemp condition is detected.

I can reproduce this with -rc2.
cpufreq_set_policy came in with this one:
2007-04-23 Christian Krafft [POWERPC] cell: use pmi in cpufreq driver

Attached patch is setting the limit like it's done for x86 in driver/acpi/processor_perflib.c
IMO the interface is more complicated as it needs to be (maybe I am wrong),
but it seems that this is the way it's intended to work.

Please review, ehh, I don't have HW to test and don't even know what pmi is,
can someone give this patch a test and try whether freq is limited and increased again correctly if "pmi" kicks in (compile tested...).

Thomas

Limit frequency for cell powerpc via cpufreq notifier chain

and get rid of cpufreq_set_policy call that caused a build failure due interfering commits

Signed-off-by Thomas Renninger <[email protected]>

---
 arch/powerpc/platforms/cell/cbe_cpufreq.c |   28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

Index: linux-2.6.22-rc2/arch/powerpc/platforms/cell/cbe_cpufreq.c
===================================================================

arch/powerpc/platforms/cell/cbe_cpufreq.c +18 -6

@@ -67,6 +67,7 @@ static u64 MIC_Slow_Next_Timer_table[] =
 	0x00003FC000000000ull,
 };
 
+static unsigned int pmi_frequency_limit = 0;
 /*
  * hardware specific functions
  */

@@ -164,7 +165,6 @@ static int set_pmode(int cpu, unsigned i
 
 static void cbe_cpufreq_handle_pmi(struct of_device *dev, pmi_message_t pmi_msg)
 {
-	struct cpufreq_policy policy;
 	u8 cpu;
 	u8 cbe_pmode_new;
 

@@ -173,15 +173,27 @@ static void cbe_cpufreq_handle_pmi(struc
 	cpu = cbe_node_to_cpu(pmi_msg.data1);
 	cbe_pmode_new = pmi_msg.data2;
 
-	cpufreq_get_policy(&policy, cpu);
+	pmi_frequency_limit = cbe_freqs[cbe_pmode_new].frequency;
 
-	policy.max = min(policy.max, cbe_freqs[cbe_pmode_new].frequency);
-	policy.min = min(policy.min, policy.max);
+	pr_debug("cbe_handle_pmi: max freq=%d\n", pmi_frequency_limit);
+}
+
+static int pmi_notifier(struct notifier_block *nb,
+				       unsigned long event, void *data)
+{
+	struct cpufreq_policy *policy = data;
+
+	if (event != CPUFREQ_INCOMPATIBLE)
+		return 0;
 
-	pr_debug("cbe_handle_pmi: new policy.min=%d policy.max=%d\n", policy.min, policy.max);
-	cpufreq_set_policy(&policy);
+	cpufreq_verify_within_limits(policy, 0, pmi_frequency_limit);
+	return 0;
 }
 
+static struct notifier_block pmi_notifier_block = {
+	.notifier_call = pmi_notifier,
+};
+
 static struct pmi_handler cbe_pmi_handler = {
 	.type			= PMI_TYPE_FREQ_CHANGE,
 	.handle_pmi_message	= cbe_cpufreq_handle_pmi,

arch/powerpc/platforms/cell/cbe_cpufreq.c +0 -0

 	/* this ensures that policy->cpuinfo_min and policy->cpuinfo_max are set correctly */
 	return cpufreq_frequency_table_cpuinfo(policy, cbe_freqs);
 }