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

Tom Rini
2011-12-15T18:30:43Z

On Thu, Dec 15, 2011 at 10:15 AM, Peter Barada <peter.barada@logicpd.com> wrote:
> 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.

#if 0'd code isn't allowed for merging so I assume this is an RFC :)

> + ? ? ? ? ? ? ? /* Let it soak for a bit */
> + ? ? ? ? ? ? ? for (i = 0; i < 0x100; ++i)
> + ? ? ? ? ? ? ? ? ? ? ? asm("nop");

Can we just use a sdelay(...) here? And in the whole function you
haven't addressed Igor's comment (unless it's incoming and I just
haven't gotten the email yet) about this just being checkboard().

> +/* GPMC CS1 settings for Logic SOM LV/Torpedo LAN92xx Ethernet chip */
> +#define LOGIC_NET_GPMC_CONFIG1 ?0x00001000
> +#define LOGIC_NET_GPMC_CONFIG2 ?0x00080801
> +#define LOGIC_NET_GPMC_CONFIG3 ?0x00000000
> +#define LOGIC_NET_GPMC_CONFIG4 ?0x08010801
> +#define LOGIC_NET_GPMC_CONFIG5 ?0x00080a0a
> +#define LOGIC_NET_GPMC_CONFIG6 ?0x03000280
> +#define LOGIC_NET_GPMC_CONFIG7 ?0x00000848
> +
> +/*
> + * Routine: setup_net_chip
> + * Description: Setting up the configuration GPMC registers specific to the
> + * ? ? ? ? ? ? Ethernet hardware.
> + */
> +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);
> +
> +}

Or this just being an enable_gpmc_cs_config call.

> +int misc_init_r(void)
> +{
> +
> + ? ? ? dieid_num_r();
> +
> + ? ? ? return 0;
> +}

Extra whitespace. I think checkpatch.pl will catch this so please run
your patch through checkpatch.pl, roughly like this:
$ git format-patch -1
$ ./tools/checkpatch.pl 0001-whatever-its-called.patch

[snip]
> +#define CONFIG_L2_OFF ? ? ? ? ? ? ? ? ?/* Keep L2 Cache Disabled */

Why do we want to do this?

[snip]
> +#define CONFIG_SYS_MAXARGS ? ? ? ? ? ? 64 ? ? ?/* max number of command args */

This is very very large and probably doesn't need to be. This is the
number of arguments to the u-boot commands, not to the linux kernel.

Thanks.

--
Tom