Pavel Machek wrote: > Hi! > > >> diff --git a/drivers/Makefile b/drivers/Makefile >> index fc2d744..f8eaf31 100644 >> --- a/drivers/Makefile >> +++ b/drivers/Makefile >> @@ -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. > > >> +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. > >> +int >> +powerop_driver_register(struct powerop_driver *p) >> +{ >> + int error = 0; >> + >> + if (! powerop_driver) { >> > > No space between ! and argument... > > >> +/* >> + * 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; >> + 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? > 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 > Pavel >