[PATCH] [2.6.22] pasemi: cpufreq driver

[PATCH] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

Cpufreq driver for PA Semi PWRficient processors.

Signed-off-by: Egor Martovetsky <egor@pasemi.com>
Signed-off-by: Olof Johansson <olof@lixom.net>


Index: powerpc/arch/powerpc/platforms/pasemi/Makefile
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pasemi/Makefile
+++ powerpc/arch/powerpc/platforms/pasemi/Makefile
@@ -1,2 +1,3 @@
 obj-y	+= setup.o pci.o time.o idle.o powersave.o iommu.o gpio_mdio.o
 
+obj-$(CONFIG_PPC_PASEMI_CPUFREQ) += cpufreq.o
Index: powerpc/arch/powerpc/platforms/pasemi/cpufreq.c
===================================================================
--- /dev/null
+++ powerpc/arch/powerpc/platforms/pasemi/cpufreq.c
@@ -0,0 +1,258 @@
+/*
+ * Copyright (C) 2007 PA Semi, Inc
+ *
+ * Author: Egor Martovetsky <egor@pasemi.com>
+ *
+ * Maintained by: Olof Johansson <olof@lixom.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ *
+ */
+
+#include <linux/cpufreq.h>
+#include <linux/timer.h>
+
+#include <asm/hw_irq.h>
+#include <asm/io.h>
+#include <asm/prom.h>
+
+#define SDCASR_REG		0x0100
+#define SDCASR_REG_STRIDE	0x1000
+#define SDCPWR_CFGA0_REG	0x0100
+#define SDCPWR_PWST0_REG	0x0000
+#define SDCPWR_GIZTIME_REG	0x0440
+
+/* SDCPWR_GIZTIME_REG fields */
+#define SDCPWR_GIZTIME_GR	0x80000000
+#define SDCPWR_GIZTIME_LONGLOCK	0x000000ff
+
+/* This should eventually come out of the device tree */
+#define SDCPWR_BASE		0xfc104000
+#define SDCPWR_SIZE		0x1000
+
+#define SDCASR_BASE		0xfc120000
+#define SDCASR_SIZE		0x2000
+
+static void __iomem *sdcpwr_mapbase;
+static void __iomem *sdcasr_mapbase;
+
+
+static DEFINE_MUTEX(pas_switch_mutex);
+
+/* Current astate, is used when waking up from power savings on
+ * one core, in case the other core has switched states during
+ * the idle time.
+ */
+static int current_astate;
+
+/* We support 5(A0-A4) power states excluding turbo(A5-A6) modes */
+static struct cpufreq_frequency_table pas_freqs[] = {
+	{0,	0},
+	{1,	0},
+	{2,	0},
+	{3,	0},
+	{4,	0},
+	{0,	CPUFREQ_TABLE_END},
+};
+
+static struct freq_attr *pas_cpu_freqs_attr[] = {
+	&cpufreq_freq_attr_scaling_available_freqs,
+	NULL,
+};
+
+/*
+ * hardware specific functions
+ */
+
+static int get_astate_freq(int astate)
+{
+	u32 ret;
+	ret = in_le32(sdcpwr_mapbase + SDCPWR_CFGA0_REG + (astate * 0x10)) & 0x3f;
+
+	return ret;
+}
+
+static int get_cur_astate(int cpu)
+{
+	u32 ret;
+
+	ret = in_le32(sdcpwr_mapbase + SDCPWR_PWST0_REG);
+	ret = (ret >> (cpu * 4)) & 0x7;
+
+	return ret;
+}
+
+static int get_gizmo_latency(void)
+{
+	u32 giztime, ret;
+	giztime = in_le32(sdcpwr_mapbase + SDCPWR_GIZTIME_REG);
+
+	/* just provide the upper bound */
+	ret = (giztime & SDCPWR_GIZTIME_LONGLOCK) * ((giztime & SDCPWR_GIZTIME_GR) ? 128 : 1) * 1000;
+	return ret;
+}
+
+static void set_astate(int cpu, unsigned int astate)
+{
+	u64 flags;
+
+	/* Return if called before init has run */
+	if (unlikely(!sdcasr_mapbase))
+		return;
+
+	local_irq_save(flags);
+
+	out_le32(sdcasr_mapbase + SDCASR_REG + SDCASR_REG_STRIDE*cpu, astate);
+
+	local_irq_restore(flags);
+}
+
+void restore_astate(int cpu)
+{
+	set_astate(cpu, current_astate);
+}
+
+/*
+ * cpufreq functions
+ */
+
+static int pas_cpufreq_cpu_init (struct cpufreq_policy *policy)
+{
+	u32 *max_freq;
+	int i, cur_astate;
+	struct device_node *cpu;
+
+	cpu = of_get_cpu_node(policy->cpu, NULL);
+
+	if(!cpu)
+		return -ENODEV;
+
+	sdcpwr_mapbase = ioremap(SDCPWR_BASE, SDCPWR_SIZE);
+	sdcasr_mapbase = ioremap(SDCASR_BASE, SDCASR_SIZE);
+	if (!sdcpwr_mapbase || !sdcasr_mapbase)
+		panic("SDCMAP: Cannot map registers!");
+
+	pr_debug("init cpufreq on CPU %d\n", policy->cpu);
+
+	max_freq = (u32*) get_property(cpu, "clock-frequency", NULL);
+
+	if(!max_freq)
+		return -EINVAL;
+
+	// we need the freq in kHz
+	*max_freq /= 1000;
+
+	pr_debug("max clock-frequency is at %u kHz\n", *max_freq);
+	pr_debug("initializing frequency table\n");
+
+	// initialize frequency table
+	for (i=0; pas_freqs[i].frequency!=CPUFREQ_TABLE_END; i++) {
+		pas_freqs[i].frequency = get_astate_freq(pas_freqs[i].index) * 100000;
+		pr_debug("%d: %d\n", i, pas_freqs[i].frequency);
+	}
+
+	policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+
+	policy->cpuinfo.transition_latency = get_gizmo_latency();
+
+	cur_astate = get_cur_astate(policy->cpu);
+	pr_debug("current astate is at %d\n",cur_astate);
+
+	policy->cur = pas_freqs[cur_astate].frequency;
+	policy->cpus = cpu_possible_map;
+
+	cpufreq_frequency_table_get_attr (pas_freqs, policy->cpu);
+
+	/* this ensures that policy->cpuinfo_min and policy->cpuinfo_max are set correctly */
+	return cpufreq_frequency_table_cpuinfo (policy, pas_freqs);
+}
+
+static int pas_cpufreq_cpu_exit(struct cpufreq_policy *policy)
+{
+	cpufreq_frequency_table_put_attr(policy->cpu);
+	return 0;
+}
+
+static int pas_cpufreq_verify(struct cpufreq_policy *policy)
+{
+	return cpufreq_frequency_table_verify(policy, pas_freqs);
+}
+
+static int pas_cpufreq_target(struct cpufreq_policy *policy, unsigned int target_freq,
+			    unsigned int relation)
+{
+	struct cpufreq_freqs freqs;
+	int pas_astate_new;
+	int i;
+
+	cpufreq_frequency_table_target(policy,
+				       pas_freqs,
+				       target_freq,
+				       relation,
+				       &pas_astate_new);
+
+	freqs.old = policy->cur;
+	freqs.new = pas_freqs[pas_astate_new].frequency;
+	freqs.cpu = policy->cpu;
+
+	mutex_lock (&pas_switch_mutex);
+	cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+
+	pr_debug("setting frequency for cpu %d to %d kHz, 1/%d of max frequency\n",
+		 policy->cpu,
+		 pas_freqs[pas_astate_new].frequency,
+		 pas_freqs[pas_astate_new].index);
+
+	current_astate = pas_astate_new;
+
+	for_each_online_cpu(i) {
+		set_astate(i, pas_astate_new);
+	}
+
+	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	mutex_unlock(&pas_switch_mutex);
+
+	return 0;
+}
+
+static struct cpufreq_driver pas_cpufreq_driver = {
+	.name		= "pas-cpufreq",
+	.owner		= THIS_MODULE,
+	.flags		= CPUFREQ_CONST_LOOPS,
+	.init		= pas_cpufreq_cpu_init,
+	.exit		= pas_cpufreq_cpu_exit,
+	.verify		= pas_cpufreq_verify,
+	.target		= pas_cpufreq_target,
+	.attr		= pas_cpu_freqs_attr,
+};
+
+/*
+ * module init and destoy
+ */
+
+static int __init pas_cpufreq_init(void)
+{
+	return cpufreq_register_driver(&pas_cpufreq_driver);
+}
+
+static void __exit pas_cpufreq_exit(void)
+{
+	cpufreq_unregister_driver(&pas_cpufreq_driver);
+}
+
+module_init(pas_cpufreq_init);
+module_exit(pas_cpufreq_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Egor Martovetsky <egor@pasemi.com>");
Index: powerpc/arch/powerpc/platforms/pasemi/idle.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pasemi/idle.c
+++ powerpc/arch/powerpc/platforms/pasemi/idle.c
@@ -61,6 +61,10 @@ static int pasemi_system_reset_exception
 		/* do system reset */
 		return 0;
 	}
+
+	/* Set higher astate since we come out of power savings at 0 */
+	restore_astate(hard_smp_processor_id());
+
 	/* everything handled */
 	regs->msr |= MSR_RI;
 	return 1;
@@ -68,6 +72,11 @@ static int pasemi_system_reset_exception
 
 void __init pasemi_idle_init(void)
 {
+#ifndef CONFIG_PPC_PASEMI_CPUFREQ
+	printk(KERN_WARNING "No cpufreq driver, powersavings modes disabled\n");
+	current_mode = 0;
+#endif
+
 	ppc_md.system_reset_exception = pasemi_system_reset_exception;
 	ppc_md.power_save = modes[current_mode].entry;
 	printk(KERN_INFO "Using PA6T idle loop (%s)\n", modes[current_mode].name);
Index: powerpc/arch/powerpc/platforms/pasemi/pasemi.h
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pasemi/pasemi.h
+++ powerpc/arch/powerpc/platforms/pasemi/pasemi.h
@@ -14,6 +14,14 @@ extern void __init pasemi_idle_init(void
 extern void idle_spin(void);
 extern void idle_doze(void);
 
+/* Restore astate to last set */
+#ifdef CONFIG_PPC_PASEMI_CPUFREQ
+extern void restore_astate(int cpu);
+#else
+static inline void restore_astate(int cpu)
+{
+}
+#endif
 
 
 #endif /* _PASEMI_PASEMI_H */
Index: powerpc/arch/powerpc/platforms/Kconfig
===================================================================
--- powerpc.orig/arch/powerpc/platforms/Kconfig
+++ powerpc/arch/powerpc/platforms/Kconfig
@@ -155,7 +155,7 @@ source "drivers/cpufreq/Kconfig"
 
 config CPU_FREQ_PMAC
 	bool "Support for Apple PowerBooks"
-	depends on CPU_FREQ && ADB_PMU && PPC32
+	depends on ADB_PMU && PPC32
 	select CPU_FREQ_TABLE
 	help
 	  This adds support for frequency switching on Apple PowerBooks,
@@ -164,11 +164,21 @@ config CPU_FREQ_PMAC
 
 config CPU_FREQ_PMAC64
 	bool "Support for some Apple G5s"
-	depends on CPU_FREQ && PPC64
+	depends on PPC64
 	select CPU_FREQ_TABLE
 	help
 	  This adds support for frequency switching on Apple iMac G5,
 	  and some of the more recent desktop G5 machines as well.
+
+config PPC_PASEMI_CPUFREQ
+	bool "Support for PA Semi PWRficient"
+	depends on PPC_PASEMI
+	default y
+	select CPU_FREQ_TABLE
+	help
+	  This adds the support for frequency switching on PA Semi
+	  PWRficient processors.
+
 endmenu
 
 config PPC601_SYNC_FIX

Re: [PATCH] [2.6.22] pasemi: cpufreq driver

[Arnd Bergmann]

On Wednesday 25 April 2007, Olof Johansson wrote:

> +/* This should eventually come out of the device tree */
> +#define SDCPWR_BASE		0xfc104000
> +#define SDCPWR_SIZE		0x1000
> +
> +#define SDCASR_BASE		0xfc120000
> +#define SDCASR_SIZE		0x2000

the comment is right. Why don't you do it then? ;-)

> +static int pas_cpufreq_cpu_init (struct cpufreq_policy *policy)
> +{
> +	u32 *max_freq;
> +	int i, cur_astate;
> +	struct device_node *cpu;
> +
> +	cpu = of_get_cpu_node(policy->cpu, NULL);
> +
> +	if(!cpu)
> +		return -ENODEV;
> +
> +	sdcpwr_mapbase = ioremap(SDCPWR_BASE, SDCPWR_SIZE);
> +	sdcasr_mapbase = ioremap(SDCASR_BASE, SDCASR_SIZE);
> +	if (!sdcpwr_mapbase || !sdcasr_mapbase)
> +		panic("SDCMAP: Cannot map registers!");

I can't see any check in here that finds out if you are actually running on
the right hardware. 
The proper way to implement this driver would be to register an
of_platform_driver for the cpufreq device and then get the register
addresses from there,

> @@ -155,7 +155,7 @@ source "drivers/cpufreq/Kconfig"
>  
>  config CPU_FREQ_PMAC
>  	bool "Support for Apple PowerBooks"
> -	depends on CPU_FREQ && ADB_PMU && PPC32
> +	depends on ADB_PMU && PPC32
>  	select CPU_FREQ_TABLE
>  	help
>  	  This adds support for frequency switching on Apple PowerBooks,
> @@ -164,11 +164,21 @@ config CPU_FREQ_PMAC
>  
>  config CPU_FREQ_PMAC64
>  	bool "Support for some Apple G5s"
> -	depends on CPU_FREQ && PPC64
> +	depends on PPC64
>  	select CPU_FREQ_TABLE
>  	help
>  	  This adds support for frequency switching on Apple iMac G5,
>  	  and some of the more recent desktop G5 machines as well.

Why this change?

> +config PPC_PASEMI_CPUFREQ
> +	bool "Support for PA Semi PWRficient"
> +	depends on PPC_PASEMI
> +	default y
> +	select CPU_FREQ_TABLE
> +	help
> +	  This adds the support for frequency switching on PA Semi
> +	  PWRficient processors.
> +
>  endmenu

Why bool and not tristate?

	Arnd <><

Re: [PATCH] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

On Thu, Apr 26, 2007 at 01:47:35AM +0200, Arnd Bergmann wrote:
> On Wednesday 25 April 2007, Olof Johansson wrote:
> 
> > +/* This should eventually come out of the device tree */
> > +#define SDCPWR_BASE		0xfc104000
> > +#define SDCPWR_SIZE		0x1000
> > +
> > +#define SDCASR_BASE		0xfc120000
> > +#define SDCASR_SIZE		0x2000
> 
> the comment is right. Why don't you do it then? ;-)

Leftovers from when it was a regular platform driver.

> > +static int pas_cpufreq_cpu_init (struct cpufreq_policy *policy)
> > +{
> > +	u32 *max_freq;
> > +	int i, cur_astate;
> > +	struct device_node *cpu;
> > +
> > +	cpu = of_get_cpu_node(policy->cpu, NULL);
> > +
> > +	if(!cpu)
> > +		return -ENODEV;
> > +
> > +	sdcpwr_mapbase = ioremap(SDCPWR_BASE, SDCPWR_SIZE);
> > +	sdcasr_mapbase = ioremap(SDCASR_BASE, SDCASR_SIZE);
> > +	if (!sdcpwr_mapbase || !sdcasr_mapbase)
> > +		panic("SDCMAP: Cannot map registers!");
> 
> I can't see any check in here that finds out if you are actually running on
> the right hardware. 
> The proper way to implement this driver would be to register an
> of_platform_driver for the cpufreq device and then get the register
> addresses from there,

Yup.

> > @@ -155,7 +155,7 @@ source "drivers/cpufreq/Kconfig"
> >  
> >  config CPU_FREQ_PMAC
> >  	bool "Support for Apple PowerBooks"
> > -	depends on CPU_FREQ && ADB_PMU && PPC32
> > +	depends on ADB_PMU && PPC32
> >  	select CPU_FREQ_TABLE
> >  	help
> >  	  This adds support for frequency switching on Apple PowerBooks,
> > @@ -164,11 +164,21 @@ config CPU_FREQ_PMAC
> >  
> >  config CPU_FREQ_PMAC64
> >  	bool "Support for some Apple G5s"
> > -	depends on CPU_FREQ && PPC64
> > +	depends on PPC64
> >  	select CPU_FREQ_TABLE
> >  	help
> >  	  This adds support for frequency switching on Apple iMac G5,
> >  	  and some of the more recent desktop G5 machines as well.
> 
> Why this change?

They're redundant, but they should have been in a separate patch. I
forgot to revert them before rediffing.

> > +config PPC_PASEMI_CPUFREQ
> > +	bool "Support for PA Semi PWRficient"
> > +	depends on PPC_PASEMI
> > +	default y
> > +	select CPU_FREQ_TABLE
> > +	help
> > +	  This adds the support for frequency switching on PA Semi
> > +	  PWRficient processors.
> > +
> >  endmenu
> 
> Why bool and not tristate?

1) The other cpufreq drivers are bool
2) See the idle loop interaction: It can go tristate once we have runtime
selection of idle loop, but until then we need it for idle=doze.


-Olof

Re: [PATCH] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

On Wed, Apr 25, 2007 at 06:57:57PM -0500, olof wrote:

> 2) See the idle loop interaction: It can go tristate once we have runtime
> selection of idle loop, but until then we need it for idle=doze.

Grmbl. Looks like I posted a stale patch that actually won't build with
PPC_PASEMI_CPUFREQ=n. I'll repost with the correct version.


-Olof

[PATCH v2] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

Cpufreq driver for PA Semi PWRficient processors.

Signed-off-by: Egor Martovetsky <egor@pasemi.com>
Signed-off-by: Olof Johansson <olof@lixom.net>

---

Changes since last version:
* Attributed copyright correctly to cbe_cpufreq.c and adjust license
  to match (this was my mistake)
* Lookup the SDC and Gizmo device nodes to get register bases
* machine_is_compatible checks
* Cleanups as suggested by Arnd + misc whitespace
* Rebased on top of MDIO config patch

I chose not to do this as an of_platform driver since it doesn't fit
that well with the cpufreq driver model; having 3 levels of init/probe
functions is excessive.

Index: powerpc/arch/powerpc/platforms/pasemi/Makefile
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pasemi/Makefile
+++ powerpc/arch/powerpc/platforms/pasemi/Makefile
@@ -1,2 +1,3 @@
 obj-y	+= setup.o pci.o time.o idle.o powersave.o iommu.o
 obj-$(CONFIG_PPC_PASEMI_MDIO)	+= gpio_mdio.o
+obj-$(CONFIG_PPC_PASEMI_CPUFREQ) += cpufreq.o
Index: powerpc/arch/powerpc/platforms/pasemi/cpufreq.c
===================================================================
--- /dev/null
+++ powerpc/arch/powerpc/platforms/pasemi/cpufreq.c
@@ -0,0 +1,307 @@
+/*
+ * Copyright (C) 2007 PA Semi, Inc
+ *
+ * Authors: Egor Martovetsky <egor@pasemi.com>
+ *	    Olof Johansson <olof@lixom.net>
+ *
+ * Maintained by: Olof Johansson <olof@lixom.net>
+ *
+ * Based on arch/powerpc/platforms/cell/cbe_cpufreq.c:
+ * (C) Copyright IBM Deutschland Entwicklung GmbH 2005
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+
+#include <linux/cpufreq.h>
+#include <linux/timer.h>
+
+#include <asm/hw_irq.h>
+#include <asm/io.h>
+#include <asm/prom.h>
+
+#define SDCASR_REG		0x0100
+#define SDCASR_REG_STRIDE	0x1000
+#define SDCPWR_CFGA0_REG	0x0100
+#define SDCPWR_PWST0_REG	0x0000
+#define SDCPWR_GIZTIME_REG	0x0440
+
+/* SDCPWR_GIZTIME_REG fields */
+#define SDCPWR_GIZTIME_GR	0x80000000
+#define SDCPWR_GIZTIME_LONGLOCK	0x000000ff
+
+/* Offset of ASR registers from SDC base */
+#define SDCASR_OFFSET		0x120000
+
+static void __iomem *sdcpwr_mapbase;
+static void __iomem *sdcasr_mapbase;
+
+static DEFINE_MUTEX(pas_switch_mutex);
+
+/* Current astate, is used when waking up from power savings on
+ * one core, in case the other core has switched states during
+ * the idle time.
+ */
+static int current_astate;
+
+/* We support 5(A0-A4) power states excluding turbo(A5-A6) modes */
+static struct cpufreq_frequency_table pas_freqs[] = {
+	{0,	0},
+	{1,	0},
+	{2,	0},
+	{3,	0},
+	{4,	0},
+	{0,	CPUFREQ_TABLE_END},
+};
+
+static struct freq_attr *pas_cpu_freqs_attr[] = {
+	&cpufreq_freq_attr_scaling_available_freqs,
+	NULL,
+};
+
+/*
+ * hardware specific functions
+ */
+
+static int get_astate_freq(int astate)
+{
+	u32 ret;
+	ret = in_le32(sdcpwr_mapbase + SDCPWR_CFGA0_REG + (astate * 0x10));
+
+	return ret & 0x3f;
+}
+
+static int get_cur_astate(int cpu)
+{
+	u32 ret;
+
+	ret = in_le32(sdcpwr_mapbase + SDCPWR_PWST0_REG);
+	ret = (ret >> (cpu * 4)) & 0x7;
+
+	return ret;
+}
+
+static int get_gizmo_latency(void)
+{
+	u32 giztime, ret;
+
+	giztime = in_le32(sdcpwr_mapbase + SDCPWR_GIZTIME_REG);
+
+	/* just provide the upper bound */
+	if (giztime & SDCPWR_GIZTIME_GR)
+		ret = (giztime & SDCPWR_GIZTIME_LONGLOCK) * 128000;
+	else
+		ret = (giztime & SDCPWR_GIZTIME_LONGLOCK) * 1000;
+
+	return ret;
+}
+
+static void set_astate(int cpu, unsigned int astate)
+{
+	u64 flags;
+
+	/* Return if called before init has run */
+	if (unlikely(!sdcasr_mapbase))
+		return;
+
+	local_irq_save(flags);
+
+	out_le32(sdcasr_mapbase + SDCASR_REG + SDCASR_REG_STRIDE*cpu, astate);
+
+	local_irq_restore(flags);
+}
+
+void restore_astate(int cpu)
+{
+	set_astate(cpu, current_astate);
+}
+
+/*
+ * cpufreq functions
+ */
+
+static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
+{
+	u32 *max_freq;
+	int i, cur_astate;
+	struct resource res;
+	struct device_node *cpu, *dn;
+	int err = -ENODEV;
+
+	cpu = of_get_cpu_node(policy->cpu, NULL);
+
+	if (!cpu)
+		goto out;
+
+	dn = of_find_compatible_node(NULL, "sdc", "1682m-sdc");
+	if (!dn)
+		goto out;
+	err = of_address_to_resource(dn, 0, &res);
+	of_node_put(dn);
+	if (err)
+		goto out;
+	sdcasr_mapbase = ioremap(res.start + SDCASR_OFFSET, 0x2000);
+	if (!sdcasr_mapbase) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	dn = of_find_compatible_node(NULL, "gizmo", "1682m-gizmo");
+	if (!dn) {
+		err = -ENODEV;
+		goto out_unmap_sdcasr;
+	}
+	err = of_address_to_resource(dn, 0, &res);
+	of_node_put(dn);
+	if (err)
+		goto out_unmap_sdcasr;
+	sdcpwr_mapbase = ioremap(res.start, 0x1000);
+	if (!sdcpwr_mapbase) {
+		err = -EINVAL;
+		goto out_unmap_sdcasr;
+	}
+
+	pr_debug("init cpufreq on CPU %d\n", policy->cpu);
+
+	max_freq = (u32*) get_property(cpu, "clock-frequency", NULL);
+	if (!max_freq) {
+		err = -EINVAL;
+		goto out_unmap_sdcpwr;
+	}
+
+	/* we need the freq in kHz */
+	*max_freq /= 1000;
+
+	pr_debug("max clock-frequency is at %u kHz\n", *max_freq);
+	pr_debug("initializing frequency table\n");
+
+	/* initialize frequency table */
+	for (i=0; pas_freqs[i].frequency!=CPUFREQ_TABLE_END; i++) {
+		pas_freqs[i].frequency = get_astate_freq(pas_freqs[i].index) * 100000;
+		pr_debug("%d: %d\n", i, pas_freqs[i].frequency);
+	}
+
+	policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+
+	policy->cpuinfo.transition_latency = get_gizmo_latency();
+
+	cur_astate = get_cur_astate(policy->cpu);
+	pr_debug("current astate is at %d\n",cur_astate);
+
+	policy->cur = pas_freqs[cur_astate].frequency;
+	policy->cpus = cpu_possible_map;
+
+	cpufreq_frequency_table_get_attr(pas_freqs, policy->cpu);
+
+	/* this ensures that policy->cpuinfo_min and policy->cpuinfo_max
+	 * are set correctly
+	 */
+	return cpufreq_frequency_table_cpuinfo(policy, pas_freqs);
+
+out_unmap_sdcpwr:
+	iounmap(sdcpwr_mapbase);
+
+out_unmap_sdcasr:
+	iounmap(sdcasr_mapbase);
+out:
+	return err;
+}
+
+static int pas_cpufreq_cpu_exit(struct cpufreq_policy *policy)
+{
+	if (sdcasr_mapbase)
+		iounmap(sdcasr_mapbase);
+	if (sdcpwr_mapbase)
+		iounmap(sdcpwr_mapbase);
+
+	cpufreq_frequency_table_put_attr(policy->cpu);
+	return 0;
+}
+
+static int pas_cpufreq_verify(struct cpufreq_policy *policy)
+{
+	return cpufreq_frequency_table_verify(policy, pas_freqs);
+}
+
+static int pas_cpufreq_target(struct cpufreq_policy *policy,
+			      unsigned int target_freq,
+			      unsigned int relation)
+{
+	struct cpufreq_freqs freqs;
+	int pas_astate_new;
+	int i;
+
+	cpufreq_frequency_table_target(policy,
+				       pas_freqs,
+				       target_freq,
+				       relation,
+				       &pas_astate_new);
+
+	freqs.old = policy->cur;
+	freqs.new = pas_freqs[pas_astate_new].frequency;
+	freqs.cpu = policy->cpu;
+
+	mutex_lock(&pas_switch_mutex);
+	cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+
+	pr_debug("setting frequency for cpu %d to %d kHz, 1/%d of max frequency\n",
+		 policy->cpu,
+		 pas_freqs[pas_astate_new].frequency,
+		 pas_freqs[pas_astate_new].index);
+
+	current_astate = pas_astate_new;
+
+	for_each_online_cpu(i)
+		set_astate(i, pas_astate_new);
+
+	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	mutex_unlock(&pas_switch_mutex);
+
+	return 0;
+}
+
+static struct cpufreq_driver pas_cpufreq_driver = {
+	.name		= "pas-cpufreq",
+	.owner		= THIS_MODULE,
+	.flags		= CPUFREQ_CONST_LOOPS,
+	.init		= pas_cpufreq_cpu_init,
+	.exit		= pas_cpufreq_cpu_exit,
+	.verify		= pas_cpufreq_verify,
+	.target		= pas_cpufreq_target,
+	.attr		= pas_cpu_freqs_attr,
+};
+
+/*
+ * module init and destoy
+ */
+
+static int __init pas_cpufreq_init(void)
+{
+	if (!machine_is_compatible("PA6T-1682M"))
+		return 0;
+
+	return cpufreq_register_driver(&pas_cpufreq_driver);
+}
+
+static void __exit pas_cpufreq_exit(void)
+{
+	cpufreq_unregister_driver(&pas_cpufreq_driver);
+}
+
+module_init(pas_cpufreq_init);
+module_exit(pas_cpufreq_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Egor Martovetsky <egor@pasemi.com>, Olof Johansson <olof@lixom.net>");
Index: powerpc/arch/powerpc/platforms/pasemi/idle.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pasemi/idle.c
+++ powerpc/arch/powerpc/platforms/pasemi/idle.c
@@ -61,6 +61,10 @@ static int pasemi_system_reset_exception
 		/* do system reset */
 		return 0;
 	}
+
+	/* Set higher astate since we come out of power savings at 0 */
+	restore_astate(hard_smp_processor_id());
+
 	/* everything handled */
 	regs->msr |= MSR_RI;
 	return 1;
@@ -68,6 +72,11 @@ static int pasemi_system_reset_exception
 
 void __init pasemi_idle_init(void)
 {
+#ifndef CONFIG_PPC_PASEMI_CPUFREQ
+	printk(KERN_WARNING "No cpufreq driver, powersavings modes disabled\n");
+	current_mode = 0;
+#endif
+
 	ppc_md.system_reset_exception = pasemi_system_reset_exception;
 	ppc_md.power_save = modes[current_mode].entry;
 	printk(KERN_INFO "Using PA6T idle loop (%s)\n", modes[current_mode].name);
Index: powerpc/arch/powerpc/platforms/pasemi/pasemi.h
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pasemi/pasemi.h
+++ powerpc/arch/powerpc/platforms/pasemi/pasemi.h
@@ -14,6 +14,14 @@ extern void __init pasemi_idle_init(void
 extern void idle_spin(void);
 extern void idle_doze(void);
 
+/* Restore astate to last set */
+#ifdef CONFIG_PPC_PASEMI_CPUFREQ
+extern void restore_astate(int cpu);
+#else
+static inline void restore_astate(int cpu)
+{
+}
+#endif
 
 
 #endif /* _PASEMI_PASEMI_H */
Index: powerpc/arch/powerpc/platforms/Kconfig
===================================================================
--- powerpc.orig/arch/powerpc/platforms/Kconfig
+++ powerpc/arch/powerpc/platforms/Kconfig
@@ -169,6 +169,16 @@ config CPU_FREQ_PMAC64
 	help
 	  This adds support for frequency switching on Apple iMac G5,
 	  and some of the more recent desktop G5 machines as well.
+
+config PPC_PASEMI_CPUFREQ
+	bool "Support for PA Semi PWRficient"
+	depends on PPC_PASEMI
+	default y
+	select CPU_FREQ_TABLE
+	help
+	  This adds the support for frequency switching on PA Semi
+	  PWRficient processors.
+
 endmenu
 
 config PPC601_SYNC_FIX

cbe_cpufreq crashes my machine

[Olof Johansson]

On Thu, Apr 26, 2007 at 01:47:35AM +0200, Arnd Bergmann wrote:
> I can't see any check in here that finds out if you are actually running on
> the right hardware. 

Seems like this came along with our use of cbe_cpufreq.c as a basis. You
don't do it either! :-)

Enabling CONFIG_CBE_CPUFREQ makes my machine die a horrible death,
see below. I'd post a patch if I knew what machine compatible fields to
compare with, but I have no clue what's considered approprate to check
for on cell.


Unable to handle kernel paging request for data at address 0x00000888
Faulting instruction address: 0xc000000000036b14
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=2
Modules linked in:
NIP: C000000000036B14 LR: C000000000036B08 CTR: C000000000036A60
REGS: c00000007fd83830 TRAP: 0300   Not tainted  (2.6.20)
MSR: 9000000000009032 <EE,ME,IR,DR>  CR: 82000022  XER: 20000000
DAR: 0000000000000888, DSISR: 0000000040000000
TASK = c00000000ffc5820[1] 'swapper' THREAD: c00000007fd80000 CPU: 0
GPR00: C00000000093F940 C00000007FD83AB0 C000000000916EC0 0000000000000000
GPR04: C00000000068343F C0000000009C1208 C00000007F8FAE00 0000000000000000
GPR08: C000000000859720 0000000000000888 C0000000009C3DE0 0000000000000000
GPR12: 0000000000000000 C000000000779600 0000000000000000 C000000000684808
GPR16: 4000000000000000 C0000000006834E0 0000000000000000 0000000000000000
GPR20: 0000000000764880 C000000000764880 C000000000764AF0 0000000000764AF0
GPR24: C000000000684308 C00000000085FAE8 C0000000025DD028 0000000000000000
GPR28: FFFFFFFFFFFFFFF4 C00000007F8FAC38 C000000000784B58 C00000007F8FAC00
NIP [C000000000036B14] .cbe_cpufreq_cpu_init+0xb4/0x12c
LR [C000000000036B08] .cbe_cpufreq_cpu_init+0xa8/0x12c
Call Trace:
[C00000007FD83AB0] [C000000000036AA8] .cbe_cpufreq_cpu_init+0x48/0x12c (unreliable)
[C00000007FD83B40] [C0000000004831FC] .cpufreq_add_dev+0x158/0x4f4
[C00000007FD83D00] [C00000000033A5E4] .sysdev_driver_register+0xbc/0x158
[C00000007FD83D90] [C000000000482FF0] .cpufreq_register_driver+0xd0/0x184
[C00000007FD83E40] [C0000000007421B4] .cbe_cpufreq_init+0x1c/0x34
[C00000007FD83EC0] [C0000000000093B4] .init+0x1c4/0x39c
[C00000007FD83F90] [C000000000022E10] .kernel_thread+0x4c/0x68
Instruction dump:
800b0000 2f80fffe 409effe4 e93e8038 380061a8 e87f000e 901f0018 f93f0030
4bffe239 60000000 39230888 7c0004ac <e9230888> 0c090000 4c00012c ebbe8008



-Olof

Re: cbe_cpufreq crashes my machine

[Benjamin Herrenschmidt]

On Thu, 2007-04-26 at 01:56 -0500, Olof Johansson wrote:
> On Thu, Apr 26, 2007 at 01:47:35AM +0200, Arnd Bergmann wrote:
> > I can't see any check in here that finds out if you are actually running on
> > the right hardware. 
> 
> Seems like this came along with our use of cbe_cpufreq.c as a basis. You
> don't do it either! :-)
> 
> Enabling CONFIG_CBE_CPUFREQ makes my machine die a horrible death,
> see below. I'd post a patch if I knew what machine compatible fields to
> compare with, but I have no clue what's considered approprate to check
> for on cell.

Or machine_is(cell)

Ben.

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Arnd Bergmann]

On Thursday 26 April 2007, Olof Johansson wrote:

> I chose not to do this as an of_platform driver since it doesn't fit
> that well with the cpufreq driver model; having 3 levels of init/probe
> functions is excessive.

<snip>

> +	dn = of_find_compatible_node(NULL, "sdc", "1682m-sdc");
> +	if (!dn)
> +		goto out;
> +	err = of_address_to_resource(dn, 0, &res);
> +	of_node_put(dn);
> +	if (err)
> +		goto out;
> +	sdcasr_mapbase = ioremap(res.start + SDCASR_OFFSET, 0x2000);
> +	if (!sdcasr_mapbase) {
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	dn = of_find_compatible_node(NULL, "gizmo", "1682m-gizmo");
> +	if (!dn) {
> +		err = -ENODEV;
> +		goto out_unmap_sdcasr;
> +	}
> +	err = of_address_to_resource(dn, 0, &res);
> +	of_node_put(dn);
> +	if (err)
> +		goto out_unmap_sdcasr;
> +	sdcpwr_mapbase = ioremap(res.start, 0x1000);
> +	if (!sdcpwr_mapbase) {
> +		err = -EINVAL;
> +		goto out_unmap_sdcasr;
> +	}

What are sdc and gizmo anyway? If they are both only used for cpufreq, maybe the
easiest way to do this with an of_platform_driver would be to have a single
node that has two register ranges.

	Arnd <><

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On Thu, 2007-04-26 at 00:37 -0500, Olof Johansson wrote:
> 
> +       policy->cur = pas_freqs[cur_astate].frequency;
> +       policy->cpus = cpu_possible_map; 

That doesn't seem right.

Either, all your processors scale along each other in which case you
should use cpu_online_map here, or they scale each on their own in which
case you just set a single bit here.

The generic code works like this:
CPU 0 is brought online and cpufreq initialised for it
CPU 1 is brought online and cpufreq initialised for it. if cpus includes
more than a single bit, cpufreq is linked to the first other CPU in
policy->cpus and then cpufreq for CPU1 is deinitialised again.

We have the same bug on powermac but for some reason the patch to fix it
that I posted a long time ago (look for "powermac: fix G5-cpufreq for
cpu on/offline") wasn't applied yet. Look at it though, it includes a
comment on what needs to be done.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

On Thu, Apr 26, 2007 at 10:55:33AM +0200, Arnd Bergmann wrote:
> On Thursday 26 April 2007, Olof Johansson wrote:
> 
> > I chose not to do this as an of_platform driver since it doesn't fit
> > that well with the cpufreq driver model; having 3 levels of init/probe
> > functions is excessive.
> 
> <snip>
> 
> > +	dn = of_find_compatible_node(NULL, "sdc", "1682m-sdc");
> > +	if (!dn)
> > +		goto out;
> > +	err = of_address_to_resource(dn, 0, &res);
> > +	of_node_put(dn);
> > +	if (err)
> > +		goto out;
> > +	sdcasr_mapbase = ioremap(res.start + SDCASR_OFFSET, 0x2000);
> > +	if (!sdcasr_mapbase) {
> > +		err = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	dn = of_find_compatible_node(NULL, "gizmo", "1682m-gizmo");
> > +	if (!dn) {
> > +		err = -ENODEV;
> > +		goto out_unmap_sdcasr;
> > +	}
> > +	err = of_address_to_resource(dn, 0, &res);
> > +	of_node_put(dn);
> > +	if (err)
> > +		goto out_unmap_sdcasr;
> > +	sdcpwr_mapbase = ioremap(res.start, 0x1000);
> > +	if (!sdcpwr_mapbase) {
> > +		err = -EINVAL;
> > +		goto out_unmap_sdcasr;
> > +	}
> 
> What are sdc and gizmo anyway? If they are both only used for cpufreq, maybe the
> easiest way to do this with an of_platform_driver would be to have a single
> node that has two register ranges.

SDC is the system and debug controller, it contains a number of smaller
devices such as the PIC, the PMU (Gizmo), RNG, and various debug
features. Some already have drivers submitted, others will later on.

Unfortunately the setting of the current active state is done to an SDC
register, while information of the states is in the PMU, so access to
both is needed in the driver.

That doesn't change the fact that making this an of_platform driver
is excessive:

* module_init to register an of_platform driver
* of_platform walks the tree, calls probes
* of_platform driver probe code to register the cpufreq driver
* cpufreq calls it's registered drivers
* the cpufreq driver in turn will do the inits

Compare to:
* module_init registers cpufreq driver if machine_is_compatible()
* cpufreq calls it's registered drivers
* the cpufreq driver in turn will do the inits

Don't get me wrong, of_platform drivers are often convenient, but I
don't see it being a benefit to use them in this case.


-Olof

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Arnd Bergmann]

On Thursday 26 April 2007, Olof Johansson wrote:
> SDC is the system and debug controller, it contains a number of smaller
> devices such as the PIC, the PMU (Gizmo), RNG, and various debug
> features. Some already have drivers submitted, others will later on.

Oh well, these chips all seem to be the same. On cell, we have solved
the problem by introducing the 'cbe_regs' helper library that gives
access to all those miscellaneous registers to the individual device
drivers, so that not all of them need to scan the device tree for
the same registers and map them individually.

The problem with an of_platform_driver for this would be that you
can only have _one_ driver attached to the registers.

> Unfortunately the setting of the current active state is done to an SDC
> register, while information of the states is in the PMU, so access to
> both is needed in the driver.

One thing that you could do is to list only this one register of the
SDC in the reg property, not all of the SDC. 
 
	Arnd <><

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Segher Boessenkool]

>> Unfortunately the setting of the current active state is done to an 
>> SDC
>> register, while information of the states is in the PMU, so access to
>> both is needed in the driver.
>
> One thing that you could do is to list only this one register of the
> SDC in the reg property, not all of the SDC.

The system-controller node should really describe
all of the system controller.  If the Linux driver
needs to access two separate devices, it should
really just do that.  Don't try to make things
"simpler" in the device tree.

You probably should map the SDC registers in the
platform startop code, and then have the various
platform drivers use that.  Maybe even do an
abstraction ;-)


Segher

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

On Thu, Apr 26, 2007 at 07:11:21PM +0200, Arnd Bergmann wrote:
> On Thursday 26 April 2007, Olof Johansson wrote:
> > SDC is the system and debug controller, it contains a number of smaller
> > devices such as the PIC, the PMU (Gizmo), RNG, and various debug
> > features. Some already have drivers submitted, others will later on.
> 
> Oh well, these chips all seem to be the same. On cell, we have solved
> the problem by introducing the 'cbe_regs' helper library that gives
> access to all those miscellaneous registers to the individual device
> drivers, so that not all of them need to scan the device tree for
> the same registers and map them individually.

Not a bad idea, I'll consider it for the future. Most of our devices are
on a pseudo-PCI bus, so it's only a handful that need special register
access. The stuff in the SDC is the biggest exception.

> The problem with an of_platform_driver for this would be that you
> can only have _one_ driver attached to the registers.
>
> > Unfortunately the setting of the current active state is done to an SDC
> > register, while information of the states is in the PMU, so access to
> > both is needed in the driver.
> 
> One thing that you could do is to list only this one register of the
> SDC in the reg property, not all of the SDC. 

That doesn't make much sense in this case. The register isn't in the
Gizmo.

Having a driver/library for the SDC that the gizmo driver can use is a
good idea, similar to what you've done on cell. I'll try to have that
ready for 2.6.23, but that shouldn't stop this base driver from going
in now.


-Olof

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

On Thu, Apr 26, 2007 at 12:26:00PM +0200, Johannes Berg wrote:
> On Thu, 2007-04-26 at 00:37 -0500, Olof Johansson wrote:
> > 
> > +       policy->cur = pas_freqs[cur_astate].frequency;
> > +       policy->cpus = cpu_possible_map; 
> 
> That doesn't seem right.
> 
> Either, all your processors scale along each other in which case you
> should use cpu_online_map here, or they scale each on their own in which
> case you just set a single bit here.

You can set the requested speed per-processor, but the actual speed
will be the max of them. Because of this, it's less surprising to set
it on both at the same time since then you'll at least know what you're
running at.

So yes, I'll change it to online_map instead of possible_map.

> The generic code works like this:
> CPU 0 is brought online and cpufreq initialised for it
> CPU 1 is brought online and cpufreq initialised for it. if cpus includes
> more than a single bit, cpufreq is linked to the first other CPU in
> policy->cpus and then cpufreq for CPU1 is deinitialised again.
>
> We have the same bug on powermac but for some reason the patch to fix it
> that I posted a long time ago (look for "powermac: fix G5-cpufreq for
> cpu on/offline") wasn't applied yet. Look at it though, it includes a
> comment on what needs to be done.

Hmm, I just tried adding debugging to the cpu_init code, and I'm not
seeing it called more than once (i.e. only for cpu 0). Mind you, I don't
have cpu hotplug support at this time.






-Olof

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

On Thu, Apr 26, 2007 at 09:05:59PM +0200, Segher Boessenkool wrote:
> >>Unfortunately the setting of the current active state is done to an 
> >>SDC
> >>register, while information of the states is in the PMU, so access to
> >>both is needed in the driver.
> >
> >One thing that you could do is to list only this one register of the
> >SDC in the reg property, not all of the SDC.
> 
> The system-controller node should really describe
> all of the system controller.  If the Linux driver
> needs to access two separate devices, it should
> really just do that.  Don't try to make things
> "simpler" in the device tree.
> 
> You probably should map the SDC registers in the
> platform startop code, and then have the various
> platform drivers use that.  Maybe even do an
> abstraction ;-)

Yep, it's a good idea to do and I'll look at it for 2.6.23 time frame.


Thanks,

-Olof

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Arnd Bergmann]

On Thursday 26 April 2007, Olof Johansson wrote:
> Having a driver/library for the SDC that the gizmo driver can use is a
> good idea, similar to what you've done on cell. I'll try to have that
> ready for 2.6.23, but that shouldn't stop this base driver from going
> in now.

Yes, agreed.

	Arnd <><

[PATCH] cell: cbe_cpufreq cleanup and crash fix

[Olof Johansson]

cbe_cpufreq cleanups:

* comment format
* whitespace
* don't init on non-cell platforms

Signed-off-by: Olof Johansson <olof@lixom.net>

Index: powerpc/arch/powerpc/platforms/cell/cbe_cpufreq.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/cell/cbe_cpufreq.c
+++ powerpc/arch/powerpc/platforms/cell/cbe_cpufreq.c
@@ -155,7 +155,7 @@ static int set_pmode_reg(int cpu, unsign
 }
 
 static int set_pmode(int cpu, unsigned int slow_mode) {
-	if(pmi_dev)
+	if (pmi_dev)
 		return set_pmode_pmi(cpu, slow_mode);
 	else
 		return set_pmode_reg(cpu, slow_mode);
@@ -167,7 +167,7 @@ static void cbe_cpufreq_handle_pmi(struc
 	u8 cpu;
 	u8 cbe_pmode_new;
 
-	BUG_ON (pmi_msg.type != PMI_TYPE_FREQ_CHANGE);
+	BUG_ON(pmi_msg.type != PMI_TYPE_FREQ_CHANGE);
 
 	cpu = cbe_node_to_cpu(pmi_msg.data1);
 	cbe_pmode_new = pmi_msg.data2;
@@ -191,7 +191,7 @@ static struct pmi_handler cbe_pmi_handle
  * cpufreq functions
  */
 
-static int cbe_cpufreq_cpu_init (struct cpufreq_policy *policy)
+static int cbe_cpufreq_cpu_init(struct cpufreq_policy *policy)
 {
 	const u32 *max_freqp;
 	u32 max_freq;
@@ -200,7 +200,7 @@ static int cbe_cpufreq_cpu_init (struct 
 
 	cpu = of_get_cpu_node(policy->cpu, NULL);
 
-	if(!cpu)
+	if (!cpu)
 		return -ENODEV;
 
 	pr_debug("init cpufreq on CPU %d\n", policy->cpu);
@@ -210,13 +210,13 @@ static int cbe_cpufreq_cpu_init (struct 
 	if (!max_freqp)
 		return -EINVAL;
 
-	// we need the freq in kHz
+	/* we need the freq in kHz */
 	max_freq = *max_freqp / 1000;
 
 	pr_debug("max clock-frequency is at %u kHz\n", max_freq);
 	pr_debug("initializing frequency table\n");
 
-	// initialize frequency table
+	/* initialize frequency table */
 	for (i=0; cbe_freqs[i].frequency!=CPUFREQ_TABLE_END; i++) {
 		cbe_freqs[i].frequency = max_freq / cbe_freqs[i].index;
 		pr_debug("%d: %d\n", i, cbe_freqs[i].frequency);
@@ -235,10 +235,10 @@ static int cbe_cpufreq_cpu_init (struct 
 	policy->cpus = cpu_sibling_map[policy->cpu];
 #endif
 
-	cpufreq_frequency_table_get_attr (cbe_freqs, policy->cpu);
+	cpufreq_frequency_table_get_attr(cbe_freqs, policy->cpu);
 
 	/* this ensures that policy->cpuinfo_min and policy->cpuinfo_max are set correctly */
-	return cpufreq_frequency_table_cpuinfo (policy, cbe_freqs);
+	return cpufreq_frequency_table_cpuinfo(policy, cbe_freqs);
 }
 
 static int cbe_cpufreq_cpu_exit(struct cpufreq_policy *policy)
@@ -270,7 +270,7 @@ static int cbe_cpufreq_target(struct cpu
 	freqs.new = cbe_freqs[cbe_pmode_new].frequency;
 	freqs.cpu = policy->cpu;
 
-	mutex_lock (&cbe_switch_mutex);
+	mutex_lock(&cbe_switch_mutex);
 	cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
 
 	pr_debug("setting frequency for cpu %d to %d kHz, 1/%d of max frequency\n",
@@ -303,6 +303,9 @@ static int __init cbe_cpufreq_init(void)
 {
 	struct device_node *np;
 
+	if (!machine_is(cell))
+		return 0;
+
 	np = of_find_node_by_type(NULL, "ibm,pmi");
 
 	pmi_dev = of_find_device_by_node(np);
@@ -315,7 +318,7 @@ static int __init cbe_cpufreq_init(void)
 
 static void __exit cbe_cpufreq_exit(void)
 {
-	if(pmi_dev)
+	if (pmi_dev)
 		pmi_unregister_handler(pmi_dev, &cbe_pmi_handler);
 
 	cpufreq_unregister_driver(&cbe_cpufreq_driver);

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Segher Boessenkool]

>> You probably should map the SDC registers in the
>> platform startop code, and then have the various
>> platform drivers use that.  Maybe even do an
>> abstraction ;-)
>
> Yep, it's a good idea to do and I'll look at it for 2.6.23 time frame.

Oh I'm sure you can get that done in time for .22 still ;-)


Segher

Re: [Cbe-oss-dev] [PATCH] cell: cbe_cpufreq cleanup and crash fix

[Akinobu Mita]

On Thu, Apr 26, 2007 at 06:07:54PM -0500, Olof Johansson wrote:
> @@ -303,6 +303,9 @@ static int __init cbe_cpufreq_init(void)
>  {
>  	struct device_node *np;
>  
> +	if (!machine_is(cell))
> +		return 0;
> +

Please return -ENODEV to prevent loading module.

Otherwise it will cause crash when unloading the module
because of cpufreq_unregister_driver() with unregistered driver.

> @@ -315,7 +318,7 @@ static int __init cbe_cpufreq_init(void)
>  
>  static void __exit cbe_cpufreq_exit(void)
>  {
> -	if(pmi_dev)
> +	if (pmi_dev)
>  		pmi_unregister_handler(pmi_dev, &cbe_pmi_handler);
>  
>  	cpufreq_unregister_driver(&cbe_cpufreq_driver);

Re: [Cbe-oss-dev] [PATCH] cell: cbe_cpufreq cleanup and crash fix

[Olof Johansson]

On Fri, Apr 27, 2007 at 02:22:06PM +0900, Akinobu Mita wrote:
> On Thu, Apr 26, 2007 at 06:07:54PM -0500, Olof Johansson wrote:
> > @@ -303,6 +303,9 @@ static int __init cbe_cpufreq_init(void)
> >  {
> >  	struct device_node *np;
> >  
> > +	if (!machine_is(cell))
> > +		return 0;
> > +
> 
> Please return -ENODEV to prevent loading module.
> 
> Otherwise it will cause crash when unloading the module
> because of cpufreq_unregister_driver() with unregistered driver.

It won't crash due to the checks in cpufreq_unregister_driver(), but
it's still better to return failure.


Thanks,

-Olof

[PATCH v2] cell: cbe_cpufreq cleanup and crash fix

[Olof Johansson]

cbe_cpufreq cleanups:

* comment format
* whitespace
* don't init on non-cell platforms

Signed-off-by: Olof Johansson <olof@lixom.net>

Index: powerpc/arch/powerpc/platforms/cell/cbe_cpufreq.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/cell/cbe_cpufreq.c
+++ powerpc/arch/powerpc/platforms/cell/cbe_cpufreq.c
@@ -155,7 +155,7 @@ static int set_pmode_reg(int cpu, unsign
 }
 
 static int set_pmode(int cpu, unsigned int slow_mode) {
-	if(pmi_dev)
+	if (pmi_dev)
 		return set_pmode_pmi(cpu, slow_mode);
 	else
 		return set_pmode_reg(cpu, slow_mode);
@@ -167,7 +167,7 @@ static void cbe_cpufreq_handle_pmi(struc
 	u8 cpu;
 	u8 cbe_pmode_new;
 
-	BUG_ON (pmi_msg.type != PMI_TYPE_FREQ_CHANGE);
+	BUG_ON(pmi_msg.type != PMI_TYPE_FREQ_CHANGE);
 
 	cpu = cbe_node_to_cpu(pmi_msg.data1);
 	cbe_pmode_new = pmi_msg.data2;
@@ -191,7 +191,7 @@ static struct pmi_handler cbe_pmi_handle
  * cpufreq functions
  */
 
-static int cbe_cpufreq_cpu_init (struct cpufreq_policy *policy)
+static int cbe_cpufreq_cpu_init(struct cpufreq_policy *policy)
 {
 	const u32 *max_freqp;
 	u32 max_freq;
@@ -200,7 +200,7 @@ static int cbe_cpufreq_cpu_init (struct 
 
 	cpu = of_get_cpu_node(policy->cpu, NULL);
 
-	if(!cpu)
+	if (!cpu)
 		return -ENODEV;
 
 	pr_debug("init cpufreq on CPU %d\n", policy->cpu);
@@ -210,13 +210,13 @@ static int cbe_cpufreq_cpu_init (struct 
 	if (!max_freqp)
 		return -EINVAL;
 
-	// we need the freq in kHz
+	/* we need the freq in kHz */
 	max_freq = *max_freqp / 1000;
 
 	pr_debug("max clock-frequency is at %u kHz\n", max_freq);
 	pr_debug("initializing frequency table\n");
 
-	// initialize frequency table
+	/* initialize frequency table */
 	for (i=0; cbe_freqs[i].frequency!=CPUFREQ_TABLE_END; i++) {
 		cbe_freqs[i].frequency = max_freq / cbe_freqs[i].index;
 		pr_debug("%d: %d\n", i, cbe_freqs[i].frequency);
@@ -235,10 +235,10 @@ static int cbe_cpufreq_cpu_init (struct 
 	policy->cpus = cpu_sibling_map[policy->cpu];
 #endif
 
-	cpufreq_frequency_table_get_attr (cbe_freqs, policy->cpu);
+	cpufreq_frequency_table_get_attr(cbe_freqs, policy->cpu);
 
 	/* this ensures that policy->cpuinfo_min and policy->cpuinfo_max are set correctly */
-	return cpufreq_frequency_table_cpuinfo (policy, cbe_freqs);
+	return cpufreq_frequency_table_cpuinfo(policy, cbe_freqs);
 }
 
 static int cbe_cpufreq_cpu_exit(struct cpufreq_policy *policy)
@@ -270,7 +270,7 @@ static int cbe_cpufreq_target(struct cpu
 	freqs.new = cbe_freqs[cbe_pmode_new].frequency;
 	freqs.cpu = policy->cpu;
 
-	mutex_lock (&cbe_switch_mutex);
+	mutex_lock(&cbe_switch_mutex);
 	cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
 
 	pr_debug("setting frequency for cpu %d to %d kHz, 1/%d of max frequency\n",
@@ -303,6 +303,9 @@ static int __init cbe_cpufreq_init(void)
 {
 	struct device_node *np;
 
+	if (!machine_is(cell))
+		return -ENODEV;
+
 	np = of_find_node_by_type(NULL, "ibm,pmi");
 
 	pmi_dev = of_find_device_by_node(np);
@@ -315,7 +318,7 @@ static int __init cbe_cpufreq_init(void)
 
 static void __exit cbe_cpufreq_exit(void)
 {
-	if(pmi_dev)
+	if (pmi_dev)
 		pmi_unregister_handler(pmi_dev, &cbe_pmi_handler);
 
 	cpufreq_unregister_driver(&cbe_cpufreq_driver);

[PATCH v3] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

Cpufreq driver for PA Semi PWRficient processors.

Signed-off-by: Egor Martovetsky <egor@pasemi.com>
Signed-off-by: Olof Johansson <olof@lixom.net>

---

v3:
* use cpu_online_map instead of possible_map
* fix CPU_FREQ dependency
* return -ENODEV instead of 0 on unsupported platforms

v2:
* Attributed copyright correctly to cbe_cpufreq.c and adjust license
  to match (this was my mistake)
* Lookup the SDC and Gizmo device nodes to get register bases
* machine_is_compatible checks
* Cleanups as suggested by Arnd + misc whitespace

I chose not to do this as an of_platform driver since it doesn't fit
that well with the cpufreq driver model; having 3 levels of init/probe
functions is excessive.

Index: powerpc/arch/powerpc/platforms/pasemi/Makefile
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pasemi/Makefile
+++ powerpc/arch/powerpc/platforms/pasemi/Makefile
@@ -1,2 +1,3 @@
 obj-y	+= setup.o pci.o time.o idle.o powersave.o iommu.o
 obj-$(CONFIG_PPC_PASEMI_MDIO)	+= gpio_mdio.o
+obj-$(CONFIG_PPC_PASEMI_CPUFREQ) += cpufreq.o
Index: powerpc/arch/powerpc/platforms/pasemi/cpufreq.c
===================================================================
--- /dev/null
+++ powerpc/arch/powerpc/platforms/pasemi/cpufreq.c
@@ -0,0 +1,307 @@
+/*
+ * Copyright (C) 2007 PA Semi, Inc
+ *
+ * Authors: Egor Martovetsky <egor@pasemi.com>
+ *	    Olof Johansson <olof@lixom.net>
+ *
+ * Maintained by: Olof Johansson <olof@lixom.net>
+ *
+ * Based on arch/powerpc/platforms/cell/cbe_cpufreq.c:
+ * (C) Copyright IBM Deutschland Entwicklung GmbH 2005
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+
+#include <linux/cpufreq.h>
+#include <linux/timer.h>
+
+#include <asm/hw_irq.h>
+#include <asm/io.h>
+#include <asm/prom.h>
+
+#define SDCASR_REG		0x0100
+#define SDCASR_REG_STRIDE	0x1000
+#define SDCPWR_CFGA0_REG	0x0100
+#define SDCPWR_PWST0_REG	0x0000
+#define SDCPWR_GIZTIME_REG	0x0440
+
+/* SDCPWR_GIZTIME_REG fields */
+#define SDCPWR_GIZTIME_GR	0x80000000
+#define SDCPWR_GIZTIME_LONGLOCK	0x000000ff
+
+/* Offset of ASR registers from SDC base */
+#define SDCASR_OFFSET		0x120000
+
+static void __iomem *sdcpwr_mapbase;
+static void __iomem *sdcasr_mapbase;
+
+static DEFINE_MUTEX(pas_switch_mutex);
+
+/* Current astate, is used when waking up from power savings on
+ * one core, in case the other core has switched states during
+ * the idle time.
+ */
+static int current_astate;
+
+/* We support 5(A0-A4) power states excluding turbo(A5-A6) modes */
+static struct cpufreq_frequency_table pas_freqs[] = {
+	{0,	0},
+	{1,	0},
+	{2,	0},
+	{3,	0},
+	{4,	0},
+	{0,	CPUFREQ_TABLE_END},
+};
+
+static struct freq_attr *pas_cpu_freqs_attr[] = {
+	&cpufreq_freq_attr_scaling_available_freqs,
+	NULL,
+};
+
+/*
+ * hardware specific functions
+ */
+
+static int get_astate_freq(int astate)
+{
+	u32 ret;
+	ret = in_le32(sdcpwr_mapbase + SDCPWR_CFGA0_REG + (astate * 0x10));
+
+	return ret & 0x3f;
+}
+
+static int get_cur_astate(int cpu)
+{
+	u32 ret;
+
+	ret = in_le32(sdcpwr_mapbase + SDCPWR_PWST0_REG);
+	ret = (ret >> (cpu * 4)) & 0x7;
+
+	return ret;
+}
+
+static int get_gizmo_latency(void)
+{
+	u32 giztime, ret;
+
+	giztime = in_le32(sdcpwr_mapbase + SDCPWR_GIZTIME_REG);
+
+	/* just provide the upper bound */
+	if (giztime & SDCPWR_GIZTIME_GR)
+		ret = (giztime & SDCPWR_GIZTIME_LONGLOCK) * 128000;
+	else
+		ret = (giztime & SDCPWR_GIZTIME_LONGLOCK) * 1000;
+
+	return ret;
+}
+
+static void set_astate(int cpu, unsigned int astate)
+{
+	u64 flags;
+
+	/* Return if called before init has run */
+	if (unlikely(!sdcasr_mapbase))
+		return;
+
+	local_irq_save(flags);
+
+	out_le32(sdcasr_mapbase + SDCASR_REG + SDCASR_REG_STRIDE*cpu, astate);
+
+	local_irq_restore(flags);
+}
+
+void restore_astate(int cpu)
+{
+	set_astate(cpu, current_astate);
+}
+
+/*
+ * cpufreq functions
+ */
+
+static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
+{
+	u32 *max_freq;
+	int i, cur_astate;
+	struct resource res;
+	struct device_node *cpu, *dn;
+	int err = -ENODEV;
+
+	cpu = of_get_cpu_node(policy->cpu, NULL);
+
+	if (!cpu)
+		goto out;
+
+	dn = of_find_compatible_node(NULL, "sdc", "1682m-sdc");
+	if (!dn)
+		goto out;
+	err = of_address_to_resource(dn, 0, &res);
+	of_node_put(dn);
+	if (err)
+		goto out;
+	sdcasr_mapbase = ioremap(res.start + SDCASR_OFFSET, 0x2000);
+	if (!sdcasr_mapbase) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	dn = of_find_compatible_node(NULL, "gizmo", "1682m-gizmo");
+	if (!dn) {
+		err = -ENODEV;
+		goto out_unmap_sdcasr;
+	}
+	err = of_address_to_resource(dn, 0, &res);
+	of_node_put(dn);
+	if (err)
+		goto out_unmap_sdcasr;
+	sdcpwr_mapbase = ioremap(res.start, 0x1000);
+	if (!sdcpwr_mapbase) {
+		err = -EINVAL;
+		goto out_unmap_sdcasr;
+	}
+
+	pr_debug("init cpufreq on CPU %d\n", policy->cpu);
+
+	max_freq = (u32*) get_property(cpu, "clock-frequency", NULL);
+	if (!max_freq) {
+		err = -EINVAL;
+		goto out_unmap_sdcpwr;
+	}
+
+	/* we need the freq in kHz */
+	*max_freq /= 1000;
+
+	pr_debug("max clock-frequency is at %u kHz\n", *max_freq);
+	pr_debug("initializing frequency table\n");
+
+	/* initialize frequency table */
+	for (i=0; pas_freqs[i].frequency!=CPUFREQ_TABLE_END; i++) {
+		pas_freqs[i].frequency = get_astate_freq(pas_freqs[i].index) * 100000;
+		pr_debug("%d: %d\n", i, pas_freqs[i].frequency);
+	}
+
+	policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
+
+	policy->cpuinfo.transition_latency = get_gizmo_latency();
+
+	cur_astate = get_cur_astate(policy->cpu);
+	pr_debug("current astate is at %d\n",cur_astate);
+
+	policy->cur = pas_freqs[cur_astate].frequency;
+	policy->cpus = cpu_online_map;
+
+	cpufreq_frequency_table_get_attr(pas_freqs, policy->cpu);
+
+	/* this ensures that policy->cpuinfo_min and policy->cpuinfo_max
+	 * are set correctly
+	 */
+	return cpufreq_frequency_table_cpuinfo(policy, pas_freqs);
+
+out_unmap_sdcpwr:
+	iounmap(sdcpwr_mapbase);
+
+out_unmap_sdcasr:
+	iounmap(sdcasr_mapbase);
+out:
+	return err;
+}
+
+static int pas_cpufreq_cpu_exit(struct cpufreq_policy *policy)
+{
+	if (sdcasr_mapbase)
+		iounmap(sdcasr_mapbase);
+	if (sdcpwr_mapbase)
+		iounmap(sdcpwr_mapbase);
+
+	cpufreq_frequency_table_put_attr(policy->cpu);
+	return 0;
+}
+
+static int pas_cpufreq_verify(struct cpufreq_policy *policy)
+{
+	return cpufreq_frequency_table_verify(policy, pas_freqs);
+}
+
+static int pas_cpufreq_target(struct cpufreq_policy *policy,
+			      unsigned int target_freq,
+			      unsigned int relation)
+{
+	struct cpufreq_freqs freqs;
+	int pas_astate_new;
+	int i;
+
+	cpufreq_frequency_table_target(policy,
+				       pas_freqs,
+				       target_freq,
+				       relation,
+				       &pas_astate_new);
+
+	freqs.old = policy->cur;
+	freqs.new = pas_freqs[pas_astate_new].frequency;
+	freqs.cpu = policy->cpu;
+
+	mutex_lock(&pas_switch_mutex);
+	cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+
+	pr_debug("setting frequency for cpu %d to %d kHz, 1/%d of max frequency\n",
+		 policy->cpu,
+		 pas_freqs[pas_astate_new].frequency,
+		 pas_freqs[pas_astate_new].index);
+
+	current_astate = pas_astate_new;
+
+	for_each_online_cpu(i)
+		set_astate(i, pas_astate_new);
+
+	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	mutex_unlock(&pas_switch_mutex);
+
+	return 0;
+}
+
+static struct cpufreq_driver pas_cpufreq_driver = {
+	.name		= "pas-cpufreq",
+	.owner		= THIS_MODULE,
+	.flags		= CPUFREQ_CONST_LOOPS,
+	.init		= pas_cpufreq_cpu_init,
+	.exit		= pas_cpufreq_cpu_exit,
+	.verify		= pas_cpufreq_verify,
+	.target		= pas_cpufreq_target,
+	.attr		= pas_cpu_freqs_attr,
+};
+
+/*
+ * module init and destoy
+ */
+
+static int __init pas_cpufreq_init(void)
+{
+	if (!machine_is_compatible("PA6T-1682M"))
+		return -ENODEV;
+
+	return cpufreq_register_driver(&pas_cpufreq_driver);
+}
+
+static void __exit pas_cpufreq_exit(void)
+{
+	cpufreq_unregister_driver(&pas_cpufreq_driver);
+}
+
+module_init(pas_cpufreq_init);
+module_exit(pas_cpufreq_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Egor Martovetsky <egor@pasemi.com>, Olof Johansson <olof@lixom.net>");
Index: powerpc/arch/powerpc/platforms/pasemi/idle.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pasemi/idle.c
+++ powerpc/arch/powerpc/platforms/pasemi/idle.c
@@ -61,6 +61,10 @@ static int pasemi_system_reset_exception
 		/* do system reset */
 		return 0;
 	}
+
+	/* Set higher astate since we come out of power savings at 0 */
+	restore_astate(hard_smp_processor_id());
+
 	/* everything handled */
 	regs->msr |= MSR_RI;
 	return 1;
@@ -68,6 +72,11 @@ static int pasemi_system_reset_exception
 
 void __init pasemi_idle_init(void)
 {
+#ifndef CONFIG_PPC_PASEMI_CPUFREQ
+	printk(KERN_WARNING "No cpufreq driver, powersavings modes disabled\n");
+	current_mode = 0;
+#endif
+
 	ppc_md.system_reset_exception = pasemi_system_reset_exception;
 	ppc_md.power_save = modes[current_mode].entry;
 	printk(KERN_INFO "Using PA6T idle loop (%s)\n", modes[current_mode].name);
Index: powerpc/arch/powerpc/platforms/pasemi/pasemi.h
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pasemi/pasemi.h
+++ powerpc/arch/powerpc/platforms/pasemi/pasemi.h
@@ -14,6 +14,14 @@ extern void __init pasemi_idle_init(void
 extern void idle_spin(void);
 extern void idle_doze(void);
 
+/* Restore astate to last set */
+#ifdef CONFIG_PPC_PASEMI_CPUFREQ
+extern void restore_astate(int cpu);
+#else
+static inline void restore_astate(int cpu)
+{
+}
+#endif
 
 
 #endif /* _PASEMI_PASEMI_H */
Index: powerpc/arch/powerpc/platforms/Kconfig
===================================================================
--- powerpc.orig/arch/powerpc/platforms/Kconfig
+++ powerpc/arch/powerpc/platforms/Kconfig
@@ -169,6 +169,16 @@ config CPU_FREQ_PMAC64
 	help
 	  This adds support for frequency switching on Apple iMac G5,
 	  and some of the more recent desktop G5 machines as well.
+
+config PPC_PASEMI_CPUFREQ
+	bool "Support for PA Semi PWRficient"
+	depends on CPU_FREQ && PPC_PASEMI
+	default y
+	select CPU_FREQ_TABLE
+	help
+	  This adds the support for frequency switching on PA Semi
+	  PWRficient processors.
+
 endmenu
 
 config PPC601_SYNC_FIX

Re: [PATCH v2] cell: cbe_cpufreq cleanup and crash fix

[Christian Krafft]

[-- Attachment #1: Type: text/plain, Size: 633 bytes --]

On Fri, 27 Apr 2007 00:33:47 -0500
olof@lixom.net (Olof Johansson) wrote:

> cbe_cpufreq cleanups:
> 
> * comment format
> * whitespace
> * don't init on non-cell platforms
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

Acked-by: Christian Krafft <krafft@de.ibm.com>

Thx for fixing ;-)

-- 
Mit freundlichen Gruessen,
kind regards,

Christian Krafft
IBM Systems & Technology Group,
Linux Kernel Development
IT Specialist


Vorsitzender des Aufsichtsrats:	Johann Weihen
Geschaeftsfuehrung:		Herbert Kircher
Sitz der Gesellschaft:		Boeblingen
Registriergericht:		Amtsgericht Stuttgart, HRB 243294


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 899 bytes --]

On Thu, 2007-04-26 at 15:37 -0500, Olof Johansson wrote:

> You can set the requested speed per-processor, but the actual speed
> will be the max of them. Because of this, it's less surprising to set
> it on both at the same time since then you'll at least know what you're
> running at.
> 
> So yes, I'll change it to online_map instead of possible_map.

Ok, yeah, so effectively they all just go together (that additional
complication of running at the max probably should be hidden as you
note)

> Hmm, I just tried adding debugging to the cpu_init code, and I'm not
> seeing it called more than once (i.e. only for cpu 0). Mind you, I don't
> have cpu hotplug support at this time.

Oh right, you need cpu hotplug support for the bug to show up. What
happens then is you unplug and replug a CPU and the sysfs cpufreq stuff
for the CPU you just replugged is gone.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

On Fri, Apr 27, 2007 at 11:40:03AM +0200, Johannes Berg wrote:

> > Hmm, I just tried adding debugging to the cpu_init code, and I'm not
> > seeing it called more than once (i.e. only for cpu 0). Mind you, I don't
> > have cpu hotplug support at this time.
> 
> Oh right, you need cpu hotplug support for the bug to show up. What
> happens then is you unplug and replug a CPU and the sysfs cpufreq stuff
> for the CPU you just replugged is gone.

Ok. Nothing for me to worry about right now then, but it's good to keep
in mind for when the time comes.


Thanks,

-Olof

Re: [PATCH v3] [2.6.22] pasemi: cpufreq driver

[Dominik Brodowski]

Hi,

On Fri, Apr 27, 2007 at 12:46:01AM -0500, Olof Johansson wrote:
> +	max_freq = (u32*) get_property(cpu, "clock-frequency", NULL);

(u32) or (32*) ?

> +	cur_astate = get_cur_astate(policy->cpu);

May the different cores have different settings at initalization? e.g.

core 0:		freq A
core 1:		freq B

with (freq B > freq A)? If so, cur_astate is set wrongly, as the _effective_
frequency would be freq B, right?

> +#ifndef CONFIG_PPC_PASEMI_CPUFREQ
> +	printk(KERN_WARNING "No cpufreq driver, powersavings modes disabled\n");
> +	current_mode = 0;
> +#endif
> +

This confuses me a bit -- does something else than cpufreq not work if
cpufreq is disabled?


Overall, the patch looks good to me.

	Dominik

Re: [PATCH v3] [2.6.22] pasemi: cpufreq driver

[Stephen Rothwell]

On Sat, 28 Apr 2007 22:50:21 -0400 Dominik Brodowski <linux@dominikbrodowski.net> wrote:
>
> Hi,
> 
> On Fri, Apr 27, 2007 at 12:46:01AM -0500, Olof Johansson wrote:
> > +	max_freq = (u32*) get_property(cpu, "clock-frequency", NULL);
> 
> (u32) or (32*) ?

Neither, get_property now returns (void *) so casting is not needed or
wanted.

Also, later in the patch, you modify the property value in place.  Do not do this. To help you remember, if you had not done the cast, gcc would have complained because get_property return a const void *.

Also, get_property is now called of_get_property.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

Re: [PATCH v3] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

On Sun, Apr 29, 2007 at 01:40:59PM +1000, Stephen Rothwell wrote:
> On Sat, 28 Apr 2007 22:50:21 -0400 Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> >
> > Hi,
> > 
> > On Fri, Apr 27, 2007 at 12:46:01AM -0500, Olof Johansson wrote:
> > > +	max_freq = (u32*) get_property(cpu, "clock-frequency", NULL);
> > 
> > (u32) or (32*) ?
> 
> Neither, get_property now returns (void *) so casting is not needed or
> wanted.
> 
> Also, later in the patch, you modify the property value in place.  Do not do this. To help you remember, if you had not done the cast, gcc would have complained because get_property return a const void *.

Yep. It came along over from the cbe_cpufreq driver, which also does
it. I'll post a patch tomorrow or Monday unless someone else beats me
to it (the driver has already been merged).

> Also, get_property is now called of_get_property.

Sure, but keeping up with the API churn during other patch merging is a mess. It's easier to
make one final pass over all new code and fix it up once more later during the merge window.


-Olof

Re: [PATCH v3] [2.6.22] pasemi: cpufreq driver

[Olof Johansson]

On Sat, Apr 28, 2007 at 10:50:21PM -0400, Dominik Brodowski wrote:
> Hi,
> 
> On Fri, Apr 27, 2007 at 12:46:01AM -0500, Olof Johansson wrote:
> > +	max_freq = (u32*) get_property(cpu, "clock-frequency", NULL);
> 
> (u32) or (32*) ?
> 
> > +	cur_astate = get_cur_astate(policy->cpu);
> 
> May the different cores have different settings at initalization? e.g.
> 
> core 0:		freq A
> core 1:		freq B
> 
> with (freq B > freq A)? If so, cur_astate is set wrongly, as the _effective_
> frequency would be freq B, right?

Firmware normally sets only cpu 0 (since cpu 1 isn't started until after
linux is loaded and started). So it'll works well by default.

> > +#ifndef CONFIG_PPC_PASEMI_CPUFREQ
> > +	printk(KERN_WARNING "No cpufreq driver, powersavings modes disabled\n");
> > +	current_mode = 0;
> > +#endif
> > +
> 
> This confuses me a bit -- does something else than cpufreq not work if
> cpufreq is disabled?

After doze, we come back out in astate 0, so we need to raise it back
up. We do so by calling the restore_astate() function. If we don't have
it, and use idle=doze, we'll end up with a cpu running at astate 0 and
no way to raise it.

> Overall, the patch looks good to me.


Thanks,

-Olof