* Re: [RFC] Clock binding
From: Stuart Yoder @ 2009-08-28 12:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list, devicetree-discuss
In-Reply-To: <1251427388.20467.122.camel@pasglop>
On Thu, Aug 27, 2009 at 9:43 PM, Benjamin
Herrenschmidt<benh@kernel.crashing.org> wrote:
> On Thu, 2009-08-27 at 16:36 -1000, Mitch Bradley wrote:
>> The idea of a wiki as a registration authority is a good one, but I'm
>> not volunteering to maintain it :-)
>
> here goes my hope :-)
>
> Do we have wiki's we could use on power.org or should we aim for a
> community place ? Anybody has suggestions ? I wouldn't even try to
> maintain a web site myself, that would be irresponsible of anybody to
> let me do so !
There was an ePAPR wiki on power.org, that I had started developing,
but it seems to have disappeared. I'm looking into what happened.
Stuart
^ permalink raw reply
* MPC866 FEC's Receive processing thru pre allocated buffers
From: Ganesh Kumar @ 2009-08-28 12:04 UTC (permalink / raw)
To: linuxppc-dev
Hi All,
I've already sent this almost before 6-7 hours, but the
mail did not appear on the Aug 2009 archives, So I'm sending
it again. Sorry for this!!. Thanks in advance.
=A0 =A0 =A0 =A0 I'm working on MPC860 with Linux Kernel 2.4.18.
As I'm fine tuning the FEC(Fast Ethernet Controller) driver,
I came across the receive side processing of the ethernet frames
where in the Rx BD rings are preallocated with the buffers and each time
a new frame is received, the whole frame will get copied from the Buffer
Descriptors to the external memory by allocating the skb.
Is this the right way to do that ?, as memcpy is not efficient inside the
ISRs.
So I did some changes in the RX BDs initialization, like allocate the skb
and initialize the BD's address pointer with the skb->data(using __pa)
and then on reception of the frame I take out the skb from theBD and alloca=
te
a new skb and reinit the BD address with the newly allocated skb->data.
It works for normal conditions, but if I load the driver then
I receive lots of corrupted frames, So I tried increasing the RX_RING_SIZE(=
16)
and also enabling the receive dscriptor active only after I come out of the=
=20
while loop (inside fec_enet_rx)=20
Increasing the Rx ring eliminated the frame corruption and runs fine on load
test.
But if I configure my Linux box in bridge mode then it doesn't work,
i.e., the bridging doesn't happen,
=A0 =A0PC-1 ---->eth0 =A0[Bridge machine] eth1 ----> PC-2
What I mean here is if we initiate a ping from the=20
PC-1 to PC-2, I don't get any response,
it continously try to resole the ARP.
What may be the reason??
Thanks in advance
Please do CC to me as I'm not subscribed to this list.
=2D-GK
^ permalink raw reply
* Re: [RFC] Clock binding
From: David Gibson @ 2009-08-28 11:23 UTC (permalink / raw)
To: Josh Boyer; +Cc: devicetree-discuss, linuxppc-dev list
In-Reply-To: <20090828105837.GC25303@zod.rchland.ibm.com>
On Fri, Aug 28, 2009 at 06:58:37AM -0400, Josh Boyer wrote:
> On Fri, Aug 28, 2009 at 12:43:08PM +1000, Benjamin Herrenschmidt wrote:
> >On Thu, 2009-08-27 at 16:36 -1000, Mitch Bradley wrote:
> >> The idea of a wiki as a registration authority is a good one, but I'm
> >> not volunteering to maintain it :-)
> >
> >here goes my hope :-)
> >
> >Do we have wiki's we could use on power.org or should we aim for a
> >community place ? Anybody has suggestions ? I wouldn't even try to
> >maintain a web site myself, that would be irresponsible of anybody to
> >let me do so !
>
> This is about the 3rd or 4th time this idea has come up over the past couple
> of years. Maybe this time it will stick?
There actually was one set up on power.org, for epapr bindings. I'm
still digging around to try to relocate the address, though.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* RE: [PATCH v2] powerpc/85xx: Add eSDHC support for MPC8536DS boards
From: Hu Mingkai-B21284 @ 2009-08-28 11:02 UTC (permalink / raw)
To: avorontsov, Kumar Gala
Cc: Ben Dooks, linux-kernel, sdhci-devel, linuxppc-dev, Andrew Morton,
Pierre Ossman, David Vrabel
In-Reply-To: <20090819015101.GA10782@oksana.dev.rtsoft.ru>
=20
> -----Original Message-----
> From:=20
> linuxppc-dev-bounces+b21284=3Dfreescale.com@lists.ozlabs.org=20
> [mailto:linuxppc-dev-bounces+b21284=3Dfreescale.com@lists.ozlabs
> .org] On Behalf Of Anton Vorontsov
> Sent: Wednesday, August 19, 2009 9:51 AM
> To: Kumar Gala
> Cc: Ben Dooks; linux-kernel@vger.kernel.org;=20
> sdhci-devel@lists.ossman.eu; linuxppc-dev@ozlabs.org; Andrew=20
> Morton; Pierre Ossman; David Vrabel
> Subject: Re: [PATCH v2] powerpc/85xx: Add eSDHC support for=20
> MPC8536DS boards
>=20
> On Tue, Aug 18, 2009 at 08:24:17PM -0500, Kumar Gala wrote:
> >=20
> > On Aug 18, 2009, at 6:38 PM, Anton Vorontsov wrote:
> >=20
> > >This patch simply adds sdhci node to the device tree.
> > >
> > >We specify clock-frequency manually, so that eSDHC will=20
> work without=20
> > >upgrading U-Boot. Though, that'll only work for default setup (1500
> > >MHz) on new board revisions. For non-default setups, it's=20
> recommended=20
> > >to upgrade U-Boot, since it will fixup clock-frequency=20
> automatically.
> > >
> > >Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> >=20
> > out of interest the 85xx eSDHC don't need the sdhci,wp-inverted=20
> > property?
>=20
> Yes, eSDHC controllers in MPC85xx report normal state in its=20
> registers.
>=20
Hi Anton,
The eSDHC controller in different silicon version on MPC8536 reports
different WP state in the register PRSSTAT:
Silicon 1.0:
Card WP pos PRSSTAT[WPSPL] RMMCR[SDHC_WP] GENCFG[SDHC_WP_INV]
------------------------------------------------------------------------
---------------------------------------------
unLock 1 1
/
Lock 0 1
/
Silicon 1.1:
Card WP pos PRSSTAT[WPSPL] RMMCR[SDHC_WP] GENCFG[SDHC_WP_INV]
------------------------------------------------------------------------
---------------------------------------------
unLock 0 1
0
Lock 1 1
0
Note: the register GENCFG is added on silicon 1.1 to invert the WP
state.
For silicon 1.0, the macro SDHCI_QUIRK_INVERTED_WRITE_PROTECT is also
defined,
so the dirver will report the error WP state in function sdhci_get_ro.
Best regards,
Mingkai
^ permalink raw reply
* Re: [RFC] Clock binding
From: Josh Boyer @ 2009-08-28 10:58 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Mitch Bradley, linuxppc-dev list, devicetree-discuss
In-Reply-To: <1251427388.20467.122.camel@pasglop>
On Fri, Aug 28, 2009 at 12:43:08PM +1000, Benjamin Herrenschmidt wrote:
>On Thu, 2009-08-27 at 16:36 -1000, Mitch Bradley wrote:
>> The idea of a wiki as a registration authority is a good one, but I'm
>> not volunteering to maintain it :-)
>
>here goes my hope :-)
>
>Do we have wiki's we could use on power.org or should we aim for a
>community place ? Anybody has suggestions ? I wouldn't even try to
>maintain a web site myself, that would be irresponsible of anybody to
>let me do so !
This is about the 3rd or 4th time this idea has come up over the past couple
of years. Maybe this time it will stick?
Btw, you probably want some kind of contributor agreement so that whatever
is posted under the wiki is freely usable by others.
josh
^ permalink raw reply
* [PATCH v2 2/2] cpu: Implement cpu-offline-state driver for pSeries.
From: Gautham R Shenoy @ 2009-08-28 10:00 UTC (permalink / raw)
To: Joel Schopp, Benjamin Herrenschmidt, Vaidyanathan Srinivasan,
Peter Zijlstra, Dipankar Sarma
Cc: Darrick J. Wong, linuxppc-dev, linux-kernel, Venkatesh Pallipadi
In-Reply-To: <20090828095741.10641.32053.stgit@sofia.in.ibm.com>
This patch implements the callbacks to handle the reads/writes
into the sysfs interfaces
/sys/devices/system/cpu/cpu<number>/available_hotplug_states
and
/sys/devices/system/cpu/cpu<number>/current_state
Currently, the patch defines two states which the processor can go to when it
is offlined. They are
- deallocate: This is the the default behaviour when the cpu is offlined even
in the absense of this driver.
The CPU would call make an rtas_stop_self() call and hand over the
CPU back to the resource pool, thereby effectively deallocating
that vCPU from the LPAR. This would result in a configuration change to the
LPAR which is visible to the outside world.
- deactivate: This cedes the vCPU to the hypervisor which in turn can put the
vCPU time to the best use. This option does not result in a configuration
change and the vCPU would be still entitled to the LPAR to which it earlier
belong to.
Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
---
arch/powerpc/platforms/pseries/Makefile | 2
arch/powerpc/platforms/pseries/hotplug-cpu.c | 76 ++++++++++-
arch/powerpc/platforms/pseries/offline_driver.c | 161 +++++++++++++++++++++++
arch/powerpc/platforms/pseries/offline_driver.h | 20 +++
arch/powerpc/platforms/pseries/smp.c | 17 ++
5 files changed, 268 insertions(+), 8 deletions(-)
create mode 100644 arch/powerpc/platforms/pseries/offline_driver.c
create mode 100644 arch/powerpc/platforms/pseries/offline_driver.h
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 790c0b8..3a569c7 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -17,7 +17,7 @@ obj-$(CONFIG_KEXEC) += kexec.o
obj-$(CONFIG_PCI) += pci.o pci_dlpar.o
obj-$(CONFIG_PSERIES_MSI) += msi.o
-obj-$(CONFIG_HOTPLUG_CPU) += hotplug-cpu.o
+obj-$(CONFIG_HOTPLUG_CPU) += hotplug-cpu.o offline_driver.o
obj-$(CONFIG_MEMORY_HOTPLUG) += hotplug-memory.o
obj-$(CONFIG_HVC_CONSOLE) += hvconsole.o
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index a20ead8..6880a1d 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -30,6 +30,7 @@
#include <asm/pSeries_reconfig.h>
#include "xics.h"
#include "plpar_wrappers.h"
+#include "offline_driver.h"
/* This version can't take the spinlock, because it never returns */
static struct rtas_args rtas_stop_self_args = {
@@ -54,13 +55,62 @@ static void rtas_stop_self(void)
panic("Alas, I survived.\n");
}
+static void cede_on_offline(void)
+{
+ unsigned int cpu = smp_processor_id();
+ unsigned int hwcpu = hard_smp_processor_id();
+
+ get_lppaca()->idle = 1;
+ if (!get_lppaca()->shared_proc)
+ get_lppaca()->donate_dedicated_cpu = 1;
+
+ printk(KERN_INFO "cpu %u (hwid %u) ceding for offline with hint %d\n",
+ cpu, hwcpu, cede_latency_hint);
+ while (get_preferred_offline_state(cpu) != CPU_STATE_ONLINE) {
+ cede_processor();
+ printk(KERN_INFO "cpu %u (hwid %u) returned from cede.\n",
+ cpu, hwcpu);
+ }
+
+ printk(KERN_INFO "cpu %u (hwid %u) got prodded to go online\n",
+ cpu, hwcpu);
+
+ if (!get_lppaca()->shared_proc)
+ get_lppaca()->donate_dedicated_cpu = 0;
+ get_lppaca()->idle = 0;
+ unregister_slb_shadow(hwcpu, __pa(get_slb_shadow()));
+
+ /*
+ * NOTE: Calling start_secondary() here for now to start
+ * a new context.
+ *
+ * However, need to do it cleanly by resetting the stack
+ * pointer.
+ */
+ start_secondary();
+}
+
static void pseries_mach_cpu_die(void)
{
+ unsigned int cpu = smp_processor_id();
+
local_irq_disable();
idle_task_exit();
xics_teardown_cpu();
- unregister_slb_shadow(hard_smp_processor_id(), __pa(get_slb_shadow()));
- rtas_stop_self();
+
+ if (get_preferred_offline_state(cpu) == CPU_DEALLOCATE) {
+
+ set_cpu_current_state(cpu, CPU_DEALLOCATE);
+ unregister_slb_shadow(hard_smp_processor_id(),
+ __pa(get_slb_shadow()));
+ rtas_stop_self();
+ goto out_bug;
+ } else if (get_preferred_offline_state(cpu) == CPU_DEACTIVATE) {
+ set_cpu_current_state(cpu, CPU_DEACTIVATE);
+ cede_on_offline();
+ }
+
+out_bug:
/* Should never get here... */
BUG();
for(;;);
@@ -112,11 +162,23 @@ static void pseries_cpu_die(unsigned int cpu)
int cpu_status;
unsigned int pcpu = get_hard_smp_processor_id(cpu);
- for (tries = 0; tries < 25; tries++) {
- cpu_status = query_cpu_stopped(pcpu);
- if (cpu_status == 0 || cpu_status == -1)
- break;
- cpu_relax();
+ if (get_preferred_offline_state(cpu) == CPU_DEACTIVATE) {
+ cpu_status = 1;
+ for (tries = 0; tries < 1000; tries++) {
+ if (get_cpu_current_state(cpu) == CPU_DEACTIVATE) {
+ cpu_status = 0;
+ break;
+ }
+ cpu_relax();
+ }
+ } else {
+
+ for (tries = 0; tries < 25; tries++) {
+ cpu_status = query_cpu_stopped(pcpu);
+ if (cpu_status == 0 || cpu_status == -1)
+ break;
+ cpu_relax();
+ }
}
if (cpu_status != 0) {
printk("Querying DEAD? cpu %i (%i) shows %i\n",
diff --git a/arch/powerpc/platforms/pseries/offline_driver.c b/arch/powerpc/platforms/pseries/offline_driver.c
new file mode 100644
index 0000000..e75e6e5
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/offline_driver.c
@@ -0,0 +1,161 @@
+#include "offline_driver.h"
+#include <linux/cpu.h>
+#include <linux/percpu-defs.h>
+
+struct cpu_hotplug_state {
+ enum cpu_state_vals state_val;
+ const char *state_name;
+ int available;
+} pSeries_cpu_hotplug_states[] = {
+ {CPU_DEALLOCATE, "deallocate", 1},
+ {CPU_DEACTIVATE, "deactivate", 1},
+ {CPU_STATE_ONLINE, "online", 1},
+ {CPU_MAX_OFFLINE_STATES, "", 0},
+};
+
+static DEFINE_PER_CPU(enum cpu_state_vals, preferred_offline_state) =
+ CPU_DEALLOCATE;
+static DEFINE_PER_CPU(enum cpu_state_vals, current_state) = CPU_DEALLOCATE;
+
+static enum cpu_state_vals default_offline_state = CPU_DEALLOCATE;
+
+enum cpu_state_vals get_cpu_current_state(int cpu)
+{
+ return per_cpu(current_state, cpu);
+}
+
+void set_cpu_current_state(int cpu, enum cpu_state_vals state)
+{
+ per_cpu(current_state, cpu) = state;
+}
+
+enum cpu_state_vals get_preferred_offline_state(int cpu)
+{
+ return per_cpu(preferred_offline_state, cpu);
+}
+
+void set_preferred_offline_state(int cpu, enum cpu_state_vals state)
+{
+ per_cpu(preferred_offline_state, cpu) = state;
+}
+
+void set_default_offline_state(int cpu)
+{
+ per_cpu(preferred_offline_state, cpu) = default_offline_state;
+}
+
+static const char *get_cpu_hotplug_state_name(enum cpu_state_vals state_val)
+{
+ return pSeries_cpu_hotplug_states[state_val].state_name;
+}
+
+static bool cpu_hotplug_state_available(enum cpu_state_vals state_val)
+{
+ return pSeries_cpu_hotplug_states[state_val].available;
+}
+
+ssize_t pSeries_read_available_states(unsigned int cpu, char *buf)
+{
+ int state;
+ ssize_t ret = 0;
+
+ for (state = CPU_DEALLOCATE; state < CPU_MAX_OFFLINE_STATES; state++) {
+ if (!cpu_hotplug_state_available(state))
+ continue;
+
+ if (ret >= (ssize_t) ((PAGE_SIZE / sizeof(char))
+ - (CPU_STATES_LEN + 2)))
+ goto out;
+ ret += scnprintf(&buf[ret], CPU_STATES_LEN, "%s ",
+ get_cpu_hotplug_state_name(state));
+ }
+
+out:
+ ret += sprintf(&buf[ret], "\n");
+ return ret;
+}
+
+ssize_t pSeries_read_current_state(unsigned int cpu, char *buf)
+{
+ int state = get_cpu_current_state(cpu);
+
+ return scnprintf(buf, CPU_STATES_LEN, "%s\n",
+ get_cpu_hotplug_state_name(state));
+}
+
+ssize_t pSeries_write_current_state(unsigned int cpu, const char *buf)
+{
+ int ret;
+ char state_name[CPU_STATES_LEN];
+ int i;
+ struct sys_device *dev = get_cpu_sysdev(cpu);
+ ret = sscanf(buf, "%15s", state_name);
+
+ if (ret != 1) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ for (i = CPU_DEALLOCATE; i < CPU_MAX_OFFLINE_STATES; i++)
+ if (!strnicmp(state_name,
+ get_cpu_hotplug_state_name(i),
+ CPU_STATES_LEN))
+ break;
+
+ if (i == CPU_MAX_OFFLINE_STATES) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ if (i == get_cpu_current_state(cpu)) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ if (i == CPU_STATE_ONLINE) {
+ ret = cpu_up(cpu);
+ if (!ret)
+ kobject_uevent(&dev->kobj, KOBJ_ONLINE);
+ goto out_unlock;
+ }
+
+ switch (i) {
+ case CPU_DEALLOCATE:
+ if (get_cpu_current_state(cpu) == CPU_DEACTIVATE) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ break;
+ case CPU_DEACTIVATE:
+ if (get_cpu_current_state(cpu) == CPU_DEALLOCATE) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+ break;
+ default:
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ set_preferred_offline_state(cpu, i);
+ ret = cpu_down(cpu);
+ if (!ret)
+ kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
+
+out_unlock:
+ return ret;
+}
+
+struct cpu_offline_driver pSeries_offline_driver = {
+ .read_available_states = pSeries_read_available_states,
+ .read_current_state = pSeries_read_current_state,
+ .write_current_state = pSeries_write_current_state,
+};
+
+static int pseries_hotplug_driver_init(void)
+{
+ return register_cpu_offline_driver(&pSeries_offline_driver);
+}
+
+arch_initcall(pseries_hotplug_driver_init);
diff --git a/arch/powerpc/platforms/pseries/offline_driver.h b/arch/powerpc/platforms/pseries/offline_driver.h
new file mode 100644
index 0000000..77b8f76
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/offline_driver.h
@@ -0,0 +1,20 @@
+#ifndef _OFFLINE_DRIVER_H_
+#define _OFFLINE_DRIVER_H_
+
+#define CPU_STATES_LEN 16
+
+/* Cpu offline states go here */
+enum cpu_state_vals {
+ CPU_DEALLOCATE,
+ CPU_DEACTIVATE,
+ CPU_STATE_ONLINE,
+ CPU_MAX_OFFLINE_STATES
+};
+
+extern enum cpu_state_vals get_cpu_current_state(int cpu);
+extern void set_cpu_current_state(int cpu, enum cpu_state_vals state);
+extern enum cpu_state_vals get_preferred_offline_state(int cpu);
+extern void set_preferred_offline_state(int cpu, enum cpu_state_vals state);
+extern int start_secondary(void);
+extern void set_default_offline_state(int cpu);
+#endif
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 1f8f6cf..cfea8db 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -48,6 +48,7 @@
#include "plpar_wrappers.h"
#include "pseries.h"
#include "xics.h"
+#include "offline_driver.h"
/*
@@ -86,6 +87,9 @@ static inline int __devinit smp_startup_cpu(unsigned int lcpu)
/* Fixup atomic count: it exited inside IRQ handler. */
task_thread_info(paca[lcpu].__current)->preempt_count = 0;
+ if (get_cpu_current_state(lcpu) == CPU_DEACTIVATE)
+ goto out;
+
/*
* If the RTAS start-cpu token does not exist then presume the
* cpu is already spinning.
@@ -100,6 +104,7 @@ static inline int __devinit smp_startup_cpu(unsigned int lcpu)
return 0;
}
+out:
return 1;
}
@@ -113,12 +118,15 @@ static void __devinit smp_xics_setup_cpu(int cpu)
vpa_init(cpu);
cpu_clear(cpu, of_spin_map);
+ set_cpu_current_state(cpu, CPU_STATE_ONLINE);
+ set_default_offline_state(cpu);
}
#endif /* CONFIG_XICS */
static void __devinit smp_pSeries_kick_cpu(int nr)
{
+ long rc;
BUG_ON(nr < 0 || nr >= NR_CPUS);
if (!smp_startup_cpu(nr))
@@ -130,6 +138,15 @@ static void __devinit smp_pSeries_kick_cpu(int nr)
* the processor will continue on to secondary_start
*/
paca[nr].cpu_start = 1;
+
+ set_preferred_offline_state(nr, CPU_STATE_ONLINE);
+
+ if (get_cpu_current_state(nr) == CPU_DEACTIVATE) {
+ rc = plpar_hcall_norets(H_PROD, nr);
+ if (rc != H_SUCCESS)
+ panic("Error: Prod to wake up processor %d Ret= %ld\n",
+ nr, rc);
+ }
}
static int smp_pSeries_cpu_bootable(unsigned int nr)
^ permalink raw reply related
* [PATCH v2 0/2] cpu: pseries: Offline state framework.
From: Gautham R Shenoy @ 2009-08-28 10:00 UTC (permalink / raw)
To: Joel Schopp, Benjamin Herrenschmidt, Vaidyanathan Srinivasan,
Peter Zijlstra, Dipankar Sarma
Cc: Darrick J. Wong, linuxppc-dev, linux-kernel, Venkatesh Pallipadi
Hi,
This is the version 2 of the patch series to provide a cpu-offline framework
that enables the administrators choose the state the offline CPU must be put
into when multiple such states are exposed by the underlying architecture.
Version 1 of the Patch can be found here:
http://lkml.org/lkml/2009/8/6/236
The patch-series exposes the following sysfs tunables to
allow the system-adminstrator to choose the state of a CPU:
To query the available hotplug states, one needs to read the sysfs tunable:
/sys/devices/system/cpu/cpu<number>/available_hotplug_states
To query or set the current state, on needs to read/write the sysfs tunable:
/sys/devices/system/cpu/cpu<number>/current_states
The patchset ensures that the writes to the "current_state" sysfs file are
serialized against the writes to the "online" file.
This patchset also contains the offline state driver implemented for
pSeries. For pSeries, we define three available_hotplug_states. They are:
online: The processor is online.
deallocate: This is the the default behaviour when the cpu is offlined
even in the absense of this driver. The CPU would call make an
rtas_stop_self() call and hand over the CPU back to the resource pool,
thereby effectively deallocating that vCPU from the LPAR.
NOTE: This would result in a configuration change to the LPAR
which is visible to the outside world.
deactivate: This cedes the vCPU to the hypervisor which
in turn can put the vCPU time to the best use.
NOTE: This option DOES NOT result in a configuration change
and the vCPU would be still entitled to the LPAR to which it earlier
belong to.
Awaiting your feedback.
---
Gautham R Shenoy (2):
cpu: Implement cpu-offline-state driver for pSeries.
cpu: Offline state Framework.
arch/powerpc/platforms/pseries/Makefile | 2
arch/powerpc/platforms/pseries/hotplug-cpu.c | 76 +++++++++-
arch/powerpc/platforms/pseries/offline_driver.c | 161 +++++++++++++++++++++
arch/powerpc/platforms/pseries/offline_driver.h | 20 +++
arch/powerpc/platforms/pseries/smp.c | 17 ++
drivers/base/cpu.c | 176 ++++++++++++++++++++++-
include/linux/cpu.h | 30 ++++
7 files changed, 465 insertions(+), 17 deletions(-)
create mode 100644 arch/powerpc/platforms/pseries/offline_driver.c
create mode 100644 arch/powerpc/platforms/pseries/offline_driver.h
--
Thanks and Regards
gautham.
^ permalink raw reply
* [PATCH v2 1/2] cpu: Offline state Framework.
From: Gautham R Shenoy @ 2009-08-28 10:00 UTC (permalink / raw)
To: Joel Schopp, Benjamin Herrenschmidt, Vaidyanathan Srinivasan,
Peter Zijlstra, Dipankar Sarma
Cc: Darrick J. Wong, linuxppc-dev, linux-kernel, Venkatesh Pallipadi
In-Reply-To: <20090828095741.10641.32053.stgit@sofia.in.ibm.com>
Provide an interface by which the system administrator can decide what state
should the CPU go to when it is offlined.
To query the hotplug states, on needs to perform a read on the sysfs tunable:
/sys/devices/system/cpu/cpu<number>/available_hotplug_states
To query or set the current state for a particular CPU, one needs to
use the sysfs interface:
/sys/devices/system/cpu/cpu<number>/current_state
This patch implements the architecture independent bits of the
cpu-offline-state framework.
Architectures which want to expose the multiple offline-states to the
userspace are expected to write a driver which can register
with this framework.
Such a driver should:
- Implement the callbacks defined in the structure struct cpu_offline_driver
which can be called into by this framework when the corresponding
sysfs interfaces are read or written into.
- Ensure that the following operation puts the CPU in the same state
as it did in the absence of the driver.
echo 0 > /sys/devices/system/cpu/cpu<number>/online
This framework also serializes the writes to the "current_state"
with respect to with the writes to the "online" sysfs tunable.
Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
---
drivers/base/cpu.c | 176 ++++++++++++++++++++++++++++++++++++++++++++++++---
include/linux/cpu.h | 30 +++++++++
2 files changed, 197 insertions(+), 9 deletions(-)
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index e62a4cc..73efc55 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -20,7 +20,161 @@ EXPORT_SYMBOL(cpu_sysdev_class);
static DEFINE_PER_CPU(struct sys_device *, cpu_sys_devices);
+struct sys_device *get_cpu_sysdev(unsigned cpu)
+{
+ if (cpu < nr_cpu_ids && cpu_possible(cpu))
+ return per_cpu(cpu_sys_devices, cpu);
+ else
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(get_cpu_sysdev);
+
+
#ifdef CONFIG_HOTPLUG_CPU
+
+struct cpu_offline_driver *cpu_offline_driver;
+static DEFINE_MUTEX(cpu_offline_driver_lock);
+
+ssize_t show_available_states(struct sys_device *dev,
+ struct sysdev_attribute *attr, char *buf)
+{
+ struct cpu *cpu = container_of(dev, struct cpu, sysdev);
+ int cpu_num = cpu->sysdev.id;
+ ssize_t ret;
+
+ mutex_lock(&cpu_offline_driver_lock);
+ if (!cpu_offline_driver) {
+ ret = -EEXIST;
+ goto out_unlock;
+ }
+
+ ret = cpu_offline_driver->read_available_states(cpu_num, buf);
+
+out_unlock:
+ mutex_unlock(&cpu_offline_driver_lock);
+
+ return ret;
+
+}
+
+ssize_t show_current_state(struct sys_device *dev,
+ struct sysdev_attribute *attr, char *buf)
+{
+ struct cpu *cpu = container_of(dev, struct cpu, sysdev);
+ int cpu_num = cpu->sysdev.id;
+ ssize_t ret = 0;
+
+ mutex_lock(&cpu_offline_driver_lock);
+ if (!cpu_offline_driver) {
+ ret = -EEXIST;
+ goto out_unlock;
+ }
+
+ ret = cpu_offline_driver->read_current_state(cpu_num, buf);
+
+out_unlock:
+ mutex_unlock(&cpu_offline_driver_lock);
+
+ return ret;
+}
+
+ssize_t store_current_state(struct sys_device *dev,
+ struct sysdev_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct cpu *cpu = container_of(dev, struct cpu, sysdev);
+ int cpu_num = cpu->sysdev.id;
+ ssize_t ret = count;
+
+ mutex_lock(&cpu_offline_driver_lock);
+ if (!cpu_offline_driver) {
+ ret = -EEXIST;
+ goto out_unlock;
+ }
+
+ ret = cpu_offline_driver->write_current_state(cpu_num, buf);
+
+out_unlock:
+ mutex_unlock(&cpu_offline_driver_lock);
+
+ if (ret >= 0)
+ ret = count;
+ return ret;
+}
+
+static SYSDEV_ATTR(available_hotplug_states, 0444, show_available_states,
+ NULL);
+static SYSDEV_ATTR(current_state, 0644, show_current_state,
+ store_current_state);
+
+/* Should be called with cpu_offline_driver_lock held */
+void cpu_offline_driver_add_cpu(struct sys_device *cpu_sys_dev)
+{
+ if (!cpu_offline_driver || !cpu_sys_dev)
+ return;
+
+ sysdev_create_file(cpu_sys_dev, &attr_available_hotplug_states);
+ sysdev_create_file(cpu_sys_dev, &attr_current_state);
+}
+
+/* Should be called with cpu_offline_driver_lock held */
+void cpu_offline_driver_remove_cpu(struct sys_device *cpu_sys_dev)
+{
+ if (!cpu_offline_driver || !cpu_sys_dev)
+ return;
+
+ sysdev_remove_file(cpu_sys_dev, &attr_available_hotplug_states);
+ sysdev_remove_file(cpu_sys_dev, &attr_current_state);
+
+}
+
+int register_cpu_offline_driver(struct cpu_offline_driver *arch_cpu_driver)
+{
+ int ret = 0;
+ int cpu;
+ mutex_lock(&cpu_offline_driver_lock);
+
+ if (cpu_offline_driver != NULL) {
+ ret = -EEXIST;
+ goto out_unlock;
+ }
+
+ if (!(arch_cpu_driver->read_available_states &&
+ arch_cpu_driver->read_current_state &&
+ arch_cpu_driver->write_current_state)) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ cpu_offline_driver = arch_cpu_driver;
+
+ for_each_possible_cpu(cpu)
+ cpu_offline_driver_add_cpu(get_cpu_sysdev(cpu));
+
+out_unlock:
+ mutex_unlock(&cpu_offline_driver_lock);
+ return ret;
+}
+
+void unregister_cpu_offline_driver(struct cpu_offline_driver *arch_cpu_driver)
+{
+ int cpu;
+ mutex_lock(&cpu_offline_driver_lock);
+
+ if (!cpu_offline_driver) {
+ WARN_ON(1);
+ mutex_unlock(&cpu_offline_driver_lock);
+ return;
+ }
+
+ for_each_possible_cpu(cpu)
+ cpu_offline_driver_remove_cpu(get_cpu_sysdev(cpu));
+
+ cpu_offline_driver = NULL;
+ mutex_unlock(&cpu_offline_driver_lock);
+}
+
+
static ssize_t show_online(struct sys_device *dev, struct sysdev_attribute *attr,
char *buf)
{
@@ -35,6 +189,7 @@ static ssize_t __ref store_online(struct sys_device *dev, struct sysdev_attribut
struct cpu *cpu = container_of(dev, struct cpu, sysdev);
ssize_t ret;
+ mutex_lock(&cpu_offline_driver_lock);
switch (buf[0]) {
case '0':
ret = cpu_down(cpu->sysdev.id);
@@ -50,6 +205,8 @@ static ssize_t __ref store_online(struct sys_device *dev, struct sysdev_attribut
ret = -EINVAL;
}
+ mutex_unlock(&cpu_offline_driver_lock);
+
if (ret >= 0)
ret = count;
return ret;
@@ -59,23 +216,33 @@ static SYSDEV_ATTR(online, 0644, show_online, store_online);
static void __cpuinit register_cpu_control(struct cpu *cpu)
{
sysdev_create_file(&cpu->sysdev, &attr_online);
+ mutex_lock(&cpu_offline_driver_lock);
+ cpu_offline_driver_add_cpu(&cpu->sysdev);
+ mutex_unlock(&cpu_offline_driver_lock);
}
+
void unregister_cpu(struct cpu *cpu)
{
int logical_cpu = cpu->sysdev.id;
unregister_cpu_under_node(logical_cpu, cpu_to_node(logical_cpu));
+ mutex_lock(&cpu_offline_driver_lock);
+ cpu_offline_driver_remove_cpu(&cpu->sysdev);
+ mutex_unlock(&cpu_offline_driver_lock);
+
sysdev_remove_file(&cpu->sysdev, &attr_online);
sysdev_unregister(&cpu->sysdev);
per_cpu(cpu_sys_devices, logical_cpu) = NULL;
return;
}
+
#else /* ... !CONFIG_HOTPLUG_CPU */
static inline void register_cpu_control(struct cpu *cpu)
{
}
+
#endif /* CONFIG_HOTPLUG_CPU */
#ifdef CONFIG_KEXEC
@@ -224,15 +391,6 @@ int __cpuinit register_cpu(struct cpu *cpu, int num)
return error;
}
-struct sys_device *get_cpu_sysdev(unsigned cpu)
-{
- if (cpu < nr_cpu_ids && cpu_possible(cpu))
- return per_cpu(cpu_sys_devices, cpu);
- else
- return NULL;
-}
-EXPORT_SYMBOL_GPL(get_cpu_sysdev);
-
int __init cpu_dev_init(void)
{
int err;
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 4d668e0..7636420 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -51,6 +51,36 @@ struct notifier_block;
#ifdef CONFIG_HOTPLUG_CPU
extern int register_cpu_notifier(struct notifier_block *nb);
extern void unregister_cpu_notifier(struct notifier_block *nb);
+
+/*
+ * struct cpu_offline_driver: Callbacks for cpu-offline state framework.
+ *
+ * Defines the hooks for the architecture dependent callbacks
+ * which can be invoked when the user queries the
+ * available_hotplug_states and current_state and sets the current_state.
+ *
+ * read_available_state: Called when the user queries available_hotplug_states.
+ * @cpu: Cpu for which available_hotplug_states are being queried.
+ * @buf: Buffer in which the available hotplug states are to be populated.
+ *
+ * read_current_state: Called when the user queries current_state.
+ * @cpu: Cpu for which current_state is being queried.
+ * @buf: Buffer in which the current_state value is to be returned.
+ *
+ * write_current_state: Called when the user wants to set the current_state.
+ * @cpu: Cpu for which the current state is being set.
+ * @buf: Buffer containing the string corresponding to the current_state
+ * that needs to be set.
+ */
+struct cpu_offline_driver {
+ ssize_t (*read_available_states)(unsigned int cpu, char *buf);
+ ssize_t (*read_current_state)(unsigned int cpu, char *buf);
+ ssize_t (*write_current_state)(unsigned int cpu, const char *buf);
+};
+
+extern int register_cpu_offline_driver(struct cpu_offline_driver *driver);
+extern void unregister_cpu_offline_driver(struct cpu_offline_driver *driver);
+
#else
#ifndef MODULE
^ permalink raw reply related
* Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c
From: Vaidyanathan Srinivasan @ 2009-08-28 8:19 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Gautham R Shenoy, Pallipadi, Venkatesh, linux-kernel,
Paul Mackerras, arun, Ingo Molnar, linuxppc-dev, Balbir Singh
In-Reply-To: <1251442872.18584.125.camel@twins>
* Peter Zijlstra <peterz@infradead.org> [2009-08-28 09:01:12]:
> On Fri, 2009-08-28 at 08:48 +0200, Peter Zijlstra wrote:
> >
> > > void cpuidle_install_idle_handler(void)
> > > {
> > > .........
> > > .........
> > > cpuidle_pm_idle = cpuidle_idle_call;
> > > }
> >
> > All I'm seeing here is a frigging mess.
> >
> > How on earths can something called: cpuidle_install_idle_handler() have
> > a void argument, _WHAT_ handler is it going to install?
>
> Argh, now I see, it installs itself as the platform idle handler.
>
> so cpuidle_install_idle_handler() pokes at the unmanaged pm_idle pointer
> to make cpuidle take control.
>
> On module load it does:
>
> pm_idle_old = pm_idle;
>
> then in the actual idle loop it does:
>
> if (!dev || !dev->enabled) {
> if (pm_idle_old)
> pm_idle_old();
>
> who is to say that the pointer stored at module init time is still
> around at that time?
>
> So cpuidle recognised the pm_idle stuff was a flaky, but instead of
> fixing it, they build a whole new layer on top of it. Brilliant.
>
> /me goes mark this whole thread read, I've got enough things to do.
Hi Peter,
I understand that you are frustrated with the mess. We are willing
to clean up the pm_idle pointer at the moment before the cpuidle
framework is exploited my more archs.
At this moment we need your suggestions on what interface should we
call 'clean' and safe.
cpuidle.c and the arch independent cpuidle subsystem is not a module
and its cpuidle_idle_call() routine is valid and can be safely called
from arch dependent process.c
The fragile part is how cpuidle_idle_call() is hooked onto arch
specific cpu_idle() function at runtime. x86 has the pm_idle pointer
exported while powerpc has ppc_md.power_save pointer being called.
At cpuidle init time we can override the platform idle function, but
that will mean we are including arch specific code in cpuidle.c
Do you think having an exported function in cpuidle.c to give us the
correct pointer to arch code be better than the current situation:
in drivers/cpuidle/cpuidle.c
void (*get_cpuidle_handler(void))(void)
{
return cpuidle_pm_idle;
}
EXPORT_SYMBOL(get_cpuidle_handler);
and from pseries/processor_idle.c,
ppc_md.power_save = get_cpuidle_handler();
--Vaidy
^ permalink raw reply
* Re: lmb: Remove __init from lmb_end_of_DRAM()
From: Benjamin Herrenschmidt @ 2009-08-28 7:21 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linuxppc-dev list, Linux Kernel list, David S. Miller
In-Reply-To: <200908271959.n7RJxNvV008624@hera.kernel.org>
On Thu, 2009-08-27 at 19:59 +0000, Linux Kernel Mailing List wrote:
> Gitweb: http://git.kernel.org/linus/4f8ee2c9cc0e885d2bb50ef26db66150ab25213e
> Commit: 4f8ee2c9cc0e885d2bb50ef26db66150ab25213e
> Parent: cf481442f2e086316ed8a1b3046f00ad23632ac4
> Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> AuthorDate: Thu Aug 27 17:20:30 2009 +1000
> Committer: Linus Torvalds <torvalds@linux-foundation.org>
> CommitDate: Thu Aug 27 12:25:26 2009 -0700
>
> lmb: Remove __init from lmb_end_of_DRAM()
>
> We call lmb_end_of_DRAM() to test whether a DMA mask is ok on a machine
> without IOMMU, but this function is marked as __init.
>
> I don't think there's a clean way to get the top of RAM max_pfn doesn't
> appear to include highmem or I missed (or we have a bug :-) so for now,
> let's just avoid having a broken 2.6.31 by making this function
> non-__init and we can revisit later.
And another brown paper bag for me ! Patch is fine, but doesn't do the
job because some idiot (me) didn't actually test (it looked so obvious)
and didn't notice somebody had the strange idea to also put __init on
the declaration in the .h file ! Ugh.
I'll send another patch later, have to run home now.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c
From: Peter Zijlstra @ 2009-08-28 7:01 UTC (permalink / raw)
To: arun
Cc: Gautham R Shenoy, Pallipadi, Venkatesh, linux-kernel,
Paul Mackerras, Ingo Molnar, linuxppc-dev, Balbir Singh
In-Reply-To: <1251442085.18584.120.camel@twins>
On Fri, 2009-08-28 at 08:48 +0200, Peter Zijlstra wrote:
>
> > void cpuidle_install_idle_handler(void)
> > {
> > .........
> > .........
> > cpuidle_pm_idle = cpuidle_idle_call;
> > }
>
> All I'm seeing here is a frigging mess.
>
> How on earths can something called: cpuidle_install_idle_handler() have
> a void argument, _WHAT_ handler is it going to install?
Argh, now I see, it installs itself as the platform idle handler.
so cpuidle_install_idle_handler() pokes at the unmanaged pm_idle pointer
to make cpuidle take control.
On module load it does:
pm_idle_old = pm_idle;
then in the actual idle loop it does:
if (!dev || !dev->enabled) {
if (pm_idle_old)
pm_idle_old();
who is to say that the pointer stored at module init time is still
around at that time?
So cpuidle recognised the pm_idle stuff was a flaky, but instead of
fixing it, they build a whole new layer on top of it. Brilliant.
/me goes mark this whole thread read, I've got enough things to do.
^ permalink raw reply
* Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c
From: Balbir Singh @ 2009-08-28 6:59 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Gautham R Shenoy, Pallipadi, Venkatesh, linux-kernel,
Paul Mackerras, arun, Ingo Molnar, linuxppc-dev
In-Reply-To: <1251442085.18584.120.camel@twins>
* Peter Zijlstra <a.p.zijlstra@chello.nl> [2009-08-28 08:48:05]:
> On Fri, 2009-08-28 at 11:44 +0530, Arun R Bharadwaj wrote:
> > * Peter Zijlstra <a.p.zijlstra@chello.nl> [2009-08-27 14:53:27]:
> >
> > Hi Peter, Ben,
> >
> > I've put the whole thing in a sort of a block diagram. Hope it
> > explains things more clearly.
> >
> >
> >
> >
> >
> >
> > ----------------
> > | CPUIDLE | (Select idle states like
> > | GOVERNORS | C1, C1e, C6 etc in case
> > | (Menu/Ladder)| x86 & nap, snooze in
> > | | case of POWER - based on
> > ---------------- latency & power req)
> > ^
> > |
> > |
> > |
> > |
> > |
> > ---------- ----------------- -------------
> > | | | | | PSERIES |
> > | ACPI |------------------> | CPUIDLE | <--------------| IDLE |
> > | | | | | |
> > ---------- ----------------- -------------
> >
> > Main idle routine- pm_idle() Main idle routine-
> > ppc_md.power_save()
> >
> > pm_idle = cpuidle_pm_idle; ppc_md.power_save =
> > (start using cpuidle's idle cpuidle_pm_idle();
> > loop, which internally calls
> > governor to select the right
> > state to go into).
> >
> >
> > Relavent code snippet from drivers/cpuidle/cpuidle.c
> > -------------------------------------
> >
> > static void cpuidle_idle_call(void)
> > {
> > ............
> > ............
> >
> > /* Call the menu_select() to select the idle state to enter. */
> > next_state = cpuidle_curr_governor->select(dev);
> >
> > ............
> > ............
> >
> > /*
> > * Enter the idle state previously selected. target_state->enter
> > * would call pseries_cpuidle_loop() which selects nap/snooze
> > * /
> > dev->last_residency = target_state->enter(dev, target_state);
> > }
> >
> > void cpuidle_install_idle_handler(void)
> > {
> > .........
> > .........
> > cpuidle_pm_idle = cpuidle_idle_call;
> > }
>
> All I'm seeing here is a frigging mess.
>
> How on earths can something called: cpuidle_install_idle_handler() have
> a void argument, _WHAT_ handler is it going to install?
>
Peter, I think that is a typo, we need a function pointer like your
snippet of code showed
> So somehow you added to the ACPI mess by now having 3 wild function
> pointers, that's _NOT_ progress.
>
The goal, IIUC is to integrate three modules
1. CPUIdle - for idle CPU management
2. Architecture specific code for idle detection
3. CPUIdle governor
Again, if IIUC
The architecture specific idle code would call an idle loop that would
call into the CPUIdle framework, which inturn would depend on the
governor selected.
Passing void* is a mess, we need function pointer registration
and a framework so that we can query what is registered.
--
Balbir
^ permalink raw reply
* Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c
From: Peter Zijlstra @ 2009-08-28 6:48 UTC (permalink / raw)
To: arun
Cc: Gautham R Shenoy, Pallipadi, Venkatesh, linux-kernel,
Paul Mackerras, Ingo Molnar, linuxppc-dev, Balbir Singh
In-Reply-To: <20090828061434.GA11863@linux.vnet.ibm.com>
On Fri, 2009-08-28 at 11:44 +0530, Arun R Bharadwaj wrote:
> * Peter Zijlstra <a.p.zijlstra@chello.nl> [2009-08-27 14:53:27]:
>
> Hi Peter, Ben,
>
> I've put the whole thing in a sort of a block diagram. Hope it
> explains things more clearly.
>
>
>
>
>
>
> ----------------
> | CPUIDLE | (Select idle states like
> | GOVERNORS | C1, C1e, C6 etc in case
> | (Menu/Ladder)| x86 & nap, snooze in
> | | case of POWER - based on
> ---------------- latency & power req)
> ^
> |
> |
> |
> |
> |
> ---------- ----------------- -------------
> | | | | | PSERIES |
> | ACPI |------------------> | CPUIDLE | <--------------| IDLE |
> | | | | | |
> ---------- ----------------- -------------
>
> Main idle routine- pm_idle() Main idle routine-
> ppc_md.power_save()
>
> pm_idle = cpuidle_pm_idle; ppc_md.power_save =
> (start using cpuidle's idle cpuidle_pm_idle();
> loop, which internally calls
> governor to select the right
> state to go into).
>
>
> Relavent code snippet from drivers/cpuidle/cpuidle.c
> -------------------------------------
>
> static void cpuidle_idle_call(void)
> {
> ............
> ............
>
> /* Call the menu_select() to select the idle state to enter. */
> next_state = cpuidle_curr_governor->select(dev);
>
> ............
> ............
>
> /*
> * Enter the idle state previously selected. target_state->enter
> * would call pseries_cpuidle_loop() which selects nap/snooze
> * /
> dev->last_residency = target_state->enter(dev, target_state);
> }
>
> void cpuidle_install_idle_handler(void)
> {
> .........
> .........
> cpuidle_pm_idle = cpuidle_idle_call;
> }
All I'm seeing here is a frigging mess.
How on earths can something called: cpuidle_install_idle_handler() have
a void argument, _WHAT_ handler is it going to install?
So somehow you added to the ACPI mess by now having 3 wild function
pointers, that's _NOT_ progress.
^ permalink raw reply
* Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c
From: Peter Zijlstra @ 2009-08-28 6:40 UTC (permalink / raw)
To: arun
Cc: Gautham R Shenoy, Pallipadi, Venkatesh, linux-kernel,
Paul Mackerras, Ingo Molnar, linuxppc-dev, Balbir Singh
In-Reply-To: <20090828044901.GA10283@linux.vnet.ibm.com>
On Fri, 2009-08-28 at 10:19 +0530, Arun R Bharadwaj wrote:
>
>
> This only does the job of picking the right idle loop for current
> latency and power requirement. This is already done in ladder/menu
> governors under the routines menu_select()/ladder_select().
> I'm not sure whats the purpose of it here.
I can't seem to find ladder_select() but menu_select() doesn't manage
pm_idle and its not clear what it does manage.
> Here we are only concerned about the main idle loop, which is
> pm_idle/ppc_md.power_save. After setting the main idle loop to
> cpuidle_pm_idle, that would call cpuidle_idle_call() which would do
> the job of picking the right low level idle loop based on latency and
> other requirements.
It also gets pm_idle unexported and avoids anybody directly tinkering
with the function pointer, _that_ is the whole goal.
pm_idle is it exists today, and the whole cpuidle_{un,}install*() is
utter crap. It relies on unmanaged access to this function pointer.
/me stop looking at drivers/cpuidle/, convoluted mess that is, shame on
you for wanting to have anything to do with it.
^ permalink raw reply
* Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c
From: Arun R Bharadwaj @ 2009-08-28 6:43 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Gautham R Shenoy, Pallipadi, Venkatesh, linux-kernel,
Paul Mackerras, Arun Bharadwaj, Ingo Molnar, linuxppc-dev,
Balbir Singh
In-Reply-To: <1251377607.18584.96.camel@twins>
* Peter Zijlstra <a.p.zijlstra@chello.nl> [2009-08-27 14:53:27]:
> On Thu, 2009-08-27 at 17:23 +0530, Arun R Bharadwaj wrote:
> > * Arun R Bharadwaj <arun@linux.vnet.ibm.com> [2009-08-27 17:19:08]:
> >
> > Cpuidle infrastructure assumes pm_idle as the default idle routine.
> > But, ppc_md.power_save is the default idle callback in case of pSeries.
> >
> > So, create a more generic, architecture independent cpuidle_pm_idle
> > function pointer in driver/cpuidle/cpuidle.c and allow the idle routines
> > of architectures to be set to cpuidle_pm_idle.
> >
> > Signed-off-by: Arun R Bharadwaj <arun@linux.vnet.ibm.com>
> > ---
> > drivers/cpuidle/cpuidle.c | 12 +++++++-----
> > include/linux/cpuidle.h | 7 +++++++
> > 2 files changed, 14 insertions(+), 5 deletions(-)
> >
> > Index: linux.trees.git/drivers/cpuidle/cpuidle.c
> > ===================================================================
> > --- linux.trees.git.orig/drivers/cpuidle/cpuidle.c
> > +++ linux.trees.git/drivers/cpuidle/cpuidle.c
> > @@ -25,6 +25,7 @@ DEFINE_PER_CPU(struct cpuidle_device *,
> > DEFINE_MUTEX(cpuidle_lock);
> > LIST_HEAD(cpuidle_detected_devices);
> > static void (*pm_idle_old)(void);
> > +void (*cpuidle_pm_idle)(void);
> >
> > static int enabled_devices;
> >
> > @@ -98,10 +99,10 @@ static void cpuidle_idle_call(void)
> > */
> > void cpuidle_install_idle_handler(void)
> > {
> > - if (enabled_devices && (pm_idle != cpuidle_idle_call)) {
> > + if (enabled_devices && (cpuidle_pm_idle != cpuidle_idle_call)) {
> > /* Make sure all changes finished before we switch to new idle */
> > smp_wmb();
> > - pm_idle = cpuidle_idle_call;
> > + cpuidle_pm_idle = cpuidle_idle_call;
> > }
> > }
> >
> > @@ -110,8 +111,9 @@ void cpuidle_install_idle_handler(void)
> > */
> > void cpuidle_uninstall_idle_handler(void)
> > {
> > - if (enabled_devices && pm_idle_old && (pm_idle != pm_idle_old)) {
> > - pm_idle = pm_idle_old;
> > + if (enabled_devices && pm_idle_old &&
> > + (cpuidle_pm_idle != pm_idle_old)) {
> > + cpuidle_pm_idle = pm_idle_old;
> > cpuidle_kick_cpus();
> > }
> > }
> > @@ -382,7 +384,7 @@ static int __init cpuidle_init(void)
> > {
> > int ret;
> >
> > - pm_idle_old = pm_idle;
> > + pm_idle_old = cpuidle_pm_idle;
> >
> > ret = cpuidle_add_class_sysfs(&cpu_sysdev_class);
> > if (ret)
> > Index: linux.trees.git/include/linux/cpuidle.h
> > ===================================================================
> > --- linux.trees.git.orig/include/linux/cpuidle.h
> > +++ linux.trees.git/include/linux/cpuidle.h
> > @@ -188,4 +188,11 @@ static inline void cpuidle_unregister_go
> > #define CPUIDLE_DRIVER_STATE_START 0
> > #endif
> >
> > +/*
> > + * Idle callback used by cpuidle to call the cpuidle_idle_call().
> > + * Platform drivers can use this to register to cpuidle's idle loop.
> > + */
> > +
> > +extern void (*cpuidle_pm_idle)(void);
> > +
> > #endif /* _LINUX_CPUIDLE_H */
>
>
> I'm not quite seeing how this makes anything any better. Not we have 3
> function pointers, where 1 should suffice.
>
Or, can we have something like:
(if exporting a function is ok, instead of exporting a function
pointer).
in drivers/cpuidle/cpuidle.c
void (*return_cpuidle_handler(void))(void)
{
return cpuidle_pm_idle;
}
EXPORT_SYMBOL(return_cpuidle_handler);
and from pseries/processor_idle.c,
ppc_md.power_save = return_cpuidle_handler;
--arun
> /me wonders what's wrong with something like:
>
> struct idle_func_desc {
> int power;
> int latency;
> void (*idle)(void);
> struct list_head list;
> };
>
> static void spin_idle(void)
> {
> for (;;)
> cpu_relax();
> }
>
> static idle_func_desc default_idle_func = {
> power = 0, /* doesn't safe any power */
> latency = INT_MAX, /* has max latency */
> idle = spin_idle,
> list = INIT_LIST_HEAD(default_idle_func.list),
> };
>
> void (*idle_func)(void);
> static struct list_head idle_func_list;
>
> static void pick_idle_func(void)
> {
> struct idle_func_desc *desc, *idle = &default_idle_desc;
>
> list_for_each_entry(desc, &idle_func_list, list) {
> if (desc->power < idle->power)
> continue;
> if (desc->latency > target_latency);
> continue;
> idle = desc;
> }
>
> pm_idle = idle->idle;
> }
>
> void register_idle_func(struct idle_func_desc *desc)
> {
> WARN_ON_ONCE(!list_empty(&desc->list));
>
> list_add_tail(&idle_func_list, &desc->list);
> pick_idle_func();
> }
>
> void unregister_idle_func(struct idle_func_desc *desc)
> {
> WARN_ON_ONCE(list_empty(&desc->list));
>
> list_del_init(&desc->list);
> if (idle_func == desc->idle)
> pick_idle_func();
> }
>
^ permalink raw reply
* Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c
From: Arun R Bharadwaj @ 2009-08-28 6:14 UTC (permalink / raw)
To: Peter Zijlstra, Benjamin Herrenschmidt
Cc: Gautham R Shenoy, Pallipadi, Venkatesh, linux-kernel,
Paul Mackerras, Arun Bharadwaj, Ingo Molnar, linuxppc-dev,
Balbir Singh
In-Reply-To: <1251377607.18584.96.camel@twins>
* Peter Zijlstra <a.p.zijlstra@chello.nl> [2009-08-27 14:53:27]:
Hi Peter, Ben,
I've put the whole thing in a sort of a block diagram. Hope it
explains things more clearly.
----------------
| CPUIDLE | (Select idle states like
| GOVERNORS | C1, C1e, C6 etc in case
| (Menu/Ladder)| x86 & nap, snooze in
| | case of POWER - based on
---------------- latency & power req)
^
|
|
|
|
|
---------- ----------------- -------------
| | | | | PSERIES |
| ACPI |------------------> | CPUIDLE | <--------------| IDLE |
| | | | | |
---------- ----------------- -------------
Main idle routine- pm_idle() Main idle routine-
ppc_md.power_save()
pm_idle = cpuidle_pm_idle; ppc_md.power_save =
(start using cpuidle's idle cpuidle_pm_idle();
loop, which internally calls
governor to select the right
state to go into).
Relavent code snippet from drivers/cpuidle/cpuidle.c
-------------------------------------
static void cpuidle_idle_call(void)
{
............
............
/* Call the menu_select() to select the idle state to enter. */
next_state = cpuidle_curr_governor->select(dev);
............
............
/*
* Enter the idle state previously selected. target_state->enter
* would call pseries_cpuidle_loop() which selects nap/snooze
* /
dev->last_residency = target_state->enter(dev, target_state);
}
void cpuidle_install_idle_handler(void)
{
.........
.........
cpuidle_pm_idle = cpuidle_idle_call;
}
--arun
^ permalink raw reply
* Re: [PATCH v2 0/8] spi_mpc8xxx: Add support for DMA transfers
From: David Brownell @ 2009-08-28 4:41 UTC (permalink / raw)
To: avorontsov
Cc: Greg Kroah-Hartman, linux-kernel, linuxppc-dev, spi-devel-general,
Andrew Morton
In-Reply-To: <20090818220304.GA23640@oksana.dev.rtsoft.ru>
On Tuesday 18 August 2009, Anton Vorontsov wrote:
> - Fix build issues in fsl_qe_udc;
> - Some minor cosmetic changes in "Add support for QE DMA mode and
> CPM1/CPM2 chips" patch.
Hmm ... the first four of these are pure PPC stuff and thus
not appropriate to send as SPI patches; but the second four
depend on them.
So I'll just say
Acked-by: David Brownell <dbrownell@users.sourceforge.net>
and ask you to merge via the PPC tree. (And hope that you
verified these are bisectable...)
^ permalink raw reply
* Re: [PATCH 5/8] spi_mpc8xxx: Fix uninitialized variable
From: Kumar Gala @ 2009-08-28 5:48 UTC (permalink / raw)
To: Anton Vorontsov
Cc: David Brownell, Greg Kroah-Hartman, linux-kernel, linuxppc-dev,
spi-devel-general, Andrew Morton
In-Reply-To: <20090818220419.GE25090@oksana.dev.rtsoft.ru>
On Aug 18, 2009, at 5:04 PM, Anton Vorontsov wrote:
> This patch fixes the following warning:
>
> CC drivers/spi/spi_mpc8xxx.o
> spi_mpc8xxx.c: In function 'of_mpc8xxx_spi_probe':
> spi_mpc8xxx.c:681: warning: 'ret' may be used uninitialized in this
> function
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> drivers/spi/spi_mpc8xxx.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
Acked-by: Kumar Gala <galak@kernel.crashing.org>
- k
^ permalink raw reply
* Re: [PATCH 4/8] powerpc/qe&cpm: Implement static inline stubs for non-QE/CPM builds
From: Kumar Gala @ 2009-08-28 5:45 UTC (permalink / raw)
To: Anton Vorontsov
Cc: David Brownell, Greg Kroah-Hartman, linux-kernel, linuxppc-dev,
spi-devel-general, Andrew Morton
In-Reply-To: <20090818220418.GD25090@oksana.dev.rtsoft.ru>
On Aug 18, 2009, at 5:04 PM, Anton Vorontsov wrote:
> This is needed to avoid ugly #ifdefs in drivers. Also update
> fsl_qe_udc
> driver so that now it doesn't define its own versions that cause build
> breakage when the generic stubs are used.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/include/asm/cpm.h | 44 ++++++++++++++++++++++++++++++
> +++++++++
> arch/powerpc/include/asm/qe.h | 11 ++++++++-
> drivers/usb/gadget/fsl_qe_udc.h | 15 -------------
> 3 files changed, 54 insertions(+), 16 deletions(-)
applied to next
- k
^ permalink raw reply
* Re: [PATCH 3/8] powerpc/cpm: Move CPMFCR_* defines into cpm.h
From: Kumar Gala @ 2009-08-28 5:45 UTC (permalink / raw)
To: Anton Vorontsov
Cc: David Brownell, Greg Kroah-Hartman, linux-kernel, linuxppc-dev,
spi-devel-general, Andrew Morton
In-Reply-To: <20090818220416.GC25090@oksana.dev.rtsoft.ru>
On Aug 18, 2009, at 5:04 PM, Anton Vorontsov wrote:
> The bits are generic to CPM devices, so let's move them to the
> common header file, so drivers won't need to privately reintroduce
> another bunch of the same bits (as we can't include cpm2.h header
> together with cpm1.h).
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/include/asm/cpm.h | 16 ++++++++++++++++
> arch/powerpc/include/asm/cpm2.h | 8 --------
> 2 files changed, 16 insertions(+), 8 deletions(-)
applied to next
- k
^ permalink raw reply
* Re: [PATCH 2/8] powerpc/qe&cpm2: Avoid redefinitions in CPM2 and QE headers
From: Kumar Gala @ 2009-08-28 5:45 UTC (permalink / raw)
To: Anton Vorontsov
Cc: David Brownell, Greg Kroah-Hartman, linux-kernel, linuxppc-dev,
spi-devel-general, Andrew Morton
In-Reply-To: <20090818220415.GB25090@oksana.dev.rtsoft.ru>
On Aug 18, 2009, at 5:04 PM, Anton Vorontsov wrote:
> struct mcc defined in both immap_qe.h and immap_cpm2.h, so they will
> conflic when included in a single file. The mcc struct is easy to deal
> with, since it isn't used in any driver (yet), so let's just rename QE
> version to qe_mcc.
>
> The ucb_ctlr is a bit trickier, since it is used by fsl_qe_udc driver,
> and the driver supports both CPM and QE UDCs, plus the QE version is
> used to form a bigger immap struct.
>
> I don't want to touch too much of USB code in this series, so for now
> let's just copy most generic version into the common cpm.h header,
> later we'll create cpm_usb.h where we'll place common USB structs that
> are used by QE/CPM UDC and QE Host drivers (FHCI).
>
> And as for the structs in qe.h and cpm2.h, just prefix them with qe_
> and cpm_.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/include/asm/cpm.h | 22 ++++++++++++++++++++++
> arch/powerpc/include/asm/immap_cpm2.h | 2 +-
> arch/powerpc/include/asm/immap_qe.h | 8 ++++----
> 3 files changed, 27 insertions(+), 5 deletions(-)
applied to next
- k
^ permalink raw reply
* Re: [PATCH 1/8] powerpc/cpm: Remove SPI defines and spi structs
From: Kumar Gala @ 2009-08-28 5:45 UTC (permalink / raw)
To: Anton Vorontsov
Cc: David Brownell, Greg Kroah-Hartman, linux-kernel, linuxppc-dev,
spi-devel-general, Andrew Morton
In-Reply-To: <20090818220410.GA25090@oksana.dev.rtsoft.ru>
On Aug 18, 2009, at 5:04 PM, Anton Vorontsov wrote:
> When cpm2.h included into spi_mpc8xxx driver, the SPI defines
> in the header conflict with defines in the driver.
>
> We don't need them in the header file, so remove them. Plus
> remove "struct spi", we'll use a better version in the driver.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/include/asm/cpm1.h | 45
> ---------------------------------------
> arch/powerpc/include/asm/cpm2.h | 39
> ---------------------------------
> 2 files changed, 0 insertions(+), 84 deletions(-)
applied to next
- k
^ permalink raw reply
* Re: [PATCH 4/5] powerpc/85xx: Add suspend/resume support
From: Kumar Gala @ 2009-08-28 5:38 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: Scott Wood, linuxppc-dev, Timur Tabi
In-Reply-To: <20090827173021.GD739@oksana.dev.rtsoft.ru>
On Aug 27, 2009, at 12:30 PM, Anton Vorontsov wrote:
> This patch adds suspend/resume support for MPC8540-compatible and
> MPC8569 CPUs.
>
> MPC8540-compatible PMCs are trivial: we just write SLP bit into PM
> control and status register.
>
> MPC8569 is a bit trickier, QE turns off during suspend and so on
> resume we must reload QE microcode firmware and reset QE.
>
> So far we don't support Deep Sleep mode as found in newer MPC85xx
> CPUs (i.e. MPC8536). It can be relatively easy implemented though,
> and for it we reserve 'mem' suspend type.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/Kconfig | 2 +-
> arch/powerpc/platforms/85xx/Makefile | 1 +
> arch/powerpc/platforms/85xx/suspend.c | 115 ++++++++++++++++++++++++
> +++++++++
> 3 files changed, 117 insertions(+), 1 deletions(-)
> create mode 100644 arch/powerpc/platforms/85xx/suspend.c
How did you test this?
I'd also like to get Scott's Ack on this and the device tree patches
before accepting them.
- k
^ permalink raw reply
* Re: [PATCH 3/5] powerpc/qe: Add qe_upload_firmware() stub for non-QE builds
From: Kumar Gala @ 2009-08-28 5:38 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: Scott Wood, linuxppc-dev, Timur Tabi
In-Reply-To: <20090827173019.GC739@oksana.dev.rtsoft.ru>
On Aug 27, 2009, at 12:30 PM, Anton Vorontsov wrote:
> This is needed to avoid #ifdefs in MPC85xx suspend/resume code.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/include/asm/qe.h | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
applied to next
- k
^ permalink raw reply
* Re: [PATCH 2/5] powerpc/qe: Make qe_reset() code path safe for repeated invocation
From: Kumar Gala @ 2009-08-28 5:34 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: Scott Wood, linuxppc-dev, Timur Tabi
In-Reply-To: <20090827173017.GB739@oksana.dev.rtsoft.ru>
On Aug 27, 2009, at 12:30 PM, Anton Vorontsov wrote:
>
> For MPC8569 CPUs we'll need to reset QE after each suspend, so make
> qe_reset() code path suitable for repeated invocation, that is:
>
> - Don't initialize rheap structures if already initialized;
> - Don't allocate muram for SDMA if already allocated, just
> reinitialize
> registers with previously allocated muram offset;
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/sysdev/cpm_common.c | 3 +++
> arch/powerpc/sysdev/qe_lib/qe.c | 10 ++++++----
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/
> cpm_common.c
> index e4b6d66..ddfca52 100644
> --- a/arch/powerpc/sysdev/cpm_common.c
> +++ b/arch/powerpc/sysdev/cpm_common.c
> @@ -81,6 +81,9 @@ int __init cpm_muram_init(void)
> int i = 0;
> int ret = 0;
>
> + if (muram_pbase)
> + return 0;
> +
> spin_lock_init(&cpm_muram_lock);
> /* initialize the info header */
> rh_init(&cpm_muram_info, 1,
> diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/
> qe_lib/qe.c
> index b06564f..f485d5a 100644
> --- a/arch/powerpc/sysdev/qe_lib/qe.c
> +++ b/arch/powerpc/sysdev/qe_lib/qe.c
> @@ -317,16 +317,18 @@ EXPORT_SYMBOL(qe_put_snum);
> static int qe_sdma_init(void)
> {
> struct sdma __iomem *sdma = &qe_immr->sdma;
> - unsigned long sdma_buf_offset;
> + static unsigned long sdma_buf_offset;
>
> if (!sdma)
> return -ENODEV;
>
> /* allocate 2 internal temporary buffers (512 bytes size each) for
> * the SDMA */
> - sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
> - if (IS_ERR_VALUE(sdma_buf_offset))
> - return -ENOMEM;
> + if (!sdma_buf_offset) {
> + sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
> + if (IS_ERR_VALUE(sdma_buf_offset))
shouldn't we zero out sdma_buf_offset otherwise if we call this again
we'll think its set.
> + return -ENOMEM;
> + }
>
> out_be32(&sdma->sdebcr, (u32) sdma_buf_offset & QE_SDEBCR_BA_MASK);
> out_be32(&sdma->sdmr, (QE_SDMR_GLB_1_MSK |
> --
> 1.6.3.3
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox