* [PATCH 2/3] regulator: tps65910: fix list_voltage for VDD1 and VDD2
2011-08-15 10:45 [PATCH 1/3] regulator: tps65910: fix device initialisation Johan Hovold
@ 2011-08-15 10:45 ` Johan Hovold
2011-08-15 13:09 ` Mark Brown
2011-08-15 10:45 ` [PATCH 3/3] regulator: tps65910: verify dcdc regulator ids Johan Hovold
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Johan Hovold @ 2011-08-15 10:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown
Cc: Liam Girdwood, Jorge Eduardo Candelaria, Samuel Ortiz,
linux-kernel, Johan Hovold, stable
Fix regression introduced by commit
a320e3c3d6351814afa5182159df88d2637e0f6f (regulator: tps65911: Add new
chip version) which broke list_voltage for VDD1 and VDD2.
Cc: stable@kernel.org
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/regulator/tps65910-regulator.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index bc5ed25..2c61e48 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -759,6 +759,7 @@ static int tps65910_list_voltage_dcdc(struct regulator_dev *dev,
mult = (selector / VDD1_2_NUM_VOLTS) + 1;
volt = VDD1_2_MIN_VOLT +
(selector % VDD1_2_NUM_VOLTS) * VDD1_2_OFFSET;
+ break;
case TPS65911_REG_VDDCTRL:
volt = VDDCTRL_MIN_VOLT + (selector * VDDCTRL_OFFSET);
}
--
1.7.6
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/3] regulator: tps65910: fix list_voltage for VDD1 and VDD2
2011-08-15 10:45 ` [PATCH 2/3] regulator: tps65910: fix list_voltage for VDD1 and VDD2 Johan Hovold
@ 2011-08-15 13:09 ` Mark Brown
2011-08-15 14:29 ` Johan Hovold
0 siblings, 1 reply; 15+ messages in thread
From: Mark Brown @ 2011-08-15 13:09 UTC (permalink / raw)
To: Johan Hovold
Cc: Liam Girdwood, Liam Girdwood, Jorge Eduardo Candelaria,
Samuel Ortiz, linux-kernel, stable
On Mon, Aug 15, 2011 at 12:45:26PM +0200, Johan Hovold wrote:
> Fix regression introduced by commit
> a320e3c3d6351814afa5182159df88d2637e0f6f (regulator: tps65911: Add new
> chip version) which broke list_voltage for VDD1 and VDD2.
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Why is this a separate patch to the previous one?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] regulator: tps65910: fix list_voltage for VDD1 and VDD2
2011-08-15 13:09 ` Mark Brown
@ 2011-08-15 14:29 ` Johan Hovold
2011-08-15 14:49 ` Mark Brown
0 siblings, 1 reply; 15+ messages in thread
From: Johan Hovold @ 2011-08-15 14:29 UTC (permalink / raw)
To: Mark Brown
Cc: Liam Girdwood, Liam Girdwood, Jorge Eduardo Candelaria,
Samuel Ortiz, linux-kernel, stable
On Mon, Aug 15, 2011 at 10:09:13PM +0900, Mark Brown wrote:
> On Mon, Aug 15, 2011 at 12:45:26PM +0200, Johan Hovold wrote:
> > Fix regression introduced by commit
> > a320e3c3d6351814afa5182159df88d2637e0f6f (regulator: tps65911: Add new
> > chip version) which broke list_voltage for VDD1 and VDD2.
>
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> Why is this a separate patch to the previous one?
Because it fixes a different bug (although introduced by the same commit
and the same kind of programming error).
Thanks,
Johan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] regulator: tps65910: fix list_voltage for VDD1 and VDD2
2011-08-15 14:29 ` Johan Hovold
@ 2011-08-15 14:49 ` Mark Brown
2011-08-15 15:12 ` Johan Hovold
0 siblings, 1 reply; 15+ messages in thread
From: Mark Brown @ 2011-08-15 14:49 UTC (permalink / raw)
To: Johan Hovold
Cc: Liam Girdwood, Liam Girdwood, Jorge Eduardo Candelaria,
Samuel Ortiz, linux-kernel, stable
On Mon, Aug 15, 2011 at 04:29:48PM +0200, Johan Hovold wrote:
> On Mon, Aug 15, 2011 at 10:09:13PM +0900, Mark Brown wrote:
> > Why is this a separate patch to the previous one?
> Because it fixes a different bug (although introduced by the same commit
> and the same kind of programming error).
You've got a bunch of missing break statements introduced by the same
commit in the same driver causing the same runtime issue...
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] regulator: tps65910: fix list_voltage for VDD1 and VDD2
2011-08-15 14:49 ` Mark Brown
@ 2011-08-15 15:12 ` Johan Hovold
2011-08-15 15:39 ` Mark Brown
0 siblings, 1 reply; 15+ messages in thread
From: Johan Hovold @ 2011-08-15 15:12 UTC (permalink / raw)
To: Mark Brown
Cc: Liam Girdwood, Liam Girdwood, Jorge Eduardo Candelaria,
Samuel Ortiz, linux-kernel, stable
On Mon, Aug 15, 2011 at 11:49:13PM +0900, Mark Brown wrote:
> On Mon, Aug 15, 2011 at 04:29:48PM +0200, Johan Hovold wrote:
> > On Mon, Aug 15, 2011 at 10:09:13PM +0900, Mark Brown wrote:
>
> > > Why is this a separate patch to the previous one?
>
> > Because it fixes a different bug (although introduced by the same commit
> > and the same kind of programming error).
>
> You've got a bunch of missing break statements introduced by the same
> commit in the same driver causing the same runtime issue...
Well, the first bug prevents any device from being used at all (as probe
always fails). The second bug makes list_voltage return the wrong value
for two regulators.
I simply prefer fixing them separately as it will be more clear from the
changelog what behaviours are being fixed -- especially as they are
marked for stable.
Thanks,
Johan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] regulator: tps65910: fix list_voltage for VDD1 and VDD2
2011-08-15 15:12 ` Johan Hovold
@ 2011-08-15 15:39 ` Mark Brown
0 siblings, 0 replies; 15+ messages in thread
From: Mark Brown @ 2011-08-15 15:39 UTC (permalink / raw)
To: Johan Hovold
Cc: Liam Girdwood, Liam Girdwood, Jorge Eduardo Candelaria,
Samuel Ortiz, linux-kernel, stable
On Mon, Aug 15, 2011 at 05:12:53PM +0200, Johan Hovold wrote:
> On Mon, Aug 15, 2011 at 11:49:13PM +0900, Mark Brown wrote:
> > You've got a bunch of missing break statements introduced by the same
> > commit in the same driver causing the same runtime issue...
> Well, the first bug prevents any device from being used at all (as probe
> always fails). The second bug makes list_voltage return the wrong value
> for two regulators.
> I simply prefer fixing them separately as it will be more clear from the
> changelog what behaviours are being fixed -- especially as they are
> marked for stable.
As a reviewer I did a double take as I wasn't immediately sure if this
was actually a different patch or not.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] regulator: tps65910: verify dcdc regulator ids
2011-08-15 10:45 [PATCH 1/3] regulator: tps65910: fix device initialisation Johan Hovold
2011-08-15 10:45 ` [PATCH 2/3] regulator: tps65910: fix list_voltage for VDD1 and VDD2 Johan Hovold
@ 2011-08-15 10:45 ` Johan Hovold
2011-08-15 13:10 ` Mark Brown
2011-08-30 16:07 ` [PATCH 3/3 v2] " Johan Hovold
2011-08-15 13:08 ` [PATCH 1/3] regulator: tps65910: fix device initialisation Mark Brown
2011-08-28 16:32 ` Liam Girdwood
3 siblings, 2 replies; 15+ messages in thread
From: Johan Hovold @ 2011-08-15 10:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown
Cc: Liam Girdwood, Jorge Eduardo Candelaria, Samuel Ortiz,
linux-kernel, Johan Hovold
Add default case to switches in dcdc voltage functions.
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/regulator/tps65910-regulator.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index 2c61e48..991dfa4 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -534,6 +534,8 @@ static int tps65910_get_voltage_dcdc(struct regulator_dev *dev)
srvsel &= VDDCTRL_SR_SEL_MASK;
vselmax = 64;
break;
+ default:
+ return -EINVAL;
}
/* multiplier 0 == 1 but 2,3 normal */
@@ -688,6 +690,9 @@ static int tps65910_set_voltage_dcdc(struct regulator_dev *dev,
case TPS65911_REG_VDDCTRL:
vsel = selector;
tps65910_reg_write(pmic, TPS65911_VDDCTRL_OP, vsel);
+ break;
+ default:
+ return -EINVAL;
}
return 0;
@@ -762,6 +767,9 @@ static int tps65910_list_voltage_dcdc(struct regulator_dev *dev,
break;
case TPS65911_REG_VDDCTRL:
volt = VDDCTRL_MIN_VOLT + (selector * VDDCTRL_OFFSET);
+ break;
+ default:
+ return -EINVAL;
}
return volt * 100 * mult;
--
1.7.6
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] regulator: tps65910: verify dcdc regulator ids
2011-08-15 10:45 ` [PATCH 3/3] regulator: tps65910: verify dcdc regulator ids Johan Hovold
@ 2011-08-15 13:10 ` Mark Brown
2011-08-30 16:07 ` [PATCH 3/3 v2] " Johan Hovold
1 sibling, 0 replies; 15+ messages in thread
From: Mark Brown @ 2011-08-15 13:10 UTC (permalink / raw)
To: Johan Hovold
Cc: Liam Girdwood, Liam Girdwood, Jorge Eduardo Candelaria,
Samuel Ortiz, linux-kernel
On Mon, Aug 15, 2011 at 12:45:27PM +0200, Johan Hovold wrote:
> Add default case to switches in dcdc voltage functions.
>
> Signed-off-by: Johan Hovold <jhovold@gmail.com>
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3 v2] regulator: tps65910: verify dcdc regulator ids
2011-08-15 10:45 ` [PATCH 3/3] regulator: tps65910: verify dcdc regulator ids Johan Hovold
2011-08-15 13:10 ` Mark Brown
@ 2011-08-30 16:07 ` Johan Hovold
2011-08-30 16:07 ` Mark Brown
2011-08-31 13:26 ` Liam Girdwood
1 sibling, 2 replies; 15+ messages in thread
From: Johan Hovold @ 2011-08-30 16:07 UTC (permalink / raw)
To: Liam Girdwood
Cc: Mark Brown, Jorge Eduardo Candelaria, Samuel Ortiz, linux-kernel,
Johan Hovold
Add default case to switches in dcdc voltage functions.
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/regulator/tps65910-regulator.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index 66d2d60..633ecd4 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -534,6 +534,8 @@ static int tps65910_get_voltage_dcdc(struct regulator_dev *dev)
srvsel &= VDDCTRL_SR_SEL_MASK;
vselmax = 64;
break;
+ default:
+ return -EINVAL;
}
/* multiplier 0 == 1 but 2,3 normal */
@@ -688,6 +690,9 @@ static int tps65910_set_voltage_dcdc(struct regulator_dev *dev,
case TPS65911_REG_VDDCTRL:
vsel = selector;
tps65910_reg_write(pmic, TPS65911_VDDCTRL_OP, vsel);
+ break;
+ default:
+ return -EINVAL;
}
return 0;
--
1.7.6
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3 v2] regulator: tps65910: verify dcdc regulator ids
2011-08-30 16:07 ` [PATCH 3/3 v2] " Johan Hovold
@ 2011-08-30 16:07 ` Mark Brown
2011-08-31 13:26 ` Liam Girdwood
1 sibling, 0 replies; 15+ messages in thread
From: Mark Brown @ 2011-08-30 16:07 UTC (permalink / raw)
To: Johan Hovold
Cc: Liam Girdwood, Jorge Eduardo Candelaria, Samuel Ortiz, linux-kernel
On Tue, Aug 30, 2011 at 06:07:06PM +0200, Johan Hovold wrote:
> Add default case to switches in dcdc voltage functions.
>
> Signed-off-by: Johan Hovold <jhovold@gmail.com>
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3 v2] regulator: tps65910: verify dcdc regulator ids
2011-08-30 16:07 ` [PATCH 3/3 v2] " Johan Hovold
2011-08-30 16:07 ` Mark Brown
@ 2011-08-31 13:26 ` Liam Girdwood
1 sibling, 0 replies; 15+ messages in thread
From: Liam Girdwood @ 2011-08-31 13:26 UTC (permalink / raw)
To: Johan Hovold
Cc: Mark Brown, Jorge Eduardo Candelaria, Samuel Ortiz, linux-kernel
On 30/08/11 17:07, Johan Hovold wrote:
> Add default case to switches in dcdc voltage functions.
>
> Signed-off-by: Johan Hovold <jhovold@gmail.com>
> ---
> drivers/regulator/tps65910-regulator.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
> index 66d2d60..633ecd4 100644
> --- a/drivers/regulator/tps65910-regulator.c
> +++ b/drivers/regulator/tps65910-regulator.c
> @@ -534,6 +534,8 @@ static int tps65910_get_voltage_dcdc(struct regulator_dev *dev)
> srvsel &= VDDCTRL_SR_SEL_MASK;
> vselmax = 64;
> break;
> + default:
> + return -EINVAL;
> }
>
> /* multiplier 0 == 1 but 2,3 normal */
> @@ -688,6 +690,9 @@ static int tps65910_set_voltage_dcdc(struct regulator_dev *dev,
> case TPS65911_REG_VDDCTRL:
> vsel = selector;
> tps65910_reg_write(pmic, TPS65911_VDDCTRL_OP, vsel);
> + break;
> + default:
> + return -EINVAL;
> }
>
> return 0;
Applied.
Thanks
Liam
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] regulator: tps65910: fix device initialisation
2011-08-15 10:45 [PATCH 1/3] regulator: tps65910: fix device initialisation Johan Hovold
2011-08-15 10:45 ` [PATCH 2/3] regulator: tps65910: fix list_voltage for VDD1 and VDD2 Johan Hovold
2011-08-15 10:45 ` [PATCH 3/3] regulator: tps65910: verify dcdc regulator ids Johan Hovold
@ 2011-08-15 13:08 ` Mark Brown
2011-08-28 16:32 ` Liam Girdwood
3 siblings, 0 replies; 15+ messages in thread
From: Mark Brown @ 2011-08-15 13:08 UTC (permalink / raw)
To: Johan Hovold
Cc: Liam Girdwood, Liam Girdwood, Jorge Eduardo Candelaria,
Samuel Ortiz, linux-kernel, stable
On Mon, Aug 15, 2011 at 12:45:25PM +0200, Johan Hovold wrote:
> Fix regression introduced by commit
> a320e3c3d6351814afa5182159df88d2637e0f6f (regulator: tps65911: Add new
> chip version) which broke probe for all devices:
>
> "Invalid tps chip version"
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] regulator: tps65910: fix device initialisation
2011-08-15 10:45 [PATCH 1/3] regulator: tps65910: fix device initialisation Johan Hovold
` (2 preceding siblings ...)
2011-08-15 13:08 ` [PATCH 1/3] regulator: tps65910: fix device initialisation Mark Brown
@ 2011-08-28 16:32 ` Liam Girdwood
2011-08-30 16:05 ` Johan Hovold
3 siblings, 1 reply; 15+ messages in thread
From: Liam Girdwood @ 2011-08-28 16:32 UTC (permalink / raw)
To: Johan Hovold
Cc: Liam Girdwood, Mark Brown, Jorge Eduardo Candelaria,
Samuel Ortiz, linux-kernel, stable
Hi Johan,
On Mon, 2011-08-15 at 12:45 +0200, Johan Hovold wrote:
> Fix regression introduced by commit
> a320e3c3d6351814afa5182159df88d2637e0f6f (regulator: tps65911: Add new
> chip version) which broke probe for all devices:
>
> "Invalid tps chip version"
>
> Cc: stable@kernel.org
> Signed-off-by: Johan Hovold <jhovold@gmail.com>
> ---
> drivers/regulator/tps65910-regulator.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
> index 55dd4e6..bc5ed25 100644
> --- a/drivers/regulator/tps65910-regulator.c
> +++ b/drivers/regulator/tps65910-regulator.c
> @@ -898,9 +898,11 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
> case TPS65910:
> pmic->get_ctrl_reg = &tps65910_get_ctrl_register;
> info = tps65910_regs;
> + break;
> case TPS65911:
> pmic->get_ctrl_reg = &tps65911_get_ctrl_register;
> info = tps65911_regs;
> + break;
> default:
> pr_err("Invalid tps chip version\n");
> return -ENODEV;
Couldn't apply these as it seems there may already be fixes to some of
the issues fixed by your patches. Could you update and resend.
Thanks !
Liam
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/3] regulator: tps65910: fix device initialisation
2011-08-28 16:32 ` Liam Girdwood
@ 2011-08-30 16:05 ` Johan Hovold
0 siblings, 0 replies; 15+ messages in thread
From: Johan Hovold @ 2011-08-30 16:05 UTC (permalink / raw)
To: Liam Girdwood
Cc: Liam Girdwood, Mark Brown, Jorge Eduardo Candelaria,
Samuel Ortiz, linux-kernel, stable, Greg Kroah-Hartman, Axel Lin
On Sun, Aug 28, 2011 at 05:32:57PM +0100, Liam Girdwood wrote:
> Hi Johan,
>
> On Mon, 2011-08-15 at 12:45 +0200, Johan Hovold wrote:
> > Fix regression introduced by commit
> > a320e3c3d6351814afa5182159df88d2637e0f6f (regulator: tps65911: Add new
> > chip version) which broke probe for all devices:
> >
> > "Invalid tps chip version"
> >
> > Cc: stable@kernel.org
> > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> > ---
> > drivers/regulator/tps65910-regulator.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
> > index 55dd4e6..bc5ed25 100644
> > --- a/drivers/regulator/tps65910-regulator.c
> > +++ b/drivers/regulator/tps65910-regulator.c
> > @@ -898,9 +898,11 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
> > case TPS65910:
> > pmic->get_ctrl_reg = &tps65910_get_ctrl_register;
> > info = tps65910_regs;
> > + break;
> > case TPS65911:
> > pmic->get_ctrl_reg = &tps65911_get_ctrl_register;
> > info = tps65911_regs;
> > + break;
> > default:
> > pr_err("Invalid tps chip version\n");
> > return -ENODEV;
>
> Couldn't apply these as it seems there may already be fixes to some of
> the issues fixed by your patches. Could you update and resend.
It seems patches 1 and 2 are subsumed by Axel Lin's
d04156bca629740a661fd0738cd69ba1f08b2b20 (regulator: tps65910: Add
missing breaks in switch/case), but this one was not marked for stable
even though it fixes a regression for tps65910 devices.
I'll send an updated patch 3 in a minute.
Thanks,
Johan
^ permalink raw reply [flat|nested] 15+ messages in thread