All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: mach-shmobile: fix Oops during system wake up
@ 2011-04-15 18:10 Guennadi Liakhovetski
  2011-04-15 18:37 ` Guennadi Liakhovetski
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Guennadi Liakhovetski @ 2011-04-15 18:10 UTC (permalink / raw
  To: linux-sh

During system resume the clock subsystem is restoring all clock rates,
if sound has not been used before suspend, the rate remains 0 and
the wake up Oopses with division by zero.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Simon Horman <horms@verge.net.au>
Cc: Magnus Damm <damm@opensource.se>
---
 arch/arm/mach-shmobile/clock-sh7372.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/clock-sh7372.c b/arch/arm/mach-shmobile/clock-sh7372.c
index e9731b5..fb29d89 100644
--- a/arch/arm/mach-shmobile/clock-sh7372.c
+++ b/arch/arm/mach-shmobile/clock-sh7372.c
@@ -458,6 +458,9 @@ static int fsidiv_set_rate(struct clk *clk, unsigned long rate)
 {
 	int idx;
 
+	if (!rate)
+		return -EDOM;
+
 	idx = (clk->parent->rate / rate) & 0xffff;
 	if (idx < 2)
 		return -EINVAL;
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] ARM: mach-shmobile: fix Oops during system wake up
  2011-04-15 18:10 [PATCH] ARM: mach-shmobile: fix Oops during system wake up Guennadi Liakhovetski
@ 2011-04-15 18:37 ` Guennadi Liakhovetski
  2011-04-16 10:38 ` Simon Horman
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Guennadi Liakhovetski @ 2011-04-15 18:37 UTC (permalink / raw
  To: linux-sh

On Fri, 15 Apr 2011, Guennadi Liakhovetski wrote:

> During system resume the clock subsystem is restoring all clock rates,
> if sound has not been used before suspend, the rate remains 0 and
> the wake up Oopses with division by zero.

Right, I see now. Magnus had a more general solution for this problem, 
which I accidentally dropped in my tests, which caused this Oops. So, 
maybe his patch to check rate in clks_core_resume() is a better fix for 
this problem. Feel free to ignore this then.

Thanks
Guennadi

> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Simon Horman <horms@verge.net.au>
> Cc: Magnus Damm <damm@opensource.se>
> ---
>  arch/arm/mach-shmobile/clock-sh7372.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/clock-sh7372.c b/arch/arm/mach-shmobile/clock-sh7372.c
> index e9731b5..fb29d89 100644
> --- a/arch/arm/mach-shmobile/clock-sh7372.c
> +++ b/arch/arm/mach-shmobile/clock-sh7372.c
> @@ -458,6 +458,9 @@ static int fsidiv_set_rate(struct clk *clk, unsigned long rate)
>  {
>  	int idx;
>  
> +	if (!rate)
> +		return -EDOM;
> +
>  	idx = (clk->parent->rate / rate) & 0xffff;
>  	if (idx < 2)
>  		return -EINVAL;
> -- 
> 1.7.2.5
> 
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ARM: mach-shmobile: fix Oops during system wake up
  2011-04-15 18:10 [PATCH] ARM: mach-shmobile: fix Oops during system wake up Guennadi Liakhovetski
  2011-04-15 18:37 ` Guennadi Liakhovetski
@ 2011-04-16 10:38 ` Simon Horman
  2011-04-18  9:09 ` Paul Mundt
  2011-04-18  9:13 ` Magnus Damm
  3 siblings, 0 replies; 5+ messages in thread
From: Simon Horman @ 2011-04-16 10:38 UTC (permalink / raw
  To: linux-sh

On Fri, Apr 15, 2011 at 08:37:55PM +0200, Guennadi Liakhovetski wrote:
> On Fri, 15 Apr 2011, Guennadi Liakhovetski wrote:
> 
> > During system resume the clock subsystem is restoring all clock rates,
> > if sound has not been used before suspend, the rate remains 0 and
> > the wake up Oopses with division by zero.
> 
> Right, I see now. Magnus had a more general solution for this problem, 
> which I accidentally dropped in my tests, which caused this Oops. So, 
> maybe his patch to check rate in clks_core_resume() is a better fix for 
> this problem. Feel free to ignore this then.

Hi Guennadi,

do you have a link/title for Magnus's patch?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ARM: mach-shmobile: fix Oops during system wake up
  2011-04-15 18:10 [PATCH] ARM: mach-shmobile: fix Oops during system wake up Guennadi Liakhovetski
  2011-04-15 18:37 ` Guennadi Liakhovetski
  2011-04-16 10:38 ` Simon Horman
@ 2011-04-18  9:09 ` Paul Mundt
  2011-04-18  9:13 ` Magnus Damm
  3 siblings, 0 replies; 5+ messages in thread
From: Paul Mundt @ 2011-04-18  9:09 UTC (permalink / raw
  To: linux-sh

On Fri, Apr 15, 2011 at 08:37:55PM +0200, Guennadi Liakhovetski wrote:
> On Fri, 15 Apr 2011, Guennadi Liakhovetski wrote:
> 
> > During system resume the clock subsystem is restoring all clock rates,
> > if sound has not been used before suspend, the rate remains 0 and
> > the wake up Oopses with division by zero.
> 
> Right, I see now. Magnus had a more general solution for this problem, 
> which I accidentally dropped in my tests, which caused this Oops. So, 
> maybe his patch to check rate in clks_core_resume() is a better fix for 
> this problem. Feel free to ignore this then.
> 
Having a solution that doesn't exist in the list archives is effectively
the same as not having a solution at all.

From your description I imagine you mean something like checking if
clkp->rate is set prior to calling in to ->set_rate, however this
probably needs a bit more of a think. Your example stipulates that we're
resuming clocks that haven't been "used", which suggests that we're
probably better off simply not touching the clock at all in the resume
path. I expect if you were to set the rate via the recalc path instead it
also wouldn't have the desired effect for your case.

In any event, it's quite obvious that clks_core_resume() is going to need
to be reworked a bit.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ARM: mach-shmobile: fix Oops during system wake up
  2011-04-15 18:10 [PATCH] ARM: mach-shmobile: fix Oops during system wake up Guennadi Liakhovetski
                   ` (2 preceding siblings ...)
  2011-04-18  9:09 ` Paul Mundt
@ 2011-04-18  9:13 ` Magnus Damm
  3 siblings, 0 replies; 5+ messages in thread
From: Magnus Damm @ 2011-04-18  9:13 UTC (permalink / raw
  To: linux-sh

On Mon, Apr 18, 2011 at 6:09 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> On Fri, Apr 15, 2011 at 08:37:55PM +0200, Guennadi Liakhovetski wrote:
>> On Fri, 15 Apr 2011, Guennadi Liakhovetski wrote:
>>
>> > During system resume the clock subsystem is restoring all clock rates,
>> > if sound has not been used before suspend, the rate remains 0 and
>> > the wake up Oopses with division by zero.
>>
>> Right, I see now. Magnus had a more general solution for this problem,
>> which I accidentally dropped in my tests, which caused this Oops. So,
>> maybe his patch to check rate in clks_core_resume() is a better fix for
>> this problem. Feel free to ignore this then.
>>
> Having a solution that doesn't exist in the list archives is effectively
> the same as not having a solution at all.

Sure. I suppose Guennadi is referring to a hunk in a local patch that
doesn't exist in list archives.

> From your description I imagine you mean something like checking if
> clkp->rate is set prior to calling in to ->set_rate, however this
> probably needs a bit more of a think. Your example stipulates that we're
> resuming clocks that haven't been "used", which suggests that we're
> probably better off simply not touching the clock at all in the resume
> path. I expect if you were to set the rate via the recalc path instead it
> also wouldn't have the desired effect for your case.
>
> In any event, it's quite obvious that clks_core_resume() is going to need
> to be reworked a bit.

I ran across this, not sure if it's the right way to fix this.

--- 0001/drivers/sh/clk/core.c
+++ work/drivers/sh/clk/core.c  2011-04-18 08:46:43.000000000 +0900
@@ -641,7 +641,7 @@ static void clks_core_resume(void)
                        if (likely(clkp->ops->set_parent))
                                clkp->ops->set_parent(clkp,
                                        clkp->parent);
-                       if (likely(clkp->ops->set_rate))
+                       if (rate && likely(clkp->ops->set_rate))
                                clkp->ops->set_rate(clkp, rate);
                        else if (likely(clkp->ops->recalc))
                                clkp->rate = clkp->ops->recalc(clkp);

This does not trigger on mainline SH-Mobile ARM since we don't support
system wide suspend.

/ magnus

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-04-18  9:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-15 18:10 [PATCH] ARM: mach-shmobile: fix Oops during system wake up Guennadi Liakhovetski
2011-04-15 18:37 ` Guennadi Liakhovetski
2011-04-16 10:38 ` Simon Horman
2011-04-18  9:09 ` Paul Mundt
2011-04-18  9:13 ` Magnus Damm

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.