[U-Boot] [PATCH 1/1] ARMV7: Add support For Logic OMAP35x/DM37x modules

Tom Rini
2011-12-15T00:15:13Z

On Wed, Dec 14, 2011 at 3:47 PM, Peter Barada <peter.barada@logicpd.com> wrote:
> From: Peter Barada <peterb@logicpd.com>
>
> This patch adds basic support for OMAP35x/DM37x SOM LV/Torpedo
> reference boards. It assumes U-boot is loaded to SDRAM with the
> help of another small bootloader (x-load) running from SRAM.

We can't merge this with the existing am3517evm support? Also, have
you tried out SPL for this board yet? It'd probably be good to switch
from x-loader.

[snip]
> +++ b/board/logicpd/omap3som/config.mk
> +CONFIG_SYS_TEXT_BASE = 0x80400000

This should be in the config file.

> diff --git a/board/logicpd/omap3som/omap3logic.c b/board/logicpd/omap3som/omap3logic.c
[snip]
> +unsigned int logic_identify(void)

Should be 'static'

> +{
> + ? ? ? unsigned int val = 0;
> + ? ? ? u32 cpu_family = get_cpu_family();
> + ? ? ? int i;
> +
> + ? ? ? MUX_LOGIC_HSUSB0_DATA5_GPIO_MUX();
> +
> + ? ? ? if (!gpio_request(189, "")) {
> +
> + ? ? ? ? ? ? ? gpio_direction_output(189, 0);
> + ? ? ? ? ? ? ? gpio_set_value(189, 1);
> +
> + ? ? ? ? ? ? ? /* Let it soak for a bit */
> + ? ? ? ? ? ? ? for (i = 0; i < 0x100; ++i)
> + ? ? ? ? ? ? ? ? ? ? ? asm("nop");

Just use one of the existing delay functions?

> + ? ? ? ? ? ? ? printf("Board:");
> + ? ? ? ? ? ? ? if (cpu_family == CPU_OMAP36XX) {
> + ? ? ? ? ? ? ? ? ? ? ? printf(" DM37xx");
> + ? ? ? ? ? ? ? ? ? ? ? if (val) {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? printf(" Torpedo\n");
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? val = MACH_TYPE_DM3730_TORPEDO;
> + ? ? ? ? ? ? ? ? ? ? ? } else {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? printf(" SOM LV\n");
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? val = MACH_TYPE_DM3730_SOM_LV;
> + ? ? ? ? ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? } else {
> + ? ? ? ? ? ? ? ? ? ? ? printf(" OMAP35xx");
> + ? ? ? ? ? ? ? ? ? ? ? if (val) {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? printf(" Torpedo\n");
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? val = MACH_TYPE_OMAP3_TORPEDO;
> + ? ? ? ? ? ? ? ? ? ? ? } else {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? printf(" SOM LV\n");
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? val = MACH_TYPE_OMAP3530_LV_SOM;
> + ? ? ? ? ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? }
> + ? ? ? }

This could be condensed into just two checks, cpu_family and then val.

> +static void setup_net_chip(void)
> +{
> + ? ? ? struct ctrl *ctrl_base = (struct ctrl *)OMAP34XX_CTRL_BASE;
> +
> + ? ? ? /* Configure GPMC registers */
> + ? ? ? writel(LOGIC_NET_GPMC_CONFIG1, &gpmc_cfg->cs[1].config1);
> + ? ? ? writel(LOGIC_NET_GPMC_CONFIG2, &gpmc_cfg->cs[1].config2);
> + ? ? ? writel(LOGIC_NET_GPMC_CONFIG3, &gpmc_cfg->cs[1].config3);
> + ? ? ? writel(LOGIC_NET_GPMC_CONFIG4, &gpmc_cfg->cs[1].config4);
> + ? ? ? writel(LOGIC_NET_GPMC_CONFIG5, &gpmc_cfg->cs[1].config5);
> + ? ? ? writel(LOGIC_NET_GPMC_CONFIG6, &gpmc_cfg->cs[1].config6);
> + ? ? ? writel(LOGIC_NET_GPMC_CONFIG7, &gpmc_cfg->cs[1].config7);
> +
> + ? ? ? /* Enable off mode for NWE in PADCONF_GPMC_NWE register */
> + ? ? ? writew(readw(&ctrl_base->gpmc_nwe) | 0x0E00, &ctrl_base->gpmc_nwe);
> + ? ? ? /* Enable off mode for NOE in PADCONF_GPMC_NADV_ALE register */
> + ? ? ? writew(readw(&ctrl_base->gpmc_noe) | 0x0E00, &ctrl_base->gpmc_noe);
> + ? ? ? /* Enable off mode for ALE in PADCONF_GPMC_NADV_ALE register */
> + ? ? ? writew(readw(&ctrl_base->gpmc_nadv_ale) | 0x0E00,
> + ? ? ? ? ? ? ? &ctrl_base->gpmc_nadv_ale);

Please switch this to enable_gpmc_cs_config(..) and note that config7
is computed so shouldn't be #defined.

[snip]
> +int board_late_init(void)
> +{
> + ? ? ? unsigned char enetaddr[6];
> +
> +#ifdef CONFIG_CMD_CACHE
> + ? ? ? dcache_enable();
> + ? ? ? printf("Data (writethrough) Cache is %s\n",
> + ? ? ? ? ? ? ? dcache_status() ? "ON" : "OFF");
> +#endif
> + ? ? ? return 0;
> +}

Cache should be defaulting to enabled already and enetaddr is unused,
so you could drop board_late_init.

> +
> +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD)
> +int board_mmc_init(bd_t *bis)
> +{
> + ? ? ? omap_mmc_init(0);
> + ? ? ? return 0;
> +}
> +#endif

Just a one line return omap_mmc_init(0);

> +/*
> + * Routine: misc_init_r
> + * Description: Init ethernet (done here so udelay works)
> + */
> +int misc_init_r(void)
> +{
> +#if defined(CONFIG_CMD_NET)
> + ? ? ? setup_net_chip();
> +#endif
> +
> + ? ? ? dieid_num_r();
> +
> + ? ? ? return 0;
> +}
> +
> +
> +
> +int board_eth_init(bd_t *bis)
> +{
> + ? ? ? int rc = 0;
> +#ifdef CONFIG_SMC911X
> + ? ? ? rc = smc911x_initialize(0, CONFIG_SMC911X_BASE);
> +#endif
> + ? ? ? return rc;
> +}

Just return smc911x_..(...); and iirc you only need to provide
board_eth_init when CONFIG_CMD_NET or perhaps CONFIG_SMC911X is set.

[snip]
> +++ b/include/configs/omap3_logic.h
[snip]
> +#define CONFIG_ARMV7 ? ? ? ? ? 1 ? ? ? /* This is an ARM V7 CPU core */
> +#define CONFIG_OMAP ? ? ? ? ? ?1 ? ? ? /* in a TI OMAP core */

In this file at large, two problems. One, you've got 'dead' #defines
like CONFIG_ARMV7 and CONFIG_NET_MULTI (and probably others, there's
been a number of recent threads and patches removing stuff I think I
see you bringing back) and two the preferred syntax is just '#define
CONFIG_FOO' for on/off stuff.

Thanks!

--
Tom