I can only think of three ifdef's that would be necessary now but it could
grow. If the #ifdef snarl is unattractive in idle.c it's easy enough to
move it to chipfamily-specific headers so that idle.c just needs to call
arch_idle() to enter an idle state.
The function pointer isn't desirable. What the correct strategy for power
saving is known at compile time so there shouldn't be a function pointer
dereference. How the #ifdef's are done doesn't really matter as long as
the inefficiency of a function pointer is avoided.
} I thought one of the linuxppc desgin goals was to keep the ifdefs to a
} minimum. I can see idle.c growing quite large and full of #ifdefs if we
} do it that way. Rather than using ppc_md, make power_save an
} abstraction similar to platform_init.
}
} >
} >
} >} This sounds like a good idea if we could use
} >} if( ppc_md.powersave != NULL)
} >} ppc_md.powersave();
} >}
} >} If it is determined that calling power_save() which is resides in an
} >} arch/processor specific file then we are talking about many files being
} >} hit. and the current power_save seems to common for many other ppc
} >} platforms other than 4xx & 8xx
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
I can only think of three ifdef's that would be necessary now but it could
grow. If the #ifdef snarl is unattractive in idle.c it's easy enough to
move it to chipfamily-specific headers so that idle.c just needs to call
arch_idle() to enter an idle state.
The function pointer isn't desirable. What the correct strategy for power
saving is known at compile time so there shouldn't be a function pointer
dereference. How the #ifdef's are done doesn't really matter as long as
the inefficiency of a function pointer is avoided.
Well, while I tend to agree with you on this, experience proved that
slightly abusing the ppc_md. indirection somewhat helped make the
code cleaner (read: more self-contained, less cruft, ...)
Also, in this specific case, we might well want to have an machine
specific power saving feature: I've had various tweaks in mind for
powermac laptops that I never ended up implementing...
ben.
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Benjamin Herrenschmidt wrote:
Also, in this specific case, we might well want to have an machine
specific power saving feature: I've had various tweaks in mind for
powermac laptops that I never ended up implementing...
This will also be true for the IBM 405LP (starting with the "Beech" eval
board), which has a number of new PM features (
http://www.research.ibm.com/arl/projects/papers/405LP.pdf ). IBM has
internally used a pm_idle function pointer a la i386, hoping that would
be the community-accepted way, in case ya wanna add that to the
suggestion pile, but I think any of the methods proposed would be fine.
--
Todd
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Benjamin Herrenschmidt wrote:
Well, while I tend to agree with you on this, experience proved that
slightly abusing the ppc_md. indirection somewhat helped make the
code cleaner (read: more self-contained, less cruft, ...)
All of the architectures except PowerPC seem to have a indirect
pointer to a power saving idle function from the idle loop. If
you don't want to follow this, we could have all of the board
specific files contain a 'power_save()' function, which could be
empty, always compile it and always call it. Today, the power
saving stuff is all 6xx/7xx/Mac specific, which kinda needs to
change if we want address the needs of embedded processors and
products.
-- Dan
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Dan Malek wrote:
Benjamin Herrenschmidt wrote:
quoted
Well, while I tend to agree with you on this, experience proved that
slightly abusing the ppc_md. indirection somewhat helped make the
code cleaner (read: more self-contained, less cruft, ...)
All of the architectures except PowerPC seem to have a indirect
pointer to a power saving idle function from the idle loop. If
you don't want to follow this, we could have all of the board
specific files contain a 'power_save()' function, which could be
empty, always compile it and always call it. Today, the power
saving stuff is all 6xx/7xx/Mac specific, which kinda needs to
change if we want address the needs of embedded processors and
products.
-- Dan
Here is what I think might work. I am borrowing the idea from i386 &
Arm. This will allow greater flexibilty for thos who need it. I have
an example for both a 4xx impilmentation and what would be needed in idle.c.
What do think :)
armin
diff -Nru a/arch/ppc/kernel/idle.c b/arch/ppc/kernel/idle.c
--- a/arch/ppc/kernel/idle.c Sat Jul 27 09:24:06 2002
+++ b/arch/ppc/kernel/idle.c Sat Jul 27 09:24:06 2002
@@ -1,5 +1,5 @@
/*
- * BK Id: %F% %I% %G% %U% %#%
+ * BK Id: SCCS/s.idle.c 1.31 04/16/02 21:42:08 paulus
*/
/*
* Idle daemon for PowerPC. Idle daemon will handle any action
@@ -11,6 +11,11 @@
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version
* 2 of the License, or (at your option) any later version.
+ *
+ * 07/27/02 - Armin
+ * added powersave idel loop indirection scheme borrowed from
+ * i386 & Arm so other ppc archs can have their own if the
+ * default is not sufficiant.
*/
#include <linux/config.h>
#include <linux/errno.h>
@@ -50,6 +55,8 @@
void zero_paged(void);
void power_save(void);
+void (*pm_idle)(void);
+
unsigned long zero_paged_on = 0;
unsigned long powersave_nap = 0;
}
}
#endif
-
if (do_power_save && !current->need_resched)
-
power_save();
+
void (*idle)(void) = pm_idle;
+
if (!idle)
+
idle = power_save;
+
if (do_power_save && !current->need_resched)
+
idle();
if (current->need_resched) {
run_light_on(1);
schedule();
diff -Nru a/arch/ppc/kernel/ppc4xx_setup.c b/arch/ppc/kernel/ppc4xx_setup.c--- a/arch/ppc/kernel/ppc4xx_setup.c Sat Jul 27 09:24:06 2002
+++ b/arch/ppc/kernel/ppc4xx_setup.c Sat Jul 27 09:24:06 2002
@@ -27,6 +27,9 @@
* History: 04/18/02 - Armin
* added ash to setting CETE bit in calibrate()
*
+ * : 07/27/02 - Armin
+ * Added powersave idle loop
+ *
*/
#include <linux/config.h>
@@ -60,6 +63,7 @@
/* Function Prototypes */
static void ppc4xx_gdb_init(void);
+static void arch_power_save(void);
extern void abort(void);
extern void ppc4xx_find_bridges(void);
@@ -87,6 +91,7 @@
extern void board_io_mapping(void);
extern void board_setup_irq(void);
extern void board_init(void);
+extern void (*pm_idle) (void);
/* Global Variables */
unsigned char __res[sizeof (bd_t)];
@@ -94,7 +99,7 @@
static void __init
ppc4xx_setup_arch(void)
{
-
+
/* Setup PCI host bridges */
#ifdef CONFIG_PCI@@ -110,6 +115,9 @@
board_setup_arch();
ppc4xx_gdb_init();
+
+
pm_idle = arch_power_save;
+
}
/*
@@ -454,4 +462,38 @@
board_init();
return;
+}
+
+void arch_power_save(void)
+{
+extern void (*pm_idle) (void);
+
extern unsigned long powersave_nap;
+
int nap = powersave_nap;
+
+
pm_idle = arch_power_save;
+
+
if (!(nap || (cur_cpu_spec[smp_processor_id()]->cpu_features &
CPU_FTR_CAN_DOZE)))
+
return;
+
/*
+
* Disable interrupts to prevent a lost wakeup
+
* when going to sleep. This is necessary even with
+
* RTLinux since we are not guaranteed an interrupt
+
* didn't come in and is waiting for a __sti() before
+
* emulating one. This way, we really do hard disable.
+
*
+
* We assume that we're sti-ed when we come in here. We
+
* are in the idle loop so if we're cli-ed then it's a bug
+
* anyway.
+
* -- Cort
+
*/
+
_nmask_and_or_msr(MSR_EE, 0);
+
if (!current->need_resched)
+
+
/* set the POW bit in the MSR, and enable interrupts
+
* so we wake up sometime! */
+
_nmask_and_or_msr(0, MSR_POW | MSR_EE);
+
+
_nmask_and_or_msr(0, MSR_EE);
+
+
pm_idle = arch_power_save;
}
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/