On 12/05/2011 03:20 AM, Christian Riesch wrote:
Hi Tom,
On Sat, Dec 3, 2011 at 6:49 AM, Christian Riesch christian.riesch@omicron.at wrote:
Hi Tom, Thanks for your comments.
On Friday, December 2, 2011, Tom Rini trini@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.
I'd say it's not obvious that a value of zero means one clock cycle, so drop the define and just comment what the full value contains.