* [U-Boot] [PATCH v1 1/4] mmc: omap_hsmmc: do not embed struct mmc in struct omap_hsmmc_plat
2018-02-22 10:25 [U-Boot] [PATCH v1 0/4] mmc: omap_hsmmc: Reduce the footprint of the driver and fix am335x clock Jean-Jacques Hiblot
@ 2018-02-22 10:25 ` Jean-Jacques Hiblot
2018-02-22 15:47 ` Tom Rini
2018-02-23 20:59 ` Simon Glass
2018-02-22 10:25 ` [U-Boot] [PATCH v1 2/4] mmc: omap_hsmmc: compile out write support if not needed Jean-Jacques Hiblot
` (3 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Jean-Jacques Hiblot @ 2018-02-22 10:25 UTC (permalink / raw)
To: u-boot
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)
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v1 1/4] mmc: omap_hsmmc: do not embed struct mmc in struct omap_hsmmc_plat
2018-02-22 10:25 ` [U-Boot] [PATCH v1 1/4] mmc: omap_hsmmc: do not embed struct mmc in struct omap_hsmmc_plat Jean-Jacques Hiblot
@ 2018-02-22 15:47 ` Tom Rini
2018-02-23 20:59 ` Simon Glass
1 sibling, 0 replies; 13+ messages in thread
From: Tom Rini @ 2018-02-22 15:47 UTC (permalink / raw)
To: u-boot
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>
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180222/3b155aa6/attachment.sig>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v1 1/4] mmc: omap_hsmmc: do not embed struct mmc in struct omap_hsmmc_plat
2018-02-22 10:25 ` [U-Boot] [PATCH v1 1/4] mmc: omap_hsmmc: do not embed struct mmc in struct omap_hsmmc_plat Jean-Jacques Hiblot
2018-02-22 15:47 ` Tom Rini
@ 2018-02-23 20:59 ` Simon Glass
2018-02-26 10:24 ` Jean-Jacques Hiblot
1 sibling, 1 reply; 13+ messages in thread
From: Simon Glass @ 2018-02-23 20:59 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v1 1/4] mmc: omap_hsmmc: do not embed struct mmc in struct omap_hsmmc_plat
2018-02-23 20:59 ` Simon Glass
@ 2018-02-26 10:24 ` Jean-Jacques Hiblot
0 siblings, 0 replies; 13+ messages in thread
From: Jean-Jacques Hiblot @ 2018-02-26 10:24 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v1 2/4] mmc: omap_hsmmc: compile out write support if not needed
2018-02-22 10:25 [U-Boot] [PATCH v1 0/4] mmc: omap_hsmmc: Reduce the footprint of the driver and fix am335x clock Jean-Jacques Hiblot
2018-02-22 10:25 ` [U-Boot] [PATCH v1 1/4] mmc: omap_hsmmc: do not embed struct mmc in struct omap_hsmmc_plat Jean-Jacques Hiblot
@ 2018-02-22 10:25 ` Jean-Jacques Hiblot
2018-02-22 15:47 ` Tom Rini
2018-02-22 10:25 ` [U-Boot] [PATCH v1 3/4] mmc: omap_hsmmc: compile out ADMA support for am33xx and omap34xx Jean-Jacques Hiblot
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Jean-Jacques Hiblot @ 2018-02-22 10:25 UTC (permalink / raw)
To: u-boot
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);
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v1 3/4] mmc: omap_hsmmc: compile out ADMA support for am33xx and omap34xx
2018-02-22 10:25 [U-Boot] [PATCH v1 0/4] mmc: omap_hsmmc: Reduce the footprint of the driver and fix am335x clock Jean-Jacques Hiblot
2018-02-22 10:25 ` [U-Boot] [PATCH v1 1/4] mmc: omap_hsmmc: do not embed struct mmc in struct omap_hsmmc_plat Jean-Jacques Hiblot
2018-02-22 10:25 ` [U-Boot] [PATCH v1 2/4] mmc: omap_hsmmc: compile out write support if not needed Jean-Jacques Hiblot
@ 2018-02-22 10:25 ` Jean-Jacques Hiblot
2018-02-22 15:47 ` Tom Rini
2018-02-22 10:25 ` [U-Boot] [PATCH v1 4/4] mmc: omap_hsmmc: use a default 52MHz max clock rate if none is specified Jean-Jacques Hiblot
2018-02-22 23:09 ` [U-Boot] [PATCH v1 0/4] mmc: omap_hsmmc: Reduce the footprint of the driver and fix am335x clock Adam Ford
4 siblings, 1 reply; 13+ messages in thread
From: Jean-Jacques Hiblot @ 2018-02-22 10:25 UTC (permalink / raw)
To: u-boot
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;
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v1 3/4] mmc: omap_hsmmc: compile out ADMA support for am33xx and omap34xx
2018-02-22 10:25 ` [U-Boot] [PATCH v1 3/4] mmc: omap_hsmmc: compile out ADMA support for am33xx and omap34xx Jean-Jacques Hiblot
@ 2018-02-22 15:47 ` Tom Rini
2018-02-23 7:06 ` Jaehoon Chung
0 siblings, 1 reply; 13+ messages in thread
From: Tom Rini @ 2018-02-22 15:47 UTC (permalink / raw)
To: u-boot
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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180222/e9e731b1/attachment.sig>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v1 3/4] mmc: omap_hsmmc: compile out ADMA support for am33xx and omap34xx
2018-02-22 15:47 ` Tom Rini
@ 2018-02-23 7:06 ` Jaehoon Chung
0 siblings, 0 replies; 13+ messages in thread
From: Jaehoon Chung @ 2018-02-23 7:06 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v1 4/4] mmc: omap_hsmmc: use a default 52MHz max clock rate if none is specified
2018-02-22 10:25 [U-Boot] [PATCH v1 0/4] mmc: omap_hsmmc: Reduce the footprint of the driver and fix am335x clock Jean-Jacques Hiblot
` (2 preceding siblings ...)
2018-02-22 10:25 ` [U-Boot] [PATCH v1 3/4] mmc: omap_hsmmc: compile out ADMA support for am33xx and omap34xx Jean-Jacques Hiblot
@ 2018-02-22 10:25 ` Jean-Jacques Hiblot
2018-02-22 15:47 ` Tom Rini
2018-02-22 23:09 ` [U-Boot] [PATCH v1 0/4] mmc: omap_hsmmc: Reduce the footprint of the driver and fix am335x clock Adam Ford
4 siblings, 1 reply; 13+ messages in thread
From: Jean-Jacques Hiblot @ 2018-02-22 10:25 UTC (permalink / raw)
To: u-boot
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;
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v1 0/4] mmc: omap_hsmmc: Reduce the footprint of the driver and fix am335x clock
2018-02-22 10:25 [U-Boot] [PATCH v1 0/4] mmc: omap_hsmmc: Reduce the footprint of the driver and fix am335x clock Jean-Jacques Hiblot
` (3 preceding siblings ...)
2018-02-22 10:25 ` [U-Boot] [PATCH v1 4/4] mmc: omap_hsmmc: use a default 52MHz max clock rate if none is specified Jean-Jacques Hiblot
@ 2018-02-22 23:09 ` Adam Ford
4 siblings, 0 replies; 13+ messages in thread
From: Adam Ford @ 2018-02-22 23:09 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread