linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/5] omap3: pm: fix for twl4030 script load
@ 2010-04-19 11:43 Lesly A M
  2010-05-04  6:27 ` Eduardo Valentin
  0 siblings, 1 reply; 6+ messages in thread
From: Lesly A M @ 2010-04-19 11:43 UTC (permalink / raw)
  To: linux-omap; +Cc: Lesly A M, Nishanth Menon, David Derrick, Samuel Ortiz

This patch will fix the TRITON sleep/wakeup sequence.

Since the function to populate the sleep script is getting called always
irrespective of the flag "TWL4030_SLEEP_SCRIPT", other scripts data
is getting over written by the sleep script.

Also removing the order checking while loading the scripts,
since the order doesn't matter. Only the values configured
in the register, which is pointing to the starting address
of each sequence should be correct.

Signed-off-by: Lesly A M <x0080970@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: David Derrick <dderrick@ti.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
---
 drivers/mfd/twl4030-power.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
index 7efa878..bd98733 100644
--- a/drivers/mfd/twl4030-power.c
+++ b/drivers/mfd/twl4030-power.c
@@ -423,7 +423,6 @@ static int __init load_twl4030_script(struct twl4030_script *tscript,
 	       u8 address)
 {
 	int err;
-	static int order;
 
 	/* Make sure the script isn't going beyond last valid address (0x3f) */
 	if ((address + tscript->size) > END_OF_SCRIPT) {
@@ -444,7 +443,6 @@ static int __init load_twl4030_script(struct twl4030_script *tscript,
 		err = twl4030_config_wakeup12_sequence(address);
 		if (err)
 			goto out;
-		order = 1;
 	}
 	if (tscript->flags & TWL4030_WAKEUP3_SCRIPT) {
 		err = twl4030_config_wakeup3_sequence(address);
@@ -452,10 +450,6 @@ static int __init load_twl4030_script(struct twl4030_script *tscript,
 			goto out;
 	}
 	if (tscript->flags & TWL4030_SLEEP_SCRIPT)
-		if (order)
-			pr_warning("TWL4030: Bad order of scripts (sleep "\
-					"script before wakeup) Leads to boot"\
-					"failure on some boards\n");
 		err = twl4030_config_sleep_sequence(address);
 out:
 	return err;
-- 
1.6.0.4


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

* Re: [PATCH v5 1/5] omap3: pm: fix for twl4030 script load
  2010-04-19 11:43 [PATCH v5 1/5] omap3: pm: fix for twl4030 script load Lesly A M
@ 2010-05-04  6:27 ` Eduardo Valentin
  2010-05-04  7:37   ` Lesly Arackal Manuel
  0 siblings, 1 reply; 6+ messages in thread
From: Eduardo Valentin @ 2010-05-04  6:27 UTC (permalink / raw)
  To: ext Lesly A M
  Cc: linux-omap@vger.kernel.org, Lesly A M, Nishanth Menon,
	David Derrick, Samuel Ortiz

Hello Lesly,

On Mon, Apr 19, 2010 at 01:43:00PM +0200, ext Lesly A M wrote:
> This patch will fix the TRITON sleep/wakeup sequence.

OK.

> 
> Since the function to populate the sleep script is getting called always
> irrespective of the flag "TWL4030_SLEEP_SCRIPT", other scripts data
> is getting over written by the sleep script.

Yes, and that is because of missing curls {}.

> 
> Also removing the order checking while loading the scripts,
> since the order doesn't matter. Only the values configured
> in the register, which is pointing to the starting address
> of each sequence should be correct.

Are sure about that. Please check commig 75a7456539224c5c5c254130afdb18bd7eb2286f
At least from that description, Amit was very convincing that loading
order matters.

Author: Amit Kucheria <amit.kucheria@verdurent.com>
Date:   Mon Aug 17 17:01:56 2009 +0300

    mfd: Print warning for twl4030 out-of-order script loading
    
    When the sleep script is loaded before the wakeup script, there is a
    chance that the system might go to sleep before the wakeup script
    loading is completed. This will lead to a system that does not wakeup
    and has been observed to cause non-booting boards.


> 
> Signed-off-by: Lesly A M <x0080970@ti.com>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: David Derrick <dderrick@ti.com>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> ---
>  drivers/mfd/twl4030-power.c |    6 ------
>  1 files changed, 0 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
> index 7efa878..bd98733 100644
> --- a/drivers/mfd/twl4030-power.c
> +++ b/drivers/mfd/twl4030-power.c
> @@ -423,7 +423,6 @@ static int __init load_twl4030_script(struct twl4030_script *tscript,
>  	       u8 address)
>  {
>  	int err;
> -	static int order;
>  
>  	/* Make sure the script isn't going beyond last valid address (0x3f) */
>  	if ((address + tscript->size) > END_OF_SCRIPT) {
> @@ -444,7 +443,6 @@ static int __init load_twl4030_script(struct twl4030_script *tscript,
>  		err = twl4030_config_wakeup12_sequence(address);
>  		if (err)
>  			goto out;
> -		order = 1;
>  	}
>  	if (tscript->flags & TWL4030_WAKEUP3_SCRIPT) {
>  		err = twl4030_config_wakeup3_sequence(address);
> @@ -452,10 +450,6 @@ static int __init load_twl4030_script(struct twl4030_script *tscript,
>  			goto out;
>  	}
>  	if (tscript->flags & TWL4030_SLEEP_SCRIPT)
> -		if (order)
> -			pr_warning("TWL4030: Bad order of scripts (sleep "\
> -					"script before wakeup) Leads to boot"\
> -					"failure on some boards\n");
>  		err = twl4030_config_sleep_sequence(address);


If the problem you are trying to address is the fact that everything goes to
sleep address, then just embrace the conditional into brackets.

>  out:
>  	return err;
> -- 
> 1.6.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* RE: [PATCH v5 1/5] omap3: pm: fix for twl4030 script load
  2010-05-04  6:27 ` Eduardo Valentin
@ 2010-05-04  7:37   ` Lesly Arackal Manuel
  2010-05-04  8:47     ` Eduardo Valentin
  2010-05-11 12:46     ` Peter 'p2' De Schrijver
  0 siblings, 2 replies; 6+ messages in thread
From: Lesly Arackal Manuel @ 2010-05-04  7:37 UTC (permalink / raw)
  To: eduardo.valentin
  Cc: linux-omap, 'Lesly A M', 'Nishanth Menon',
	'David Derrick', 'Samuel Ortiz'

> -----Original Message-----
> From: Eduardo Valentin [mailto:eduardo.valentin@nokia.com]
> Sent: Tuesday, May 04, 2010 11:57 AM
> To: ext Lesly A M
> Cc: linux-omap@vger.kernel.org; Lesly A M; Nishanth Menon; David Derrick;
> Samuel Ortiz
> Subject: Re: [PATCH v5 1/5] omap3: pm: fix for twl4030 script load
> 
> Hello Lesly,
> 
> On Mon, Apr 19, 2010 at 01:43:00PM +0200, ext Lesly A M wrote:
> > This patch will fix the TRITON sleep/wakeup sequence.
> 
> OK.
> 
> >
> > Since the function to populate the sleep script is getting called always
> > irrespective of the flag "TWL4030_SLEEP_SCRIPT", other scripts data
> > is getting over written by the sleep script.
> 
> Yes, and that is because of missing curls {}.
> 
> >
> > Also removing the order checking while loading the scripts,
> > since the order doesn't matter. Only the values configured
> > in the register, which is pointing to the starting address
> > of each sequence should be correct.
> 
> Are sure about that. Please check commig
> 75a7456539224c5c5c254130afdb18bd7eb2286f
> At least from that description, Amit was very convincing that loading
> order matters.
> 
> Author: Amit Kucheria <amit.kucheria@verdurent.com>
> Date:   Mon Aug 17 17:01:56 2009 +0300
> 
>     mfd: Print warning for twl4030 out-of-order script loading
> 
>     When the sleep script is loaded before the wakeup script, there is a
>     chance that the system might go to sleep before the wakeup script
>     loading is completed. This will lead to a system that does not wakeup
>     and has been observed to cause non-booting boards.
>
> >
> > Signed-off-by: Lesly A M <x0080970@ti.com>
> > Cc: Nishanth Menon <nm@ti.com>
> > Cc: David Derrick <dderrick@ti.com>
> > Cc: Samuel Ortiz <sameo@linux.intel.com>
> > ---
> >  drivers/mfd/twl4030-power.c |    6 ------
> >  1 files changed, 0 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
> > index 7efa878..bd98733 100644
> > --- a/drivers/mfd/twl4030-power.c
> > +++ b/drivers/mfd/twl4030-power.c
> > @@ -423,7 +423,6 @@ static int __init load_twl4030_script(struct
> twl4030_script *tscript,
> >  	       u8 address)
> >  {
> >  	int err;
> > -	static int order;
> >
> >  	/* Make sure the script isn't going beyond last valid address (0x3f)
> */
> >  	if ((address + tscript->size) > END_OF_SCRIPT) {
> > @@ -444,7 +443,6 @@ static int __init load_twl4030_script(struct
> twl4030_script *tscript,
> >  		err = twl4030_config_wakeup12_sequence(address);
> >  		if (err)
> >  			goto out;
> > -		order = 1;
> >  	}
> >  	if (tscript->flags & TWL4030_WAKEUP3_SCRIPT) {
> >  		err = twl4030_config_wakeup3_sequence(address);
> > @@ -452,10 +450,6 @@ static int __init load_twl4030_script(struct
> twl4030_script *tscript,
> >  			goto out;
> >  	}
> >  	if (tscript->flags & TWL4030_SLEEP_SCRIPT)
> > -		if (order)
> > -			pr_warning("TWL4030: Bad order of scripts (sleep "\
> > -					"script before wakeup) Leads to
boot"\
> > -					"failure on some boards\n");
> >  		err = twl4030_config_sleep_sequence(address);
> 
> 
> If the problem you are trying to address is the fact that everything goes
> to
> sleep address, then just embrace the conditional into brackets.
> 


Hi Eduardo,

The load_twl4030_script() is called from twl4030_power_init() which is again
called from twl_probe(), and this is getting called before
omap3_idle_init().

So the scripts are loaded before the cpuidle is initialized.
Then I don't think the system will hit sys_off before loading the scripts.

Regards,
Lesly



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

* Re: [PATCH v5 1/5] omap3: pm: fix for twl4030 script load
  2010-05-04  7:37   ` Lesly Arackal Manuel
@ 2010-05-04  8:47     ` Eduardo Valentin
  2010-05-11 12:46     ` Peter 'p2' De Schrijver
  1 sibling, 0 replies; 6+ messages in thread
From: Eduardo Valentin @ 2010-05-04  8:47 UTC (permalink / raw)
  To: ext Lesly Arackal Manuel
  Cc: Valentin Eduardo (Nokia-D/Helsinki), linux-omap@vger.kernel.org,
	'Lesly A M', 'Nishanth Menon',
	'David Derrick', 'Samuel Ortiz', Amit Kucheria

Hello Lesly,

On Tue, May 04, 2010 at 09:37:10AM +0200, ext Lesly Arackal Manuel wrote:
> > -----Original Message-----
> > From: Eduardo Valentin [mailto:eduardo.valentin@nokia.com]
> > Sent: Tuesday, May 04, 2010 11:57 AM
> > To: ext Lesly A M
> > Cc: linux-omap@vger.kernel.org; Lesly A M; Nishanth Menon; David Derrick;
> > Samuel Ortiz
> > Subject: Re: [PATCH v5 1/5] omap3: pm: fix for twl4030 script load
> > 
> > Hello Lesly,
> > 
> > On Mon, Apr 19, 2010 at 01:43:00PM +0200, ext Lesly A M wrote:
> > > This patch will fix the TRITON sleep/wakeup sequence.
> > 
> > OK.
> > 
> > >
> > > Since the function to populate the sleep script is getting called always
> > > irrespective of the flag "TWL4030_SLEEP_SCRIPT", other scripts data
> > > is getting over written by the sleep script.
> > 
> > Yes, and that is because of missing curls {}.
> > 
> > >
> > > Also removing the order checking while loading the scripts,
> > > since the order doesn't matter. Only the values configured
> > > in the register, which is pointing to the starting address
> > > of each sequence should be correct.
> > 
> > Are sure about that. Please check commig
> > 75a7456539224c5c5c254130afdb18bd7eb2286f
> > At least from that description, Amit was very convincing that loading
> > order matters.
> > 
> > Author: Amit Kucheria <amit.kucheria@verdurent.com>
> > Date:   Mon Aug 17 17:01:56 2009 +0300
> > 
> >     mfd: Print warning for twl4030 out-of-order script loading
> > 
> >     When the sleep script is loaded before the wakeup script, there is a
> >     chance that the system might go to sleep before the wakeup script
> >     loading is completed. This will lead to a system that does not wakeup
> >     and has been observed to cause non-booting boards.
> >
> > >
> > > Signed-off-by: Lesly A M <x0080970@ti.com>
> > > Cc: Nishanth Menon <nm@ti.com>
> > > Cc: David Derrick <dderrick@ti.com>
> > > Cc: Samuel Ortiz <sameo@linux.intel.com>
> > > ---
> > >  drivers/mfd/twl4030-power.c |    6 ------
> > >  1 files changed, 0 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
> > > index 7efa878..bd98733 100644
> > > --- a/drivers/mfd/twl4030-power.c
> > > +++ b/drivers/mfd/twl4030-power.c
> > > @@ -423,7 +423,6 @@ static int __init load_twl4030_script(struct
> > twl4030_script *tscript,
> > >  	       u8 address)
> > >  {
> > >  	int err;
> > > -	static int order;
> > >
> > >  	/* Make sure the script isn't going beyond last valid address (0x3f)
> > */
> > >  	if ((address + tscript->size) > END_OF_SCRIPT) {
> > > @@ -444,7 +443,6 @@ static int __init load_twl4030_script(struct
> > twl4030_script *tscript,
> > >  		err = twl4030_config_wakeup12_sequence(address);
> > >  		if (err)
> > >  			goto out;
> > > -		order = 1;
> > >  	}
> > >  	if (tscript->flags & TWL4030_WAKEUP3_SCRIPT) {
> > >  		err = twl4030_config_wakeup3_sequence(address);
> > > @@ -452,10 +450,6 @@ static int __init load_twl4030_script(struct
> > twl4030_script *tscript,
> > >  			goto out;
> > >  	}
> > >  	if (tscript->flags & TWL4030_SLEEP_SCRIPT)
> > > -		if (order)
> > > -			pr_warning("TWL4030: Bad order of scripts (sleep "\
> > > -					"script before wakeup) Leads to
> boot"\
> > > -					"failure on some boards\n");
> > >  		err = twl4030_config_sleep_sequence(address);
> > 
> > 
> > If the problem you are trying to address is the fact that everything goes
> > to
> > sleep address, then just embrace the conditional into brackets.
> > 
> 
> 
> Hi Eduardo,
> 
> The load_twl4030_script() is called from twl4030_power_init() which is again
> called from twl_probe(), and this is getting called before
> omap3_idle_init().
> 
> So the scripts are loaded before the cpuidle is initialized.
> Then I don't think the system will hit sys_off before loading the scripts.

Yes indeed. At first glance I would expect same thing. But there must be a reason
for that patch :-). Maybe Amit can comment better on the problem he was addressing.
I'm ccing him.


> 
> Regards,
> Lesly
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v5 1/5] omap3: pm: fix for twl4030 script load
  2010-05-04  7:37   ` Lesly Arackal Manuel
  2010-05-04  8:47     ` Eduardo Valentin
@ 2010-05-11 12:46     ` Peter 'p2' De Schrijver
  2010-05-13  9:43       ` Lesly Arackal Manuel
  1 sibling, 1 reply; 6+ messages in thread
From: Peter 'p2' De Schrijver @ 2010-05-11 12:46 UTC (permalink / raw)
  To: ext Lesly Arackal Manuel
  Cc: Valentin Eduardo (Nokia-D/Helsinki), linux-omap@vger.kernel.org,
	'Lesly A M', 'Nishanth Menon',
	'David Derrick', 'Samuel Ortiz'

Hi,

> 
> Hi Eduardo,
> 
> The load_twl4030_script() is called from twl4030_power_init() which is again
> called from twl_probe(), and this is getting called before
> omap3_idle_init().
> 
> So the scripts are loaded before the cpuidle is initialized.
> Then I don't think the system will hit sys_off before loading the scripts.
> 
> Regards,
> Lesly
> 

Hi,

I agree that in our usecase the problem can probably never happen. But
what if NSLEEP2 would be controlled by some external component (Ie. not
OMAP) ? I don't think we can be sure that the sleep script will not be
executed while the wakeup script is not yet loaded (assuming they would
be loaded in the wrong order). Note that loading the script also sets
the address in the corresponding TWL4030 register. So I think this
safeguard is still useful.

Thanks,

Peter.

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

* RE: [PATCH v5 1/5] omap3: pm: fix for twl4030 script load
  2010-05-11 12:46     ` Peter 'p2' De Schrijver
@ 2010-05-13  9:43       ` Lesly Arackal Manuel
  0 siblings, 0 replies; 6+ messages in thread
From: Lesly Arackal Manuel @ 2010-05-13  9:43 UTC (permalink / raw)
  To: 'Peter 'p2' De Schrijver'
  Cc: 'Valentin Eduardo (Nokia-D/Helsinki)', linux-omap,
	'Lesly A M', 'Nishanth Menon',
	'David Derrick', 'Samuel Ortiz'


> -----Original Message-----
> From: Peter 'p2' De Schrijver [mailto:peter.de-schrijver@nokia.com]
> Sent: Tuesday, May 11, 2010 6:17 PM
> To: ext Lesly Arackal Manuel
> Cc: Valentin Eduardo (Nokia-D/Helsinki); linux-omap@vger.kernel.org;
> 'Lesly A M'; 'Nishanth Menon'; 'David Derrick'; 'Samuel Ortiz'
> Subject: Re: [PATCH v5 1/5] omap3: pm: fix for twl4030 script load
> 
> Hi,
> 
> >
> > Hi Eduardo,
> >
> > The load_twl4030_script() is called from twl4030_power_init() which is
> again
> > called from twl_probe(), and this is getting called before
> > omap3_idle_init().
> >
> > So the scripts are loaded before the cpuidle is initialized.
> > Then I don't think the system will hit sys_off before loading the
> scripts.
> >
> > Regards,
> > Lesly
> >
> 
> Hi,
> 
> I agree that in our usecase the problem can probably never happen. But
> what if NSLEEP2 would be controlled by some external component (Ie. not
> OMAP) ? I don't think we can be sure that the sleep script will not be
> executed while the wakeup script is not yet loaded (assuming they would
> be loaded in the wrong order). Note that loading the script also sets
> the address in the corresponding TWL4030 register. So I think this
> safeguard is still useful.
> 
> Thanks,
> Peter.

Hi Peter,

 I am not very much sure whether the MODEM(or any co processor) connected to
NSLEEP2 of TRITON will assert the sleep signal, before OMAP3430(AP) boots
up.

 So if this is confirmed, it will be better to check the order while
populating the scripts.

Thanks & Regards,
Lesly A M




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

end of thread, other threads:[~2010-05-13  9:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-19 11:43 [PATCH v5 1/5] omap3: pm: fix for twl4030 script load Lesly A M
2010-05-04  6:27 ` Eduardo Valentin
2010-05-04  7:37   ` Lesly Arackal Manuel
2010-05-04  8:47     ` Eduardo Valentin
2010-05-11 12:46     ` Peter 'p2' De Schrijver
2010-05-13  9:43       ` Lesly Arackal Manuel

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).