Re: [RFC] PowerOP, PowerOP Core 1/3

[Pavel Machek <pavel@suse.cz>]

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?


> +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?

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