[U-Boot-Users] Imminent u-boot-fdt pull request

[Jerry Van Baren <gerald.vanbaren@smiths-aerospace.com>]

Kim Phillips wrote:
> On Fri, 25 May 2007 11:27:47 +0200
> Wolfgang Grandegger <wg@grandegger.com> wrote:
> 
>> Kim Phillips wrote:
>>> On Tue, 22 May 2007 00:06:06 -0400
>>> Jerry Van Baren <gvb.uboot@gmail.com> 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

(Aaargh, hit the wrong button.  Sorry for the line noise.)

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) Hmm, the OF_CPU definition comes from include/config.h and I'm not 
pulling it in.  Grrrrr.

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",
Ooops, another config.h missing.  Double grrrrr.  Also, the CPU-specific 
fixups from fdt_chosen() (point #2) should be moved here.

I'll clean this up unless someone beats me to it.  :-/

Best regards,
gvb