All of lore.kernel.org
 help / color / mirror / Atom feed
* cpufreq suspend patch.
@ 2005-08-08 22:44 Dave Jones
  2005-08-09  8:32 ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Jones @ 2005-08-08 22:44 UTC (permalink / raw)
  To: Ben Herrenschmidt; +Cc: cpufreq, dwmw2

Ben,
 Dave Woodhouse added this to the Fedora kernel a while back
with the helpful comment "Fix ppc32 sleep/cpufreq harder"
I believe this came from you originally.

Can you comment on this ? 

It seems to make sense to me,  but if you have a newer patch
I'm all ears..

		Dave



Index: linux-work/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-work.orig/drivers/cpufreq/cpufreq.c	2005-03-30 09:42:18.000000000 +1000
+++ linux-work/drivers/cpufreq/cpufreq.c	2005-03-30 09:47:22.000000000 +1000
@@ -223,7 +223,7 @@
 	}
 	if ((val == CPUFREQ_PRECHANGE  && ci->old < ci->new) ||
 	    (val == CPUFREQ_POSTCHANGE && ci->old > ci->new) ||
-	    (val == CPUFREQ_RESUMECHANGE)) {
+	    (val == CPUFREQ_RESUMECHANGE || val == CPUFREQ_SUSPENDCHANGE)) {
 		loops_per_jiffy = cpufreq_scale(l_p_j_ref, l_p_j_ref_freq, ci->new);
 		dprintk("scaling loops_per_jiffy to %lu for frequency %u kHz\n", loops_per_jiffy, ci->new);
 	}
@@ -866,16 +866,96 @@
 
 
 /**
+ *	cpufreq_suspend - let the low level driver prepare for suspend
+ */
+
+static int cpufreq_suspend(struct sys_device * sysdev, u32 state)
+{
+	int cpu = sysdev->id;
+	unsigned int ret = 0;
+	unsigned int cur_freq = 0;
+	struct cpufreq_policy *cpu_policy;
+
+	dprintk("resuming cpu %u\n", cpu);
+
+	if (!cpu_online(cpu))
+		return 0;
+
+	/* we may be lax here as interrupts are off. Nonetheless
+	 * we need to grab the correct cpu policy, as to check
+	 * whether we really run on this CPU.
+	 */
+
+	cpu_policy = cpufreq_cpu_get(cpu);
+	if (!cpu_policy)
+		return -EINVAL;
+
+	/* only handle each CPU group once */
+	if (unlikely(cpu_policy->cpu != cpu)) {
+		cpufreq_cpu_put(cpu_policy);
+		return 0;
+	}
+
+	if (cpufreq_driver->suspend) {
+		ret = cpufreq_driver->suspend(cpu_policy, state);
+		if (ret) {
+			printk(KERN_ERR "cpufreq: suspend failed in ->suspend "
+					"step on CPU %u\n", cpu_policy->cpu);
+			cpufreq_cpu_put(cpu_policy);
+			return ret;
+		}
+	}
+
+
+	if (cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)
+		goto out;
+
+	if (cpufreq_driver->get)
+		cur_freq = cpufreq_driver->get(cpu_policy->cpu);
+
+	if (!cur_freq || !cpu_policy->cur) {
+		printk(KERN_ERR "cpufreq: suspend failed to assert current "
+		       "frequency is what timing core thinks it is.\n");
+		goto out;
+	}
+
+	if (unlikely(cur_freq != cpu_policy->cur)) {
+		struct cpufreq_freqs freqs;
+
+		if (!(cpufreq_driver->flags & CPUFREQ_PM_NO_WARN))
+			printk(KERN_DEBUG "Warning: CPU frequency is %u, "
+			       "cpufreq assumed %u kHz.\n",
+			       cur_freq, cpu_policy->cur);
+
+		freqs.cpu = cpu;
+		freqs.old = cpu_policy->cur;
+		freqs.new = cur_freq;
+
+		notifier_call_chain(&cpufreq_transition_notifier_list,
+				    CPUFREQ_SUSPENDCHANGE, &freqs);
+		adjust_jiffies(CPUFREQ_SUSPENDCHANGE, &freqs);
+
+		cpu_policy->cur = cur_freq;
+	}
+
+ out:
+	cpufreq_cpu_put(cpu_policy);
+	return 0;
+}
+
+/**
  *	cpufreq_resume -  restore proper CPU frequency handling after resume
  *
  *	1.) resume CPUfreq hardware support (cpufreq_driver->resume())
  *	2.) if ->target and !CPUFREQ_CONST_LOOPS: verify we're in sync
- *	3.) schedule call cpufreq_update_policy() ASAP as interrupts are restored.
+ *	3.) schedule call cpufreq_update_policy() ASAP as interrupts are
+ *	    restored.
  */
 static int cpufreq_resume(struct sys_device * sysdev)
 {
 	int cpu = sysdev->id;
 	unsigned int ret = 0;
+	unsigned int cur_freq = 0;
 	struct cpufreq_policy *cpu_policy;
 
 	dprintk("resuming cpu %u\n", cpu);
@@ -908,32 +988,34 @@
 		}
 	}
 
-	if (!(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) {
-		unsigned int cur_freq = 0;
-
-		if (cpufreq_driver->get)
-			cur_freq = cpufreq_driver->get(cpu_policy->cpu);
-
-		if (!cur_freq || !cpu_policy->cur) {
-			printk(KERN_ERR "cpufreq: resume failed to assert current frequency is what timing core thinks it is.\n");
-			goto out;
-		}
-
-		if (unlikely(cur_freq != cpu_policy->cur)) {
-			struct cpufreq_freqs freqs;
+	if (cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)
+		goto out;
 
-			printk(KERN_WARNING "Warning: CPU frequency is %u, "
-			       "cpufreq assumed %u kHz.\n", cur_freq, cpu_policy->cur);
+	if (cpufreq_driver->get)
+		cur_freq = cpufreq_driver->get(cpu_policy->cpu);
 
-			freqs.cpu = cpu;
-			freqs.old = cpu_policy->cur;
-			freqs.new = cur_freq;
+	if (!cur_freq || !cpu_policy->cur) {
+		printk(KERN_ERR "cpufreq: resume failed to assert current "
+		       "frequency is what timing core thinks it is.\n");
+		goto out;
+	}
 
-			notifier_call_chain(&cpufreq_transition_notifier_list, CPUFREQ_RESUMECHANGE, &freqs);
-			adjust_jiffies(CPUFREQ_RESUMECHANGE, &freqs);
+	if (unlikely(cur_freq != cpu_policy->cur)) {
+		struct cpufreq_freqs freqs;
 
-			cpu_policy->cur = cur_freq;
-		}
+		if (!(cpufreq_driver->flags & CPUFREQ_PM_NO_WARN))
+			printk(KERN_DEBUG "Warning: CPU frequency is %u, "
+			       "cpufreq assumed %u kHz.\n",
+			       cur_freq, cpu_policy->cur);
+
+		freqs.cpu = cpu;
+		freqs.old = cpu_policy->cur;
+		freqs.new = cur_freq;
+
+		notifier_call_chain(&cpufreq_transition_notifier_list,
+				    CPUFREQ_RESUMECHANGE, &freqs);
+		adjust_jiffies(CPUFREQ_RESUMECHANGE, &freqs);
+		cpu_policy->cur = cur_freq;
 	}
 
 out:
@@ -945,6 +1027,7 @@
 static struct sysdev_driver cpufreq_sysdev_driver = {
 	.add		= cpufreq_add_dev,
 	.remove		= cpufreq_remove_dev,
+	.suspend	= cpufreq_suspend,
 	.resume		= cpufreq_resume,
 };
 
Index: linux-work/include/linux/cpufreq.h
===================================================================
--- linux-work.orig/include/linux/cpufreq.h	2005-03-30 09:42:18.000000000 +1000
+++ linux-work/include/linux/cpufreq.h	2005-03-30 09:46:25.000000000 +1000
@@ -103,6 +103,7 @@
 #define CPUFREQ_PRECHANGE	(0)
 #define CPUFREQ_POSTCHANGE	(1)
 #define CPUFREQ_RESUMECHANGE	(8)
+#define CPUFREQ_SUSPENDCHANGE	(9)
 
 struct cpufreq_freqs {
 	unsigned int cpu;	/* cpu nr */
@@ -200,6 +201,7 @@
 
 	/* optional */
 	int	(*exit)		(struct cpufreq_policy *policy);
+	int	(*suspend)	(struct cpufreq_policy *policy, u32 state);
 	int	(*resume)	(struct cpufreq_policy *policy);
 	struct freq_attr	**attr;
 };
@@ -211,7 +213,8 @@
 #define CPUFREQ_CONST_LOOPS 	0x02	/* loops_per_jiffy or other kernel
 					 * "constants" aren't affected by
 					 * frequency transitions */
-
+#define CPUFREQ_PM_NO_WARN	0x04	/* don't warn on suspend/resume speed
+					 * mismatches */
 
 int cpufreq_register_driver(struct cpufreq_driver *driver_data);
 int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: cpufreq suspend patch.
  2005-08-08 22:44 cpufreq suspend patch Dave Jones
@ 2005-08-09  8:32 ` Benjamin Herrenschmidt
  2005-08-09  8:53   ` David Woodhouse
  2005-08-09 16:25   ` Dave Jones
  0 siblings, 2 replies; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2005-08-09  8:32 UTC (permalink / raw)
  To: Dave Jones; +Cc: cpufreq, dwmw2

On Mon, 2005-08-08 at 18:44 -0400, Dave Jones wrote:
> Ben,
>  Dave Woodhouse added this to the Fedora kernel a while back
> with the helpful comment "Fix ppc32 sleep/cpufreq harder"
> I believe this came from you originally.
> 
> Can you comment on this ? 
> 
> It seems to make sense to me,  but if you have a newer patch
> I'm all ears..

Can you compare with what's currently upstream ? (I'm still travelling
around so it's more difficult for me).

Ben.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: cpufreq suspend patch.
  2005-08-09  8:32 ` Benjamin Herrenschmidt
@ 2005-08-09  8:53   ` David Woodhouse
  2005-08-09  8:54     ` Benjamin Herrenschmidt
  2005-08-09 16:25   ` Dave Jones
  1 sibling, 1 reply; 5+ messages in thread
From: David Woodhouse @ 2005-08-09  8:53 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: cpufreq, Dave Jones

On Tue, 2005-08-09 at 10:32 +0200, Benjamin Herrenschmidt wrote:
> Can you compare with what's currently upstream ? (I'm still travelling
> around so it's more difficult for me).

It's very similar to what's upstream, but not identical. Your original
patch has some extra changes in cpufreq_resume()...

 static int cpufreq_resume(struct sys_device * sysdev)
 {
        int cpu = sysdev->id;
        unsigned int ret = 0;
+       unsigned int cur_freq = 0;
        struct cpufreq_policy *cpu_policy;
 
        dprintk("resuming cpu %u\n", cpu);
@@ -908,32 +988,34 @@
                }
        }
 
-       if (!(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) {
-               unsigned int cur_freq = 0;
-
-               if (cpufreq_driver->get)
-                       cur_freq = cpufreq_driver->get(cpu_policy->cpu);
-
-               if (!cur_freq || !cpu_policy->cur) {
-                       printk(KERN_ERR "cpufreq: resume failed to assert current frequency is what timing core thinks it is.\n");
-                       goto out;
-               }
-
-               if (unlikely(cur_freq != cpu_policy->cur)) {
-                       struct cpufreq_freqs freqs;
+       if (cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)
+               goto out;
 
-                       printk(KERN_WARNING "Warning: CPU frequency is %u, "
-                              "cpufreq assumed %u kHz.\n", cur_freq, cpu_policy->cur);
+       if (cpufreq_driver->get)
+               cur_freq = cpufreq_driver->get(cpu_policy->cpu);
 
-                       freqs.cpu = cpu;
-                       freqs.old = cpu_policy->cur;
-                       freqs.new = cur_freq;
+       if (!cur_freq || !cpu_policy->cur) {
+               printk(KERN_ERR "cpufreq: resume failed to assert current "
+                      "frequency is what timing core thinks it is.\n");
+               goto out;
+       }
 
-                       notifier_call_chain(&cpufreq_transition_notifier_list, CPUFREQ_RESUMECHANGE, &freqs);
-                       adjust_jiffies(CPUFREQ_RESUMECHANGE, &freqs);
+       if (unlikely(cur_freq != cpu_policy->cur)) {
+               struct cpufreq_freqs freqs;
 
-                       cpu_policy->cur = cur_freq;
-               }
+               if (!(cpufreq_driver->flags & CPUFREQ_PM_NO_WARN))
+                       printk(KERN_DEBUG "Warning: CPU frequency is %u, "
+                              "cpufreq assumed %u kHz.\n",
+                              cur_freq, cpu_policy->cur);
+
+               freqs.cpu = cpu;
+               freqs.old = cpu_policy->cur;
+               freqs.new = cur_freq;
+
+               notifier_call_chain(&cpufreq_transition_notifier_list,
+                                   CPUFREQ_RESUMECHANGE, &freqs);
+               adjust_jiffies(CPUFREQ_RESUMECHANGE, &freqs);
+               cpu_policy->cur = cur_freq;
        }
-- 
dwmw2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: cpufreq suspend patch.
  2005-08-09  8:53   ` David Woodhouse
@ 2005-08-09  8:54     ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2005-08-09  8:54 UTC (permalink / raw)
  To: David Woodhouse; +Cc: cpufreq, Dave Jones

On Tue, 2005-08-09 at 09:53 +0100, David Woodhouse wrote:
> On Tue, 2005-08-09 at 10:32 +0200, Benjamin Herrenschmidt wrote:
> > Can you compare with what's currently upstream ? (I'm still travelling
> > around so it's more difficult for me).
> 
> It's very similar to what's upstream, but not identical. Your original
> patch has some extra changes in cpufreq_resume()...

Ah yes, I just reworked the if () statements to drift less to the right
iirc, no functional changes.

Ben.


>  static int cpufreq_resume(struct sys_device * sysdev)
>  {
>         int cpu = sysdev->id;
>         unsigned int ret = 0;
> +       unsigned int cur_freq = 0;
>         struct cpufreq_policy *cpu_policy;
>  
>         dprintk("resuming cpu %u\n", cpu);
> @@ -908,32 +988,34 @@
>                 }
>         }
>  
> -       if (!(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) {
> -               unsigned int cur_freq = 0;
> -
> -               if (cpufreq_driver->get)
> -                       cur_freq = cpufreq_driver->get(cpu_policy->cpu);
> -
> -               if (!cur_freq || !cpu_policy->cur) {
> -                       printk(KERN_ERR "cpufreq: resume failed to assert current frequency is what timing core thinks it is.\n");
> -                       goto out;
> -               }
> -
> -               if (unlikely(cur_freq != cpu_policy->cur)) {
> -                       struct cpufreq_freqs freqs;
> +       if (cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)
> +               goto out;
>  
> -                       printk(KERN_WARNING "Warning: CPU frequency is %u, "
> -                              "cpufreq assumed %u kHz.\n", cur_freq, cpu_policy->cur);
> +       if (cpufreq_driver->get)
> +               cur_freq = cpufreq_driver->get(cpu_policy->cpu);
>  
> -                       freqs.cpu = cpu;
> -                       freqs.old = cpu_policy->cur;
> -                       freqs.new = cur_freq;
> +       if (!cur_freq || !cpu_policy->cur) {
> +               printk(KERN_ERR "cpufreq: resume failed to assert current "
> +                      "frequency is what timing core thinks it is.\n");
> +               goto out;
> +       }
>  
> -                       notifier_call_chain(&cpufreq_transition_notifier_list, CPUFREQ_RESUMECHANGE, &freqs);
> -                       adjust_jiffies(CPUFREQ_RESUMECHANGE, &freqs);
> +       if (unlikely(cur_freq != cpu_policy->cur)) {
> +               struct cpufreq_freqs freqs;
>  
> -                       cpu_policy->cur = cur_freq;
> -               }
> +               if (!(cpufreq_driver->flags & CPUFREQ_PM_NO_WARN))
> +                       printk(KERN_DEBUG "Warning: CPU frequency is %u, "
> +                              "cpufreq assumed %u kHz.\n",
> +                              cur_freq, cpu_policy->cur);
> +
> +               freqs.cpu = cpu;
> +               freqs.old = cpu_policy->cur;
> +               freqs.new = cur_freq;
> +
> +               notifier_call_chain(&cpufreq_transition_notifier_list,
> +                                   CPUFREQ_RESUMECHANGE, &freqs);
> +               adjust_jiffies(CPUFREQ_RESUMECHANGE, &freqs);
> +               cpu_policy->cur = cur_freq;
>         }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: cpufreq suspend patch.
  2005-08-09  8:32 ` Benjamin Herrenschmidt
  2005-08-09  8:53   ` David Woodhouse
@ 2005-08-09 16:25   ` Dave Jones
  1 sibling, 0 replies; 5+ messages in thread
From: Dave Jones @ 2005-08-09 16:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: cpufreq, dwmw2

On Tue, Aug 09, 2005 at 10:32:39AM +0200, Ben Herrenschmidt wrote:
 > On Mon, 2005-08-08 at 18:44 -0400, Dave Jones wrote:
 > > Ben,
 > >  Dave Woodhouse added this to the Fedora kernel a while back
 > > with the helpful comment "Fix ppc32 sleep/cpufreq harder"
 > > I believe this came from you originally.
 > > 
 > > Can you comment on this ? 
 > > 
 > > It seems to make sense to me,  but if you have a newer patch
 > > I'm all ears..
 > 
 > Can you compare with what's currently upstream ? (I'm still travelling
 > around so it's more difficult for me).

Gah, I've been bitten by the 'in Fedora cvs, but not applied to
spec file' thing again. A variant of this is already upstream.

Thanks.

		Dave

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2005-08-09 16:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-08-08 22:44 cpufreq suspend patch Dave Jones
2005-08-09  8:32 ` Benjamin Herrenschmidt
2005-08-09  8:53   ` David Woodhouse
2005-08-09  8:54     ` Benjamin Herrenschmidt
2005-08-09 16:25   ` Dave Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.