Re: [PATCHv4 1/2] regulator: of: Add support for parsing regulator_state for suspend state

[Chanwoo Choi]

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 linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/