[U-Boot-Users] [PATCH 2/3] Fix fdt_chosen() to call ft_board_setup(), clean up long lines.

[Kim Phillips]

On Tue, 29 May 2007 22:00:25 -0400
Jerry Van Baren <gvb.uboot at gmail.com> wrote:

> Kim Phillips wrote:
> > On Sat, 26 May 2007 08:52:31 -0400
> > Jerry Van Baren <gvb.uboot at gmail.com> wrote:
> > 
<snip>
> >> +#ifdef CONFIG_OF_BOARD_SETUP
> >> +	/*
> >> +	 * ft_board_setup() sets various board-specific properties to
> >> +	 * the proper values.
> >> +	 *
> >> +	 * STRICTLY SPEAKING, this is out of place, but it isn't clear
> >> +	 * where a better place would be.
> >> +	 */
> >> +	ft_board_setup(fdt, bd);
> >> +#endif
> >> +
> > 
> > I think it's fine here, btw.  What are your reservations?
> 
> Not really.  ft_board_setup() is not related at all to creating the 
> /chosen node.  I've been thinking about this and believe the best 
> approach is to create another "fdt" command "fdt boardsetup" that calls 
> ft_board_setup().
> 
> The user can then run each part of the fdt setup separately and watch 
> what is done to the blob:
> 
> fdt boardsetup - updates the board-specific fdt values
> fdt chosen     - creates or updates the /chosen node
> fdt env        - creates the /u-boot-env node
> fdt bd_t       - creates the /bd_t node
> 
> The cmd_bootm.c will then call all four.  Currently it calls the last 3 
> and fdt_chosen() calls fdt_board_setup() inappropriately IMHO.  That is 
> my reservation.
> 
sounds like you've found the better place.

<snip>
> >> @@ -323,7 +346,10 @@ int fdt_bd_t(void *fdt)
> >>  	 */
> >>  	nodeoffset = fdt_add_subnode(fdt, 0, "bd_t");
> >>  	if (nodeoffset < 0) {
> >> -		printf("WARNING fdt_bd_t: could not create the \"/bd_t node\" (libfdt error %s).\n", fdt_strerror(nodeoffset));
> >> +		printf("WARNING fdt_bd_t: "
> >> +			"could not create the \"/bd_t node\" "
> >> +			"(libfdt error %s).\n",
> >> +			fdt_strerror(nodeoffset));
> >>  		printf("libfdt: %s\n", fdt_strerror(nodeoffset));
> > 
> > just in case the user didn't get it the first time, eh?  ;)
> 
> No, the three routines are called separately by fdt * commands, so each 
> could fill the blob's available space separately.
> 
> The bootm commands call the three sequentially and aborts on the first 
> one that fails.  The chosen node could be created successfully but the 
> u-boot-env node could run out of space.
> 
> Did that cover your ;) or did I miss the point?
> 
You're printf'ing the same thing twice.

Kim