From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerry Van Baren Date: Thu, 23 Aug 2007 13:01:14 -0400 Subject: [U-Boot-Users] [PATCH] RFC: generic property fixup mechanism for LIBFDT In-Reply-To: <20070823112404.92bb25a0.kim.phillips@freescale.com> References: <20070802182403.GA27073@frozen.semihalf.com> <20070803090755.GB793@frozen.semihalf.com> <20070822115424.GA27378@frozen.semihalf.com> <46CC379C.7000003@smiths-aerospace.com> <20070822160915.5cf9e116.kim.phillips@freescale.com> <46CCC3C0.8050800@gmail.com> <20070823112404.92bb25a0.kim.phillips@freescale.com> Message-ID: <46CDBD5A.3020106@smiths-aerospace.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Kim Phillips wrote: > On Wed, 22 Aug 2007 19:16:16 -0400 > Jerry Van Baren 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 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