Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Gross, Mark]

>-----Original Message-----
>From: Eugeny S. Mints [mailto:eugeny.mints@gmail.com]
>Sent: Thursday, July 20, 2006 1:00 PM
>To: linux-pm@lists.osdl.org
>Cc: Matthew Locke; toddpoynor@gmail.com; linux@dominikbrodowski.net;
Gross, Mark;
>igor.stoppa@nokia.com; amit.kucheria@nokia.com;
sampsa.fabritius@nokia.com; r-woodruff2@ti.com;
>Mochel, Patrick
>Subject: Re: [RFC] PowerOP Take 3, sysfs UI core 2/5
>
>Now with patch attached.
>Sorry.
>
>2006/7/20, Eugeny S. Mints <eugeny.mints@gmail.com>:
>> A sysfs interface for PowerOP that allows operating points to be
created
>> and activated from userspace.
>>
>> The platform-specific backend provides the code to read and write
sysfs
>> attributes for each power parameter; the core sysfs interface has no
>> knowledge of the struct powerop_point contents.  This interface could
be
>> seen as possible extension of cpufreq sysfs.  It is not
>> an integral part of PowerOP and is provided in part to facilitate
>> discussion and experimentation with PowerOP, but could serve as a
basis
>> for a basic userspace power policy management stack.
>>
>> Operating points are created by writing the name of the operating
point
>> to /sys/powerop/new.  This may be a job for configfs.
>> /sys/powerop/<op>/ will contain an attribute for each power parameter
>> that may be written to set the associated parameter for the new
>> operating point.  An operating point may be activated by writing its
>> name to /sys/powerop/active.  The hardware power parameters currently
>> set may be read and written via /sys/powerop/hw/, a special operating
>> point that reads and writes parameter attribute values immediately,
>> primarily for diagnostic purposes.
>>
>> Buried in this interface is also the notion of a registry of "named
>> operating points", allowing operating points created by some other
>> interface (such as cpufreq or loading a module with the definitions
as
>> suggested previously by David Brownell) to be activated from
userspace
>> via /sys/powerop/active.
>>
>> Please note that the interface is not hooked up with the rest of the
code
>> yet and is provided just for reference/review.

Drivers/powerop/powerop_sysfs.c : includes asm/powerop.h and will not
compile on i386.
There are other places where it will not compile for non-omap
architectures.  The architecture independent code should build for all
architectures.

Now just rambling a bit, while catching up on the email thread some of
this may be wrong but....

Is it realistic to have the dimensionality of the operating points
defined at the architecture level?  Why couldn't there be multiple types
of operating points based on presence of different peripherals?  Having
the array defined at compile time seems wrong.  Couldn't the platform
startup code enumerate all its power control knobs and define the number
of parameters per operating point, or have one have the dimensions grow
with platform power drivers, one per control knob?

While we are at it, would it make more sense to define the UI for the
operating points in some way that exports mins, max, and target values
for each dimension?

Would it make sense to have the different dimensions broken out into
separate sysfs entries / attributes?

Would it also make sense to support multiple powerop_driver instances?
Powerop.c can only register one powerop_driver instance at a time.  What
if I want to break up my power management into multiple powerop_driver
(power, voltage, clock) domains?  Being stuck defining a large n-tuple
space of operating points at compile time doesn't work for me.  I want
to have them at least built up at startup time, and preferably
associated with some platform power driver loading operation.

--mgross

PS I'm sorry for using outlook on this email thread  I know I'm going to
regret it more than I all ready do....

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Matthew Locke]

On Jul 26, 2006, at 4:55 PM, Gross, Mark wrote:

>
>
>> -----Original Message-----
>> From: Eugeny S. Mints [mailto:eugeny.mints@gmail.com]
>> Sent: Thursday, July 20, 2006 1:00 PM
>> To: linux-pm@lists.osdl.org
>> Cc: Matthew Locke; toddpoynor@gmail.com; linux@dominikbrodowski.net;
> Gross, Mark;
>> igor.stoppa@nokia.com; amit.kucheria@nokia.com;
> sampsa.fabritius@nokia.com; r-woodruff2@ti.com;
>> Mochel, Patrick
>> Subject: Re: [RFC] PowerOP Take 3, sysfs UI core 2/5
>>
>> Now with patch attached.
>> Sorry.
>>
>> 2006/7/20, Eugeny S. Mints <eugeny.mints@gmail.com>:
>>> A sysfs interface for PowerOP that allows operating points to be
> created
>>> and activated from userspace.
>>>
>>> The platform-specific backend provides the code to read and write
> sysfs
>>> attributes for each power parameter; the core sysfs interface has no
>>> knowledge of the struct powerop_point contents.  This interface could
> be
>>> seen as possible extension of cpufreq sysfs.  It is not
>>> an integral part of PowerOP and is provided in part to facilitate
>>> discussion and experimentation with PowerOP, but could serve as a
> basis
>>> for a basic userspace power policy management stack.
>>>
>>> Operating points are created by writing the name of the operating
> point
>>> to /sys/powerop/new.  This may be a job for configfs.
>>> /sys/powerop/<op>/ will contain an attribute for each power parameter
>>> that may be written to set the associated parameter for the new
>>> operating point.  An operating point may be activated by writing its
>>> name to /sys/powerop/active.  The hardware power parameters currently
>>> set may be read and written via /sys/powerop/hw/, a special operating
>>> point that reads and writes parameter attribute values immediately,
>>> primarily for diagnostic purposes.
>>>
>>> Buried in this interface is also the notion of a registry of "named
>>> operating points", allowing operating points created by some other
>>> interface (such as cpufreq or loading a module with the definitions
> as
>>> suggested previously by David Brownell) to be activated from
> userspace
>>> via /sys/powerop/active.
>>>
>>> Please note that the interface is not hooked up with the rest of the
> code
>>> yet and is provided just for reference/review.
>
> Drivers/powerop/powerop_sysfs.c : includes asm/powerop.h and will not
> compile on i386.
> There are other places where it will not compile for non-omap
> architectures.  The architecture independent code should build for all
> architectures.

That should be fixed before the next rev of patches are sent out.

>
> Now just rambling a bit, while catching up on the email thread some of
> this may be wrong but....
>
> Is it realistic to have the dimensionality of the operating points
> defined at the architecture level?  Why couldn't there be multiple 
> types
> of operating points based on presence of different peripherals?  Having
> the array defined at compile time seems wrong.

An operating point definition is the list of system/board power 
parameters.  Generally this means clocks/busses and voltages.  All 
peripherals are connected to a CPU by a bus and therefore a clock, even 
peripherals external to an SoC.  The list of parameters does not change 
when a USB, PCI hotplug or MMC device is plugged in.  The valid values 
of the those parameters may change but not the list of parameters.  The 
device state and device internal power parameters should be (is) 
handled by its driver.  The driver is responsible for changing a 
devices state and internal power parameters based on activity, user 
space input, or another kernel subsystem.

So the operating point is defined at compile time because the 
definition doesn't change.

> Couldn't the platform
> startup code enumerate all its power control knobs and define the 
> number
> of parameters per operating point, or have one have the dimensions grow
> with platform power drivers, one per control knob?

I think there is too much interdependencies between the various knobs 
to control them separately.


> While we are at it, would it make more sense to define the UI for the
> operating points in some way that exports mins, max, and target values
> for each dimension?
>
> Would it make sense to have the different dimensions broken out into
> separate sysfs entries / attributes?
>
> Would it also make sense to support multiple powerop_driver instances?
> Powerop.c can only register one powerop_driver instance at a time.  
> What
> if I want to break up my power management into multiple powerop_driver
> (power, voltage, clock) domains?  Being stuck defining a large n-tuple
> space of operating points at compile time doesn't work for me.  I want
> to have them at least built up at startup time, and preferably
> associated with some platform power driver loading operation.

Not sure about these points.  Again, I think the different areas are 
too connected to control independently.

Summary:
  - Operating point definition is static because it represents the base 
system (board) physical parameters which are clocks and voltages.
  - Devices are connected to the CPU by clocks/busses and therefore are 
represented in the operating point definition that does not change if a 
device is present or not.
  - Valid operating point values may change if a device is present or 
not.
  - Device state and internal power parameters should be controlled by 
the device driver.

>
> --mgross
>
> PS I'm sorry for using outlook on this email thread  I know I'm going 
> to
> regret it more than I all ready do....
>
>

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Pavel Machek]

Hi!

> >> Please note that the interface is not hooked up with the rest of the
> code
> >> yet and is provided just for reference/review.
> 
> Drivers/powerop/powerop_sysfs.c : includes asm/powerop.h and will not
> compile on i386.
> There are other places where it will not compile for non-omap
> architectures.  The architecture independent code should build for all
> architectures.
> 
> Now just rambling a bit, while catching up on the email thread some of
> this may be wrong but....
> 
> Is it realistic to have the dimensionality of the operating points
> defined at the architecture level?  Why couldn't there be multiple types
> of operating points based on presence of different peripherals?  Having
> the array defined at compile time seems wrong.  Couldn't the platform

It is complex enough already, please don't overdesign it even more.

							Pavel
-- 
Thanks for all the (sleeping) penguins.

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Gross, Mark]

>-----Original Message-----
>From: linux-pm-bounces@lists.osdl.org
[mailto:linux-pm-bounces@lists.osdl.org] On Behalf Of Matthew
>Locke
>Sent: Wednesday, July 26, 2006 12:45 AM
>To: David Brownell
>Cc: Mochel, Patrick; Pavel Machek; linux-pm@lists.osdl.org;
sampsa.fabritius@nokia.com;
>linux@dominikbrodowski.net
>Subject: Re: [linux-pm] [RFC] PowerOP Take 3, sysfs UI core 2/5
>
>
>On Jul 24, 2006, at 11:48 AM, David Brownell wrote:
>
>> On Monday 24 July 2006 10:29 am, Pavel Machek wrote:
>>
>>> That looks quite ugly to do in sysfs, indeed.
>>
>> Yes, it's long been one of the things I most dislike about this
>> PowerOP thing.
>> Not just the UI, but the models it reflects.
>>
>> So I was glad to see it split out as fully optional... although from
>> what I
>> see, the internal models in the code have derived from this sysfs
>> model, so
>> I'd argue those need to change too.
>
>Is there something specific in the internal model that are you
>referring to?
>
>>
>> It'd be lots better to just have named operating points that get
>> selected just
>> the /sys/power/state file selects the, erm, "sleep point".
>
>I've tried to fit sleep into the operating point concept before but
>sleep always ends up being a special case.   The code has to check to
>see if the selected operating the "sleep point" and call the suspend
>function.  I think tying in sleep to the op point concept needs more
>discussion and probably should be handled separately until we get
>further along with power op.
>
>btw, we are in complete agreement about using named points.  This first
>set of patches is meant to introduce the operating point concept into
>the kernel without requiring massive changes to cpufreq.  This layer
>extends the bottom part of cpufreq to enable the control of multiple
>parameters beyond just cpu frequency and voltage across architectures.
>Currently, cpufreq does not use named operating points so we need to
>make sure cpufreq can use the power op API and continue to function.
>This topic also ties into our other email discussion on creating op
>points and names are assigned to op points.

I'm sorry if I take things off into a too large of a tangent with this.

I've been wanting to try to get multiple power parameter control some
how integrated with cpufreq for a while now.  Call it an itch and I may
be all wet on the following:  

The way this plugs into the cpufreq code is from the architecture
dependent cpufreq_driver level.  I guess this is an easy first step, but
I wonder if / how something could be brought up to a higher level
somehow.  I don't know what this would look like at the moment.  But,
having a control system (cpufreq + governors) that control a single
variable, cpu frequency, that gets mapped into multi-variable control
(the operating point n-tuple) seems clunky to me.  

Could we do better or more with a multivariable control from the
beginning?

--mgross

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Gross, Mark]

>-----Original Message-----
>From: linux-pm-bounces@lists.osdl.org
[mailto:linux-pm-bounces@lists.osdl.org] On Behalf Of Pavel
>Machek
>Sent: Monday, July 24, 2006 10:30 AM
>To: Eugeny S. Mints
>Cc: Matthew Locke; Mochel, Patrick; linux-pm@lists.osdl.org;
sampsa.fabritius@nokia.com;
>linux@dominikbrodowski.net
>Subject: Re: [linux-pm] [RFC] PowerOP Take 3, sysfs UI core 2/5
>
>Hi!
>
>> A sysfs interface for PowerOP that allows operating points to be
created
>> and activated from userspace.
>>
>> The platform-specific backend provides the code to read and write
sysfs
>> attributes for each power parameter; the core sysfs interface has no
>> knowledge of the struct powerop_point contents.  This interface could
be
>> seen as possible extension of cpufreq sysfs.  It is not
>> an integral part of PowerOP and is provided in part to facilitate
>> discussion and experimentation with PowerOP, but could serve as a
basis
>> for a basic userspace power policy management stack.
>>
>> Operating points are created by writing the name of the operating
point
>> to /sys/powerop/new.  This may be a job for configfs.
>
>That looks quite ugly to do in sysfs, indeed.
>
>And it definitely needs some Documentation/ patch. But better run this
>by lkml.

It's too soon for that.

--mgross

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Gross, Mark]

>-----Original Message-----
>From: Eugeny S. Mints [mailto:eugeny.mints@gmail.com]
>Sent: Thursday, July 20, 2006 12:56 PM
>To: linux-pm@lists.osdl.org
>Cc: Matthew Locke; toddpoynor@gmail.com; linux@dominikbrodowski.net;
Gross, Mark;
>igor.stoppa@nokia.com; amit.kucheria@nokia.com;
sampsa.fabritius@nokia.com; r-woodruff2@ti.com;
>Mochel, Patrick
>Subject: [RFC] PowerOP Take 3, sysfs UI core 2/5
>
>A sysfs interface for PowerOP that allows operating points to be
created
>and activated from userspace.
>
>The platform-specific backend provides the code to read and write sysfs
>attributes for each power parameter; the core sysfs interface has no
>knowledge of the struct powerop_point contents.  This interface could
be
>seen as possible extension of cpufreq sysfs.  It is not
>an integral part of PowerOP and is provided in part to facilitate
>discussion and experimentation with PowerOP, but could serve as a basis
>for a basic userspace power policy management stack.
>
>Operating points are created by writing the name of the operating point
>to /sys/powerop/new.  This may be a job for configfs.

Why create a new top level sysfs entry?  Can't we just put power op
under /sys/devices/platform or /sys/power ?

>/sys/powerop/<op>/ will contain an attribute for each power parameter
>that may be written to set the associated parameter for the new
>operating point.  An operating point may be activated by writing its
>name to /sys/powerop/active.  The hardware power parameters currently
>set may be read and written via /sys/powerop/hw/, a special operating
>point that reads and writes parameter attribute values immediately,
>primarily for diagnostic purposes.

Whaaa?  Is this for creating named operating points like say,
full_speed, deep_idle, pay_back, ui points, that could be activated by
echoing their name to /sys/powerop/active ?

These aren't new points, just named existing ones.  Right?


>Buried in this interface is also the notion of a registry of "named
>operating points", allowing operating points created by some other
>interface (such as cpufreq or loading a module with the definitions as
>suggested previously by David Brownell) to be activated from userspace
>via /sys/powerop/active.

Would it make more sense to echo "active" to the named <op> sysfs node?

>
>Please note that the interface is not hooked up with the rest of the
code
>yet and is provided just for reference/review.

[RFC] PowerOP Take 3, sysfs UI core 2/5

[Eugeny S. Mints]

A sysfs interface for PowerOP that allows operating points to be created
and activated from userspace.

The platform-specific backend provides the code to read and write sysfs
attributes for each power parameter; the core sysfs interface has no
knowledge of the struct powerop_point contents.  This interface could be
seen as possible extension of cpufreq sysfs.  It is not
an integral part of PowerOP and is provided in part to facilitate
discussion and experimentation with PowerOP, but could serve as a basis
for a basic userspace power policy management stack.

Operating points are created by writing the name of the operating point
to /sys/powerop/new.  This may be a job for configfs.
/sys/powerop/<op>/ will contain an attribute for each power parameter
that may be written to set the associated parameter for the new
operating point.  An operating point may be activated by writing its
name to /sys/powerop/active.  The hardware power parameters currently
set may be read and written via /sys/powerop/hw/, a special operating
point that reads and writes parameter attribute values immediately,
primarily for diagnostic purposes.

Buried in this interface is also the notion of a registry of "named
operating points", allowing operating points created by some other
interface (such as cpufreq or loading a module with the definitions as
suggested previously by David Brownell) to be activated from userspace
via /sys/powerop/active.

Please note that the interface is not hooked up with the rest of the code
yet and is provided just for reference/review.

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Eugeny S. Mints]

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

Now with patch attached.
Sorry.

2006/7/20, Eugeny S. Mints <eugeny.mints@gmail.com>:
> A sysfs interface for PowerOP that allows operating points to be created
> and activated from userspace.
>
> The platform-specific backend provides the code to read and write sysfs
> attributes for each power parameter; the core sysfs interface has no
> knowledge of the struct powerop_point contents.  This interface could be
> seen as possible extension of cpufreq sysfs.  It is not
> an integral part of PowerOP and is provided in part to facilitate
> discussion and experimentation with PowerOP, but could serve as a basis
> for a basic userspace power policy management stack.
>
> Operating points are created by writing the name of the operating point
> to /sys/powerop/new.  This may be a job for configfs.
> /sys/powerop/<op>/ will contain an attribute for each power parameter
> that may be written to set the associated parameter for the new
> operating point.  An operating point may be activated by writing its
> name to /sys/powerop/active.  The hardware power parameters currently
> set may be read and written via /sys/powerop/hw/, a special operating
> point that reads and writes parameter attribute values immediately,
> primarily for diagnostic purposes.
>
> Buried in this interface is also the notion of a registry of "named
> operating points", allowing operating points created by some other
> interface (such as cpufreq or loading a module with the definitions as
> suggested previously by David Brownell) to be activated from userspace
> via /sys/powerop/active.
>
> Please note that the interface is not hooked up with the rest of the code
> yet and is provided just for reference/review.
>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: powerop.core.UI.sysfs.interface.patch --]
[-- Type: text/x-patch; name="powerop.core.UI.sysfs.interface.patch", Size: 11213 bytes --]

diff --git a/drivers/powerop/Kconfig b/drivers/powerop/Kconfig
index b252952..ffd7a1b 100644
--- a/drivers/powerop/Kconfig
+++ b/drivers/powerop/Kconfig
@@ -8,5 +8,10 @@ config POWEROP
 	bool "PowerOP Core"
 	help
 
+config POWEROP_SYSFS
+	bool "  Enable PowerOP sysfs interface"
+	depends on POWEROP && SYSFS
+	help
+
 endmenu
 
diff --git a/drivers/powerop/Makefile b/drivers/powerop/Makefile
index 131b983..1d430ce 100644
--- a/drivers/powerop/Makefile
+++ b/drivers/powerop/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_POWEROP)		+= powerop.o
+obj-$(CONFIG_POWEROP_SYSFS)     += powerop_sysfs.o
 
diff --git a/drivers/powerop/powerop_sysfs.c b/drivers/powerop/powerop_sysfs.c
new file mode 100644
index 0000000..73806c2
--- /dev/null
+++ b/drivers/powerop/powerop_sysfs.c
@@ -0,0 +1,398 @@
+/*
+ * PowerOP sysfs UI
+ *
+ * Author: Todd Poynor <tpoynor@mvista.com>
+ *
+ * 2005 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/powerop_sysfs.h>
+#include <linux/init.h>
+#include <linux/kobject.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+
+#include <asm/powerop.h>
+
+int powerop_register_point(const char *id, struct powerop_point *point);
+int powerop_select_point(const char *id);
+
+struct namedop {
+	struct kobject kobj;
+	struct powerop_point *point;
+	struct list_head node;
+	struct completion released;
+};
+
+#define to_namedop(_kobj) container_of(_kobj,\
+	struct namedop,kobj)
+
+static DECLARE_MUTEX(namedop_list_mutex);
+static struct list_head namedop_list;
+static struct namedop *activeop;
+
+struct sysfsop {
+	struct powerop_point point;
+	struct list_head node;
+};
+
+static DECLARE_MUTEX(sysfsop_list_mutex);
+static struct list_head sysfsop_list;
+
+static struct powerop_point *hwop;
+
+#define powerop_attr(_name) \
+static struct subsys_attribute _name##_attr = { \
+	.attr   = {				\
+		.name = __stringify(_name),	\
+		.mode = 0644,			\
+		.owner = THIS_MODULE,		\
+	},					\
+	.show   = _name##_show,                 \
+	.store  = _name##_store,                \
+}
+
+static struct attribute_group param_attr_group;
+
+#define to_powerop_param_attr(_attr) container_of(_attr,\
+	struct powerop_param_attribute,attr)
+
+
+decl_subsys(powerop, NULL, NULL);
+static int subsys_reg;
+static int sysfs_init;
+
+static void namedop_release(struct kobject *kobj)
+{
+	struct namedop *op = to_namedop(kobj);
+
+	complete(&op->released);
+	return;
+}
+
+static struct sysfsop *sysfsop_create(const char *id)
+{
+	struct sysfsop *op;
+	int error;
+
+	if ((op = kmalloc(sizeof(struct sysfsop), GFP_KERNEL)) == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	down(&sysfsop_list_mutex);
+	list_add_tail(&op->node, &sysfsop_list);
+	up(&sysfsop_list_mutex);
+	memset(&op->point, 0, sizeof(struct powerop_point));
+	return (error = powerop_register_point(id, &op->point)) == 0
+		? op : ERR_PTR(error);
+}
+
+static ssize_t
+powerop_param_attr_show(struct kobject * kobj, struct attribute * attr,
+			char * buf)
+{
+	struct powerop_param_attribute * param_attr =
+		to_powerop_param_attr(attr);
+	struct namedop * namedop = to_namedop(kobj);
+	ssize_t ret = 0;
+
+	if (namedop->point == hwop)
+		powerop_get_point(hwop);
+
+	if (param_attr->show)
+		ret = param_attr->show(namedop->point,buf);
+
+	return ret;
+}
+
+static ssize_t
+powerop_param_attr_store(struct kobject * kobj, struct attribute * attr,
+			 const char * buf, size_t count)
+{
+	struct powerop_param_attribute * param_attr =
+		to_powerop_param_attr(attr);
+	struct namedop * namedop = to_namedop(kobj);
+	ssize_t ret = 0;
+
+	if (namedop->point == hwop)
+		powerop_get_point(hwop);
+
+        if (param_attr->store)
+                ret = param_attr->store(namedop->point,buf,count);
+
+	if (namedop->point == hwop)
+		powerop_set_point(hwop);
+
+	return ret;
+}
+
+static struct sysfs_ops namedop_sysfs_ops = {
+	.show	= powerop_param_attr_show,
+	.store	= powerop_param_attr_store,
+};
+
+static struct kobj_type ktype_namedop = {
+	.release        = namedop_release,
+	.sysfs_ops	= &namedop_sysfs_ops,
+};
+
+static ssize_t new_show(struct subsystem * subsys, char * buf)
+{
+	return 0;
+}
+
+static ssize_t new_store(struct subsystem * subsys, const char * buf,
+			 size_t n)
+{
+	struct sysfsop *op;
+
+	return IS_ERR(op = sysfsop_create(buf)) ? PTR_ERR(op) : n;
+}
+
+
+powerop_attr(new);
+
+static ssize_t active_show(struct subsystem * subsys, char * buf)
+{
+	int ret = 0;
+
+	down(&namedop_list_mutex);
+	if (activeop)
+		ret = sprintf(buf, "%s\n", activeop->kobj.name);
+	up(&namedop_list_mutex);
+
+	return ret;
+}
+
+static ssize_t active_store(struct subsystem * subsys, const char * buf,
+			    size_t n)
+{
+	int error;
+
+	return (error = powerop_select_point(buf)) == 0 ? n : error;
+}
+
+powerop_attr(active);
+
+static struct attribute * g[] = {
+	&new_attr.attr,
+	&active_attr.attr,
+	NULL,
+};
+
+static struct attribute_group attr_group = {
+	.attrs = g,
+};
+
+
+
+static int create_namedop_attrs(struct namedop *op)
+{
+	int error = 0;
+
+	if (param_attr_group.attrs)
+		if ((error = sysfs_create_group(&op->kobj,
+						&param_attr_group)))
+			printk(KERN_ERR
+			       "sysfs_create_group for op %s failed.\n",
+			       op->kobj.name);
+	return error;
+}
+
+static void remove_namedop_attrs(struct namedop *op)
+{
+	if (param_attr_group.attrs)
+		sysfs_remove_group(&op->kobj, &param_attr_group);
+}
+
+int powerop_register_point(const char *id, struct powerop_point *point)
+{
+	struct namedop *op;
+	int error;
+
+	if ((op = kmalloc(sizeof(struct namedop), GFP_KERNEL)) == NULL)
+		return -ENOMEM;
+
+	memset(op, 0, sizeof(struct namedop));
+	kobject_set_name(&op->kobj, id);
+	op->point = point;
+
+	op->kobj.ktype = &ktype_namedop;
+	op->kobj.kset = &powerop_subsys.kset;
+	init_completion(&op->released);
+
+	if ((error = kobject_register(&op->kobj))) {
+		printk(KERN_ERR
+		       "PowerOP kobject_register for op %s failed.\n",
+		       id);
+		kfree(op);
+		return error;
+	}
+
+	create_namedop_attrs(op);
+	down(&namedop_list_mutex);
+	list_add_tail(&op->node, &namedop_list);
+	up(&namedop_list_mutex);
+	return 0;
+}
+
+
+static void remove_namedop(struct namedop *op)
+{
+	list_del(&op->node);
+	remove_namedop_attrs(op);
+	kobject_unregister(&op->kobj);
+	wait_for_completion(&op->released);
+	kfree(op);
+}
+
+int powerop_unregister_point(const char *id)
+{
+	struct namedop *op, *tmpop;
+	int ret = -EINVAL;
+
+	down(&namedop_list_mutex);
+
+	list_for_each_entry_safe(op, tmpop, &namedop_list, node) {
+		if (strcmp(op->kobj.name, id) == 0) {
+			remove_namedop(op);
+			ret = 0;
+			break;
+		}
+	}
+
+	up(&namedop_list_mutex);
+	return ret;
+}
+
+int powerop_select_point(const char *id)
+{
+	struct namedop *op, *selectedop = NULL;
+	int ret;
+	down(&namedop_list_mutex);
+
+	list_for_each_entry(op, &namedop_list, node) {
+		if (strcmp(op->kobj.name, id) == 0) {
+			selectedop = op;
+			break;
+		}
+	}
+
+	ret = (selectedop == NULL) ?
+		-EINVAL : powerop_set_point(op->point);
+
+	if (ret == 0)
+		activeop = selectedop;
+
+	up(&namedop_list_mutex);
+	return ret;
+}
+
+EXPORT_SYMBOL_GPL(powerop_register_point);
+EXPORT_SYMBOL_GPL(powerop_unregister_point);
+EXPORT_SYMBOL_GPL(powerop_select_point);
+
+int powerop_sysfs_register(struct attribute **param_attrs)
+{
+	struct namedop *namedop;
+
+	if (param_attr_group.attrs)
+		return -EBUSY;
+
+	param_attr_group.attrs = param_attrs;
+
+	if (! sysfs_init)
+		return 0;
+
+	down(&namedop_list_mutex);
+	list_for_each_entry(namedop, &namedop_list, node)
+		create_namedop_attrs(namedop);
+        up(&namedop_list_mutex);
+	return 0;
+}
+
+void powerop_sysfs_unregister(struct attribute **param_attrs)
+{
+	struct namedop *namedop;
+
+	if ((param_attr_group.attrs != param_attrs) || !sysfs_init)
+		return;
+
+	down(&namedop_list_mutex);
+	list_for_each_entry(namedop, &namedop_list, node)
+		remove_namedop_attrs(namedop);
+        up(&namedop_list_mutex);
+
+	param_attr_group.attrs = NULL;
+}
+
+EXPORT_SYMBOL_GPL(powerop_sysfs_register);
+EXPORT_SYMBOL_GPL(powerop_sysfs_unregister);
+
+static int __init powerop_sysfs_init(void)
+{
+	struct sysfsop *sysfsop;
+        int error;
+
+	if ((error = subsystem_register(&powerop_subsys))) {
+		printk(KERN_ERR
+		       "PowerOP SysFS subsystem_register failed.\n");
+		return error;
+	}
+
+	subsys_reg = 1;
+
+	if ((error =
+	     sysfs_create_group(&powerop_subsys.kset.kobj,&attr_group))) {
+		printk(KERN_ERR
+		       "PowerOP subsys sysfs_create_group failed.\n");
+		return error;
+	}
+
+	INIT_LIST_HEAD(&namedop_list);
+	INIT_LIST_HEAD(&sysfsop_list);
+
+	if (! IS_ERR(sysfsop = sysfsop_create("hw")))
+		hwop = &sysfsop->point;
+
+	sysfs_init = 1;
+        return 0;
+}
+
+static void __exit powerop_sysfs_exit(void)
+{
+	struct namedop *namedop, *tnamedop;
+	struct sysfsop *sysfsop, *tsysfsop;
+
+	powerop_sysfs_unregister(param_attr_group.attrs);
+
+	down(&namedop_list_mutex);
+	list_for_each_entry_safe(namedop, tnamedop, &namedop_list, node)
+		remove_namedop(namedop);
+        up(&namedop_list_mutex);
+
+	down(&sysfsop_list_mutex);
+	list_for_each_entry_safe(sysfsop, tsysfsop, &sysfsop_list, node) {
+		list_del(&sysfsop->node);
+		kfree(sysfsop);
+	}
+	up(&sysfsop_list_mutex);
+
+	if (subsys_reg) {
+		sysfs_remove_group(&powerop_subsys.kset.kobj,&attr_group);
+		subsystem_unregister(&powerop_subsys);
+	}
+}
+
+module_init(powerop_sysfs_init);
+module_exit(powerop_sysfs_exit);
+
+MODULE_DESCRIPTION("PowerOP Power Management SysFS UI");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/powerop_sysfs.h b/include/linux/powerop_sysfs.h
new file mode 100644
index 0000000..81f1537
--- /dev/null
+++ b/include/linux/powerop_sysfs.h
@@ -0,0 +1,41 @@
+/*
+ * PowerOP SysFS UI
+ *
+ * Author: Todd Poynor <tpoynor@mvista.com>
+ *
+ * 2005 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#ifndef __POWEROP_SYSFS_H__
+#define __POWEROP_SYSFS_H__
+
+#include <linux/sysfs.h>
+#include <linux/kobject.h>
+#include <asm/powerop.h>
+
+struct powerop_param_attribute {
+        struct attribute        attr;
+        ssize_t (*show)(struct powerop_point *op, char * buf);
+        ssize_t (*store)(struct powerop_point *op, const char * buf, size_t count);};
+
+#define powerop_param_attr(_name) \
+static struct powerop_param_attribute _name##_attr = { \
+	.attr   = {				\
+		.name = __stringify(_name),	\
+		.mode = 0644,			\
+		.owner = THIS_MODULE,		\
+	},					\
+	.show   = _name##_show,			\
+	.store  = _name##_store,		\
+}
+
+int powerop_register_point(const char *id, struct powerop_point *point);
+int powerop_unregister_point(const char *id);
+int powerop_select_point(const char *id);
+int powerop_sysfs_register(struct attribute **param_attrs);
+void powerop_sysfs_unregister(struct attribute **param_attrs);
+
+#endif /*__POWEROP_SYSFS_H__*/

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Pavel Machek]

Hi!

> A sysfs interface for PowerOP that allows operating points to be created
> and activated from userspace.
> 
> The platform-specific backend provides the code to read and write sysfs
> attributes for each power parameter; the core sysfs interface has no
> knowledge of the struct powerop_point contents.  This interface could be
> seen as possible extension of cpufreq sysfs.  It is not
> an integral part of PowerOP and is provided in part to facilitate
> discussion and experimentation with PowerOP, but could serve as a basis
> for a basic userspace power policy management stack.
> 
> Operating points are created by writing the name of the operating point
> to /sys/powerop/new.  This may be a job for configfs.

That looks quite ugly to do in sysfs, indeed.

And it definitely needs some Documentation/ patch. But better run this
by lkml.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[David Brownell]

On Monday 24 July 2006 10:29 am, Pavel Machek wrote:

> That looks quite ugly to do in sysfs, indeed.

Yes, it's long been one of the things I most dislike about this PowerOP thing.
Not just the UI, but the models it reflects.

So I was glad to see it split out as fully optional... although from what I
see, the internal models in the code have derived from this sysfs model, so
I'd argue those need to change too.

It'd be lots better to just have named operating points that get selected just
the /sys/power/state file selects the, erm, "sleep point".


> But better run this by lkml.

I'd rather see some rough consensus on this list that this is the right way
to head, before running things like this by LKML.  

- Dave

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Pavel Machek]

> > But better run this by lkml.
> 
> I'd rather see some rough consensus on this list that this is the right way
> to head, before running things like this by LKML.  

I meant "they could suggest how to do the sysfs thing, in reasonable
way". Like echo new_config > file is extermely ugly, but perhaps configfs
is suitable?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Matthew Locke]

On Jul 24, 2006, at 12:35 PM, Pavel Machek wrote:

>
>>> But better run this by lkml.
>>
>> I'd rather see some rough consensus on this list that this is the 
>> right way
>> to head, before running things like this by LKML.
>
> I meant "they could suggest how to do the sysfs thing, in reasonable
> way". Like echo new_config > file is extermely ugly, but perhaps 
> configfs
> is suitable?

Ah, Is that what you consider ugly, that each parameter is a sysfs file 
and the values have to be written to a file to set it up?

We can look into configfs and ask lkml (or specific people) for 
recommendations on the best mechanism to use.

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[David Brownell]

On Monday 24 July 2006 12:35 pm, Pavel Machek wrote:
> 
> > > But better run this by lkml.
> > 
> > I'd rather see some rough consensus on this list that this is the right way
> > to head, before running things like this by LKML.  
> 
> I meant "they could suggest how to do the sysfs thing, in reasonable
> way". Like echo new_config > file is extermely ugly, but perhaps configfs
> is suitable?

Makes some sense.  But I'm still puzzled why _creating_ an operating point
would be done outside of the arch/.../board-xx.c file. 

- Dave

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Preece Scott-PREECE]

If they're defined dynamically, you can change them without recompiling
the system, building a new rootfs image, etc. This is especially useful
during development and tuning of systems built on new hardware, since
the set of Ops available (that is, that are documented by the chip
vendor to work) can vary over time and even board-to-board.

scott

-----Original Message-----
From: linux-pm-bounces@lists.osdl.org
[mailto:linux-pm-bounces@lists.osdl.org] On Behalf Of David Brownell
Sent: Monday, July 24, 2006 4:47 PM
To: linux-pm@lists.osdl.org
Cc: Matthew Locke; patrick.mochel@intel.com; linux@dominikbrodowski.net;
sampsa.fabritius@nokia.com; Pavel Machek
Subject: Re: [linux-pm] [RFC] PowerOP Take 3, sysfs UI core 2/5

On Monday 24 July 2006 12:35 pm, Pavel Machek wrote:
> 
> > > But better run this by lkml.
> > 
> > I'd rather see some rough consensus on this list that this is the 
> > right way to head, before running things like this by LKML.
> 
> I meant "they could suggest how to do the sysfs thing, in reasonable 
> way". Like echo new_config > file is extermely ugly, but perhaps 
> configfs is suitable?

Makes some sense.  But I'm still puzzled why _creating_ an operating
point would be done outside of the arch/.../board-xx.c file. 

- Dave

_______________________________________________
linux-pm mailing list
linux-pm@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/linux-pm

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[David Brownell]

On Monday 24 July 2006 2:58 pm, Preece Scott-PREECE wrote:
> If they're defined dynamically, you can change them without recompiling
> the system, building a new rootfs image, etc. This is especially useful
> during development and tuning of systems built on new hardware, since
> the set of Ops available (that is, that are documented by the chip
> vendor to work) can vary over time and even board-to-board.

I could easily buy such a mechanism being dependent on EXPERIMENTAL,
for use with developer/prototype boards ... thanks for that scenario.

But I have a harder time seeing it used in production systems, burnt
into flash on a manufacturing line that already had to qualify that
new hardware before the next production run (of say 10,000 units) was
approved by the powers-that-be.

- Dave


> > I meant "they could suggest how to do the sysfs thing, in reasonable 
> > way". Like echo new_config > file is extermely ugly, but perhaps 
> > configfs is suitable?
> 
> Makes some sense.  But I'm still puzzled why _creating_ an operating
> point would be done outside of the arch/.../board-xx.c file. 

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Amit Kucheria]

On Mon, 2006-07-24 at 17:32 -0700, ext David Brownell wrote:
> On Monday 24 July 2006 2:58 pm, Preece Scott-PREECE wrote:
> > If they're defined dynamically, you can change them without recompiling
> > the system, building a new rootfs image, etc. This is especially useful
> > during development and tuning of systems built on new hardware, since
> > the set of Ops available (that is, that are documented by the chip
> > vendor to work) can vary over time and even board-to-board.
> 
> I could easily buy such a mechanism being dependent on EXPERIMENTAL,
> for use with developer/prototype boards ... thanks for that scenario.
> 
> But I have a harder time seeing it used in production systems, burnt
> into flash on a manufacturing line that already had to qualify that
> new hardware before the next production run (of say 10,000 units) was
> approved by the powers-that-be.
> 
> - Dave

Sometimes a certain operating point is not desired for regular operation
of the device. So it would not be in the board-xx.c file. 

But connect a peripheral and suddenly this is the most attractive OP for
the system. So the ability to add operating points from userspace might
be helpful there.

Regards,
Amit

> 
> > > I meant "they could suggest how to do the sysfs thing, in reasonable 
> > > way". Like echo new_config > file is extermely ugly, but perhaps 
> > > configfs is suitable?
> > 
> > Makes some sense.  But I'm still puzzled why _creating_ an operating
> > point would be done outside of the arch/.../board-xx.c file. 
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/linux-pm
-- 
Amit Kucheria <amit.kucheria@nokia.com>
Nokia

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[David Brownell]

On Tuesday 25 July 2006 3:09 am, Amit Kucheria wrote:
> On Mon, 2006-07-24 at 17:32 -0700, ext David Brownell wrote:
> > On Monday 24 July 2006 2:58 pm, Preece Scott-PREECE wrote:
> > > If they're defined dynamically, you can change them without recompiling
> > > the system, building a new rootfs image, etc. This is especially useful
> > > during development and tuning of systems built on new hardware, since
> > > the set of Ops available (that is, that are documented by the chip
> > > vendor to work) can vary over time and even board-to-board.
> > 
> > I could easily buy such a mechanism being dependent on EXPERIMENTAL,
> > for use with developer/prototype boards ... thanks for that scenario.
> > 
> > But I have a harder time seeing it used in production systems, burnt
> > into flash on a manufacturing line that already had to qualify that
> > new hardware before the next production run (of say 10,000 units) was
> > approved by the powers-that-be.
> > 
> > - Dave
> 
> Sometimes a certain operating point is not desired for regular operation
> of the device. So it would not be in the board-xx.c file. 
> 
> But connect a peripheral and suddenly this is the most attractive OP for
> the system.

And you'd know that in advance, and thus could predefine that OP.  :)

Even in the worst case, where you somehow add a driver without
upgrading the rest of the kernel in flash, that driver should be
able to define an OP without userspace.  (I suppose there are some
product vendors willing to do that type of field upgrade.)

I'm still not persuaded that a UI for OP creation is needed except
for development.  Feel free to keep trying to persuade me though;
I'm just pushing back on what I see as weak points, since that's
the best way I know to come up with good solutions.

- Dave


> So the ability to add operating points from userspace might 
> be helpful there.
> 
> Regards,
> Amit
> 
> > 
> > > > I meant "they could suggest how to do the sysfs thing, in reasonable 
> > > > way". Like echo new_config > file is extermely ugly, but perhaps 
> > > > configfs is suitable?
> > > 
> > > Makes some sense.  But I'm still puzzled why _creating_ an operating
> > > point would be done outside of the arch/.../board-xx.c file. 
> > _______________________________________________
> > linux-pm mailing list
> > linux-pm@lists.osdl.org
> > https://lists.osdl.org/mailman/listinfo/linux-pm
> -- 
> Amit Kucheria <amit.kucheria@nokia.com>
> Nokia
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/linux-pm
> 

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Matthew Locke]

Here is the use case we are picturing:

The semiconductor vendor creates the full list of operating points 
supported on a SoC (and reference board).  A device (cell phone, etc) 
manufacturer decides to use only a subset of the full operating points 
due to their use cases and board design.   We want to enable the 
semiconductor vendor and device manufacturer to easily update the 
operating points and selectively enable ones for their use in a way 
that doesn't require maintaining out of tree patches or submitting 
patches for every update and every device made.

We came up with the idea of creating operating points at run-time 
(initscript at boot usually) to address this scenario.   Hard coding a 
list operating points in the kernel is fine as long as there is an 
interface to override and/or extend that list.

Another use case for creating operating points is to enable device 
manufacturers and others to create a custom name for the  operating 
point.  The entire operating point does not need to be created to 
address this case but we would need a method for creating a mapping of 
points to names.

On Jul 25, 2006, at 10:05 PM, David Brownell wrote:

> On Tuesday 25 July 2006 3:09 am, Amit Kucheria wrote:
>> On Mon, 2006-07-24 at 17:32 -0700, ext David Brownell wrote:
>>> On Monday 24 July 2006 2:58 pm, Preece Scott-PREECE wrote:
>>>> If they're defined dynamically, you can change them without 
>>>> recompiling
>>>> the system, building a new rootfs image, etc. This is especially 
>>>> useful
>>>> during development and tuning of systems built on new hardware, 
>>>> since
>>>> the set of Ops available (that is, that are documented by the chip
>>>> vendor to work) can vary over time and even board-to-board.
>>>
>>> I could easily buy such a mechanism being dependent on EXPERIMENTAL,
>>> for use with developer/prototype boards ... thanks for that scenario.
>>>
>>> But I have a harder time seeing it used in production systems, burnt
>>> into flash on a manufacturing line that already had to qualify that
>>> new hardware before the next production run (of say 10,000 units) was
>>> approved by the powers-that-be.
>>>
>>> - Dave
>>
>> Sometimes a certain operating point is not desired for regular 
>> operation
>> of the device. So it would not be in the board-xx.c file.
>>
>> But connect a peripheral and suddenly this is the most attractive OP 
>> for
>> the system.
>
> And you'd know that in advance, and thus could predefine that OP.  :)
>
> Even in the worst case, where you somehow add a driver without
> upgrading the rest of the kernel in flash, that driver should be
> able to define an OP without userspace.  (I suppose there are some
> product vendors willing to do that type of field upgrade.)
>
> I'm still not persuaded that a UI for OP creation is needed except
> for development.  Feel free to keep trying to persuade me though;
> I'm just pushing back on what I see as weak points, since that's
> the best way I know to come up with good solutions.
>
> - Dave
>
>
>> So the ability to add operating points from userspace might
>> be helpful there.
>>
>> Regards,
>> Amit
>>
>>>
>>>>> I meant "they could suggest how to do the sysfs thing, in 
>>>>> reasonable
>>>>> way". Like echo new_config > file is extermely ugly, but perhaps
>>>>> configfs is suitable?
>>>>
>>>> Makes some sense.  But I'm still puzzled why _creating_ an operating
>>>> point would be done outside of the arch/.../board-xx.c file.
>>> _______________________________________________
>>> linux-pm mailing list
>>> linux-pm@lists.osdl.org
>>> https://lists.osdl.org/mailman/listinfo/linux-pm
>> -- 
>> Amit Kucheria <amit.kucheria@nokia.com>
>> Nokia
>> _______________________________________________
>> linux-pm mailing list
>> linux-pm@lists.osdl.org
>> https://lists.osdl.org/mailman/listinfo/linux-pm
>>
>

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Pavel Machek]

Hi!

> Here is the use case we are picturing:
> 
> The semiconductor vendor creates the full list of operating points 
> supported on a SoC (and reference board).  A device (cell phone, etc) 
> manufacturer decides to use only a subset of the full operating points 
> due to their use cases and board design.   We want to enable the 
> semiconductor vendor and device manufacturer to easily update the 
> operating points and selectively enable ones for their use in a way 
> that doesn't require maintaining out of tree patches or submitting 
> patches for every update and every device made.

Thats more complex than it needs to be. Easy solution is to allways
compile in full set from semiconductor vendor and then maybe only use
subset.

'Changing kernel is hard, lets do it from initscripts' is perverse
reason for overdesign... (And btw it should be kernel doing the
hardware abstraction, you are turning things upside down).
						Pavel
-- 
Thanks for all the (sleeping) penguins.

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Vitaly Wool]

> Thats more complex than it needs to be. Easy solution is to allways
> compile in full set from semiconductor vendor and then maybe only use
> subset.

Completely disagree.
That solution might be easy from a first glance, but you definitely
not seeing the whole picture.
Let's think a bit how it all should work from the embedded POV.
For a SoC, there can be different use cases, like GPS navigator,
GSM/GPRS phone, WiFi phone, MP3 player and so on. For EACH use case
(which is usually denoted as a 'policy'), there will be different sets
of devices enabled and sets of operating points valid for this use
case.
Given that the operating points are defined in kernel, how the
userspace application managing operating point transitions (power
management daemon) will work? The situation would be rather ridiculous
in this case: the description of valid OPs for a policy and OP
transitions will be separated from the description of OPs themselves,
and it would lead to a lot of redundant code and pretty unclear
overall structure.
So, the bottom line is: all OPs compiled in kernel and no runtime OPs
creation is the idea that is *orthogonal* to the dynamic power
management concept.

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Eugeny S. Mints]

David Brownell wrote:
> On Tuesday 25 July 2006 3:09 am, Amit Kucheria wrote:
>   
>> On Mon, 2006-07-24 at 17:32 -0700, ext David Brownell wrote:
>>     
>>> On Monday 24 July 2006 2:58 pm, Preece Scott-PREECE wrote:
>>>       
>>>> If they're defined dynamically, you can change them without recompiling
>>>> the system, building a new rootfs image, etc. This is especially useful
>>>> during development and tuning of systems built on new hardware, since
>>>> the set of Ops available (that is, that are documented by the chip
>>>> vendor to work) can vary over time and even board-to-board.
>>>>         
>>> I could easily buy such a mechanism being dependent on EXPERIMENTAL,
>>> for use with developer/prototype boards ... thanks for that scenario.
>>>
>>> But I have a harder time seeing it used in production systems, burnt
>>> into flash on a manufacturing line that already had to qualify that
>>> new hardware before the next production run (of say 10,000 units) was
>>> approved by the powers-that-be.
>>>
>>> - Dave
>>>       
>> Sometimes a certain operating point is not desired for regular operation
>> of the device. So it would not be in the board-xx.c file. 
>>
>> But connect a peripheral and suddenly this is the most attractive OP for
>> the system.
>>     
>
> And you'd know that in advance, and thus could predefine that OP.  :)
>
> Even in the worst case, where you somehow add a driver without
> upgrading the rest of the kernel in flash, that driver should be
> able to define an OP without userspace.  (I suppose there are some
> product vendors willing to do that type of field upgrade.)
>
> I'm still not persuaded that a UI for OP creation is needed except
> for development.  Feel free to keep trying to persuade me though;
> I'm just pushing back on what I see as weak points, since that's
> the best way I know to come up with good solutions.
>   
I'd like to highlight the only thing that PowerOP sysfs layer provides
two interfaces for operating points creation - one is UI and another is
powerop_register_point/select_point() and the latter is intended to be
utilized by a kernel entity. (Btw, please note that select routine receives
'name' argument)

This may be up to a system designer to define which interface to use
therefore I see it as an advantage of PowerOP rather than as a weak
point.

Since UI part seems most painful one I can think of additional splitting
of PowerOP sysfs layer into two parts. First would be
powerop_register/unregister_point(), powerop_select_point() and
another one would be UI sysfs part. And the latter would be optional.

Thanks,
Eugeny
> - Dave
>
>
>   
>> So the ability to add operating points from userspace might 
>> be helpful there.
>>
>> Regards,
>> Amit
>>
>>     
>>>>> I meant "they could suggest how to do the sysfs thing, in reasonable 
>>>>> way". Like echo new_config > file is extermely ugly, but perhaps 
>>>>> configfs is suitable?
>>>>>           
>>>> Makes some sense.  But I'm still puzzled why _creating_ an operating
>>>> point would be done outside of the arch/.../board-xx.c file. 
>>>>         
>>> _______________________________________________
>>> linux-pm mailing list
>>> linux-pm@lists.osdl.org
>>> https://lists.osdl.org/mailman/listinfo/linux-pm
>>>       
>> -- 
>> Amit Kucheria <amit.kucheria@nokia.com>
>> Nokia
>> _______________________________________________
>> linux-pm mailing list
>> linux-pm@lists.osdl.org
>> https://lists.osdl.org/mailman/listinfo/linux-pm
>>
>>     
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/linux-pm
>
>   

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[David Brownell]

On Wednesday 26 July 2006 2:11 pm, Eugeny S. Mints wrote:

> Since UI part seems most painful one I can think of additional splitting
> of PowerOP sysfs layer into two parts. First would be
> powerop_register/unregister_point(), powerop_select_point() and
> another one would be UI sysfs part. And the latter would be optional.

Well, I think it'd be fair to say that something like /sys/power/state
appling to an OP belongs in the core of that framework.  Luckily that
would be straightforward to implmenent, explain, and understand.

- Dave

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Matthew Locke]

On Jul 24, 2006, at 11:48 AM, David Brownell wrote:

> On Monday 24 July 2006 10:29 am, Pavel Machek wrote:
>
>> That looks quite ugly to do in sysfs, indeed.
>
> Yes, it's long been one of the things I most dislike about this 
> PowerOP thing.
> Not just the UI, but the models it reflects.
>
> So I was glad to see it split out as fully optional... although from 
> what I
> see, the internal models in the code have derived from this sysfs 
> model, so
> I'd argue those need to change too.

Is there something specific in the internal model that are you 
referring to?

>
> It'd be lots better to just have named operating points that get 
> selected just
> the /sys/power/state file selects the, erm, "sleep point".

I've tried to fit sleep into the operating point concept before but 
sleep always ends up being a special case.   The code has to check to 
see if the selected operating the "sleep point" and call the suspend 
function.  I think tying in sleep to the op point concept needs more 
discussion and probably should be handled separately until we get 
further along with power op.

btw, we are in complete agreement about using named points.  This first 
set of patches is meant to introduce the operating point concept into 
the kernel without requiring massive changes to cpufreq.  This layer 
extends the bottom part of cpufreq to enable the control of multiple 
parameters beyond just cpu frequency and voltage across architectures.  
Currently, cpufreq does not use named operating points so we need to 
make sure cpufreq can use the power op API and continue to function.  
This topic also ties into our other email discussion on creating op 
points and names are assigned to op points.
>
>
>> But better run this by lkml.
>
> I'd rather see some rough consensus on this list that this is the 
> right way
> to head, before running things like this by LKML.

That is why we submitted here first:)  We will figure out if there is a 
better interface for this patch to use by discussing with Greg or LKML. 
  Meanwhile let's continue to discuss the ideas here for a bit.
>
> - Dave
>

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Christian Krafft]

Matthew Locke <matt@nomadgs.com> wrote on 07/26/2006 09:44:52 AM:

> btw, we are in complete agreement about using named points.  This first
> set of patches is meant to introduce the operating point concept into
> the kernel without requiring massive changes to cpufreq.  This layer
> extends the bottom part of cpufreq to enable the control of multiple
> parameters beyond just cpu frequency and voltage across architectures.
> Currently, cpufreq does not use named operating points so we need to
> make sure cpufreq can use the power op API and continue to function.
> This topic also ties into our other email discussion on creating op
> points and names are assigned to op points.

Hi,

Maybe I missed those discussions about extending cpufreq with other
attributes.
On what list has it been posted ?

I am working on a throttling module and would like to implement it as a
cpufreq driver.
Also closer integration of the on-demand governor and the cpu scheduler
(and hopefully thermal control) would be nice.
This governor uses aged information to control cpus, whereas the scheduler
knows the future.
Thermal control could hint the scheduler, before a cpu gets too hot and
throttled down.
Are there any ideas floating around ?

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[David Brownell]

On Wednesday 26 July 2006 12:44 am, Matthew Locke wrote:
> 
> On Jul 24, 2006, at 11:48 AM, David Brownell wrote:
> 
> > So I was glad to see it split out as fully optional... although from 
> > what I
> > see, the internal models in the code have derived from this sysfs 
> > model, so
> > I'd argue those need to change too.
> 
> Is there something specific in the internal model that are you 
> referring to?

I touched on some of the issues in earlier emails to Eugeny.

One is that the struct declaration seems too monolithic to
handle board-specific differences well.  Related to that is the
way things are just packaged as a set/struct of numbers, rather
than a set of objects/components with internal rules/models
(which might be individually reusable, e.g. "SOC X").

Maybe another way to think of it is that this sysfs stuff defines
some structural notions that seem to be artifacts of one style
for creating operating points ... and I'd be more comfortable
deriving those structural notions by addressing requirements of
specific board families (not just SOCs!).  And _then_ coming up
with a sysfs model to suit.


> > It'd be lots better to just have named operating points that get 
> > selected just
> > the /sys/power/state file selects the, erm, "sleep point".
> 
> I've tried to fit sleep into the operating point concept before but 
> sleep always ends up being a special case.   The code has to check to 
> see if the selected operating the "sleep point" and call the suspend 
> function.  I think tying in sleep to the op point concept needs more 
> discussion and probably should be handled separately until we get 
> further along with power op.

I'm comfortable with discussing sleep point separately from operating
points, so long as everyone understands how fuzzy a line that is.  In
a system with dedicated asymmetric CPUs for example, is "this cpu is
asleep and that one is running" a sleep state?  Or an operating point?
Ditto with hardware components that may be running or not, and don't
get to brag about some fancy instruction set.  :)

In fact, I find it maybe easier to think of "run states" and "sleep
states", where a "run state" would include a collection of specific
operating points and some mechanism to select some operating point
to activate.  I don't think that notion of "operating point" is what
you are talking about here with PowerOP, though...


> btw, we are in complete agreement about using named points.  This first 
> set of patches is meant to introduce the operating point concept into 
> the kernel without requiring massive changes to cpufreq.  This layer 
> extends the bottom part of cpufreq to enable the control of multiple 
> parameters beyond just cpu frequency and voltage across architectures.

I guess I haven't read the patches enough to see how it extends that.


> Currently, cpufreq does not use named operating points so we need to 
> make sure cpufreq can use the power op API and continue to function.  
> This topic also ties into our other email discussion on creating op 
> points and names are assigned to op points.

Hmm, I'll be watching to see how that works out.  I keep thinking that
in the big picture, a "governor" type component may not map well to
choosing operating points ... though a given component might well use
such a mechanism internally.

- Dave

Re: [RFC] PowerOP Take 3, sysfs UI core 2/5

[Matthew Locke]

On Jul 26, 2006, at 5:55 PM, David Brownell wrote:

> On Wednesday 26 July 2006 12:44 am, Matthew Locke wrote:
>>
>> On Jul 24, 2006, at 11:48 AM, David Brownell wrote:
>>
>>> So I was glad to see it split out as fully optional... although from
>>> what I
>>> see, the internal models in the code have derived from this sysfs
>>> model, so
>>> I'd argue those need to change too.
>>
>> Is there something specific in the internal model that are you
>> referring to?
>
> I touched on some of the issues in earlier emails to Eugeny.
>
> One is that the struct declaration seems too monolithic to
> handle board-specific differences well.  Related to that is the
> way things are just packaged as a set/struct of numbers, rather
> than a set of objects/components with internal rules/models
> (which might be individually reusable, e.g. "SOC X").
>
> Maybe another way to think of it is that this sysfs stuff defines
> some structural notions that seem to be artifacts of one style
> for creating operating points ... and I'd be more comfortable
> deriving those structural notions by addressing requirements of
> specific board families (not just SOCs!).  And _then_ coming up
> with a sysfs model to suit.
>

Got it.  I'll let Eugeny respond on this one.

>
>>> It'd be lots better to just have named operating points that get
>>> selected just
>>> the /sys/power/state file selects the, erm, "sleep point".
>>
>> I've tried to fit sleep into the operating point concept before but
>> sleep always ends up being a special case.   The code has to check to
>> see if the selected operating the "sleep point" and call the suspend
>> function.  I think tying in sleep to the op point concept needs more
>> discussion and probably should be handled separately until we get
>> further along with power op.
>
> I'm comfortable with discussing sleep point separately from operating
> points, so long as everyone understands how fuzzy a line that is.  In
> a system with dedicated asymmetric CPUs for example, is "this cpu is
> asleep and that one is running" a sleep state?  Or an operating point?
> Ditto with hardware components that may be running or not, and don't
> get to brag about some fancy instruction set.  :)

It is very fuzzy.  If we can make it fit without forcing it,  I think 
it would be a very nice way to define the various sleep points 
supported on the SoC's.

>
> In fact, I find it maybe easier to think of "run states" and "sleep
> states", where a "run state" would include a collection of specific
> operating points and some mechanism to select some operating point
> to activate.  I don't think that notion of "operating point" is what
> you are talking about here with PowerOP, though...

PowerOP is the definition of the operating point and the interface to 
set one.  I actually think the same way as you describe.  There are run 
states and sleep states.   As the system changes "states"  it will go 
to an operating point.  We are not including the state changing or 
definition with PowerOp just the mechanism to list and select the 
points.  A component(s) of top of PowerOP would decide which operating 
points belong to a run state and when to change. Examples of this could 
be as simple as  /sys/power/state, a more complex cpufreq 
governor/policy or something new.



>
>
>> btw, we are in complete agreement about using named points.  This 
>> first
>> set of patches is meant to introduce the operating point concept into
>> the kernel without requiring massive changes to cpufreq.  This layer
>> extends the bottom part of cpufreq to enable the control of multiple
>> parameters beyond just cpu frequency and voltage across architectures.
>
> I guess I haven't read the patches enough to see how it extends that.

>
>
>> Currently, cpufreq does not use named operating points so we need to
>> make sure cpufreq can use the power op API and continue to function.
>> This topic also ties into our other email discussion on creating op
>> points and names are assigned to op points.
>
> Hmm, I'll be watching to see how that works out.  I keep thinking that
> in the big picture, a "governor" type component may not map well to
> choosing operating points ... though a given component might well use
> such a mechanism internally.
>
> - Dave
>