[U-Boot] [PATCH v4] at91: Add support for Bluewater Systems Snapper 9G45 module

["Andreas Bießmann" <andreas.devel@googlemail.com>]

Dear Simon Glass,

are you still interested getting this in mainline?
It does not apply cleanly on current u-boot-atmel HEAD, please rebase on
top of current u-boot-atmel.
And please fix the minor changes commented later on.

On 07.11.11 01:34, Simon Glass wrote:
> Snapper 9G45 is a ARM9-based CPU module with 1GB NAND and 128MB
> DDR SDRAM. This patch includes NAND and Ethernet support.
> 
> Signed-off-by: Simon Glass <sglass@bluewatersys.com>
> ---
> Changes in v2:
> - Removed unneeded i2c config
> - Added machine type define
> 
> Changes in v3:
> - Use CONFIG_MACH_TYPE instead of custom code
> - Reduce PHY reset delay to minimum required
> 
> Changes in v4:
> - Add MAINTAINERS entry and update Snapper boards since Ryan has moved
> - Fix ip= bootarg error
> - Remove I2C as this is not needed
> 
>  MAINTAINERS                               |    3 +-
>  board/bluewater/snapper9g45/Makefile      |   43 ++++++++
>  board/bluewater/snapper9g45/snapper9g45.c |  151 +++++++++++++++++++++++++++
>  boards.cfg                                |    1 +
>  include/configs/snapper9g45.h             |  157 +++++++++++++++++++++++++++++
>  5 files changed, 354 insertions(+), 1 deletions(-)
>  create mode 100644 board/bluewater/snapper9g45/Makefile
>  create mode 100644 board/bluewater/snapper9g45/snapper9g45.c
>  create mode 100644 include/configs/snapper9g45.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 576fea8..c12ee54 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -300,10 +300,11 @@ Dan Malek <dan@embeddedalley.com>
>  	stxssa		MPC85xx
>  	stxxtc		MPC8xx
>  
> -Ryan Mallon <ryan@bluewatersys.com>
> +Simon Glass <sglass@bluewatersys.com>
>  
>  	snapper9260		ARM926EJS (AT91SAM9260 SoC)
>  	snapper9g20		ARM926EJS (AT91SAM9G20 SoC)
> +	snapper9g45		ARM926EJS (AT91SAM9G45 SoC)
>  
>  Eran Man <eran@nbase.co.il>
>  
> diff --git a/board/bluewater/snapper9g45/Makefile b/board/bluewater/snapper9g45/Makefile
> new file mode 100644
> index 0000000..9016e5a
> --- /dev/null
> +++ b/board/bluewater/snapper9g45/Makefile
> @@ -0,0 +1,43 @@
> +#
> +# (C) Copyright 2003-2008
> +# Wolfgang Denk, DENX Software Engineering, wd at denx.de.

really?

> +#
> +# 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
> +#
> +
> +include $(TOPDIR)/config.mk
> +
> +LIB	= $(obj)lib$(BOARD).o
> +
> +COBJS-y += snapper9g45.o
> +
> +SRCS	:= $(COBJS-y:.o=.c)
> +OBJS	:= $(addprefix $(obj),$(COBJS-y))
> +
> +$(LIB):	$(obj).depend $(OBJS)
> +	$(call cmd_link_o_target, $(OBJS))
> +
> +#########################################################################
> +
> +# defines $(obj).depend target
> +include $(SRCTREE)/rules.mk
> +
> +sinclude $(obj).depend
> +
> +#########################################################################
> diff --git a/board/bluewater/snapper9g45/snapper9g45.c b/board/bluewater/snapper9g45/snapper9g45.c
> new file mode 100644
> index 0000000..4e5afb1
> --- /dev/null
> +++ b/board/bluewater/snapper9g45/snapper9g45.c
> @@ -0,0 +1,151 @@
> +/*
> + * (C) Copyright 2011 Bluewater Systems Ltd
> + *   Author: Andre Renaud <andre@bluewatersys.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
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/arch/at91sam9g45_matrix.h>
> +#include <asm/arch/at91sam9_smc.h>
> +#include <asm/arch/at91_common.h>
> +#include <asm/arch/at91_pmc.h>
> +#include <asm/arch/at91_rstc.h>
> +#include <asm/arch/gpio.h>
> +#include <asm/arch/hardware.h>
> +#include <net.h>
> +#include <netdev.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static void macb_hw_init(void)
> +{
> +	struct at91_pmc *pmc   = (struct at91_pmc  *)ATMEL_BASE_PMC;
--------------------------------------------------^
I haven't seen such an alignment somewhere in u-boot and I personally
dislike it. It is OK in that case here cause the types have almost the
same length, but how about a +20 char type vs. a int?
So could you please change this to '(<type><one blank>*)'?

> +	struct at91_port *pioa = (struct at91_port *)ATMEL_BASE_PIOA;
> +	struct at91_rstc *rstc = (struct at91_rstc *)ATMEL_BASE_RSTC;
> +	unsigned long erstl;
> +
> +	/* Enable clock */
> +	writel(1 << ATMEL_ID_EMAC, &pmc->pcer);
> +
> +	/*
> +	 * Disable pull-up on:
> +	 *	RXDV (PA15) => PHY normal mode (not Test mode) / CRSDV
> +	 *	ERX0 (PA12) => PHY ADDR0 / RXD0
> +	 *	ERX1 (PA13) => PHY ADDR1 => PHYADDR = 0x0 / RXD1
> +	 *
> +	 * PHY has internal pull-down
> +	 */
> +	writel(pin_to_mask(AT91_PIN_PA15) |
> +	       pin_to_mask(AT91_PIN_PA12) |
> +	       pin_to_mask(AT91_PIN_PA13),
> +	       &pioa->pudr);
> +
> +	/* Need to reset PHY -> needs 100us, so use minimum reset period */
> +	erstl = readl(&rstc->mr) & AT91_RSTC_MR_ERSTL_MASK;
> +	writel(AT91_RSTC_KEY | AT91_RSTC_MR_ERSTL(0) |
> +	       AT91_RSTC_MR_URSTEN, &rstc->mr);
> +	writel(AT91_RSTC_KEY | AT91_RSTC_CR_EXTRST, &rstc->cr);
> +
> +	/* Wait for end hardware reset */
> +	while (!(readl(&rstc->sr) & AT91_RSTC_SR_NRSTL))
> +		;
> +
> +	/* Restore NRST value */
> +	writel(AT91_RSTC_KEY | erstl | AT91_RSTC_MR_URSTEN, &rstc->mr);
> +
> +	/* The phy internal reset take 21ms */
> +	mdelay(21);
> +
> +	/* Re-enable pull-up */
> +	writel(pin_to_mask(AT91_PIN_PA15) |
> +	       pin_to_mask(AT91_PIN_PA12) |
> +	       pin_to_mask(AT91_PIN_PA13),
> +	       &pioa->puer);
> +
> +	at91_macb_hw_init();
> +}
> +
> +static void nand_hw_init(void)
> +{
> +	struct at91_smc *smc       = (struct at91_smc    *)ATMEL_BASE_SMC;
-------------------------------------------------------^
please only one blank here.

> +	struct at91_matrix *matrix = (struct at91_matrix *)ATMEL_BASE_MATRIX;
> +	unsigned long csa;
> +
> +	/* Enable CS3 as NAND/SmartMedia */
> +	csa = readl(&matrix->ebicsa);
> +	csa |= AT91_MATRIX_EBI_CS3A_SMC_SMARTMEDIA;
> +	writel(csa, &matrix->ebicsa);
> +
> +	/* Configure SMC CS3 for NAND/SmartMedia */
> +	writel(AT91_SMC_SETUP_NWE(2) | AT91_SMC_SETUP_NCS_WR(0) |
> +	       AT91_SMC_SETUP_NRD(2) | AT91_SMC_SETUP_NCS_RD(0),
> +	       &smc->cs[3].setup);
> +	writel(AT91_SMC_PULSE_NWE(4) | AT91_SMC_PULSE_NCS_WR(4) |
> +	       AT91_SMC_PULSE_NRD(4) | AT91_SMC_PULSE_NCS_RD(4),
> +	       &smc->cs[3].pulse);
> +	writel(AT91_SMC_CYCLE_NWE(7) | AT91_SMC_CYCLE_NRD(7),
> +	       &smc->cs[3].cycle);
> +	writel(AT91_SMC_MODE_RM_NRD | AT91_SMC_MODE_WM_NWE |
> +	       AT91_SMC_MODE_EXNW_DISABLE |
> +	       AT91_SMC_MODE_DBW_8 |
> +	       AT91_SMC_MODE_TDF_CYCLE(3),
> +	       &smc->cs[3].mode);
> +
> +	/* Configure RDY/BSY */
> +	at91_set_gpio_input(CONFIG_SYS_NAND_READY_PIN, 1);
> +
> +	/* Enable NandFlash */
> +	at91_set_gpio_output(CONFIG_SYS_NAND_ENABLE_PIN, 1);
> +}
> +
> +int board_init(void)
> +{
> +	struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
> +
> +	/* Enable PIO clocks */
> +	writel((1 << ATMEL_ID_PIOA) |
> +		(1 << ATMEL_ID_PIOB) |
> +		(1 << ATMEL_ID_PIOC |
> +		(1 << ATMEL_ID_PIODE)), &pmc->pcer);
> +
> +	/* Address of boot parameters */
> +	gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100;
> +
> +	/* Initialise peripherals */
> +	at91_seriald_hw_init();
> +	nand_hw_init();
> +
> +	macb_hw_init();
> +
> +	return 0;
> +}
> +
> +int board_eth_init(bd_t *bis)
> +{
> +	return macb_eth_initialize(0, (void *)ATMEL_BASE_EMAC, 0x1b);
> +}
> +
> +int dram_init(void)
> +{
> +	gd->ram_size = get_ram_size((void *)CONFIG_SYS_SDRAM_BASE,
> +				    CONFIG_SYS_SDRAM_SIZE);
> +	return 0;
> +}
> diff --git a/boards.cfg b/boards.cfg
> index 604becf..b6123c2 100644
> --- a/boards.cfg
> +++ b/boards.cfg
> @@ -94,6 +94,7 @@ at91sam9xeek_dataflash_cs0   arm         arm926ejs   at91sam9260ek       atmel
>  at91sam9xeek_dataflash_cs1   arm         arm926ejs   at91sam9260ek       atmel          at91        at91sam9260ek:AT91SAM9XE,SYS_USE_DATAFLASH_CS1
>  snapper9260                  arm         arm926ejs   -                   bluewater      at91        snapper9260:AT91SAM9260
>  snapper9g20                  arm         arm926ejs   snapper9260         bluewater      at91        snapper9260:AT91SAM9G20
> +snapper9g45                  arm         arm926ejs   snapper9g45         bluewater      at91        snapper9g45:AT91SAM9G45
>  sbc35_a9g20_nandflash        arm         arm926ejs   sbc35_a9g20         calao          at91        sbc35_a9g20:AT91SAM9G20,SYS_USE_NANDFLASH
>  sbc35_a9g20_eeprom           arm         arm926ejs   sbc35_a9g20         calao          at91        sbc35_a9g20:AT91SAM9G20,SYS_USE_EEPROM
>  tny_a9g20_nandflash          arm         arm926ejs   tny_a9260           calao          at91        tny_a9260:AT91SAM9G20,SYS_USE_NANDFLASH
> diff --git a/include/configs/snapper9g45.h b/include/configs/snapper9g45.h
> new file mode 100644
> index 0000000..30181ad
> --- /dev/null
> +++ b/include/configs/snapper9g45.h
> @@ -0,0 +1,157 @@
> +/*
> + * Bluewater Systems Snapper 9G45 modules
> + *
> + * (C) Copyright 2011 Bluewater Systems
> + *   Author: Andre Renaud <andre@bluewatersys.com>
> + *   Author: Ryan Mallon <ryan@bluewatersys.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
> + */
> +
> +#ifndef __CONFIG_H
> +#define __CONFIG_H
> +
> +/* SoC type is defined in boards.cfg */
> +#include <asm/hardware.h>
> +#include <asm/sizes.h>
You do not use the (prohibited) sz macros in here, so please remove the
asm/sizes.h

> +
> +#define CONFIG_SYS_TEXT_BASE		0x70000000
> +
> +/* ARM asynchronous clock */
> +#define CONFIG_SYS_AT91_MAIN_CLOCK	12000000	/* 12 MHz crystal */
> +#define CONFIG_SYS_AT91_SLOW_CLOCK	32768
> +#define CONFIG_SYS_HZ			1000
> +
> +/* CPU */
> +#define CONFIG_ARCH_CPU_INIT
> +
> +#define CONFIG_CMDLINE_TAG		/* enable passing of ATAGs	*/
--------------------------------------------------------------------^
only one blank here, please.

> +#define CONFIG_SETUP_MEMORY_TAGS
> +#define CONFIG_INITRD_TAG
> +#define CONFIG_SKIP_LOWLEVEL_INIT
> +
> +#define CONFIG_DISPLAY_CPUINFO
> +#define CONFIG_FIT
> +
> +/* SDRAM */
> +#define CONFIG_NR_DRAM_BANKS		1
> +#define CONFIG_SYS_SDRAM_BASE		ATMEL_BASE_CS6
> +#define CONFIG_SYS_SDRAM_SIZE		(128 * 1024 * 1024) /* 128MB */
> +#define CONFIG_SYS_INIT_SP_ADDR		(ATMEL_BASE_SRAM + 0x1000 - \
> +					 GENERATED_GBL_DATA_SIZE)
> +
> +/* Mem test settings */
> +#define CONFIG_SYS_MEMTEST_START	CONFIG_SYS_SDRAM_BASE
> +#define CONFIG_SYS_MEMTEST_END		(CONFIG_SYS_SDRAM_BASE + (1024 * 1024))
> +
> +/* NAND flash */
> +#define CONFIG_NAND_ATMEL
> +#define CONFIG_SYS_NO_FLASH
> +#define CONFIG_SYS_MAX_NAND_DEVICE	1
> +#define CONFIG_SYS_NAND_BASE		ATMEL_BASE_CS3 /*0x40000000*/
> +#define CONFIG_SYS_NAND_DBW_8
> +#define CONFIG_SYS_NAND_MASK_ALE	(1 << 21) /* AD21 */
> +#define CONFIG_SYS_NAND_MASK_CLE	(1 << 22) /* AD22 */
> +#define CONFIG_SYS_NAND_ENABLE_PIN	AT91_PIN_PC14
> +#define CONFIG_SYS_NAND_READY_PIN	AT91_PIN_PC8
> +
> +/* Ethernet */
> +#define CONFIG_MACB
> +#define CONFIG_RMII
> +#define CONFIG_NET_RETRY_COUNT		20
> +#define CONFIG_TFTP_PORT
> +#define CONFIG_TFTP_TSIZE
> +
> +/* USB */
> +#define CONFIG_USB_ATMEL
> +#define CONFIG_USB_OHCI_NEW
> +#define CONFIG_DOS_PARTITION
> +#define CONFIG_SYS_USB_OHCI_CPU_INIT
> +#define CONFIG_SYS_USB_OHCI_REGS_BASE	0x00700000 /* _UHP_OHCI_BASE */
> +#define CONFIG_SYS_USB_OHCI_SLOT_NAME	"at91sam9g45"
> +#define CONFIG_SYS_USB_OHCI_MAX_ROOT_PORTS	2
> +#define CONFIG_USB_STORAGE
> +
> +/* GPIOs and IO expander */
> +#define CONFIG_AT91_LEGACY
Is this really required? I guess not, can you please also have a look
for snapper9260 regarding the CONFIG_AT91_LEGACY?

> +#define CONFIG_ATMEL_LEGACY
This may be still required, please check and remove if not. If it is
required ... patches for removing the need for CONFIG_ATMEL_LEGACY are
expected wistfully.

> +#define CONFIG_AT91_GPIO
> +#define CONFIG_AT91_GPIO_PULLUP		1
> +
> +/* UARTs/Serial console */
> +#define CONFIG_ATMEL_USART
> +#define CONFIG_USART_BASE		ATMEL_BASE_DBGU
> +#define CONFIG_USART_ID			ATMEL_ID_SYS
> +#define CONFIG_BAUDRATE			115200
> +#define CONFIG_SYS_BAUDRATE_TABLE	{115200 , 19200, 38400, 57600, 9600 }
Please remove, we have a generic definition now.

> +#define CONFIG_SYS_PROMPT		"Snapper> "
> +
> +/* Boot options */
> +#define CONFIG_SYS_LOAD_ADDR		0x71000000
> +#define CONFIG_BOOTDELAY		3
> +#define CONFIG_ZERO_BOOTDELAY_CHECK
> +
> +#define CONFIG_BOOTP_BOOTFILESIZE
> +#define CONFIG_BOOTP_BOOTPATH
> +#define CONFIG_BOOTP_GATEWAY
> +#define CONFIG_BOOTP_HOSTNAME
> +
> +/* Environment settings */
> +#define CONFIG_ENV_IS_IN_NAND
> +#define CONFIG_ENV_OFFSET		(512 << 10)
I see you use two different styles to define the sizes (here the shift
type and the multiply type above). Don't get me wrong, it is ok to do
so. But I ask myself if it was intended to do so or just a copy from
somewhere and never touched cause they worked.

> +#define CONFIG_ENV_SIZE			(256 << 10)
> +#define CONFIG_ENV_OVERWRITE
> +#define CONFIG_BOOTARGS			"console=ttyS0,115200 ip=dhcp"
> +
> +/* Console settings */
> +#define CONFIG_SYS_CBSIZE		256
> +#define CONFIG_SYS_MAXARGS		16
> +#define CONFIG_SYS_PBSIZE		(CONFIG_SYS_CBSIZE +		\
> +					 sizeof(CONFIG_SYS_PROMPT) + 16)
> +#define CONFIG_SYS_LONGHELP
> +#define CONFIG_SYS_EXTBDINFO
I didn't saw this define before, a grep showed up that is only used in
ppc and m68k, so please remove.

> +#define CONFIG_CMDLINE_EDITING
> +#define CONFIG_AUTO_COMPLETE
> +#define CONFIG_SYS_HUSH_PARSER
> +#define CONFIG_SYS_PROMPT_HUSH_PS2	"> "
Please remove, this is the default value.

> +
> +/* U-Boot memory settings */
> +#define CONFIG_SYS_MALLOC_LEN		(1 << 20)
> +#define CONFIG_STACKSIZE		(256 << 10)
Please remove, CONFIG_STACKSIZE is not used anywhere (except avr32 ...)

> +#define CONFIG_MACH_TYPE		2817
> +
> +/* Command line configuration */
> +#include <config_cmd_default.h>
> +
> +#undef CONFIG_CMD_BDI
Please do not undef the bdinfo command, it is a real useful command and
every u-boot should provide it.

> +#undef CONFIG_CMD_FPGA
> +#undef CONFIG_CMD_IMI
> +#undef CONFIG_CMD_IMLS
You defined CONFIG_SYS_NO_FLASH, so CONFIG_CMD_IMLS should not defined
here, please remove.

> +#undef CONFIG_CMD_LOADS
> +#undef CONFIG_CMD_SOURCE
> +
> +#define CONFIG_CMD_PING
> +#define CONFIG_CMD_DHCP
> +#define CONFIG_CMD_FAT
> +#undef CONFIG_CMD_GPIO
Who defined CONFIG_CMD_GPIO before?

> +#define CONFIG_CMD_USB
> +#define CONFIG_CMD_MII
> +#define CONFIG_CMD_NAND
> +
> +#endif
> 

Best regards

Andreas Bie?mann