diff mbox series

[4/8] drivers: ddr: imx8m: add print for DRAM rate

Message ID 20200511095330.9798-5-peng.fan@nxp.com
State New
Headers show
Series imx: drivers: ddr: ddr driver update | expand

Commit Message

Peng Fan May 11, 2020, 9:53 a.m. UTC
From: Ye Li <ye.li at nxp.com>

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

Reviewed-by: Peng Fan <peng.fan at nxp.com>
Signed-off-by: Ye Li <ye.li at nxp.com>
Signed-off-by:  Peng Fan <peng.fan at 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(-)

Comments

Fabio Estevam May 11, 2020, 2:14 p.m. UTC | #1
Hi Peng,

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

This is basically a revert from:

commit 0d3bc81391ac031758affdb0811bc9c8b905978c
Author: Fabio Estevam <festevam at 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 at gmail.com>
    Reviewed-by: Peng Fan <peng.fan at 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.
Adam Ford May 11, 2020, 2:21 p.m. UTC | #2
On Mon, May 11, 2020 at 9:13 AM Fabio Estevam <festevam at gmail.com> wrote:
>
> Hi Peng,
>
> On Mon, May 11, 2020 at 6:30 AM Peng Fan <peng.fan at nxp.com> wrote:
> >
> > From: Ye Li <ye.li at 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 at nxp.com>
> > Signed-off-by: Ye Li <ye.li at nxp.com>
> > Signed-off-by:  Peng Fan <peng.fan at nxp.com>
>
> This is basically a revert from:
>
> commit 0d3bc81391ac031758affdb0811bc9c8b905978c
> Author: Fabio Estevam <festevam at 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 at gmail.com>
>     Reviewed-by: Peng Fan <peng.fan at 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
Stefano Babic May 11, 2020, 2:59 p.m. UTC | #3
On 11.05.20 11:53, Peng Fan wrote:
> From: Ye Li <ye.li at nxp.com>
> 
> Enable print to show the DRAM rate of current setting and training
> result.
> 
> Reviewed-by: Peng Fan <peng.fan at nxp.com>
> Signed-off-by: Ye Li <ye.li at nxp.com>
> Signed-off-by:  Peng Fan <peng.fan at 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;
>  		}
>  	}
>
Peng Fan May 12, 2020, 1:01 a.m. UTC | #4
> 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 at nxp.com>
> >
> > Enable print to show the DRAM rate of current setting and training
> > result.
> >
> > Reviewed-by: Peng Fan <peng.fan at nxp.com>
> > Signed-off-by: Ye Li <ye.li at nxp.com>
> > Signed-off-by:  Peng Fan <peng.fan at 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
> ==============================================================
> =======
diff mbox series

Patch

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;
 		}
 	}