[U-Boot] [PATCH v2] mmc: socfpga_dw_mmc: Move drvsel and smplsel to dts

[Chin Liang See]

On Thu, 2015-11-26 at 10:33 +0900, Jaehoon Chung wrote:
> On 11/26/2015 10:10 AM, Chin Liang See wrote:
> > socfpga_dw_mmc driver will obtain the drvsel and
> > smplsel value from device tree instead of definition
> > in config header file.
> > 
> > Signed-off-by: Chin Liang See <clsee at altera.com>
> > Cc: Dinh Nguyen <dinguyen at opensource.altera.com>
> > Cc: Dinh Nguyen <dinh.linux at gmail.com>
> > Cc: Pavel Machek <pavel at denx.de>
> > Cc: Marek Vasut <marex at denx.de>
> > Cc: Stefan Roese <sr at denx.de>
> > Cc: Pantelis Antoniou <pantelis.antoniou at konsulko.com>
> > Cc: Simon Glass <sjg at chromium.org>
> > Cc: Jaehoon Chung <jh80.chung at samsung.com>
> > ---
> > Changes for v2
> > - Put default value for drvsel to 3 in case node in DT missing
> > - Remove unnecessary ad-hoc vairable
> > - Free up first calloc if second calloc failed
> > ---
> >  arch/arm/dts/socfpga_cyclone5.dtsi |  2 ++
> >  drivers/mmc/socfpga_dw_mmc.c       | 22 ++++++++++++++++++++--
> >  include/configs/socfpga_common.h   |  2 --
> >  3 files changed, 22 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/dts/socfpga_cyclone5.dtsi
> > b/arch/arm/dts/socfpga_cyclone5.dtsi
> > index de36209..040b236 100644
> > --- a/arch/arm/dts/socfpga_cyclone5.dtsi
> > +++ b/arch/arm/dts/socfpga_cyclone5.dtsi
> > @@ -25,6 +25,8 @@
> >  			bus-width = <4>;
> >  			cap-mmc-highspeed;
> >  			cap-sd-highspeed;
> > +			drvsel = <3>;
> > +			smplsel = <0>;
> >  		};
> >  
> >  		sysmgr at ffd08000 {
> > diff --git a/drivers/mmc/socfpga_dw_mmc.c
> > b/drivers/mmc/socfpga_dw_mmc.c
> > index 8076761..38eb783 100644
> > --- a/drivers/mmc/socfpga_dw_mmc.c
> > +++ b/drivers/mmc/socfpga_dw_mmc.c
> > @@ -19,18 +19,25 @@ static const struct socfpga_clock_manager
> > *clock_manager_base =
> >  static const struct socfpga_system_manager *system_manager_base =
> >  		(void *)SOCFPGA_SYSMGR_ADDRESS;
> >  
> > +/* socfpga implmentation specific drver private data */
> > +struct dwmci_socfpga_priv_data {
> > +	unsigned int drvsel;
> > +	unsigned int smplsel;
> > +};
> > +
> >  static void socfpga_dwmci_clksel(struct dwmci_host *host)
> >  {
> >  	unsigned int drvsel;
> >  	unsigned int smplsel;
> > +	struct dwmci_socfpga_priv_data *priv = host->priv;
> >  
> >  	/* Disable SDMMC clock. */
> >  	clrbits_le32(&clock_manager_base->per_pll.en,
> >  		CLKMGR_PERPLLGRP_EN_SDMMCCLK_MASK);
> >  
> >  	/* Configures drv_sel and smpl_sel */
> > -	drvsel = CONFIG_SOCFPGA_DWMMC_DRVSEL;
> > -	smplsel = CONFIG_SOCFPGA_DWMMC_SMPSEL;
> > +	drvsel = priv->drvsel;
> > +	smplsel = priv->smplsel;
> 
> I found that drvsel and smplsel are used at only here.
> I think they should be removed.
> (priv->drvsel/smplse don't need to assign to drvsel/smplsel.)
> 
> how about?
> 

Yup, Marek just note this to me too.
I will get rid the variable and use the priv directly

Thanks
Chin Liang

> Best Regards,
> Jaehoon Chung
> 
> >  
> >  	debug("%s: drvsel %d smplsel %d\n", __func__, drvsel,
> > smplsel);
> >  	writel(SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel),
> > @@ -50,6 +57,7 @@ static int socfpga_dwmci_of_probe(const void
> > *blob, int node, const int idx)
> >  	const unsigned long clk = cm_get_mmc_controller_clk_hz();
> >  
> >  	struct dwmci_host *host;
> > +	struct dwmci_socfpga_priv_data *priv;
> >  	fdt_addr_t reg_base;
> >  	int bus_width, fifo_depth;
> >  
> > @@ -83,6 +91,13 @@ static int socfpga_dwmci_of_probe(const void
> > *blob, int node, const int idx)
> >  	if (!host)
> >  		return -ENOMEM;
> >  
> > +	/* Allocate the priv */
> > +	priv = calloc(1, sizeof(*priv));
> > +	if (!priv) {
> > +		free(host);
> > +		return -ENOMEM;
> > +	}
> > +
> >  	host->name = "SOCFPGA DWMMC";
> >  	host->ioaddr = (void *)reg_base;
> >  	host->buswidth = bus_width;
> > @@ -92,6 +107,9 @@ static int socfpga_dwmci_of_probe(const void
> > *blob, int node, const int idx)
> >  	host->bus_hz = clk;
> >  	host->fifoth_val = MSIZE(0x2) |
> >  		RX_WMARK(fifo_depth / 2 - 1) | TX_WMARK(fifo_depth
> > / 2);
> > +	priv->drvsel = fdtdec_get_uint(blob, node, "drvsel", 3);
> > +	priv->smplsel = fdtdec_get_uint(blob, node, "smplsel", 0);
> > +	host->priv = priv;
> >  
> >  	return add_dwmci(host, host->bus_hz, 400000);
> >  }
> > diff --git a/include/configs/socfpga_common.h
> > b/include/configs/socfpga_common.h
> > index f6808b5..b661cc2 100644
> > --- a/include/configs/socfpga_common.h
> > +++ b/include/configs/socfpga_common.h
> > @@ -153,8 +153,6 @@
> >  #define CONFIG_DWMMC
> >  #define CONFIG_SOCFPGA_DWMMC
> >  #define CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH	1024
> > -#define CONFIG_SOCFPGA_DWMMC_DRVSEL	3
> > -#define CONFIG_SOCFPGA_DWMMC_SMPSEL	0
> >  /* FIXME */
> >  /* using smaller max blk cnt to avoid flooding the limited stack
> > we have */
> >  #define CONFIG_SYS_MMC_MAX_BLK_COUNT	256	/* FIXME --
> > SPL only? */
> > 
>