Messages in this thread | | | Date | Thu, 16 Aug 2012 15:14:19 +0100 | From | Mark Brown <> | Subject | Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences |
| |
On Thu, Aug 16, 2012 at 03:08:55PM +0900, Alexandre Courbot wrote:
> + power-off-sequence { > + step0 { > + gpio = "enable"; > + disable;
I'd change the name to "reset" or something in the example - avoids any confusion with the action.
> +#ifdef CONFIG_REGULATOR > + case POWER_SEQ_REGULATOR: > + if (pdata->regulator.enable) > + err = regulator_enable(step->resource->regulator); > + else > + err = regulator_disable(step->resource->regulator); > + break;
The regulator and GPIO APIs should stub themselves out adequately to not need the ifdefs at least for regulator I'd use the stubs since...
> + /* > + * should never happen unless the sequence includes a step which > + * type does not have support compiled in > + */ > + default: > + return -EINVAL;
...this probably isn't what's meant. It might also be nice to have support for bulk_enable() but that's definitely something that could be added later.
> + err = power_seq_step_run(&seq->steps[cpt]); > + if (err) { > + dev_err(dev, "error %d while running power sequence!\n", > + err); > + return err;
I'd also log at least the step number, it'll make diagnostics easier.
| |