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

[Bartlomiej Sieka <tur@semihalf.com>]

Jerry Van Baren wrote:
> Jerry Van Baren wrote:
>> Grant Likely wrote:
[...]
>>> 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.

How about having two functions lite this:

static int fixup_int_prop(void *fdt, char *node, char *prop, u32 val, 
int create);

static int fixup_str_prop(void *fdt, char *node, char *prop, char *val, 
int len, int create)

?

The last argument ('create' i.e., Jerry's 'force') tells whether the 
property should be created if it doesn't exist. fixup_str_prop() (a 
better name?) takes a len argument just in case we ever wanted to set 
properties with length different than the MAC address' length.

The above functions are generic and can be used by all boards; the 
following specific code would go into ft_cpu_setup():

fixup_int_prop(blob, "/cpus/" OF_CPU, "timebase-frequency", OF_TBCLK, 1);
fixup_int_prop(blob, "/cpus/" OF_CPU, "bus-frequency", bd->bi_busfreq, 1);
fixup_int_prop(blob, "/cpus/" OF_CPU, "clock-frequency", bd->bi_intfreq, 1);
fixup_int_prop(blob, "/cpus/" OF_CPU, "bus-frequency", bd->ipbfreq, 1);
fixup_str_prop(blob, "/" OF_SOC "/ethernet at 3000", "mac-address", 
bd->bi_enetaddr, 6, 0);
fixup_str_prop(blob, "/" OF_SOC "/ethernet at 3000", "local-mac-address", 
bd->bi_enetaddr, 6, 0);


If in the future there's a need for setting a property other than int or 
char[], a function similar to the two ones above can be added.

Regards,
Bartlomiej