[U-Boot-Users] [PATCH] RFC: generic property fixup mechanism for LIBFDT

[Jerry Van Baren <vanbargw@gmail.com>]

Jerry Van Baren wrote:
> Grant Likely wrote:
>> On 8/23/07, Jerry Van Baren <gerald.vanbaren@smiths-aerospace.com> wrote:
>>> The first half of Grant's comment was:
>>>  > These 4 functions are pretty close to identical (except for the
>>>  > parameter to cpu_to_be32()).  Surely there is a more compact way to do
>>>  > this.
>>>
>>> My answer is "no, the whole reason there is one line different is
>>> because it *has to be*."  As the French say about the "Y" chromosome
>>> "...and viva la difference."  My first attempt, and Bartlomiej's
>>> proposed patch, tried (tries) to put the value in the table, but it is a
>>> *WORSE* solution because you have the problem with byteswapping or not
>>> and different sizes of values.  The scorecard for putting the values in
>>> the tables and also having it maintainable is 0 for 2.
>> Yes, this is the icky bit; but I still  argue that there is a more
>> compact way to do it.  Table driven is nice when it works; but (as I
>> also mentioned in my original comment) whie it doesn't its probably
>> better to just fall back to something programatic

[snip pseudo code any my reply]

>> void
>> ft_cpu_setup(void *blob, bd_t *bd)
>> {
>>         fixup_int_prop(blob, "/cpus/" OF_CPU, "timebase-frequency", OF_TBCLK);
>>         fixup_int_prop(blob, "/cpus/" OF_CPU, "bus-frequency", bd->bi_busfreq);
>>         fixup_int_prop(blob, "/cpus/" OF_CPU, "clock-frequency",
>> bd->bi_intfreq);
>>         fixup_int_prop(blob, "/cpus/" OF_CPU, "bus-frequency", bd->ipbfreq);
>>         fixup_prop(blob, "/" OF_SOC "/ethernet at 3000", "mac-address",
>>                    bd->bi_enetaddr, 6);
	fixup_mac_prop(blob, "/" OF_SOC "/ethernet at 3000", "mac-address",
		bd->bi_enetaddr, 1);
>>         fixup_prop(blob, "/" OF_SOC "/ethernet at 3000", "local-mac-address",
>>                    bd->bi_enetaddr, 6);
	fixup_mac_prop(blob, "/" OF_SOC "/ethernet at 3000",
		"local-mac-address", bd->bi_enetaddr, 0);
>> }
>>
>> This adds 2 helper functions instead of 5, and one of them is the same
>> path for all of the fixups.  Drops the table approach entirely due to
>> the problems with extracting values out of bd.  (Which is what I meant
>> in the third part of my original comment)
> 
> I think 2 is too optimistic, but it is still possible this approach 
> would be better than the current 5.  The above looks better, but I claim 
> only because it is oversimplified - I contend the 2 helper functions 
> will expand to 5 functions that basically are the current "setter" 
> functions.  On the other hand, maybe not.  It is worth trying.

[more snippage]

>> The 2 new helpers could also be generalized for use by all boards.
> 
> s/2/n/ (where n is probably 5) and I would agree 100%. :-/
> 
>> Cheers,
>> g.

Thinking about it and a quick glance (I'm running remotely), I think we 
could do the job with three "setter" functions based on Grant's 
proposal, which would be a Good Thing[tm].

static int fixup_int_prop(void *fdt, char *node, char *prop, u32 val)
- Like Grant proposes, note it does a cpu_to_be32()

static int fixup_mac_prop(void *fdt, char *node, char *prop, void *val, 
int force)
- Note I replaced "size" with "force".  We know the size (6).  The 
"force" flag (better name, anyone?) would be whether the MAC addr should 
be created if it _doesn't_ exist.

Best regards from the lovely Sault Ste. Marie,
gvb

<https://webcam.crrel.usace.army.mil/soo/camera2.html>
<http://maps.google.com/maps?q=sault+ste+marie&ie=UTF8&oe=utf-8&ll=46.503429,-84.351032&spn=0.005952,0.015814&t=h&z=16&om=1>