Patchwork [U-Boot,v3,06/12] OMAP3: Suffix all Micron memory timing parts with their speed

login
register
mail settings
Submitter Tom Rini
Date Nov. 9, 2011, 5:11 p.m.
Message ID <1320858666-17113-7-git-send-email-trini@ti.com>
Download mbox | patch
Permalink /patch/124623/
State Superseded
Delegated to: Sandeep Paulraj
Headers show

Comments

Tom Rini - Nov. 9, 2011, 5:11 p.m.
Signed-off-by: Tom Rini <trini@ti.com>
---
 arch/arm/include/asm/arch-omap3/mem.h |   50 +++++++++++++++++----------------
 1 files changed, 26 insertions(+), 24 deletions(-)
Heiko Schocher - Nov. 10, 2011, 6:23 a.m.
Hello Tom,

Tom Rini wrote:
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
>  arch/arm/include/asm/arch-omap3/mem.h |   50 +++++++++++++++++----------------
>  1 files changed, 26 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
> index 9775b59..4d69c94 100644
> --- a/arch/arm/include/asm/arch-omap3/mem.h
> +++ b/arch/arm/include/asm/arch-omap3/mem.h
> @@ -144,28 +144,30 @@ enum {
>  		ACTIM_CTRLB(MICRON_TWTR_165, MICRON_TCKE_165,	\
>  				MICRON_TXP_165,	MICRON_XSR_165)
>  
> -#define MICRON_RAMTYPE			0x1
> -#define MICRON_DDRTYPE			0x0
> -#define MICRON_DEEPPD			0x1
> -#define MICRON_B32NOT16			0x1
> -#define MICRON_BANKALLOCATION	0x2
> -#define MICRON_RAMSIZE			((PHYS_SDRAM_1_SIZE/(1024*1024))/2)
> -#define MICRON_ADDRMUXLEGACY	0x1
> -#define MICRON_CASWIDTH			0x5
> -#define MICRON_RASWIDTH			0x2
> -#define MICRON_LOCKSTATUS		0x0
> -#define MICRON_V_MCFG ((MICRON_LOCKSTATUS << 30) | (MICRON_RASWIDTH << 24) | \
> -	(MICRON_CASWIDTH << 20) | (MICRON_ADDRMUXLEGACY << 19) | \
> -	(MICRON_RAMSIZE << 8) | (MICRON_BANKALLOCATION << 6) | \
> -	(MICRON_B32NOT16 << 4) | (MICRON_DEEPPD << 3) | \
> -	(MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE))
> -
> -#define MICRON_BL				0x2
> -#define MICRON_SIL				0x0
> -#define MICRON_CASL				0x3
> -#define MICRON_WBST				0x0
> -#define MICRON_V_MR ((MICRON_WBST << 9) | (MICRON_CASL << 4) | \
> -	(MICRON_SIL << 3) | (MICRON_BL))
> +#define MICRON_RAMTYPE_165		0x1
> +#define MICRON_DDRTYPE_165		0x0
> +#define MICRON_DEEPPD_165		0x1
> +#define MICRON_B32NOT16_165		0x1
> +#define MICRON_BANKALLOCATION_165	0x2
> +#define MICRON_RAMSIZE_165		((PHYS_SDRAM_1_SIZE/(1024*1024))/2)
> +#define MICRON_ADDRMUXLEGACY_165	0x1
> +#define MICRON_CASWIDTH_165		0x5
> +#define MICRON_RASWIDTH_165		0x2
> +#define MICRON_LOCKSTATUS_165		0x0
> +#define MICRON_V_MCFG_165		((MICRON_LOCKSTATUS_165 << 30) | \
                                                                   ^
Please substitute this magic values in this patch by an define.

bye,
Heiko
Tom Rini - Nov. 17, 2011, 9:50 p.m.
On 11/09/2011 11:23 PM, Heiko Schocher wrote:
> Hello Tom,
> 
> Tom Rini wrote:
[snip]
>> +#define MICRON_V_MCFG_165		((MICRON_LOCKSTATUS_165 << 30) | \
>                                                                    ^
> Please substitute this magic values in this patch by an define.

OK, what I've got now is a macro to calculate the MCFG values based on a
few inputs.  What I don't think helps readability is doing:

#define V_MCFG_CASWIDTH_SHIFT 20
#define V_MCFG_CASWIDTH (0x3 << V_MCFG_CASWIDTH_SHIFT)
when
#define V_MCFG_CASWIDTH (0x3 << 20) /* 20:23 */

will do.

It can make sense in places where it's not otherwise clear why you're
doing a shift but in these cases

Patch

diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
index 9775b59..4d69c94 100644
--- a/arch/arm/include/asm/arch-omap3/mem.h
+++ b/arch/arm/include/asm/arch-omap3/mem.h
@@ -144,28 +144,30 @@  enum {
 		ACTIM_CTRLB(MICRON_TWTR_165, MICRON_TCKE_165,	\
 				MICRON_TXP_165,	MICRON_XSR_165)
 
-#define MICRON_RAMTYPE			0x1
-#define MICRON_DDRTYPE			0x0
-#define MICRON_DEEPPD			0x1
-#define MICRON_B32NOT16			0x1
-#define MICRON_BANKALLOCATION	0x2
-#define MICRON_RAMSIZE			((PHYS_SDRAM_1_SIZE/(1024*1024))/2)
-#define MICRON_ADDRMUXLEGACY	0x1
-#define MICRON_CASWIDTH			0x5
-#define MICRON_RASWIDTH			0x2
-#define MICRON_LOCKSTATUS		0x0
-#define MICRON_V_MCFG ((MICRON_LOCKSTATUS << 30) | (MICRON_RASWIDTH << 24) | \
-	(MICRON_CASWIDTH << 20) | (MICRON_ADDRMUXLEGACY << 19) | \
-	(MICRON_RAMSIZE << 8) | (MICRON_BANKALLOCATION << 6) | \
-	(MICRON_B32NOT16 << 4) | (MICRON_DEEPPD << 3) | \
-	(MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE))
-
-#define MICRON_BL				0x2
-#define MICRON_SIL				0x0
-#define MICRON_CASL				0x3
-#define MICRON_WBST				0x0
-#define MICRON_V_MR ((MICRON_WBST << 9) | (MICRON_CASL << 4) | \
-	(MICRON_SIL << 3) | (MICRON_BL))
+#define MICRON_RAMTYPE_165		0x1
+#define MICRON_DDRTYPE_165		0x0
+#define MICRON_DEEPPD_165		0x1
+#define MICRON_B32NOT16_165		0x1
+#define MICRON_BANKALLOCATION_165	0x2
+#define MICRON_RAMSIZE_165		((PHYS_SDRAM_1_SIZE/(1024*1024))/2)
+#define MICRON_ADDRMUXLEGACY_165	0x1
+#define MICRON_CASWIDTH_165		0x5
+#define MICRON_RASWIDTH_165		0x2
+#define MICRON_LOCKSTATUS_165		0x0
+#define MICRON_V_MCFG_165		((MICRON_LOCKSTATUS_165 << 30) | \
+		(MICRON_RASWIDTH_165 << 24) | (MICRON_CASWIDTH_165 << 20) | \
+		(MICRON_ADDRMUXLEGACY_165 << 19) | (MICRON_RAMSIZE_165 << 8) | \
+		(MICRON_BANKALLOCATION_165 << 6) | \
+		(MICRON_B32NOT16_165 << 4) | (MICRON_DEEPPD_165 << 3) | \
+		(MICRON_DDRTYPE_165 << 2) | (MICRON_RAMTYPE_165))
+
+#define MICRON_BL_165			0x2
+#define MICRON_SIL_165			0x0
+#define MICRON_CASL_165			0x3
+#define MICRON_WBST_165			0x0
+#define MICRON_V_MR_165			((MICRON_WBST_165 << 9) | \
+		(MICRON_CASL_165 << 4) | (MICRON_SIL_165 << 3) | \
+		(MICRON_BL_165))
 
 /* NUMONYX part of IGEP v2 (165MHz optimized) 6.06ns */
 #define NUMONYX_TDAL_165	6	/* Twr/Tck + Trp/tck		*/
@@ -201,9 +203,9 @@  enum {
 #ifdef CONFIG_OMAP3_MICRON_DDR
 #define V_ACTIMA_165		MICRON_V_ACTIMA_165
 #define V_ACTIMB_165		MICRON_V_ACTIMB_165
-#define V_MCFG			MICRON_V_MCFG
+#define V_MCFG			MICRON_V_MCFG_165
 #define V_RFR_CTRL		SDP_3430_SDRC_RFR_CTRL_165MHz
-#define V_MR			MICRON_V_MR
+#define V_MR			MICRON_V_MR_165
 #endif
 
 #ifdef CONFIG_OMAP3_NUMONYX_DDR