On 6/29/11, Benjamin Herrenschmidt [off-list ref] wrote:

Before I comment on this last one, a quick Q. for Dave: Do you want to
handle this or should I merge it via powerpc.git ? (It depends on
another change to the arch code to expose the SCOM functions that it
uses, and that patch is going to be in my -next branch).

Now some remaining small nits:

On Fri, 2011-06-17 at 17:10 +0400, Dmitry Eremin-Solenikov wrote:

quoted

Add simple cpufreq driver for Maple-based boards (ppc970fx evaluation
kit and others). Driver is based on a cpufreq driver for 64-bit powermac
boxes with all pmac-dependant features removed and simple cleanup
applied.

Signed-off-by: Dmitry Eremin-Solenikov <redacted>
---
 drivers/cpufreq/Kconfig         |    5 +
 drivers/cpufreq/Kconfig.powerpc |    7 +
 drivers/cpufreq/Makefile        |    5 +
 drivers/cpufreq/maple-cpufreq.c |  314
+++++++++++++++++++++++++++++++++++++++

If we're going to have a Kconfig.powerpc, should we maybe just have a
powerpc subdirectory instead with the driver in it ?

I'm happy at some later point to try moving some of my other ones there.

As Dave also isn't sure about subdirs, should I create cpufreq/powerpc
directory,
or not?

 .../...

quoted

+	/* Look for the powertune data in the device-tree */
+	maple_pmode_data = of_get_property(cpunode, "power-mode-data", &psize);
+	if (!maple_pmode_data) {
+		DBG("No power-mode-data !\n");
+		goto bail_noprops;
+	}
+	maple_pmode_max = psize / sizeof(u32) - 1;

Do you get that property in your device-tree ? Or have you modified your
firmware ? If that requires a modified firmware, you should probably put
at least a link indicating where to get it somewhere and display a nicer
error code.

PIBS firmware (used on PPC970FX devkit/original Maple-D board) generates
this property, if the board is started with dual CPUs (it can also be started
with only one CPU selected). On the other hand SLOF firmware (used
on JS2x blade servers) doesn't generate this property. It can be adapted
however to generate it.

Also this driver is specific to the Maple HW, you don't want it to kick
in and mess around on ... an Apple G5 for example. So stick somewhere a

	if (!machine_is(maple))
		return 0;

quoted

+	printk(KERN_INFO "Registering G5 CPU frequency driver\n");

s/G5/Maple

Hmmm. I'm actually thinking about doing it the other way: as this driver
is mostly c&p of PowerMac G5 driver, as we are moving those from
arch/powerpc to drivers/cpufreq, maybe I should merge two drivers (this
one with cpufreq_64 from powermac)?

quoted

+	printk(KERN_INFO "Frequency method: SCOM, Voltage method: none\n");

This is useless.

Leftover from powermac thing.

-- 
With best wishes
Dmitry