diff mbox

[U-Boot] generic_board: do not set gd->fdt_blob unless CONFIG_OF_CONTROL=y

Message ID 1410014340-13644-1-git-send-email-yamada.m@jp.panasonic.com
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Masahiro Yamada Sept. 6, 2014, 2:39 p.m. UTC
gd->fdt_blob is used for FDT control of U-Boot.
If CONFIG_OF_CONTROL is not defined, it is useless.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
Cc: Simon Glass <sjg@chromium.org>
---

 common/board_f.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Simon Glass Sept. 6, 2014, 4:33 p.m. UTC | #1
Hi Masahiro,

On 6 September 2014 08:39, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> gd->fdt_blob is used for FDT control of U-Boot.
> If CONFIG_OF_CONTROL is not defined, it is useless.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>
>  common/board_f.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/common/board_f.c b/common/board_f.c
> index 4ece2b6..b2ab63f 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -341,21 +341,23 @@ static int setup_ram_buf(void)
>
>  static int setup_fdt(void)
>  {
> -#ifdef CONFIG_OF_EMBED
> +#ifdef CONFIG_OF_CONTROL
> +# ifdef CONFIG_OF_EMBED
>         /* Get a pointer to the FDT */
>         gd->fdt_blob = __dtb_dt_begin;
> -#elif defined CONFIG_OF_SEPARATE
> +# elif defined CONFIG_OF_SEPARATE
>         /* FDT is at end of image */
>         gd->fdt_blob = (ulong *)&_end;
> -#elif defined(CONFIG_OF_HOSTFILE)
> +# elif defined(CONFIG_OF_HOSTFILE)
>         if (read_fdt_from_file()) {
>                 puts("Failed to read control FDT\n");
>                 return -1;
>         }
> -#endif
> +# endif

I don't think the above has any effect, since CONFIG_OF_EMBED,
CONFIG_OF_SEPARATE and CONFIG_OF_HOSTFILE should not be set if
CONFIG_OF_CONTROL is not set.

>         /* Allow the early environment to override the fdt address */
>         gd->fdt_blob = (void *)getenv_ulong("fdtcontroladdr", 16,
>                                                 (uintptr_t)gd->fdt_blob);

You effectively just remove this statement. Is it worth it for the
extra #ifdefs?

Regards,
Simon
Masahiro Yamada Sept. 6, 2014, 5:09 p.m. UTC | #2
Hi Simon,


2014-09-07 1:33 GMT+09:00 Simon Glass <sjg@chromium.org>:
>>         /* Allow the early environment to override the fdt address */
>>         gd->fdt_blob = (void *)getenv_ulong("fdtcontroladdr", 16,
>>                                                 (uintptr_t)gd->fdt_blob);
>
> You effectively just remove this statement. Is it worth it for the
> extra #ifdefs?

IMHO, yes, I think meaningless code should not be compiled.
Simon Glass Sept. 8, 2014, 6:41 p.m. UTC | #3
Hi Masahiro,

On 6 September 2014 11:09, Masahiro YAMADA <yamada.m@jp.panasonic.com> wrote:
> Hi Simon,
>
>
> 2014-09-07 1:33 GMT+09:00 Simon Glass <sjg@chromium.org>:
>>>         /* Allow the early environment to override the fdt address */
>>>         gd->fdt_blob = (void *)getenv_ulong("fdtcontroladdr", 16,
>>>                                                 (uintptr_t)gd->fdt_blob);
>>
>> You effectively just remove this statement. Is it worth it for the
>> extra #ifdefs?
>
> IMHO, yes, I think meaningless code should not be compiled.

OK. While I don't like the extra #ifdefs for small benefit, I agree
there is not much sense in providing this FDT unless U-Boot is
actually using it.

Acked-by: Simon Glass <sjg@chromium.org>

Regards,
Simon
Tom Rini Sept. 17, 2014, 12:46 a.m. UTC | #4
On Sat, Sep 06, 2014 at 11:39:00PM +0900, Masahiro Yamada wrote:

> gd->fdt_blob is used for FDT control of U-Boot.
> If CONFIG_OF_CONTROL is not defined, it is useless.
> 
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Simon Glass <sjg@chromium.org>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!
diff mbox

Patch

diff --git a/common/board_f.c b/common/board_f.c
index 4ece2b6..b2ab63f 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -341,21 +341,23 @@  static int setup_ram_buf(void)
 
 static int setup_fdt(void)
 {
-#ifdef CONFIG_OF_EMBED
+#ifdef CONFIG_OF_CONTROL
+# ifdef CONFIG_OF_EMBED
 	/* Get a pointer to the FDT */
 	gd->fdt_blob = __dtb_dt_begin;
-#elif defined CONFIG_OF_SEPARATE
+# elif defined CONFIG_OF_SEPARATE
 	/* FDT is at end of image */
 	gd->fdt_blob = (ulong *)&_end;
-#elif defined(CONFIG_OF_HOSTFILE)
+# elif defined(CONFIG_OF_HOSTFILE)
 	if (read_fdt_from_file()) {
 		puts("Failed to read control FDT\n");
 		return -1;
 	}
-#endif
+# endif
 	/* Allow the early environment to override the fdt address */
 	gd->fdt_blob = (void *)getenv_ulong("fdtcontroladdr", 16,
 						(uintptr_t)gd->fdt_blob);
+#endif
 	return 0;
 }