Gaaah! Sorry, forgot to CC the list.

On Sat, Mar 22, 2008 at 8:05 AM, Ben Warren biggerbadderben@gmail.com wrote:

On Sat, Mar 22, 2008 at 1:03 AM, Shinya Kuribayashi skuribay@ruby.dti.ne.jp wrote:

...

Hi Ben,

with a quick glance, find the comments below.

Ben Warren wrote:

...

diff --git a/include/netdev.h b/include/netdev.h new file mode 100644 index 0000000..19195fa --- /dev/null +++ b/include/netdev.h @@ -0,0 +1,35 @@ +/*

• • (C) Copyright 2008
• • Benjamin Warren, biggerbadderben@gmail.com
• • See file CREDITS for list of people who contributed to this
• • project.
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License as
• • published by the Free Software Foundation; either version 2 of
• • the License, or (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • You should have received a copy of the GNU General Public License
• • along with this program; if not, write to the Free Software
• • Foundation, Inc., 59 Temple Place, Suite 330, Boston,
• • MA 02111-1307 USA
• */

+/*

• • netdev.h - definitions an prototypes for network devices
• */

+#ifndef _NETDEV_H_ +#define _NETDEV_H_

+int tsec_initialize(bd_t * bis, int index, char *devname);

+#endif /* _NETDEV_H_ */

Do we need such controller dependent declarations in netdev.h? FWIW board_eth_init() is reasonable for me.

This was done to prevent the board code from complaining at compile time. I have much bigger plans than initialize() prototypes for this file, but this is all it needs for now. Another option I considered here was to #include netdev from within common.h, but I've never been a fan of nesting header files.

...

And I know we need to go on a step-by-step basis, do you have any plan to have register/unregister interfaces?

Ah, remove the last 2 lines :-)

...

diff --git a/net/eth.c b/net/eth.c index 16a6dcb..3d02272 100644 --- a/net/eth.c +++ b/net/eth.c @@ -28,6 +28,13 @@

#if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)

+int __board_eth_init(bd_t *bis) +{

• return 0;
  

+}

+int board_eth_init(bd_t *bis) __attribute((weak, alias("__board_eth_init")));

#ifdef CFG_GT_6426x extern int gt6426x_eth_initialize(bd_t *bis); #endif

[This comment is not for Ben, but for everyone.]

Do we need such alias to empty function? Only __attribute((weak)) is enough for me. If we have some default behavior which isn't empty, I can see the weak & alias. Thought?

I plead ignorance on this one. I've never used weak functions before, and every example I found within U-boot, GCC documentation and Google had the alias. I'll try to get rid of it.

...

Shinya

Thanks for the feedback!

regards, Ben