diff mbox

[2/3] MFD: TWL4030: print warning for out-of-order script loading

Message ID 1247050175-31163-1-git-send-email-amit.kucheria@verdurent.com (mailing list archive)
State Awaiting Upstream, archived
Headers show

Commit Message

Amit Kucheria July 8, 2009, 10:49 a.m. UTC
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.

Various options were considered to solve this problem, including
modification of the core twl4030 power code to be smart enough to
reorder the loading of the scripts. But it felt too over-engineered.

Hence this patch just warns the DPS script developer so that they may be
reordered in the board-code itself.

Signed-off-by: Amit Kucheria <amit.kucheria@verdurent.com>
---
 drivers/mfd/twl4030-power.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

Comments

Samuel Ortiz Aug. 4, 2009, 6 p.m. UTC | #1
Hi Amit,

On Wed, Jul 08, 2009 at 01:49:35PM +0300, Amit Kucheria wrote:
> 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.
> 
> Various options were considered to solve this problem, including
> modification of the core twl4030 power code to be smart enough to
> reorder the loading of the scripts. But it felt too over-engineered.
> 
> Hence this patch just warns the DPS script developer so that they may be
> reordered in the board-code itself.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@verdurent.com>
> ---
>  drivers/mfd/twl4030-power.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
> index bb9e45f..ef4cc4e 100644
> --- a/drivers/mfd/twl4030-power.c
> +++ b/drivers/mfd/twl4030-power.c
> @@ -315,6 +315,7 @@ static int __init load_triton_script(struct twl4030_script *tscript)
>  {
>  	u8 address = triton_next_free_address;
>  	int err;
> +	static u8 mask = 0;
>  
>  	/* Make sure the script isn't going beyond last valid address */
>  	if ((address + tscript->size) > (END_OF_SCRIPT-1)) {
> @@ -331,14 +332,20 @@ static int __init load_triton_script(struct twl4030_script *tscript)
>  	if (tscript->flags & TRITON_WRST_SCRIPT)
>  		err |= config_warmreset_sequence(address);
>  
> -	if (tscript->flags & TRITON_WAKEUP12_SCRIPT)
> +	if (tscript->flags & TRITON_WAKEUP12_SCRIPT) {
>  		err |= config_wakeup12_sequence(address);
> +		mask |= TRITON_WAKEUP12_SCRIPT;
> +	}
>  
>  	if (tscript->flags & TRITON_WAKEUP3_SCRIPT)
>  		err |= config_wakeup3_sequence(address);
>  
> -	if (tscript->flags & TRITON_SLEEP_SCRIPT)
> +	if (tscript->flags & TRITON_SLEEP_SCRIPT) {
Wouldnt something like:

     if (tscript->flags & (TRITON_SLEEP_SCRIPT | TRITON_WAKEUP12_SCRIPT)) {

save us from using an additional mask variable and simplify this patch ?

Cheers,
Samuel.


> +		if (!(mask & TRITON_WAKEUP12_SCRIPT))
> +			printk(KERN_WARNING
> +			       "TWL4030: Wakeup script not yet loaded. Might lead to boot failure on some boards\n");
>  		err |= config_sleep_sequence(address);
> +	}
>  
>  	return err;
>  }
> -- 
> 1.6.3.3
>
Amit Kucheria Aug. 17, 2009, 11:43 a.m. UTC | #2
On 09 Aug 04, Samuel Ortiz wrote:
> Hi Amit,
> 
> On Wed, Jul 08, 2009 at 01:49:35PM +0300, Amit Kucheria wrote:
> > 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.
> > 
> > Various options were considered to solve this problem, including
> > modification of the core twl4030 power code to be smart enough to
> > reorder the loading of the scripts. But it felt too over-engineered.
> > 
> > Hence this patch just warns the DPS script developer so that they may be
> > reordered in the board-code itself.
> > 
> > Signed-off-by: Amit Kucheria <amit.kucheria@verdurent.com>
> > ---
> >  drivers/mfd/twl4030-power.c |   11 +++++++++--
> >  1 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
> > index bb9e45f..ef4cc4e 100644
> > --- a/drivers/mfd/twl4030-power.c
> > +++ b/drivers/mfd/twl4030-power.c
> > @@ -315,6 +315,7 @@ static int __init load_triton_script(struct twl4030_script *tscript)
> >  {
> >  	u8 address = triton_next_free_address;
> >  	int err;
> > +	static u8 mask = 0;
> >  
> >  	/* Make sure the script isn't going beyond last valid address */
> >  	if ((address + tscript->size) > (END_OF_SCRIPT-1)) {
> > @@ -331,14 +332,20 @@ static int __init load_triton_script(struct twl4030_script *tscript)
> >  	if (tscript->flags & TRITON_WRST_SCRIPT)
> >  		err |= config_warmreset_sequence(address);
> >  
> > -	if (tscript->flags & TRITON_WAKEUP12_SCRIPT)
> > +	if (tscript->flags & TRITON_WAKEUP12_SCRIPT) {
> >  		err |= config_wakeup12_sequence(address);
> > +		mask |= TRITON_WAKEUP12_SCRIPT;
> > +	}
> >  
> >  	if (tscript->flags & TRITON_WAKEUP3_SCRIPT)
> >  		err |= config_wakeup3_sequence(address);
> >  
> > -	if (tscript->flags & TRITON_SLEEP_SCRIPT)
> > +	if (tscript->flags & TRITON_SLEEP_SCRIPT) {
> Wouldnt something like:
> 
>      if (tscript->flags & (TRITON_SLEEP_SCRIPT | TRITON_WAKEUP12_SCRIPT)) {
> 
> save us from using an additional mask variable and simplify this patch ?
> 

I don't see how this will save the mask variable. What I need to detect
that the SLEEP script is loaded before the WAKEUP script. That should
trigger a warning.
diff mbox

Patch

diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
index bb9e45f..ef4cc4e 100644
--- a/drivers/mfd/twl4030-power.c
+++ b/drivers/mfd/twl4030-power.c
@@ -315,6 +315,7 @@  static int __init load_triton_script(struct twl4030_script *tscript)
 {
 	u8 address = triton_next_free_address;
 	int err;
+	static u8 mask = 0;
 
 	/* Make sure the script isn't going beyond last valid address */
 	if ((address + tscript->size) > (END_OF_SCRIPT-1)) {
@@ -331,14 +332,20 @@  static int __init load_triton_script(struct twl4030_script *tscript)
 	if (tscript->flags & TRITON_WRST_SCRIPT)
 		err |= config_warmreset_sequence(address);
 
-	if (tscript->flags & TRITON_WAKEUP12_SCRIPT)
+	if (tscript->flags & TRITON_WAKEUP12_SCRIPT) {
 		err |= config_wakeup12_sequence(address);
+		mask |= TRITON_WAKEUP12_SCRIPT;
+	}
 
 	if (tscript->flags & TRITON_WAKEUP3_SCRIPT)
 		err |= config_wakeup3_sequence(address);
 
-	if (tscript->flags & TRITON_SLEEP_SCRIPT)
+	if (tscript->flags & TRITON_SLEEP_SCRIPT) {
+		if (!(mask & TRITON_WAKEUP12_SCRIPT))
+			printk(KERN_WARNING
+			       "TWL4030: Wakeup script not yet loaded. Might lead to boot failure on some boards\n");
 		err |= config_sleep_sequence(address);
+	}
 
 	return err;
 }