[U-Boot] [PATCH v2 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
Changes in v2: - Use a Kconfig option to compile out the ADMA support instead of relying on the platform architecture
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: make it possible to compile out ADMA support mmc: omap_hsmmc: use a default 52MHz max clock rate if none is specified
arch/arm/include/asm/omap_mmc.h | 2 +- drivers/mmc/Kconfig | 9 +++++++++ drivers/mmc/omap_hsmmc.c | 31 ++++++++++++++++++++----------- 3 files changed, 30 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 ---
Changes in v2: None
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 Fri, Feb 23, 2018 at 10:40:16AM +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
On 02/23/2018 09:21 PM, Tom Rini wrote:
On Fri, Feb 23, 2018 at 10:40:16AM +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
Applied to u-boot-mmc. Thanks!
Best Regards, Jaehoon Chung
This reduces the size of the binary by about 196 bytes.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com ---
Changes in v2: None
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 Fri, Feb 23, 2018 at 10:40:17AM +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
On 02/23/2018 09:21 PM, Tom Rini wrote:
On Fri, Feb 23, 2018 at 10:40:17AM +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
Applied to u-boot-mmc. Thanks!
Best Regards, Jaehoon Chung
Some platforms don't have ADMA controllers. For those platforms, compiling it out reduces the size of the binary by about 600 bytes. Leaving the support in doesn't break things as the driver checks at runtime if the ADMA2 controller is present.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
---
Changes in v2: - Use a Kconfig option to compile out the ADMA support instead of relying on the platform architecture
drivers/mmc/Kconfig | 9 +++++++++ drivers/mmc/omap_hsmmc.c | 12 ++++++------ 2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index f2d8256..88a1359 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -239,6 +239,15 @@ config MMC_OMAP_HS
If unsure, say N.
+config MMC_OMAP_HS_ADMA + bool "ADMA support for OMAP HS MMC" + depends on MMC_OMAP_HS && !OMAP34XX + default y if !AM33XX + help + This enables support for the ADMA2 controller (SDA3.00 Part A2 DMA + controller). If supported by the hardware, selecting this option will + increase performances. + config MMC_OMAP36XX_PINS bool "Enable MMC1 on OMAP36xx/37xx" depends on OMAP34XX && MMC_OMAP_HS diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index 8b57edc..3d2836d 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -93,7 +93,7 @@ struct omap_hsmmc_data { enum bus_mode mode; #endif u8 controller_flags; -#ifndef CONFIG_OMAP34XX +#ifdef CONFIG_MMC_OMAP_HS_ADMA struct omap_hsmmc_adma_desc *adma_desc_table; uint desc_slot; #endif @@ -117,7 +117,7 @@ struct omap_mmc_of_data { u8 controller_flags; };
-#ifndef CONFIG_OMAP34XX +#ifdef CONFIG_MMC_OMAP_HS_ADMA struct omap_hsmmc_adma_desc { u8 attr; u8 reserved; @@ -741,7 +741,7 @@ static int omap_hsmmc_init_setup(struct mmc *mmc) return -ETIMEDOUT; } } -#ifndef CONFIG_OMAP34XX +#ifdef CONFIG_MMC_OMAP_HS_ADMA reg_val = readl(&mmc_base->hl_hwinfo); if (reg_val & MADMA_EN) priv->controller_flags |= OMAP_HSMMC_USE_ADMA; @@ -834,7 +834,7 @@ static void mmc_reset_controller_fsm(struct hsmmc *mmc_base, u32 bit) } }
-#ifndef CONFIG_OMAP34XX +#ifdef CONFIG_MMC_OMAP_HS_ADMA 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 +1037,7 @@ static int omap_hsmmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd, else flags |= (DP_DATA | DDIR_WRITE);
-#ifndef CONFIG_OMAP34XX +#ifdef CONFIG_MMC_OMAP_HS_ADMA if ((priv->controller_flags & OMAP_HSMMC_USE_ADMA) && !mmc_is_tuning_cmd(cmd->cmdidx)) { omap_hsmmc_prepare_data(mmc, data); @@ -1082,7 +1082,7 @@ static int omap_hsmmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd, } }
-#ifndef CONFIG_OMAP34XX +#ifdef CONFIG_MMC_OMAP_HS_ADMA if ((priv->controller_flags & OMAP_HSMMC_USE_ADMA) && data && !mmc_is_tuning_cmd(cmd->cmdidx)) { u32 sz_mb, timeout;
On Fri, Feb 23, 2018 at 10:40:18AM +0100, Jean-Jacques Hiblot wrote:
Some platforms don't have ADMA controllers. For those platforms, compiling it out reduces the size of the binary by about 600 bytes. Leaving the support in doesn't break things as the driver checks at runtime if the ADMA2 controller is present.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2:
- Use a Kconfig option to compile out the ADMA support instead of relying on the platform architecture
drivers/mmc/Kconfig | 9 +++++++++ drivers/mmc/omap_hsmmc.c | 12 ++++++------ 2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index f2d8256..88a1359 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -239,6 +239,15 @@ config MMC_OMAP_HS
If unsure, say N.+config MMC_OMAP_HS_ADMA
- bool "ADMA support for OMAP HS MMC"
- depends on MMC_OMAP_HS && !OMAP34XX
- default y if !AM33XX
Is this logic really the right restrictions? Does it work on some AM33XX platforms (AM43xx?) and just never OMAP34XX? Thanks!
On 23/02/2018 13:21, Tom Rini wrote:
On Fri, Feb 23, 2018 at 10:40:18AM +0100, Jean-Jacques Hiblot wrote:
Some platforms don't have ADMA controllers. For those platforms, compiling it out reduces the size of the binary by about 600 bytes. Leaving the support in doesn't break things as the driver checks at runtime if the ADMA2 controller is present.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2:
Use a Kconfig option to compile out the ADMA support instead of relying on the platform architecture
drivers/mmc/Kconfig | 9 +++++++++ drivers/mmc/omap_hsmmc.c | 12 ++++++------ 2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index f2d8256..88a1359 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -239,6 +239,15 @@ config MMC_OMAP_HS
If unsure, say N.+config MMC_OMAP_HS_ADMA
- bool "ADMA support for OMAP HS MMC"
- depends on MMC_OMAP_HS && !OMAP34XX
- default y if !AM33XX
Is this logic really the right restrictions? Does it work on some AM33XX platforms (AM43xx?) and just never OMAP34XX? Thanks!
Starting with platform am33xx there is a way to check if the ADMA controller is present or not by reading a register. This register is not available in OMAP34XX. That being said I don't think that ADMA is available on any variant of am33xx.
JJ
On Fri, Feb 23, 2018 at 01:47:36PM +0100, Jean-Jacques Hiblot wrote:
On 23/02/2018 13:21, Tom Rini wrote:
On Fri, Feb 23, 2018 at 10:40:18AM +0100, Jean-Jacques Hiblot wrote:
Some platforms don't have ADMA controllers. For those platforms, compiling it out reduces the size of the binary by about 600 bytes. Leaving the support in doesn't break things as the driver checks at runtime if the ADMA2 controller is present.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2:
- Use a Kconfig option to compile out the ADMA support instead of relying on the platform architecture
drivers/mmc/Kconfig | 9 +++++++++ drivers/mmc/omap_hsmmc.c | 12 ++++++------ 2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index f2d8256..88a1359 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -239,6 +239,15 @@ config MMC_OMAP_HS If unsure, say N. +config MMC_OMAP_HS_ADMA
- bool "ADMA support for OMAP HS MMC"
- depends on MMC_OMAP_HS && !OMAP34XX
- default y if !AM33XX
Is this logic really the right restrictions? Does it work on some AM33XX platforms (AM43xx?) and just never OMAP34XX? Thanks!
Starting with platform am33xx there is a way to check if the ADMA controller is present or not by reading a register. This register is not available in OMAP34XX. That being said I don't think that ADMA is available on any variant of am33xx.
OK, thanks!
Reviewed-by: Tom Rini trini@konsulko.com
On 02/23/2018 09:57 PM, Tom Rini wrote:
On Fri, Feb 23, 2018 at 01:47:36PM +0100, Jean-Jacques Hiblot wrote:
On 23/02/2018 13:21, Tom Rini wrote:
On Fri, Feb 23, 2018 at 10:40:18AM +0100, Jean-Jacques Hiblot wrote:
Some platforms don't have ADMA controllers. For those platforms, compiling it out reduces the size of the binary by about 600 bytes. Leaving the support in doesn't break things as the driver checks at runtime if the ADMA2 controller is present.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2:
- Use a Kconfig option to compile out the ADMA support instead of relying on the platform architecture
drivers/mmc/Kconfig | 9 +++++++++ drivers/mmc/omap_hsmmc.c | 12 ++++++------ 2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index f2d8256..88a1359 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -239,6 +239,15 @@ config MMC_OMAP_HS If unsure, say N. +config MMC_OMAP_HS_ADMA
- bool "ADMA support for OMAP HS MMC"
- depends on MMC_OMAP_HS && !OMAP34XX
- default y if !AM33XX
Is this logic really the right restrictions? Does it work on some AM33XX platforms (AM43xx?) and just never OMAP34XX? Thanks!
Starting with platform am33xx there is a way to check if the ADMA controller is present or not by reading a register. This register is not available in OMAP34XX. That being said I don't think that ADMA is available on any variant of am33xx.
OK, thanks!
Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot-mmc. Thanks!
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
---
Changes in v2: None
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 3d2836d..caaa914 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -1832,6 +1832,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 Fri, Feb 23, 2018 at 10:40:19AM +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 02/23/2018 09:21 PM, Tom Rini wrote:
On Fri, Feb 23, 2018 at 10:40:19AM +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
Applied to u-boot-mmc. Thanks!
Best Regards, Jaehoon Chung
On Fri, Feb 23, 2018 at 3:40 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
I tested this version as well.
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
Changes in v2:
- Use a Kconfig option to compile out the ADMA support instead of relying on the platform architecture
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: make it possible to compile out ADMA support mmc: omap_hsmmc: use a default 52MHz max clock rate if none is specified
arch/arm/include/asm/omap_mmc.h | 2 +- drivers/mmc/Kconfig | 9 +++++++++ drivers/mmc/omap_hsmmc.c | 31 ++++++++++++++++++++----------- 3 files changed, 30 insertions(+), 12 deletions(-)
-- 1.9.1
On 02/23/2018 07:24 PM, Adam Ford wrote:
On Fri, Feb 23, 2018 at 3:40 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
I tested this version as well.
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
After testing, I will apply these patches with your tag.
Best Regards, Jaehoon Chung
Changes in v2:
- Use a Kconfig option to compile out the ADMA support instead of relying on the platform architecture
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: make it possible to compile out ADMA support mmc: omap_hsmmc: use a default 52MHz max clock rate if none is specified
arch/arm/include/asm/omap_mmc.h | 2 +- drivers/mmc/Kconfig | 9 +++++++++ drivers/mmc/omap_hsmmc.c | 31 ++++++++++++++++++++----------- 3 files changed, 30 insertions(+), 12 deletions(-)
-- 1.9.1
participants (4)
-
Adam Ford -
Jaehoon Chung -
Jean-Jacques Hiblot -
Tom Rini