From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerry Van Baren Date: Fri, 25 May 2007 12:56:13 -0400 Subject: [U-Boot-Users] Imminent u-boot-fdt pull request In-Reply-To: <20070525105833.3d1dbd0e.kim.phillips@freescale.com> References: <46526C2E.3070100@gmail.com> <20070524222612.381a3587.kim.phillips@freescale.com> <4656AC13.2010300@grandegger.com> <20070525105833.3d1dbd0e.kim.phillips@freescale.com> Message-ID: <4657152D.7030606@smiths-aerospace.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Kim Phillips wrote: > On Fri, 25 May 2007 11:27:47 +0200 > Wolfgang Grandegger wrote: > >> Kim Phillips wrote: >>> On Tue, 22 May 2007 00:06:06 -0400 >>> Jerry Van Baren wrote: >>> >>>> Dear all, >>>> >>>> Please review the u-boot-fdt changes. I intend to request Wolfgang pull >>>> the pending changes soon. All of the changes (except the last trivial >>> I have properties being placed at the wrong level, timebase- and >>> bus-frequencies not being updated, there are multiple copies of the >>> code, and, it is hard to read - I saw a line length of 139! >> Could you be more specific, please? Preferably by adding comments to the > > sorry, my comments were not specific, it's because the nature of the > problem as I see it is general, and I decided to send a quick note to > save Wolfgang from them ;) > >> patch. And are these problems related to the patch? >> > > This is current top-of-fdt-tree behaviour: > > => fdt addr $fdtaddr > => fdt chosen > => fdt print /cpus > cpus { > clock-frequency=<1f78a400>; // should be one level down > #address-cells=<00000001>; > #size-cells=<00000000>; > PowerPC,8360 at 0 { > device_type="cpu"; > reg=<00000000>; > d-cache-line-size=<00000020>; > i-cache-line-size=<00000020>; > d-cache-size=<00008000>; > i-cache-size=<00008000>; > timebase-frequency=<00000000>; // not updated > bus-frequency=<00000000>; // not updated > clock-frequency=<00000000>; // not updated, see above > 32-bit; > }; > }; > > I wouldn't call this quality material. > > I'll post (specific) patches once I get a kernel booting, unless someone > beats me to it. > > Kim Hi Kim, FWIIW, that is in the (infamous) common/fdt_support.c. It was adapted from ft_build.c. It looks like I lost a OF_CPU in the constructed string when I was adapting. Original code: p = ft_get_prop(blob, "/cpus/" OF_CPU "/clock-frequency", &len); Comments: 1) It looks like you are missing the OF_CPU definition, or I'm not pulling in the right .h file 2) Some of the code is in fdt_support.c fdt_chosen(). The stuff messing with CPU-specific values does NOT belong in fdt_chosen(), it should go in cpu/mpc83xx/cpu.c 3) /cpu/PowerPC,8360 at 0/bus-frequency is properly set in cpu/mpc83xx/cpu.c ft_cpu_setup(). The table of nodes and properties correctly has "/cpus/" OF_CPU, "bus-frequency", which seems to indicate that I don't have the right header pulled in to define OF_CPU. The defines from point #2 Screwed up code in fdt_chosen() #ifdef OF_TBCLK nodeoffset = fdt_find_node_by_path (fdt, "/cpus/" OF_CPU "/timebase-frequency"); if (nodeoffset >= 0) { clock = cpu_to_be32(OF_TBCLK); err = fdt_setprop(fdt, nodeoffset, "clock-frequency", &clock, 4); The code in question is in