Skip to contentSkip to conversation

PowerOP, PowerOP Core, 1/2

PATCH In review Eugeny S. Mints 4 participants 7 messages Sep 14, 2006 – Sep 19, 2006

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 PowerOP, PowerOP Core, 1/2

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.

include/linux/powerop.h new file +42 -0
new file mode 100644
index 0000000..704620d
@@ -0,0 +1,42 @@
+/*
+ * PowerOP core definitions
+ *
+ * Author: Eugeny S. Mints <[email protected]>
+ *
+ * 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 <[email protected]>
+ */
+#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__ */
include/linux/powerop_sysfs.h new file +59 -0

59 lines not on this page · raw message

kernel/power/Kconfig +28 -0
index ae44a70..2bef726 100644
@@ -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
kernel/power/Makefile +9 -0
index 8d0af3d..1045163 100644
@@ -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
kernel/power/main.c +2 -1
index 6d295c7..b113f79 100644
@@ -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.
kernel/power/powerop.c new file +407 -0

407 lines not on this page · raw message

kernel/power/powerop_point.h new file +36 -0
new file mode 100644
index 0000000..64b5591
@@ -0,0 +1,36 @@
+/*
+ * PowerOP core non-public header
+ *
+ * Author: Eugeny S. Mints <[email protected]>
+ *
+ * 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 <[email protected]>
+ */
+#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__ */
kernel/power/powerop_sysfs.c new file +254 -0

254 lines not on this page · raw message

kernel/power/powerop_sysfs_point.h new file +42 -0

42 lines not on this page · raw message

#1 lore raw

Hi!

Quoted 5 lines

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.

include/linux/powerop.h line 27

Quoted 6 lines
include/linux/powerop.h new file +6 -0
+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?

kernel/power/Kconfig config SUSPEND_SMP · line 116

Quoted 10 lines
kernel/power/Kconfig +10 -0
+
+#
+# powerop
+#
+
+menu "PowerOP (Power Management)"
+
+config POWEROP
+	tristate "PowerOP Core"
+	help

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

Quoted 9 lines
kernel/power/powerop.c new file +9 -0
+	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;
+

kernel/power/powerop.c line 205

Quoted 10 lines
kernel/power/powerop.c new file +10 -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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#2 lore raw
Quoted 9 lines · Pavel Machek

Hi!

Quoted 5 lines

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.

Quoted 3 lines

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

seems already discussed

Quoted 8 lines

include/linux/powerop.h line 27

Quoted 6 lines
include/linux/powerop.h new file +6 -0
+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.

Quoted 13 lines

kernel/power/Kconfig config SUSPEND_SMP · line 116

Quoted 11 lines
kernel/power/Kconfig +10 -0
+
+#
+# powerop
+#
+
+menu "PowerOP (Power Management)"
+
+config POWEROP
+	tristate "PowerOP Core"
+	help

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

please elaborate.

Quoted 13 lines
Quoted 13 lines
kernel/power/powerop.c new file +13 -0
+	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:
Quoted 8 lines

kernel/power/powerop.c line 205

Quoted 6 lines
kernel/power/powerop.c new file +6 -0
+	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.

Quoted 2 lines

...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.

Quoted 2 lines

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

Quoted 1 line

Pavel

#6 lore raw
In reply to Eugeny S. Mints
Quoted 5 lines · Eugeny S. Mints

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

#3 lore raw
In reply to Eugeny S. Mints
Quoted 3 lines · Eugeny S. Mints
Quoted 2 lines · Pavel Machek

Hi!

[skip]

Quoted 2 lines
Quoted 2 lines

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

Quoted 8 lines

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

Eugeny

Quoted 1 line

Pavel

#4 lore raw

Eugeny,

Quoted 3 lines · Eugeny S. Mints

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

#5 lore raw
In reply to Eugeny S. Mints

Hi!

Quoted 11 lines
Quoted 9 lines
Quoted 7 lines
include/linux/powerop.h new file +5 -0
+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.

Quoted 12 lines
Quoted 11 lines

kernel/power/Kconfig config SUSPEND_SMP · line 116

Quoted 9 lines
kernel/power/Kconfig +9 -0
+#
+# powerop
+#
+
+menu "PowerOP (Power Management)"
+
+config POWEROP
+	tristate "PowerOP Core"
+	help

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

Quoted 1 line

please elaborate.

You probably want to include some help text.

Quoted 4 lines
Quoted 2 lines

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

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