Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> writes: > On 11/21/2014 07:03 PM, Lina Iyer wrote: >> Add cpuidle driver interface to allow cpus to go into C-States. Use the >> cpuidle DT interface, common across ARM architectures, to provide the >> idle state information to the cpuidle framework. >> >> Supported modes at this time are Standby and Standalone Power Collapse. >> >> Signed-off-by: Lina Iyer <lina.iyer@xxxxxxxxxx> > > One nit and one comment below. Other than that: > > Acked-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> > > [ ... ] > >> +static int qcom_cpu_stby(struct cpuidle_device *dev, >> + struct cpuidle_driver *drv, int index) >> +{ >> + lpm_ops->standby(NULL); > > In my last comment I was referring about a check for entering > successfully the idle state: > > if (lpm_ops->standby(NULL)) > return -1; > >> + return index; >> +} >> + >> +static int qcom_cpu_spc(struct cpuidle_device *dev, >> + struct cpuidle_driver *drv, int index) >> +{ >> + lpm_ops->spc(NULL); >> + >> + return index; Similar to Daniel's comment above. if lpm_ops->spc() fails, do you want to fall back to standby. Hmm, using the DT idle states, it doesn't look as straight forward as it used to be to fall back to a "safe state." Kevin -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html