On 11/15/2010 04:32 PM, Ben Gardiner wrote:

>> --- /dev/null
>> +++ b/board/davinci/ea20/ea20.c
>> @@ -0,0 +1,237 @@
>> +/*
>> + * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com/
>> + *
>> + * Based on da830evm.c. Original Copyrights follow:
> 
> It seems to me that this file is based off of da850evm.c;

Correct.

> the
> da850ev.c file contains this comment which is where it likely came
> from. If it is based on da850evm.c then I think the file's comment
> should reflect this.

This is part of the original copyright in the da850evm.c, and this is
the reason why I have not changed it. Probably to be more correct I
should add at the beginning another comment, inidicating where the file
comes from.

> 
>> [...]
>> +
>> +static void da850_emac_mii_mode_sel(int mode_sel)
>> +{
>> +       int val;
>> +
>> +       val = readl(&davinci_syscfg_regs->cfgchip3);
>> +       if (mode_sel == 0)
>> +               val &= ~(1 << 8);
>> +       else
>> +               val |= (1 << 8);
>> +       writel(val, &davinci_syscfg_regs->cfgchip3);
>> +}
> 
> This is a function common to any da850 board using RMII, could it be
> extracted to a shared .c file?

Agree, the function is a good candidate to be moved into misc.c :-).

> 
>> [...]
>> +#ifdef CONFIG_NAND_DAVINCI
>> +       /*
>> +        * NAND CS setup - cycle counts based on da850evm NAND timings in the
>> +        * Linux kernel @ 25MHz EMIFA
>> +        */
>> +       writel((DAVINCI_ABCR_WSETUP(0) |
>> +               DAVINCI_ABCR_WSTROBE(0) |
>> +               DAVINCI_ABCR_WHOLD(0) |
>> +               DAVINCI_ABCR_RSETUP(0) |
>> +               DAVINCI_ABCR_RSTROBE(1) |
>> +               DAVINCI_ABCR_RHOLD(0) |
>> +               DAVINCI_ABCR_TA(0) |
>> +               DAVINCI_ABCR_ASIZE_8BIT),
>> +              &davinci_emif_regs->ab2cr); /* CS3 */
>> +#endif
> 
> This looks like it was copied from da850evm.c; note that these timings
> are based on the integer multiples of EMA_CLK periods that meet the
> timing specifications of the NAND chip found on the UI board. A
> different part could have different timings...

The ea20 board will support NAND, but not at the moment. There will be a
future release with storage on board, I will drop these NAND definitions
because they do not reflect the actual hardware.

> 
> Also, from your description of the ea20: "It supports SPI Flash,
> Ethernet with RMII." -- there is no NAND support.

No NAND support - defines are useless now ;-)

> Perhaps it would be prudent to _enable_ the MDC line via the GPIO2[6]
> pin instead of a no-op.

It should be not necessary. U-Boot is stored on the SPI flash, and if
the SPI is not enabled at reset by the HW, the board cannot boot. It
seems to me it is already ensured that the buffer output is enabled and
the code does not need to set it.

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

Reply via email to