[PATCH 1/2] fdt: Avoid early panic() when there is no FDT present

[PATCH 1/2] fdt: Avoid early panic() when there is no FDT present

[Simon Glass]

CONFIG_OF_CONTROL requires a valid device tree. However, we cannot call
panic() before the console is set up since the message does not appear,
and we get a silent failure.

Remove the panic from fdtdec_check_fdt() and provide a new function to
prepare the fdt for use. This will be called after the console is ready.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 include/fdtdec.h |   17 +++++++++++++++--
 lib/fdtdec.c     |   24 +++++++++++++++++++-----
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/include/fdtdec.h b/include/fdtdec.h
index 766e0bd..d45acbe 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -157,8 +157,21 @@ s32 fdtdec_get_int(const void *blob, int node, const char *prop_name,
 int fdtdec_get_is_enabled(const void *blob, int node);
 
 /**
- * Checks whether we have a valid fdt available to control U-Boot, and panic
- * if not.
+ * Make sure we have a valid fdt available to control U-Boot.
+ *
+ * If not, a message is printed to the console if the console is ready.
+ *
+ * @return 0 if all ok, -1 if not
+ */
+int fdtdec_prepare_fdt(void);
+
+/**
+ * Checks that we have a valid fdt available to control U-Boot.
+
+ * However, if not then for the moment nothing is done, since this function
+ * is called too early to panic().
+ *
+ * @returns 0
  */
 int fdtdec_check_fdt(void);
 
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 9241d13..0fb6d17 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -276,17 +276,31 @@ int fdtdec_add_aliases_for_id(const void *blob, const char *name,
 	return num_found;
 }
 
+int fdtdec_check_fdt(void)
+{
+	/*
+	 * We must have an FDT, but we cannot panic() yet since the console
+	 * is not ready. So for now, just assert(). Boards which need an early
+	 * FDT (prior to console ready) will need to make their own
+	 * arrangements and do their own checks.
+	 */
+	assert(!fdtdec_prepare_fdt());
+	return 0;
+}
+
 /*
  * This function is a little odd in that it accesses global data. At some
  * point if the architecture board.c files merge this will make more sense.
  * Even now, it is common code.
  */
-int fdtdec_check_fdt(void)
+int fdtdec_prepare_fdt(void)
 {
-	/* We must have an fdt */
-	if (((uintptr_t)gd->fdt_blob & 3) || fdt_check_header(gd->fdt_blob))
-		panic("No valid fdt found - please append one to U-Boot\n"
-			"binary or define CONFIG_OF_EMBED\n");
+	if (((uintptr_t)gd->fdt_blob & 3) || fdt_check_header(gd->fdt_blob)) {
+		printf("No valid FDT found - please append one to U-Boot "
+			"binary, use u-boot-dtb.bin or define "
+			"CONFIG_OF_EMBED\n");
+		return -1;
+	}
 	return 0;
 }
 
-- 
1.7.7.3

Re: [PATCH 1/2] fdt: Avoid early panic() when there is no FDT present

[Tom Warren]

Simon,

> -----Original Message-----
> From: Simon Glass [mailto:sjg@chromium.org]
> Sent: Wednesday, March 28, 2012 1:08 PM
> To: U-Boot Mailing List
> Cc: Tom Warren; Stephen Warren; Albert Aribaud; Simon Glass; Jerry Van
> Baren; Devicetree Discuss
> Subject: [PATCH 1/2] fdt: Avoid early panic() when there is no FDT present
> 
> CONFIG_OF_CONTROL requires a valid device tree. However, we cannot call
> panic() before the console is set up since the message does not appear, and
> we get a silent failure.
> 
> Remove the panic from fdtdec_check_fdt() and provide a new function to
> prepare the fdt for use. This will be called after the console is ready.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Tested-by: Tom Warren <twarren@nvidia.com>
Acked-by: Tom Warren <twarren@nvidia.com>

> ---
>  include/fdtdec.h |   17 +++++++++++++++--
>  lib/fdtdec.c     |   24 +++++++++++++++++++-----
>  2 files changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/include/fdtdec.h b/include/fdtdec.h index 766e0bd..d45acbe
> 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -157,8 +157,21 @@ s32 fdtdec_get_int(const void *blob, int node, const
> char *prop_name,  int fdtdec_get_is_enabled(const void *blob, int node);
> 
>  /**
> - * Checks whether we have a valid fdt available to control U-Boot, and
> panic
> - * if not.
> + * Make sure we have a valid fdt available to control U-Boot.
> + *
> + * If not, a message is printed to the console if the console is ready.
> + *
> + * @return 0 if all ok, -1 if not
> + */
> +int fdtdec_prepare_fdt(void);
> +
> +/**
> + * Checks that we have a valid fdt available to control U-Boot.
> +
> + * However, if not then for the moment nothing is done, since this
> + function
> + * is called too early to panic().
> + *
> + * @returns 0
>   */
>  int fdtdec_check_fdt(void);
> 
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 9241d13..0fb6d17 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -276,17 +276,31 @@ int fdtdec_add_aliases_for_id(const void *blob, const
> char *name,
>  	return num_found;
>  }
> 
> +int fdtdec_check_fdt(void)
> +{
> +	/*
> +	 * We must have an FDT, but we cannot panic() yet since the console
> +	 * is not ready. So for now, just assert(). Boards which need an
> early
> +	 * FDT (prior to console ready) will need to make their own
> +	 * arrangements and do their own checks.
> +	 */
> +	assert(!fdtdec_prepare_fdt());
> +	return 0;
> +}
> +
>  /*
>   * This function is a little odd in that it accesses global data. At some
>   * point if the architecture board.c files merge this will make more sense.
>   * Even now, it is common code.
>   */
> -int fdtdec_check_fdt(void)
> +int fdtdec_prepare_fdt(void)
>  {
> -	/* We must have an fdt */
> -	if (((uintptr_t)gd->fdt_blob & 3) || fdt_check_header(gd->fdt_blob))
> -		panic("No valid fdt found - please append one to U-Boot\n"
> -			"binary or define CONFIG_OF_EMBED\n");
> +	if (((uintptr_t)gd->fdt_blob & 3) || fdt_check_header(gd->fdt_blob))
> {
> +		printf("No valid FDT found - please append one to U-Boot "
> +			"binary, use u-boot-dtb.bin or define "
> +			"CONFIG_OF_EMBED\n");
> +		return -1;
> +	}
>  	return 0;
>  }
> 
> --
> 1.7.7.3
-- 
nvpublic

Re: [PATCH 1/2] fdt: Avoid early panic() when there is no FDT present

[Wolfgang Denk]

Dear Simon Glass,

In message <1332965305-21151-1-git-send-email-sjg@chromium.org> you wrote:
> CONFIG_OF_CONTROL requires a valid device tree. However, we cannot call
> panic() before the console is set up since the message does not appear,
> and we get a silent failure.
...
> +int fdtdec_prepare_fdt(void);
> +
> +/**
> + * Checks that we have a valid fdt available to control U-Boot.
> +
> + * However, if not then for the moment nothing is done, since this function
> + * is called too early to panic().
> + *
> + * @returns 0

If the function always returns 0, then it makes no sense to return any
value at all.  Please make it void, then.

> +int fdtdec_check_fdt(void)
> +{
> +	/*
> +	 * We must have an FDT, but we cannot panic() yet since the console
> +	 * is not ready. So for now, just assert(). Boards which need an early
> +	 * FDT (prior to console ready) will need to make their own
> +	 * arrangements and do their own checks.
> +	 */
> +	assert(!fdtdec_prepare_fdt());
> +	return 0;
> +}

Ditto - make that "void fdtdec_check_fdt(void)".

> +int fdtdec_prepare_fdt(void)
...
>  	return 0;
>  }

Ditto - make that "void fdtdec_prepare_fdt(void)".


I have to admit that I do not understand how this patch will help you
anything.  You write "we cannot panic()", but "just assert()".

If the assertion fails, it will call __assert_fail() - which in turn
will just call panic().  So you are out of the frying pan into the
fire.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Life. Don't talk to me about life.      - Marvin the Paranoid Android

Re: [PATCH 1/2] fdt: Avoid early panic() when there is no FDT present

[Simon Glass]

Hi Wolfgang,

On Wed, Mar 28, 2012 at 11:30 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <1332965305-21151-1-git-send-email-sjg@chromium.org> you wrote:
>> CONFIG_OF_CONTROL requires a valid device tree. However, we cannot call
>> panic() before the console is set up since the message does not appear,
>> and we get a silent failure.
> ...
>> +int fdtdec_prepare_fdt(void);
>> +
>> +/**
>> + * Checks that we have a valid fdt available to control U-Boot.
>> +
>> + * However, if not then for the moment nothing is done, since this function
>> + * is called too early to panic().
>> + *
>> + * @returns 0
>
> If the function always returns 0, then it makes no sense to return any
> value at all.  Please make it void, then.

fdtdec_check_fdt() is used in the ARM init sequence, so I need to
follow that signature and return a value. In fact I must return 0 or
the board will hang.

>
>> +int fdtdec_check_fdt(void)
>> +{
>> +     /*
>> +      * We must have an FDT, but we cannot panic() yet since the console
>> +      * is not ready. So for now, just assert(). Boards which need an early
>> +      * FDT (prior to console ready) will need to make their own
>> +      * arrangements and do their own checks.
>> +      */
>> +     assert(!fdtdec_prepare_fdt());
>> +     return 0;
>> +}
>
> Ditto - make that "void fdtdec_check_fdt(void)".
>
>> +int fdtdec_prepare_fdt(void)
> ...
>>       return 0;
>>  }
>
> Ditto - make that "void fdtdec_prepare_fdt(void)".

This fdtdec_prepare_fdt() is intended to provide a way to find out if
there is a valid fdt. The 'return 0' that you have shown is only if it
succeeds - it does actually return -1 on failure. I want a function
which just checks this, and returns the result (rather than dying if
it fails, which the behaviour that in principle I would like in the
init sequence).

>
>
> I have to admit that I do not understand how this patch will help you
> anything.  You write "we cannot panic()", but "just assert()".
>
> If the assertion fails, it will call __assert_fail() - which in turn
> will just call panic().  So you are out of the frying pan into the
> fire.

Yes, but only if DEBUG is turned on in fdtdec. Otherwise we just
continue and panic later when we can report the error.

Regards,
Simon

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> Life. Don't talk to me about life.      - Marvin the Paranoid Android