Kim Phillips wrote:
On Wed, 22 Aug 2007 19:16:16 -0400 Jerry Van Baren vanbargw@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