[PATCH] PowerOP, PowerOP Core, 1/2

[PATCH] PowerOP, PowerOP Core, 1/2

[Eugeny S. Mints]

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

The PowerOP Core provides completely arch independent interface
to create and control operating points which consist of arbitrary
subset of power parameters available on a certain platform.
Also, PowerOP Core provides optional SysFS interface to access
operating point from userspace.





[-- Attachment #2: powerop.core.patch --]
[-- Type: text/x-patch, Size: 25166 bytes --]

diff --git a/include/linux/powerop.h b/include/linux/powerop.h
new file mode 100644
index 0000000..704620d
--- /dev/null
+++ b/include/linux/powerop.h
@@ -0,0 +1,42 @@
+/*
+ * PowerOP core definitions
+ *
+ * Author: Eugeny S. Mints <eugeny.@nomadgs.com>
+ *
+ * 2006 (C) Nomad Global Solutions, 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.
+ *
+ * Based on ideas and code by Todd Poynor <tpoynor@mvista.com>
+ */
+#ifndef __POWEROP_H__
+#define __POWEROP_H__
+
+#define POWEROP_MAX_OPT_NAME_LENGTH 32
+
+#define POWEROP_REGISTER_EVENT    1
+#define POWEROP_UNREGISTER_EVENT  2
+
+/* Interface to an arch PM Core layer */
+struct powerop_driver {
+	char *name;
+	void *(*create_point) (const char *pwr_params, va_list args);
+	int (*set_point) (void *md_opt);
+	int (*get_point) (void *md_opt, const char *pwr_params, va_list args);
+};
+
+int powerop_driver_register(struct powerop_driver *p);
+void powerop_driver_unregister(struct powerop_driver *p);
+
+/* Main PowerOP Core interface */
+int powerop_register_point(const char *id, const char *pwr_params, ...);
+int powerop_unregister_point(const char *id);
+int powerop_set_point(const char *id);
+int powerop_get_point(const char *id, const char *pwr_params, ...);
+int powerop_get_registered_opt_names(char *opt_names_list[], int *length);
+void powerop_put_registered_opt_names(char *opt_names_list[]);
+int powerop_register_notifier(struct notifier_block *nb);
+int powerop_unregister_notifier(struct notifier_block *nb);
+
+#endif /* __POWEROP_H__ */
diff --git a/include/linux/powerop_sysfs.h b/include/linux/powerop_sysfs.h
new file mode 100644
index 0000000..40b5379
--- /dev/null
+++ b/include/linux/powerop_sysfs.h
@@ -0,0 +1,59 @@
+/*
+ * PowerOP SysFS UI
+ *
+ * Author: Todd Poynor <tpoynor@mvista.com>
+ * 2006-08 Update by Eugeny S. Mints <eugeny@nomadgs.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__
+
+#ifdef CONFIG_POWEROP_SYSFS
+
+#include <linux/sysfs.h>
+#include <linux/kobject.h>
+
+struct powerop_pwr_param_attribute {
+	struct attribute attr;
+	ssize_t(*store) (void *md_opt, char *pname, const char *buf,
+			 size_t count);
+};
+
+#define powerop_pwr_param_attr(_name) \
+static struct powerop_pwr_param_attribute _name##_attr = { \
+	.attr   = {				\
+		.name = __stringify(_name),	\
+		.mode = 0644,			\
+		.owner = THIS_MODULE,		\
+	},					\
+	.store  = _name##_store,		\
+}
+
+#define to_powerop_pwr_param_attr(_attr) container_of(_attr,\
+	struct powerop_pwr_param_attribute, attr)
+
+int powerop_sysfs_register_pwr_params(struct attribute **param_attrs);
+void powerop_sysfs_unregister_pwr_params(struct attribute **param_attrs);
+#else
+
+#define powerop_param_attr(_name)
+#define to_powerop_pwr_param_attr(_attr)
+
+static inline int
+powerop_sysfs_register_pwr_params(struct attribute **param_attrs)
+{
+	return 0;
+}
+
+static inline void
+powerop_sysfs_unregister_pwr_params(struct attribute **param_attrs)
+{
+}
+#endif /* CONFIG_POWEROP_SYSFS */
+
+#endif /*__POWEROP_SYSFS_H__*/
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index ae44a70..2bef726 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -104,3 +104,31 @@ config SUSPEND_SMP
 	bool
 	depends on HOTPLUG_CPU && X86 && PM
 	default y
+
+#
+# powerop
+#
+
+menu "PowerOP (Power Management)"
+
+config POWEROP
+	tristate "PowerOP Core"
+	help
+
+config POWEROP_SYSFS
+	bool "Enable PowerOP sysfs interface"
+	depends on PM && POWEROP && SYSFS
+	help
+
+config POWEROP_SYSFS_OP_CREATE
+	bool "Enable creation of operating point via sysfs interface"
+	depends on POWEROP_SYSFS
+	help
+
+config POWEROP_SYSFS_OP_DEBUG_IF
+	bool "Enable special hw operating point"
+	depends on POWEROP_SYSFS
+	help 
+	"hw point(ro) is used to get snapshots of power parameter values"
+
+endmenu
diff --git a/kernel/power/Makefile b/kernel/power/Makefile
index 8d0af3d..1045163 100644
--- a/kernel/power/Makefile
+++ b/kernel/power/Makefile
@@ -10,3 +10,12 @@ obj-$(CONFIG_SOFTWARE_SUSPEND)	+= swsusp
 obj-$(CONFIG_SUSPEND_SMP)	+= smp.o
 
 obj-$(CONFIG_MAGIC_SYSRQ)	+= poweroff.o
+
+# PowerOP
+
+powerop-pm-objs := powerop.o
+ifeq ($(CONFIG_POWEROP_SYSFS),y)
+  powerop-pm-objs += powerop_sysfs.o
+endif
+
+obj-$(CONFIG_POWEROP)           += powerop-pm.o
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 6d295c7..b113f79 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -16,6 +16,7 @@ #include <linux/errno.h>
 #include <linux/init.h>
 #include <linux/pm.h>
 #include <linux/console.h>
+#include <linux/module.h>
 
 #include "power.h"
 
@@ -233,7 +234,7 @@ int pm_suspend(suspend_state_t state)
 
 
 decl_subsys(power,NULL,NULL);
-
+EXPORT_SYMBOL_GPL(power_subsys);
 
 /**
  *	state - control system power state.
diff --git a/kernel/power/powerop.c b/kernel/power/powerop.c
new file mode 100644
index 0000000..88faf4f
--- /dev/null
+++ b/kernel/power/powerop.c
@@ -0,0 +1,407 @@
+/*
+ * PowerOP Core routines
+ *
+ * Author: Eugeny S. Mints <eugeny@nomadgs.com>
+ * 2006 (C) Nomad Global Solutions, Inc. 
+ *
+ * Original 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/module.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/notifier.h>
+#include <linux/powerop.h>
+
+#include "powerop_point.h"
+#include "powerop_sysfs_point.h"
+
+/* 
+ * FIXME: temporary limit. next implementation will handle unlimited number 
+ * of operating point
+ */
+#define POWEROP_MAX_OPT_NUMBER      20
+/* current number of registered operating points */
+static int registered_opt_number;
+
+/* array of registered opereting point names */
+static char *registered_names[POWEROP_MAX_OPT_NUMBER];
+
+/* notifications about an operating point registration/deregistration */
+static BLOCKING_NOTIFIER_HEAD(powerop_notifier_list);
+
+static struct powerop_driver *powerop_driver;
+
+/* list of named operating points maintained by PowerOP Core layer */
+static struct list_head named_opt_list;
+static DECLARE_MUTEX(named_opt_list_mutex);
+static int powerop_initialized;
+
+/* hw access serialization */
+static DECLARE_MUTEX(powerop_mutex);
+
+/* Auxiliary PowerOP Core internal routines */
+
+static void *create_point(const char *pwr_params, va_list args)
+{
+	void *res;
+
+	down(&powerop_mutex);
+	res = powerop_driver && powerop_driver->create_point ?
+	    powerop_driver->create_point(pwr_params, args) : NULL;
+	up(&powerop_mutex);
+
+	return res;
+}
+
+static int set_point(void *md_opt)
+{
+	int rc;
+
+	down(&powerop_mutex);
+	rc = md_opt && powerop_driver && powerop_driver->set_point ?
+	    powerop_driver->set_point(md_opt) : -EINVAL;
+	up(&powerop_mutex);
+
+	return rc;
+}
+
+/**
+ * get_point - get value of specified power paramenters
+ * @md_opt: pointer to operating point to be processed or NULL to get
+ * values of currently active operating point
+ * @pwr_params - name of requested power parameters
+ * @args - array of result placeholders 
+ *
+ * Get value of specified power paramenters of operating
+ * point pointed by 'md_opt'. Returns 0 on success, error code otherwise
+ */
+static int get_point(void *md_opt, const char *pwr_params, va_list args)
+{
+	int rc;
+
+	down(&powerop_mutex);
+	rc = powerop_driver && powerop_driver->get_point ?
+	    powerop_driver->get_point(md_opt, pwr_params, args) : -EINVAL;
+	up(&powerop_mutex);
+
+	return rc;
+}
+
+/* PowerOP Core public interface */
+
+int powerop_driver_register(struct powerop_driver *p)
+{
+	int error = -EBUSY;
+
+	if (!powerop_driver) {
+		printk(KERN_INFO "PowerOP registering driver %s.\n", p->name);
+		powerop_driver = p;
+		error = 0;
+	}
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(powerop_driver_register);
+
+void powerop_driver_unregister(struct powerop_driver *p)
+{
+	if (powerop_driver == p) {
+		printk(KERN_INFO "PowerOP unregistering driver %s\n", p->name);
+		powerop_driver = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(powerop_driver_unregister);
+
+/**
+ * powerop_register_point - register an operating point
+ * @id: operating point name
+ * @pwr_params: set of (power parameter name, value) pairs
+ *
+ * Add new operating point with a given name to
+ * operating points list. A caller passes power parameters for new operating
+ * points as pairs of name/value and passes only those parameter names the
+ * caller is interested in. PowerOP Core calls powerop driver to initialize
+ * arch dependent part of new operating point and links new named operating 
+ * point to the list maintained by PowerOP Core. Returns zero on success,
+ * -EEXIST or error code otherwise
+ */
+int powerop_register_point(const char *id, const char *pwr_params, ...)
+{
+	int err = 0;
+	struct powerop_point *opt, *tmpopt;
+	va_list args;
+
+	if ((!powerop_initialized) || (id == NULL) ||
+	    (strlen(id) > POWEROP_MAX_OPT_NAME_LENGTH) ||
+	    (registered_opt_number >= POWEROP_MAX_OPT_NUMBER))
+		return -EINVAL;
+
+	/* check whether operating point with specified name already exists */
+	down(&named_opt_list_mutex);
+	list_for_each_entry_safe(opt, tmpopt, &named_opt_list, node) {
+		if (strcmp(kobject_name(&opt->kobj), id) == 0) {
+			err = -EEXIST;
+			break;
+		}
+	}
+	up(&named_opt_list_mutex);
+
+	if (err == -EEXIST)
+		return err;
+
+	if ((opt = kzalloc(sizeof(struct powerop_point), GFP_KERNEL)) == NULL)
+		return -ENOMEM;
+
+	if ((registered_names[registered_opt_number] =
+	     kzalloc(sizeof(char) * POWEROP_MAX_OPT_NAME_LENGTH,
+		     GFP_KERNEL)) == NULL) {
+		err = -ENOMEM;
+		goto fail_name_nomem;
+	}
+
+	va_start(args, pwr_params);
+	opt->md_opt = create_point(pwr_params, args);
+	va_end(args);
+
+	if (opt->md_opt == NULL) {
+		err = -EINVAL;
+		goto fail_opt_create;
+	}
+
+	err = kobject_set_name(&opt->kobj, id);
+	if (err != 0)
+		goto fail_set_name;
+
+	down(&named_opt_list_mutex);
+	err = powerop_sysfs_register_point(opt);
+	if (err != 0) {
+		up(&named_opt_list_mutex);
+		goto fail_set_name;
+	}
+
+	list_add_tail(&opt->node, &named_opt_list);
+	strcpy(registered_names[registered_opt_number], id);
+	registered_opt_number++;
+	up(&named_opt_list_mutex);
+
+	blocking_notifier_call_chain(&powerop_notifier_list,
+				     POWEROP_REGISTER_EVENT, id);
+	return 0;
+
+      fail_set_name:
+	kfree(opt->md_opt);
+
+      fail_opt_create:
+	kfree(registered_names[registered_opt_number]);
+
+      fail_name_nomem:
+	kfree(opt);
+	return err;
+}
+EXPORT_SYMBOL_GPL(powerop_register_point);
+
+/** 
+ * powerop_unregister_point - unregister am operating point
+ * @id: name of operating point
+ *
+ * Search for operating point with specified name and remove it from 
+ * operating points list. Returns zero on success, -EINVAL if no operating
+ * point with specified name is found
+ */
+int powerop_unregister_point(const char *id)
+{
+	struct powerop_point *opt, *tmpopt;
+	int i = 0, ret = -EINVAL;
+
+	if ((!powerop_initialized) || (id == NULL))
+		return ret;
+
+	down(&named_opt_list_mutex);
+
+	list_for_each_entry_safe(opt, tmpopt, &named_opt_list, node) {
+		if (strcmp(kobject_name(&opt->kobj), id) == 0) {
+			/* FIXME: can't remove a point if it's the active */
+			list_del(&opt->node);
+			powerop_sysfs_unregister_point(opt);
+			kfree(opt->md_opt);
+			kfree(opt);
+			ret = 0;
+			break;
+		}
+	}
+
+	for (i = 0; i < registered_opt_number; i++) {
+		if (strcmp(registered_names[registered_opt_number], id) == 0) {
+			kfree(registered_names[i]);
+			registered_names[i] =
+			    registered_names[registered_opt_number];
+			break;
+		}
+	}
+	registered_opt_number++;
+	up(&named_opt_list_mutex);
+
+	blocking_notifier_call_chain(&powerop_notifier_list,
+				     POWEROP_UNREGISTER_EVENT, id);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(powerop_unregister_point);
+
+/** 
+ * powerop_set_point - set systems to the operating point
+ * @id: name of operating point
+ *
+ * Search for operating point with specified name and switch the system to the
+ * specified operating point. Returns zero on success, -EINVAL if no operating
+ * point with specified name is found or error code otherwise
+ */
+int powerop_set_point(const char *id)
+{
+	struct powerop_point *opt, *selected_opt = NULL;
+	int ret;
+
+	if ((!powerop_initialized) || (id == NULL))
+		return -EINVAL;
+
+	down(&named_opt_list_mutex);
+
+	list_for_each_entry(opt, &named_opt_list, node) {
+		if (strcmp(kobject_name(&opt->kobj), id) == 0) {
+			selected_opt = opt;
+			break;
+		}
+	}
+
+	ret = (selected_opt == NULL) ? -EINVAL : set_point(opt->md_opt);
+	if (ret == 0)
+		powerop_sysfs_set_activeop(id);
+
+	up(&named_opt_list_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(powerop_set_point);
+
+/** 
+ * powerop_get_point - get operating point pwr parameter values
+ * @id: name of operating point or NULL to get values for current active
+ * operating point
+ * @pwr_params: set of (power parameter name, result placeholder) pairs
+ *
+ * Search for operating point with specified name 
+ * and return value of power parameters corresponding to the operating point.
+ * NULL name is reserved to get power parameter values of current active
+ * operating point (from hardware). Returns zero on success, -EINVAL if no
+ * operating point with specified name is found
+ */
+int powerop_get_point(const char *id, const char *pwr_params, ...)
+{
+	int ret = -EINVAL;
+	struct powerop_point *opt;
+	va_list args;
+	void *md_opt = NULL;
+
+	if (!powerop_initialized)
+		return ret;
+
+	down(&named_opt_list_mutex);
+
+	/* FIXME: get rid of sema for NULL case */
+	if (id != NULL) {
+		list_for_each_entry(opt, &named_opt_list, node) {
+			if (strcmp(kobject_name(&opt->kobj), id) == 0) {
+				md_opt = opt->md_opt;
+				ret = 0;
+				break;
+			}
+		}
+		/* 
+		 * name is specified but corresponding operating point 
+		 * is not found 
+		 */
+		if (ret != 0)
+			goto out;
+	}
+
+	va_start(args, pwr_params);
+	ret = get_point(md_opt, pwr_params, args);
+	va_end(args);
+      out:
+	up(&named_opt_list_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(powerop_get_point);
+
+/** 
+ * powerop_get_registered_opt_names - get registered operating point names list
+ * @opt_names_list: array of pointers to name strings
+ * @length: array size
+ *
+ */
+int powerop_get_registered_opt_names(char *opt_names_list[], int *num)
+{
+	down(&named_opt_list_mutex);
+	opt_names_list = registered_names;
+	*num = registered_opt_number;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(powerop_get_registered_opt_names);
+
+void powerop_put_registered_opt_names(char *opt_names_list[])
+{
+	up(&named_opt_list_mutex);
+}
+EXPORT_SYMBOL_GPL(powerop_put_registered_opt_names);
+
+int powerop_register_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&powerop_notifier_list, nb);
+}
+EXPORT_SYMBOL(powerop_register_notifier);
+
+int powerop_unregister_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&powerop_notifier_list, nb);
+}
+EXPORT_SYMBOL(powerop_unregister_notifier);
+
+static int __init powerop_init(void)
+{
+	int rc = 0;
+
+	INIT_LIST_HEAD(&named_opt_list);
+	rc = powerop_sysfs_init();
+	if (rc == 0)
+		powerop_initialized = 1;
+
+	return rc;
+}
+
+static void __exit powerop_exit(void)
+{
+	struct powerop_point *opt, *tmp_opt;
+
+	down(&named_opt_list_mutex);
+
+	list_for_each_entry_safe(opt, tmp_opt, &named_opt_list, node) {
+		list_del(&opt->node);
+		powerop_sysfs_unregister_point(opt);
+		kfree(opt->md_opt);
+		kfree(opt);
+	}
+
+	up(&named_opt_list_mutex);
+	powerop_sysfs_exit();
+}
+
+module_init(powerop_init);
+module_exit(powerop_exit);
+
+MODULE_DESCRIPTION("PowerOP Core");
+MODULE_LICENSE("GPL");
diff --git a/kernel/power/powerop_point.h b/kernel/power/powerop_point.h
new file mode 100644
index 0000000..64b5591
--- /dev/null
+++ b/kernel/power/powerop_point.h
@@ -0,0 +1,36 @@
+/*
+ * PowerOP core non-public header
+ *
+ * Author: Eugeny S. Mints <eugeny.@nomadgs.com>
+ *
+ * 2006 (C) Nomad Global Solutions, 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.
+ *
+ * Based on ideas and code by Todd Poynor <tpoynor@mvista.com>
+ */
+#ifndef __POWEROP_INT_H__
+#define __POWEROP_INT_H__
+
+#define POWEROP_MAX_OPT_NAME_LENGTH 32
+
+/**
+ * struct powerop_point - PowerOP Core representation of operating point
+ * @kobj: hook to reference an operating point in some arch independent way
+ * (for ex. sysfs)
+ * @md_opt: pointer to opaque arch dependent set of power parameters
+ *
+ */
+struct powerop_point {
+	struct kobject kobj;
+	void *md_opt;
+	struct list_head node;
+#ifdef CONFIG_POWEROP_SYSFS
+	struct completion released;
+#endif
+};
+
+#define to_namedop(_kobj) container_of(_kobj, struct powerop_point, kobj)
+
+#endif /* __POWEROP_INT_H__ */
diff --git a/kernel/power/powerop_sysfs.c b/kernel/power/powerop_sysfs.c
new file mode 100644
index 0000000..94906c9
--- /dev/null
+++ b/kernel/power/powerop_sysfs.c
@@ -0,0 +1,254 @@
+/*
+ * PowerOP sysfs UI
+ *
+ * Author: Todd Poynor <tpoynor@mvista.com>
+ *
+ * Integration with updated PowerOP interface by 
+ * Eugeny S. Mints <eugeny@nomadgs.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/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 <linux/powerop.h>
+#include <linux/powerop_sysfs.h>
+
+#include "powerop_sysfs_point.h"
+
+#define POWEROP_SYSFS_HW_OPT "hw"
+
+extern struct subsystem power_subsys;
+
+static DECLARE_MUTEX(activeop_mutex);
+static char activeop[POWEROP_MAX_OPT_NAME_LENGTH];
+
+#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;
+
+static void powerop_sysfs_point_release(struct kobject *kobj)
+{
+	struct powerop_point *op = to_namedop(kobj);
+
+	complete(&op->released);
+	return;
+}
+
+static ssize_t
+powerop_sysfs_pwr_param_attr_show(struct kobject *kobj, struct attribute *attr,
+				  char *buf)
+{
+	struct powerop_point *opt = to_namedop(kobj);
+	ssize_t ret = 0;
+	int pval = 0;
+
+#ifdef CONFIG_POWEROP_SYSFS_OP_DEBUG_IF
+	if (strcmp(kobject_name(&opt->kobj), POWEROP_SYSFS_HW_OPT) == 0)
+		ret = powerop_get_point(NULL, attr->name, &pval);
+	else
+#endif				/* CONFIG_POWEROP_SYSFS_OP_DEBUG_IF */
+		ret = powerop_get_point(kobject_name(&opt->kobj), attr->name,
+					&pval);
+
+	return ret ? 0 : sprintf(buf, "%d\n", pval);
+}
+
+static ssize_t
+powerop_sysfs_pwr_param_attr_store(struct kobject *kobj,
+				   struct attribute *attr, const char *buf,
+				   size_t count)
+{
+#ifdef CONFIG_POWEROP_SYSFS_OP_CREATE
+	struct powerop_pwr_param_attribute *param_attr =
+	    to_powerop_pwr_param_attr(attr);
+	struct powerop_point *opt = to_namedop(kobj);
+	ssize_t ret = 0;
+
+#ifdef CONFIG_POWEROP_SYSFS_OP_DEBUG_IF
+	if (strcmp(kobject_name(&opt->kobj), POWEROP_SYSFS_HW_OPT) == 0)
+		return -EINVAL;
+#endif				/* CONFIG_POWEROP_SYSFS_OP_DEBUG_IF */
+	if (param_attr->store)
+		ret = param_attr->store(opt->md_opt, attr->name, buf, count);
+
+	return ret;
+#else
+	return -ENOTSUPP;
+#endif				/* CONFIG_POWEROP_SYSFS_OP_CREATE */
+}
+
+static struct sysfs_ops powerop_sysfs_ops = {
+	.show = powerop_sysfs_pwr_param_attr_show,
+	.store = powerop_sysfs_pwr_param_attr_store,
+};
+
+static struct kobj_type ktype_powerop_point = {
+	.release = powerop_sysfs_point_release,
+	.sysfs_ops = &powerop_sysfs_ops,
+};
+
+#ifdef CONFIG_POWEROP_SYSFS_OP_CREATE
+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)
+{
+	int rc = 0;
+
+	return ((rc = powerop_register_point(buf, " ")) == 0) ? n : rc;
+}
+
+powerop_attr(new);
+#endif				/* CONFIG_POWEROP_SYSFS_OP_CREATE */
+
+static ssize_t active_show(struct subsystem *subsys, char *buf)
+{
+	int ret = 0;
+
+	down(&activeop_mutex);
+	ret = sprintf(buf, "%s\n", activeop);
+	up(&activeop_mutex);
+
+	return ret;
+}
+
+static ssize_t active_store(struct subsystem *subsys, const char *buf,
+			    size_t n)
+{
+	int error;
+
+	return (error = powerop_set_point(buf)) == 0 ? n : error;
+}
+
+powerop_attr(active);
+
+static struct attribute *g[] = {
+#ifdef CONFIG_POWEROP_SYSFS_OP_CREATE
+	&new_attr.attr,
+#endif				/* CONFIG_POWEROP_SYSFS_OP_CREATE */
+	&active_attr.attr,
+	NULL,
+};
+
+static struct attribute_group attr_group = {
+	.attrs = g,
+};
+
+static int create_point_attrs(struct kobject *kobj)
+{
+	int error = 0;
+
+	if (param_attr_group.attrs)
+		if ((error = sysfs_create_group(kobj, &param_attr_group)))
+			printk(KERN_ERR
+			       "sysfs_create_group for op %s failed.\n",
+			       kobject_name(kobj));
+	return error;
+}
+
+static void remove_point_attrs(struct kobject *kobj)
+{
+	if (param_attr_group.attrs)
+		sysfs_remove_group(kobj, &param_attr_group);
+}
+
+int powerop_sysfs_register_point(struct powerop_point *op)
+{
+	int error;
+
+	op->kobj.ktype = &ktype_powerop_point;
+	op->kobj.kset = &power_subsys.kset;
+	init_completion(&op->released);
+
+	if ((error = kobject_register(&op->kobj))) {
+		printk(KERN_ERR "PowerOP kobject_register for op %s failed.\n",
+		       kobject_name(&op->kobj));
+		return error;
+	}
+
+	/* FIXME: check rc */
+	create_point_attrs(&op->kobj);
+	return 0;
+}
+
+void powerop_sysfs_unregister_point(struct powerop_point *op)
+{
+	remove_point_attrs(&op->kobj);
+	kobject_unregister(&op->kobj);
+	wait_for_completion(&op->released);
+}
+
+void powerop_sysfs_set_activeop(const char *id)
+{
+	down(&activeop_mutex);
+	strcpy(activeop, id);
+	up(&activeop_mutex);
+}
+
+int powerop_sysfs_register_pwr_params(struct attribute **param_attrs)
+{
+	if (param_attr_group.attrs)
+		return -EBUSY;
+
+	param_attr_group.attrs = param_attrs;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(powerop_sysfs_register_pwr_params);
+
+void powerop_sysfs_unregister_pwr_params(struct attribute **param_attrs)
+{
+	if (param_attr_group.attrs != param_attrs)
+		return;
+
+	param_attr_group.attrs = NULL;
+}
+EXPORT_SYMBOL_GPL(powerop_sysfs_unregister_pwr_params);
+
+int powerop_sysfs_init(void)
+{
+	int error;
+
+	if (param_attr_group.attrs == NULL) {
+		printk(KERN_ERR "PowerOP SysFS: register pwr params first!\n");
+		return -EINVAL;
+	}
+
+	if ((error =
+	     sysfs_create_group(&power_subsys.kset.kobj, &attr_group))) {
+		printk(KERN_ERR "PowerOP subsys sysfs_create_group failed.\n");
+		return error;
+	}
+#ifdef CONFIG_POWEROP_SYSFS_OP_DEBUG_IF
+	if (powerop_register_point(POWEROP_SYSFS_HW_OPT, " "))
+		printk(KERN_ERR "PowerOP subsys HW point creation failed\n");
+#endif
+	powerop_sysfs_set_activeop("Unknown");
+	return 0;
+}
+
+void powerop_sysfs_exit(void)
+{
+	powerop_sysfs_unregister_pwr_params(param_attr_group.attrs);
+	sysfs_remove_group(&power_subsys.kset.kobj, &attr_group);
+}
diff --git a/kernel/power/powerop_sysfs_point.h b/kernel/power/powerop_sysfs_point.h
new file mode 100644
index 0000000..9c0baa9
--- /dev/null
+++ b/kernel/power/powerop_sysfs_point.h
@@ -0,0 +1,42 @@
+/*
+ * PowerOP SysFS non-public header
+ *
+ * Author: Eugeny S. Mints <eugeny.@nomadgs.com>
+ *
+ * 2006 (C) Nomad Global Solutions, 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_INT_H__
+#define __POWEROP_SYSFS_INT_H__
+#include "powerop_point.h"
+
+#ifdef CONFIG_POWEROP_SYSFS
+int powerop_sysfs_register_point(struct powerop_point *opt);
+void powerop_sysfs_unregister_point(struct powerop_point *op);
+void powerop_sysfs_set_activeop(const char *id);
+int powerop_sysfs_init(void);
+void powerop_sysfs_exit(void);
+#else
+static inline int powerop_sysfs_register_point(struct powerop_point *opt)
+{
+	return 0;
+}
+static inline int powerop_sysfs_unregister_point(struct powerop_point *opt)
+{
+	return 0;
+}
+static inline void powerop_sysfs_set_activeop(const char *id)
+{
+}
+static inline int powerop_sysfs_init(void)
+{
+	return 0;
+}
+static inline void powerop_sysfs_exit(void)
+{
+}
+#endif /* CONFIG_POWEROP_SYSFS */
+
+#endif /* __POWEROP_SYSFS_INT_H__ */

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Pavel Machek]

Hi!

> The PowerOP Core provides completely arch independent interface
> to create and control operating points which consist of arbitrary
> subset of power parameters available on a certain platform.
> Also, PowerOP Core provides optional SysFS interface to access
> operating point from userspace.

Please inline patches and sign them off.

Also if you are providing new userland interface, describe it... in
Documentation/ABI.

> +struct powerop_driver {
> +	char *name;
> +	void *(*create_point) (const char *pwr_params, va_list args);
> +	int (*set_point) (void *md_opt);
> +	int (*get_point) (void *md_opt, const char *pwr_params, va_list args);
> +};

We can certainly get better interface than va_list, right?

> +
> +#
> +# powerop
> +#
> +
> +menu "PowerOP (Power Management)"
> +
> +config POWEROP
> +	tristate "PowerOP Core"
> +	help

Hohum, this is certainly going to be clear to confused user...

> +	list_add_tail(&opt->node, &named_opt_list);
> +	strcpy(registered_names[registered_opt_number], id);
> +	registered_opt_number++;
> +	up(&named_opt_list_mutex);
> +
> +	blocking_notifier_call_chain(&powerop_notifier_list,
> +				     POWEROP_REGISTER_EVENT, id);
> +	return 0;
> +
> +      fail_set_name:
> +	kfree(opt->md_opt);
> +
> +      fail_opt_create:
> +	kfree(registered_names[registered_opt_number]);
> +
> +      fail_name_nomem:
> +	kfree(opt);
> +	return err;
> +}

Careful about spaces vs. tabs...

...so, you got support for 20 operating points... And this should
include devices, too, right? How is it going to work on 8cpu box? will
you have states like cpu1_800MHz_cpu2_1600MHz_cpu3_800MHz_... ?

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

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Eugeny S. Mints]

Pavel Machek wrote:
> Hi!
> 
>> The PowerOP Core provides completely arch independent interface
>> to create and control operating points which consist of arbitrary
>> subset of power parameters available on a certain platform.
>> Also, PowerOP Core provides optional SysFS interface to access
>> operating point from userspace.
> 
> Please inline patches and sign them off.

hmm, seems my bad. will double check.

> 
> Also if you are providing new userland interface, describe it... in
> Documentation/ABI.

seems already discussed

>> +struct powerop_driver {
>> +	char *name;
>> +	void *(*create_point) (const char *pwr_params, va_list args);
>> +	int (*set_point) (void *md_opt);
>> +	int (*get_point) (void *md_opt, const char *pwr_params, va_list args);
>> +};
> 
> We can certainly get better interface than va_list, right?

Please elaborate.

> 
>> +
>> +#
>> +# powerop
>> +#
>> +
>> +menu "PowerOP (Power Management)"
>> +
>> +config POWEROP
>> +	tristate "PowerOP Core"
>> +	help
> 
> Hohum, this is certainly going to be clear to confused user...

please elaborate.

>> +	list_add_tail(&opt->node, &named_opt_list);
>> +	strcpy(registered_names[registered_opt_number], id);
>> +	registered_opt_number++;
>> +	up(&named_opt_list_mutex);
>> +
>> +	blocking_notifier_call_chain(&powerop_notifier_list,
>> +				     POWEROP_REGISTER_EVENT, id);
>> +	return 0;
>> +
>> +      fail_set_name:
>> +	kfree(opt->md_opt);
>> +
>> +      fail_opt_create:
>> +	kfree(registered_names[registered_opt_number]);
>> +
>> +      fail_name_nomem:
>> +	kfree(opt);
>> +	return err;
>> +}
> 
> Careful about spaces vs. tabs...

will double check but I'm pretty sure I ran all files thru lindent.


> ...so, you got support for 20 operating points... And this should
> include devices, too, right? 

sorry, don't understand the question. an operating point is a set of platform 
power parameters.

>How is it going to work on 8cpu box? will
> you have states like cpu1_800MHz_cpu2_1600MHz_cpu3_800MHz_... ?

i do not operate with term 'state' so I don't understand what it means here.

	Eugeny

> 								Pavel

Re: [PATCH] PowerOP, PowerOP Core, 1/2

[Greg KH]

On Thu, Sep 14, 2006 at 06:37:39PM +0400, Eugeny S. Mints wrote:
> The PowerOP Core provides completely arch independent interface
> to create and control operating points which consist of arbitrary
> subset of power parameters available on a certain platform.
> Also, PowerOP Core provides optional SysFS interface to access
> operating point from userspace.

module reference count issues have still not been addressed.

thanks,

greg k-h

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Eugeny S. Mints]

Eugeny S. Mints wrote:
> Pavel Machek wrote:
>> Hi!
>>
[skip]
>> How is it going to work on 8cpu box? will
>> you have states like cpu1_800MHz_cpu2_1600MHz_cpu3_800MHz_... ?

basically I guess you are asking about what the names of operating points are 
and how to distinguish between operating points from userspace on 8cpu box.

An advantage of PowerOP approach is that operating point name is used as a 
_handle_ and may or may not be meaningful. The idea is that if a policy manager 
  needs to make a decision and needs to distinguish between operating points it 
can check value of any power parameters of operating points in question. Power 
parameter values may be obtained under <op_name> dir name.

With such approach a policy manger may compare operating points at runtime and 
should not rely on compile time knowledge about what name corresponds to  what 
set of power parameter values. It uses name as a handle.

	Eugeny
> 
> i do not operate with term 'state' so I don't understand what it means 
> here.
> 
>     Eugeny
> 
>>                                 Pavel
> 
> 

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Vitaly Wool]

Eugeny,

On 9/18/06, Eugeny S. Mints <eugeny.mints@gmail.com> wrote:

> With such approach a policy manger may compare operating points at runtime and
> should not rely on compile time knowledge about what name corresponds to  what
> set of power parameter values. It uses name as a handle.

I suggest that you supply a meaningful yet simple example for SMP case.

Thanks,
   Vitaly

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Pavel Machek]

Hi!

> >>+struct powerop_driver {
> >>+	char *name;
> >>+	void *(*create_point) (const char *pwr_params, va_list args);
> >>+	int (*set_point) (void *md_opt);
> >>+	int (*get_point) (void *md_opt, const char *pwr_params, va_list 
> >>args);
> >>+};
> >
> >We can certainly get better interface than va_list, right?
> 
> Please elaborate.

va_list does not provide adequate type checking. I do not think it
suitable in driver<->core interface.

> >>+#
> >>+# powerop
> >>+#
> >>+
> >>+menu "PowerOP (Power Management)"
> >>+
> >>+config POWEROP
> >>+	tristate "PowerOP Core"
> >>+	help
> >
> >Hohum, this is certainly going to be clear to confused user...
> 
> please elaborate.

You probably want to include some help text.

> >How is it going to work on 8cpu box? will
> >you have states like cpu1_800MHz_cpu2_1600MHz_cpu3_800MHz_... ?
> 
> i do not operate with term 'state' so I don't understand what it means here.

Okay, state here means "operating point". How will operating points
look on 8cpu box? That's 256 states if cpus only support "low" and
"high". How do you name them?
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Scott E. Preece]

| From: Pavel Machek<pavel@ucw.cz>
| 
| > >>+struct powerop_driver {
| > >>+	char *name;
| > >>+	void *(*create_point) (const char *pwr_params, va_list args);
| > >>+	int (*set_point) (void *md_opt);
| > >>+	int (*get_point) (void *md_opt, const char *pwr_params, va_list 
| > >>args);
| > >>+};
| > >
| > >We can certainly get better interface than va_list, right?
| > 
| > Please elaborate.
| 
| va_list does not provide adequate type checking. I do not think it
| suitable in driver<->core interface.
---

Well, in this particular case the typing probably has to be weak, one
way or another. The meaning of the parameters is arguably opaque at
the interface - the attributes may be meaningful to specific components
of the system, but are not defined in the standardized interface (which
would otherwise have to know about all possible kinds of power
attributes and be changed every time a new one is added).

So, if you'd rather have an array of char* or void* values, that would
probably also meet the need, but my guess is that the typing is
intentionally opaque.

---
| ...
| > >How is it going to work on 8cpu box? will
| > >you have states like cpu1_800MHz_cpu2_1600MHz_cpu3_800MHz_... ?
| > 
| > i do not operate with term 'state' so I don't understand what it means here.
| 
| Okay, state here means "operating point". How will operating points
| look on 8cpu box? That's 256 states if cpus only support "low" and
| "high". How do you name them?
---

I don't think you would name the compounded states. Each CPU would need
to have its own defined set of operating points (since the capabilities
of the CPUs can reasonably be different).

scott
-- 
scott preece
motorola mobile devices, il67, 1800 s. oak st., champaign, il  61820  
e-mail:	preece@motorola.com	fax:	+1-217-384-8550
phone:	+1-217-384-8589	cell: +1-217-433-6114	pager: 2174336114@vtext.com

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Eugeny S. Mints]

Scott E. Preece wrote:
> | From: Pavel Machek<pavel@ucw.cz>
> | 
> | > >>+struct powerop_driver {
> | > >>+	char *name;
> | > >>+	void *(*create_point) (const char *pwr_params, va_list args);
> | > >>+	int (*set_point) (void *md_opt);
> | > >>+	int (*get_point) (void *md_opt, const char *pwr_params, va_list 
> | > >>args);
> | > >>+};
> | > >
> | > >We can certainly get better interface than va_list, right?
> | > 
> | > Please elaborate.
> | 
> | va_list does not provide adequate type checking. I do not think it
> | suitable in driver<->core interface.
> ---
> 
> Well, in this particular case the typing probably has to be weak, one
> way or another. The meaning of the parameters is arguably opaque at
> the interface - the attributes may be meaningful to specific components
> of the system, but are not defined in the standardized interface (which
> would otherwise have to know about all possible kinds of power
> attributes and be changed every time a new one is added).
> 
> So, if you'd rather have an array of char* or void* values, that would
> probably also meet the need, but my guess is that the typing is
> intentionally opaque.

exactly. Thanks Scott, I don't have anything meaningful to add here.

> ---
> | ...
> | > >How is it going to work on 8cpu box? will
> | > >you have states like cpu1_800MHz_cpu2_1600MHz_cpu3_800MHz_... ?
> | > 
> | > i do not operate with term 'state' so I don't understand what it means here.
> | 
> | Okay, state here means "operating point". How will operating points
> | look on 8cpu box? That's 256 states if cpus only support "low" and
> | "high". How do you name them?
> ---
> 
> I don't think you would name the compounded states. Each CPU would need
> to have its own defined set of operating points (since the capabilities
> of the CPUs can reasonably be different).

i'm not sure - may be it's the mail list issues but in fact I sent out
my additional comment on this question in a separate letter on this thread
a while ago.

Eugeny

> scott

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Scott E. Preece]

From: "Eugeny S. Mints" <eugeny.mints@gmail.com>

| Eugeny S. Mints wrote:
| > Pavel Machek wrote:
| >> Hi!
| >>
| [skip]
| >> How is it going to work on 8cpu box? will
| >> you have states like cpu1_800MHz_cpu2_1600MHz_cpu3_800MHz_... ?
| 
| basically I guess you are asking about what the names of operating
| points are and how to distinguish between operating points from
| userspace on 8cpu box.
| 
| An advantage of PowerOP approach is that operating point name is used as
| a _handle_ and may or may not be meaningful. The idea is that if a
| policy manager needs to make a decision and needs to distinguish between
| operating points it can check value of any power parameters of operating
| points in question. Power parameter values may be obtained under
| <op_name> dir name.
| 
| With such approach a policy manger may compare operating points at
| runtime and should not rely on compile time knowledge about what name
| corresponds to what set of power parameter values. It uses name as a
| handle.
---

Hmm. If you assume the CPUs in an SMP system can be in different
operating points, this would (as Pavel pointed out) result in an
explosion of operating points.

I see several possible responses to this problem:

(1) Make operating points a CPU-level abstraction, rather than a
system-level abstraction, with the set of OPs for each CPU defined
individually. This allows for non-symmetric CPUs. Each CPU would have
its own policy manager driving OP selection for that CPU (the set of
policy managers could be shared, as I believe cpufreq shares governors
among CPU).

(2) Create CPU-group operating points that captured the power
parameters for each of a set of CPUs (that is, a group OP would be a
vector of CPU OPs).

(3) Create CPU-group operating points that varied a group of CPUs
symetrically (that is, one set of CPU-level power parameters shared
across a set of CPUs), with group-level policy managers that control
transitions for the group in lockstep. 

(4) Create CPU-group operating points that varied a group of CPUs
symetrically (as in (3)), and have both group-level policy managers and
a system-level policy manager that moves CPUs from one group to
another.

scott
-- 
scott preece
motorola mobile devices, il67, 1800 s. oak st., champaign, il  61820  
e-mail:	preece@motorola.com	fax:	+1-217-384-8550
phone:	+1-217-384-8589	cell: +1-217-433-6114	pager: 2174336114@vtext.com

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Pavel Machek]

Hi!

> | > >>+struct powerop_driver {
> | > >>+	char *name;
> | > >>+	void *(*create_point) (const char *pwr_params, va_list args);
> | > >>+	int (*set_point) (void *md_opt);
> | > >>+	int (*get_point) (void *md_opt, const char *pwr_params, va_list 
> | > >>args);
> | > >>+};
> | > >
> | > >We can certainly get better interface than va_list, right?
> | > 
> | > Please elaborate.
> | 
> | va_list does not provide adequate type checking. I do not think it
> | suitable in driver<->core interface.
> ---
> 
> Well, in this particular case the typing probably has to be weak, one
> way or another. The meaning of the parameters is arguably opaque at

Why not have struct powerop_parameters, defined in machine-specific
header somewhere, but used everywhere?

> the interface - the attributes may be meaningful to specific components
> of the system, but are not defined in the standardized interface (which
> would otherwise have to know about all possible kinds of power
> attributes and be changed every time a new one is added).
> 
> So, if you'd rather have an array of char* or void* values, that would
> probably also meet the need, but my guess is that the typing is
> intentionally opaque.

Actually array of integers would be better than this.

> | > >How is it going to work on 8cpu box? will
> | > >you have states like cpu1_800MHz_cpu2_1600MHz_cpu3_800MHz_... ?
> | > 
> | > i do not operate with term 'state' so I don't understand what it means here.
> | 
> | Okay, state here means "operating point". How will operating points
> | look on 8cpu box? That's 256 states if cpus only support "low" and
> | "high". How do you name them?
> 
> I don't think you would name the compounded states. Each CPU would need
> to have its own defined set of operating points (since the capabilities
> of the CPUs can reasonably be different).

Well, having few "powerop domains" per system would likely solve that
problem... and problem of 20 devices on my PC. Can we get that?

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

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Pavel Machek]

Hi!

> Hmm. If you assume the CPUs in an SMP system can be in different
> operating points, this would (as Pavel pointed out) result in an
> explosion of operating points.

Problem is not only CPUs, devices are mostly independent in PC
case... it would be nice to solve that problem with same approach.

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

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Igor Stoppa]

On Fri, 2006-09-22 at 16:11 +0200, ext Pavel Machek wrote:
> Hi!
> 
> > Hmm. If you assume the CPUs in an SMP system can be in different
> > operating points, this would (as Pavel pointed out) result in an
> > explosion of operating points.
> 
> Problem is not only CPUs, devices are mostly independent in PC
> case... it would be nice to solve that problem with same approach.
> 
> 								Pavel

This whole discussion is, imho, very misleading.

The number of CPU in a box or the number of cores in a chip is not a
significant element, per se.

What is really important is how interdependent they are.
In the case of a board with 2, 4 or 8 CPU, the decisione if their states
are tied together or not is not based on the packaging, but rather on
the (possibly suboptimal) HW design: shared clock or power sources
impose constraints and correlations.

Correlations lead to the multiplication of subsystem states, while
constraints curb the number, because if CPU1 and CPU2 share the same
voltage source, then of all the possible states, only those where this
constraint is satisfied are possible.

Remember what an OP is:
a set of values that exaustively and uniquely define the state of a
system.

So if your box has 256 CPUs, I bet that they are not all on the same
board and probably you also have several independently programmable
power sources.
If every power source feeds say 8 CPUs, then the box contains 16
independent subsystems.

Of course one probably would like to orchestrate all of them, but that's
a 2nd level problem, that could be addressed by a power/workload
manager.

However, even starting with localised dynamic power management would
yeld a significant improvement.

About other device within a PC: SW design cannot really change whatever
constraints the HW design is imposing: if 2 devices are sharing a
programmable v/f source, the source is generating a system which
comprises both devices and it has to be addressed as such.

The innterdipendency could be masked at some high abstract level, but
then going down, close to HW, it has to be explicitly dealt with.

-- 
Cheers,
           Igor

Igor Stoppa (Nokia M - OSSO / Tampere)

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Scott E. Preece]

| From pavel@ucw.cz Fri Sep 22 09:12:25 2006

Hi, Pavel,

| ...
|
| > | Okay, state here means "operating point". How will operating points
| > | look on 8cpu box? That's 256 states if cpus only support "low" and
| > | "high". How do you name them?
| > 
| > I don't think you would name the compounded states. Each CPU would need
| > to have its own defined set of operating points (since the capabilities
| > of the CPUs can reasonably be different).
| 
| Well, having few "powerop domains" per system would likely solve that
| problem... and problem of 20 devices on my PC. Can we get that?
---

I've been hoping somebody would send a good description of exactly what
they mean by "power domain" or "voltage domain", so we could talk about
it... 

scott
-- 
scott preece
motorola mobile devices, il67, 1800 s. oak st., champaign, il  61820  
e-mail:	preece@motorola.com	fax:	+1-217-384-8550
phone:	+1-217-384-8589	cell: +1-217-433-6114	pager: 2174336114@vtext.com

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Scott E. Preece]

Note that I don't think PowerOp would cover all devices. In fact, I
think most devices would remain autonomous or controlled as part of
specific subsystems. The only things that PowerOp would bundle together
would be things that aren't independent (and may not even be visible as
"devices" in the usual Linux sense), but that have to be managed
together in changing frequency/voltage. At least, that's the way I
imagined it would work.

scott


| From pavel@ucw.cz Fri Sep 22 09:13:53 2006
| 
| Hi!
| 
| > Hmm. If you assume the CPUs in an SMP system can be in different
| > operating points, this would (as Pavel pointed out) result in an
| > explosion of operating points.
| 
| Problem is not only CPUs, devices are mostly independent in PC
| case... it would be nice to solve that problem with same approach.
| 
| 								Pavel
| -- 
| (english) http://www.livejournal.com/~pavelmachek
| (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
| 

-- 
scott preece
motorola mobile devices, il67, 1800 s. oak st., champaign, il  61820  
e-mail:	preece@motorola.com	fax:	+1-217-384-8550
phone:	+1-217-384-8589	cell: +1-217-433-6114	pager: 2174336114@vtext.com

RE: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Woodruff, Richard]

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

> I've been hoping somebody would send a good description of exactly
what
> they mean by "power domain" or "voltage domain", so we could talk
about
> it...
> 
> scott

Here are a couple early mails which illustrate it a bit (sorry if they
mess up any ones mail reader).  I try an place a device inside of some
of the abstractions.

Regards,
Richard W.


[-- Attachment #2: Type: message/rfc822, Size: 4577 bytes --]

From: "Woodruff, Richard" <r-woodruff2@ti.com>
To: "David Brownell" <david-b@pacbell.net>, "Gross, Mark" <mark.gross@intel.com>
Cc: "Rhoads, Rob" <rob.rhoads@intel.com>, <alb@google.com>, <amit.kucheria@nokia.com>, <mail@dominikbrodowski.de>, <k.stewart@freescale.com>, <matthew.a.locke@comcast.net>, <sampsa.fabritius@nokia.com>, "Sven-Thorsten Dietrich" <sven@mvista.com>, <ext-tuukka.tikkanen@nokia.com>, "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>, "Mochel, Patrick" <patrick.mochel@intel.com>, <mochel@digitalimplant.org>, <toddpoynor@gmail.com>, <abelay@mit.edu>, "Griffiths, Richard" <Richard.Griffiths@windriver.com>, "Loeliger Jon-LOELIGER" <jdl@freescale.com>, "deVries, Alex" <Alex.deVries@windriver.com>, <dsingleton@mvista.com>, "Cai, Stanley" <stanley.cai@intel.com>
Subject: RE: PM framework
Date: Sun, 28 May 2006 09:55:50 -0500

> > We want to
> > support multiple parents, and the notion of dependencies and
> > constraints. Is a core capability that is needed to accommodate the
> > hierarchical and overlapping areas of influence for clock and
voltage
> > domains.
> 
> It supports reparenting, which is how I read "multiple parents".
> If that's not what you mean by "multiple parents", please give an
> example of what you mean.  As a rule, once a base clock is generated
> (say, by a PLL) it gets gated into children (and maybe subdivided).
> 
> Dependencies of the parent/child type are explicit, while any others
> (as well as constraints, if any) are platform-specific.  And right
> now there's no generic code that needs to know about **either** ...

I need to do a little catch up reading...  I'm not sure this bit is
complete on target, but...

As far as clocks go a functional module can have multiple independent
clocks required for operation.  Say for MMC on OMAP2, the DPLL derived
clock might supply the interface clock, a fixed APLL might provide the
functional clock, and a 32KHz clock might provide denounce.  All clocks
are necessary for proper operation.  And in the case of OMAP2 all 3 are
controllable at the module level.

In a more general sense for a module's functional operation there may be
many dependencies.  This kind of thing goes beyond just clocks.  This
can be inter-driver, LCD needs I2C to turn on a backlight.

If you look at any block of functionality you create a 'driver' for
there can be a lot of external dependencies.  How to best express these
dependencies is where things get unclear.  Currently things like sysfs
and the clk_api handle part of them.  They are not complete.  

It all gets less and less clear as you layer on more intra-soc-chip
levels.  A chip has multiple voltage domains (VD), within each VD there
are several power domains (PD), across several of the PDs (or fully
within a PD) there are multiple clock domains (CD).  Each domain level
has a number of sub levels with certain functionality states.  Finally
you drop the end module function say MMC into this.  Software writes a
single end driver for this which is likely part of some subsystem stack.
This functional block (say MMC) will be part of a VD, PD, multiple CD's
(each domain has multiple levels of operation).  To achieve power
management at the end module you get somewhat simple controls
(clk_enable), to achieve domain power controls, you must act on entire
groups with-in each domain type.  Domains are contained with-in domains
(VD contains PD) so in order to make a VD change all internal PD need to
be at a certain state, and for this to happen all CD in a PD need to be
in at a certain state.  For this to happen end functions (MMC) must have
each managed their end clocks and idles properly.  

If you handle each layer from the bottom up, you can achieve massive
power savings.  There are many explicit relationships in this picture
which need to be expressed.  You can't look at devices individually and
get a domain state, you must look at them together.

OMAP1 which you are very familiar with does not provide this type of
domain partitioning nor the intermediate states (and control of each)
with in each domain.  You get much more of the whole chip in sleep
states, but not all the partial states which are useable during active
mode.

Regards,
Richard W.




[-- Attachment #3: Type: message/rfc822, Size: 7779 bytes --]

From: "David Brownell" <david-b@pacbell.net>
To: "Woodruff, Richard" <r-woodruff2@ti.com>
Cc: "Gross, Mark" <mark.gross@intel.com>, "Rhoads, Rob" <rob.rhoads@intel.com>, <alb@google.com>, <amit.kucheria@nokia.com>, <mail@dominikbrodowski.de>, <k.stewart@freescale.com>, <matthew.a.locke@comcast.net>, <sampsa.fabritius@nokia.com>, "Sven-Thorsten Dietrich" <sven@mvista.com>, <ext-tuukka.tikkanen@nokia.com>, "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>, "Mochel, Patrick" <patrick.mochel@intel.com>, <mochel@digitalimplant.org>, <toddpoynor@gmail.com>, <abelay@mit.edu>, "Griffiths, Richard" <Richard.Griffiths@windriver.com>, "Loeliger Jon-LOELIGER" <jdl@freescale.com>, "deVries, Alex" <Alex.deVries@windriver.com>, <dsingleton@mvista.com>, "Cai, Stanley" <stanley.cai@intel.com>
Subject: Re: PM framework
Date: Mon, 29 May 2006 10:20:48 -0500
Message-ID: <200605290820.51778.david-b@pacbell.net>

On Sunday 28 May 2006 7:55 am, Woodruff, Richard wrote:

> I need to do a little catch up reading...  I'm not sure this bit is
> complete on target, but...

In terms of explaining clock, power, and voltage domains I think it's a
good explanation, likely worth putting into the framework writeup.  The
concepts apply both within chips and across larger systems (like boards),
but they're not always apparent to folks who don't build computers that
run for a long time on itty bitty batteries.  ;)

Plus it didn't disagree with that point I was making:  the clock API has
no major issues handling some of the most complex real-world clock trees
that Linux deals with today, except the missing clk_must_disable() call.


> As far as clocks go a functional module can have multiple independent
> clocks required for operation.  Say for MMC on OMAP2, the DPLL derived
> clock might supply the interface clock, a fixed APLL might provide the
> functional clock, and a 32KHz clock might provide denounce.  All clocks
> are necessary for proper operation.  And in the case of OMAP2 all 3 are
> controllable at the module level.

And I'm sure Intel and Freescale can come up with similar examples using
their own most power-efficient chips:  devices that use multiple clock
domains, each clock having a single parent.


> In a more general sense for a module's functional operation there may be
> many dependencies.  This kind of thing goes beyond just clocks.  This
> can be inter-driver, LCD needs I2C to turn on a backlight.
> 
> If you look at any block of functionality you create a 'driver' for
> there can be a lot of external dependencies.  How to best express these
> dependencies is where things get unclear.  Currently things like sysfs
> and the clk_api handle part of them.  They are not complete.  

Well, sysfs is just a way to present kernel data structures, emphasis
currently being on device and driver related information.  Using that
kobject framework also helps solve some memory management nightmares.

One example of something that's missing would be an entity explicitly
representing a power or voltage domain.  While "struct clk" exposes
clock (sub)domains so drivers can work with them, and associates them
with device model tree nodes (which tend to reflect bus hierarchies not
power or clock hierarchies...) the corresponding notions for switching
power domains on/off/higher/lower are platform-specific rarities.
Any driver needing to use one adds lots of platform-specific code...
which is contrary to the goal of maintainability.


> It all gets less and less clear as you layer on more intra-soc-chip
> levels.  A chip has multiple voltage domains (VD), within each VD there
> are several power domains (PD), across several of the PDs (or fully
> within a PD) there are multiple clock domains (CD).  Each domain level
> has a number of sub levels with certain functionality states.  Finally
> you drop the end module function say MMC into this.  Software writes a
> single end driver for this which is likely part of some subsystem stack.
> This functional block (say MMC) will be part of a VD, PD, multiple CD's
> (each domain has multiple levels of operation).  To achieve power
> management at the end module you get somewhat simple controls
> (clk_enable), to achieve domain power controls, you must act on entire
> groups with-in each domain type.  Domains are contained with-in domains
> (VD contains PD) so in order to make a VD change all internal PD need to
> be at a certain state, and for this to happen all CD in a PD need to be
> in at a certain state.  For this to happen end functions (MMC) must have
> each managed their end clocks and idles properly.  

Most of that also applies at the board integration level.  MMC is a good
example to build on here, because "supply 1.8V to slot 2" is likely to
have a variety of board-specific implementations, and a given MMC card
might well need a voltage that is not currently available.


> If you handle each layer from the bottom up, you can achieve massive
> power savings.  There are many explicit relationships in this picture
> which need to be expressed.  You can't look at devices individually and
> get a domain state, you must look at them together.

Does "need to be expressed" need to be much more than "here's the
API that drivers use"?  That is, what's the point of expressing
that stuff?  It's easy to understand "writing power-aware drivers".
Also "monitoring power-aware systems".  Neither of those goals would
seem to need much in terms of infrastructure, beyond adopting some
lightweight APIs, analagous to the current clock API.


> OMAP1 which you are very familiar with does not provide this type of
> domain partitioning nor the intermediate states (and control of each)
> with in each domain.  You get much more of the whole chip in sleep
> states, but not all the partial states which are useable during active
> mode.

So who defines the states a given Linux system will use ... in terms
of the scope this new "framework" will address?

SOC/chip vendor docs more or less define what the SOC is expected to handle. 
Board designers add to that, including constraints that reduce scope, as
does that platform's Linux support.  The application software layer(s) do
the same.  And most end users won't care about many of the details.

- Dave

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Vitaly Wool]

On 9/23/06, Scott E. Preece <preece@motorola.com> wrote:
>
> I've been hoping somebody would send a good description of exactly what
> they mean by "power domain" or "voltage domain", so we could talk about
> it...

I understand it as an arbitrary set of devices grouped according to a
certain rule. An operating point will then deal with set of power
domains not with set of devices.

Vitaly

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Pavel Machek]

Hi!

> Note that I don't think PowerOp would cover all devices. In fact, I
> think most devices would remain autonomous or controlled as part of
> specific subsystems. The only things that PowerOp would bundle together
> would be things that aren't independent (and may not even be visible as
> "devices" in the usual Linux sense), but that have to be managed
> together in changing frequency/voltage. At least, that's the way I
> imagined it would work.

Well, two objections to that

a) current powerop code does not handle 256 CPU machine, because that
would need 256 independend bundles, and powerop has hardcoded "only
one bundle" rule.

b) having some devices controlled by powerop and some by specific
subsystem is indeed ugly. I'd hope powerop would cover all the
devices. (Or maybe cover _no_ devices). Userland should not need to
know if touchscreen is part of SoC or if it happens to be independend
on given machine.

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

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Matthew Locke]

On Sep 23, 2006, at 4:18 AM, Pavel Machek wrote:

> Hi!
>
>> Note that I don't think PowerOp would cover all devices. In fact, I
>> think most devices would remain autonomous or controlled as part of
>> specific subsystems. The only things that PowerOp would bundle 
>> together
>> would be things that aren't independent (and may not even be visible 
>> as
>> "devices" in the usual Linux sense), but that have to be managed
>> together in changing frequency/voltage. At least, that's the way I
>> imagined it would work.
>
> Well, two objections to that
>
> a) current powerop code does not handle 256 CPU machine, because that
> would need 256 independend bundles, and powerop has hardcoded "only
> one bundle" rule.

The 256 is only a temporary implementation limitation.

>
> b) having some devices controlled by powerop and some by specific
> subsystem is indeed ugly. I'd hope powerop would cover all the
> devices. (Or maybe cover _no_ devices). Userland should not need to
> know if touchscreen is part of SoC or if it happens to be independend
> on given machine.

PowerOP does *not* cover devices.  It covers system level parameters 
such clocks, buses, voltages.

>
> 								Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/linux-pm
>

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Pavel Machek]

Hi!
> >Well, two objections to that
> >
> >a) current powerop code does not handle 256 CPU machine, because that
> >would need 256 independend bundles, and powerop has hardcoded "only
> >one bundle" rule.
> 
> The 256 is only a temporary implementation limitation.

Really? 256 CPUs mean 2^256 states. How do you handle that without
introducing vectors?

> >b) having some devices controlled by powerop and some by specific
> >subsystem is indeed ugly. I'd hope powerop would cover all the
> >devices. (Or maybe cover _no_ devices). Userland should not need to
> >know if touchscreen is part of SoC or if it happens to be independend
> >on given machine.
> 
> PowerOP does *not* cover devices.  It covers system level parameters 
> such clocks, buses, voltages.

I've seen "usb enabled" in one of examples.. and that sure seems like
device to me.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [linux-pm] [PATCH] PowerOP, PowerOP Core, 1/2

[Matthew Locke]

On Sep 22, 2006, at 7:09 AM, Pavel Machek wrote:

> Hi!
>
>> | > >>+struct powerop_driver {
>> | > >>+	char *name;
>> | > >>+	void *(*create_point) (const char *pwr_params, va_list args);
>> | > >>+	int (*set_point) (void *md_opt);
>> | > >>+	int (*get_point) (void *md_opt, const char *pwr_params, 
>> va_list
>> | > >>args);
>> | > >>+};
>> | > >
>> | > >We can certainly get better interface than va_list, right?
>> | >
>> | > Please elaborate.
>> |
>> | va_list does not provide adequate type checking. I do not think it
>> | suitable in driver<->core interface.
>> ---
>>
>> Well, in this particular case the typing probably has to be weak, one
>> way or another. The meaning of the parameters is arguably opaque at
>
> Why not have struct powerop_parameters, defined in machine-specific
> header somewhere, but used everywhere?

We started out with a machine-specific header.  The general feedback on 
the linux-pm list was that a complete machine independent interface was 
preferred.  At first Eugeny and I were against that but the resulting 
interface is much more flexible.  Users of the PowerOP API do not have 
to include an asm/powerop.h to use it.  Instead you can get all the 
information you need from the interface.  Also, using the current 
implementation you can get/set any arbitrary number and order of the 
power parameters.  For example, a platform has parameters p0-p5.  An 
operating point could be registered with values for p0,p1 and another 
with p0,p1,p2,p3,p4 and p5.  The code that registers operating points 
and accesses existing operating points don't have to know the correct 
order of  parameters, provide values for the entire parameter list or 
have a machine specific header.  In addition to being machine 
independent, the current implementation seems to  works nicely for 
integrating with cpufreq.

We are not completely satisfied with the string parsing that results 
from the current interface but we can improve it over time.


>
>> the interface - the attributes may be meaningful to specific 
>> components
>> of the system, but are not defined in the standardized interface 
>> (which
>> would otherwise have to know about all possible kinds of power
>> attributes and be changed every time a new one is added).
>>
>> So, if you'd rather have an array of char* or void* values, that would
>> probably also meet the need, but my guess is that the typing is
>> intentionally opaque.
>
> Actually array of integers would be better than this.
>
>> | > >How is it going to work on 8cpu box? will
>> | > >you have states like cpu1_800MHz_cpu2_1600MHz_cpu3_800MHz_... ?
>> | >
>> | > i do not operate with term 'state' so I don't understand what it 
>> means here.
>> |
>> | Okay, state here means "operating point". How will operating points
>> | look on 8cpu box? That's 256 states if cpus only support "low" and
>> | "high". How do you name them?
>>
>> I don't think you would name the compounded states. Each CPU would 
>> need
>> to have its own defined set of operating points (since the 
>> capabilities
>> of the CPUs can reasonably be different).
>
> Well, having few "powerop domains" per system would likely solve that
> problem... and problem of 20 devices on my PC. Can we get that?
>
> 								Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/linux-pm
>