[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Jerry Van Baren <gerald.vanbaren@ge.com>]

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