ddr: vybrid: various fixes

PATCH New Stefan Agner 2 participants 12 messages Dec 4, 2018 – Dec 14, 2018

Subsystems (4)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape Arm/Freescale vybrid Arm architecture

lore

Share & tools

lore

Stefan Agner

author reviewer

2018-12-04 10:10 UTC #0 lore raw

cover ddr: vybrid: various fixes

From: Stefan Agner <[email protected]>

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

Signature · 1 line

2.19.1

Stefan Agner

author reviewer

2018-12-04 10:10 UTC #1 lore raw

PATCH 1/4 toradex: colibri_vf: fix memory initialization

From: Stefan Agner <[email protected]>

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 <[email protected]>

Acked-by Max Krummenacher <[email protected]>

---

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

board/toradex/colibri_vf/colibri_vf.c +1 -9

index 4db1757469..19cf748c5d 100644
@@ -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 },

Signature · 1 line

2.19.1

Lukasz Majewski

2018-12-04 11:05 UTC #2 lore raw

PATCH 1/4 toradex: colibri_vf: fix memory initialization

Hi Stefan,

Quoted 8 lines

From: Stefan Agner <[email protected]>
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.

Quoted 13 lines

Fixes 3f353ceccbbb ("vf610: refactor DDRMC code")
Signed-off-by Stefan Agner <[email protected]>
Acked-by Max Krummenacher <[email protected]>
---
 board/toradex/colibri_vf/colibri_vf.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)
(quoted diff) +0 -0
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

board/toradex/colibri_vf/colibri_vf.c DECLARE_GLOBAL_DATA_PTR; · line 48

Quoted 7 lines

board/toradex/colibri_vf/colibri_vf.c +0 -4

@@ -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).

Review DECLARE_GLOBAL_DATA_PTR; · line 51

Quoted 3 lines

(quoted diff) +0 -2

-	{ 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.

board/toradex/colibri_vf/colibri_vf.c DECLARE_GLOBAL_DATA_PTR; · line 51

Quoted 1 line

board/toradex/colibri_vf/colibri_vf.c +0 -1
-	{ 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

Quoted 12 lines

(quoted diff) +0 -1
-	{ 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 },
(quoted diff) +0 -0
@@ -88,7 +80,7 @@ static struct ddrmc_cr_setting
colibri_vf_cr_settings[] = { DDRMC_CR154_PAD_ZQ_MODE(1) |
board/toradex/colibri_vf/colibri_vf.c +0 -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 },
(quoted diff) +1 -0
+	{ DDRMC_CR155_PAD_ODT_BYTE1(2) |
DDRMC_CR155_PAD_ODT_BYTE0(2), 155 }, { DDRMC_CR158_TWR(6), 158 },

board/toradex/colibri_vf/colibri_vf.c static struct ddrmc_cr_setting colibri_vf_cr_settings[] = { · line 86

Quoted 2 lines

board/toradex/colibri_vf/colibri_vf.c +0 -0

 	{ 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>

Stefan Agner

author reviewer

2018-12-04 14:48 UTC #3 lore raw

PATCH 1/4 toradex: colibri_vf: fix memory initialization

Quoted 11 lines · Lukasz Majewski

Hi Stefan,
Quoted 8 lines
From: Stefan Agner <[email protected]>
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.

Quoted 3 lines

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

Quoted 65 lines

Quoted 13 lines
Fixes 3f353ceccbbb ("vf610: refactor DDRMC code")
Signed-off-by Stefan Agner <[email protected]>
Acked-by Max Krummenacher <[email protected]>
---
 board/toradex/colibri_vf/colibri_vf.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)
(quoted diff) +0 -0
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
board/toradex/colibri_vf/colibri_vf.c DECLARE_GLOBAL_DATA_PTR; · line 48
Quoted 7 lines
board/toradex/colibri_vf/colibri_vf.c +0 -4
@@ -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).
Review DECLARE_GLOBAL_DATA_PTR; · line 51
Quoted 3 lines
(quoted diff) +0 -2
-	{ 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.
board/toradex/colibri_vf/colibri_vf.c DECLARE_GLOBAL_DATA_PTR; · line 51
Quoted 1 line
board/toradex/colibri_vf/colibri_vf.c +0 -1
-	{ 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
Quoted 12 lines
(quoted diff) +0 -1
-	{ 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 },
(quoted diff) +0 -0
@@ -88,7 +80,7 @@ static struct ddrmc_cr_setting
colibri_vf_cr_settings[] = { DDRMC_CR154_PAD_ZQ_MODE(1) |
board/toradex/colibri_vf/colibri_vf.c +0 -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 },
(quoted diff) +1 -0
+	{ DDRMC_CR155_PAD_ODT_BYTE1(2) |
DDRMC_CR155_PAD_ODT_BYTE0(2), 155 }, { DDRMC_CR158_TWR(6), 158 },
board/toradex/colibri_vf/colibri_vf.c static struct ddrmc_cr_setting colibri_vf_cr_settings[] = { · line 86
Quoted 2 lines
board/toradex/colibri_vf/colibri_vf.c +0 -0
 	{ 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

Stefan Agner

author reviewer

2018-12-04 10:10 UTC #4 lore raw

PATCH 2/4 ARM: vf610: ddrmc: fix CR138 preprocessor define

In reply to Stefan Agner

From: Stefan Agner <[email protected]>

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 <[email protected]>

Acked-by Marcel Ziswiler <[email protected]>

---

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

arch/arm/include/asm/arch-vf610/imx-regs.h +1 -1

index 08ba8e94f8..b7374bfb8f 100644
@@ -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)

Signature · 1 line

2.19.1

Lukasz Majewski

2018-12-04 10:57 UTC #5 lore raw

PATCH 2/4 ARM: vf610: ddrmc: fix CR138 preprocessor define

Hi Stefan,

Quoted 13 lines

From: Stefan Agner <[email protected]>
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 <[email protected]>
Acked-by Marcel Ziswiler <[email protected]>
---
 arch/arm/include/asm/arch-vf610/imx-regs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0

Quoted 15 lines

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 <[email protected]>

Best regards,

Lukasz Majewski

Stefan Agner

author reviewer

2018-12-04 10:10 UTC #6 lore raw

PATCH 3/4 ARM: vf610: ddrmc: fix initialization completion detection

In reply to Stefan Agner

From: Stefan Agner <[email protected]>

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 <[email protected]>

Acked-by Marcel Ziswiler <[email protected]>

---

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

arch/arm/include/asm/arch-vf610/imx-regs.h +2 -1

index b7374bfb8f..f71fbf4e73 100644
@@ -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)

arch/arm/mach-imx/ddrmc-vf610.c +2 -1

index 3d7da1c25e..d121a53898 100644
@@ -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]);
 }

Signature · 1 line

2.19.1

Lukasz Majewski

2018-12-04 10:59 UTC #7 lore raw

PATCH 3/4 ARM: vf610: ddrmc: fix initialization completion detection

Hi Stefan,

Quoted 6 lines

From: Stefan Agner <[email protected]>
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).

Quoted 19 lines

Signed-off-by Stefan Agner <[email protected]>
Acked-by Marcel Ziswiler <[email protected]>
---
 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(-)
(quoted diff) +0 -0
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

arch/arm/include/asm/arch-vf610/imx-regs.h line 204

Quoted 2 lines

arch/arm/include/asm/arch-vf610/imx-regs.h +2 -0

+#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.

Quoted 13 lines

#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]);
(quoted diff) +1 -1
-	while (!(readl(&ddrmr->cr[80]) && 0x100))
+	while (!(readl(&ddrmr->cr[80]) &
DDRMC_CR80_MC_INIT_COMPLETE)) udelay(10);

arch/arm/mach-imx/ddrmc-vf610.c void ddrmc_ctrl_init_ddr3(struct ddr3_jedec_timings const *timings, · line 237

Quoted 2 lines

arch/arm/mach-imx/ddrmc-vf610.c +1 -0
+	writel(DDRMC_CR80_MC_INIT_COMPLETE, &ddrmr->cr[81]);
 }

Best regards,

Lukasz Majewski

Stefan Agner

author reviewer

2018-12-04 11:20 UTC #8 lore raw

PATCH 3/4 ARM: vf610: ddrmc: fix initialization completion detection

Quoted 21 lines · Lukasz Majewski

Hi Stefan,
Quoted 6 lines
From: Stefan Agner <[email protected]>
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).
Quoted 9 lines
Signed-off-by Stefan Agner <[email protected]>
Acked-by Marcel Ziswiler <[email protected]>
---
 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(-)

Quoted 16 lines

arch/arm/include/asm/arch-vf610/imx-regs.h line 204
Quoted 12 lines
(quoted diff) +0 -0
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
arch/arm/include/asm/arch-vf610/imx-regs.h +2 -0
+#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

Quoted 28 lines

Quoted 13 lines
#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]);
(quoted diff) +1 -1
-	while (!(readl(&ddrmr->cr[80]) && 0x100))
+	while (!(readl(&ddrmr->cr[80]) &
DDRMC_CR80_MC_INIT_COMPLETE)) udelay(10);
arch/arm/mach-imx/ddrmc-vf610.c void ddrmc_ctrl_init_ddr3(struct ddr3_jedec_timings const *timings, · line 237
Quoted 2 lines
arch/arm/mach-imx/ddrmc-vf610.c +1 -0
+	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

Stefan Agner

author reviewer

2018-12-14 13:22 UTC #9 lore raw

PATCH 3/4 ARM: vf610: ddrmc: fix initialization completion detection

Lukasz, Stefano,

Quoted 39 lines · Stefan Agner

Quoted 21 lines · Lukasz Majewski
Hi Stefan,
Quoted 6 lines
From: Stefan Agner <[email protected]>
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).
Quoted 9 lines
Signed-off-by Stefan Agner <[email protected]>
Acked-by Marcel Ziswiler <[email protected]>
---
 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(-)
Quoted 16 lines
arch/arm/include/asm/arch-vf610/imx-regs.h line 204
Quoted 12 lines
(quoted diff) +0 -0
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
arch/arm/include/asm/arch-vf610/imx-regs.h +2 -0
+#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.

Quoted 3 lines

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

Quoted 32 lines

--
Stefan
Quoted 28 lines
Quoted 13 lines
#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]);
(quoted diff) +1 -1
-	while (!(readl(&ddrmr->cr[80]) && 0x100))
+	while (!(readl(&ddrmr->cr[80]) &
DDRMC_CR80_MC_INIT_COMPLETE)) udelay(10);
arch/arm/mach-imx/ddrmc-vf610.c void ddrmc_ctrl_init_ddr3(struct ddr3_jedec_timings const *timings, · line 237
Quoted 2 lines
arch/arm/mach-imx/ddrmc-vf610.c +1 -0
+	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

Stefan Agner

author reviewer

2018-12-04 10:10 UTC #10 lore raw

PATCH 4/4 ARM: vf610: ddrmc: do not write CR79 by default

In reply to Stefan Agner

From: Stefan Agner <[email protected]>

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 <[email protected]>

Acked-by Marcel Ziswiler <[email protected]>

---

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

arch/arm/mach-imx/ddrmc-vf610.c +0 -1

index d121a53898..d4926b5cee 100644
@@ -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]);

Signature · 1 line

2.19.1

Lukasz Majewski

2018-12-04 11:00 UTC #11 lore raw

PATCH 4/4 ARM: vf610: ddrmc: do not write CR79 by default

Hi Stefan,

Quoted 21 lines

From: Stefan Agner <[email protected]>
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 <[email protected]>
Acked-by Marcel Ziswiler <[email protected]>
---
 arch/arm/mach-imx/ddrmc-vf610.c | 1 -
 1 file changed, 1 deletion(-)
(quoted diff) +0 -0
b/arch/arm/mach-imx/ddrmc-vf610.c index d121a53898..d4926b5cee 100644
arch/arm/mach-imx/ddrmc-vf610.c +0 -0
@@ -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) |

arch/arm/mach-imx/ddrmc-vf610.c void ddrmc_ctrl_init_ddr3(struct ddr3_jedec_timings const *timings, · line 192

Quoted 4 lines

arch/arm/mach-imx/ddrmc-vf610.c +0 -1

 		   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 <[email protected]>

Best regards,

Lukasz Majewski