On 21/11/2011 09:44, Tapani Utriainen wrote:
>
> Add support for TechNexion TAM3517 SoM
>
> Signed-off-by: Tapani Utriainen <tap...@technexion.com>
> CC: Sandeep Paulraj <s-paul...@ti.com>
Hi Tapani,
> ---
> arch/arm/include/asm/mach-types.h | 1
> board/technexion/tam3517/Makefile | 49 ++++
> board/technexion/tam3517/tam3517.c | 150 ++++++++++++
> board/technexion/tam3517/tam3517.h | 388 +++++++++++++++++++++++++++++++++
> boards.cfg | 1
> include/configs/tam3517.h | 427
> +++++++++++++++++++++++++++++++++++++
> 6 files changed, 1016 insertions(+)
>
I am also porting a TAM3517 based custom board to U-Boot mainline. Maybe
we can share some experiences ;-)
> diff --git a/arch/arm/include/asm/mach-types.h
> b/arch/arm/include/asm/mach-types.h
> index 2d5c3bc..685b46f 100644
> --- a/arch/arm/include/asm/mach-types.h
> +++ b/arch/arm/include/asm/mach-types.h
> @@ -467,6 +467,7 @@ extern unsigned int __machine_arch_type;
> #define MACH_TYPE_OMAP4_PANDA 2791
> #define MACH_TYPE_TI8168EVM 2800
> #define MACH_TYPE_TETON_BGA 2816
> +#define MACH_TYPE_TAM3517 2818
This is wrong. The mach-types.h is taken from Linux and the rule is to
always be in sync with tthe kernel. If the MACH-TYPE is not set, it must
be set in the board configuration file.
> diff --git a/board/technexion/tam3517/tam3517.c
> b/board/technexion/tam3517/tam3517.c
> new file mode 100644
As I said, I am also porting a similar board and I am facing the problem
to support multiple board using the same module (TAM3517). My idea was
to have a common include/configs/tam3517.h file, that is included by all
boards using the module, such as the twister and a custom boards. This
allows to factorize most of common setup.
Then we could have a board/technexion/twister board in parallel with
other boards using only the module. What do you think about ?
> +#include <asm/mach-types.h>
We do not need mach-types.h
> +#include "tam3517.h"
> +
> +#define AM3517_IP_SW_RESET 0x48002598
> +#define CPGMACSS_SW_RST (1 << 1)
These are obsolete, too. It is (or it will be soon) common code. See
previous patches for AM 3517 (mcx board).
> +
> +/*
> + * Routine: board_init
> + * Description: Early hardware init.
> + */
> +int board_init(void)
> +{
> + DECLARE_GLOBAL_DATA_PTR;
> +
> + gpmc_init(); /* in SRAM or SDRAM, finish GPMC */
> + /* board id for Linux */
> + gd->bd->bi_arch_number = MACH_TYPE_TAM3517;
This should be removed, it is part of generic arm library.
> +#if defined(CONFIG_XR16L2751)
If I have understood the schematics, the twister board (because this
code corresponds to this board) has always a XR16L2751 UART. Should we
not drop the #ifdef ?
> +/*
> + * Routine: misc_init_r
> + */
> +int misc_init_r(void)
> +{
> + int ctr = 0;
> +
> + i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
> +#ifdef CONFIG_BOOT_FROM_MMC
> + omap_set_gpio_direction(TAM3517_SW3_PIN8, 1);
This is obsolete - use the standard GPIO framework.
> +#if defined(CONFIG_DRIVER_TI_EMAC)
> + /* allow the PHY to stabilize and settle down */
> + do {
> + udelay(1000);
> + ctr++;
> + } while (ctr < 300);
> +
> + /*ensure that the module is out of reset*/
> + reset = readl(AM3517_IP_SW_RESET);
> + reset &= (~CPGMACSS_SW_RST);
> + writel(reset, AM3517_IP_SW_RESET);
> +#endif
Drop this part - it is in EMAC initialization, not needed in board code.
> + if (smc911x_initialize(0, CONFIG_SMC911X_BASE) <= 0)
> + n_eth--;
Why do we need to count the interfaces ? This is already done by the
network subsystem.
> diff --git a/board/technexion/tam3517/tam3517.h
> b/board/technexion/tam3517/tam3517.h
> new file mode 10064
> + /* ETK (ES2 onwards) */\
> + MUX_VAL(CP(ETK_CLK_ES2), (IDIS | PTU | EN | M0)) \
> + MUX_VAL(CP(ETK_CTL_ES2), (IDIS | PTD | DIS | M0)) \
> + MUX_VAL(CP(ETK_D0_ES2), (IEN | PTD | DIS | M0)) \
> + MUX_VAL(CP(ETK_D1_ES2), (IEN | PTD | DIS | M0)) \
> + MUX_VAL(CP(ETK_D2_ES2), (IEN | PTD | EN | M0)) \
..but the pins for ETK are used as USB for EHCI-OMAP, are'nt they ? This
setup seems to me wrong.
> diff --git a/include/configs/tam3517.h b/include/configs/tam3517.h
See my previous comment regarding a common configuration file for the
module and a specific file for the boards.
> +/*
> + * High Level Configuration Options
> + */
> +#define CONFIG_ARMV7 1 /* This is an ARM V7 CPU core */
"1" is not needed, this must fixed globally.
> +
> +#define CONFIG_CMDLINE_TAG 1 /* enable passing of ATAGs */
> +#define CONFIG_SETUP_MEMORY_TAGS 1
> +#define CONFIG_INITRD_TAG 1
> +#define CONFIG_REVISION_TAG 1
Ditto.
> +#if CONFIG_DRAM_BUS_WIDTH == 16
> +#define _CONFIG_DRAM_BUS_WIDTH "16"
> +#else
> +#ifndef CONFIG_DRAM_BUS_WIDTH
> +#define CONFIG_DRAM_BUS_WIDTH 32
> +#endif
> +#define _CONFIG_DRAM_BUS_WIDTH "32"
> +#endif
I have not used any of these CONFIG_DRAM_BUS_WIDTH. Why do we need it ?
And I do not see code in u-boot using it. IMHO it is dead code.
> +/* USB
> + * Enable CONFIG_MUSB_HCD for Host functionalities MSC, keyboard
> + * Enable CONFIG_MUSB_UDD for Device functionalities.
> + */
> +#define CONFIG_USB_AM35X 1
> +#define CONFIG_MUSB_HCD 1
Is it working ? I do not understand. I have USB working on the twister
board, but using EHCI-OMAP and with another configuration for the pin
multiplexer.
> +/*-----------------------------------------------------------------------
> + * CFI FLASH driver setup
> + */
> +/* timeout values are in ticks */
> +#define CONFIG_SYS_FLASH_ERASE_TOUT (100 * CONFIG_SYS_HZ)
> +#define CONFIG_SYS_FLASH_WRITE_TOUT (100 * CONFIG_SYS_HZ)
Which CFI has the board ? I suposed none.
> +/*-----------------------------------------------------
> + * ethernet support for TAM3517
> + *-----------------------------------------------------
> + */
> +#define CONFIG_MII
> +
> +#if defined(CONFIG_CMD_NET)
> +/* Disable u-boot support for EMAC LAN, due compile problems
> + Linux kernel support is enough to enable the interface
> +*/
> +/*
> +#define CONFIG_DRIVER_TI_EMAC
> +#define CONFIG_DRIVER_TI_EMAC_USE_RMII
> +#define CONFIG_EMAC_MDIO_PHY_NUM 0
> +#define CONFIG_ARM926EJS 1
> +*/
> +#define CONFIG_NET_RETRY_COUNT 10
> +#define CONFIG_NET_MULTI
> +
> +#define CONFIG_SMC911X 1
> +#define CONFIG_SMC911X_16_BIT 1
> +#define CONFIG_SMC911X_BASE 0x2C000000
> +#define CONFIG_SMC911X_NO_EEPROM 1
> +
The whole setup for SPL is missing. I have already implemented and
tested, and I am asking you how we can proceed at the best. Should I
post my patches (only for TAM3517), too, and then check together what is
still missing ? I can add you in the Signed-off and put you as board
maintainer, as you suggest (by the way, I have not seen a change in the
MAINTAINER file...)
Let me know what you think
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
