[U-Boot] [PATCH v2] mmc: socfpga_dw_mmc: Move drvsel and smplsel to dts
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 --- 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@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;
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? */
On Thursday, November 26, 2015 at 02:10:04 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@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
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@ffd08000 {
I'm not very fond of bunding DT changes and driver changes together, these should be separate, so it'd be nice if you sent a V3 and split this into two patches.
Otherwise Acked-by: Marek Vasut marex@denx.de
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;
You can probably just use priv->xxx directly in the code and drop those vars.
debug("%s: drvsel %d smplsel %d\n", __func__, drvsel, smplsel); writel(SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel),
[...]
On Thu, 2015-11-26 at 02:26 +0100, Marek Vasut wrote:
On Thursday, November 26, 2015 at 02:10:04 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@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
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@ffd08000 {
I'm not very fond of bunding DT changes and driver changes together, these should be separate, so it'd be nice if you sent a V3 and split this into two patches.
Otherwise Acked-by: Marek Vasut marex@denx.de
Sure, I can split that as its easy to do that.
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;
You can probably just use priv->xxx directly in the code and drop those vars.
Sure, I can enhance that too
Thanks Chin Liang
debug("%s: drvsel %d smplsel %d\n", __func__, drvsel, smplsel); writel(SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel),
[...]
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@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
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@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?
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? */
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@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
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@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? */
participants (3)
-
Chin Liang See -
Jaehoon Chung -
Marek Vasut