Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Mon, Dec 17, 2012 at 05:18:48PM +0200, Felipe Balbi wrote: > On Mon, Dec 17, 2012 at 02:15:36PM +, Mark Brown wrote: > > Uh, no. Think about this for a minute - we want bug fixes backporting, > > we don't want to be putting process blockers in the way of that > > especially not in the cases where fixes apply to all the stable kernels > > that are currently active. > then omit the # v3.x, v3.y part, but Cc: sta...@vger.kernel.org should > still be there Oh, yes - that's needed. Sorry, I thought it was the v3.x bit you were complaining was missing. signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Hi, On Mon, Dec 17, 2012 at 02:15:36PM +, Mark Brown wrote: > On Sat, Dec 15, 2012 at 04:57:38PM +0200, Felipe Balbi wrote: > > On Sat, Dec 15, 2012 at 11:15:20PM +0900, Mark Brown wrote: > > > > It's perfectly OK to omit this unless there's an awareness that the > > > backport won't work on some kernels. > > > that's not what Greg says, but fair enough. Won't discuss it... > > Uh, no. Think about this for a minute - we want bug fixes backporting, > we don't want to be putting process blockers in the way of that > especially not in the cases where fixes apply to all the stable kernels > that are currently active. then omit the # v3.x, v3.y part, but Cc: sta...@vger.kernel.org should still be there -- balbi signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Sat, Dec 15, 2012 at 04:57:38PM +0200, Felipe Balbi wrote: > On Sat, Dec 15, 2012 at 11:15:20PM +0900, Mark Brown wrote: > > It's perfectly OK to omit this unless there's an awareness that the > > backport won't work on some kernels. > that's not what Greg says, but fair enough. Won't discuss it... Uh, no. Think about this for a minute - we want bug fixes backporting, we don't want to be putting process blockers in the way of that especially not in the cases where fixes apply to all the stable kernels that are currently active. signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Sat, Dec 15, 2012 at 04:57:38PM +0200, Felipe Balbi wrote: On Sat, Dec 15, 2012 at 11:15:20PM +0900, Mark Brown wrote: It's perfectly OK to omit this unless there's an awareness that the backport won't work on some kernels. that's not what Greg says, but fair enough. Won't discuss it... Uh, no. Think about this for a minute - we want bug fixes backporting, we don't want to be putting process blockers in the way of that especially not in the cases where fixes apply to all the stable kernels that are currently active. signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Hi, On Mon, Dec 17, 2012 at 02:15:36PM +, Mark Brown wrote: On Sat, Dec 15, 2012 at 04:57:38PM +0200, Felipe Balbi wrote: On Sat, Dec 15, 2012 at 11:15:20PM +0900, Mark Brown wrote: It's perfectly OK to omit this unless there's an awareness that the backport won't work on some kernels. that's not what Greg says, but fair enough. Won't discuss it... Uh, no. Think about this for a minute - we want bug fixes backporting, we don't want to be putting process blockers in the way of that especially not in the cases where fixes apply to all the stable kernels that are currently active. then omit the # v3.x, v3.y part, but Cc: sta...@vger.kernel.org should still be there -- balbi signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Mon, Dec 17, 2012 at 05:18:48PM +0200, Felipe Balbi wrote: On Mon, Dec 17, 2012 at 02:15:36PM +, Mark Brown wrote: Uh, no. Think about this for a minute - we want bug fixes backporting, we don't want to be putting process blockers in the way of that especially not in the cases where fixes apply to all the stable kernels that are currently active. then omit the # v3.x, v3.y part, but Cc: sta...@vger.kernel.org should still be there Oh, yes - that's needed. Sorry, I thought it was the v3.x bit you were complaining was missing. signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Hi, On Sat, Dec 15, 2012 at 11:15:20PM +0900, Mark Brown wrote: > On Fri, Dec 14, 2012 at 05:40:30PM +0200, Felipe Balbi wrote: > > > No no, I mean that before you SoB you should have a: > > > Cc: # v3.x v3.y ... > > It's perfectly OK to omit this unless there's an awareness that the > backport won't work on some kernels. that's not what Greg says, but fair enough. Won't discuss it... -- balbi signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Thu, Dec 13, 2012 at 10:13:00AM +0100, Paolo Pisati wrote: > Signed-off-by: Paolo Pisati > Tested-by: Robert Nelson Applied, thanks. signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Fri, Dec 14, 2012 at 05:40:30PM +0200, Felipe Balbi wrote: > No no, I mean that before you SoB you should have a: > Cc: # v3.x v3.y ... It's perfectly OK to omit this unless there's an awareness that the backport won't work on some kernels. signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Fri, Dec 14, 2012 at 05:40:30PM +0200, Felipe Balbi wrote: No no, I mean that before you SoB you should have a: Cc: sta...@vger.kernel.org # v3.x v3.y ... It's perfectly OK to omit this unless there's an awareness that the backport won't work on some kernels. signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Thu, Dec 13, 2012 at 10:13:00AM +0100, Paolo Pisati wrote: Signed-off-by: Paolo Pisati paolo.pis...@canonical.com Tested-by: Robert Nelson robertcnel...@gmail.com Applied, thanks. signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Hi, On Sat, Dec 15, 2012 at 11:15:20PM +0900, Mark Brown wrote: On Fri, Dec 14, 2012 at 05:40:30PM +0200, Felipe Balbi wrote: No no, I mean that before you SoB you should have a: Cc: sta...@vger.kernel.org # v3.x v3.y ... It's perfectly OK to omit this unless there's an awareness that the backport won't work on some kernels. that's not what Greg says, but fair enough. Won't discuss it... -- balbi signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Hi, On Fri, Dec 14, 2012 at 04:39:43PM +0100, Paolo Pisati wrote: > On Thu, Dec 13, 2012 at 11:47:15AM +0200, Felipe Balbi wrote: > > On Thu, Dec 13, 2012 at 10:13:00AM +0100, Paolo Pisati wrote: > > > Signed-off-by: Paolo Pisati > > > Tested-by: Robert Nelson > > > > please read Documentation/stable_kernel_rules.txt, you'll see this is > > wrong. > > you mean that i should have waited for the commit to hit Linus's tree before > cc-ing stable? No no, I mean that before you SoB you should have a: Cc: # v3.x v3.y ... Something like the commit below: commit 041d81f493d90c940ec41f0ec98bc7c4f2fba431 Author: Felipe Balbi Date: Thu Oct 4 11:58:00 2012 +0300 usb: dwc3: gadget: fix 'endpoint always busy' bug If a USB transfer has already been started, meaning we have already issued StartTransfer command to that particular endpoint, DWC3_EP_BUSY flag has also already been set. When we try to cancel this transfer which is already in controller's cache, we will not receive XferComplete event and we must clear DWC3_EP_BUSY in order to allow subsequent requests to be properly started. The best place to clear that flag is right after issuing DWC3_DEPCMD_ENDTRANSFER. Cc: sta...@vger.kernel.org # v3.4 v3.5 v3.6 Reported-by: Moiz Sonasath Signed-off-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Thu, Dec 13, 2012 at 11:47:15AM +0200, Felipe Balbi wrote: > On Thu, Dec 13, 2012 at 10:13:00AM +0100, Paolo Pisati wrote: > > Signed-off-by: Paolo Pisati > > Tested-by: Robert Nelson > > please read Documentation/stable_kernel_rules.txt, you'll see this is > wrong. you mean that i should have waited for the commit to hit Linus's tree before cc-ing stable? -- bye, p. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Thu, Dec 13, 2012 at 11:47:15AM +0200, Felipe Balbi wrote: On Thu, Dec 13, 2012 at 10:13:00AM +0100, Paolo Pisati wrote: Signed-off-by: Paolo Pisati paolo.pis...@canonical.com Tested-by: Robert Nelson robertcnel...@gmail.com please read Documentation/stable_kernel_rules.txt, you'll see this is wrong. you mean that i should have waited for the commit to hit Linus's tree before cc-ing stable? -- bye, p. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Hi, On Fri, Dec 14, 2012 at 04:39:43PM +0100, Paolo Pisati wrote: On Thu, Dec 13, 2012 at 11:47:15AM +0200, Felipe Balbi wrote: On Thu, Dec 13, 2012 at 10:13:00AM +0100, Paolo Pisati wrote: Signed-off-by: Paolo Pisati paolo.pis...@canonical.com Tested-by: Robert Nelson robertcnel...@gmail.com please read Documentation/stable_kernel_rules.txt, you'll see this is wrong. you mean that i should have waited for the commit to hit Linus's tree before cc-ing stable? No no, I mean that before you SoB you should have a: Cc: sta...@vger.kernel.org # v3.x v3.y ... Something like the commit below: commit 041d81f493d90c940ec41f0ec98bc7c4f2fba431 Author: Felipe Balbi ba...@ti.com Date: Thu Oct 4 11:58:00 2012 +0300 usb: dwc3: gadget: fix 'endpoint always busy' bug If a USB transfer has already been started, meaning we have already issued StartTransfer command to that particular endpoint, DWC3_EP_BUSY flag has also already been set. When we try to cancel this transfer which is already in controller's cache, we will not receive XferComplete event and we must clear DWC3_EP_BUSY in order to allow subsequent requests to be properly started. The best place to clear that flag is right after issuing DWC3_DEPCMD_ENDTRANSFER. Cc: sta...@vger.kernel.org # v3.4 v3.5 v3.6 Reported-by: Moiz Sonasath m-sonas...@ti.com Signed-off-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Thu, Dec 13, 2012 at 10:13:00AM +0100, Paolo Pisati wrote: > Signed-off-by: Paolo Pisati > Tested-by: Robert Nelson please read Documentation/stable_kernel_rules.txt, you'll see this is wrong. -- balbi signature.asc Description: Digital signature
[PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Signed-off-by: Paolo Pisati
Tested-by: Robert Nelson
---
drivers/regulator/core.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e872c8b..c347fd0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2250,6 +2250,7 @@ int regulator_set_voltage(struct regulator *regulator,
int min_uV, int max_uV)
{
struct regulator_dev *rdev = regulator->rdev;
int ret = 0;
+ int old_min_uV, old_max_uV;
mutex_lock(>mutex);
@@ -2271,18 +2272,29 @@ int regulator_set_voltage(struct regulator *regulator,
int min_uV, int max_uV)
ret = regulator_check_voltage(rdev, _uV, _uV);
if (ret < 0)
goto out;
+
+ /* restore original values in case of error */
+ old_min_uV = regulator->min_uV;
+ old_max_uV = regulator->max_uV;
regulator->min_uV = min_uV;
regulator->max_uV = max_uV;
ret = regulator_check_consumers(rdev, _uV, _uV);
if (ret < 0)
- goto out;
+ goto out2;
ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
-
+ if (ret < 0)
+ goto out2;
+
out:
mutex_unlock(>mutex);
return ret;
+out2:
+ regulator->min_uV = old_min_uV;
+ regulator->max_uV = old_max_uV;
+ mutex_unlock(>mutex);
+ return ret;
}
EXPORT_SYMBOL_GPL(regulator_set_voltage);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regulator: core: if voltage scaling fails, restore original
On Thu, Dec 13, 2012 at 05:15:33AM +, Mark Brown wrote: > On Wed, Dec 12, 2012 at 12:45:52PM +0100, Paolo Pisati wrote: > > And after a second look it's clear what's going on: > > After a second look at what? You've not provided any context, I've no > idea what you're talking about here. forgot the --in-reply-to=..., i'll resend as a new thread. -- bye, p. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regulator: core: if voltage scaling fails, restore original
On Thu, Dec 13, 2012 at 05:15:33AM +, Mark Brown wrote: On Wed, Dec 12, 2012 at 12:45:52PM +0100, Paolo Pisati wrote: And after a second look it's clear what's going on: After a second look at what? You've not provided any context, I've no idea what you're talking about here. forgot the --in-reply-to=..., i'll resend as a new thread. -- bye, p. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Signed-off-by: Paolo Pisati paolo.pis...@canonical.com
Tested-by: Robert Nelson robertcnel...@gmail.com
---
drivers/regulator/core.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e872c8b..c347fd0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2250,6 +2250,7 @@ int regulator_set_voltage(struct regulator *regulator,
int min_uV, int max_uV)
{
struct regulator_dev *rdev = regulator-rdev;
int ret = 0;
+ int old_min_uV, old_max_uV;
mutex_lock(rdev-mutex);
@@ -2271,18 +2272,29 @@ 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;
+
+ /* restore original values in case of error */
+ old_min_uV = regulator-min_uV;
+ old_max_uV = 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 out2;
ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
-
+ if (ret 0)
+ goto out2;
+
out:
mutex_unlock(rdev-mutex);
return ret;
+out2:
+ regulator-min_uV = old_min_uV;
+ regulator-max_uV = old_max_uV;
+ mutex_unlock(rdev-mutex);
+ return ret;
}
EXPORT_SYMBOL_GPL(regulator_set_voltage);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Thu, Dec 13, 2012 at 10:13:00AM +0100, Paolo Pisati wrote: Signed-off-by: Paolo Pisati paolo.pis...@canonical.com Tested-by: Robert Nelson robertcnel...@gmail.com please read Documentation/stable_kernel_rules.txt, you'll see this is wrong. -- balbi signature.asc Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original
On Wed, Dec 12, 2012 at 12:45:52PM +0100, Paolo Pisati wrote: > And after a second look it's clear what's going on: After a second look at what? You've not provided any context, I've no idea what you're talking about here. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regulator: core: if voltage scaling fails, restore original
On Thu, 13 Dec 2012, Paul Walmsley wrote: > Seems to me, naïvely, that in the above code, regulator->min_uV and > regulator->max_uV should be set only after _regulator_do_set_voltage() > succeeds? Eh, never mind. Looks like you took a similar strategy in the subsequent patch you sent.. - Paul
Re: [PATCH] regulator: core: if voltage scaling fails, restore original
On Wed, 12 Dec 2012, Paolo Pisati wrote: > but inside regulator_set_voltage(), we save the new regulator voltage before > actually ramping up: > > core.c::regulator_set_voltage(): > ... > regulator->min_uV = min_uV; > regulator->max_uV = max_uV; > > ret = regulator_check_consumers(rdev, _uV, _uV); > if (ret < 0) > goto out2; > > ret = _regulator_do_set_voltage(rdev, min_uV, max_uV); <-- ERROR!!! > if (ret < 0) > goto out2; > ... I'm not too familiar with this code. But isn't this where the bug is, rather than in that optimization commit you mentioned? Seems to me, naïvely, that in the above code, regulator->min_uV and regulator->max_uV should be set only after _regulator_do_set_voltage() succeeds? - Paul
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Hi,
On Wed, Dec 12, 2012 at 12:13:35PM -0600, Robert Nelson wrote:
> On Wed, Dec 12, 2012 at 5:45 AM, Paolo Pisati
> wrote:
> > Signed-off-by: Paolo Pisati
> > ---
> > drivers/regulator/core.c | 16 ++--
> > 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> > index e872c8b..c347fd0 100644
> > --- a/drivers/regulator/core.c
> > +++ b/drivers/regulator/core.c
> > @@ -2250,6 +2250,7 @@ int regulator_set_voltage(struct regulator
> > *regulator, int min_uV, int max_uV)
> > {
> > struct regulator_dev *rdev = regulator->rdev;
> > int ret = 0;
> > + int old_min_uV, old_max_uV;
> >
> > mutex_lock(>mutex);
> >
> > @@ -2271,18 +2272,29 @@ int regulator_set_voltage(struct regulator
> > *regulator, int min_uV, int max_uV)
> > ret = regulator_check_voltage(rdev, _uV, _uV);
> > if (ret < 0)
> > goto out;
> > +
> > + /* restore original values in case of error */
> > + old_min_uV = regulator->min_uV;
> > + old_max_uV = regulator->max_uV;
> > regulator->min_uV = min_uV;
> > regulator->max_uV = max_uV;
> >
> > ret = regulator_check_consumers(rdev, _uV, _uV);
> > if (ret < 0)
> > - goto out;
> > + goto out2;
> >
> > ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
> > -
> > + if (ret < 0)
> > + goto out2;
> > +
> > out:
> > mutex_unlock(>mutex);
> > return ret;
> > +out2:
> > + regulator->min_uV = old_min_uV;
> > + regulator->max_uV = old_max_uV;
> > + mutex_unlock(>mutex);
> > + return ret;
> > }
> > EXPORT_SYMBOL_GPL(regulator_set_voltage);
> >
> > --
> > 1.7.10.4
>
> Nice! This fixes the 800Mhz lockup on bootup after a software reset we
> were seeing on the Beagle xM's with v3.7.x/v3.6.x...
>
> Tested-by: Robert Nelson
if this fixes a boot lockup with older kernel, I believe this deserves a
Cc: sta...@vger.kernel.org.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Wed, Dec 12, 2012 at 5:45 AM, Paolo Pisati
wrote:
> Signed-off-by: Paolo Pisati
> ---
> drivers/regulator/core.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index e872c8b..c347fd0 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -2250,6 +2250,7 @@ int regulator_set_voltage(struct regulator *regulator,
> int min_uV, int max_uV)
> {
> struct regulator_dev *rdev = regulator->rdev;
> int ret = 0;
> + int old_min_uV, old_max_uV;
>
> mutex_lock(>mutex);
>
> @@ -2271,18 +2272,29 @@ int regulator_set_voltage(struct regulator
> *regulator, int min_uV, int max_uV)
> ret = regulator_check_voltage(rdev, _uV, _uV);
> if (ret < 0)
> goto out;
> +
> + /* restore original values in case of error */
> + old_min_uV = regulator->min_uV;
> + old_max_uV = regulator->max_uV;
> regulator->min_uV = min_uV;
> regulator->max_uV = max_uV;
>
> ret = regulator_check_consumers(rdev, _uV, _uV);
> if (ret < 0)
> - goto out;
> + goto out2;
>
> ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
> -
> + if (ret < 0)
> + goto out2;
> +
> out:
> mutex_unlock(>mutex);
> return ret;
> +out2:
> + regulator->min_uV = old_min_uV;
> + regulator->max_uV = old_max_uV;
> + mutex_unlock(>mutex);
> + return ret;
> }
> EXPORT_SYMBOL_GPL(regulator_set_voltage);
>
> --
> 1.7.10.4
Nice! This fixes the 800Mhz lockup on bootup after a software reset we
were seeing on the Beagle xM's with v3.7.x/v3.6.x...
Tested-by: Robert Nelson
Regards,
--
Robert Nelson
http://www.rcn-ee.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regulator: core: if voltage scaling fails, restore original
And after a second look it's clear what's going on:
[...]
[5.575744] cpu cpu0: cpufreq-omap: 300 MHz, -1 mV --> 800 MHz, 1325 mV
[5.582946] voltdm_scale: No voltage scale API registered for vdd_mpu_iva
[5.590332] cpu cpu0: omap_target: unable to scale voltage up.
[1]
[5.596649] notification 1 of frequency transition to 30 kHz
[5.603179] FREQ: 30 - CPU: 0
[5.606597] governor: change or update limits
[5.611572] __cpufreq_governor for CPU 0, event 3
[5.616699] setting to 80 kHz because of event 3
[5.622039] target for CPU 0: 80 kHz, relation 1
[5.627441] request for target 80 kHz (relation: 1) for cpu 0
[5.634063] target is 2 (80 kHz, 2)
[5.638183] notification 0 of frequency transition to 80 kHz
[5.644775] cpu cpu0: cpufreq-omap: 300 MHz, -1 mV --> 800 MHz, 1325 mV
[2]
[hangs here]
first time[1] it tries to ramp up frequency (and thus voltage),
regulator_set_voltage() returns an error and we exit:
omap-cpufreq.c::omap_target():
...
dev_dbg(mpu_dev, "cpufreq-omap: %u MHz, %ld mV --> %u MHz, %ld mV\n",
freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
freqs.new / 1000, volt ? volt / 1000 : -1);
/* scaling up? scale voltage before frequency */
if (mpu_reg && (freqs.new > freqs.old)) {
r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
if (r < 0) {
dev_warn(mpu_dev, "%s: unable to scale voltage up.\n",
__func__);
freqs.new = freqs.old;
goto done;
}
}
ret = clk_set_rate(mpu_clk, freqs.new * 1000);
...
but inside regulator_set_voltage(), we save the new regulator voltage before
actually ramping up:
core.c::regulator_set_voltage():
...
regulator->min_uV = min_uV;
regulator->max_uV = max_uV;
ret = regulator_check_consumers(rdev, _uV, _uV);
if (ret < 0)
goto out2;
ret = _regulator_do_set_voltage(rdev, min_uV, max_uV); <-- ERROR!!!
if (ret < 0)
goto out2;
...
and by the time we try to change frequency again[2], regulator_set_voltage()
is a noop because it thinks we are already at the desired voltage:
core.c::regulator_set_voltage():
...
/* If we're setting the same range as last time the change
* should be a noop (some cpufreq implementations use the same
* voltage for multiple frequencies, for example).
*/
if (regulator->min_uV == min_uV && regulator->max_uV == max_uV)
goto out;
...
and as a consequence, in omap_target(), we call clk_set_rate() with
the wrong voltage.
The attached patch restore the original regulator voltage values in case
of an error.
CCing stable@ since it predates 2.6.38rc1 when the noop optimization was
introduced:
commit 95a3c23ae620c1b4c499746e70f4034bdc067737
Author: Mark Brown
Date: Thu Dec 16 15:49:37 2010 +
regulator: Optimise out noop voltage changes
Paolo Pisati (1):
regulator: core: if voltage scaling fails, restore original voltage
values
drivers/regulator/core.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Signed-off-by: Paolo Pisati
---
drivers/regulator/core.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e872c8b..c347fd0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2250,6 +2250,7 @@ int regulator_set_voltage(struct regulator *regulator,
int min_uV, int max_uV)
{
struct regulator_dev *rdev = regulator->rdev;
int ret = 0;
+ int old_min_uV, old_max_uV;
mutex_lock(>mutex);
@@ -2271,18 +2272,29 @@ int regulator_set_voltage(struct regulator *regulator,
int min_uV, int max_uV)
ret = regulator_check_voltage(rdev, _uV, _uV);
if (ret < 0)
goto out;
+
+ /* restore original values in case of error */
+ old_min_uV = regulator->min_uV;
+ old_max_uV = regulator->max_uV;
regulator->min_uV = min_uV;
regulator->max_uV = max_uV;
ret = regulator_check_consumers(rdev, _uV, _uV);
if (ret < 0)
- goto out;
+ goto out2;
ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
-
+ if (ret < 0)
+ goto out2;
+
out:
mutex_unlock(>mutex);
return ret;
+out2:
+ regulator->min_uV = old_min_uV;
+ regulator->max_uV = old_max_uV;
+ mutex_unlock(>mutex);
+ return ret;
}
EXPORT_SYMBOL_GPL(regulator_set_voltage);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Signed-off-by: Paolo Pisati paolo.pis...@canonical.com
---
drivers/regulator/core.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e872c8b..c347fd0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2250,6 +2250,7 @@ int regulator_set_voltage(struct regulator *regulator,
int min_uV, int max_uV)
{
struct regulator_dev *rdev = regulator-rdev;
int ret = 0;
+ int old_min_uV, old_max_uV;
mutex_lock(rdev-mutex);
@@ -2271,18 +2272,29 @@ 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;
+
+ /* restore original values in case of error */
+ old_min_uV = regulator-min_uV;
+ old_max_uV = 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 out2;
ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
-
+ if (ret 0)
+ goto out2;
+
out:
mutex_unlock(rdev-mutex);
return ret;
+out2:
+ regulator-min_uV = old_min_uV;
+ regulator-max_uV = old_max_uV;
+ mutex_unlock(rdev-mutex);
+ return ret;
}
EXPORT_SYMBOL_GPL(regulator_set_voltage);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regulator: core: if voltage scaling fails, restore original
And after a second look it's clear what's going on:
[...]
[5.575744] cpu cpu0: cpufreq-omap: 300 MHz, -1 mV -- 800 MHz, 1325 mV
[5.582946] voltdm_scale: No voltage scale API registered for vdd_mpu_iva
[5.590332] cpu cpu0: omap_target: unable to scale voltage up.
[1]
[5.596649] notification 1 of frequency transition to 30 kHz
[5.603179] FREQ: 30 - CPU: 0
[5.606597] governor: change or update limits
[5.611572] __cpufreq_governor for CPU 0, event 3
[5.616699] setting to 80 kHz because of event 3
[5.622039] target for CPU 0: 80 kHz, relation 1
[5.627441] request for target 80 kHz (relation: 1) for cpu 0
[5.634063] target is 2 (80 kHz, 2)
[5.638183] notification 0 of frequency transition to 80 kHz
[5.644775] cpu cpu0: cpufreq-omap: 300 MHz, -1 mV -- 800 MHz, 1325 mV
[2]
[hangs here]
first time[1] it tries to ramp up frequency (and thus voltage),
regulator_set_voltage() returns an error and we exit:
omap-cpufreq.c::omap_target():
...
dev_dbg(mpu_dev, cpufreq-omap: %u MHz, %ld mV -- %u MHz, %ld mV\n,
freqs.old / 1000, volt_old ? volt_old / 1000 : -1,
freqs.new / 1000, volt ? volt / 1000 : -1);
/* scaling up? scale voltage before frequency */
if (mpu_reg (freqs.new freqs.old)) {
r = regulator_set_voltage(mpu_reg, volt - tol, volt + tol);
if (r 0) {
dev_warn(mpu_dev, %s: unable to scale voltage up.\n,
__func__);
freqs.new = freqs.old;
goto done;
}
}
ret = clk_set_rate(mpu_clk, freqs.new * 1000);
...
but inside regulator_set_voltage(), we save the new regulator voltage before
actually ramping up:
core.c::regulator_set_voltage():
...
regulator-min_uV = min_uV;
regulator-max_uV = max_uV;
ret = regulator_check_consumers(rdev, min_uV, max_uV);
if (ret 0)
goto out2;
ret = _regulator_do_set_voltage(rdev, min_uV, max_uV); -- ERROR!!!
if (ret 0)
goto out2;
...
and by the time we try to change frequency again[2], regulator_set_voltage()
is a noop because it thinks we are already at the desired voltage:
core.c::regulator_set_voltage():
...
/* If we're setting the same range as last time the change
* should be a noop (some cpufreq implementations use the same
* voltage for multiple frequencies, for example).
*/
if (regulator-min_uV == min_uV regulator-max_uV == max_uV)
goto out;
...
and as a consequence, in omap_target(), we call clk_set_rate() with
the wrong voltage.
The attached patch restore the original regulator voltage values in case
of an error.
CCing stable@ since it predates 2.6.38rc1 when the noop optimization was
introduced:
commit 95a3c23ae620c1b4c499746e70f4034bdc067737
Author: Mark Brown broo...@opensource.wolfsonmicro.com
Date: Thu Dec 16 15:49:37 2010 +
regulator: Optimise out noop voltage changes
Paolo Pisati (1):
regulator: core: if voltage scaling fails, restore original voltage
values
drivers/regulator/core.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
On Wed, Dec 12, 2012 at 5:45 AM, Paolo Pisati
paolo.pis...@canonical.com wrote:
Signed-off-by: Paolo Pisati paolo.pis...@canonical.com
---
drivers/regulator/core.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e872c8b..c347fd0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2250,6 +2250,7 @@ int regulator_set_voltage(struct regulator *regulator,
int min_uV, int max_uV)
{
struct regulator_dev *rdev = regulator-rdev;
int ret = 0;
+ int old_min_uV, old_max_uV;
mutex_lock(rdev-mutex);
@@ -2271,18 +2272,29 @@ 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;
+
+ /* restore original values in case of error */
+ old_min_uV = regulator-min_uV;
+ old_max_uV = 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 out2;
ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
-
+ if (ret 0)
+ goto out2;
+
out:
mutex_unlock(rdev-mutex);
return ret;
+out2:
+ regulator-min_uV = old_min_uV;
+ regulator-max_uV = old_max_uV;
+ mutex_unlock(rdev-mutex);
+ return ret;
}
EXPORT_SYMBOL_GPL(regulator_set_voltage);
--
1.7.10.4
Nice! This fixes the 800Mhz lockup on bootup after a software reset we
were seeing on the Beagle xM's with v3.7.x/v3.6.x...
Tested-by: Robert Nelson robertcnel...@gmail.com
Regards,
--
Robert Nelson
http://www.rcn-ee.com/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regulator: core: if voltage scaling fails, restore original voltage values
Hi,
On Wed, Dec 12, 2012 at 12:13:35PM -0600, Robert Nelson wrote:
On Wed, Dec 12, 2012 at 5:45 AM, Paolo Pisati
paolo.pis...@canonical.com wrote:
Signed-off-by: Paolo Pisati paolo.pis...@canonical.com
---
drivers/regulator/core.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e872c8b..c347fd0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2250,6 +2250,7 @@ int regulator_set_voltage(struct regulator
*regulator, int min_uV, int max_uV)
{
struct regulator_dev *rdev = regulator-rdev;
int ret = 0;
+ int old_min_uV, old_max_uV;
mutex_lock(rdev-mutex);
@@ -2271,18 +2272,29 @@ 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;
+
+ /* restore original values in case of error */
+ old_min_uV = regulator-min_uV;
+ old_max_uV = 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 out2;
ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
-
+ if (ret 0)
+ goto out2;
+
out:
mutex_unlock(rdev-mutex);
return ret;
+out2:
+ regulator-min_uV = old_min_uV;
+ regulator-max_uV = old_max_uV;
+ mutex_unlock(rdev-mutex);
+ return ret;
}
EXPORT_SYMBOL_GPL(regulator_set_voltage);
--
1.7.10.4
Nice! This fixes the 800Mhz lockup on bootup after a software reset we
were seeing on the Beagle xM's with v3.7.x/v3.6.x...
Tested-by: Robert Nelson robertcnel...@gmail.com
if this fixes a boot lockup with older kernel, I believe this deserves a
Cc: sta...@vger.kernel.org.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] regulator: core: if voltage scaling fails, restore original
On Wed, 12 Dec 2012, Paolo Pisati wrote: but inside regulator_set_voltage(), we save the new regulator voltage before actually ramping up: core.c::regulator_set_voltage(): ... regulator-min_uV = min_uV; regulator-max_uV = max_uV; ret = regulator_check_consumers(rdev, min_uV, max_uV); if (ret 0) goto out2; ret = _regulator_do_set_voltage(rdev, min_uV, max_uV); -- ERROR!!! if (ret 0) goto out2; ... I'm not too familiar with this code. But isn't this where the bug is, rather than in that optimization commit you mentioned? Seems to me, naïvely, that in the above code, regulator-min_uV and regulator-max_uV should be set only after _regulator_do_set_voltage() succeeds? - Paul
Re: [PATCH] regulator: core: if voltage scaling fails, restore original
On Thu, 13 Dec 2012, Paul Walmsley wrote: Seems to me, naïvely, that in the above code, regulator-min_uV and regulator-max_uV should be set only after _regulator_do_set_voltage() succeeds? Eh, never mind. Looks like you took a similar strategy in the subsequent patch you sent.. - Paul
Re: [PATCH] regulator: core: if voltage scaling fails, restore original
On Wed, Dec 12, 2012 at 12:45:52PM +0100, Paolo Pisati wrote: And after a second look it's clear what's going on: After a second look at what? You've not provided any context, I've no idea what you're talking about here. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
