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

[Jerry Van Baren]

Kim Phillips wrote:
> On Wed, 22 Aug 2007 19:16:16 -0400
> Jerry Van Baren <vanbargw at gmail.com> wrote:
>> Kim Phillips wrote:

[snip]

>>> What about having a list of functions to call?  It could be called
>>> something like fdt_update_sequence, and be similar in implementation to
>>> lib_ppc/board.c's init_sequence.  It could also reside in lib_ppc,
>>> esp. since, e.g. all powerpc's have a timebase, and to help naturally
>>> enforce a certain level of consistency among powerpc board code.  The
>>> updater/fixup/"setter" functions would only need be passed the pointer
>>> to the base of the blob to update.
>> We _have_ "a list of functions to call".  That is what is done with the 
>> 83xx which is what I advocated and what Bartlomiej had before rewriting 
>> it back into a table without functions.
> 
> yeah, I meant /just/ a list of functions to call, without all the extra
> fluff in both the existing 83xx and Bartlomiej's implementations.
> 
> Kim

Well, the table has the node, the property, and the "setter" routine. 
This allows us to re-use the "setter" routine where possible.  Where 
there are multiple setter routines is where, for instance, the property 
is based on a different clock or a different MAC address.  In those 
cases the setters are identical except for one line, but that one line 
is *why* they are different.

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.

Note that the original code is a repetitive in-line coding of what is 
inside the "setter" functions, with 100% duplication of code.  My change 
to a table driven method with "setter" functions drops the 100% 
duplication of code to less, probably 80%.

I would lay down a challenge to make the code both more compact *and* 
not lose maintainability.

"Make everything as simple as possible, but not simpler."
   -- Albert Einstein <http://www.quotedb.com/quotes/1360>

The second half of Grant's comment was:
 > In addition, these function don't really contain anything that
 > screams out "5xxx only!". Can this be common support code usable by
 > all boards?

To this I simply say "yup."  To date, this has been 83xx only (mostly?). 
  With more PPC CPU types comes the opportunity to identify and abstract 
the common "setter" functions.

*This* is where Bartlomiej should start IMHO.  It is where the payback is.

Best regards,
gvb