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. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html