[U-Boot-Users] [PATCH libfdt 1/3] Fix errors when CONFIG_OF_LIBFDT is enabled

[Kim Phillips <kim.phillips@freescale.com>]

On Sat, 26 May 2007 08:50:58 -0400
Jerry Van Baren <gvb.uboot@gmail.com> wrote:

> Several node strings were not correct (trailing slashes and properties
>   in the strings)
> Added setting of the timebase-frequency (if OF_TBCLK is defined).
> Added more error printouts.
> 
> Signed-off-by: Gerald Van Baren <vanbaren@cideas.com>
> ---
> Kim:
> 
> Please ack/nack.
> 
> Thanks,
> gvb
> 
>  cpu/mpc83xx/cpu.c |   63 +++++++++++++++++++++++++++++++++++++----------------
>  1 files changed, 44 insertions(+), 19 deletions(-)
> 
> diff --git a/cpu/mpc83xx/cpu.c b/cpu/mpc83xx/cpu.c
> index 01d4dd6..42c32c3 100644
> --- a/cpu/mpc83xx/cpu.c
> +++ b/cpu/mpc83xx/cpu.c
> @@ -337,7 +337,7 @@ static int fdt_set_eth0(void *fdt, int nodeoffset, const char *name, bd_t *bd)
>  	if (fdt_get_property(fdt, nodeoffset, name, 0)) {
>  		return fdt_setprop(fdt, nodeoffset, name, bd->bi_enetaddr, 6);
>  	}
> -	return -FDT_ERR_NOTFOUND;
> +	return 0;	/* doesn't exist, but is not a problem. */
>  }
>  #ifdef CONFIG_HAS_ETH1
>  /* second onboard ethernet port */
> @@ -349,7 +349,7 @@ static int fdt_set_eth1(void *fdt, int nodeoffset, const char *name, bd_t *bd)
>  	if (fdt_get_property(fdt, nodeoffset, name, 0)) {
>  		return fdt_setprop(fdt, nodeoffset, name, bd->bi_enet1addr, 6);
>  	}
> -	return -FDT_ERR_NOTFOUND;
> +	return 0;	/* doesn't exist, but is not a problem. */
>  }
>  #endif
>  #ifdef CONFIG_HAS_ETH2
> @@ -362,7 +362,7 @@ static int fdt_set_eth2(void *fdt, int nodeoffset, const char *name, bd_t *bd)
>  	if (fdt_get_property(fdt, nodeoffset, name, 0)) {
>  		return fdt_setprop(fdt, nodeoffset, name, bd->bi_enet2addr, 6);
>  	}
> -	return -FDT_ERR_NOTFOUND;
> +	return 0;	/* doesn't exist, but is not a problem. */
>  }
>  #endif
>  #ifdef CONFIG_HAS_ETH3
> @@ -375,7 +375,7 @@ static int fdt_set_eth3(void *fdt, int nodeoffset, const char *name, bd_t *bd)
>  	if (fdt_get_property(fdt, nodeoffset, name, 0)) {
>  		return fdt_setprop(fdt, nodeoffset, name, bd->bi_enet3addr, 6);
>  	}
> -	return -FDT_ERR_NOTFOUND;
> +	return 0;	/* doesn't exist, but is not a problem. */

This comment probably applies to the commit log more than in the code. 
In the code, you mean it's not fatal?  People can see that in the
'return 0'.  Remove the comment?

>  }
>  #endif
>  
> @@ -389,6 +389,19 @@ static int fdt_set_busfreq(void *fdt, int nodeoffset, const char *name, bd_t *bd
>  	return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
>  }
>  
> +#ifdef OF_TBCLK
> +static int fdt_set_tbfreq(void *fdt, int nodeoffset, const char *name, bd_t *bd)
> +{
> +	u32  tmp;
> +	/*
> +	 * Create or update the property.
> +	 */
> +	tmp = cpu_to_be32(OF_TBCLK);
> +	return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
> +}
> +#endif
> +
> +
>  /*
>   * Fixups to the fdt.  If "create" is TRUE, the node is created
>   * unconditionally.  If "create" is FALSE, the node is updated
> @@ -399,19 +412,25 @@ static const struct {
>  	char *prop;
>  	int (*set_fn)(void *fdt, int nodeoffset, const char *name, bd_t *bd);
>  } fixup_props[] = {
> +#ifdef OF_TBCLK
>  	{	"/cpus/" OF_CPU,
> -		 "bus-frequency",
> -		fdt_set_busfreq
> +		"timebase-frequency",
> +		fdt_set_tbfreq
>  	},
> -	{	"/cpus/" OF_SOC,
> +#endif
> +	{	"/cpus/" OF_CPU,
>  		"bus-frequency",
>  		fdt_set_busfreq
>  	},
> -	{	"/" OF_SOC "/serial at 4500/",
> +	{	"/cpus/" OF_CPU,
> +		"clock-frequency",
> +		fdt_set_busfreq
> +	},
> +	{	"/" OF_SOC "/serial at 4500",
>  		"clock-frequency",
>  		fdt_set_busfreq
>  	},
> -	{	"/" OF_SOC "/serial at 4600/",
> +	{	"/" OF_SOC "/serial at 4600",
>  		"clock-frequency",
>  		fdt_set_busfreq
>  	},
> @@ -437,20 +456,20 @@ static const struct {

you missed the TSEC entries here (there are unterminated strings
there!).  

>  #endif
>  #ifdef CONFIG_UEC_ETH1
>  #if CFG_UEC1_UCC_NUM == 0  /* UCC1 */
> -	{	"/" OF_QE "/ucc at 2000/mac-address",
> +	{	"/" OF_QE "/ucc at 2000",
>  		"mac-address",
>  		fdt_set_eth0
>  	},
> -	{	"/" OF_QE "/ucc at 2000/mac-address",
> +	{	"/" OF_QE "/ucc at 2000",
>  		"local-mac-address",
>  		fdt_set_eth0
>  	},
>  #elif CFG_UEC1_UCC_NUM == 2  /* UCC3 */
> -	{	"/" OF_QE "/ucc at 2200/mac-address",
> +	{	"/" OF_QE "/ucc at 2200",
>  		"mac-address",
>  		fdt_set_eth0
>  	},
> -	{	"/" OF_QE "/ucc at 2200/mac-address",
> +	{	"/" OF_QE "/ucc at 2200",
>  		"local-mac-address",
>  		fdt_set_eth0
>  	},
> @@ -458,20 +477,20 @@ static const struct {
>  #endif
>  #ifdef CONFIG_UEC_ETH2
>  #if CFG_UEC2_UCC_NUM == 1  /* UCC2 */
> -	{	"/" OF_QE "/ucc at 3000/mac-address",
> +	{	"/" OF_QE "/ucc at 3000",
>  		"mac-address",
>  		fdt_set_eth1
>  	},
> -	{	"/" OF_QE "/ucc at 3000/mac-address",
> +	{	"/" OF_QE "/ucc at 3000",
>  		"local-mac-address",
>  		fdt_set_eth1
>  	},
>  #elif CFG_UEC1_UCC_NUM == 3  /* UCC4 */
> -	{	"/" OF_QE "/ucc at 3200/mac-address",
> +	{	"/" OF_QE "/ucc at 3200",
>  		"mac-address",
>  		fdt_set_eth1
>  	},
> -	{	"/" OF_QE "/ucc at 3200/mac-address",
> +	{	"/" OF_QE "/ucc at 3200",
>  		"local-mac-address",
>  		fdt_set_eth1
>  	},
> @@ -489,12 +508,18 @@ ft_cpu_setup(void *blob, bd_t *bd)
>  	for (j = 0; j < (sizeof(fixup_props) / sizeof(fixup_props[0])); j++) {
>  		nodeoffset = fdt_find_node_by_path(fdt, fixup_props[j].node);
>  		if (nodeoffset >= 0) {
> -			err = fixup_props[j].set_fn(blob, nodeoffset, fixup_props[j].prop, bd);
> +			err = fixup_props[j].set_fn(
> +				blob, nodeoffset, fixup_props[j].prop, bd);
>  			if (err < 0)
> -				printf("set_fn/libfdt: %s %s returned %s\n",
> +				printf("set_fn(%s, %s) returned %s\n",

naming functions in user messages is more a property of debug code; if
you want, either make it debug code, or say something like "problem
setting %s/%s: %s".  Probably best to make it debug code since I don't
see this error occurring often.

>  					fixup_props[j].node,
>  					fixup_props[j].prop,
>  					fdt_strerror(err));
> +		} else {
> +			printf("fdt_find_node_by_path(%s) returned %s\n",
> +				fixup_props[j].node,
> +				fdt_strerror(nodeoffset));
> +			

"couldn't find %s: %s"

Please respin, base on top of u-boot-testing (you might create a
conflict with another TSEC patch if you don't), and since it applies to
cpu/mpc83xx/cpu.c, submit through the mpc83xx tree (i.e. prepend the
string 'mpc83xx: ' in your subject, and wait for me to ask Wolfgang to
pull).

This seems to have fixed most of the functional issues; thanks for that.
Please read linux-2.6/Documentation/CodingStyle if you haven't already. 
Thank you!

Kim