Dear Mark, On 09/05/2014 07:49 AM, Mark Brown wrote: > On Mon, Aug 18, 2014 at 03:27:02PM +0900, Chanwoo Choi wrote: > >> + suspend_uV = of_get_property(suspend_np, "regulator-volt", >> + NULL); >> + if (suspend_uV) { >> + suspend_state->uV = be32_to_cpu(*suspend_uV); >> + >> + if (suspend_state->uV < constraints->min_uV) >> + suspend_state->uV = constraints->min_uV; >> + if (suspend_state->uV > constraints->max_uV) >> + suspend_state->uV = constraints->max_uV; >> + } > > A few things here. One is that this will fail if we don't have a > voltage range specified at runtime. It is possible that the user needs > to override the voltage in suspend mode but not at runtime. Perhaps we > want to make them specify the runtime voltage anyway but if we do then > the binding needs to say that this is mandatory even if it's just > restating the hardware default. > > It's also not clear to me that the suspend voltage needs to be in the > range that we can vary at runtime, especially if we're not giving > permission for runtime variation. From that point of view we should > probably just not check, but if we are going to check we should print a > warning that we're doing it so people know that what they set in the DT > isn't being followed. > Firstly, I'm so sorry about late reply because of vacation. I'll drop 'regulator-volt' property because of remaining with not clear state. Next patchset (v5) will just include 'regulator-{on,off}-in-suspend' without 'regulator-volt' property. Best Regards, Chanwoo Choi -- 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