diff mbox

[U-Boot,v3,05/12] OMAP3: Add optimal SDRC autorefresh control values

Message ID 1320858666-17113-6-git-send-email-trini@ti.com
State Superseded
Delegated to: Sandeep Paulraj
Headers show

Commit Message

Tom Rini Nov. 9, 2011, 5:10 p.m. UTC
This adds the optimal SDRC autorefresh control register values for
100Mhz, 133MHz, 165MHz and 200MHz clocks.  We switch to using this
to provide the default 165MHz value.

Signed-off-by: Tom Rini <trini@ti.com>
---
 arch/arm/include/asm/arch-omap3/mem.h |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

Comments

Heiko Schocher Nov. 10, 2011, 6:20 a.m. UTC | #1
Hello Tom,

Tom Rini wrote:
> This adds the optimal SDRC autorefresh control register values for
> 100Mhz, 133MHz, 165MHz and 200MHz clocks.  We switch to using this
> to provide the default 165MHz value.
> 
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
>  arch/arm/include/asm/arch-omap3/mem.h |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
> index db6a696..9775b59 100644
> --- a/arch/arm/include/asm/arch-omap3/mem.h
> +++ b/arch/arm/include/asm/arch-omap3/mem.h
> @@ -43,6 +43,12 @@ enum {
>  #define SDRC_SHARING	0x00000100
>  #define SDRC_MR_0_SDR	0x00000031
>  
> +/* optimized timings good for current shipping parts */
> +#define SDP_3430_SDRC_RFR_CTRL_100MHz	0x0002da01
> +#define SDP_3430_SDRC_RFR_CTRL_133MHz	0x0003de01 /* 7.8us/7.5ns - 50=0x3de */
> +#define SDP_3430_SDRC_RFR_CTRL_165MHz	0x0004e201 /* 7.8us/6ns - 50=0x4e2 */
> +#define SDP_3430_SDRC_RFR_CTRL_200MHz	0x0005e601 /* 7.8us/5ns - 50=0x5e6 */

You should use something like that here:

#define OMAP3_SDP_SDRC_xx_SHIFT	8
#define OMAP3_SDP_SDRC_yy	(1 << 0)

#define SDP_3430_SDRC_RFR_CTRL_200MHz ((0x5e6 << OMAP3_SDP_SDRC_xx_SHIFT) |
					OMAP3_SDP_SDRC_yy)

of course with the right "names" for xx and yy, I have not
the cpu doc at hand actual.

bye,
Heiko
Robert Hurdle Nov. 10, 2011, 9:32 p.m. UTC | #2
Really?

Using defines for the number of bits something gets shifted IN A DEFINE?

Doesn't that just obscure how it works?

And filling defines with shifts of ZERO bits just creates more noise.

Robert Hurdle

-----Original Message-----
From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Heiko Schocher
Sent: Wednesday, November 09, 2011 10:21 PM
To: Tom Rini
Cc: u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH v3 05/12] OMAP3: Add optimal SDRC autorefresh control values

Hello Tom,

Tom Rini wrote:
> This adds the optimal SDRC autorefresh control register values for 
> 100Mhz, 133MHz, 165MHz and 200MHz clocks.  We switch to using this to 
> provide the default 165MHz value.
> 
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
>  arch/arm/include/asm/arch-omap3/mem.h |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-omap3/mem.h 
> b/arch/arm/include/asm/arch-omap3/mem.h
> index db6a696..9775b59 100644
> --- a/arch/arm/include/asm/arch-omap3/mem.h
> +++ b/arch/arm/include/asm/arch-omap3/mem.h
> @@ -43,6 +43,12 @@ enum {
>  #define SDRC_SHARING	0x00000100
>  #define SDRC_MR_0_SDR	0x00000031
>  
> +/* optimized timings good for current shipping parts */
> +#define SDP_3430_SDRC_RFR_CTRL_100MHz	0x0002da01
> +#define SDP_3430_SDRC_RFR_CTRL_133MHz	0x0003de01 /* 7.8us/7.5ns - 50=0x3de */
> +#define SDP_3430_SDRC_RFR_CTRL_165MHz	0x0004e201 /* 7.8us/6ns - 50=0x4e2 */
> +#define SDP_3430_SDRC_RFR_CTRL_200MHz	0x0005e601 /* 7.8us/5ns - 50=0x5e6 */

You should use something like that here:

#define OMAP3_SDP_SDRC_xx_SHIFT	8
#define OMAP3_SDP_SDRC_yy	(1 << 0)

#define SDP_3430_SDRC_RFR_CTRL_200MHz ((0x5e6 << OMAP3_SDP_SDRC_xx_SHIFT) |
					OMAP3_SDP_SDRC_yy)

of course with the right "names" for xx and yy, I have not the cpu doc at hand actual.

bye,
Heiko
Tom Rini Nov. 17, 2011, 9:44 p.m. UTC | #3
On 11/09/2011 11:20 PM, Heiko Schocher wrote:
> Hello Tom,
> 
> Tom Rini wrote:
>> This adds the optimal SDRC autorefresh control register values for
>> 100Mhz, 133MHz, 165MHz and 200MHz clocks.  We switch to using this
>> to provide the default 165MHz value.
[snip]
>> +#define SDP_3430_SDRC_RFR_CTRL_133MHz	0x0003de01 /* 7.8us/7.5ns - 50=0x3de */
>> +#define SDP_3430_SDRC_RFR_CTRL_165MHz	0x0004e201 /* 7.8us/6ns - 50=0x4e2 */
>> +#define SDP_3430_SDRC_RFR_CTRL_200MHz	0x0005e601 /* 7.8us/5ns - 50=0x5e6 */
> 
> You should use something like that here:
> 
> #define OMAP3_SDP_SDRC_xx_SHIFT	8
> #define OMAP3_SDP_SDRC_yy	(1 << 0)
> 
> #define SDP_3430_SDRC_RFR_CTRL_200MHz ((0x5e6 << OMAP3_SDP_SDRC_xx_SHIFT) |
> 					OMAP3_SDP_SDRC_yy)

OK, I hadn't forgotten about this, I've just been a bit busy.  I broke
out the TRM, split these values out into binary and, breaking it out
into shifts won't help make it more understandable.  It needs a better
comment, which I will happily do.  Bits 1:0 are autofresh enable is 0x1
and 0x2/0x3 are bursts.  7:2 are reserved (as are 24:31) and 8:23 are
autorefresh counter value.
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
index db6a696..9775b59 100644
--- a/arch/arm/include/asm/arch-omap3/mem.h
+++ b/arch/arm/include/asm/arch-omap3/mem.h
@@ -43,6 +43,12 @@  enum {
 #define SDRC_SHARING	0x00000100
 #define SDRC_MR_0_SDR	0x00000031
 
+/* optimized timings good for current shipping parts */
+#define SDP_3430_SDRC_RFR_CTRL_100MHz	0x0002da01
+#define SDP_3430_SDRC_RFR_CTRL_133MHz	0x0003de01 /* 7.8us/7.5ns - 50=0x3de */
+#define SDP_3430_SDRC_RFR_CTRL_165MHz	0x0004e201 /* 7.8us/6ns - 50=0x4e2 */
+#define SDP_3430_SDRC_RFR_CTRL_200MHz	0x0005e601 /* 7.8us/5ns - 50=0x5e6 */
+
 #define DLL_OFFSET		0
 #define DLL_WRITEDDRCLKX2DIS	1
 #define DLL_ENADLL		1
@@ -154,10 +160,6 @@  enum {
 	(MICRON_B32NOT16 << 4) | (MICRON_DEEPPD << 3) | \
 	(MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE))
 
-#define MICRON_ARCV				2030
-#define MICRON_ARE				0x1
-#define MICRON_V_RFR_CTRL ((MICRON_ARCV << 8) | (MICRON_ARE))
-
 #define MICRON_BL				0x2
 #define MICRON_SIL				0x0
 #define MICRON_CASL				0x3
@@ -200,7 +202,7 @@  enum {
 #define V_ACTIMA_165		MICRON_V_ACTIMA_165
 #define V_ACTIMB_165		MICRON_V_ACTIMB_165
 #define V_MCFG			MICRON_V_MCFG
-#define V_RFR_CTRL		MICRON_V_RFR_CTRL
+#define V_RFR_CTRL		SDP_3430_SDRC_RFR_CTRL_165MHz
 #define V_MR			MICRON_V_MR
 #endif