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

[Chin Liang See]

Hi Marek,

On Thu, 2015-11-26 at 01:50 +0100, Marek Vasut wrote:
> On Thursday, November 26, 2015 at 01:41:12 AM, clsee wrote:
> > From: Chin Liang See <clsee at altera.com>
> > 
> > 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>
> > ---
> >  arch/arm/dts/socfpga_cyclone5.dtsi |  2 ++
> >  drivers/mmc/socfpga_dw_mmc.c       | 24 ++++++++++++++++++++++--
> >  include/configs/socfpga_common.h   |  2 --
> >  3 files changed, 24 insertions(+), 4 deletions(-)
> 
> Hi!
> 
> [...]
> 
> > @@ -78,11 +87,19 @@ static int socfpga_dwmci_of_probe(const void
> > *blob, int
> > node, const int idx) return -EINVAL;
> >  	}
> > 
> > +	drvsel = fdtdec_get_uint(blob, node, "drvsel", 0);
> 
> The default value here should be 3, otherwise this won't preserve the
> original
> behavior of the driver in case the nodes in DT are missing.

Nice thinking, will update this.

> 
> > +	smplsel = fdtdec_get_uint(blob, node, "smplsel", 0);
> > +
> >  	/* Allocate the host */
> >  	host = calloc(1, sizeof(*host));
> >  	if (!host)
> >  		return -ENOMEM;
> > 
> > +	/* Allocate the priv */
> > +	priv = calloc(1, sizeof(*priv));
> > +	if (!priv)
> > +		return -ENOMEM;
> 
> If this call fails, you're leaking memory, since you calloc() some
> stuff before.
> 

Oops, yup, this need to be fixed

> > +
> >  	host->name = "SOCFPGA DWMMC";
> >  	host->ioaddr = (void *)reg_base;
> >  	host->buswidth = bus_width;
> > @@ -92,6 +109,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 = drvsel;
> > +	priv->smplsel = smplsel;
> > +	host->priv = priv;
> 
> You can move the fdtdec_get_uint() calls here directly, no need to
> introduce
> ad-hoc variable and then just assign it into the private data.

Let me enhance this too

Thanks
Chin Liang

> 
> >  	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? */