[PATCH v3 1/5] omap3: pm: fix for twl4030 script load

[PATCH v3 1/5] omap3: pm: fix for twl4030 script load

[Lesly A M]

Keeping the twl4030_config_sleep_sequence() call under the flag checking will fix
the over writting of other sequences with the sleep sequences when loading the scripts.

Removed the warning print with checking order of scripts, since the order
is not important. 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

Re: [PATCH v3 1/5] omap3: pm: fix for twl4030 script load

[Kevin Hilman]

Lesly A M <leslyam@ti.com> writes:

> Keeping the twl4030_config_sleep_sequence() call under the flag checking will fix
> the over writting of other sequences with the sleep sequences when loading the scripts.

Sorry, but I read this 4-5 times and still have no idea what it means,
or how it relates to this patch.

> Removed the warning print with checking order of scripts, since the order
> is not important. 

OK, this much I get, and see what the patch is doing.

> Only the values configured in the register, which is pointing to
> the starting address of each sequence should be correct.

aw, bummer. -ECONFUSED again

Please fix the changelog to not only summarize *what* is being changed,
but *why*.

Kevin

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

RE: [PATCH v3 1/5] omap3: pm: fix for twl4030 script load

[Lesly Arackal Manuel]

Hi Kevin,

Comments inline

Thanks & Regards,
Lesly A M

-----Original Message-----
From: Kevin Hilman [mailto:khilman@deeprootsystems.com] 
Sent: Friday, March 19, 2010 4:51 AM
To: Lesly A M
Cc: linux-omap@vger.kernel.org; Lesly A M; Nishanth Menon; David Derrick;
Samuel Ortiz
Subject: Re: [PATCH v3 1/5] omap3: pm: fix for twl4030 script load

Lesly A M <leslyam@ti.com> writes:

> Keeping the twl4030_config_sleep_sequence() call under the flag checking
will fix
> the over writting of other sequences with the sleep sequences when loading
the scripts.

Sorry, but I read this 4-5 times and still have no idea what it means,
or how it relates to this patch.

[Lesly] All other sequence are getting over written by sleep seq.
Because twl4030_config_sleep_sequence() function is getting called always
when we enter load_twl4030_script().
And it will also over write the starting address of all seq with 0x2F.

> Removed the warning print with checking order of scripts, since the order
> is not important. 

OK, this much I get, and see what the patch is doing.

> Only the values configured in the register, which is pointing to
> the starting address of each sequence should be correct.

aw, bummer. -ECONFUSED again

Please fix the changelog to not only summarize *what* is being changed,
but *why*.

[Lesly] Why the order is not important.
Since each seq is independent and not falling trough any other seq.
Only thing we have to do is program the start address of each seq correctly.

Kevin

> 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");


Since calling this is not executed under the flag TWL4030_SLEEP_SCRIPT, will
be executed always.
>  		err = twl4030_config_sleep_sequence(address);


>  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

Re: [PATCH v3 1/5] omap3: pm: fix for twl4030 script load

[Kevin Hilman]

"Lesly Arackal Manuel" <leslyam@ti.com> writes:

[...]

> Please fix the changelog to not only summarize *what* is being changed,
> but *why*.
>
> [Lesly] Why the order is not important.
> Since each seq is independent and not falling trough any other seq.
> Only thing we have to do is program the start address of each seq correctly.

The point is that this needs to be summarized better in the changelog.

You need to explain it in general way so even folks who are not
intimately involved with the code as you are can understand the
motiviation behind the change.

Kevin