[U-Boot] [PATCH v3 13/15] arm, da850evm: Add an SPL for SPI boot

[Christian Riesch]

Hi Tom,

On Sat, Dec 3, 2011 at 6:49 AM, Christian Riesch
<christian.riesch at omicron.at> wrote:
> Hi Tom,
> Thanks for your comments.
>
>
> On Friday, December 2, 2011, Tom Rini <trini at ti.com> wrote:
>> On 12/02/2011 09:12 AM, Christian Riesch wrote:
>>
>> [snip]
>>>  include/configs/da850evm.h            |   87
>>> +++++++++++++++++++++++++++++++++
>> [snip]
>>> +#define CONFIG_SYS_DA850_DDR2_SDTIMR (0 |    \
>>> +     (14 << DV_DDR_SDTMR1_RFC_SHIFT) |       \
>>> +     (2 << DV_DDR_SDTMR1_RP_SHIFT) |         \
>>> +     (2 << DV_DDR_SDTMR1_RCD_SHIFT) |        \
>>> +     (1 << DV_DDR_SDTMR1_WR_SHIFT) |         \
>>> +     (5 << DV_DDR_SDTMR1_RAS_SHIFT) |        \
>>> +     (8 << DV_DDR_SDTMR1_RC_SHIFT) |         \
>>> +     (1 << DV_DDR_SDTMR1_RRD_SHIFT) |        \
>>> +     (0 << DV_DDR_SDTMR1_WTR_SHIFT))
>>
>> '0 | ..' and '0 << ...' don't help readability over just value saying it
>> (same with shifting 0).  Also, unless the manual these come from uses
>> decimal here, hex is preferred.  Thanks!
>>
>
> '0 | ...'.   I agree, I'll remove this.
>
> '0 << ...'
> Aaaahhh... Yes, that's pretty useless here, the WTR bits are reserved bits
> :-/
>

Uh, sorry, they are not reserved. But a zero value here means that we
have one clock cycle (because it's number of clock cycles minus one).
So I'd like to keep the line because it will help others to see that
WTR is set to one clock cycle.
Christian

> Decimal values: I'd like to keep them. This is DDR timing, the numbers mean
> number of clock cycles minus one. So actually (1 << DV_DDR_SDTMR1_RRD_SHIFT)
> means that trrd=13.3333... ns (two clock cycles) at 150 MHz. So I must
> calculate with these numbers and this is why I prefer decimal over hex. I am
> lazy ;-)
>
> Christian
>
>
>
>> --
>> Tom
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>