[PATCH V2] Regulator: Reorder the min max assignment in the sequence of regulator_set_voltage function

[PATCH V2] Regulator: Reorder the min max assignment in the sequence of regulator_set_voltage function

[J Keerthy]

The min and max values for regulators are getting assigned before actually
the voltage is set. So making sure that min and max values are assigned
only if the voltage is successfully set else keeping the last successfully
set voltage's min and max values.

This is boot tested on OMAP4430 and OMAP4460 boards.

V2: Fixed comment from Mark. Introduced local variables to hold current
min and max values.
 
Signed-off-by: J Keerthy <j-keerthy@ti.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/regulator/core.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 2785843..44a9b84 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2293,7 +2293,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 {
 	struct regulator_dev *rdev = regulator->rdev;
-	int ret = 0;
+	int ret = 0, curr_min, curr_max;
 
 	mutex_lock(&rdev->mutex);
 
@@ -2315,15 +2315,23 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 	ret = regulator_check_voltage(rdev, &min_uV, &max_uV);
 	if (ret < 0)
 		goto out;
+	curr_min = regulator->min_uV;
+	curr_max = regulator->max_uV;
 	regulator->min_uV = min_uV;
 	regulator->max_uV = max_uV;
 
 	ret = regulator_check_consumers(rdev, &min_uV, &max_uV);
 	if (ret < 0)
-		goto out;
+		goto err;
 
 	ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
 
+err:
+	if (ret) {
+		regulator->min_uV = curr_min;
+		regulator->max_uV = curr_max;
+	}
+
 out:
 	mutex_unlock(&rdev->mutex);
 	return ret;
-- 
1.7.5.4

Re: [PATCH V2] Regulator: Reorder the min max assignment in the sequence of regulator_set_voltage function

[Mark Brown]

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

On Mon, Feb 11, 2013 at 12:19:34PM +0530, J Keerthy wrote:

> The min and max values for regulators are getting assigned before actually
> the voltage is set. So making sure that min and max values are assigned
> only if the voltage is successfully set else keeping the last successfully
> set voltage's min and max values.

> This is boot tested on OMAP4430 and OMAP4460 boards.

This seems fine but doesn't apply against topic/core or -next, can you
please regenerate against topic/core and resend?

> V2: Fixed comment from Mark. Introduced local variables to hold current
> min and max values.

Don't include things like this in the changelog, include them after the
--- as covered in SubmittingPatches.

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

RE: [PATCH V2] Regulator: Reorder the min max assignment in the sequence of regulator_set_voltage function

[J, KEERTHY]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
> Sent: Monday, February 11, 2013 10:35 PM
> To: J, KEERTHY
> Cc: linux-kernel@vger.kernel.org; lgirdwood@gmail.com
> Subject: Re: [PATCH V2] Regulator: Reorder the min max assignment in
> the sequence of regulator_set_voltage function
> 
> On Mon, Feb 11, 2013 at 12:19:34PM +0530, J Keerthy wrote:
> 
> > The min and max values for regulators are getting assigned before
> > actually the voltage is set. So making sure that min and max values
> > are assigned only if the voltage is successfully set else keeping the
> > last successfully set voltage's min and max values.
> 
> > This is boot tested on OMAP4430 and OMAP4460 boards.
> 
> This seems fine but doesn't apply against topic/core or -next, can you
> please regenerate against topic/core and resend?

Ok. I will rebase.

> 
> > V2: Fixed comment from Mark. Introduced local variables to hold
> > current min and max values.
> 
> Don't include things like this in the changelog, include them after the
> --- as covered in SubmittingPatches.

Ok.

RE: [PATCH V2] Regulator: Reorder the min max assignment in the sequence of regulator_set_voltage function

[J, KEERTHY]

Hi Mark,

> -----Original Message-----
> From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
> Sent: Monday, February 11, 2013 10:35 PM
> To: J, KEERTHY
> Cc: linux-kernel@vger.kernel.org; lgirdwood@gmail.com
> Subject: Re: [PATCH V2] Regulator: Reorder the min max assignment in
> the sequence of regulator_set_voltage function
> 
> On Mon, Feb 11, 2013 at 12:19:34PM +0530, J Keerthy wrote:
> 
> > The min and max values for regulators are getting assigned before
> > actually the voltage is set. So making sure that min and max values
> > are assigned only if the voltage is successfully set else keeping the
> > last successfully set voltage's min and max values.
> 
> > This is boot tested on OMAP4430 and OMAP4460 boards.
> 
> This seems fine but doesn't apply against topic/core or -next, can you
> please regenerate against topic/core and resend?

I pulled the topic/core on to my tree. Seems like the exact fix has
Already been made in that branch :-P.

commit 92d7a55879c01b30349045501108e775655a4b92
Author: Paolo Pisati <paolo.pisati@canonical.com>
Date:   Thu Dec 13 10:13:00 2012 +0100

    regulator: core: if voltage scaling fails, restore original voltage values

It is already taken care. 
> 
> > V2: Fixed comment from Mark. Introduced local variables to hold
> > current min and max values.
> 
> Don't include things like this in the changelog, include them after the
> --- as covered in SubmittingPatches.

Ok

Thanks,
Keerthy