[U-Boot] [patch] csb337 board support (ARM at91rm9200)

[Wolfgang Denk]

Dear David Brownell,

In message <200906121458.58296.david-b at pacbell.net> you wrote:
> Add support for csb337, an older at91rm9200 board.  These boards
> originally shipped with MicroMonitor, not U-Boot.  This config
> supports boot from Ethernet, and talks over I2C and console.
> 
> Open issues:
>   - Console refuses to start at 115200 baud, and misbehaves if it's
>     later changed to that speed.  So, use 38400 (like MicroMonitor).
> 
>   - USB didn't work; the software wouldn't detect usb-storage devices.
>     So it's not yet enabled.
> 
>   - There seems to be some issue copying lots of data from NOR flash.
>     It works OK in chunks of up to 512 KB or so, but copying a kernel
>     (1.4 MB) failed ... copying *way* over 15 MBytes, and trashing
>     the DRAM image of U-Boot that was running.  (Compiler issue?)
> 
> Sending this along anyway; it basically works, bugs can be fixed later.
> 
> Signed-off-by: David Brownell <dbrownell at users.sourceforge.net>
> ---
> NOTE:  depends on cpu/arm920t/at91rm9200/ether.c patch to change
> how the Ethernet address is stored in chip registers so it matches
> the old MicroMonitor bug that Linux is working around.
> 
>  MAKEALL                  |    1 
>  Makefile                 |    3 
>  board/csb337/Makefile    |   49 +++++++++
>  board/csb337/config.mk   |    1 
>  board/csb337/csb337.c    |   74 +++++++++++++++
>  board/csb337/u-boot.lds  |   56 +++++++++++
>  include/configs/csb337.h |  222 +++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 406 insertions(+)

Entry to MAINTAINERS missing.

> --- a/MAKEALL
> +++ b/MAKEALL
> @@ -579,6 +579,7 @@ LIST_at91="		\
>  	at91sam9g20ek	\
>  	at91sam9rlek	\
>  	cmc_pu2		\
> +	csb337		\
>  	csb637		\
>  	kb9202		\
>  	mp2usb		\
> --- a/Makefile
> +++ b/Makefile
> @@ -2638,6 +2638,9 @@ at91rm9200ek_config	:	unconfig
>  cmc_pu2_config	:	unconfig
>  	@$(MKCONFIG) $(@:_config=) arm arm920t cmc_pu2 NULL at91rm9200
>  
> +csb337_config	:	unconfig
> +	@$(MKCONFIG) $(@:_config=) arm arm920t csb337 NULL at91rm9200
> +
>  csb637_config	:	unconfig
>  	@$(MKCONFIG) $(@:_config=) arm arm920t csb637 NULL at91rm9200
>  
> --- /dev/null
> +++ b/board/csb337/Makefile
...
> +#########################################################################
> +
> +# defines $(obj).depend target
> +include $(SRCTREE)/rules.mk
> +
> +sinclude $(obj).depend
> +

Please don't add trailing empty lines.

> --- /dev/null
> +++ b/board/csb337/csb337.c
...
> +#include <common.h>
> +#include <asm/arch/AT91RM9200.h>
> +#include <at91rm9200_net.h>
> +#include <lxt971a.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +

Only one blank line here.

> +int dram_init(void)
> +{
> +	gd->bd->bi_dram[0].start = PHYS_SDRAM;
> +	gd->bd->bi_dram[0].size = PHYS_SDRAM_SIZE;
> +	return 0;
> +}

How about using get_ram_size() to auto-detect the size and test the
memory?

> +#if defined(CONFIG_DRIVER_ETHER) && defined(CONFIG_CMD_NET)
> +
> +/* The AT91 lxt972 glue modified the original lxt971 code by
> + * changing names and generalizing a bit.  So we use "lxt972"
> + * names here even though the CSB337 has an lxt971 chip.
> + */
> +extern unsigned char lxt972_InitPhy(AT91S_EMAC *mac);
> +extern unsigned lxt972_IsPhyConnected(AT91S_EMAC *mac);
> +extern unsigned char lxt972_GetLinkSpeed(AT91S_EMAC *mac);
> +extern unsigned char lxt972_AutoNegotiate(AT91S_EMAC *mac, int *status);

Move prototypes to some header file, please.

> --- /dev/null
> +++ b/include/configs/csb337.h
...
> +/* default timings shipped by Cogent (PLLA --> MCK):
> + *  184.32 MHz for CPU (PLLA)
> + *   46.08 MHz for memory and peripherals (PLL/4)
> + *   96.00 MHz for USB (PLLB)
> + */

Incorrect multiline comment style.

> +#if 0	/* can't use this from startup.S, sigh */
> +#define CONFIG_SYS_PLLAR_VAL \
> +	( AT91C_CKGR_SRCA | AT91C_CKGR_OUTA_2 | (0x3e << 8) \
> +	| ((AT91_PLLA_MULT - 1) << 16) | (AT91_PLLA_DIV << 0))
> +#define CONFIG_SYS_PLLBR_VAL \
> +	( AT91C_CKGR_USB_PLL | AT91C_CKGR_OUTB_0 | (0x3e << 8) \
> +	| ((AT91_PLLB_MULT - 1) << 16) | (AT91_PLLB_DIV << 0))
> +#define CONFIG_SYS_MCKR_VAL \
> +	(AT91C_PMC_MDIV_4 | AT91C_PMC_CSS_PLLA_CLK)
> +#else

Please do not add dead code.

...
> +/* except maybe for SDRAM1, none of these are board-specic */
> +#define CONFIG_SYS_SDRAM	0x20000000	/* address of SDRAM */
> +#define CONFIG_SYS_SDRAM1	0x20000080	/* address of SDRAM + mode register value */

Line too long. (Please check globally).

> +#if 0
> +/* REVISIT:  USB failed to enumerate devices of any kind ... sw issues */
> +
> +/* allow loading from USB media */
> +#define CONFIG_USB_OHCI_NEW
> +#define CONFIG_SYS_USB_OHCI_MAX_ROOT_PORTS	2
> +#define CONFIG_SYS_USB_OHCI_REGS_BASE		AT91_USB_HOST_BASE
> +#define CONFIG_SYS_USB_OHCI_SLOT_NAME		"at91-ohci"
> +#define CONFIG_USB_STORAGE
> +#endif

Please do not add dead code.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Until you walk a mile in another man's moccasins, you  can't  imagine
the smell.