[U-Boot] [PATCH] mmc: socfpga_dw_mmc: Move drvsel and smplsel to dts
From: Chin Liang See clsee@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@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com Cc: Dinh Nguyen dinh.linux@gmail.com Cc: Pavel Machek pavel@denx.de Cc: Marek Vasut marex@denx.de Cc: Stefan Roese sr@denx.de Cc: Pantelis Antoniou pantelis.antoniou@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Jaehoon Chung jh80.chung@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(-)
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@ffd08000 { diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c index 8076761..2cd7a51 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;
debug("%s: drvsel %d smplsel %d\n", __func__, drvsel, smplsel); writel(SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel), @@ -50,8 +57,10 @@ 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; + unsigned int drvsel, smplsel;
if (clk == 0) { printf("DWMMC%d: MMC clock is zero!", idx); @@ -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); + 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; + 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;
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? */
On Thursday, November 26, 2015 at 01:41:12 AM, clsee wrote:
From: Chin Liang See clsee@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@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com Cc: Dinh Nguyen dinh.linux@gmail.com Cc: Pavel Machek pavel@denx.de Cc: Marek Vasut marex@denx.de Cc: Stefan Roese sr@denx.de Cc: Pantelis Antoniou pantelis.antoniou@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Jaehoon Chung jh80.chung@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.
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.
- 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.
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? */
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@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@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com Cc: Dinh Nguyen dinh.linux@gmail.com Cc: Pavel Machek pavel@denx.de Cc: Marek Vasut marex@denx.de Cc: Stefan Roese sr@denx.de Cc: Pantelis Antoniou pantelis.antoniou@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Jaehoon Chung jh80.chung@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? */
participants (3)
-
Chin Liang See -
clsee
-
Marek Vasut