[U-Boot] [PATCH 1/1] net: Add Xilinx LL Temac driver version2

[Michal Simek <monstr@monstr.eu>]

Ben Warren wrote:
> Hi Michal,
> 
> Michal Simek wrote:
>> Hi Ben,
>>  
>>> Hi Michal,
>>>
>>> Michal Simek wrote:
>>>
>>> <snip>
>>>    
>>>>> All of the above mentioned issues are ones that I could easily deal
>>>>> with, but one thing that really does need to change is that you
>>>>> need to
>>>>> use the CONFIG_NET_MULTI API.  In other words, your driver should
>>>>> have a
>>>>> single initialize() function (prototyped in include/netdev.h), and an
>>>>> eth_device struct that gets registered.  All your access functions
>>>>> (eth_init, eth_send, eth_recv etc.) will be static and pointed to
>>>>> by the
>>>>> eth_device struct.  Most drivers are already this way.
>>>>>             
>>>> I look at it and I did some change and the main problem is in
>>>> Microblaze GCC.
>>>> We use GCC 3.4.1 and CONFIG_NET_MULTI use weak function and
>>>> board_eth_init is
>>>> never called. We are working on GCC 4.1.2 but I don't know when I
>>>> get it.
>>>>
>>>>         
>>> According to the documentation I could find, weak symbols were present
>>> in gcc 3.4.1.  Are you sure you're using them properly?  Due to the way
>>> linking is performed in U-boot, any weak symbol overrides need to be in
>>> source files that have strongly linked symbols.  You'll see that all
>>> implementations of cpu_eth_init() and board_eth_init() are in files that
>>> already contain stuff that is sure to be linked.
>>>     
>>
>>
>> hmm. I did some tests and I found that the my problem is with this
>> line 40. (I
>> use board_eth_init initialization)
>> int board_eth_init(bd_t *bis) __attribute((weak,
>> alias("__def_eth_init")));
>> I am not gcc specialist but I smell problem with GCC.
>>
>>   
> This essentially says "board_eth_init() = __def_eth_init() unless
> overridden by a strongly linked function".  Here's how I debug this
> stuff, and you don't need to instrument your code or even run it to know
> if the linking worked properly:
> 
> Look in the System.map file that the build system generates (it's
> human-readable).   If there are no overriding functions, you'd expect
> that the addresses of __def_eth_int(), cpu_eth_init() and
> board_eth_init() to be identical.  If YOUR board_eth_init() is linked
> in, it will have a different address.  That's it!

Address are different for both cases
[monstr at monstr u-boot-eth]$ cat System.map | grep board_eth_init
9181cb34 T board_eth_init
[monstr at monstr u-boot-eth]$ cat System.map | grep def_eth_init
918036c4 t __def_eth_init
[monstr at monstr u-boot-eth]$ cat System.map | grep cpu_eth_init
918036c4 W cpu_eth_init


918038c8:       b000ffff        imm     -1
918038cc:       b9f4fdf8        brlid   r15, -520       // 918036c4 <cpu_eth_init>
918038d0:       10b30000        addk    r5, r19, r0
918038d4:       bca30010        bgei    r3, 16          // 918038e4
918038d8:       b000ffff        imm     -1
918038dc:       b9f4fde8        brlid   r15, -536       // 918036c4 <cpu_eth_init>


from net/eth.c 148:149
        if (board_eth_init(bis) < 0)
                cpu_eth_init(bis);

As you can see from objdump above -> my toolchain add the same address for
board_eth_init and cpu_eth_init but that's wrong.

Thanks,
Michal


> 
> regards,
> Ben


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854