[U-Boot] [PATCH] Support for Embedtronics/MRFSA board.
Albin Tonnerre
albin.tonnerre at free-electrons.com
Sat Aug 29 21:05:36 CEST 2009
Hi Sami,
Please note that I'm not an u-boot custodian, so what follows is by no means
authoritative.
However, I do have some comments about your code:
On Sat, Aug 29, 2009 at 08:22:09PM +0300, Sami Kantoluoto wrote :
> diff --git a/board/embedtronics/mrfsa/mrfsa.c b/board/embedtronics/mrfsa/mrfsa.c
> new file mode 100644
> index 0000000..6420c4b
> --- /dev/null
> +++ b/board/embedtronics/mrfsa/mrfsa.c
> @@ -0,0 +1,481 @@
> +/*
> + * (C) Copyright 2007-2008
> + * Stelian Pop <stelian.pop at leadtechdesign.com>
> + * Lead Tech Design <www.leadtechdesign.com>
> + *
> + * (C) Copyright 2009
> + * Sami Kantoluoto <sami.kantoluoto at embedtronics.fi>
> + * Embedtronics Oy <www.embedtronics.fi>
> + *
> + * 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/arch/at91_common.h>
> +#include <asm/arch/at91sam9260.h>
> +#include <asm/arch/at91sam9260_matrix.h>
> +#include <asm/arch/at91sam9_smc.h>
> +#include <asm/arch/at91_pmc.h>
> +#include <asm/arch/at91_wdt.h>
> +#include <asm/arch/at91_rstc.h>
> +#include <asm/arch/hardware.h>
> +#include <asm/arch/at91_spi.h>
> +#include <asm/arch/at91_pio.h>
> +#include <asm/arch/gpio.h>
> +#include <asm/arch/io.h>
> +#if defined(CONFIG_RESET_PHY_R) || defined(CONFIG_MACB)
> +#include <net.h>
> +#include <netdev.h>
> +#endif
> +
> +#include "ks8995ma.h"
> +#include "iopins.h"
> +
> +#ifndef CONFIG_NO_VLANS
> +#define CONFIG_NO_VLANS 1
> +#endif
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/* ------------------------------------------------------------------------- */
> +/*
> + * Miscelaneous platform dependent initialisations
> + */
> +
> +static void mrfsa_serial_hw_init (void)
> +{
> +#ifdef CONFIG_USART0
> + at91_set_A_periph (AT91_PIN_PB4, 1); /* TXD0 */
> + at91_set_A_periph (AT91_PIN_PB5, 0); /* RXD0 */
> + at91_sys_write (AT91_PMC_PCER, 1 << AT91_ID_US0);
> +#endif
> +
> +#ifdef CONFIG_USART1
> + at91_set_A_periph (AT91_PIN_PB6, 1); /* TXD1 */
> + at91_set_A_periph (AT91_PIN_PB7, 0); /* RXD1 */
> + at91_sys_write (AT91_PMC_PCER, 1 << AT91_ID_US1);
> +#endif
> +
> +#ifdef CONFIG_USART2
> + at91_set_A_periph (AT91_PIN_PB8, 1); /* TXD2 */
> + at91_set_A_periph (AT91_PIN_PB9, 0); /* RXD2 */
> + at91_sys_write (AT91_PMC_PCER, 1 << AT91_ID_US2);
> +#endif
> +
> +#ifdef CONFIG_USART3 /* DBGU */
> + at91_set_A_periph (AT91_PIN_PB14, 0); /* DRXD */
> + at91_set_A_periph (AT91_PIN_PB15, 1); /* DTXD */
> + at91_sys_write (AT91_PMC_PCER, 1 << AT91_ID_SYS);
> +#endif
> +}
There's already an at91_serial_hw_init() function which exactly performs that,
why don't you use it?
> +static void mrfsa_spi_hw_init (void)
> +{
> + unsigned long mode;
> +
> + /* Enable clock for SPI */
> + at91_sys_write (AT91_PMC_PCER, 1 << AT91SAM9260_ID_SPI0);
> +
> + at91_set_A_periph (AT91_PIN_PA3, 1); /* SPI0_NPCS0 */
> + at91_set_B_periph (AT91_PIN_PC11, 1); /* SPI0_NPCS1 */
> + at91_set_gpio_output (PIN_SPI0_CSETH, 1); /* CSETH */
> + at91_set_gpio_output (PIN_SPI0_CSSR, 1); /* CSSR */
> + at91_set_gpio_output (PIN_SPI0_CSZB, 1); /* CSZB */
> +
> + at91_set_A_periph (AT91_PIN_PA0, 0); /* SPI0_MISO */
> + at91_set_A_periph (AT91_PIN_PA1, 0); /* SPI0_MOSI */
> + at91_set_A_periph (AT91_PIN_PA2, 0); /* SPI0_SPCK */
> +}
> +
Same here. at91_spi0_hw_init() seems to do a large part of that, you should
consider using it
> +static int spi_transfer (unsigned csr, unsigned cspin, const void *src,
> + void *dst, int len)
> +{
> + unsigned long mode;
> + unsigned int timeout;
> +
> + /* make sure SPI is not clocking data */
> + writel (AT91_SPI_TXTDIS + AT91_SPI_RXTDIS,
> + AT91_BASE_SPI + AT91_SPI_PTCR);
> +
> + /* select slave parameters */
> + mode = readl (AT91_BASE_SPI + AT91_SPI_MR);
> + mode &= 0xfff0ffff;
> + writel (mode | (((0xf & ~(1 << csr)) << 16) & AT91_SPI_PCS),
> + AT91_BASE_SPI + AT91_SPI_MR);
> +
> + /* Initialize the Transmit and Receive Pointer */
> + writel ((unsigned int)dst, AT91_BASE_SPI + AT91_SPI_RPR);
> + writel ((unsigned int)src, AT91_BASE_SPI + AT91_SPI_TPR);
> +
> + /* Initialize the Transmit and Receive Counters */
> + writel (len, AT91_BASE_SPI + AT91_SPI_RCR);
> + writel (len, AT91_BASE_SPI + AT91_SPI_TCR);
> +
> + /* arm simple, non interrupt dependent timer */
> + reset_timer_masked ();
> + timeout = 0;
> +
> + /* select slave */
> + at91_set_gpio_value (cspin, 0);
> +
> + /* transmit data */
> + writel (AT91_SPI_TXTEN + AT91_SPI_RXTEN, AT91_BASE_SPI + AT91_SPI_PTCR);
> + while (!(readl (AT91_BASE_SPI + AT91_SPI_SR) & AT91_SPI_RXBUFF) &&
> + ((timeout = get_timer_masked ()) < CONFIG_SYS_SPI_WRITE_TOUT)) ;
> + writel (AT91_SPI_TXTDIS + AT91_SPI_RXTDIS,
> + AT91_BASE_SPI + AT91_SPI_PTCR);
> +
> + /* deselect slave */
> + at91_set_gpio_value (cspin, 1);
> +
> + /* return error on failure */
> + if (timeout >= CONFIG_SYS_SPI_WRITE_TOUT) {
> + printf ("Error Timeout\n\r");
> + return -1;
> + }
> +
> + /* success! */
> + return 0;
> +}
> +
Why do you need this one ? drivers/spi/atmel_spi.c should provide you with the
spi_xfer function, which likely does what you want. All you need is
#define CONFIG_ATMEL_SPI in your board config.h
> +int dram_init (void)
> +{
> + gd->bd->bi_dram[0].start = PHYS_SDRAM;
> + gd->bd->bi_dram[0].size = PHYS_SDRAM_SIZE;
> + return 0;
> +}
You should use get_ram_size() here, as it will help detecting is your RAM is
somehow damaged.
> +#endif /* CONFIG_HW_WATCHDOG */
> diff --git a/common/console.c b/common/console.c
> index 867c12c..fb6e409 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -311,6 +311,12 @@ int getc(void)
>
> int tstc(void)
> {
> +#ifdef CONFIG_HW_WATCHDOG
> + hw_watchdog_reset();
> +#endif
> +#ifdef CONFIG_WATCHDOG
> + watchdog_reset();
> +#endif
> #ifdef CONFIG_DISABLE_CONSOLE
> if (gd->flags & GD_FLG_DISABLE_CONSOLE)
> return 0;
> diff --git a/cpu/arm926ejs/at91/timer.c b/cpu/arm926ejs/at91/timer.c
> index 811bb3c..03fce54 100644
> --- a/cpu/arm926ejs/at91/timer.c
> +++ b/cpu/arm926ejs/at91/timer.c
> @@ -83,7 +83,12 @@ int timer_init(void)
> unsigned long long get_ticks(void)
> {
> ulong now = READ_TIMER;
> -
> +#ifdef CONFIG_HW_WATCHDOG
> + hw_watchdog_reset();
> +#endif
> +#ifdef CONFIG_WATCHDOG
> + watchdog_reset();
> +#endif
> if (now >= lastinc) /* normal mode (non roll) */
> /* move stamp forward with absolut diff ticks */
> timestamp += (now - lastinc);
You should provide a rationale for making such a change. So far this seems to
have worked for everyone, so your change is unlikely to be accepted unless you
justify why this is needed.
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index c184353..9e2e771 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -291,6 +291,7 @@ static int macb_recv(struct eth_device *netdev)
> return 0;
> }
>
> +#ifndef CONFIG_NO_PHY
> static void macb_phy_reset(struct macb_device *macb)
> {
> struct eth_device *netdev = &macb->netdev;
> @@ -403,6 +404,7 @@ static int macb_phy_init(struct macb_device *macb)
> return 1;
> }
> }
> +#endif /* CONFIG_NO_PHY */
>
> static int macb_init(struct eth_device *netdev, bd_t *bd)
> {
> @@ -463,8 +465,10 @@ static int macb_init(struct eth_device *netdev, bd_t *bd)
> #endif
> #endif /* CONFIG_RMII */
>
> +#ifndef CONFIG_NO_PHY
> if (!macb_phy_init(macb))
> return -1;
> +#endif /* CONFIG_NO_PHY */
>
> /* Enable TX and RX */
> macb_writel(macb, NCR, MACB_BIT(TE) | MACB_BIT(RE));
Same here. You should explain what problems you're trying to fix.
> diff --git a/include/configs/mrfsa.h b/include/configs/mrfsa.h
> new file mode 100644
> index 0000000..83ad18b
> --- /dev/null
> +++ b/include/configs/mrfsa.h
> @@ -0,0 +1,197 @@
> +/*
> + * (C) Copyright 2009
> + * Sami Kantoluoto <sami.kantoluoto at embedtronics.fi>
> + * Embedtronics Oy <www.embedtronics.fi>
> + *
> + * Configuation settings for the MRFSA board.
> + *
> + * 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
> +
> +/* ARM asynchronous clock */
> +#define AT91_MAIN_CLOCK 20000000 /* 20.000 MHz crystal */
> +#define AT91C_MASTER_CLOCK 133333333 /* peripheral clock */
AT91C_MASTER_CLOCK is not needed
> +#define CONFIG_SYS_HZ 1000
> +
> +#define CONFIG_ARM926EJS 1 /* This is an ARM926EJS Core */
> +
> +#define AT91_CPU_NAME "AT91SAM9G20"
Neither is AT91_CPU_NAME
> +#define CONFIG_AT91SAM9G20 1 /* It's an Atmel AT91SAM9G20 SoC */
> +
> +#define CONFIG_ARCH_CPU_INIT
> +#undef CONFIG_USE_IRQ /* we don't need IRQ/FIQ stuff */
> +
> +#define CONFIG_CMDLINE_TAG 1 /* enable passing of ATAGs */
> +#define CONFIG_SETUP_MEMORY_TAGS 1
> +#define CONFIG_INITRD_TAG 1
> +
> +#define CONFIG_SKIP_LOWLEVEL_INIT
> +#define CONFIG_SKIP_RELOCATE_UBOOT
> +
> +/*
> + * Hardware drivers
> + */
> +#define CONFIG_ATMEL_USART 1
> +#undef CONFIG_USART0
> +#undef CONFIG_USART1
> +#undef CONFIG_USART2
> +#define CONFIG_USART3 1 /* USART 3 is DBGU */
> +
> +/* LED */
> +#undef CONFIG_AT91_LED
> +
> +#define CONFIG_BOOTDELAY 3
> +
> +/*
> + * BOOTP options
> + */
> +#define CONFIG_BOOTP_BOOTFILESIZE 1
> +#define CONFIG_BOOTP_BOOTPATH 1
> +#define CONFIG_BOOTP_GATEWAY 1
> +#define CONFIG_BOOTP_HOSTNAME 1
> +
> +/*
> + * Command line configuration.
> + */
> +#include <config_cmd_default.h>
> +#undef CONFIG_CMD_BDI
> +#undef CONFIG_CMD_FPGA
> +#undef CONFIG_CMD_IMI
> +#undef CONFIG_CMD_IMLS
> +#undef CONFIG_CMD_LOADS
> +#undef CONFIG_CMD_SOURCE
> +
> +#define CONFIG_CMD_MMC 1
> +#define CONFIG_CMD_PING 1
> +#define CONFIG_CMD_DHCP 1
> +#define CONFIG_CMD_USB 1
> +#define CONFIG_CMD_FAT 1
> +#define CONFIG_CMD_EXT2 1
> +
> +#define CONFIG_MMC 1
> +#define CONFIG_ATMEL_MCI 1
> +
> +#define CONFIG_DOS_PARTITION 1 /* support DOS partition table */
> +
> +/* SDRAM */
> +#define CONFIG_NR_DRAM_BANKS 1
> +#define PHYS_SDRAM 0x20000000
> +#define PHYS_SDRAM_SIZE 0x04000000 /* 64 megs */
> +
> +/* DataFlash */
> +#define CONFIG_ATMEL_DATAFLASH_SPI
> +#define CONFIG_HAS_DATAFLASH 1
> +#define CONFIG_SYS_SPI_WRITE_TOUT (5*CONFIG_SYS_HZ)
> +#define CONFIG_SYS_MAX_DATAFLASH_BANKS 2
> +#define CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS0 0xC0000000 /* CS0 */
> +#define CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS1 0xD0000000 /* CS1 */
> +#define AT91_SPI_CLK 15000000
> +
> +#define DATAFLASH_TCSS (0x22 << 16)
> +#define DATAFLASH_TCHS (0x1 << 24)
> +
> +/* NAND flash - no nand flash on this board */
> +#ifdef CONFIG_CMD_NAND
> +#error No NAND FLASH
> +#endif
> +#define CONFIG_SYS_NO_NAND 1
There's no such thing as CONFIG_SYS_NO_NAND
> +/* NOR flash - no real flash on this board */
> +#define CONFIG_SYS_NO_FLASH 1
> +
> +/* Ethernet */
> +#define CONFIG_MACB 1
> +#define CONFIG_NO_PHY 1
> +#undef CONFIG_RMII
> +#define CONFIG_NET_MULTI 1
> +#define CONFIG_NET_RETRY_COUNT 20
> +#define CONFIG_RESET_PHY_R 1
> +
> +/* USB */
> +#define CONFIG_USB_ATMEL
> +#define CONFIG_USB_OHCI_NEW 1
> +#define CONFIG_DOS_PARTITION 1
> +#define CONFIG_SYS_USB_OHCI_CPU_INIT 1
> +#define CONFIG_SYS_USB_OHCI_REGS_BASE 0x00500000 /* AT91SAM9260_UHP_BASE */
> +#define CONFIG_SYS_USB_OHCI_SLOT_NAME "at91sam9g20"
> +#define CONFIG_SYS_USB_OHCI_MAX_ROOT_PORTS 2
> +#define CONFIG_USB_STORAGE 1
> +
> +#define CONFIG_SYS_LOAD_ADDR 0x22000000 /* load address */
> +
> +#define CONFIG_SYS_MEMTEST_START PHYS_SDRAM
> +#define CONFIG_SYS_MEMTEST_END 0x23e00000
> +
> +#ifdef CONFIG_SYS_USE_DATAFLASH_CS0
> +
> +/* bootstrap + u-boot + env + linux in dataflash on CS0 */
> +#define CONFIG_ENV_IS_IN_DATAFLASH 1
> +#define CONFIG_SYS_MONITOR_BASE (CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS0 + 0x8400)
> +#define CONFIG_ENV_OFFSET 0x4200
> +#define CONFIG_ENV_ADDR (CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS0 + CONFIG_ENV_OFFSET)
> +#define CONFIG_ENV_SIZE 0x4200
> +#define CONFIG_BOOTCOMMAND "cp.b 0xC0042000 0x22000000 0x210000; bootm"
> +#define CONFIG_BOOTARGS "console=ttyS0,115200 " \
> + "root=/dev/mmcblk0p1 rootwait rw"
> +
> +#elif CONFIG_SYS_USE_DATAFLASH_CS1
> +
> +/* bootstrap + u-boot + env + linux in dataflash on CS1 */
> +#define CONFIG_ENV_IS_IN_DATAFLASH 1
> +#define CONFIG_SYS_MONITOR_BASE (CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS1 + 0x8400)
> +#define CONFIG_ENV_OFFSET 0x4200
> +#define CONFIG_ENV_ADDR (CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS1 + CONFIG_ENV_OFFSET)
> +#define CONFIG_ENV_SIZE 0x4200
> +#define CONFIG_BOOTCOMMAND "cp.b 0xD0042000 0x22000000 0x210000; bootm"
> +#define CONFIG_BOOTARGS "console=ttyS0,115200 " \
> + "root=/dev/mmcblk0p1 rootwait rw"
> +
> +#endif
> +
> +#define CONFIG_BAUDRATE 115200
> +#define CONFIG_SYS_BAUDRATE_TABLE {115200 , 19200, 38400, 57600, 9600 }
> +
> +#define CONFIG_SYS_PROMPT "U-Boot> "
> +#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 1
> +#define CONFIG_CMDLINE_EDITING 1
> +
> +/*
> + * Size of malloc() pool
> + */
> +#define CONFIG_SYS_MALLOC_LEN ROUND(3 * CONFIG_ENV_SIZE + 128*1024, 0x1000)
> +#define CONFIG_SYS_GBL_DATA_SIZE 128 /* 128 bytes for initial data */
> +
> +#define CONFIG_STACKSIZE (32*1024) /* regular stack */
> +
> +#ifdef CONFIG_USE_IRQ
> +#error CONFIG_USE_IRQ not supported
> +#endif
> +
> +#define CONFIG_HW_WATCHDOG 1 /* we've hardware watchdog */
> +
> +#define BOARD_LATE_INIT 1
> +#define MMCI_BASE AT91SAM9260_BASE_MCI
As per my previous email, this really should be done at some common level.
Defining this - especially on boards which have only 1 controller - in every
config file is nothing but duplication
> +#endif
> diff --git a/lib_arm/bootm.c b/lib_arm/bootm.c
> index 128b7e3..e882d9b 100644
> --- a/lib_arm/bootm.c
> +++ b/lib_arm/bootm.c
> @@ -27,6 +27,11 @@
> #include <u-boot/zlib.h>
> #include <asm/byteorder.h>
>
> +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
> +#include <watchdog.h>
> +#endif
> +
> +
> DECLARE_GLOBAL_DATA_PTR;
>
> #if defined (CONFIG_SETUP_MEMORY_TAGS) || \
> @@ -123,8 +128,16 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
> }
> #endif
>
> +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
> + WATCHDOG_RESET();
> +#endif
> +
> cleanup_before_linux ();
>
> +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
> + WATCHDOG_RESET();
> +#endif
> +
> theKernel (0, machid, bd->bi_boot_params);
> /* does not return */
You might want to provide a rationale for this change, too.
Regards,
--
Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com
More information about the U-Boot
mailing list