[U-Boot] 83xx: add support for ve8313 board

[Heiko Schocher]

Hello Kim,

Kim Phillips wrote:
> On Mon, 5 Jul 2010 12:23:10 +0200
> Heiko Schocher <hs at denx.de> wrote:
> 
>> ---
>>  board/ve8313/Makefile    |   50 +++++
>>  board/ve8313/config.mk   |   10 +
>>  board/ve8313/ve8313.c    |  212 ++++++++++++++++++
>>  boards.cfg               |    1 +
>>  include/configs/ve8313.h |  534 ++++++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 807 insertions(+), 0 deletions(-)
> 
> missing MAKEALL, MAINTAINERS entries, and, if there's anything unique
> about this board, a doc/README.ve8313.

There is nothing unique on this board, so don;t think a
doc/README.ve8313 is useful.

>> diff --git a/board/ve8313/config.mk b/board/ve8313/config.mk
> 
>> +ifndef TEXT_BASE
>> +#TEXT_BASE = 0x100000
>> +TEXT_BASE = 0xfe000000
>> +endif
>> +
>> +#PLATFORM_CPPFLAGS += -DDEBUG
> 
> please clean this up - remove commented out code.

fixed.

>> diff --git a/board/ve8313/ve8313.c b/board/ve8313/ve8313.c
>> new file mode 100644
>> index 0000000..b13d1f3
>> --- /dev/null
>> +++ b/board/ve8313/ve8313.c
>> @@ -0,0 +1,212 @@
>> +/*
>> + * (C) Copyright 2010
>> + * Heiko Schocher, DENX Software Engineering, hs at denx.de.
> 
> this file contains code that looks like it was copied from files with
> Freescale copyrights; why are you deleting Freescale's copyrights?

Hups, Sorry for that.

>> +#if defined(CONFIG_OF_LIBFDT)
>> +#include <libfdt.h>
>> +#endif
> 
> I realise this was copied in, but we don't need the ifdef anymore.

done.

>> +/* Fixed sdram init -- doesn't use serial presence detect.
>> + *
>> + * This is useful for faster booting in configs where the RAM is unlikely
>> + * to be changed, or for things like NAND booting where space is tight.
>> + */
> 
> Is the memory soldered on to the board?  If not, does the serial
> presence detect code not work for you?

It is soldered.

>> +#if defined(CONFIG_HW_WATCHDOG)
>> +	clrbits_be32(&gpio->dat, (VE8313_WDT_EN | VE8313_WDT_TRIG));
>> +	/* set WDT pins as output */
>> +	setbits_be32(&gpio->dir, (VE8313_WDT_EN | VE8313_WDT_TRIG));
>> +#else
>> +	/* disable WDT */
>> +	setbits_be32(&gpio->dat, (VE8313_WDT_EN | VE8313_WDT_TRIG));
> 
> inner parens not necessary, also the last WDT_EN bit setting doesn't
> match the comment above it (ENable vs. disable) - perhaps nothing
> need be done if HW_WATCHDOG is not set?

I must explicitely disable the WDT.

>> +	setbits_be32(&gpio->dat, (VE8313_WDT_TRIG));
>> +	clrbits_be32(&gpio->dat, (VE8313_WDT_TRIG));
> 
> inner parens not necessary.

done.

> 
>> +#undef CONFIG_HW_WATCHDOG
> 
> no #undefs please.

removed.

>> +#define CONFIG_SYS_LBC_LBCR	(0x00040000)
> 
> parens not necessary.

fixed.

>> +#define CONFIG_SYS_PCI_SUBSYS_VENDORID 0x1057	/* Motorola */
> 
> It's Freescale: 0x1957.

fixed.

Thanks for your review.

bye
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany