Kim Phillips wrote:
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
[snip]
#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?
When I wrote it originally, I mistakenly returned -FDT_ERR_NOTFOUND, wanted to make it clear to the next editor that 0 (OK) was intentional.
The ETH* do a set if it exists, do not create if it doesn't exist. Other properties are created if they don't exist. Part of the above comments were to highlight that the different pattern/behavior was intentional.
} #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
Whups, "create" no longer exists. That comment needs to be edited.
@@ -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@4500/",
- { "/cpus/" OF_CPU,
"clock-frequency",fdt_set_busfreq- },
- { "/" OF_SOC "/serial@4500", "clock-frequency", fdt_set_busfreq },
- { "/" OF_SOC "/serial@4600/",
- { "/" OF_SOC "/serial@4600", "clock-frequency", fdt_set_busfreq },
@@ -437,20 +456,20 @@ static const struct {
you missed the TSEC entries here (there are unterminated strings there!).
TSEC entries weren't in the code when I wrote this. Now they are and need to be added. :-/
I don't see any unterminated strings (and it compiles for me). Pasting together strings and macros is nasty.
[snip]
} 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
Will do.
Thanks, gvb