From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerry Van Baren Date: Thu, 30 Aug 2007 07:51:52 -0400 Subject: [U-Boot-Users] [PATCH] RFC: generic property fixup mechanism for LIBFDT In-Reply-To: <46D62E23.9060903@gmail.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> <46CDBD5A.3020106@smiths-aerospace.com> <46D62E23.9060903@gmail.com> Message-ID: <46D6AF58.2060408@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Jerry Van Baren wrote: > Grant Likely wrote: >> On 8/23/07, Jerry Van Baren wrote: >>> 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. >> Yes, this is the icky bit; but I still argue that there is a more >> compact way to do it. Table driven is nice when it works; but (as I >> also mentioned in my original comment) whie it doesn't its probably >> better to just fall back to something programatic [snip pseudo code any my reply] >> void >> 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. Best regards from the lovely Sault Ste. Marie, gvb