[PATCH 1/2] regulator: Document binding for regulator suspend voltage

[PATCH 1/2] regulator: Document binding for regulator suspend voltage

[Doug Anderson]

This patch builds upon (291d761 regulator: Document binding for
regulator suspend state for PM state) to allow setting the uV in
addition to the state at suspend time.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 Documentation/devicetree/bindings/regulator/regulator.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index aaad615..4e7ed76 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -28,6 +28,8 @@ Optional properties:
 - regulator-state-[mem/disk] node has following common properties:
 	- regulator-on-in-suspend: regulator should be on in suspend state.
 	- regulator-off-in-suspend: regulator should be off in suspend state.
+	- regulator-suspend-microvolt: regulator should be set to this voltage
+	  in suspend.
 
 Deprecated properties:
 - regulator-compatible: If a regulator chip contains multiple
-- 
2.1.0.rc2.206.gedb03e5

[PATCH 2/2] regulator: of: Add support for parsing microvolts for suspend state

[Doug Anderson]

Leverage all the work that was done in (40e20d6 regulator: of: Add
support for parsing regulator_state for suspend state) and throw in
the ability to set suspend microvolts from the device tree.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/regulator/of_regulator.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index f0d19fc..7fe93c5 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -107,6 +107,10 @@ static void of_get_regulation_constraints(struct device_node *np,
 					"regulator-off-in-suspend"))
 			suspend_state->disabled = true;
 
+		if (!of_property_read_u32(suspend_np,
+					"regulator-suspend-microvolt", &pval))
+			suspend_state->uV = pval;
+
 		of_node_put(suspend_np);
 		suspend_state = NULL;
 		suspend_np = NULL;
-- 
2.1.0.rc2.206.gedb03e5

Re: [PATCH 1/2] regulator: Document binding for regulator suspend voltage

[Javier Martinez Canillas]

Hello Doug,

On 11/01/2014 04:52 AM, Doug Anderson wrote:
> This patch builds upon (291d761 regulator: Document binding for
> regulator suspend state for PM state) to allow setting the uV in
> addition to the state at suspend time.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  Documentation/devicetree/bindings/regulator/regulator.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
> index aaad615..4e7ed76 100644
> --- a/Documentation/devicetree/bindings/regulator/regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/regulator.txt
> @@ -28,6 +28,8 @@ Optional properties:
>  - regulator-state-[mem/disk] node has following common properties:
>  	- regulator-on-in-suspend: regulator should be on in suspend state.
>  	- regulator-off-in-suspend: regulator should be off in suspend state.
> +	- regulator-suspend-microvolt: regulator should be set to this voltage
> +	  in suspend.

The patch looks good to me:

Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

On thing I wonder is if the binding should say that the suspend voltage is
independent of the runtime one and it may be outside of the runtime range?

Best regards,
Javier

Re: [PATCH 2/2] regulator: of: Add support for parsing microvolts for suspend state

[Javier Martinez Canillas]

Hello Doug,

On 11/01/2014 04:52 AM, Doug Anderson wrote:
> Leverage all the work that was done in (40e20d6 regulator: of: Add
> support for parsing regulator_state for suspend state) and throw in
> the ability to set suspend microvolts from the device tree.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  drivers/regulator/of_regulator.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
> index f0d19fc..7fe93c5 100644
> --- a/drivers/regulator/of_regulator.c
> +++ b/drivers/regulator/of_regulator.c
> @@ -107,6 +107,10 @@ static void of_get_regulation_constraints(struct device_node *np,
>  					"regulator-off-in-suspend"))
>  			suspend_state->disabled = true;
>  
> +		if (!of_property_read_u32(suspend_np,
> +					"regulator-suspend-microvolt", &pval))
> +			suspend_state->uV = pval;
> +
>  		of_node_put(suspend_np);
>  		suspend_state = NULL;
>  		suspend_np = NULL;
> 

Looks good to me.

Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier

Re: [PATCH 1/2] regulator: Document binding for regulator suspend voltage

[Chris Zhong]

On 11/01/2014 04:45 PM, Javier Martinez Canillas wrote:
> Hello Doug,
>
> On 11/01/2014 04:52 AM, Doug Anderson wrote:
>> This patch builds upon (291d761 regulator: Document binding for
>> regulator suspend state for PM state) to allow setting the uV in
>> addition to the state at suspend time.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> ---
>>   Documentation/devicetree/bindings/regulator/regulator.txt | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
>> index aaad615..4e7ed76 100644
>> --- a/Documentation/devicetree/bindings/regulator/regulator.txt
>> +++ b/Documentation/devicetree/bindings/regulator/regulator.txt
>> @@ -28,6 +28,8 @@ Optional properties:
>>   - regulator-state-[mem/disk] node has following common properties:
>>   	- regulator-on-in-suspend: regulator should be on in suspend state.
>>   	- regulator-off-in-suspend: regulator should be off in suspend state.
>> +	- regulator-suspend-microvolt: regulator should be set to this voltage
>> +	  in suspend.
> The patch looks good to me:
>
> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>
> On thing I wonder is if the binding should say that the suspend voltage is
> independent of the runtime one and it may be outside of the runtime range?
>
> Best regards,
> Javier
>
>
>

Reviewed-by: Chris Zhong <zyw@rock-chips.com>

Re: [PATCH 2/2] regulator: of: Add support for parsing microvolts for suspend state

[Chris Zhong]

On 11/01/2014 04:49 PM, Javier Martinez Canillas wrote:
> Hello Doug,
>
> On 11/01/2014 04:52 AM, Doug Anderson wrote:
>> Leverage all the work that was done in (40e20d6 regulator: of: Add
>> support for parsing regulator_state for suspend state) and throw in
>> the ability to set suspend microvolts from the device tree.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> ---
>>   drivers/regulator/of_regulator.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
>> index f0d19fc..7fe93c5 100644
>> --- a/drivers/regulator/of_regulator.c
>> +++ b/drivers/regulator/of_regulator.c
>> @@ -107,6 +107,10 @@ static void of_get_regulation_constraints(struct device_node *np,
>>   					"regulator-off-in-suspend"))
>>   			suspend_state->disabled = true;
>>   
>> +		if (!of_property_read_u32(suspend_np,
>> +					"regulator-suspend-microvolt", &pval))
>> +			suspend_state->uV = pval;
>> +
>>   		of_node_put(suspend_np);
>>   		suspend_state = NULL;
>>   		suspend_np = NULL;
>>
> Looks good to me.
>
> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>
> Best regards,
> Javier
>
>
>
Reviewed-by: Chris Zhong <zyw@rock-chips.com>

Re: [PATCH 2/2] regulator: of: Add support for parsing microvolts for suspend state

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

On Sat, Nov 01, 2014 at 09:49:42AM +0100, Javier Martinez Canillas wrote:
> On 11/01/2014 04:52 AM, Doug Anderson wrote:

> > +
> >  		of_node_put(suspend_np);
> >  		suspend_state = NULL;
> >  		suspend_np = NULL;

> Looks good to me.

> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

If you're reviewing something please don't quote the entire patch, the
same rule applies as always - just include relevant context - and
especially don't include your one line of content at the very end of the
patch.  Otherwise people have to page through the entire thing to find
what you've said (if they don't get fed up first).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 1/2] regulator: Document binding for regulator suspend voltage

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 260 bytes --]

On Fri, Oct 31, 2014 at 08:52:57PM -0700, Doug Anderson wrote:
> This patch builds upon (291d761 regulator: Document binding for
> regulator suspend state for PM state) to allow setting the uV in
> addition to the state at suspend time.

Applied both, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 2/2] regulator: of: Add support for parsing microvolts for suspend state

[Javier Martinez Canillas]

On 11/03/2014 01:29 PM, Mark Brown wrote:
> 
> If you're reviewing something please don't quote the entire patch, the
> same rule applies as always - just include relevant context - and
> especially don't include your one line of content at the very end of the
> patch.  Otherwise people have to page through the entire thing to find
> what you've said (if they don't get fed up first).
> 

Yes, I usually add my {Review,Acked,Tested}-by tags at the end of the
commit message and remove the complete patch but since it was a 4-lines
patch I didn't think it was that bad.

Sorry about that, I will do it next time no matter the patch size.

Best regards,
Javier

Re: [PATCH 2/2] regulator: of: Add support for parsing microvolts for suspend state

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

On Mon, Nov 03, 2014 at 01:45:15PM +0100, Javier Martinez Canillas wrote:

> Yes, I usually add my {Review,Acked,Tested}-by tags at the end of the
> commit message and remove the complete patch but since it was a 4-lines
> patch I didn't think it was that bad.

It's the patch, the diffstat, the signoff, the commit message...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]