From mboxrd@z Thu Jan  1 00:00:00 1970
From: Shinya Kuribayashi <skuribay@ruby.dti.ne.jp>
Date: Sat, 22 Mar 2008 14:03:28 +0900
Subject: [U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function
In-Reply-To: <47E472FD.7010907@gmail.com>
References: <47E472FD.7010907@gmail.com>
Message-ID: <47E49320.1080504@ruby.dti.ne.jp>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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 at 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.

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?

  Shinya