On 12/15/2011 01:30 PM, Tom Rini wrote:
> On Thu, Dec 15, 2011 at 10:15 AM, Peter Barada <peter.bar...@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 :)
My bad - sent the wrong patch. I'll update and send again.
>> + /* 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().
Done - "sdelay(0x100)" instead of the delay loop.
As for the function, in arch/arm/cpu/armv7/omap3/board.c there's already
a checkboard() function that is called early (before relocation) that
prints out the banner. I tried to make that a weak alias and override
it in my board file, but when its called, gd->bd is not setup so that
code aborts when it tries to set gd->bd->bi_arch_number. I then tried
storing the value in a global and then set gd->bd->bi_arch_number in
board_init(), but between the call to checkboard() and board_init() the
BSS section is zeroed. If I stored/load the computed bi_arch_number
into a non-zero static value it works, but I feel that is really fragile.
Remember that identify_board() not only prints a banner but also
determines one of four machine IDs passed to the linux kernel to
identify which of the variants its running on.
>> +/* 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.
Done.
>> +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
I rand checkpatch.pl over the result of "git diff --cached" and it
didn't complain about the emty line after the open brace. I've removed it.
> [snip]
>> +#define CONFIG_L2_OFF /* Keep L2 Cache Disabled */
> Why do we want to do this?
This is a holdover from a previous verison ofu-boot (2010.06) where I
added LCD/HDMI support for up to a 720p 32bpp display and when I did
that the kernel wouldn't start w/o disabling L2 in u-boot (possibly
because the reserved memory for the display in u-boot alone is larger
than the L2 cache in a OMAP35x/DM37x), but I never figured out why it
failed. I've removed it and will readdress when I add in the LCD/HDMI
support.
> [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.
You're right, yet another holdover from an older version of u-boot.
I've reduced it 16 to match up with most of the other boards.
So the big question before I submit V3 of this patch is whether to
include overriding OMAP3's checkboard() function or to use
identify_board() as is to compute bi_arch_number... I can do it either
way, but don't want to waste anyone's time (with the wrong method).
Thanks in advance!
--
Peter Barada
peter.bar...@logicpd.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
