From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerry Van Baren Date: Tue, 25 Mar 2008 11:56:07 -0400 Subject: [U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function In-Reply-To: <1206459104.7589.226.camel@gentoo-jocke.transmode.se> References: <200803221114.40956.sr@denx.de> <20080322140107.F088D24A93@gemini.denx.de> <200803250804.13314.sr@denx.de> <47E90A8A.5000607@gmail.com> <1206456102.7589.223.camel@gentoo-jocke.transmode.se> <47E912F3.9060705@gmail.com> <1206459104.7589.226.camel@gentoo-jocke.transmode.se> Message-ID: <47E92097.1000506@ge.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Joakim Tjernlund wrote: > On Tue, 2008-03-25 at 10:57 -0400, Ben Warren wrote: >> Joakim Tjernlund wrote: >>> On Tue, 2008-03-25 at 10:22 -0400, Ben Warren wrote: >>> >>>> Stefan Roese wrote: >>>> >>>>> On Saturday 22 March 2008, Ben Warren wrote: [snip] >>>>> Using Markus's idea, why not use a cpu (platform) specific *and* a board >>>>> specific init function, both with an empty weak alias in the common eth.c >>>>> code: >>>>> >>>>> cpu_eth_init(bis); >>>>> board_eth_init(bis); >>>>> >>>> I thought about this some more, and the problem is that cpu_eth_init() and board_eth_init() >>>> are mutually exclusive, with board_eth_init() having a higher priority. >>>> I think the following will work, but would appreciate some feedback. >>>> >>>> ----- >>>> >>>> int board_eth_init(bd_t *bis) __attribute(weak); >>>> int cpu_eth_init(bd_t *bis) __attribute(weak); >>>> >>>> . >>>> . >>>> . >>>> >>>> if (board_eth_init) >>>> board_eth_init(bis); >>>> else if (cpu_eth_init) >>>> cpu_eth_init(bis); >>>> >>>> ----- >>>> >>>> This gets rid of the pointless aliases and gives precedence to the board-specific initialization. >>>> >>>> >>>> regards, >>>> Ben >>>> >>> I think you must enable full relocation, CONFIG_RELOC_FIXUP_WORKS, to >>> make the "if (board_eth_init)" work. This is just a guess though. >>> >>> Jocke >>> >>> >> Nothing a little testing can't figure out. >> >> thanks, >> Ben > > You could do too: > if (!board_eth_init(bis)) > cpu_eth_init(bis); > > Jocke Per an earlier discussion on how weak functions are implemented, these are not equivalent. Functions marked "weak" *without* a weak implementation become NULL pointers. The code if (board_eth_init) board_eth_init(bis); else if (cpu_eth_init) cpu_eth_init(bis); uses that knowledge to see if the weak function board_eth_init() exists and then calls it if it does. If it doesn't exist, it sees if cpu_eth_init() exists and calls it if it does. Your counter proposal assumes that a weak function board_eth_init() *does* exist and uses the returned result as the condition of executing cpu_eth_init() (assuming it also exists). If you define a weak function that simply returns failure, your alternative is close, but still not the same because an overridden (*real*) board_eth_init() could return failure too, in which case it will (probably erroneously) execute cpu_eth_init(). Beyond that, if cpu_eth_init() doesn't exist (doesn't have a default weak function defined), the call to it will go *SPLAT*. HTH, gvb