[PATCH v2 1/3] regulator: s2mps11: Fix accidental enable of buck6 ramp delay

[PATCH v2 1/3] regulator: s2mps11: Fix accidental enable of buck6 ramp delay

[Krzysztof Kozlowski]

S2MPS11 supports enabling/disabling ramp delay only for buck[2346].
Other bucks have ramp delay enabled always.

However the bit shift for enabling buck6 ramp delay in register is equal
to 0. When ramp delay was set for the bucks unsupporting enable/disable
(buck[15789] and buck10), the ramp delay for buck6 was also enabled.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Fixes: b96244fad953 ("regulator: s2mps11: Don't check enable_shift before setting enable ramp rate")
---

Changes since v2:
 * Apply Axel Lin's suggestion (don't change the meaning
   of enable_shift variable).

---
 drivers/regulator/s2mps11.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index e713c162fbd4..aaca37e1424f 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -202,11 +202,16 @@ static int s2mps11_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
 	if (!ramp_enable)
 		goto ramp_disable;
 
-	ret = regmap_update_bits(rdev->regmap, S2MPS11_REG_RAMP,
-				 1 << enable_shift, 1 << enable_shift);
-	if (ret) {
-		dev_err(&rdev->dev, "failed to enable ramp rate\n");
-		return ret;
+	/* Ramp delay can be enabled/disabled only for buck[2346] */
+	if ((rdev_get_id(rdev) >= S2MPS11_BUCK2 &&
+			rdev_get_id(rdev) <= S2MPS11_BUCK4) ||
+			rdev_get_id(rdev) == S2MPS11_BUCK6)  {
+		ret = regmap_update_bits(rdev->regmap, S2MPS11_REG_RAMP,
+					 1 << enable_shift, 1 << enable_shift);
+		if (ret) {
+			dev_err(&rdev->dev, "failed to enable ramp rate\n");
+			return ret;
+		}
 	}
 
 	ramp_val = get_ramp_delay(ramp_delay);
-- 
1.9.1

[PATCH v2 2/3] regulator: s2mpa01: Fix accidental enable of buck4 ramp delay

[Krzysztof Kozlowski]

S2MPA01 supports enabling/disabling ramp delay only for buck[1234].
Other bucks have ramp delay enabled always.

However the bit shift for enabling buck4 ramp delay in register is equal
to 0. When ramp delay was set for the bucks unsupporting enable/disable
(buck[56789] and buck10), the ramp delay for buck4 was also enabled.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Fixes: f7b1a8dc1c1c ("regulator: s2mpa01: Don't check enable_shift before setting enable ramp rate")
---

Changes since v2:
 * Apply Axel Lin's suggestion (don't change the meaning
   of enable_shift variable).

---
 drivers/regulator/s2mpa01.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
index f19a30f0fb42..cab1a2b9efc5 100644
--- a/drivers/regulator/s2mpa01.c
+++ b/drivers/regulator/s2mpa01.c
@@ -192,11 +192,15 @@ static int s2mpa01_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
 	if (!ramp_enable)
 		goto ramp_disable;
 
-	ret = regmap_update_bits(rdev->regmap, S2MPA01_REG_RAMP1,
-				 1 << enable_shift, 1 << enable_shift);
-	if (ret) {
-		dev_err(&rdev->dev, "failed to enable ramp rate\n");
-		return ret;
+	/* Ramp delay can be enabled/disabled only for buck[1234] */
+	if (rdev_get_id(rdev) >= S2MPA01_BUCK1 &&
+			rdev_get_id(rdev) <= S2MPA01_BUCK4) {
+		ret = regmap_update_bits(rdev->regmap, S2MPA01_REG_RAMP1,
+					 1 << enable_shift, 1 << enable_shift);
+		if (ret) {
+			dev_err(&rdev->dev, "failed to enable ramp rate\n");
+			return ret;
+		}
 	}
 
 	ramp_val = get_ramp_delay(ramp_delay);
-- 
1.9.1

[PATCH v2 3/3] regulator: s2mpa01: Use rdev_get_id() to access id of regulator

[Krzysztof Kozlowski]

Use regulator API rdev_get_id() to access id of regulator.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/regulator/s2mpa01.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
index cab1a2b9efc5..b71e11a6c773 100644
--- a/drivers/regulator/s2mpa01.c
+++ b/drivers/regulator/s2mpa01.c
@@ -61,7 +61,7 @@ static int s2mpa01_regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 	unsigned int ramp_delay = 0;
 	int old_volt, new_volt;
 
-	switch (rdev->desc->id) {
+	switch (rdev_get_id(rdev)) {
 	case S2MPA01_BUCK2:
 	case S2MPA01_BUCK4:
 		ramp_delay = s2mpa01->ramp_delay24;
@@ -102,7 +102,7 @@ static int s2mpa01_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
 	unsigned int ramp_enable = 1, enable_shift = 0;
 	int ret;
 
-	switch (rdev->desc->id) {
+	switch (rdev_get_id(rdev)) {
 	case S2MPA01_BUCK1:
 		enable_shift = S2MPA01_BUCK1_RAMP_EN_SHIFT;
 		if (!ramp_delay) {
-- 
1.9.1

Re: [PATCH v2 3/3] regulator: s2mpa01: Use rdev_get_id() to access id of regulator

[Mark Brown]

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

On Tue, May 06, 2014 at 08:37:38AM +0200, Krzysztof Kozlowski wrote:
> Use regulator API rdev_get_id() to access id of regulator.

Applied, thanks (I did actually apply the v2 of the first two, I just
replied to the wrong version in my mailbox).

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

Re: [PATCH v2 1/3] regulator: s2mps11: Fix accidental enable of buck6 ramp delay

[Axel Lin]

2014-05-06 14:37 GMT+08:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>:
> S2MPS11 supports enabling/disabling ramp delay only for buck[2346].
> Other bucks have ramp delay enabled always.
>
> However the bit shift for enabling buck6 ramp delay in register is equal
> to 0. When ramp delay was set for the bucks unsupporting enable/disable
> (buck[15789] and buck10), the ramp delay for buck6 was also enabled.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Fixes: b96244fad953 ("regulator: s2mps11: Don't check enable_shift before setting enable ramp rate")

For this serial,
Reviewed-by: Axel Lin <axel.lin@ingics.com>

Re: [PATCH v2 1/3] regulator: s2mps11: Fix accidental enable of buck6 ramp delay

[Krzysztof Kozlowski]

I forgot to CC the stable.

Mark, if the patches are OK and if you'll apply them, could you add a CC
stable to the commit msg?

Best regards,
Krzysztof


On wto, 2014-05-06 at 08:37 +0200, Krzysztof Kozlowski wrote:
> S2MPS11 supports enabling/disabling ramp delay only for buck[2346].
> Other bucks have ramp delay enabled always.
> 
> However the bit shift for enabling buck6 ramp delay in register is equal
> to 0. When ramp delay was set for the bucks unsupporting enable/disable
> (buck[15789] and buck10), the ramp delay for buck6 was also enabled.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Fixes: b96244fad953 ("regulator: s2mps11: Don't check enable_shift before setting enable ramp rate")
> ---
> 
> Changes since v2:
>  * Apply Axel Lin's suggestion (don't change the meaning
>    of enable_shift variable).
> 
> ---
>  drivers/regulator/s2mps11.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
> index e713c162fbd4..aaca37e1424f 100644
> --- a/drivers/regulator/s2mps11.c
> +++ b/drivers/regulator/s2mps11.c
> @@ -202,11 +202,16 @@ static int s2mps11_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
>  	if (!ramp_enable)
>  		goto ramp_disable;
>  
> -	ret = regmap_update_bits(rdev->regmap, S2MPS11_REG_RAMP,
> -				 1 << enable_shift, 1 << enable_shift);
> -	if (ret) {
> -		dev_err(&rdev->dev, "failed to enable ramp rate\n");
> -		return ret;
> +	/* Ramp delay can be enabled/disabled only for buck[2346] */
> +	if ((rdev_get_id(rdev) >= S2MPS11_BUCK2 &&
> +			rdev_get_id(rdev) <= S2MPS11_BUCK4) ||
> +			rdev_get_id(rdev) == S2MPS11_BUCK6)  {
> +		ret = regmap_update_bits(rdev->regmap, S2MPS11_REG_RAMP,
> +					 1 << enable_shift, 1 << enable_shift);
> +		if (ret) {
> +			dev_err(&rdev->dev, "failed to enable ramp rate\n");
> +			return ret;
> +		}
>  	}
>  
>  	ramp_val = get_ramp_delay(ramp_delay);

Re: [PATCH v2 1/3] regulator: s2mps11: Fix accidental enable of buck6 ramp delay

[Axel Lin]

2014-05-07 17:48 GMT+08:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>:
> I forgot to CC the stable.
>
> Mark, if the patches are OK and if you'll apply them, could you add a CC
> stable to the commit msg?

Hi Krzysztof,
Unless this serial misses 3.15, I think this serial does not need to CC stable.

Both
b96244fad953 ("regulator: s2mps11: Don't check enable_shift before
setting enable ramp rate")
f7b1a8dc1c1c ("regulator: s2mpa01: Don't check enable_shift before
setting enable ramp rate")
are exist in 3.15 only.

$ git describe --contains b96244fad953
regulator-v3.15-fixes^3
$ git describe --contains f7b1a8dc1c1c
regulator-v3.15-fixes^3~1

Regards,
Axel