Hi!
> Here is the core patch for the PowerOp concept. It adds the powerop
> struct
> for opertaing point support to linux/pm.h and adds support to
> transition to supported operating points by
> setting their name into /sys/power/state.
>
> The supported operating points are shown in a readonly sysfs file,
> /sys/power/supported_states.
Could you fix the /sys interface to be value-per-file, and split it to
separate patch?
> -static const char * const pm_states[PM_SUSPEND_MAX] = {
> - [PM_SUSPEND_STANDBY] = "standby",
> - [PM_SUSPEND_MEM] = "mem",
> +static struct powerop standby = {
> + .name = "standby",
> + .description = "Power-On Suspend ACPI State: S1",
> + .type = PM_SUSPEND_STANDBY,
> +};
You got the description fields wrong...
> +static struct powerop mem = {
> + .name = "mem ",
> + .description = "Suspend-to-RAM ACPI State: S3",
> + .type = PM_SUSPEND_MEM,
> +};
...for example mem is used on non-acpi machines, too. Plus, it is
useless.
> +#endif
>
> -static inline int valid_state(suspend_state_t state)
> +/*
> + *
> + */
> +static int pm_change_state(struct powerop *state)
Eh?
> + /*
> + * list_find new operating point.
> + * compare to current operating point.
> + * if different change to new operating point.
> + */
> + list_for_each_entry_safe(this, next, head, list) {
> + if (strncmp(state->name, this->name, len) == 0) {
> + if ((strcmp(current_state->name, this->name))
> == 0) {
Do we really need to do list search here?
> + return 0;
> + }
And please avoid {} around single return.
> + /*
> + * now lets wait for the transition latency
> + */
> + udelay(this->latency);
udelay in generic code seems wrong.
> @@ -211,7 +275,15 @@ static int enter_state(suspend_state_t s
> */
> int software_suspend(void)
> {
> - return enter_state(PM_SUSPEND_DISK);
> + struct powerop *this, *next;
> + struct list_head *head = &pm_states.list;
> + int error = 0;
> +
> + list_for_each_entry_safe(this, next, head, list) {
> + if (this->type == PM_SUSPEND_DISK)
> + error= enter_state(this);
> + }
> + return error;
> }
Ugly....
> +struct powerop {
> + struct list_head list;
> + suspend_state_t type;
> + char name[PM_NAME_SIZE];
> + char description[PM_DESCRIPTION_SIZE];
> + unsigned int frequency; /* in KHz */
> + unsigned int voltage; /* mV */
having voltage/frequency/latency field for suspend-to-disk seems wrong.
--
Thanks for all the (sleeping) penguins.
