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
