diff mbox

[U-Boot,16/28] ddr: altera: sdram: Clean up sdram_mmr_init_full() part 4

Message ID 1438464897-8051-17-git-send-email-marex@denx.de
State Accepted
Delegated to: Marek Vasut
Headers show

Commit Message

Marek Vasut Aug. 1, 2015, 9:34 p.m. UTC 
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(-)

Comments

Dinh Nguyen Aug. 5, 2015, 3:59 p.m. UTC | #1 
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?

Dinh
Marek Vasut Aug. 5, 2015, 4:12 p.m. UTC | #2 
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
diff mbox

Patch

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);
 }
 
-static void set_sdr_dram_timing(struct socfpga_sdram_config *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);
-}
-
 static void set_sdr_addr_rw(struct socfpga_sdram_config *cfg)
 {
 	/*
@@ -536,44 +518,6 @@  static void set_sdr_addr_rw(struct socfpga_sdram_config *cfg)
 	       &sdr_ctrl->dram_addrw);
 }
 
-static void set_sdr_static_cfg(struct socfpga_sdram_config *cfg)
-{
-	debug("Configuring STATICCFG\n");
-	writel(cfg->static_cfg, &sdr_ctrl->static_cfg);
-}
-
-static void set_sdr_fifo_cfg(struct socfpga_sdram_config *cfg)
-{
-	debug("Configuring FIFOCFG\n");
-	writel(cfg->fifo_cfg, &sdr_ctrl->fifo_cfg);
-}
-
-static void set_sdr_mp_weight(struct socfpga_sdram_config *cfg)
-{
-	debug("Configuring MPWEIGHT_MPWEIGHT_0\n");
-	writel(cfg->mp_weight0, &sdr_ctrl->mp_weight0);
-	writel(cfg->mp_weight1, &sdr_ctrl->mp_weight1);
-	writel(cfg->mp_weight2, &sdr_ctrl->mp_weight2);
-	writel(cfg->mp_weight3, &sdr_ctrl->mp_weight3);
-}
-
-static void set_sdr_mp_pacing(struct socfpga_sdram_config *cfg)
-{
-	debug("Configuring MPPACING_MPPACING_0\n");
-	writel(cfg->mp_pacing0, &sdr_ctrl->mp_pacing0);
-	writel(cfg->mp_pacing1, &sdr_ctrl->mp_pacing1);
-	writel(cfg->mp_pacing2, &sdr_ctrl->mp_pacing2);
-	writel(cfg->mp_pacing3, &sdr_ctrl->mp_pacing3);
-}
-
-static void set_sdr_mp_threshold(struct socfpga_sdram_config *cfg)
-{
-	debug("Configuring MPTHRESHOLDRST_MPTHRESHOLDRST_0\n");
-	writel(cfg->mp_threshold0, &sdr_ctrl->mp_threshold0);
-	writel(cfg->mp_threshold1, &sdr_ctrl->mp_threshold1);
-	writel(cfg->mp_threshold2, &sdr_ctrl->mp_threshold2);
-}
-
 /* Function to initialize SDRAM MMR */
 unsigned sdram_mmr_init_full(unsigned int sdr_phy_reg)
 {
@@ -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);
+
 	set_sdr_addr_rw(cfg);
 
 	debug("Configuring DRAMIFWIDTH\n");
@@ -601,7 +560,8 @@  unsigned sdram_mmr_init_full(unsigned int sdr_phy_reg)
 	debug("Configuring DRAMINTR\n");
 	writel(cfg->dram_intr, &sdr_ctrl->dram_intr);
 
-	set_sdr_static_cfg(cfg);
+	debug("Configuring STATICCFG\n");
+	writel(cfg->static_cfg, &sdr_ctrl->static_cfg);
 
 	debug("Configuring CTRLWIDTH\n");
 	writel(cfg->ctrl_width, &sdr_ctrl->ctrl_width);
@@ -609,14 +569,28 @@  unsigned sdram_mmr_init_full(unsigned int sdr_phy_reg)
 	debug("Configuring PORTCFG\n");
 	writel(cfg->port_cfg, &sdr_ctrl->port_cfg);
 
-	set_sdr_fifo_cfg(cfg);
+	debug("Configuring FIFOCFG\n");
+	writel(cfg->fifo_cfg, &sdr_ctrl->fifo_cfg);
 
 	debug("Configuring MPPRIORITY\n");
 	writel(cfg->mp_priority, &sdr_ctrl->mp_priority);
 
-	set_sdr_mp_weight(cfg);
-	set_sdr_mp_pacing(cfg);
-	set_sdr_mp_threshold(cfg);
+	debug("Configuring MPWEIGHT_MPWEIGHT_0\n");
+	writel(cfg->mp_weight0, &sdr_ctrl->mp_weight0);
+	writel(cfg->mp_weight1, &sdr_ctrl->mp_weight1);
+	writel(cfg->mp_weight2, &sdr_ctrl->mp_weight2);
+	writel(cfg->mp_weight3, &sdr_ctrl->mp_weight3);
+
+	debug("Configuring MPPACING_MPPACING_0\n");
+	writel(cfg->mp_pacing0, &sdr_ctrl->mp_pacing0);
+	writel(cfg->mp_pacing1, &sdr_ctrl->mp_pacing1);
+	writel(cfg->mp_pacing2, &sdr_ctrl->mp_pacing2);
+	writel(cfg->mp_pacing3, &sdr_ctrl->mp_pacing3);
+
+	debug("Configuring MPTHRESHOLDRST_MPTHRESHOLDRST_0\n");
+	writel(cfg->mp_threshold0, &sdr_ctrl->mp_threshold0);
+	writel(cfg->mp_threshold1, &sdr_ctrl->mp_threshold1);
+	writel(cfg->mp_threshold2, &sdr_ctrl->mp_threshold2);
 
 	debug("Configuring PHYCTRL_PHYCTRL_0\n");
 	writel(cfg->phy_ctrl0, &sdr_ctrl->phy_ctrl0);