Skip to contentSkip to conversation

PowerOP, PowerOP Core 1/3

RFC In review Eugeny S. Mints 2 participants 4 messages Aug 8, 2006 – Aug 24, 2006 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

RFC 1/1 PowerOP, PowerOP Core 1/3

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. PowerOP Core maintains list of operating points and provides capability to reference operating points by name.

arch/arm/Kconfig +2 -0
index f81a623..bad6269 100644
@@ -899,6 +899,8 @@ source "drivers/mmc/Kconfig"
 
 source "drivers/rtc/Kconfig"
 
+source "drivers/powerop/Kconfig"
+
 endmenu
 
 source "fs/Kconfig"
drivers/Makefile +1 -0
index fc2d744..f8eaf31 100644
@@ -65,6 +65,7 @@ obj-$(CONFIG_ISDN)		+= isdn/
 obj-$(CONFIG_EDAC)		+= edac/
 obj-$(CONFIG_MCA)		+= mca/
 obj-$(CONFIG_EISA)		+= eisa/
+obj-$(CONFIG_POWEROP)		+= powerop/
 obj-$(CONFIG_CPU_FREQ)		+= cpufreq/
 obj-$(CONFIG_MMC)		+= mmc/
 obj-$(CONFIG_NEW_LEDS)		+= leds/
drivers/powerop/Kconfig new file +12 -0
new file mode 100644
index 0000000..94d2459
@@ -0,0 +1,12 @@
+#
+# powerop
+#
+
+menu "PowerOP (Power Management)"
+
+config POWEROP
+	bool "PowerOP Core"
+	help
+
+endmenu
+
drivers/powerop/Makefile new file +2 -0
new file mode 100644
index 0000000..131b983
@@ -0,0 +1,2 @@
+obj-$(CONFIG_POWEROP)		+= powerop.o
+
drivers/powerop/powerop.c new file +389 -0

389 lines not on this page · raw message

include/linux/powerop.h new file +110 -0

110 lines not on this page · raw message

#1 lore raw

Hi!

drivers/Makefile obj-$(CONFIG_ISDN) += isdn/ · line 68

Quoted 9 lines
drivers/Makefile +1 -0
index fc2d744..f8eaf31 100644
@@ -65,6 +65,7 @@ obj-$(CONFIG_ISDN)		+= isdn/
 obj-$(CONFIG_EDAC)		+= edac/
 obj-$(CONFIG_MCA)		+= mca/
 obj-$(CONFIG_EISA)		+= eisa/
+obj-$(CONFIG_POWEROP)		+= powerop/

Can we make it drivers/power? It is probably suitable for more than powerop.

drivers/Makefile obj-$(CONFIG_ISDN) += isdn/ · line 71

Quoted 3 lines
drivers/Makefile +3 -0
+static int
+get_point(void *md_opt, const char *pwr_params, va_list args)
+{

Uhuh... is there a way to use something safer than unchecked va_list s?

drivers/Makefile obj-$(CONFIG_ISDN) += isdn/

Quoted 6 lines
drivers/Makefile +6 -0
+int 
+powerop_driver_register(struct powerop_driver *p)
+{
+	int error = 0;
+
+	if (! powerop_driver) {

No space between ! and argument...

Quoted 24 lines
drivers/Makefile +24 -0
+/* 
+ * powerop_register_point - 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
+ * 
+ *
+ * INPUT
+ *   id         - operating point name
+ *   pwr_params - set of (power parameter name, value) pairs
+ *
+ * OUTPUT
+ *   none
+ *
+ * RETURN
+ *   zero on success, error code otherwise
+ *    
+ */
+int 
+powerop_register_point(const char *id, const char *pwr_params, ...)
+{
+	int err = 0;

drivers/Makefile

Quoted 10 lines
drivers/Makefile +10 -0
+	struct powerop_point *opt;
+	va_list args;
+
+	if ((!powerop_initialized) || (id == NULL))
+		return -EINVAL;
+
+	if ((opt = kmalloc(sizeof(struct powerop_point), GFP_KERNEL)) == NULL)
+		return -ENOMEM;
+
+	memset(opt, 0, sizeof(struct powerop_point));

kzalloc?

Why do we want one more string parser in kernel?

Pavel

Signature · 1 line
Thanks for all the (sleeping) penguins.
#2 lore raw
Quoted 18 lines · Pavel Machek

Hi!

drivers/Makefile obj-$(CONFIG_ISDN) += isdn/ · line 69

Quoted 10 lines
drivers/Makefile +1 -0
index fc2d744..f8eaf31 100644
@@ -65,6 +65,7 @@ obj-$(CONFIG_ISDN)		+= isdn/
 obj-$(CONFIG_EDAC)		+= edac/
 obj-$(CONFIG_MCA)		+= mca/
 obj-$(CONFIG_EISA)		+= eisa/
+obj-$(CONFIG_POWEROP)		+= powerop/
 

Can we make it drivers/power? It is probably suitable for more than powerop.

Quoted 9 lines

drivers/Makefile obj-$(CONFIG_ISDN) += isdn/

Quoted 4 lines
drivers/Makefile +3 -0
+static int
+get_point(void *md_opt, const char *pwr_params, va_list args)
+{

Uhuh... is there a way to use something safer than unchecked va_list s?

please elaborate.

Quoted 50 lines

drivers/Makefile

Quoted 8 lines
drivers/Makefile +6 -0
+int 
+powerop_driver_register(struct powerop_driver *p)
+{
+	int error = 0;
+
+	if (! powerop_driver) {

No space between ! and argument...

Quoted 25 lines
drivers/Makefile +24 -0
+/* 
+ * powerop_register_point - 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
+ * 
+ *
+ * INPUT
+ *   id         - operating point name
+ *   pwr_params - set of (power parameter name, value) pairs
+ *
+ * OUTPUT
+ *   none
+ *
+ * RETURN
+ *   zero on success, error code otherwise
+ *    
+ */
+int 
+powerop_register_point(const char *id, const char *pwr_params, ...)
+{
+	int err = 0;

drivers/Makefile

Quoted 10 lines
drivers/Makefile +10 -0
+	struct powerop_point *opt;
+	va_list args;
+
+	if ((!powerop_initialized) || (id == NULL))
+		return -EINVAL;
+
+	if ((opt = kmalloc(sizeof(struct powerop_point), GFP_KERNEL)) == NULL)
+		return -ENOMEM;
+
+	memset(opt, 0, sizeof(struct powerop_point));

kzalloc?

Quoted 2 lines

Why do we want one more string parser in kernel?

referencing power parameters by name interface is result of gradual evolution of PowerOP interface mainly in order to address the requirement to be truly arch independent interface and to get rid of sharing an arch dependent data structures (a structure which defines operating point for a certain platform) between layers on top and beneath PowerOP (see the picture of the layers; thus a layer which creates operating points with help of PowerOP don't need to include arch dependent definition of operating point structure from PM Core layer). The latter fixes the overall PM architecture with OOP design concept.

Thanks,
Eugeny

Quoted 2 lines

Pavel

#3 lore raw

Hi!

Quoted 30 lines
Quoted 28 lines
Quoted 23 lines

+/*
+ * powerop_register_point - 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
+ *
+ *
+ * INPUT
+ * id - operating point name
+ * pwr_params - set of (power parameter name, value) pairs
+ *
+ * OUTPUT
+ * none
+ *
+ * RETURN
+ * zero on success, error code otherwise
+ *
+ */

Review

Quoted 2 lines
(quoted diff) +2 -0
+int 
+powerop_register_point(const char *id, const char *pwr_params, ...)

kzalloc?

Quoted 2 lines

Why do we want one more string parser in kernel?

Quoted 9 lines

referencing power parameters by name interface is result of gradual evolution of PowerOP interface mainly in order to address the requirement to be truly arch independent interface and to get rid of sharing an arch dependent data structures (a structure which defines operating point for a certain platform) between layers on top and beneath PowerOP (see the picture of the layers; thus a layer which creates operating points with help of PowerOP don't need to include arch dependent definition of operating point structure from PM Core layer). The latter fixes the overall PM architecture with OOP design concept.

We need better solution. Putting string parser into kernel because you feel OOP is nice is bad idea. See vfs for example of OOP-like design without string parsers.
Pavel

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