diff mbox series

[U-Boot,v1,1/4] toradex: colibri_vf: fix memory initialization

Message ID 20181204101021.22817-2-stefan@agner.ch
State Accepted
Commit fb41ce0db0284e0455b062c92f8d07ad5487851c
Delegated to: Stefano Babic
Headers show
Series ddr: vybrid: various fixes | expand

Commit Message

Stefan Agner Dec. 4, 2018, 10:10 a.m. UTC
From: Stefan Agner <stefan.agner@toradex.com>

Commit 3f353ceccbbb ("vf610: refactor DDRMC code") changed on-die
termination (ODT) values from 120 Ohm to 60 Ohm and enabled a static
read/write leveling which has not been tested with this board. This
commit reverts both changes and makes sure that memory gets
initialized as it has been done before the mentioned commit.

Fixes: 3f353ceccbbb ("vf610: refactor DDRMC code")
Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
Acked-by: Max Krummenacher <max.krummenacher@toradex.com>
---

 board/toradex/colibri_vf/colibri_vf.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

Comments

Lukasz Majewski Dec. 4, 2018, 11:05 a.m. UTC | #1
Hi Stefan,

> From: Stefan Agner <stefan.agner@toradex.com>
> 
> Commit 3f353ceccbbb ("vf610: refactor DDRMC code") changed on-die
> termination (ODT) values from 120 Ohm to 60 Ohm and enabled a static
> read/write leveling which has not been tested with this board. This
> commit reverts both changes and makes sure that memory gets
> initialized as it has been done before the mentioned commit.
> 

This is of course colibri_vf specific code, but I had some issue with
it on my board.

> Fixes: 3f353ceccbbb ("vf610: refactor DDRMC code")
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> Acked-by: Max Krummenacher <max.krummenacher@toradex.com>
> ---
> 
>  board/toradex/colibri_vf/colibri_vf.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/board/toradex/colibri_vf/colibri_vf.c
> b/board/toradex/colibri_vf/colibri_vf.c index 4db1757469..19cf748c5d
> 100644 --- a/board/toradex/colibri_vf/colibri_vf.c
> +++ b/board/toradex/colibri_vf/colibri_vf.c
> @@ -42,14 +42,6 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define USB_CDET_GPIO		102
>  
>  static struct ddrmc_cr_setting colibri_vf_cr_settings[] = {
> -	/* levelling */
> -	{ DDRMC_CR97_WRLVL_EN, 97 },
> -	{ DDRMC_CR98_WRLVL_DL_0(0), 98 },
> -	{ DDRMC_CR99_WRLVL_DL_1(0), 99 },

If you have single DDR3 x16, then this is NOT needed (according to NXP).

> -	{ DDRMC_CR102_RDLVL_REG_EN | DDRMC_CR102_RDLVL_GT_REGEN,
> 102 },
> -	{ DDRMC_CR105_RDLVL_DL_0(0), 105 },

The recommended value for RDLVL_DL_x is 0x20.

> -	{ DDRMC_CR106_RDLVL_GTDL_0(4), 106 },

For the GTDL the recommended value is 0x4.

It is interesting why the board is not working with recommended values.
Probably some other values are wrong - as described in:

https://community.nxp.com/thread/490391

> -	{ DDRMC_CR110_RDLVL_DL_1(0) | DDRMC_CR110_RDLVL_GTDL_1(4),
> 110 }, /* AXI */
>  	{ DDRMC_CR117_AXI0_W_PRI(0) | DDRMC_CR117_AXI0_R_PRI(0),
> 117 }, { DDRMC_CR118_AXI1_W_PRI(1) | DDRMC_CR118_AXI1_R_PRI(1), 118 },
> @@ -88,7 +80,7 @@ static struct ddrmc_cr_setting
> colibri_vf_cr_settings[] = { DDRMC_CR154_PAD_ZQ_MODE(1) |
>  		   DDRMC_CR154_DDR_SEL_PAD_CONTR(3) |
>  		   DDRMC_CR154_PAD_ZQ_HW_FOR(1), 154 },
> -	{ DDRMC_CR155_PAD_ODT_BYTE1(1) |
> DDRMC_CR155_PAD_ODT_BYTE0(1), 155 },
> +	{ DDRMC_CR155_PAD_ODT_BYTE1(2) |
> DDRMC_CR155_PAD_ODT_BYTE0(2), 155 }, { DDRMC_CR158_TWR(6), 158 },
>  	{ DDRMC_CR161_ODT_EN(1) | DDRMC_CR161_TODTH_RD(2) |
>  		   DDRMC_CR161_TODTH_WR(2), 161 },




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Stefan Agner Dec. 4, 2018, 2:48 p.m. UTC | #2
On 04.12.2018 12:05, Lukasz Majewski wrote:
> Hi Stefan,
> 
>> From: Stefan Agner <stefan.agner@toradex.com>
>>
>> Commit 3f353ceccbbb ("vf610: refactor DDRMC code") changed on-die
>> termination (ODT) values from 120 Ohm to 60 Ohm and enabled a static
>> read/write leveling which has not been tested with this board. This
>> commit reverts both changes and makes sure that memory gets
>> initialized as it has been done before the mentioned commit.
>>
> 
> This is of course colibri_vf specific code, but I had some issue with
> it on my board.
> 

Note that this patch *drops* the static leveling.

This values have been introduced in the commit mentioned below, but were
never properly evaluated for this board.

We are in the process of evaluating new values using the DDRv tool, but
I prefer to first drop them until we are ready to upstream the new
values.

--
Stefan


>> Fixes: 3f353ceccbbb ("vf610: refactor DDRMC code")
>> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>> Acked-by: Max Krummenacher <max.krummenacher@toradex.com>
>> ---
>>
>>  board/toradex/colibri_vf/colibri_vf.c | 10 +---------
>>  1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/board/toradex/colibri_vf/colibri_vf.c
>> b/board/toradex/colibri_vf/colibri_vf.c index 4db1757469..19cf748c5d
>> 100644 --- a/board/toradex/colibri_vf/colibri_vf.c
>> +++ b/board/toradex/colibri_vf/colibri_vf.c
>> @@ -42,14 +42,6 @@ DECLARE_GLOBAL_DATA_PTR;
>>  #define USB_CDET_GPIO		102
>>
>>  static struct ddrmc_cr_setting colibri_vf_cr_settings[] = {
>> -	/* levelling */
>> -	{ DDRMC_CR97_WRLVL_EN, 97 },
>> -	{ DDRMC_CR98_WRLVL_DL_0(0), 98 },
>> -	{ DDRMC_CR99_WRLVL_DL_1(0), 99 },
> 
> If you have single DDR3 x16, then this is NOT needed (according to NXP).
> 
>> -	{ DDRMC_CR102_RDLVL_REG_EN | DDRMC_CR102_RDLVL_GT_REGEN,
>> 102 },
>> -	{ DDRMC_CR105_RDLVL_DL_0(0), 105 },
> 
> The recommended value for RDLVL_DL_x is 0x20.
> 
>> -	{ DDRMC_CR106_RDLVL_GTDL_0(4), 106 },
> 
> For the GTDL the recommended value is 0x4.
> 
> It is interesting why the board is not working with recommended values.
> Probably some other values are wrong - as described in:
> 
> https://community.nxp.com/thread/490391
> 
>> -	{ DDRMC_CR110_RDLVL_DL_1(0) | DDRMC_CR110_RDLVL_GTDL_1(4),
>> 110 }, /* AXI */
>>  	{ DDRMC_CR117_AXI0_W_PRI(0) | DDRMC_CR117_AXI0_R_PRI(0),
>> 117 }, { DDRMC_CR118_AXI1_W_PRI(1) | DDRMC_CR118_AXI1_R_PRI(1), 118 },
>> @@ -88,7 +80,7 @@ static struct ddrmc_cr_setting
>> colibri_vf_cr_settings[] = { DDRMC_CR154_PAD_ZQ_MODE(1) |
>>  		   DDRMC_CR154_DDR_SEL_PAD_CONTR(3) |
>>  		   DDRMC_CR154_PAD_ZQ_HW_FOR(1), 154 },
>> -	{ DDRMC_CR155_PAD_ODT_BYTE1(1) |
>> DDRMC_CR155_PAD_ODT_BYTE0(1), 155 },
>> +	{ DDRMC_CR155_PAD_ODT_BYTE1(2) |
>> DDRMC_CR155_PAD_ODT_BYTE0(2), 155 }, { DDRMC_CR158_TWR(6), 158 },
>>  	{ DDRMC_CR161_ODT_EN(1) | DDRMC_CR161_TODTH_RD(2) |
>>  		   DDRMC_CR161_TODTH_WR(2), 161 },
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff mbox series

Patch

diff --git a/board/toradex/colibri_vf/colibri_vf.c b/board/toradex/colibri_vf/colibri_vf.c
index 4db1757469..19cf748c5d 100644
--- a/board/toradex/colibri_vf/colibri_vf.c
+++ b/board/toradex/colibri_vf/colibri_vf.c
@@ -42,14 +42,6 @@  DECLARE_GLOBAL_DATA_PTR;
 #define USB_CDET_GPIO		102
 
 static struct ddrmc_cr_setting colibri_vf_cr_settings[] = {
-	/* levelling */
-	{ DDRMC_CR97_WRLVL_EN, 97 },
-	{ DDRMC_CR98_WRLVL_DL_0(0), 98 },
-	{ DDRMC_CR99_WRLVL_DL_1(0), 99 },
-	{ DDRMC_CR102_RDLVL_REG_EN | DDRMC_CR102_RDLVL_GT_REGEN, 102 },
-	{ DDRMC_CR105_RDLVL_DL_0(0), 105 },
-	{ DDRMC_CR106_RDLVL_GTDL_0(4), 106 },
-	{ DDRMC_CR110_RDLVL_DL_1(0) | DDRMC_CR110_RDLVL_GTDL_1(4), 110 },
 	/* AXI */
 	{ DDRMC_CR117_AXI0_W_PRI(0) | DDRMC_CR117_AXI0_R_PRI(0), 117 },
 	{ DDRMC_CR118_AXI1_W_PRI(1) | DDRMC_CR118_AXI1_R_PRI(1), 118 },
@@ -88,7 +80,7 @@  static struct ddrmc_cr_setting colibri_vf_cr_settings[] = {
 		   DDRMC_CR154_PAD_ZQ_MODE(1) |
 		   DDRMC_CR154_DDR_SEL_PAD_CONTR(3) |
 		   DDRMC_CR154_PAD_ZQ_HW_FOR(1), 154 },
-	{ DDRMC_CR155_PAD_ODT_BYTE1(1) | DDRMC_CR155_PAD_ODT_BYTE0(1), 155 },
+	{ DDRMC_CR155_PAD_ODT_BYTE1(2) | DDRMC_CR155_PAD_ODT_BYTE0(2), 155 },
 	{ DDRMC_CR158_TWR(6), 158 },
 	{ DDRMC_CR161_ODT_EN(1) | DDRMC_CR161_TODTH_RD(2) |
 		   DDRMC_CR161_TODTH_WR(2), 161 },