All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] imx: drivers: ddr: ddr driver update
@ 2020-05-11  9:53 Peng Fan
  2020-05-11  9:53 ` [PATCH 1/8] driver: ddr: imx: skip ddr_ss_gpr config on imx8mn Peng Fan
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Peng Fan @ 2020-05-11  9:53 UTC (permalink / raw)
  To: u-boot

This is to upstream NXP vendor tree ddr driver related fix and update

Jacky Bai (2):
  driver: ddr: imx: skip ddr_ss_gpr config on imx8mn
  driver: ddr: imx: correct the pwrctl setting of selfref_en on imx8m

Jian Li (3):
  imx8mp: enable rd_port_urgent
  imx8mp: DDR performance tunning
  imx8mp: Disables use of MR4 TUF flag (MR4[7]) bit

Oliver Chen (1):
  drivers: ddr: imx Workaround for i.MX8M DDRPHY rank to rank issue

Sherry Sun (1):
  imx8mp: ddr: Add inline ECC feature support

Ye Li (1):
  drivers: ddr: imx8m: add print for DRAM rate

 arch/arm/include/asm/arch-imx8m/ddr.h      |  10 ++
 board/freescale/imx8mp_evk/lpddr4_timing.c |  32 +++++-
 drivers/ddr/imx/imx8m/Kconfig              |   7 ++
 drivers/ddr/imx/imx8m/ddr_init.c           |  86 ++++++++++++++-
 drivers/ddr/imx/imx8m/ddrphy_train.c       |   7 ++
 drivers/ddr/imx/imx8m/ddrphy_utils.c       | 166 ++++++++++++++++++++++++++++-
 6 files changed, 300 insertions(+), 8 deletions(-)

-- 
2.16.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/8] driver: ddr: imx: skip ddr_ss_gpr config on imx8mn
  2020-05-11  9:53 [PATCH 0/8] imx: drivers: ddr: ddr driver update Peng Fan
@ 2020-05-11  9:53 ` Peng Fan
  2020-05-11  9:53 ` [PATCH 2/8] driver: ddr: imx: correct the pwrctl setting of selfref_en on imx8m Peng Fan
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Peng Fan @ 2020-05-11  9:53 UTC (permalink / raw)
  To: u-boot

From: Jacky Bai <ping.bai@nxp.com>

There is no DDR_SS_GPR0 exits on i.MX8MN, so skip setting
this register on i.MX8MN.

Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/ddr/imx/imx8m/ddr_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ddr/imx/imx8m/ddr_init.c b/drivers/ddr/imx/imx8m/ddr_init.c
index af8c1427d2..ba5ae05035 100644
--- a/drivers/ddr/imx/imx8m/ddr_init.c
+++ b/drivers/ddr/imx/imx8m/ddr_init.c
@@ -73,7 +73,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
 
 	/* if ddr type is LPDDR4, do it */
 	tmp = reg32_read(DDRC_MSTR(0));
-	if (tmp & (0x1 << 5))
+	if (tmp & (0x1 << 5) && !is_imx8mn())
 		reg32_write(DDRC_DDR_SS_GPR0, 0x01); /* LPDDR4 mode */
 
 	/* determine the initial boot frequency */
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/8] driver: ddr: imx: correct the pwrctl setting of selfref_en on imx8m
  2020-05-11  9:53 [PATCH 0/8] imx: drivers: ddr: ddr driver update Peng Fan
  2020-05-11  9:53 ` [PATCH 1/8] driver: ddr: imx: skip ddr_ss_gpr config on imx8mn Peng Fan
@ 2020-05-11  9:53 ` Peng Fan
  2020-05-11  9:53 ` [PATCH 3/8] imx8mp: ddr: Add inline ECC feature support Peng Fan
  2020-05-11  9:53 ` [PATCH 4/8] drivers: ddr: imx8m: add print for DRAM rate Peng Fan
  3 siblings, 0 replies; 11+ messages in thread
From: Peng Fan @ 2020-05-11  9:53 UTC (permalink / raw)
  To: u-boot

From: Jacky Bai <ping.bai@nxp.com>

The 'selfref_en' should be bit'0', so correct the setting to
enable the auto self-refresh.

Reviewed-by: Jian Li <jian.li@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/ddr/imx/imx8m/ddr_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ddr/imx/imx8m/ddr_init.c b/drivers/ddr/imx/imx8m/ddr_init.c
index ba5ae05035..06b4341b11 100644
--- a/drivers/ddr/imx/imx8m/ddr_init.c
+++ b/drivers/ddr/imx/imx8m/ddr_init.c
@@ -162,7 +162,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
 	/* Step26: Set back register in Step4 to the original values if desired */
 	reg32_write(DDRC_RFSHCTL3(0), 0x0000000);
 	/* enable selfref_en by default */
-	setbits_le32(DDRC_PWRCTL(0), 0x1 << 3);
+	setbits_le32(DDRC_PWRCTL(0), 0x1);
 
 	/* enable port 0 */
 	reg32_write(DDRC_PCTRL_0(0), 0x00000001);
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/8] imx8mp: ddr: Add inline ECC feature support
  2020-05-11  9:53 [PATCH 0/8] imx: drivers: ddr: ddr driver update Peng Fan
  2020-05-11  9:53 ` [PATCH 1/8] driver: ddr: imx: skip ddr_ss_gpr config on imx8mn Peng Fan
  2020-05-11  9:53 ` [PATCH 2/8] driver: ddr: imx: correct the pwrctl setting of selfref_en on imx8m Peng Fan
@ 2020-05-11  9:53 ` Peng Fan
  2020-05-12  0:26   ` Fabio Estevam
  2020-05-11  9:53 ` [PATCH 4/8] drivers: ddr: imx8m: add print for DRAM rate Peng Fan
  3 siblings, 1 reply; 11+ messages in thread
From: Peng Fan @ 2020-05-11  9:53 UTC (permalink / raw)
  To: u-boot

From: Sherry Sun <sherry.sun@nxp.com>

Add inline ECC support for lpddr4 on imx8mp-evk. And add a config which
can enable/disable inline ECC feature for lpddr4 on imx8mp-evk board.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm/include/asm/arch-imx8m/ddr.h      |  7 +++
 board/freescale/imx8mp_evk/lpddr4_timing.c | 27 +++++++++++
 drivers/ddr/imx/imx8m/Kconfig              |  7 +++
 drivers/ddr/imx/imx8m/ddr_init.c           | 72 ++++++++++++++++++++++++++++++
 4 files changed, 113 insertions(+)

diff --git a/arch/arm/include/asm/arch-imx8m/ddr.h b/arch/arm/include/asm/arch-imx8m/ddr.h
index 7a2a2d8edc..04c9c962cf 100644
--- a/arch/arm/include/asm/arch-imx8m/ddr.h
+++ b/arch/arm/include/asm/arch-imx8m/ddr.h
@@ -529,6 +529,8 @@ enum msg_response {
 #define DDRC_SBRWDATA0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf2c)
 #define DDRC_SBRWDATA1(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf30)
 #define DDRC_PDCH(X)             (DDRC_IPS_BASE_ADDR(X) + 0xf34)
+#define DDRC_SBRSTART0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf38)
+#define DDRC_SBRRANGE0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf40)
 
 #define DDRC_FREQ1_DERATEEN(X)         (DDRC_IPS_BASE_ADDR(X) + 0x2020)
 #define DDRC_FREQ1_DERATEINT(X)        (DDRC_IPS_BASE_ADDR(X) + 0x2024)
@@ -708,6 +710,11 @@ int ddr_cfg_phy(struct dram_timing_info *timing_info);
 void load_lpddr4_phy_pie(void);
 void ddrphy_trained_csr_save(struct dram_cfg_param *param, unsigned int num);
 void dram_config_save(struct dram_timing_info *info, unsigned long base);
+void board_dram_ecc_scrub(void);
+void ddrc_inline_ecc_scrub(unsigned int start_address,
+			   unsigned int range_address);
+void ddrc_inline_ecc_scrub_end(unsigned int start_address,
+			       unsigned int range_address);
 
 /* utils function for ddr phy training */
 int wait_ddrphy_training_complete(void);
diff --git a/board/freescale/imx8mp_evk/lpddr4_timing.c b/board/freescale/imx8mp_evk/lpddr4_timing.c
index 14542490bc..4e5ebf9424 100644
--- a/board/freescale/imx8mp_evk/lpddr4_timing.c
+++ b/board/freescale/imx8mp_evk/lpddr4_timing.c
@@ -14,6 +14,9 @@ struct dram_cfg_param ddr_ddrc_cfg[] = {
 	{ 0x3d400020, 0x323 },
 	{ 0x3d400024, 0x1e84800 },
 	{ 0x3d400064, 0x7a0118 },
+#ifdef CONFIG_IMX8M_DRAM_INLINE_ECC
+	{ 0x3d400070, 0x01027f44 },
+#endif
 	{ 0x3d4000d0, 0xc00307a3 },
 	{ 0x3d4000d4, 0xc50000 },
 	{ 0x3d4000dc, 0xf4003f },
@@ -45,12 +48,21 @@ struct dram_cfg_param ddr_ddrc_cfg[] = {
 	{ 0x3d4001c4, 0x1 },
 	{ 0x3d4000f4, 0xc99 },
 	{ 0x3d400108, 0x9121c1c },
+#ifdef CONFIG_IMX8M_DRAM_INLINE_ECC
+	{ 0x3d400200, 0x13 },
+	{ 0x3d40020c, 0x13131300 },
+	{ 0x3d400210, 0x1f1f },
+	{ 0x3d400204, 0x50505 },
+	{ 0x3d400214, 0x4040404 },
+	{ 0x3d400218, 0x68040404 },
+#else
 	{ 0x3d400200, 0x16 },
 	{ 0x3d40020c, 0x0 },
 	{ 0x3d400210, 0x1f1f },
 	{ 0x3d400204, 0x80808 },
 	{ 0x3d400214, 0x7070707 },
 	{ 0x3d400218, 0x68070707 },
+#endif
 	{ 0x3d40021c, 0xf08 },
 	{ 0x3d400250, 0x29001701 },
 	{ 0x3d400254, 0x2c },
@@ -1845,3 +1857,18 @@ struct dram_timing_info dram_timing = {
 	.ddrphy_pie_num = ARRAY_SIZE(ddr_phy_pie),
 	.fsp_table = { 4000, 400, 100, },
 };
+
+#ifdef CONFIG_IMX8M_DRAM_INLINE_ECC
+void board_dram_ecc_scrub(void)
+{
+	/* add inline scrb function MPlus spcific */
+	/* scrub 0-1.75G */
+	ddrc_inline_ecc_scrub(0x0, 0x1bffffff);
+	/* scrub 2-3.75G */
+	ddrc_inline_ecc_scrub(0x20000000, 0x3bffffff);
+	/* scrub 4-5.75G */
+	ddrc_inline_ecc_scrub(0x40000000, 0x5bffffff);
+	/* set scruber read range 0-6G */
+	ddrc_inline_ecc_scrub_end(0x0, 0x5fffffff);
+}
+#endif
diff --git a/drivers/ddr/imx/imx8m/Kconfig b/drivers/ddr/imx/imx8m/Kconfig
index 5bf61eb258..a5f5524fbe 100644
--- a/drivers/ddr/imx/imx8m/Kconfig
+++ b/drivers/ddr/imx/imx8m/Kconfig
@@ -29,4 +29,11 @@ config SAVED_DRAM_TIMING_BASE
 	  info into memory for low power use. OCRAM_S is used for this
 	  purpose on i.MX8MM.
 	default 0x180000
+
+config IMX8M_DRAM_INLINE_ECC
+	bool "imx8mp inline ECC"
+	depends on IMX8MP && IMX8M_LPDDR4
+	help
+	  Select this config if you want to use inline ecc feature for
+	  imx8mp-evk board.
 endmenu
diff --git a/drivers/ddr/imx/imx8m/ddr_init.c b/drivers/ddr/imx/imx8m/ddr_init.c
index 06b4341b11..f573a778d9 100644
--- a/drivers/ddr/imx/imx8m/ddr_init.c
+++ b/drivers/ddr/imx/imx8m/ddr_init.c
@@ -20,6 +20,76 @@ void ddr_cfg_umctl2(struct dram_cfg_param *ddrc_cfg, int num)
 	}
 }
 
+#ifdef CONFIG_IMX8M_DRAM_INLINE_ECC
+void ddrc_inline_ecc_scrub(unsigned int start_address,
+			   unsigned int range_address)
+{
+	unsigned int tmp;
+
+	/* Step1: Enable quasi-dynamic programming */
+	reg32_write(DDRC_SWCTL(0), 0x00000000);
+	/* Step2: Set ECCCFG1.ecc_parity_region_lock to 1 */
+	reg32setbit(DDRC_ECCCFG1(0), 0x4);
+	/* Step3: Block the AXI ports from taking the transaction */
+	reg32_write(DDRC_PCTRL_0(0), 0x0);
+	/* Step4: Set scrub start address */
+	reg32_write(DDRC_SBRSTART0(0), start_address);
+	/* Step5: Set scrub range address */
+	reg32_write(DDRC_SBRRANGE0(0), range_address);
+	/* Step6: Set scrub_mode to write */
+	reg32_write(DDRC_SBRCTL(0), 0x00000014);
+	/* Step7: Set the desired pattern through SBRWDATA0 registers */
+	reg32_write(DDRC_SBRWDATA0(0), 0x55aa55aa);
+	/* Step8: Enable the SBR by programming SBRCTL.scrub_en=1 */
+	reg32setbit(DDRC_SBRCTL(0), 0x0);
+	/* Step9: Poll SBRSTAT.scrub_done=1 */
+	tmp = reg32_read(DDRC_SBRSTAT(0));
+	while (tmp != 0x00000002)
+		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x2;
+	/* Step10: Poll SBRSTAT.scrub_busy=0 */
+	tmp = reg32_read(DDRC_SBRSTAT(0));
+	while (tmp != 0x0)
+		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x1;
+	/* Step11: Disable SBR by programming SBRCTL.scrub_en=0 */
+	clrbits_le32(DDRC_SBRCTL(0), 0x1);
+	/* Step12: Prepare for normal scrub operation(Read) and set scrub_interval*/
+	reg32_write(DDRC_SBRCTL(0), 0x100);
+	/* Step13: Enable the SBR by programming SBRCTL.scrub_en=1 */
+	reg32_write(DDRC_SBRCTL(0), 0x101);
+	/* Step14: Enable AXI ports by programming */
+	reg32_write(DDRC_PCTRL_0(0), 0x1);
+	/* Step15: Disable quasi-dynamic programming */
+	reg32_write(DDRC_SWCTL(0), 0x00000001);
+}
+
+void ddrc_inline_ecc_scrub_end(unsigned int start_address,
+			       unsigned int range_address)
+{
+	/* Step1: Enable quasi-dynamic programming */
+	reg32_write(DDRC_SWCTL(0), 0x00000000);
+	/* Step2: Block the AXI ports from taking the transaction */
+	reg32_write(DDRC_PCTRL_0(0), 0x0);
+	/* Step3: Set scrub start address */
+	reg32_write(DDRC_SBRSTART0(0), start_address);
+	/* Step4: Set scrub range address */
+	reg32_write(DDRC_SBRRANGE0(0), range_address);
+	/* Step5: Disable SBR by programming SBRCTL.scrub_en=0 */
+	clrbits_le32(DDRC_SBRCTL(0), 0x1);
+	/* Step6: Prepare for normal scrub operation(Read) and set scrub_interval */
+	reg32_write(DDRC_SBRCTL(0), 0x100);
+	/* Step7: Enable the SBR by programming SBRCTL.scrub_en=1 */
+	reg32_write(DDRC_SBRCTL(0), 0x101);
+	/* Step8: Enable AXI ports by programming */
+	reg32_write(DDRC_PCTRL_0(0), 0x1);
+	/* Step9: Disable quasi-dynamic programming */
+	reg32_write(DDRC_SWCTL(0), 0x00000001);
+}
+#endif
+
+void __weak board_dram_ecc_scrub(void)
+{
+}
+
 int ddr_init(struct dram_timing_info *dram_timing)
 {
 	unsigned int tmp, initial_drate, target_freq;
@@ -168,6 +238,8 @@ int ddr_init(struct dram_timing_info *dram_timing)
 	reg32_write(DDRC_PCTRL_0(0), 0x00000001);
 	debug("DDRINFO: ddrmix config done\n");
 
+	board_dram_ecc_scrub();
+
 	/* save the dram timing config into memory */
 	dram_config_save(dram_timing, CONFIG_SAVED_DRAM_TIMING_BASE);
 
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/8] drivers: ddr: imx8m: add print for DRAM rate
  2020-05-11  9:53 [PATCH 0/8] imx: drivers: ddr: ddr driver update Peng Fan
                   ` (2 preceding siblings ...)
  2020-05-11  9:53 ` [PATCH 3/8] imx8mp: ddr: Add inline ECC feature support Peng Fan
@ 2020-05-11  9:53 ` Peng Fan
  2020-05-11 14:14   ` Fabio Estevam
  2020-05-11 14:59   ` Stefano Babic
  3 siblings, 2 replies; 11+ messages in thread
From: Peng Fan @ 2020-05-11  9:53 UTC (permalink / raw)
  To: u-boot

From: Ye Li <ye.li@nxp.com>

Enable print to show the DRAM rate of current setting and training
result.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by:  Peng Fan <peng.fan@nxp.com>
---
 drivers/ddr/imx/imx8m/ddr_init.c     | 7 ++++---
 drivers/ddr/imx/imx8m/ddrphy_utils.c | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/ddr/imx/imx8m/ddr_init.c b/drivers/ddr/imx/imx8m/ddr_init.c
index f573a778d9..a1d2d21692 100644
--- a/drivers/ddr/imx/imx8m/ddr_init.c
+++ b/drivers/ddr/imx/imx8m/ddr_init.c
@@ -95,7 +95,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
 	unsigned int tmp, initial_drate, target_freq;
 	int ret;
 
-	debug("DDRINFO: start DRAM init\n");
+	printf("DDRINFO: start DRAM init\n");
 
 	/* Step1: Follow the power up procedure */
 	if (is_imx8mq()) {
@@ -118,6 +118,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
 
 	initial_drate = dram_timing->fsp_msg[0].drate;
 	/* default to the frequency point 0 clock */
+	printf("DDRINFO: DRAM rate %dMTS\n", initial_drate);
 	ddrphy_init_set_dfi_clk(initial_drate);
 
 	/* D-aasert the presetn */
@@ -184,7 +185,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
 		tmp = reg32_read(DDRPHY_CalBusy(0));
 	} while ((tmp & 0x1));
 
-	debug("DDRINFO:ddrphy calibration done\n");
+	printf("DDRINFO:ddrphy calibration done\n");
 
 	/* Step15: Set SWCTL.sw_done to 0 */
 	reg32_write(DDRC_SWCTL(0), 0x00000000);
@@ -236,7 +237,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
 
 	/* enable port 0 */
 	reg32_write(DDRC_PCTRL_0(0), 0x00000001);
-	debug("DDRINFO: ddrmix config done\n");
+	printf("DDRINFO: ddrmix config done\n");
 
 	board_dram_ecc_scrub();
 
diff --git a/drivers/ddr/imx/imx8m/ddrphy_utils.c b/drivers/ddr/imx/imx8m/ddrphy_utils.c
index 9ac7ca923c..9d2378d7dd 100644
--- a/drivers/ddr/imx/imx8m/ddrphy_utils.c
+++ b/drivers/ddr/imx/imx8m/ddrphy_utils.c
@@ -97,7 +97,7 @@ int wait_ddrphy_training_complete(void)
 			debug("Training PASS\n");
 			return 0;
 		} else if (mail == 0xff) {
-			debug("Training FAILED\n");
+			printf("Training FAILED\n");
 			return -1;
 		}
 	}
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/8] drivers: ddr: imx8m: add print for DRAM rate
  2020-05-11  9:53 ` [PATCH 4/8] drivers: ddr: imx8m: add print for DRAM rate Peng Fan
@ 2020-05-11 14:14   ` Fabio Estevam
  2020-05-11 14:21     ` Adam Ford
  2020-05-11 14:59   ` Stefano Babic
  1 sibling, 1 reply; 11+ messages in thread
From: Fabio Estevam @ 2020-05-11 14:14 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On Mon, May 11, 2020 at 6:30 AM Peng Fan <peng.fan@nxp.com> wrote:
>
> From: Ye Li <ye.li@nxp.com>
>
> Enable print to show the DRAM rate of current setting and training
> result.
>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Signed-off-by: Ye Li <ye.li@nxp.com>
> Signed-off-by:  Peng Fan <peng.fan@nxp.com>

This is basically a revert from:

commit 0d3bc81391ac031758affdb0811bc9c8b905978c
Author: Fabio Estevam <festevam@gmail.com>
Date:   Wed Dec 11 17:37:09 2019 -0300

    imx8m: ddr_init: Move ddr_init() messages to debug level

    Currently inside ddr_init() there is a mix of printf() and debug()
    level messages.

    Since this type of information is useful for debug purposes,
    convert all of them to debug level for consistency.

    Signed-off-by: Fabio Estevam <festevam@gmail.com>
    Reviewed-by: Peng Fan <peng.fan@nxp.com>

In the normal boot cases I don't think these messages are helpful.

> diff --git a/drivers/ddr/imx/imx8m/ddrphy_utils.c b/drivers/ddr/imx/imx8m/ddrphy_utils.c
> index 9ac7ca923c..9d2378d7dd 100644
> --- a/drivers/ddr/imx/imx8m/ddrphy_utils.c
> +++ b/drivers/ddr/imx/imx8m/ddrphy_utils.c
> @@ -97,7 +97,7 @@ int wait_ddrphy_training_complete(void)
>                         debug("Training PASS\n");
>                         return 0;
>                 } else if (mail == 0xff) {
> -                       debug("Training FAILED\n");
> +                       printf("Training FAILED\n");

This one is an error message, so I agree that it is useful to have it printed.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/8] drivers: ddr: imx8m: add print for DRAM rate
  2020-05-11 14:14   ` Fabio Estevam
@ 2020-05-11 14:21     ` Adam Ford
  0 siblings, 0 replies; 11+ messages in thread
From: Adam Ford @ 2020-05-11 14:21 UTC (permalink / raw)
  To: u-boot

On Mon, May 11, 2020 at 9:13 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Peng,
>
> On Mon, May 11, 2020 at 6:30 AM Peng Fan <peng.fan@nxp.com> wrote:
> >
> > From: Ye Li <ye.li@nxp.com>
> >
> > Enable print to show the DRAM rate of current setting and training
> > result.

I am not a fan of this.

> >
> > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> > Signed-off-by: Ye Li <ye.li@nxp.com>
> > Signed-off-by:  Peng Fan <peng.fan@nxp.com>
>
> This is basically a revert from:
>
> commit 0d3bc81391ac031758affdb0811bc9c8b905978c
> Author: Fabio Estevam <festevam@gmail.com>
> Date:   Wed Dec 11 17:37:09 2019 -0300
>
>     imx8m: ddr_init: Move ddr_init() messages to debug level
>
>     Currently inside ddr_init() there is a mix of printf() and debug()
>     level messages.
>
>     Since this type of information is useful for debug purposes,
>     convert all of them to debug level for consistency.
>
>     Signed-off-by: Fabio Estevam <festevam@gmail.com>
>     Reviewed-by: Peng Fan <peng.fan@nxp.com>
>
> In the normal boot cases I don't think these messages are helpful.

I would agree.  As a user, I don't think most people will want to know
this, and it creates a bunch of chatter.  For people who want it,
enable the debug.

>
> > diff --git a/drivers/ddr/imx/imx8m/ddrphy_utils.c b/drivers/ddr/imx/imx8m/ddrphy_utils.c
> > index 9ac7ca923c..9d2378d7dd 100644
> > --- a/drivers/ddr/imx/imx8m/ddrphy_utils.c
> > +++ b/drivers/ddr/imx/imx8m/ddrphy_utils.c
> > @@ -97,7 +97,7 @@ int wait_ddrphy_training_complete(void)
> >                         debug("Training PASS\n");
> >                         return 0;
> >                 } else if (mail == 0xff) {
> > -                       debug("Training FAILED\n");
> > +                       printf("Training FAILED\n");
>
> This one is an error message, so I agree that it is useful to have it printed.
I would agree here.

adam

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/8] drivers: ddr: imx8m: add print for DRAM rate
  2020-05-11  9:53 ` [PATCH 4/8] drivers: ddr: imx8m: add print for DRAM rate Peng Fan
  2020-05-11 14:14   ` Fabio Estevam
@ 2020-05-11 14:59   ` Stefano Babic
  2020-05-12  1:01     ` Peng Fan
  1 sibling, 1 reply; 11+ messages in thread
From: Stefano Babic @ 2020-05-11 14:59 UTC (permalink / raw)
  To: u-boot

On 11.05.20 11:53, Peng Fan wrote:
> From: Ye Li <ye.li@nxp.com>
> 
> Enable print to show the DRAM rate of current setting and training
> result.
> 
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Signed-off-by: Ye Li <ye.li@nxp.com>
> Signed-off-by:  Peng Fan <peng.fan@nxp.com>
> ---

This changes the boottime, too. The printf() are really useful only for
debugging, IMHO it is better to let it as it is. If something is going
wrong, one set DEBUG to get the output.

Regards,
Stefano

>  drivers/ddr/imx/imx8m/ddr_init.c     | 7 ++++---
>  drivers/ddr/imx/imx8m/ddrphy_utils.c | 2 +-
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ddr/imx/imx8m/ddr_init.c b/drivers/ddr/imx/imx8m/ddr_init.c
> index f573a778d9..a1d2d21692 100644
> --- a/drivers/ddr/imx/imx8m/ddr_init.c
> +++ b/drivers/ddr/imx/imx8m/ddr_init.c
> @@ -95,7 +95,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
>  	unsigned int tmp, initial_drate, target_freq;
>  	int ret;
>  
> -	debug("DDRINFO: start DRAM init\n");
> +	printf("DDRINFO: start DRAM init\n");
>  
>  	/* Step1: Follow the power up procedure */
>  	if (is_imx8mq()) {
> @@ -118,6 +118,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
>  
>  	initial_drate = dram_timing->fsp_msg[0].drate;
>  	/* default to the frequency point 0 clock */
> +	printf("DDRINFO: DRAM rate %dMTS\n", initial_drate);



>  	ddrphy_init_set_dfi_clk(initial_drate);
>  
>  	/* D-aasert the presetn */
> @@ -184,7 +185,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
>  		tmp = reg32_read(DDRPHY_CalBusy(0));
>  	} while ((tmp & 0x1));
>  
> -	debug("DDRINFO:ddrphy calibration done\n");
> +	printf("DDRINFO:ddrphy calibration done\n");
>  
>  	/* Step15: Set SWCTL.sw_done to 0 */
>  	reg32_write(DDRC_SWCTL(0), 0x00000000);
> @@ -236,7 +237,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
>  
>  	/* enable port 0 */
>  	reg32_write(DDRC_PCTRL_0(0), 0x00000001);
> -	debug("DDRINFO: ddrmix config done\n");
> +	printf("DDRINFO: ddrmix config done\n");
>  
>  	board_dram_ecc_scrub();
>  
> diff --git a/drivers/ddr/imx/imx8m/ddrphy_utils.c b/drivers/ddr/imx/imx8m/ddrphy_utils.c
> index 9ac7ca923c..9d2378d7dd 100644
> --- a/drivers/ddr/imx/imx8m/ddrphy_utils.c
> +++ b/drivers/ddr/imx/imx8m/ddrphy_utils.c
> @@ -97,7 +97,7 @@ int wait_ddrphy_training_complete(void)
>  			debug("Training PASS\n");
>  			return 0;
>  		} else if (mail == 0xff) {
> -			debug("Training FAILED\n");
> +			printf("Training FAILED\n");
>  			return -1;
>  		}
>  	}
> 


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/8] imx8mp: ddr: Add inline ECC feature support
  2020-05-11  9:53 ` [PATCH 3/8] imx8mp: ddr: Add inline ECC feature support Peng Fan
@ 2020-05-12  0:26   ` Fabio Estevam
  2020-05-12  1:03     ` Peng Fan
  0 siblings, 1 reply; 11+ messages in thread
From: Fabio Estevam @ 2020-05-12  0:26 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On Mon, May 11, 2020 at 6:30 AM Peng Fan <peng.fan@nxp.com> wrote:
>
> From: Sherry Sun <sherry.sun@nxp.com>
>
> Add inline ECC support for lpddr4 on imx8mp-evk. And add a config which
> can enable/disable inline ECC feature for lpddr4 on imx8mp-evk board.

Please elaborate more on this inline ECC feature: when and why does it
need to be selected, for example.

> --- a/board/freescale/imx8mp_evk/lpddr4_timing.c
> +++ b/board/freescale/imx8mp_evk/lpddr4_timing.c
> @@ -14,6 +14,9 @@ struct dram_cfg_param ddr_ddrc_cfg[] = {
>         { 0x3d400020, 0x323 },
>         { 0x3d400024, 0x1e84800 },
>         { 0x3d400064, 0x7a0118 },
> +#ifdef CONFIG_IMX8M_DRAM_INLINE_ECC

I see no user for this symbol at the moment.

It is usually better to introduce the symbol when there is a real user for it.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/8] drivers: ddr: imx8m: add print for DRAM rate
  2020-05-11 14:59   ` Stefano Babic
@ 2020-05-12  1:01     ` Peng Fan
  0 siblings, 0 replies; 11+ messages in thread
From: Peng Fan @ 2020-05-12  1:01 UTC (permalink / raw)
  To: u-boot

> Subject: Re: [PATCH 4/8] drivers: ddr: imx8m: add print for DRAM rate
> 
> On 11.05.20 11:53, Peng Fan wrote:
> > From: Ye Li <ye.li@nxp.com>
> >
> > Enable print to show the DRAM rate of current setting and training
> > result.
> >
> > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> > Signed-off-by: Ye Li <ye.li@nxp.com>
> > Signed-off-by:  Peng Fan <peng.fan@nxp.com>
> > ---
> 
> This changes the boottime, too. The printf() are really useful only for
> debugging, IMHO it is better to let it as it is. If something is going wrong, one
> set DEBUG to get the output.

ok. I'll drop this patch from the patchset.

Thanks,
Peng.

> 
> Regards,
> Stefano
> 
> >  drivers/ddr/imx/imx8m/ddr_init.c     | 7 ++++---
> >  drivers/ddr/imx/imx8m/ddrphy_utils.c | 2 +-
> >  2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/ddr/imx/imx8m/ddr_init.c
> > b/drivers/ddr/imx/imx8m/ddr_init.c
> > index f573a778d9..a1d2d21692 100644
> > --- a/drivers/ddr/imx/imx8m/ddr_init.c
> > +++ b/drivers/ddr/imx/imx8m/ddr_init.c
> > @@ -95,7 +95,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
> >  	unsigned int tmp, initial_drate, target_freq;
> >  	int ret;
> >
> > -	debug("DDRINFO: start DRAM init\n");
> > +	printf("DDRINFO: start DRAM init\n");
> >
> >  	/* Step1: Follow the power up procedure */
> >  	if (is_imx8mq()) {
> > @@ -118,6 +118,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
> >
> >  	initial_drate = dram_timing->fsp_msg[0].drate;
> >  	/* default to the frequency point 0 clock */
> > +	printf("DDRINFO: DRAM rate %dMTS\n", initial_drate);
> 
> 
> 
> >  	ddrphy_init_set_dfi_clk(initial_drate);
> >
> >  	/* D-aasert the presetn */
> > @@ -184,7 +185,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
> >  		tmp = reg32_read(DDRPHY_CalBusy(0));
> >  	} while ((tmp & 0x1));
> >
> > -	debug("DDRINFO:ddrphy calibration done\n");
> > +	printf("DDRINFO:ddrphy calibration done\n");
> >
> >  	/* Step15: Set SWCTL.sw_done to 0 */
> >  	reg32_write(DDRC_SWCTL(0), 0x00000000); @@ -236,7 +237,7 @@ int
> > ddr_init(struct dram_timing_info *dram_timing)
> >
> >  	/* enable port 0 */
> >  	reg32_write(DDRC_PCTRL_0(0), 0x00000001);
> > -	debug("DDRINFO: ddrmix config done\n");
> > +	printf("DDRINFO: ddrmix config done\n");
> >
> >  	board_dram_ecc_scrub();
> >
> > diff --git a/drivers/ddr/imx/imx8m/ddrphy_utils.c
> > b/drivers/ddr/imx/imx8m/ddrphy_utils.c
> > index 9ac7ca923c..9d2378d7dd 100644
> > --- a/drivers/ddr/imx/imx8m/ddrphy_utils.c
> > +++ b/drivers/ddr/imx/imx8m/ddrphy_utils.c
> > @@ -97,7 +97,7 @@ int wait_ddrphy_training_complete(void)
> >  			debug("Training PASS\n");
> >  			return 0;
> >  		} else if (mail == 0xff) {
> > -			debug("Training FAILED\n");
> > +			printf("Training FAILED\n");
> >  			return -1;
> >  		}
> >  	}
> >
> 
> 
> --
> ==============================================================
> =======
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> ==============================================================
> =======

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/8] imx8mp: ddr: Add inline ECC feature support
  2020-05-12  0:26   ` Fabio Estevam
@ 2020-05-12  1:03     ` Peng Fan
  0 siblings, 0 replies; 11+ messages in thread
From: Peng Fan @ 2020-05-12  1:03 UTC (permalink / raw)
  To: u-boot

> Subject: Re: [PATCH 3/8] imx8mp: ddr: Add inline ECC feature support
> 
> Hi Peng,
> 
> On Mon, May 11, 2020 at 6:30 AM Peng Fan <peng.fan@nxp.com> wrote:
> >
> > From: Sherry Sun <sherry.sun@nxp.com>
> >
> > Add inline ECC support for lpddr4 on imx8mp-evk. And add a config
> > which can enable/disable inline ECC feature for lpddr4 on imx8mp-evk
> board.
> 
> Please elaborate more on this inline ECC feature: when and why does it need
> to be selected, for example.

ok.

> 
> > --- a/board/freescale/imx8mp_evk/lpddr4_timing.c
> > +++ b/board/freescale/imx8mp_evk/lpddr4_timing.c
> > @@ -14,6 +14,9 @@ struct dram_cfg_param ddr_ddrc_cfg[] = {
> >         { 0x3d400020, 0x323 },
> >         { 0x3d400024, 0x1e84800 },
> >         { 0x3d400064, 0x7a0118 },
> > +#ifdef CONFIG_IMX8M_DRAM_INLINE_ECC
> 
> I see no user for this symbol at the moment.
> 
> It is usually better to introduce the symbol when there is a real user for it.

This is the the just the driver part. To enable this feature, we need a dedicated
defconfig. The defconfig part will be posted in other patches.

Thanks,
Peng.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-05-12  1:03 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-11  9:53 [PATCH 0/8] imx: drivers: ddr: ddr driver update Peng Fan
2020-05-11  9:53 ` [PATCH 1/8] driver: ddr: imx: skip ddr_ss_gpr config on imx8mn Peng Fan
2020-05-11  9:53 ` [PATCH 2/8] driver: ddr: imx: correct the pwrctl setting of selfref_en on imx8m Peng Fan
2020-05-11  9:53 ` [PATCH 3/8] imx8mp: ddr: Add inline ECC feature support Peng Fan
2020-05-12  0:26   ` Fabio Estevam
2020-05-12  1:03     ` Peng Fan
2020-05-11  9:53 ` [PATCH 4/8] drivers: ddr: imx8m: add print for DRAM rate Peng Fan
2020-05-11 14:14   ` Fabio Estevam
2020-05-11 14:21     ` Adam Ford
2020-05-11 14:59   ` Stefano Babic
2020-05-12  1:01     ` Peng Fan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.