[U-Boot] [PATCH v1 0/4] ddr: vybrid: various fixes

[U-Boot] [PATCH v1 0/4] ddr: vybrid: various fixes

[Stefan Agner]

From: Stefan Agner <stefan.agner@toradex.com>

Some random fixes in vf610 DDR controller initialization.
Especially the first patch fixes boot on some Toradex
Colibri VFxx modules.

--
Stefan


Stefan Agner (4):
  toradex: colibri_vf: fix memory initialization
  ARM: vf610: ddrmc: fix CR138 preprocessor define
  ARM: vf610: ddrmc: fix initialization completion detection
  ARM: vf610: ddrmc: do not write CR79 by default

 arch/arm/include/asm/arch-vf610/imx-regs.h |  5 +++--
 arch/arm/mach-imx/ddrmc-vf610.c            |  4 ++--
 board/toradex/colibri_vf/colibri_vf.c      | 10 +---------
 3 files changed, 6 insertions(+), 13 deletions(-)

-- 
2.19.1

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

[Stefan Agner]

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(-)

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 },
-- 
2.19.1

[U-Boot] [PATCH v1 2/4] ARM: vf610: ddrmc: fix CR138 preprocessor define

[Stefan Agner]

From: Stefan Agner <stefan.agner@toradex.com>

According to the data sheet bits 10-8 are PHYDRAM_CK_EN. Fix mask
to allow setting PHYDRAM_CK_EN correctly.

Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 arch/arm/include/asm/arch-vf610/imx-regs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-vf610/imx-regs.h b/arch/arm/include/asm/arch-vf610/imx-regs.h
index 08ba8e94f8..b7374bfb8f 100644
--- a/arch/arm/include/asm/arch-vf610/imx-regs.h
+++ b/arch/arm/include/asm/arch-vf610/imx-regs.h
@@ -239,7 +239,7 @@
 #define DDRMC_CR132_RDLAT_ADJ(v)			((v) & 0x3f)
 #define DDRMC_CR137_PHYCTL_DL(v)			(((v) & 0xf) << 16)
 #define DDRMC_CR138_PHY_WRLV_MXDL(v)			(((v) & 0xffff) << 16)
-#define DDRMC_CR138_PHYDRAM_CK_EN(v)			(((v) & 0x8) << 8)
+#define DDRMC_CR138_PHYDRAM_CK_EN(v)			(((v) & 0x7) << 8)
 #define DDRMC_CR139_PHY_WRLV_RESPLAT(v)			(((v) & 0xff) << 24)
 #define DDRMC_CR139_PHY_WRLV_LOAD(v)			(((v) & 0xff) << 16)
 #define DDRMC_CR139_PHY_WRLV_DLL(v)			(((v) & 0xff) << 8)
-- 
2.19.1

[U-Boot] [PATCH v1 3/4] ARM: vf610: ddrmc: fix initialization completion detection

[Stefan Agner]

From: Stefan Agner <stefan.agner@toradex.com>

The CR80 register has multiple interrupt bits, the code is supposed
to check bit 8 but instead uses a logical and. In most cases this
probably did not affect real operations since at that stage typically
none of the other bits are set.

Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 arch/arm/include/asm/arch-vf610/imx-regs.h | 3 ++-
 arch/arm/mach-imx/ddrmc-vf610.c            | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-vf610/imx-regs.h b/arch/arm/include/asm/arch-vf610/imx-regs.h
index b7374bfb8f..f71fbf4e73 100644
--- a/arch/arm/include/asm/arch-vf610/imx-regs.h
+++ b/arch/arm/include/asm/arch-vf610/imx-regs.h
@@ -200,7 +200,8 @@
 #define DDRMC_CR78_Q_FULLNESS(v)			(((v) & 0x7) << 24)
 #define DDRMC_CR78_BUR_ON_FLY_BIT(v)			((v) & 0xf)
 #define DDRMC_CR79_CTLUPD_AREF(v)			(((v) & 0x1) << 24)
-#define DDRMC_CR82_INT_MASK				0x10000000
+#define DDRMC_CR80_MC_INIT_COMPLETE			(1 << 8)
+#define DDRMC_CR82_INT_MASK				(1 << 28)
 #define DDRMC_CR87_ODT_WR_MAPCS0(v)			((v) << 24)
 #define DDRMC_CR87_ODT_RD_MAPCS0(v)			((v) << 16)
 #define DDRMC_CR88_TODTL_CMD(v)				(((v) & 0x1f) << 16)
diff --git a/arch/arm/mach-imx/ddrmc-vf610.c b/arch/arm/mach-imx/ddrmc-vf610.c
index 3d7da1c25e..d121a53898 100644
--- a/arch/arm/mach-imx/ddrmc-vf610.c
+++ b/arch/arm/mach-imx/ddrmc-vf610.c
@@ -231,6 +231,7 @@ void ddrmc_ctrl_init_ddr3(struct ddr3_jedec_timings const *timings,
 	/* all inits done, start the DDR controller */
 	writel(DDRMC_CR00_DRAM_CLASS_DDR3 | DDRMC_CR00_START, &ddrmr->cr[0]);
 
-	while (!(readl(&ddrmr->cr[80]) && 0x100))
+	while (!(readl(&ddrmr->cr[80]) & DDRMC_CR80_MC_INIT_COMPLETE))
 		udelay(10);
+	writel(DDRMC_CR80_MC_INIT_COMPLETE, &ddrmr->cr[81]);
 }
-- 
2.19.1

[U-Boot] [PATCH v1 4/4] ARM: vf610: ddrmc: do not write CR79 by default

[Stefan Agner]

From: Stefan Agner <stefan.agner@toradex.com>

The current value CTLUPD_AREF(0) is the reset value of the register,
so there is no need to write a value. If needed, the register can be
written using board specific CR settings.

Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 arch/arm/mach-imx/ddrmc-vf610.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-imx/ddrmc-vf610.c b/arch/arm/mach-imx/ddrmc-vf610.c
index d121a53898..d4926b5cee 100644
--- a/arch/arm/mach-imx/ddrmc-vf610.c
+++ b/arch/arm/mach-imx/ddrmc-vf610.c
@@ -188,7 +188,6 @@ void ddrmc_ctrl_init_ddr3(struct ddr3_jedec_timings const *timings,
 		   DDRMC_CR77_SWAP_EN, &ddrmr->cr[77]);
 	writel(DDRMC_CR78_Q_FULLNESS(timings->q_fullness) |
 		   DDRMC_CR78_BUR_ON_FLY_BIT(12), &ddrmr->cr[78]);
-	writel(DDRMC_CR79_CTLUPD_AREF(0), &ddrmr->cr[79]);
 
 	writel(DDRMC_CR82_INT_MASK, &ddrmr->cr[82]);
 
-- 
2.19.1

[U-Boot] [PATCH v1 2/4] ARM: vf610: ddrmc: fix CR138 preprocessor define

[Lukasz Majewski]

Hi Stefan,

> From: Stefan Agner <stefan.agner@toradex.com>
> 
> According to the data sheet bits 10-8 are PHYDRAM_CK_EN. Fix mask
> to allow setting PHYDRAM_CK_EN correctly.
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
> 
>  arch/arm/include/asm/arch-vf610/imx-regs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/arch-vf610/imx-regs.h
> b/arch/arm/include/asm/arch-vf610/imx-regs.h index
> 08ba8e94f8..b7374bfb8f 100644 ---
> a/arch/arm/include/asm/arch-vf610/imx-regs.h +++
> b/arch/arm/include/asm/arch-vf610/imx-regs.h @@ -239,7 +239,7 @@
>  #define DDRMC_CR132_RDLAT_ADJ(v)			((v) & 0x3f)
>  #define DDRMC_CR137_PHYCTL_DL(v)			(((v) & 0xf)
> << 16) #define DDRMC_CR138_PHY_WRLV_MXDL(v)
> (((v) & 0xffff) << 16) -#define
> DDRMC_CR138_PHYDRAM_CK_EN(v)			(((v) & 0x8) <<
> 8) +#define DDRMC_CR138_PHYDRAM_CK_EN(v)			(((v)
> & 0x7) << 8) #define
> DDRMC_CR139_PHY_WRLV_RESPLAT(v)			(((v) & 0xff)
> << 24) #define DDRMC_CR139_PHY_WRLV_LOAD(v)
> (((v) & 0xff) << 16) #define
> DDRMC_CR139_PHY_WRLV_DLL(v)			(((v) & 0xff) << 8)

Reviewed-by: Lukasz Majewski <lukma@denx.de>


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 at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181204/4a03f01a/attachment.sig>

[U-Boot] [PATCH v1 3/4] ARM: vf610: ddrmc: fix initialization completion detection

[Lukasz Majewski]

Hi Stefan,

> From: Stefan Agner <stefan.agner@toradex.com>
> 
> The CR80 register has multiple interrupt bits, the code is supposed
> to check bit 8 but instead uses a logical and. In most cases this
> probably did not affect real operations since at that stage typically
> none of the other bits are set.

It can exit the loop when any error bit is set (but anyway with broken
DDR initialization we hang latter anyway).

> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
> 
>  arch/arm/include/asm/arch-vf610/imx-regs.h | 3 ++-
>  arch/arm/mach-imx/ddrmc-vf610.c            | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-vf610/imx-regs.h
> b/arch/arm/include/asm/arch-vf610/imx-regs.h index
> b7374bfb8f..f71fbf4e73 100644 ---
> a/arch/arm/include/asm/arch-vf610/imx-regs.h +++
> b/arch/arm/include/asm/arch-vf610/imx-regs.h @@ -200,7 +200,8 @@
>  #define DDRMC_CR78_Q_FULLNESS(v)			(((v) & 0x7)
> << 24) #define DDRMC_CR78_BUR_ON_FLY_BIT(v)
> ((v) & 0xf) #define DDRMC_CR79_CTLUPD_AREF(v)
> (((v) & 0x1) << 24) -#define
> DDRMC_CR82_INT_MASK				0x10000000
> +#define DDRMC_CR80_MC_INIT_COMPLETE			(1 << 8)
> +#define DDRMC_CR82_INT_MASK				(1 << 28)

When I was working on my code - the checkpatch.pl was complaining about
(1 << x) code. It was recommended to use BIT(x) instead.

> #define DDRMC_CR87_ODT_WR_MAPCS0(v)			((v) <<
> 24) #define DDRMC_CR87_ODT_RD_MAPCS0(v)			((v)
> << 16) #define DDRMC_CR88_TODTL_CMD(v)
> (((v) & 0x1f) << 16) diff --git a/arch/arm/mach-imx/ddrmc-vf610.c
> b/arch/arm/mach-imx/ddrmc-vf610.c index 3d7da1c25e..d121a53898 100644
> --- a/arch/arm/mach-imx/ddrmc-vf610.c +++
> b/arch/arm/mach-imx/ddrmc-vf610.c @@ -231,6 +231,7 @@ void
> ddrmc_ctrl_init_ddr3(struct ddr3_jedec_timings const *timings, /* all
> inits done, start the DDR controller */
> writel(DDRMC_CR00_DRAM_CLASS_DDR3 | DDRMC_CR00_START, &ddrmr->cr[0]); 
> -	while (!(readl(&ddrmr->cr[80]) && 0x100))
> +	while (!(readl(&ddrmr->cr[80]) &
> DDRMC_CR80_MC_INIT_COMPLETE)) udelay(10);
> +	writel(DDRMC_CR80_MC_INIT_COMPLETE, &ddrmr->cr[81]);
>  }




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 at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181204/78550028/attachment.sig>

[U-Boot] [PATCH v1 4/4] ARM: vf610: ddrmc: do not write CR79 by default

[Lukasz Majewski]

Hi Stefan,

> From: Stefan Agner <stefan.agner@toradex.com>
> 
> The current value CTLUPD_AREF(0) is the reset value of the register,
> so there is no need to write a value. If needed, the register can be
> written using board specific CR settings.
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
> 
>  arch/arm/mach-imx/ddrmc-vf610.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/ddrmc-vf610.c
> b/arch/arm/mach-imx/ddrmc-vf610.c index d121a53898..d4926b5cee 100644
> --- a/arch/arm/mach-imx/ddrmc-vf610.c
> +++ b/arch/arm/mach-imx/ddrmc-vf610.c
> @@ -188,7 +188,6 @@ void ddrmc_ctrl_init_ddr3(struct
> ddr3_jedec_timings const *timings, DDRMC_CR77_SWAP_EN,
> &ddrmr->cr[77]); writel(DDRMC_CR78_Q_FULLNESS(timings->q_fullness) |
>  		   DDRMC_CR78_BUR_ON_FLY_BIT(12), &ddrmr->cr[78]);
> -	writel(DDRMC_CR79_CTLUPD_AREF(0), &ddrmr->cr[79]);
>  
>  	writel(DDRMC_CR82_INT_MASK, &ddrmr->cr[82]);
>  

Reviewed-by: Lukasz Majewski <lukma@denx.de>


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 at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181204/3d9ad9d7/attachment.sig>

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

[Lukasz Majewski]

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 at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181204/786a2932/attachment.sig>

[U-Boot] [PATCH v1 3/4] ARM: vf610: ddrmc: fix initialization completion detection

[Stefan Agner]

On 04.12.2018 11:59, Lukasz Majewski wrote:
> Hi Stefan,
> 
>> From: Stefan Agner <stefan.agner@toradex.com>
>>
>> The CR80 register has multiple interrupt bits, the code is supposed
>> to check bit 8 but instead uses a logical and. In most cases this
>> probably did not affect real operations since at that stage typically
>> none of the other bits are set.
> 
> It can exit the loop when any error bit is set (but anyway with broken
> DDR initialization we hang latter anyway).
> 
>>
>> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>> Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>> ---
>>
>>  arch/arm/include/asm/arch-vf610/imx-regs.h | 3 ++-
>>  arch/arm/mach-imx/ddrmc-vf610.c            | 3 ++-
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-vf610/imx-regs.h
>> b/arch/arm/include/asm/arch-vf610/imx-regs.h index
>> b7374bfb8f..f71fbf4e73 100644 ---
>> a/arch/arm/include/asm/arch-vf610/imx-regs.h +++
>> b/arch/arm/include/asm/arch-vf610/imx-regs.h @@ -200,7 +200,8 @@
>>  #define DDRMC_CR78_Q_FULLNESS(v)			(((v) & 0x7)
>> << 24) #define DDRMC_CR78_BUR_ON_FLY_BIT(v)
>> ((v) & 0xf) #define DDRMC_CR79_CTLUPD_AREF(v)
>> (((v) & 0x1) << 24) -#define
>> DDRMC_CR82_INT_MASK				0x10000000
>> +#define DDRMC_CR80_MC_INIT_COMPLETE			(1 << 8)
>> +#define DDRMC_CR82_INT_MASK				(1 << 28)
> 
> When I was working on my code - the checkpatch.pl was complaining about
> (1 << x) code. It was recommended to use BIT(x) instead.
> 

Yeah I noticed that too, but arch/arm/include/asm/arch-vf610/imx-regs.h
does use bit shifting so far. I rated consistency higher than the new
recommendation :-)

--
Stefan

>> #define DDRMC_CR87_ODT_WR_MAPCS0(v)			((v) <<
>> 24) #define DDRMC_CR87_ODT_RD_MAPCS0(v)			((v)
>> << 16) #define DDRMC_CR88_TODTL_CMD(v)
>> (((v) & 0x1f) << 16) diff --git a/arch/arm/mach-imx/ddrmc-vf610.c
>> b/arch/arm/mach-imx/ddrmc-vf610.c index 3d7da1c25e..d121a53898 100644
>> --- a/arch/arm/mach-imx/ddrmc-vf610.c +++
>> b/arch/arm/mach-imx/ddrmc-vf610.c @@ -231,6 +231,7 @@ void
>> ddrmc_ctrl_init_ddr3(struct ddr3_jedec_timings const *timings, /* all
>> inits done, start the DDR controller */
>> writel(DDRMC_CR00_DRAM_CLASS_DDR3 | DDRMC_CR00_START, &ddrmr->cr[0]);
>> -	while (!(readl(&ddrmr->cr[80]) && 0x100))
>> +	while (!(readl(&ddrmr->cr[80]) &
>> DDRMC_CR80_MC_INIT_COMPLETE)) udelay(10);
>> +	writel(DDRMC_CR80_MC_INIT_COMPLETE, &ddrmr->cr[81]);
>>  }
> 
> 
> 
> 
> 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 at denx.de

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

[Stefan Agner]

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 at denx.de

[U-Boot] [PATCH v1 3/4] ARM: vf610: ddrmc: fix initialization completion detection

[Stefan Agner]

Lukasz, Stefano,

On 04.12.2018 12:20, Stefan Agner wrote:
> On 04.12.2018 11:59, Lukasz Majewski wrote:
>> Hi Stefan,
>>
>>> From: Stefan Agner <stefan.agner@toradex.com>
>>>
>>> The CR80 register has multiple interrupt bits, the code is supposed
>>> to check bit 8 but instead uses a logical and. In most cases this
>>> probably did not affect real operations since at that stage typically
>>> none of the other bits are set.
>>
>> It can exit the loop when any error bit is set (but anyway with broken
>> DDR initialization we hang latter anyway).
>>
>>>
>>> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>>> Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>>> ---
>>>
>>>  arch/arm/include/asm/arch-vf610/imx-regs.h | 3 ++-
>>>  arch/arm/mach-imx/ddrmc-vf610.c            | 3 ++-
>>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/arch-vf610/imx-regs.h
>>> b/arch/arm/include/asm/arch-vf610/imx-regs.h index
>>> b7374bfb8f..f71fbf4e73 100644 ---
>>> a/arch/arm/include/asm/arch-vf610/imx-regs.h +++
>>> b/arch/arm/include/asm/arch-vf610/imx-regs.h @@ -200,7 +200,8 @@
>>>  #define DDRMC_CR78_Q_FULLNESS(v)			(((v) & 0x7)
>>> << 24) #define DDRMC_CR78_BUR_ON_FLY_BIT(v)
>>> ((v) & 0xf) #define DDRMC_CR79_CTLUPD_AREF(v)
>>> (((v) & 0x1) << 24) -#define
>>> DDRMC_CR82_INT_MASK				0x10000000
>>> +#define DDRMC_CR80_MC_INIT_COMPLETE			(1 << 8)
>>> +#define DDRMC_CR82_INT_MASK				(1 << 28)
>>
>> When I was working on my code - the checkpatch.pl was complaining about
>> (1 << x) code. It was recommended to use BIT(x) instead.
>>
> 
> Yeah I noticed that too, but arch/arm/include/asm/arch-vf610/imx-regs.h
> does use bit shifting so far. I rated consistency higher than the new
> recommendation :-)

Do you want me to change the whole file to BIT here? I'd rather prefer
to keep it for now.

But I think we otherwise are ok with this patch set right? I guess
technically patch 4 would not be your realm Stefano, but maybe you can
also take it through your tree?

--
Stefan

> 
> --
> Stefan
> 
>>> #define DDRMC_CR87_ODT_WR_MAPCS0(v)			((v) <<
>>> 24) #define DDRMC_CR87_ODT_RD_MAPCS0(v)			((v)
>>> << 16) #define DDRMC_CR88_TODTL_CMD(v)
>>> (((v) & 0x1f) << 16) diff --git a/arch/arm/mach-imx/ddrmc-vf610.c
>>> b/arch/arm/mach-imx/ddrmc-vf610.c index 3d7da1c25e..d121a53898 100644
>>> --- a/arch/arm/mach-imx/ddrmc-vf610.c +++
>>> b/arch/arm/mach-imx/ddrmc-vf610.c @@ -231,6 +231,7 @@ void
>>> ddrmc_ctrl_init_ddr3(struct ddr3_jedec_timings const *timings, /* all
>>> inits done, start the DDR controller */
>>> writel(DDRMC_CR00_DRAM_CLASS_DDR3 | DDRMC_CR00_START, &ddrmr->cr[0]);
>>> -	while (!(readl(&ddrmr->cr[80]) && 0x100))
>>> +	while (!(readl(&ddrmr->cr[80]) &
>>> DDRMC_CR80_MC_INIT_COMPLETE)) udelay(10);
>>> +	writel(DDRMC_CR80_MC_INIT_COMPLETE, &ddrmr->cr[81]);
>>>  }
>>
>>
>>
>>
>> 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 at denx.de