Kim Phillips wrote:
On Wed, 22 Aug 2007 09:18:20 -0400 Jerry Van Baren gerald.vanbaren@smiths-aerospace.com wrote:
IMO your proposal is not acceptable. Follow Grant's advice and the current cpu/mpc83xx/cpu.c methodology.
? Bartlomiej's first patch followed the 83xx 'methodology', which Grant commented on, which is what Bartlomiej tried to fix here. Maybe I missed something, but anyway, now that the window has closed, we can arrange to fix libfdt support for all powerpc boards for the next release.
Well, perhaps Grant can speak to the issue better than me, but I read Grant's reply as a request to coelesce the four "setter" functions into one, not to remove all setter functions and change it back to the original table mechanism. IMHO, using hardcoded [0] [1]... indexes is MUCH MORE EVIL than having four similar setter functions.
While I agree that extra setter functions are undesirable, I don't know if Grant's suggestion can be achieved in a way better than the current way (that is a challenge ;-).
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.
If the "setter" functions can be segregated into general PPC ones and CPU-specific ones, that would be great. If the number of "setter" functions can be reduced without degrading reliability etc, that would be greate.
The update functions need to be rewritten to be independent of the OF_CPU, OF_SOC, OF_QE, OF_STDOUT_PATH defines. Those should all be removed (OF_TBCLK might be a bit harder to get rid of though -- implement TBCLK_DIVISOR instead?). btw, Linux is changing the name of the soc node from, e.g. "soc8360", to simply "soc", and u-boot needs to handle transitions like these better. Instead of adding support for both name types, the update functions can handle these situations by using some of the higher level functions available in libfdt, such as fdt_find_node_by_type or even fdt_find_compatible_node.
Yes, that needs to go on the list. Done. http://www.denx.de/wiki/view/UBoot/UBootFdtInfo#ToDo
Best regards, gvb