From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Wed, 5 Aug 2015 18:12:32 +0200
Subject: [U-Boot] [PATCH 16/28] ddr: altera: sdram: Clean up
	sdram_mmr_init_full() part 4
In-Reply-To: <55C232FC.5050807@opensource.altera.com>
References: <1438464897-8051-1-git-send-email-marex@denx.de>
 <1438464897-8051-17-git-send-email-marex@denx.de>
 <55C232FC.5050807@opensource.altera.com>
Message-ID: <201508051812.32931.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Wednesday, August 05, 2015 at 05:59:56 PM, Dinh Nguyen wrote:
> On 8/1/15 4:34 PM, Marek Vasut wrote:
> > Merge sdr_set_*() functions which are just setting registers among
> > the sea of register setting in sdram_mmr_init_full(). There is no
> > need to keep them separate this way, there is nothing special about
> > them.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > ---
> > 
> >  drivers/ddr/altera/sdram.c | 98
> >  +++++++++++++++++----------------------------- 1 file changed, 36
> >  insertions(+), 62 deletions(-)
> > 
> > diff --git a/drivers/ddr/altera/sdram.c b/drivers/ddr/altera/sdram.c
> > index 595f2a4..199e8b8 100644
> > --- a/drivers/ddr/altera/sdram.c
> > +++ b/drivers/ddr/altera/sdram.c
> > @@ -501,24 +501,6 @@ static void set_sdr_ctrlcfg(struct
> > socfpga_sdram_config *cfg)
> > 
> >  	writel(ctrl_cfg, &sdr_ctrl->ctrl_cfg);
> >  
> >  }
> 
> <snip>
> 
> > @@ -586,7 +530,22 @@ unsigned sdram_mmr_init_full(unsigned int
> > sdr_phy_reg)
> > 
> >  	writel(rows, &sysmgr_regs->iswgrp_handoff[4]);
> >  	
> >  	set_sdr_ctrlcfg(cfg);
> > 
> > -	set_sdr_dram_timing(cfg);
> > +
> > +	debug("Configuring DRAMTIMING1\n");
> > +	writel(cfg->dram_timing1, &sdr_ctrl->dram_timing1);
> > +
> > +	debug("Configuring DRAMTIMING2\n");
> > +	writel(cfg->dram_timing2, &sdr_ctrl->dram_timing2);
> > +
> > +	debug("Configuring DRAMTIMING3\n");
> > +	writel(cfg->dram_timing3, &sdr_ctrl->dram_timing3);
> > +
> > +	debug("Configuring DRAMTIMING4\n");
> > +	writel(cfg->dram_timing4, &sdr_ctrl->dram_timing4);
> > +
> > +	debug("Configuring LOWPWRTIMING\n");
> > +	writel(cfg->lowpwr_timing, &sdr_ctrl->lowpwr_timing);
> > +
> 
> I don't think we need all of these debug prints?

Me neither ;-) I think there's waaaay too many (useless) debug() prints
throughout the entire DDR driver, not only here. I tried to preserve the
debug prints in their pristine state so far.

I think that in sequencer.c, the debug prints cause even more mess,
since there is a lot of code only to cater for printing debug stuff.
All kinda of variables get computed only to be used in some debug()
print and then discarded. I think a lot of code could be removed from
there if we discard those debug() prints.

I'd be happy if someone does a debug() print cleanup once things settle.
Would you like to prepare the patches ? :-)

Best regards,
Marek Vasut