* [PATCH 1/2] regulator: s2mps11: Fix accidental enable of buck6 ramp delay
@ 2014-05-05 15:10 Krzysztof Kozlowski
2014-05-05 15:10 ` [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4 " Krzysztof Kozlowski
2014-05-17 13:26 ` [PATCH 1/2] regulator: s2mps11: Fix accidental enable of buck6 " Mark Brown
0 siblings, 2 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2014-05-05 15:10 UTC (permalink / raw)
To: Sangbeom Kim, Liam Girdwood, Mark Brown, linux-kernel, Sachin Kamat
Cc: Axel Lin, 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 these other bucks (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")
---
drivers/regulator/s2mps11.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index e713c162fbd4..db677a4be4d3 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -110,7 +110,8 @@ static int s2mps11_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
{
struct s2mps11_info *s2mps11 = rdev_get_drvdata(rdev);
unsigned int ramp_val, ramp_shift, ramp_reg = S2MPS11_REG_RAMP_BUCK;
- unsigned int ramp_enable = 1, enable_shift = 0;
+ unsigned int ramp_enable = 1;
+ int enable_shift = -1;
int ret;
switch (rdev_get_id(rdev)) {
@@ -202,11 +203,13 @@ 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;
+ if (enable_shift >= 0) {
+ 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4 ramp delay
2014-05-05 15:10 [PATCH 1/2] regulator: s2mps11: Fix accidental enable of buck6 ramp delay Krzysztof Kozlowski
@ 2014-05-05 15:10 ` Krzysztof Kozlowski
2014-05-05 15:27 ` Axel Lin
2014-05-17 13:27 ` Mark Brown
2014-05-17 13:26 ` [PATCH 1/2] regulator: s2mps11: Fix accidental enable of buck6 " Mark Brown
1 sibling, 2 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2014-05-05 15:10 UTC (permalink / raw)
To: Sangbeom Kim, Liam Girdwood, Mark Brown, linux-kernel, Sachin Kamat
Cc: Axel Lin, 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 these other bucks (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")
---
drivers/regulator/s2mpa01.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
index f19a30f0fb42..901504880ab6 100644
--- a/drivers/regulator/s2mpa01.c
+++ b/drivers/regulator/s2mpa01.c
@@ -99,7 +99,8 @@ static int s2mpa01_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
{
struct s2mpa01_info *s2mpa01 = rdev_get_drvdata(rdev);
unsigned int ramp_val, ramp_shift, ramp_reg = S2MPA01_REG_RAMP2;
- unsigned int ramp_enable = 1, enable_shift = 0;
+ unsigned int ramp_enable = 1;
+ int enable_shift = -1;
int ret;
switch (rdev->desc->id) {
@@ -192,11 +193,13 @@ 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;
+ if (enable_shift >= 0) {
+ 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4 ramp delay
2014-05-05 15:10 ` [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4 " Krzysztof Kozlowski
@ 2014-05-05 15:27 ` Axel Lin
2014-05-06 6:25 ` Krzysztof Kozlowski
2014-05-17 13:27 ` Mark Brown
1 sibling, 1 reply; 6+ messages in thread
From: Axel Lin @ 2014-05-05 15:27 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sangbeom Kim, Liam Girdwood, Mark Brown, linux-kernel, Sachin Kamat
2014-05-05 23:10 GMT+08:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>:
> 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 these other bucks (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")
> ---
> drivers/regulator/s2mpa01.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
> index f19a30f0fb42..901504880ab6 100644
> --- a/drivers/regulator/s2mpa01.c
> +++ b/drivers/regulator/s2mpa01.c
> @@ -99,7 +99,8 @@ static int s2mpa01_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
> {
> struct s2mpa01_info *s2mpa01 = rdev_get_drvdata(rdev);
> unsigned int ramp_val, ramp_shift, ramp_reg = S2MPA01_REG_RAMP2;
> - unsigned int ramp_enable = 1, enable_shift = 0;
> + unsigned int ramp_enable = 1;
> + int enable_shift = -1;
Hi Krzysztof,
This makes the meaning of enable_shift looks confusing.
I think below change has better readability.
diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
index f19a30f..eb5ce18 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;
+ /* S2MPA01 supports enabling/disabling ramp delay only for buck[1234] */
+ if (rdev->desc->id >= S2MPA01_BUCK1 &&
+ rdev->desc->id <= 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);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4 ramp delay
2014-05-05 15:27 ` Axel Lin
@ 2014-05-06 6:25 ` Krzysztof Kozlowski
0 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2014-05-06 6:25 UTC (permalink / raw)
To: Axel Lin
Cc: Sangbeom Kim, Liam Girdwood, Mark Brown, linux-kernel, Sachin Kamat
On pon, 2014-05-05 at 23:27 +0800, Axel Lin wrote:
> 2014-05-05 23:10 GMT+08:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>:
> > 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 these other bucks (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")
> > ---
> > drivers/regulator/s2mpa01.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
> > index f19a30f0fb42..901504880ab6 100644
> > --- a/drivers/regulator/s2mpa01.c
> > +++ b/drivers/regulator/s2mpa01.c
> > @@ -99,7 +99,8 @@ static int s2mpa01_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
> > {
> > struct s2mpa01_info *s2mpa01 = rdev_get_drvdata(rdev);
> > unsigned int ramp_val, ramp_shift, ramp_reg = S2MPA01_REG_RAMP2;
> > - unsigned int ramp_enable = 1, enable_shift = 0;
> > + unsigned int ramp_enable = 1;
> > + int enable_shift = -1;
> Hi Krzysztof,
> This makes the meaning of enable_shift looks confusing.
> I think below change has better readability.
>
> diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
> index f19a30f..eb5ce18 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;
> + /* S2MPA01 supports enabling/disabling ramp delay only for buck[1234] */
> + if (rdev->desc->id >= S2MPA01_BUCK1 &&
> + rdev->desc->id <= 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);
Yes, it looks better for S2MPA01. Thanks for idea, I'll send v2.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] regulator: s2mps11: Fix accidental enable of buck6 ramp delay
2014-05-05 15:10 [PATCH 1/2] regulator: s2mps11: Fix accidental enable of buck6 ramp delay Krzysztof Kozlowski
2014-05-05 15:10 ` [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4 " Krzysztof Kozlowski
@ 2014-05-17 13:26 ` Mark Brown
1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-05-17 13:26 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sangbeom Kim, Liam Girdwood, linux-kernel, Sachin Kamat, Axel Lin
[-- Attachment #1: Type: text/plain, Size: 203 bytes --]
On Mon, May 05, 2014 at 05:10:02PM +0200, Krzysztof Kozlowski wrote:
> S2MPS11 supports enabling/disabling ramp delay only for buck[2346].
> Other bucks have ramp delay enabled always.
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4 ramp delay
2014-05-05 15:10 ` [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4 " Krzysztof Kozlowski
2014-05-05 15:27 ` Axel Lin
@ 2014-05-17 13:27 ` Mark Brown
1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-05-17 13:27 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sangbeom Kim, Liam Girdwood, linux-kernel, Sachin Kamat, Axel Lin
[-- Attachment #1: Type: text/plain, Size: 203 bytes --]
On Mon, May 05, 2014 at 05:10:03PM +0200, Krzysztof Kozlowski wrote:
> S2MPA01 supports enabling/disabling ramp delay only for buck[1234].
> Other bucks have ramp delay enabled always.
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-05-17 13:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-05 15:10 [PATCH 1/2] regulator: s2mps11: Fix accidental enable of buck6 ramp delay Krzysztof Kozlowski
2014-05-05 15:10 ` [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4 " Krzysztof Kozlowski
2014-05-05 15:27 ` Axel Lin
2014-05-06 6:25 ` Krzysztof Kozlowski
2014-05-17 13:27 ` Mark Brown
2014-05-17 13:26 ` [PATCH 1/2] regulator: s2mps11: Fix accidental enable of buck6 " Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).