[U-Boot] [PATCH v1 0/4] mmc: omap_hsmmc: Reduce the footprint of the driver and fix am335x clock
This series aims at reducing the footprint of the omap_hsmmc driver in the SPL (1.5 kB gain in the case of the SPL for the am335x evm). It also fixes an issue with the am335x_evm by setting a default maximum frequency if none is defined in the dts.
tested on am335x_evm, bbb, bbb (vboot), and dra7 evm
Jean-Jacques Hiblot (4): mmc: omap_hsmmc: do not embed struct mmc in struct omap_hsmmc_plat mmc: omap_hsmmc: compile out write support if not needed mmc: omap_hsmmc: compile out ADMA support for am33xx and omap34xx mmc: omap_hsmmc: use a default 52MHz max clock rate if none is specified
arch/arm/include/asm/omap_mmc.h | 2 +- drivers/mmc/omap_hsmmc.c | 35 ++++++++++++++++++++++++----------- 2 files changed, 25 insertions(+), 12 deletions(-)
The area for struct mmc can be allocated dynamically. It greatly reduces the size of struct omap_hsmmc_plat. This is useful in cases where the board level code declares one or two struct omap_hsmmc_plat because it doesn't use the Driver Model.
This saves around 740 bytes for the am335x_evm SPL.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com ---
arch/arm/include/asm/omap_mmc.h | 2 +- drivers/mmc/omap_hsmmc.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/omap_mmc.h b/arch/arm/include/asm/omap_mmc.h index 3d70148..42ce8dc 100644 --- a/arch/arm/include/asm/omap_mmc.h +++ b/arch/arm/include/asm/omap_mmc.h @@ -67,7 +67,7 @@ struct hsmmc { struct omap_hsmmc_plat { struct mmc_config cfg; struct hsmmc *base_addr; - struct mmc mmc; + struct mmc *mmc; bool cd_inverted; u32 controller_flags; const char *hw_rev; diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index 02970f2..e0b679a 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -1858,8 +1858,8 @@ static int omap_hsmmc_ofdata_to_platdata(struct udevice *dev) static int omap_hsmmc_bind(struct udevice *dev) { struct omap_hsmmc_plat *plat = dev_get_platdata(dev); - - return mmc_bind(dev, &plat->mmc, &plat->cfg); + plat->mmc = calloc(1, sizeof(struct mmc)); + return mmc_bind(dev, plat->mmc, &plat->cfg); } #endif static int omap_hsmmc_probe(struct udevice *dev) @@ -1882,7 +1882,7 @@ static int omap_hsmmc_probe(struct udevice *dev) #endif
#ifdef CONFIG_BLK - mmc = &plat->mmc; + mmc = plat->mmc; #else mmc = mmc_create(cfg, priv); if (mmc == NULL)
On Thu, Feb 22, 2018 at 11:25:45AM +0100, Jean-Jacques Hiblot wrote:
The area for struct mmc can be allocated dynamically. It greatly reduces the size of struct omap_hsmmc_plat. This is useful in cases where the board level code declares one or two struct omap_hsmmc_plat because it doesn't use the Driver Model.
This saves around 740 bytes for the am335x_evm SPL.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com
Hi Jean-Jacques,
On 22 February 2018 at 03:25, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
The area for struct mmc can be allocated dynamically. It greatly reduces the size of struct omap_hsmmc_plat. This is useful in cases where the board level code declares one or two struct omap_hsmmc_plat because it doesn't use the Driver Model.
This saves around 740 bytes for the am335x_evm SPL.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
arch/arm/include/asm/omap_mmc.h | 2 +- drivers/mmc/omap_hsmmc.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I would like to understand why this saves memory though. Presumably the pointer has to point to a real struct anyway, which uses memory. So how does this help?
- Simon
On 23/02/2018 21:59, Simon Glass wrote:
Hi Jean-Jacques,
On 22 February 2018 at 03:25, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
The area for struct mmc can be allocated dynamically. It greatly reduces the size of struct omap_hsmmc_plat. This is useful in cases where the board level code declares one or two struct omap_hsmmc_plat because it doesn't use the Driver Model.
This saves around 740 bytes for the am335x_evm SPL.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
arch/arm/include/asm/omap_mmc.h | 2 +- drivers/mmc/omap_hsmmc.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I would like to understand why this saves memory though. Presumably the pointer has to point to a real struct anyway, which uses memory. So how does this help?
struct omap_hsmmc_plat are initialized variables so they are part of the binary. With this patch the memory is dynamically allocated so that it's not taking space in the binary.
JJ
- Simon
This reduces the size of the binary by about 196 bytes.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com ---
drivers/mmc/omap_hsmmc.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index e0b679a..8b57edc 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -1181,8 +1181,9 @@ static int mmc_read_data(struct hsmmc *mmc_base, char *buf, unsigned int size) return 0; }
+#if CONFIG_IS_ENABLED(MMC_WRITE) static int mmc_write_data(struct hsmmc *mmc_base, const char *buf, - unsigned int size) + unsigned int size) { unsigned int *input_buf = (unsigned int *)buf; unsigned int mmc_stat; @@ -1235,7 +1236,13 @@ static int mmc_write_data(struct hsmmc *mmc_base, const char *buf, } return 0; } - +#else +static int mmc_write_data(struct hsmmc *mmc_base, const char *buf, + unsigned int size) +{ + return -ENOTSUPP; +} +#endif static void omap_hsmmc_stop_clock(struct hsmmc *mmc_base) { writel(readl(&mmc_base->sysctl) & ~CEN_ENABLE, &mmc_base->sysctl);
On Thu, Feb 22, 2018 at 11:25:46AM +0100, Jean-Jacques Hiblot wrote:
This reduces the size of the binary by about 196 bytes.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com
This reduces the size of the binary by about 600 bytes.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com ---
drivers/mmc/omap_hsmmc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index 8b57edc..31c1f66 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -48,6 +48,10 @@ #include <dm.h> #include <power/regulator.h>
+#if !defined(CONFIG_AM33XX) && !defined(CONFIG_OMAP34XX) +#define ADMA_SUPPORT +#endif + DECLARE_GLOBAL_DATA_PTR;
/* simplify defines to OMAP_HSMMC_USE_GPIO */ @@ -93,7 +97,7 @@ struct omap_hsmmc_data { enum bus_mode mode; #endif u8 controller_flags; -#ifndef CONFIG_OMAP34XX +#ifdef ADMA_SUPPORT struct omap_hsmmc_adma_desc *adma_desc_table; uint desc_slot; #endif @@ -117,7 +121,7 @@ struct omap_mmc_of_data { u8 controller_flags; };
-#ifndef CONFIG_OMAP34XX +#ifdef ADMA_SUPPORT struct omap_hsmmc_adma_desc { u8 attr; u8 reserved; @@ -741,7 +745,7 @@ static int omap_hsmmc_init_setup(struct mmc *mmc) return -ETIMEDOUT; } } -#ifndef CONFIG_OMAP34XX +#ifdef ADMA_SUPPORT reg_val = readl(&mmc_base->hl_hwinfo); if (reg_val & MADMA_EN) priv->controller_flags |= OMAP_HSMMC_USE_ADMA; @@ -834,7 +838,7 @@ static void mmc_reset_controller_fsm(struct hsmmc *mmc_base, u32 bit) } }
-#ifndef CONFIG_OMAP34XX +#ifdef ADMA_SUPPORT static void omap_hsmmc_adma_desc(struct mmc *mmc, char *buf, u16 len, bool end) { struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc); @@ -1037,7 +1041,7 @@ static int omap_hsmmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd, else flags |= (DP_DATA | DDIR_WRITE);
-#ifndef CONFIG_OMAP34XX +#ifdef ADMA_SUPPORT if ((priv->controller_flags & OMAP_HSMMC_USE_ADMA) && !mmc_is_tuning_cmd(cmd->cmdidx)) { omap_hsmmc_prepare_data(mmc, data); @@ -1082,7 +1086,7 @@ static int omap_hsmmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd, } }
-#ifndef CONFIG_OMAP34XX +#ifdef ADMA_SUPPORT if ((priv->controller_flags & OMAP_HSMMC_USE_ADMA) && data && !mmc_is_tuning_cmd(cmd->cmdidx)) { u32 sz_mb, timeout;
On Thu, Feb 22, 2018 at 11:25:47AM +0100, Jean-Jacques Hiblot wrote:
This reduces the size of the binary by about 600 bytes.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
drivers/mmc/omap_hsmmc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index 8b57edc..31c1f66 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -48,6 +48,10 @@ #include <dm.h> #include <power/regulator.h>
+#if !defined(CONFIG_AM33XX) && !defined(CONFIG_OMAP34XX) +#define ADMA_SUPPORT +#endif
DECLARE_GLOBAL_DATA_PTR;
/* simplify defines to OMAP_HSMMC_USE_GPIO */ @@ -93,7 +97,7 @@ struct omap_hsmmc_data { enum bus_mode mode; #endif u8 controller_flags; -#ifndef CONFIG_OMAP34XX +#ifdef ADMA_SUPPORT
How about we add CONFIG_MMC_OMAP_HS_ADMA, and select it on !AM33XX && !OMAP34XX, and then test that here? Thanks!
On 02/23/2018 12:47 AM, Tom Rini wrote:
On Thu, Feb 22, 2018 at 11:25:47AM +0100, Jean-Jacques Hiblot wrote:
This reduces the size of the binary by about 600 bytes.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
drivers/mmc/omap_hsmmc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index 8b57edc..31c1f66 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -48,6 +48,10 @@ #include <dm.h> #include <power/regulator.h>
+#if !defined(CONFIG_AM33XX) && !defined(CONFIG_OMAP34XX) +#define ADMA_SUPPORT +#endif
DECLARE_GLOBAL_DATA_PTR;
/* simplify defines to OMAP_HSMMC_USE_GPIO */ @@ -93,7 +97,7 @@ struct omap_hsmmc_data { enum bus_mode mode; #endif u8 controller_flags; -#ifndef CONFIG_OMAP34XX +#ifdef ADMA_SUPPORT
How about we add CONFIG_MMC_OMAP_HS_ADMA, and select it on !AM33XX && !OMAP34XX, and then test that here? Thanks!
I agreed Tom's suggestion. If you can send v2 with it, I will pick these series with Tom's Reviewed's tag and Adam's Tested tag.
Best Regards, Jaehoon Chung
mmc_of_parse() doesn't set a default value if none is available in DT. In that case, use a default 52MHz clock rate.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
---
drivers/mmc/omap_hsmmc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index 31c1f66..f61fae9 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -1836,6 +1836,8 @@ static int omap_hsmmc_ofdata_to_platdata(struct udevice *dev) if (ret < 0) return ret;
+ if (!cfg->f_max) + cfg->f_max = 52000000; cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS; cfg->f_min = 400000; cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
On Thu, Feb 22, 2018 at 11:25:48AM +0100, Jean-Jacques Hiblot wrote:
mmc_of_parse() doesn't set a default value if none is available in DT. In that case, use a default 52MHz clock rate.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com
On Thu, Feb 22, 2018 at 4:25 AM, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
This series aims at reducing the footprint of the omap_hsmmc driver in the SPL (1.5 kB gain in the case of the SPL for the am335x evm). It also fixes an issue with the am335x_evm by setting a default maximum frequency if none is defined in the dts.
tested on am335x_evm, bbb, bbb (vboot), and dra7 evm
This series fixes 2d7482cf793f ("mmc: omap_hsmmc: use mmc_of_parse to populate mmc_config") which made my DM3730 unable to probe the MMC.
Tested-by: Adam Ford aford173@gmail.com #omap3_logic
Jean-Jacques Hiblot (4): mmc: omap_hsmmc: do not embed struct mmc in struct omap_hsmmc_plat mmc: omap_hsmmc: compile out write support if not needed mmc: omap_hsmmc: compile out ADMA support for am33xx and omap34xx mmc: omap_hsmmc: use a default 52MHz max clock rate if none is specified
arch/arm/include/asm/omap_mmc.h | 2 +- drivers/mmc/omap_hsmmc.c | 35 ++++++++++++++++++++++++----------- 2 files changed, 25 insertions(+), 12 deletions(-)
-- 1.9.1
participants (5)
-
Adam Ford -
Jaehoon Chung -
Jean-Jacques Hiblot -
Simon Glass -
Tom Rini