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

[Tom Rini]

On Wed, Dec 14, 2011 at 3:47 PM, Peter Barada <peter.barada at logicpd.com> wrote:
> From: Peter Barada <peterb at 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