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 <cl...@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 <cl...@altera.com> > > Cc: Dinh Nguyen <dingu...@opensource.altera.com> > > Cc: Dinh Nguyen <dinh.li...@gmail.com> > > Cc: Pavel Machek <pa...@denx.de> > > Cc: Marek Vasut <ma...@denx.de> > > Cc: Stefan Roese <s...@denx.de> > > Cc: Pantelis Antoniou <pantelis.anton...@konsulko.com> > > Cc: Simon Glass <s...@chromium.org> > > Cc: Jaehoon Chung <jh80.ch...@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? */ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
