[U-Boot] [PATCH 05/10] nds32: add nds32 board with common support

Ben Warren biggerbadderben at gmail.com
Mon Jun 14 07:01:21 CEST 2010


Hello Macpaul,

On Fri, Jun 11, 2010 at 2:34 AM, Macpaul Lin <macpaul at andestech.com> wrote:

>        Add nds32 based common board related support.
>
> Signed-off-by: Macpaul Lin <macpaul at andestech.com>
> ---
>  board/AndesTech/common/env.c         |  138 ++++++
>  board/AndesTech/common/flash.c       |  621 +++++++++++++++++++++++++++
>  board/AndesTech/common/flib_flash.c  |  721
> ++++++++++++++++++++++++++++++++
>  board/AndesTech/common/flib_serial.c |  373 +++++++++++++++++
>  board/AndesTech/common/fotg2xx.c     |   60 +++
>  board/AndesTech/common/ftmac100.c    |  766
> ++++++++++++++++++++++++++++++++++
>  board/AndesTech/common/ftpci100.c    |  712
> +++++++++++++++++++++++++++++++
>  board/AndesTech/common/serial.c      |  141 +++++++
>
Please respect the file hierarchy.  Drivers go in the proper place (e.g.
flib_serial.c should go in drivers/serial, ftmac100.c should go in
drivers/net etc.)
<snip>

> diff --git a/board/AndesTech/common/ftmac100.c
> b/board/AndesTech/common/ftmac100.c
> new file mode 100644
> index 0000000..825031d
> --- /dev/null
> +++ b/board/AndesTech/common/ftmac100.c
> @@ -0,0 +1,766 @@
> +/*
> + * Copyright (C) 2009 Andes Technology Corporation
> + * Shawn Lin, Andes Technology Corporation <nobuhiro at andestech.com>
> + * Macpaul Lin, Andes Technology Corporation <macpaul at andestech.com>
> + *
> + * 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
> + */
> +
> +// --------------------------------------------------------------------
> +//     lmc83:  modified from smc91111.c (2002-11-29)
> +// --------------------------------------------------------------------
> +
> +
> +#include <common.h>
> +#include <asm/andesboot.h>
> +#include <malloc.h>
> +#include <command.h>
> +#include "../include/porting.h"
> +#include "../include/ftmac100.h"
> +#include <net.h>
> +
> +
> +#ifdef CONFIG_DRIVER_FTMAC100
> +
> +// Use power-down feature of the chip
> +#define POWER_DOWN     0
> +
> +static unsigned char ftmac100_mac_addr[] = {0x00, 0x41, 0x71, 0x99, 0x00,
> 0x00};
>
Please don't make assumptions like this.

> +
> +static const char version[] =
> +       "Faraday FTMAC100 Driver, (Linux Kernel 2.4) 10/18/02 - by
> Faraday\n";
>
I don't think so.

> +
> +#define inl(addr)                      (*((volatile u32 *)(addr)))
> +#define inw(addr)                      (*((volatile u16 *)(addr)))
> +#define outl(value, addr)      (*((volatile u32 *)(addr)) = value)
> +#define outb(value, addr)      (*((volatile u8 *)(addr)) = value)
>
This isn't the place to define these.

> +
> +struct net_device dev_eth0;
> +int tx_rx_cnt = 0;
> +/*
> + *
> + * Configuration options, for the experienced user to change.
> + *
> + */
> +/*
> + * DEBUGGING LEVELS
> + *
> + * 0 for normal operation
> + * 1 for slightly more details
> + * >2 for various levels of increasingly useless information
> + *     2 for interrupt tracking, status flags
> + *     3 for packet info
> + *     4 for complete packet dumps
> + */
> +
> +#define DO_PRINT(args...) printk(args)
> +
> +//#define FTMAC100_DEBUG 5 // Must be defined in makefile
> +
> +#if (FTMAC100_DEBUG > 2 )
> +#define PRINTK3(args...) DO_PRINT(args)
> +#else
> +#define PRINTK3(args...)
> +#endif
> +
> +#if FTMAC100_DEBUG > 1
> +#define PRINTK2(args...) DO_PRINT(args)
> +#else
> +#define PRINTK2(args...)
> +#endif
> +
> +#ifdef FTMAC100_DEBUG
> +#define PRINTK(args...) DO_PRINT(args)
> +#else
> +#define PRINTK(args...)
> +#endif
>
Please don't do this stuff.  The standard debug() will suffice.

> +
> +
> +///#define FTMAC100_TIMER
> +
> +/*
> + *
> + * The internal workings of the driver.  If you are changing anything
> + * here with the SMC stuff, you should have the datasheet and know
> + * what you are doing.
> + *
> + */
> +#define CARDNAME "FTMAC100"
> +
> +#ifdef FTMAC100_TIMER
> +       static struct timer_list ftmac100_timer;
> +#endif
> +
> +#define ETH_ZLEN 60
> +
> +#ifdef  CONFIG_SMC_USE_32_BIT
>
Please use your own CONFIG

> +#define USE_32_BIT
> +#else
> +#undef USE_32_BIT
>
This sort of thing needs more namespace

> +#endif
> +/*
> + *
> + * The driver can be entered at any of the following entry points.
> + *
> + */
> +
> +extern int eth_init(bd_t *bd);
> +extern void eth_halt(void);
> +extern int eth_rx(void);
> +extern int eth_send(volatile void *packet, int length);
>
You're using an API that's tagged as deprecated.  Please read the
documentation, update the driver to the appropriate API and then we'll
proceed with the review.

regards,
Ben


More information about the U-Boot mailing list