Patchwork [U-Boot,RFC] POST: add test before execution of post_run to avoid unneeded run

login
register
mail settings
Submitter Valentin Longchamp
Date July 25, 2011, 2:07 p.m.
Message ID <1311602833-29320-1-git-send-email-valentin.longchamp@keymile.com>
Download mbox | patch
Permalink /patch/106677/
State RFC
Headers show

Comments

Valentin Longchamp - July 25, 2011, 2:07 p.m.
Some boards have the environment variables defined in a slow EEPROM. post_run
accesses these environment variables to define which tests have to be run (in
post_get_flags). This is very slow before the code relocation on some boards
with a slow I2C EEPROM for environement variables.

This patch adds a check at the beginning of post_run to avoid to run
post_get_flags (which may be slow on some boards for the above reason) for some
given POST flags if no tests are defined with these flags (for instance, no need
to run POST tests with POST_ROM if no tests for POST_ROM are defined).

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
cc: Holger Brunck <holger.brunck@keymile.com>
cc: Wolfgang Denk <wd@denx.de>
---
 post/post.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)
Wolfgang Denk - July 26, 2011, 3:44 p.m.
Dear Valentin Longchamp,

In message <1311602833-29320-1-git-send-email-valentin.longchamp@keymile.com> you wrote:
> Some boards have the environment variables defined in a slow EEPROM. post_run
> accesses these environment variables to define which tests have to be run (in
> post_get_flags). This is very slow before the code relocation on some boards
> with a slow I2C EEPROM for environement variables.
> 
> This patch adds a check at the beginning of post_run to avoid to run
> post_get_flags (which may be slow on some boards for the above reason) for some
> given POST flags if no tests are defined with these flags (for instance, no need
> to run POST tests with POST_ROM if no tests for POST_ROM are defined).

I think I understand what you are tying to do, but I'm sorry, I cannot
understand your code.

> diff --git a/post/post.c b/post/post.c
> index 1b7f2aa..d97f330 100644
> --- a/post/post.c
> +++ b/post/post.c
> @@ -169,6 +169,18 @@ static void post_bootmode_test_off (void)
>  	post_word_store (word);
>  }
>  
> +static int post_test_defined(int flags)
> +{
> +	int i;
> +	int run_flags = flags & ~(POST_RAM | POST_ROM);

Why are you masking out the POST_RAM and POST_ROM bits here?

Assuming your intention is to shortcut post_run(), then this should
be done always, independent of the mode.

> +	for (i = 0; i < post_list_size; i++)
> +		if (post_list[i].flags & run_flags)
> +			return 1;

Please try if omitting run_flags and testing 

		if (post_list[i].flags & flags)

instead still has the intended effect.  If so, then please resubmit a
cleaned up patch.

If not, then please explain in detail how your patch is supposed to
work.

Thanks.

Best regards,

Wolfgang Denk
Valentin Longchamp - July 27, 2011, 9:54 a.m.
On 07/26/2011 05:44 PM, Wolfgang Denk wrote:
> 
> Please try if omitting run_flags and testing 
> 
> 		if (post_list[i].flags & flags)
> 
> instead still has the intended effect.  If so, then please resubmit a
> cleaned up patch.
> 
> If not, then please explain in detail how your patch is supposed to
> work.
> 

I agree with you and will test this version and if it works resubmit the patch.

Patch

diff --git a/post/post.c b/post/post.c
index 1b7f2aa..d97f330 100644
--- a/post/post.c
+++ b/post/post.c
@@ -169,6 +169,18 @@  static void post_bootmode_test_off (void)
 	post_word_store (word);
 }
 
+static int post_test_defined(int flags)
+{
+	int i;
+	int run_flags = flags & ~(POST_RAM | POST_ROM);
+
+	for (i = 0; i < post_list_size; i++)
+		if (post_list[i].flags & run_flags)
+			return 1;
+
+	return 0;
+}
+
 static void post_get_flags (int *test_flags)
 {
 	int  flag[] = {  POST_POWERON,   POST_NORMAL,   POST_SLOWTEST,
@@ -301,6 +313,9 @@  int post_run (char *name, int flags)
 	unsigned int i;
 	int test_flags[POST_MAX_NUMBER];
 
+	if (!post_test_defined(flags))
+		return 0;
+
 	post_get_flags (test_flags);
 
 	if (name == NULL) {