public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/5] SPL/MMC size fixes
@ 2013-09-04 15:12 Paul Burton
  2013-09-04 15:12 ` [U-Boot] [PATCH 1/5] spl: remove unnecessary (& ARM specific) include of asm/utils.h Paul Burton
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Paul Burton @ 2013-09-04 15:12 UTC (permalink / raw)
  To: u-boot

This series reduces the size of the SPL when compiled with MMC support,
and fixes build issues if the target does not include libcommon in SPL,
or if the target is not ARM based. Both of these are true of my board
which is currently out of tree, but which I hope to be able to upstream
soon. In the meantime I figured the size optimisations may be of use to
others.

Paul Burton (5):
  spl: remove unnecessary (& ARM specific) include of asm/utils.h
  spl_mmc: only call printf or puts with CONFIG_SPL_LIBCOMMON_SUPPORT
  mmc: don't call *printf or puts when SPL &
    !CONFIG_SPL_LIBCOMMON_SUPPORT
  mmc: size optimization when !CONFIG_MMC_SPI
  mmc: don't support write & erase for SPL builds

 common/spl/spl_mmc.c | 17 ++++++++++++++++-
 drivers/mmc/mmc.c    | 44 ++++++++++++++++++++++++++++++++++++++++++++
 include/mmc.h        |  4 ++++
 3 files changed, 64 insertions(+), 1 deletion(-)

-- 
1.8.3.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 1/5] spl: remove unnecessary (& ARM specific) include of asm/utils.h
  2013-09-04 15:12 [U-Boot] [PATCH 0/5] SPL/MMC size fixes Paul Burton
@ 2013-09-04 15:12 ` Paul Burton
  2013-09-04 15:12 ` [U-Boot] [PATCH 2/5] spl_mmc: only call printf or puts with CONFIG_SPL_LIBCOMMON_SUPPORT Paul Burton
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2013-09-04 15:12 UTC (permalink / raw)
  To: u-boot

ARM is the only architecture which includes this header and nothing in
spl_mmc.c makes use of it. Remove the include.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---
 common/spl/spl_mmc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index f27b4c2..5e7e0fe 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -9,7 +9,6 @@
 #include <common.h>
 #include <spl.h>
 #include <asm/u-boot.h>
-#include <asm/utils.h>
 #include <mmc.h>
 #include <fat.h>
 #include <version.h>
-- 
1.8.3.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/5] spl_mmc: only call printf or puts with CONFIG_SPL_LIBCOMMON_SUPPORT
  2013-09-04 15:12 [U-Boot] [PATCH 0/5] SPL/MMC size fixes Paul Burton
  2013-09-04 15:12 ` [U-Boot] [PATCH 1/5] spl: remove unnecessary (& ARM specific) include of asm/utils.h Paul Burton
@ 2013-09-04 15:12 ` Paul Burton
  2013-09-04 15:12 ` [U-Boot] [PATCH 3/5] mmc: don't call *printf or puts when SPL & !CONFIG_SPL_LIBCOMMON_SUPPORT Paul Burton
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2013-09-04 15:12 UTC (permalink / raw)
  To: u-boot

If we don't have CONFIG_SPL_LIBCOMMON_SUPPORT defined then stdio
functions are unavailable & calling them will cause a link failure.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---
 common/spl/spl_mmc.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index 5e7e0fe..fc2f226 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -44,8 +44,10 @@ static int mmc_load_image_raw(struct mmc *mmc, unsigned long sector)
 					(void *)spl_image.load_addr);
 
 end:
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 	if (err == 0)
 		printf("spl: mmc blk read err - %lu\n", err);
+#endif
 
 	return (err == 0);
 }
@@ -57,7 +59,9 @@ static int mmc_load_image_raw_os(struct mmc *mmc)
 				       CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTOR,
 				       CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS,
 				       (void *)CONFIG_SYS_SPL_ARGS_ADDR)) {
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		printf("mmc args blk read error\n");
+#endif
 		return -1;
 	}
 
@@ -83,9 +87,11 @@ static int mmc_load_image_fat(struct mmc *mmc, const char *filename)
 	err = file_fat_read(filename, (u8 *)spl_image.load_addr, 0);
 
 end:
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 	if (err <= 0)
 		printf("spl: error reading image %s, err - %d\n",
 		       filename, err);
+#endif
 
 	return (err <= 0);
 }
@@ -98,8 +104,10 @@ static int mmc_load_image_fat_os(struct mmc *mmc)
 	err = file_fat_read(CONFIG_SPL_FAT_LOAD_ARGS_NAME,
 			    (void *)CONFIG_SYS_SPL_ARGS_ADDR, 0);
 	if (err <= 0) {
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		printf("spl: error reading image %s, err - %d\n",
 		       CONFIG_SPL_FAT_LOAD_ARGS_NAME, err);
+#endif
 		return -1;
 	}
 
@@ -119,13 +127,17 @@ void spl_mmc_load_image(void)
 	/* We register only one device. So, the dev id is always 0 */
 	mmc = find_mmc_device(0);
 	if (!mmc) {
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		puts("spl: mmc device not found!!\n");
+#endif
 		hang();
 	}
 
 	err = mmc_init(mmc);
 	if (err) {
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		printf("spl: mmc init failed: err - %d\n", err);
+#endif
 		hang();
 	}
 
@@ -144,7 +156,9 @@ void spl_mmc_load_image(void)
 		err = fat_register_device(&mmc->block_dev,
 					  CONFIG_SYS_MMC_SD_FAT_BOOT_PARTITION);
 		if (err) {
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 			printf("spl: fat register err - %d\n", err);
+#endif
 			hang();
 		}
 
@@ -154,7 +168,9 @@ void spl_mmc_load_image(void)
 		err = mmc_load_image_fat(mmc, CONFIG_SPL_FAT_LOAD_PAYLOAD_NAME);
 #endif
 	} else {
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		puts("spl: wrong MMC boot mode\n");
+#endif
 		hang();
 	}
 
-- 
1.8.3.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/5] mmc: don't call *printf or puts when SPL & !CONFIG_SPL_LIBCOMMON_SUPPORT
  2013-09-04 15:12 [U-Boot] [PATCH 0/5] SPL/MMC size fixes Paul Burton
  2013-09-04 15:12 ` [U-Boot] [PATCH 1/5] spl: remove unnecessary (& ARM specific) include of asm/utils.h Paul Burton
  2013-09-04 15:12 ` [U-Boot] [PATCH 2/5] spl_mmc: only call printf or puts with CONFIG_SPL_LIBCOMMON_SUPPORT Paul Burton
@ 2013-09-04 15:12 ` Paul Burton
  2013-09-06 12:48   ` Pantelis Antoniou
  2013-09-04 15:12 ` [U-Boot] [PATCH 4/5] mmc: size optimization when !CONFIG_MMC_SPI Paul Burton
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Paul Burton @ 2013-09-04 15:12 UTC (permalink / raw)
  To: u-boot

If we don't have CONFIG_SPL_LIBCOMMON_SUPPORT defined then stdio
& *printf functions are unavailable & calling them will cause a link
failure.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---
 drivers/mmc/mmc.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 5502675..30a985b 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -135,8 +135,10 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
 			     MMC_STATE_PRG)
 				break;
 			else if (cmd.response[0] & MMC_STATUS_MASK) {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 				printf("Status Error: 0x%08X\n",
 					cmd.response[0]);
+#endif
 				return COMM_ERR;
 			}
 		} else if (--retries < 0)
@@ -151,7 +153,9 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
 	printf("CURR STATE:%d\n", status);
 #endif
 	if (timeout <= 0) {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 		printf("Timeout waiting card ready\n");
+#endif
 		return TIMEOUT;
 	}
 
@@ -181,7 +185,9 @@ struct mmc *find_mmc_device(int dev_num)
 			return m;
 	}
 
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 	printf("MMC Device %d not found\n", dev_num);
+#endif
 
 	return NULL;
 }
@@ -233,7 +239,9 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
 	return 0;
 
 err_out:
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 	puts("mmc erase failed\n");
+#endif
 	return err;
 }
 
@@ -248,6 +256,7 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
 	if (!mmc)
 		return -1;
 
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 	if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
 		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
 		       "The erase range would be change to "
@@ -255,6 +264,7 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
 		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
 		       ((start + blkcnt + mmc->erase_grp_size)
 		       & ~(mmc->erase_grp_size - 1)) - 1);
+#endif
 
 	while (blk < blkcnt) {
 		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
@@ -281,8 +291,10 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
 	int timeout = 1000;
 
 	if ((start + blkcnt) > mmc->block_dev.lba) {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
 			start + blkcnt, mmc->block_dev.lba);
+#endif
 		return 0;
 	}
 
@@ -306,7 +318,9 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
 	data.flags = MMC_DATA_WRITE;
 
 	if (mmc_send_cmd(mmc, &cmd, &data)) {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 		printf("mmc write failed\n");
+#endif
 		return 0;
 	}
 
@@ -318,7 +332,9 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
 		cmd.cmdarg = 0;
 		cmd.resp_type = MMC_RSP_R1b;
 		if (mmc_send_cmd(mmc, &cmd, NULL)) {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 			printf("mmc fail to send stop cmd\n");
+#endif
 			return 0;
 		}
 	}
@@ -385,7 +401,9 @@ static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
 		cmd.cmdarg = 0;
 		cmd.resp_type = MMC_RSP_R1b;
 		if (mmc_send_cmd(mmc, &cmd, NULL)) {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 			printf("mmc fail to send stop cmd\n");
+#endif
 			return 0;
 		}
 	}
@@ -405,8 +423,10 @@ static ulong mmc_bread(int dev_num, lbaint_t start, lbaint_t blkcnt, void *dst)
 		return 0;
 
 	if ((start + blkcnt) > mmc->block_dev.lba) {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
 			start + blkcnt, mmc->block_dev.lba);
+#endif
 		return 0;
 	}
 
@@ -1268,6 +1288,7 @@ static int mmc_startup(struct mmc *mmc)
 	mmc->block_dev.blksz = mmc->read_bl_len;
 	mmc->block_dev.log2blksz = LOG2(mmc->block_dev.blksz);
 	mmc->block_dev.lba = lldiv(mmc->capacity, mmc->read_bl_len);
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 	sprintf(mmc->block_dev.vendor, "Man %06x Snr %04x%04x",
 		mmc->cid[0] >> 24, (mmc->cid[2] & 0xffff),
 		(mmc->cid[3] >> 16) & 0xffff);
@@ -1277,6 +1298,11 @@ static int mmc_startup(struct mmc *mmc)
 		(mmc->cid[2] >> 24) & 0xff);
 	sprintf(mmc->block_dev.revision, "%d.%d", (mmc->cid[2] >> 20) & 0xf,
 		(mmc->cid[2] >> 16) & 0xf);
+#else
+	mmc->block_dev.vendor[0] = 0;
+	mmc->block_dev.product[0] = 0;
+	mmc->block_dev.revision[0] = 0;
+#endif
 #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBDISK_SUPPORT)
 	init_part(&mmc->block_dev);
 #endif
@@ -1343,7 +1369,9 @@ int mmc_start_init(struct mmc *mmc)
 
 	if (mmc_getcd(mmc) == 0) {
 		mmc->has_init = 0;
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 		printf("MMC: no card present\n");
+#endif
 		return NO_CARD_ERR;
 	}
 
@@ -1378,7 +1406,9 @@ int mmc_start_init(struct mmc *mmc)
 		err = mmc_send_op_cond(mmc);
 
 		if (err && err != IN_PROGRESS) {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 			printf("Card did not respond to voltage select!\n");
+#endif
 			return UNUSABLE_ERR;
 		}
 	}
@@ -1434,6 +1464,8 @@ static int __def_mmc_init(bd_t *bis)
 int cpu_mmc_init(bd_t *bis) __attribute__((weak, alias("__def_mmc_init")));
 int board_mmc_init(bd_t *bis) __attribute__((weak, alias("__def_mmc_init")));
 
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
+
 void print_mmc_devices(char separator)
 {
 	struct mmc *m;
@@ -1451,6 +1483,10 @@ void print_mmc_devices(char separator)
 	printf("\n");
 }
 
+#else
+void print_mmc_devices(char separator) { }
+#endif
+
 int get_mmc_num(void)
 {
 	return cur_dev_num;
-- 
1.8.3.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 4/5] mmc: size optimization when !CONFIG_MMC_SPI
  2013-09-04 15:12 [U-Boot] [PATCH 0/5] SPL/MMC size fixes Paul Burton
                   ` (2 preceding siblings ...)
  2013-09-04 15:12 ` [U-Boot] [PATCH 3/5] mmc: don't call *printf or puts when SPL & !CONFIG_SPL_LIBCOMMON_SUPPORT Paul Burton
@ 2013-09-04 15:12 ` Paul Burton
  2013-09-04 15:14 ` [U-Boot] [PATCH 5/5] mmc: don't support write & erase for SPL builds Paul Burton
  2013-09-04 15:15 ` [U-Boot] [PATCH 0/5] SPL/MMC size fixes Paul Burton
  5 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2013-09-04 15:12 UTC (permalink / raw)
  To: u-boot

When CONFIG_MMC_SPI is not enabled, the MMC_MODE_SPI capability can
never be set. However there is code in mmc.c which uses the
mmc_host_is_spi macro to check that capability & act accordingly. If we
expand that macro to 0 when CONFIG_MMC_SPI is not set (since it will
always be 0 at runtime anyway) then the compiler can optimize away the
SPI-specific code paths in mmc.c.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---
 include/mmc.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/mmc.h b/include/mmc.h
index 228d771..214b9ed 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -335,7 +335,11 @@ int mmc_start_init(struct mmc *mmc);
 void mmc_set_preinit(struct mmc *mmc, int preinit);
 
 #ifdef CONFIG_GENERIC_MMC
+#ifdef CONFIG_MMC_SPI
 #define mmc_host_is_spi(mmc)	((mmc)->host_caps & MMC_MODE_SPI)
+#else
+#define mmc_host_is_spi(mmc)	0
+#endif
 struct mmc *mmc_spi_init(uint bus, uint cs, uint speed, uint mode);
 #else
 int mmc_legacy_init(int verbose);
-- 
1.8.3.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 5/5] mmc: don't support write & erase for SPL builds
  2013-09-04 15:12 [U-Boot] [PATCH 0/5] SPL/MMC size fixes Paul Burton
                   ` (3 preceding siblings ...)
  2013-09-04 15:12 ` [U-Boot] [PATCH 4/5] mmc: size optimization when !CONFIG_MMC_SPI Paul Burton
@ 2013-09-04 15:14 ` Paul Burton
  2013-09-06 12:56   ` Pantelis Antoniou
  2013-09-04 15:15 ` [U-Boot] [PATCH 0/5] SPL/MMC size fixes Paul Burton
  5 siblings, 1 reply; 17+ messages in thread
From: Paul Burton @ 2013-09-04 15:14 UTC (permalink / raw)
  To: u-boot

For SPL builds this is just dead code since we'll only need to read.
Eliminating it results in a significant size reduction for the SPL
binary.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---
 drivers/mmc/mmc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 30a985b..d305257 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -248,6 +248,7 @@ err_out:
 static unsigned long
 mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
 {
+#ifndef CONFIG_SPL_BUILD
 	int err = 0;
 	struct mmc *mmc = find_mmc_device(dev_num);
 	lbaint_t blk = 0, blk_r = 0;
@@ -281,6 +282,9 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
 	}
 
 	return blk;
+#else /* CONFIG_SPL_BUILD */
+	return -1;
+#endif
 }
 
 static ulong
@@ -349,6 +353,7 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
 static ulong
 mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void*src)
 {
+#ifndef CONFIG_SPL_BUILD
 	lbaint_t cur, blocks_todo = blkcnt;
 
 	struct mmc *mmc = find_mmc_device(dev_num);
@@ -368,6 +373,9 @@ mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void*src)
 	} while (blocks_todo > 0);
 
 	return blkcnt;
+#else /* CONFIG_SPL_BUILD */
+	return 0;
+#endif
 }
 
 static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
-- 
1.8.3.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 0/5] SPL/MMC size fixes
  2013-09-04 15:12 [U-Boot] [PATCH 0/5] SPL/MMC size fixes Paul Burton
                   ` (4 preceding siblings ...)
  2013-09-04 15:14 ` [U-Boot] [PATCH 5/5] mmc: don't support write & erase for SPL builds Paul Burton
@ 2013-09-04 15:15 ` Paul Burton
  5 siblings, 0 replies; 17+ messages in thread
From: Paul Burton @ 2013-09-04 15:15 UTC (permalink / raw)
  To: u-boot

Bah, the subject should have read "size optimisations and fixes" but you 
get the idea...

Thanks,
     Paul

On 04/09/13 16:12, Paul Burton wrote:
> This series reduces the size of the SPL when compiled with MMC support,
> and fixes build issues if the target does not include libcommon in SPL,
> or if the target is not ARM based. Both of these are true of my board
> which is currently out of tree, but which I hope to be able to upstream
> soon. In the meantime I figured the size optimisations may be of use to
> others.
>
> Paul Burton (5):
>    spl: remove unnecessary (& ARM specific) include of asm/utils.h
>    spl_mmc: only call printf or puts with CONFIG_SPL_LIBCOMMON_SUPPORT
>    mmc: don't call *printf or puts when SPL &
>      !CONFIG_SPL_LIBCOMMON_SUPPORT
>    mmc: size optimization when !CONFIG_MMC_SPI
>    mmc: don't support write & erase for SPL builds
>
>   common/spl/spl_mmc.c | 17 ++++++++++++++++-
>   drivers/mmc/mmc.c    | 44 ++++++++++++++++++++++++++++++++++++++++++++
>   include/mmc.h        |  4 ++++
>   3 files changed, 64 insertions(+), 1 deletion(-)
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/5] mmc: don't call *printf or puts when SPL & !CONFIG_SPL_LIBCOMMON_SUPPORT
  2013-09-04 15:12 ` [U-Boot] [PATCH 3/5] mmc: don't call *printf or puts when SPL & !CONFIG_SPL_LIBCOMMON_SUPPORT Paul Burton
@ 2013-09-06 12:48   ` Pantelis Antoniou
  2013-09-06 12:51     ` Paul Burton
  0 siblings, 1 reply; 17+ messages in thread
From: Pantelis Antoniou @ 2013-09-06 12:48 UTC (permalink / raw)
  To: u-boot

Hi Paul

On Sep 4, 2013, at 6:12 PM, Paul Burton wrote:

> If we don't have CONFIG_SPL_LIBCOMMON_SUPPORT defined then stdio
> & *printf functions are unavailable & calling them will cause a link
> failure.
> 

> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> ---
> drivers/mmc/mmc.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 5502675..30a985b 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -135,8 +135,10 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
> 			     MMC_STATE_PRG)
> 				break;
> 			else if (cmd.response[0] & MMC_STATUS_MASK) {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 				printf("Status Error: 0x%08X\n",
> 					cmd.response[0]);
> +#endif
> 				return COMM_ERR;
> 			}
> 		} else if (--retries < 0)
> @@ -151,7 +153,9 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
> 	printf("CURR STATE:%d\n", status);
> #endif
> 	if (timeout <= 0) {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 		printf("Timeout waiting card ready\n");
> +#endif
> 		return TIMEOUT;
> 	}
> 
> @@ -181,7 +185,9 @@ struct mmc *find_mmc_device(int dev_num)
> 			return m;
> 	}
> 
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 	printf("MMC Device %d not found\n", dev_num);
> +#endif
> 
> 	return NULL;
> }
> @@ -233,7 +239,9 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
> 	return 0;
> 
> err_out:
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 	puts("mmc erase failed\n");
> +#endif
> 	return err;
> }
> 
> @@ -248,6 +256,7 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
> 	if (!mmc)
> 		return -1;
> 
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 	if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
> 		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
> 		       "The erase range would be change to "
> @@ -255,6 +264,7 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
> 		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
> 		       ((start + blkcnt + mmc->erase_grp_size)
> 		       & ~(mmc->erase_grp_size - 1)) - 1);
> +#endif
> 
> 	while (blk < blkcnt) {
> 		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
> @@ -281,8 +291,10 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
> 	int timeout = 1000;
> 
> 	if ((start + blkcnt) > mmc->block_dev.lba) {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
> 			start + blkcnt, mmc->block_dev.lba);
> +#endif
> 		return 0;
> 	}
> 
> @@ -306,7 +318,9 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
> 	data.flags = MMC_DATA_WRITE;
> 
> 	if (mmc_send_cmd(mmc, &cmd, &data)) {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 		printf("mmc write failed\n");
> +#endif
> 		return 0;
> 	}
> 
> @@ -318,7 +332,9 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
> 		cmd.cmdarg = 0;
> 		cmd.resp_type = MMC_RSP_R1b;
> 		if (mmc_send_cmd(mmc, &cmd, NULL)) {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 			printf("mmc fail to send stop cmd\n");
> +#endif
> 			return 0;
> 		}
> 	}
> @@ -385,7 +401,9 @@ static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
> 		cmd.cmdarg = 0;
> 		cmd.resp_type = MMC_RSP_R1b;
> 		if (mmc_send_cmd(mmc, &cmd, NULL)) {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 			printf("mmc fail to send stop cmd\n");
> +#endif
> 			return 0;
> 		}
> 	}
> @@ -405,8 +423,10 @@ static ulong mmc_bread(int dev_num, lbaint_t start, lbaint_t blkcnt, void *dst)
> 		return 0;
> 
> 	if ((start + blkcnt) > mmc->block_dev.lba) {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
> 			start + blkcnt, mmc->block_dev.lba);
> +#endif
> 		return 0;
> 	}

The idea is sound, but I don't like peppering the source with #ifdefs here.

Why not create a varargs orintf macro and use it instead

i.e.


#if !defined(CONFIG_SPL_BUILD_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
#define mmc_printf(...) ...
#else
#define mmc_printf(...) /* nothing */
#endif


> 
> @@ -1268,6 +1288,7 @@ static int mmc_startup(struct mmc *mmc)
> 	mmc->block_dev.blksz = mmc->read_bl_len;
> 	mmc->block_dev.log2blksz = LOG2(mmc->block_dev.blksz);
> 	mmc->block_dev.lba = lldiv(mmc->capacity, mmc->read_bl_len);
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 	sprintf(mmc->block_dev.vendor, "Man %06x Snr %04x%04x",
> 		mmc->cid[0] >> 24, (mmc->cid[2] & 0xffff),
> 		(mmc->cid[3] >> 16) & 0xffff);
> @@ -1277,6 +1298,11 @@ static int mmc_startup(struct mmc *mmc)
> 		(mmc->cid[2] >> 24) & 0xff);
> 	sprintf(mmc->block_dev.revision, "%d.%d", (mmc->cid[2] >> 20) & 0xf,
> 		(mmc->cid[2] >> 16) & 0xf);
> +#else
> +	mmc->block_dev.vendor[0] = 0;
> +	mmc->block_dev.product[0] = 0;
> +	mmc->block_dev.revision[0] = 0;
> +#endif

^^^ 
What goes on here?

> #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBDISK_SUPPORT)
> 	init_part(&mmc->block_dev);
> #endif
> @@ -1343,7 +1369,9 @@ int mmc_start_init(struct mmc *mmc)
> 
> 	if (mmc_getcd(mmc) == 0) {
> 		mmc->has_init = 0;
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 		printf("MMC: no card present\n");
> +#endif
> 		return NO_CARD_ERR;
> 	}
> 
> @@ -1378,7 +1406,9 @@ int mmc_start_init(struct mmc *mmc)
> 		err = mmc_send_op_cond(mmc);
> 
> 		if (err && err != IN_PROGRESS) {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> 			printf("Card did not respond to voltage select!\n");
> +#endif
> 			return UNUSABLE_ERR;
> 		}
> 	}
> @@ -1434,6 +1464,8 @@ static int __def_mmc_init(bd_t *bis)
> int cpu_mmc_init(bd_t *bis) __attribute__((weak, alias("__def_mmc_init")));
> int board_mmc_init(bd_t *bis) __attribute__((weak, alias("__def_mmc_init")));
> 
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> +
> void print_mmc_devices(char separator)
> {
> 	struct mmc *m;
> @@ -1451,6 +1483,10 @@ void print_mmc_devices(char separator)
> 	printf("\n");
> }
> 
> +#else
> +void print_mmc_devices(char separator) { }
> +#endif
> +
> int get_mmc_num(void)
> {
> 	return cur_dev_num;
> -- 
> 1.8.3.4
> 
> 

CCing Tom Rini on this.

Regards

-- Pantelis

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/5] mmc: don't call *printf or puts when SPL & !CONFIG_SPL_LIBCOMMON_SUPPORT
  2013-09-06 12:48   ` Pantelis Antoniou
@ 2013-09-06 12:51     ` Paul Burton
  2013-09-06 12:55       ` Pantelis Antoniou
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Burton @ 2013-09-06 12:51 UTC (permalink / raw)
  To: u-boot


On 06/09/13 13:48, Pantelis Antoniou wrote:
> Hi Paul
>
> On Sep 4, 2013, at 6:12 PM, Paul Burton wrote:
>
>> If we don't have CONFIG_SPL_LIBCOMMON_SUPPORT defined then stdio
>> & *printf functions are unavailable & calling them will cause a link
>> failure.
>>
>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>> ---
>> drivers/mmc/mmc.c | 36 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index 5502675..30a985b 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -135,8 +135,10 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
>> 			     MMC_STATE_PRG)
>> 				break;
>> 			else if (cmd.response[0] & MMC_STATUS_MASK) {
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 				printf("Status Error: 0x%08X\n",
>> 					cmd.response[0]);
>> +#endif
>> 				return COMM_ERR;
>> 			}
>> 		} else if (--retries < 0)
>> @@ -151,7 +153,9 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
>> 	printf("CURR STATE:%d\n", status);
>> #endif
>> 	if (timeout <= 0) {
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 		printf("Timeout waiting card ready\n");
>> +#endif
>> 		return TIMEOUT;
>> 	}
>>
>> @@ -181,7 +185,9 @@ struct mmc *find_mmc_device(int dev_num)
>> 			return m;
>> 	}
>>
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 	printf("MMC Device %d not found\n", dev_num);
>> +#endif
>>
>> 	return NULL;
>> }
>> @@ -233,7 +239,9 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
>> 	return 0;
>>
>> err_out:
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 	puts("mmc erase failed\n");
>> +#endif
>> 	return err;
>> }
>>
>> @@ -248,6 +256,7 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
>> 	if (!mmc)
>> 		return -1;
>>
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 	if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
>> 		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
>> 		       "The erase range would be change to "
>> @@ -255,6 +264,7 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
>> 		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
>> 		       ((start + blkcnt + mmc->erase_grp_size)
>> 		       & ~(mmc->erase_grp_size - 1)) - 1);
>> +#endif
>>
>> 	while (blk < blkcnt) {
>> 		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
>> @@ -281,8 +291,10 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
>> 	int timeout = 1000;
>>
>> 	if ((start + blkcnt) > mmc->block_dev.lba) {
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
>> 			start + blkcnt, mmc->block_dev.lba);
>> +#endif
>> 		return 0;
>> 	}
>>
>> @@ -306,7 +318,9 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
>> 	data.flags = MMC_DATA_WRITE;
>>
>> 	if (mmc_send_cmd(mmc, &cmd, &data)) {
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 		printf("mmc write failed\n");
>> +#endif
>> 		return 0;
>> 	}
>>
>> @@ -318,7 +332,9 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
>> 		cmd.cmdarg = 0;
>> 		cmd.resp_type = MMC_RSP_R1b;
>> 		if (mmc_send_cmd(mmc, &cmd, NULL)) {
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 			printf("mmc fail to send stop cmd\n");
>> +#endif
>> 			return 0;
>> 		}
>> 	}
>> @@ -385,7 +401,9 @@ static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
>> 		cmd.cmdarg = 0;
>> 		cmd.resp_type = MMC_RSP_R1b;
>> 		if (mmc_send_cmd(mmc, &cmd, NULL)) {
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 			printf("mmc fail to send stop cmd\n");
>> +#endif
>> 			return 0;
>> 		}
>> 	}
>> @@ -405,8 +423,10 @@ static ulong mmc_bread(int dev_num, lbaint_t start, lbaint_t blkcnt, void *dst)
>> 		return 0;
>>
>> 	if ((start + blkcnt) > mmc->block_dev.lba) {
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
>> 			start + blkcnt, mmc->block_dev.lba);
>> +#endif
>> 		return 0;
>> 	}
> The idea is sound, but I don't like peppering the source with #ifdefs here.
>
> Why not create a varargs orintf macro and use it instead
>
> i.e.
>
>
> #if !defined(CONFIG_SPL_BUILD_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> #define mmc_printf(...) ...
> #else
> #define mmc_printf(...) /* nothing */
> #endif
That would work too, I'm fine with changing to that if people prefer it.


>> @@ -1268,6 +1288,7 @@ static int mmc_startup(struct mmc *mmc)
>> 	mmc->block_dev.blksz = mmc->read_bl_len;
>> 	mmc->block_dev.log2blksz = LOG2(mmc->block_dev.blksz);
>> 	mmc->block_dev.lba = lldiv(mmc->capacity, mmc->read_bl_len);
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 	sprintf(mmc->block_dev.vendor, "Man %06x Snr %04x%04x",
>> 		mmc->cid[0] >> 24, (mmc->cid[2] & 0xffff),
>> 		(mmc->cid[3] >> 16) & 0xffff);
>> @@ -1277,6 +1298,11 @@ static int mmc_startup(struct mmc *mmc)
>> 		(mmc->cid[2] >> 24) & 0xff);
>> 	sprintf(mmc->block_dev.revision, "%d.%d", (mmc->cid[2] >> 20) & 0xf,
>> 		(mmc->cid[2] >> 16) & 0xf);
>> +#else
>> +	mmc->block_dev.vendor[0] = 0;
>> +	mmc->block_dev.product[0] = 0;
>> +	mmc->block_dev.revision[0] = 0;
>> +#endif
> ^^^
> What goes on here?
Thanks for pointing that out, perhaps I should comment on it in the 
source. Basically it needs to avoid sprintf, and since there's no 
possibility that the information will ever be displayed in this case 
(and it has no other uses than display) I figured it would be fine to 
just have empty strings.

>> #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBDISK_SUPPORT)
>> 	init_part(&mmc->block_dev);
>> #endif
>> @@ -1343,7 +1369,9 @@ int mmc_start_init(struct mmc *mmc)
>>
>> 	if (mmc_getcd(mmc) == 0) {
>> 		mmc->has_init = 0;
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 		printf("MMC: no card present\n");
>> +#endif
>> 		return NO_CARD_ERR;
>> 	}
>>
>> @@ -1378,7 +1406,9 @@ int mmc_start_init(struct mmc *mmc)
>> 		err = mmc_send_op_cond(mmc);
>>
>> 		if (err && err != IN_PROGRESS) {
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> 			printf("Card did not respond to voltage select!\n");
>> +#endif
>> 			return UNUSABLE_ERR;
>> 		}
>> 	}
>> @@ -1434,6 +1464,8 @@ static int __def_mmc_init(bd_t *bis)
>> int cpu_mmc_init(bd_t *bis) __attribute__((weak, alias("__def_mmc_init")));
>> int board_mmc_init(bd_t *bis) __attribute__((weak, alias("__def_mmc_init")));
>>
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> +
>> void print_mmc_devices(char separator)
>> {
>> 	struct mmc *m;
>> @@ -1451,6 +1483,10 @@ void print_mmc_devices(char separator)
>> 	printf("\n");
>> }
>>
>> +#else
>> +void print_mmc_devices(char separator) { }
>> +#endif
>> +
>> int get_mmc_num(void)
>> {
>> 	return cur_dev_num;
>> -- 
>> 1.8.3.4
>>
>>
> CCing Tom Rini on this.
>
> Regards
>
> -- Pantelis
>

Thanks,
     Paul

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/5] mmc: don't call *printf or puts when SPL & !CONFIG_SPL_LIBCOMMON_SUPPORT
  2013-09-06 12:51     ` Paul Burton
@ 2013-09-06 12:55       ` Pantelis Antoniou
  0 siblings, 0 replies; 17+ messages in thread
From: Pantelis Antoniou @ 2013-09-06 12:55 UTC (permalink / raw)
  To: u-boot

Hi Paul

On Sep 6, 2013, at 3:51 PM, Paul Burton wrote:

> 
> On 06/09/13 13:48, Pantelis Antoniou wrote:
>> Hi Paul
>> 
>> On Sep 4, 2013, at 6:12 PM, Paul Burton wrote:
>> 
>>> If we don't have CONFIG_SPL_LIBCOMMON_SUPPORT defined then stdio
>>> & *printf functions are unavailable & calling them will cause a link
>>> failure.
>>> 
>>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>>> ---
>>> drivers/mmc/mmc.c | 36 ++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 36 insertions(+)
>>> 
>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>>> index 5502675..30a985b 100644
>>> --- a/drivers/mmc/mmc.c
>>> +++ b/drivers/mmc/mmc.c
>>> @@ -135,8 +135,10 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
>>> 			     MMC_STATE_PRG)
>>> 				break;
>>> 			else if (cmd.response[0] & MMC_STATUS_MASK) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 				printf("Status Error: 0x%08X\n",
>>> 					cmd.response[0]);
>>> +#endif
>>> 				return COMM_ERR;
>>> 			}
>>> 		} else if (--retries < 0)
>>> @@ -151,7 +153,9 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
>>> 	printf("CURR STATE:%d\n", status);
>>> #endif
>>> 	if (timeout <= 0) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 		printf("Timeout waiting card ready\n");
>>> +#endif
>>> 		return TIMEOUT;
>>> 	}
>>> 
>>> @@ -181,7 +185,9 @@ struct mmc *find_mmc_device(int dev_num)
>>> 			return m;
>>> 	}
>>> 
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 	printf("MMC Device %d not found\n", dev_num);
>>> +#endif
>>> 
>>> 	return NULL;
>>> }
>>> @@ -233,7 +239,9 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
>>> 	return 0;
>>> 
>>> err_out:
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 	puts("mmc erase failed\n");
>>> +#endif
>>> 	return err;
>>> }
>>> 
>>> @@ -248,6 +256,7 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
>>> 	if (!mmc)
>>> 		return -1;
>>> 
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 	if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
>>> 		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
>>> 		       "The erase range would be change to "
>>> @@ -255,6 +264,7 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
>>> 		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
>>> 		       ((start + blkcnt + mmc->erase_grp_size)
>>> 		       & ~(mmc->erase_grp_size - 1)) - 1);
>>> +#endif
>>> 
>>> 	while (blk < blkcnt) {
>>> 		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
>>> @@ -281,8 +291,10 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
>>> 	int timeout = 1000;
>>> 
>>> 	if ((start + blkcnt) > mmc->block_dev.lba) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
>>> 			start + blkcnt, mmc->block_dev.lba);
>>> +#endif
>>> 		return 0;
>>> 	}
>>> 
>>> @@ -306,7 +318,9 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
>>> 	data.flags = MMC_DATA_WRITE;
>>> 
>>> 	if (mmc_send_cmd(mmc, &cmd, &data)) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 		printf("mmc write failed\n");
>>> +#endif
>>> 		return 0;
>>> 	}
>>> 
>>> @@ -318,7 +332,9 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
>>> 		cmd.cmdarg = 0;
>>> 		cmd.resp_type = MMC_RSP_R1b;
>>> 		if (mmc_send_cmd(mmc, &cmd, NULL)) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 			printf("mmc fail to send stop cmd\n");
>>> +#endif
>>> 			return 0;
>>> 		}
>>> 	}
>>> @@ -385,7 +401,9 @@ static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
>>> 		cmd.cmdarg = 0;
>>> 		cmd.resp_type = MMC_RSP_R1b;
>>> 		if (mmc_send_cmd(mmc, &cmd, NULL)) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 			printf("mmc fail to send stop cmd\n");
>>> +#endif
>>> 			return 0;
>>> 		}
>>> 	}
>>> @@ -405,8 +423,10 @@ static ulong mmc_bread(int dev_num, lbaint_t start, lbaint_t blkcnt, void *dst)
>>> 		return 0;
>>> 
>>> 	if ((start + blkcnt) > mmc->block_dev.lba) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
>>> 			start + blkcnt, mmc->block_dev.lba);
>>> +#endif
>>> 		return 0;
>>> 	}
>> The idea is sound, but I don't like peppering the source with #ifdefs here.
>> 
>> Why not create a varargs orintf macro and use it instead
>> 
>> i.e.
>> 
>> 
>> #if !defined(CONFIG_SPL_BUILD_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> #define mmc_printf(...) ...
>> #else
>> #define mmc_printf(...) /* nothing */
>> #endif
> That would work too, I'm fine with changing to that if people prefer it.
> 
> 

Speaking of which this isn't really mmc specific, more like SPL specific; perhaps what needs to
be done is have by default printf defined to empty on SPL and have an foo_printf that always outputs.

Tom, what do you think?

>>> @@ -1268,6 +1288,7 @@ static int mmc_startup(struct mmc *mmc)
>>> 	mmc->block_dev.blksz = mmc->read_bl_len;
>>> 	mmc->block_dev.log2blksz = LOG2(mmc->block_dev.blksz);
>>> 	mmc->block_dev.lba = lldiv(mmc->capacity, mmc->read_bl_len);
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 	sprintf(mmc->block_dev.vendor, "Man %06x Snr %04x%04x",
>>> 		mmc->cid[0] >> 24, (mmc->cid[2] & 0xffff),
>>> 		(mmc->cid[3] >> 16) & 0xffff);
>>> @@ -1277,6 +1298,11 @@ static int mmc_startup(struct mmc *mmc)
>>> 		(mmc->cid[2] >> 24) & 0xff);
>>> 	sprintf(mmc->block_dev.revision, "%d.%d", (mmc->cid[2] >> 20) & 0xf,
>>> 		(mmc->cid[2] >> 16) & 0xf);
>>> +#else
>>> +	mmc->block_dev.vendor[0] = 0;
>>> +	mmc->block_dev.product[0] = 0;
>>> +	mmc->block_dev.revision[0] = 0;
>>> +#endif
>> ^^^
>> What goes on here?
> Thanks for pointing that out, perhaps I should comment on it in the source. Basically it needs to avoid sprintf, and since there's no possibility that the information will ever be displayed in this case (and it has no other uses than display) I figured it would be fine to just have empty strings.
> 

Drop this please then, and put it in another patch if need be. I still don't like it.

>>> #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBDISK_SUPPORT)
>>> 	init_part(&mmc->block_dev);
>>> #endif
>>> @@ -1343,7 +1369,9 @@ int mmc_start_init(struct mmc *mmc)
>>> 
>>> 	if (mmc_getcd(mmc) == 0) {
>>> 		mmc->has_init = 0;
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 		printf("MMC: no card present\n");
>>> +#endif
>>> 		return NO_CARD_ERR;
>>> 	}
>>> 
>>> @@ -1378,7 +1406,9 @@ int mmc_start_init(struct mmc *mmc)
>>> 		err = mmc_send_op_cond(mmc);
>>> 
>>> 		if (err && err != IN_PROGRESS) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> 			printf("Card did not respond to voltage select!\n");
>>> +#endif
>>> 			return UNUSABLE_ERR;
>>> 		}
>>> 	}
>>> @@ -1434,6 +1464,8 @@ static int __def_mmc_init(bd_t *bis)
>>> int cpu_mmc_init(bd_t *bis) __attribute__((weak, alias("__def_mmc_init")));
>>> int board_mmc_init(bd_t *bis) __attribute__((weak, alias("__def_mmc_init")));
>>> 
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> +
>>> void print_mmc_devices(char separator)
>>> {
>>> 	struct mmc *m;
>>> @@ -1451,6 +1483,10 @@ void print_mmc_devices(char separator)
>>> 	printf("\n");
>>> }
>>> 
>>> +#else
>>> +void print_mmc_devices(char separator) { }
>>> +#endif
>>> +
>>> int get_mmc_num(void)
>>> {
>>> 	return cur_dev_num;
>>> -- 
>>> 1.8.3.4
>>> 
>>> 
>> CCing Tom Rini on this.
>> 
>> Regards
>> 
>> -- Pantelis
>> 
> 
> Thanks,
>    Paul
> 

Regards

-- Pantelis

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 5/5] mmc: don't support write & erase for SPL builds
  2013-09-04 15:14 ` [U-Boot] [PATCH 5/5] mmc: don't support write & erase for SPL builds Paul Burton
@ 2013-09-06 12:56   ` Pantelis Antoniou
  2013-09-06 13:43     ` [U-Boot] [PATCH v2] " Paul Burton
  0 siblings, 1 reply; 17+ messages in thread
From: Pantelis Antoniou @ 2013-09-06 12:56 UTC (permalink / raw)
  To: u-boot

Hi Paul,

On Sep 4, 2013, at 6:14 PM, Paul Burton wrote:

> For SPL builds this is just dead code since we'll only need to read.
> Eliminating it results in a significant size reduction for the SPL
> binary.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> ---

I understand what you're trying to do, but I don't like the #ifdef thing.

I'd prefer moving the mmc_berase & mmc_bwrite in a different file that's
only built for non-SPL and then use the a conditional macro in a header
file.

I.e. 

#ifndef CONFIG_SPL_BUILD
extern unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt);
#else
#define mmc_berase(dev_num, start, blkcnt) /* nothing */
#endif

Etc.

Regards

-- Pantelis


> drivers/mmc/mmc.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 30a985b..d305257 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -248,6 +248,7 @@ err_out:
> static unsigned long
> mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
> {
> +#ifndef CONFIG_SPL_BUILD
> 	int err = 0;
> 	struct mmc *mmc = find_mmc_device(dev_num);
> 	lbaint_t blk = 0, blk_r = 0;
> @@ -281,6 +282,9 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
> 	}
> 
> 	return blk;
> +#else /* CONFIG_SPL_BUILD */
> +	return -1;
> +#endif
> }
> 
> static ulong
> @@ -349,6 +353,7 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*sr
> static ulong
> mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void*src)
> {
> +#ifndef CONFIG_SPL_BUILD
> 	lbaint_t cur, blocks_todo = blkcnt;
> 
> 	struct mmc *mmc = find_mmc_device(dev_num);
> @@ -368,6 +373,9 @@ mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void*src)
> 	} while (blocks_todo > 0);
> 
> 	return blkcnt;
> +#else /* CONFIG_SPL_BUILD */
> +	return 0;
> +#endif
> }
> 
> static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
> -- 
> 1.8.3.4
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH v2] mmc: don't support write & erase for SPL builds
  2013-09-06 12:56   ` Pantelis Antoniou
@ 2013-09-06 13:43     ` Paul Burton
  2013-09-06 13:54       ` Pantelis Antoniou
  2013-09-08  8:48       ` Andreas Bießmann
  0 siblings, 2 replies; 17+ messages in thread
From: Paul Burton @ 2013-09-06 13:43 UTC (permalink / raw)
  To: u-boot

For SPL builds this is just dead code since we'll only need to read.
Eliminating it results in a significant size reduction for the SPL
binary, which may be critical for certain platforms where the binary
size is highly constrained.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---
Changes in v2:
  - Move the mmc_bwrite & mmc_berase functions to a new mmc_write.c
    file which is only compiled for non-SPL builds, as per a request
    from Pantelis Antoniou. This requires that a few formerly static
    functions in mmc.c be accessible to the new file, so they are
    declared in a new mmc_private.h header along with the write &
    erase functions. For what it's worth I prefered v1, but hey ho.
---
 drivers/mmc/Makefile      |   2 +
 drivers/mmc/mmc.c         | 186 +--------------------------------------------
 drivers/mmc/mmc_private.h |  45 +++++++++++
 drivers/mmc/mmc_write.c   | 189 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 240 insertions(+), 182 deletions(-)
 create mode 100644 drivers/mmc/mmc_private.h
 create mode 100644 drivers/mmc/mmc_write.c

diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
index bedf833..06280d1 100644
--- a/drivers/mmc/Makefile
+++ b/drivers/mmc/Makefile
@@ -34,6 +34,8 @@ COBJS-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
 COBJS-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
 ifdef CONFIG_SPL_BUILD
 COBJS-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o
+else
+COBJS-$(CONFIG_GENERIC_MMC) += mmc_write.o
 endif
 
 COBJS	:= $(COBJS-y)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 30a985b..666f77b 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -15,6 +15,7 @@
 #include <malloc.h>
 #include <linux/list.h>
 #include <div64.h>
+#include "mmc_private.h"
 
 /* Set block count limit because of 16 bit register limit on some hardware*/
 #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
@@ -52,8 +53,7 @@ int __board_mmc_getcd(struct mmc *mmc) {
 int board_mmc_getcd(struct mmc *mmc)__attribute__((weak,
 	alias("__board_mmc_getcd")));
 
-static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
-			struct mmc_data *data)
+int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 {
 	struct mmc_data backup;
 	int ret;
@@ -114,7 +114,7 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	return ret;
 }
 
-static int mmc_send_status(struct mmc *mmc, int timeout)
+int mmc_send_status(struct mmc *mmc, int timeout)
 {
 	struct mmc_cmd cmd;
 	int err, retries = 5;
@@ -162,7 +162,7 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
 	return 0;
 }
 
-static int mmc_set_blocklen(struct mmc *mmc, int len)
+int mmc_set_blocklen(struct mmc *mmc, int len)
 {
 	struct mmc_cmd cmd;
 
@@ -192,184 +192,6 @@ struct mmc *find_mmc_device(int dev_num)
 	return NULL;
 }
 
-static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
-{
-	struct mmc_cmd cmd;
-	ulong end;
-	int err, start_cmd, end_cmd;
-
-	if (mmc->high_capacity)
-		end = start + blkcnt - 1;
-	else {
-		end = (start + blkcnt - 1) * mmc->write_bl_len;
-		start *= mmc->write_bl_len;
-	}
-
-	if (IS_SD(mmc)) {
-		start_cmd = SD_CMD_ERASE_WR_BLK_START;
-		end_cmd = SD_CMD_ERASE_WR_BLK_END;
-	} else {
-		start_cmd = MMC_CMD_ERASE_GROUP_START;
-		end_cmd = MMC_CMD_ERASE_GROUP_END;
-	}
-
-	cmd.cmdidx = start_cmd;
-	cmd.cmdarg = start;
-	cmd.resp_type = MMC_RSP_R1;
-
-	err = mmc_send_cmd(mmc, &cmd, NULL);
-	if (err)
-		goto err_out;
-
-	cmd.cmdidx = end_cmd;
-	cmd.cmdarg = end;
-
-	err = mmc_send_cmd(mmc, &cmd, NULL);
-	if (err)
-		goto err_out;
-
-	cmd.cmdidx = MMC_CMD_ERASE;
-	cmd.cmdarg = SECURE_ERASE;
-	cmd.resp_type = MMC_RSP_R1b;
-
-	err = mmc_send_cmd(mmc, &cmd, NULL);
-	if (err)
-		goto err_out;
-
-	return 0;
-
-err_out:
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-	puts("mmc erase failed\n");
-#endif
-	return err;
-}
-
-static unsigned long
-mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
-{
-	int err = 0;
-	struct mmc *mmc = find_mmc_device(dev_num);
-	lbaint_t blk = 0, blk_r = 0;
-	int timeout = 1000;
-
-	if (!mmc)
-		return -1;
-
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-	if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
-		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
-		       "The erase range would be change to "
-		       "0x" LBAF "~0x" LBAF "\n\n",
-		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
-		       ((start + blkcnt + mmc->erase_grp_size)
-		       & ~(mmc->erase_grp_size - 1)) - 1);
-#endif
-
-	while (blk < blkcnt) {
-		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
-			mmc->erase_grp_size : (blkcnt - blk);
-		err = mmc_erase_t(mmc, start + blk, blk_r);
-		if (err)
-			break;
-
-		blk += blk_r;
-
-		/* Waiting for the ready status */
-		if (mmc_send_status(mmc, timeout))
-			return 0;
-	}
-
-	return blk;
-}
-
-static ulong
-mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*src)
-{
-	struct mmc_cmd cmd;
-	struct mmc_data data;
-	int timeout = 1000;
-
-	if ((start + blkcnt) > mmc->block_dev.lba) {
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
-			start + blkcnt, mmc->block_dev.lba);
-#endif
-		return 0;
-	}
-
-	if (blkcnt == 0)
-		return 0;
-	else if (blkcnt == 1)
-		cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK;
-	else
-		cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK;
-
-	if (mmc->high_capacity)
-		cmd.cmdarg = start;
-	else
-		cmd.cmdarg = start * mmc->write_bl_len;
-
-	cmd.resp_type = MMC_RSP_R1;
-
-	data.src = src;
-	data.blocks = blkcnt;
-	data.blocksize = mmc->write_bl_len;
-	data.flags = MMC_DATA_WRITE;
-
-	if (mmc_send_cmd(mmc, &cmd, &data)) {
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-		printf("mmc write failed\n");
-#endif
-		return 0;
-	}
-
-	/* SPI multiblock writes terminate using a special
-	 * token, not a STOP_TRANSMISSION request.
-	 */
-	if (!mmc_host_is_spi(mmc) && blkcnt > 1) {
-		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
-		cmd.cmdarg = 0;
-		cmd.resp_type = MMC_RSP_R1b;
-		if (mmc_send_cmd(mmc, &cmd, NULL)) {
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-			printf("mmc fail to send stop cmd\n");
-#endif
-			return 0;
-		}
-	}
-
-	/* Waiting for the ready status */
-	if (mmc_send_status(mmc, timeout))
-		return 0;
-
-	return blkcnt;
-}
-
-static ulong
-mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void*src)
-{
-	lbaint_t cur, blocks_todo = blkcnt;
-
-	struct mmc *mmc = find_mmc_device(dev_num);
-	if (!mmc)
-		return 0;
-
-	if (mmc_set_blocklen(mmc, mmc->write_bl_len))
-		return 0;
-
-	do {
-		cur = (blocks_todo > mmc->b_max) ?  mmc->b_max : blocks_todo;
-		if(mmc_write_blocks(mmc, start, cur, src) != cur)
-			return 0;
-		blocks_todo -= cur;
-		start += cur;
-		src += cur * mmc->write_bl_len;
-	} while (blocks_todo > 0);
-
-	return blkcnt;
-}
-
 static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
 			   lbaint_t blkcnt)
 {
diff --git a/drivers/mmc/mmc_private.h b/drivers/mmc/mmc_private.h
new file mode 100644
index 0000000..16dcf9f
--- /dev/null
+++ b/drivers/mmc/mmc_private.h
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2008,2010 Freescale Semiconductor, Inc
+ * Andy Fleming
+ *
+ * Based (loosely) on the Linux code
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _MMC_PRIVATE_H_
+#define _MMC_PRIVATE_H_
+
+#include <mmc.h>
+
+extern int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
+			struct mmc_data *data);
+extern int mmc_send_status(struct mmc *mmc, int timeout);
+extern int mmc_set_blocklen(struct mmc *mmc, int len);
+
+#ifndef CONFIG_SPL_BUILD
+
+extern unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt);
+
+extern ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt,
+		const void *src);
+
+#else /* CONFIG_SPL_BUILD */
+
+/* SPL will never write or erase, declare dummies to reduce code size. */
+
+static inline unsigned long mmc_berase(int dev_num, lbaint_t start,
+		lbaint_t blkcnt)
+{
+	return 0;
+}
+
+static inline ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt,
+		const void *src)
+{
+	return 0;
+}
+
+#endif /* CONFIG_SPL_BUILD */
+
+#endif /* _MMC_PRIVATE_H_ */
diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
new file mode 100644
index 0000000..dde5cf2
--- /dev/null
+++ b/drivers/mmc/mmc_write.c
@@ -0,0 +1,189 @@
+/*
+ * Copyright 2008, Freescale Semiconductor, Inc
+ * Andy Fleming
+ *
+ * Based vaguely on the Linux code
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <config.h>
+#include <common.h>
+#include <part.h>
+#include "mmc_private.h"
+
+static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
+{
+	struct mmc_cmd cmd;
+	ulong end;
+	int err, start_cmd, end_cmd;
+
+	if (mmc->high_capacity) {
+		end = start + blkcnt - 1;
+	} else {
+		end = (start + blkcnt - 1) * mmc->write_bl_len;
+		start *= mmc->write_bl_len;
+	}
+
+	if (IS_SD(mmc)) {
+		start_cmd = SD_CMD_ERASE_WR_BLK_START;
+		end_cmd = SD_CMD_ERASE_WR_BLK_END;
+	} else {
+		start_cmd = MMC_CMD_ERASE_GROUP_START;
+		end_cmd = MMC_CMD_ERASE_GROUP_END;
+	}
+
+	cmd.cmdidx = start_cmd;
+	cmd.cmdarg = start;
+	cmd.resp_type = MMC_RSP_R1;
+
+	err = mmc_send_cmd(mmc, &cmd, NULL);
+	if (err)
+		goto err_out;
+
+	cmd.cmdidx = end_cmd;
+	cmd.cmdarg = end;
+
+	err = mmc_send_cmd(mmc, &cmd, NULL);
+	if (err)
+		goto err_out;
+
+	cmd.cmdidx = MMC_CMD_ERASE;
+	cmd.cmdarg = SECURE_ERASE;
+	cmd.resp_type = MMC_RSP_R1b;
+
+	err = mmc_send_cmd(mmc, &cmd, NULL);
+	if (err)
+		goto err_out;
+
+	return 0;
+
+err_out:
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
+	puts("mmc erase failed\n");
+#endif
+	return err;
+}
+
+unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
+{
+	int err = 0;
+	struct mmc *mmc = find_mmc_device(dev_num);
+	lbaint_t blk = 0, blk_r = 0;
+	int timeout = 1000;
+
+	if (!mmc)
+		return -1;
+
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
+	if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
+		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
+		       "The erase range would be change to "
+		       "0x" LBAF "~0x" LBAF "\n\n",
+		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
+		       ((start + blkcnt + mmc->erase_grp_size)
+		       & ~(mmc->erase_grp_size - 1)) - 1);
+#endif
+
+	while (blk < blkcnt) {
+		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
+			mmc->erase_grp_size : (blkcnt - blk);
+		err = mmc_erase_t(mmc, start + blk, blk_r);
+		if (err)
+			break;
+
+		blk += blk_r;
+
+		/* Waiting for the ready status */
+		if (mmc_send_status(mmc, timeout))
+			return 0;
+	}
+
+	return blk;
+}
+
+static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
+		lbaint_t blkcnt, const void *src)
+{
+	struct mmc_cmd cmd;
+	struct mmc_data data;
+	int timeout = 1000;
+
+	if ((start + blkcnt) > mmc->block_dev.lba) {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
+		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
+		       start + blkcnt, mmc->block_dev.lba);
+#endif
+		return 0;
+	}
+
+	if (blkcnt == 0)
+		return 0;
+	else if (blkcnt == 1)
+		cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK;
+	else
+		cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK;
+
+	if (mmc->high_capacity)
+		cmd.cmdarg = start;
+	else
+		cmd.cmdarg = start * mmc->write_bl_len;
+
+	cmd.resp_type = MMC_RSP_R1;
+
+	data.src = src;
+	data.blocks = blkcnt;
+	data.blocksize = mmc->write_bl_len;
+	data.flags = MMC_DATA_WRITE;
+
+	if (mmc_send_cmd(mmc, &cmd, &data)) {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
+		printf("mmc write failed\n");
+#endif
+		return 0;
+	}
+
+	/* SPI multiblock writes terminate using a special
+	 * token, not a STOP_TRANSMISSION request.
+	 */
+	if (!mmc_host_is_spi(mmc) && blkcnt > 1) {
+		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
+		cmd.cmdarg = 0;
+		cmd.resp_type = MMC_RSP_R1b;
+		if (mmc_send_cmd(mmc, &cmd, NULL)) {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
+			printf("mmc fail to send stop cmd\n");
+#endif
+			return 0;
+		}
+	}
+
+	/* Waiting for the ready status */
+	if (mmc_send_status(mmc, timeout))
+		return 0;
+
+	return blkcnt;
+}
+
+ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void *src)
+{
+	lbaint_t cur, blocks_todo = blkcnt;
+
+	struct mmc *mmc = find_mmc_device(dev_num);
+	if (!mmc)
+		return 0;
+
+	if (mmc_set_blocklen(mmc, mmc->write_bl_len))
+		return 0;
+
+	do {
+		cur = (blocks_todo > mmc->b_max) ?  mmc->b_max : blocks_todo;
+		if (mmc_write_blocks(mmc, start, cur, src) != cur)
+			return 0;
+		blocks_todo -= cur;
+		start += cur;
+		src += cur * mmc->write_bl_len;
+	} while (blocks_todo > 0);
+
+	return blkcnt;
+}
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH v2] mmc: don't support write & erase for SPL builds
  2013-09-06 13:43     ` [U-Boot] [PATCH v2] " Paul Burton
@ 2013-09-06 13:54       ` Pantelis Antoniou
  2013-09-08  8:48       ` Andreas Bießmann
  1 sibling, 0 replies; 17+ messages in thread
From: Pantelis Antoniou @ 2013-09-06 13:54 UTC (permalink / raw)
  To: u-boot

Hi Paul,

On Sep 6, 2013, at 4:43 PM, Paul Burton wrote:

> For SPL builds this is just dead code since we'll only need to read.
> Eliminating it results in a significant size reduction for the SPL
> binary, which may be critical for certain platforms where the binary
> size is highly constrained.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> ---
> Changes in v2:
>  - Move the mmc_bwrite & mmc_berase functions to a new mmc_write.c
>    file which is only compiled for non-SPL builds, as per a request
>    from Pantelis Antoniou. This requires that a few formerly static
>    functions in mmc.c be accessible to the new file, so they are
>    declared in a new mmc_private.h header along with the write &
>    erase functions. For what it's worth I prefered v1, but hey ho.
> ---
> drivers/mmc/Makefile      |   2 +
> drivers/mmc/mmc.c         | 186 +--------------------------------------------
> drivers/mmc/mmc_private.h |  45 +++++++++++
> drivers/mmc/mmc_write.c   | 189 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 240 insertions(+), 182 deletions(-)
> create mode 100644 drivers/mmc/mmc_private.h
> create mode 100644 drivers/mmc/mmc_write.c
> 
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index bedf833..06280d1 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -34,6 +34,8 @@ COBJS-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
> COBJS-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
> ifdef CONFIG_SPL_BUILD
> COBJS-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o
> +else
> +COBJS-$(CONFIG_GENERIC_MMC) += mmc_write.o
> endif
> 
> COBJS	:= $(COBJS-y)
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 30a985b..666f77b 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -15,6 +15,7 @@
> #include <malloc.h>
> #include <linux/list.h>
> #include <div64.h>
> +#include "mmc_private.h"
> 
> /* Set block count limit because of 16 bit register limit on some hardware*/
> #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
> @@ -52,8 +53,7 @@ int __board_mmc_getcd(struct mmc *mmc) {
> int board_mmc_getcd(struct mmc *mmc)__attribute__((weak,
> 	alias("__board_mmc_getcd")));
> 
> -static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> -			struct mmc_data *data)
> +int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
> {
> 	struct mmc_data backup;
> 	int ret;
> @@ -114,7 +114,7 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> 	return ret;
> }
> 
> -static int mmc_send_status(struct mmc *mmc, int timeout)
> +int mmc_send_status(struct mmc *mmc, int timeout)
> {
> 	struct mmc_cmd cmd;
> 	int err, retries = 5;
> @@ -162,7 +162,7 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
> 	return 0;
> }
> 
> -static int mmc_set_blocklen(struct mmc *mmc, int len)
> +int mmc_set_blocklen(struct mmc *mmc, int len)
> {
> 	struct mmc_cmd cmd;
> 
> @@ -192,184 +192,6 @@ struct mmc *find_mmc_device(int dev_num)
> 	return NULL;
> }
> 
> -static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
> -{
> -	struct mmc_cmd cmd;
> -	ulong end;
> -	int err, start_cmd, end_cmd;
> -
> -	if (mmc->high_capacity)
> -		end = start + blkcnt - 1;
> -	else {
> -		end = (start + blkcnt - 1) * mmc->write_bl_len;
> -		start *= mmc->write_bl_len;
> -	}
> -
> -	if (IS_SD(mmc)) {
> -		start_cmd = SD_CMD_ERASE_WR_BLK_START;
> -		end_cmd = SD_CMD_ERASE_WR_BLK_END;
> -	} else {
> -		start_cmd = MMC_CMD_ERASE_GROUP_START;
> -		end_cmd = MMC_CMD_ERASE_GROUP_END;
> -	}
> -
> -	cmd.cmdidx = start_cmd;
> -	cmd.cmdarg = start;
> -	cmd.resp_type = MMC_RSP_R1;
> -
> -	err = mmc_send_cmd(mmc, &cmd, NULL);
> -	if (err)
> -		goto err_out;
> -
> -	cmd.cmdidx = end_cmd;
> -	cmd.cmdarg = end;
> -
> -	err = mmc_send_cmd(mmc, &cmd, NULL);
> -	if (err)
> -		goto err_out;
> -
> -	cmd.cmdidx = MMC_CMD_ERASE;
> -	cmd.cmdarg = SECURE_ERASE;
> -	cmd.resp_type = MMC_RSP_R1b;
> -
> -	err = mmc_send_cmd(mmc, &cmd, NULL);
> -	if (err)
> -		goto err_out;
> -
> -	return 0;
> -
> -err_out:
> -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> -	puts("mmc erase failed\n");
> -#endif
> -	return err;
> -}
> -
> -static unsigned long
> -mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
> -{
> -	int err = 0;
> -	struct mmc *mmc = find_mmc_device(dev_num);
> -	lbaint_t blk = 0, blk_r = 0;
> -	int timeout = 1000;
> -
> -	if (!mmc)
> -		return -1;
> -
> -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> -	if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
> -		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
> -		       "The erase range would be change to "
> -		       "0x" LBAF "~0x" LBAF "\n\n",
> -		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
> -		       ((start + blkcnt + mmc->erase_grp_size)
> -		       & ~(mmc->erase_grp_size - 1)) - 1);
> -#endif
> -
> -	while (blk < blkcnt) {
> -		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
> -			mmc->erase_grp_size : (blkcnt - blk);
> -		err = mmc_erase_t(mmc, start + blk, blk_r);
> -		if (err)
> -			break;
> -
> -		blk += blk_r;
> -
> -		/* Waiting for the ready status */
> -		if (mmc_send_status(mmc, timeout))
> -			return 0;
> -	}
> -
> -	return blk;
> -}
> -
> -static ulong
> -mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*src)
> -{
> -	struct mmc_cmd cmd;
> -	struct mmc_data data;
> -	int timeout = 1000;
> -
> -	if ((start + blkcnt) > mmc->block_dev.lba) {
> -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> -		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
> -			start + blkcnt, mmc->block_dev.lba);
> -#endif
> -		return 0;
> -	}
> -
> -	if (blkcnt == 0)
> -		return 0;
> -	else if (blkcnt == 1)
> -		cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK;
> -	else
> -		cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK;
> -
> -	if (mmc->high_capacity)
> -		cmd.cmdarg = start;
> -	else
> -		cmd.cmdarg = start * mmc->write_bl_len;
> -
> -	cmd.resp_type = MMC_RSP_R1;
> -
> -	data.src = src;
> -	data.blocks = blkcnt;
> -	data.blocksize = mmc->write_bl_len;
> -	data.flags = MMC_DATA_WRITE;
> -
> -	if (mmc_send_cmd(mmc, &cmd, &data)) {
> -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> -		printf("mmc write failed\n");
> -#endif
> -		return 0;
> -	}
> -
> -	/* SPI multiblock writes terminate using a special
> -	 * token, not a STOP_TRANSMISSION request.
> -	 */
> -	if (!mmc_host_is_spi(mmc) && blkcnt > 1) {
> -		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
> -		cmd.cmdarg = 0;
> -		cmd.resp_type = MMC_RSP_R1b;
> -		if (mmc_send_cmd(mmc, &cmd, NULL)) {
> -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> -			printf("mmc fail to send stop cmd\n");
> -#endif
> -			return 0;
> -		}
> -	}
> -
> -	/* Waiting for the ready status */
> -	if (mmc_send_status(mmc, timeout))
> -		return 0;
> -
> -	return blkcnt;
> -}
> -
> -static ulong
> -mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void*src)
> -{
> -	lbaint_t cur, blocks_todo = blkcnt;
> -
> -	struct mmc *mmc = find_mmc_device(dev_num);
> -	if (!mmc)
> -		return 0;
> -
> -	if (mmc_set_blocklen(mmc, mmc->write_bl_len))
> -		return 0;
> -
> -	do {
> -		cur = (blocks_todo > mmc->b_max) ?  mmc->b_max : blocks_todo;
> -		if(mmc_write_blocks(mmc, start, cur, src) != cur)
> -			return 0;
> -		blocks_todo -= cur;
> -		start += cur;
> -		src += cur * mmc->write_bl_len;
> -	} while (blocks_todo > 0);
> -
> -	return blkcnt;
> -}
> -
> static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
> 			   lbaint_t blkcnt)
> {
> diff --git a/drivers/mmc/mmc_private.h b/drivers/mmc/mmc_private.h
> new file mode 100644
> index 0000000..16dcf9f
> --- /dev/null
> +++ b/drivers/mmc/mmc_private.h
> @@ -0,0 +1,45 @@
> +/*
> + * Copyright 2008,2010 Freescale Semiconductor, Inc
> + * Andy Fleming
> + *
> + * Based (loosely) on the Linux code
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef _MMC_PRIVATE_H_
> +#define _MMC_PRIVATE_H_
> +
> +#include <mmc.h>
> +
> +extern int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> +			struct mmc_data *data);
> +extern int mmc_send_status(struct mmc *mmc, int timeout);
> +extern int mmc_set_blocklen(struct mmc *mmc, int len);
> +
> +#ifndef CONFIG_SPL_BUILD
> +
> +extern unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt);
> +
> +extern ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt,
> +		const void *src);
> +
> +#else /* CONFIG_SPL_BUILD */
> +
> +/* SPL will never write or erase, declare dummies to reduce code size. */
> +
> +static inline unsigned long mmc_berase(int dev_num, lbaint_t start,
> +		lbaint_t blkcnt)
> +{
> +	return 0;
> +}
> +
> +static inline ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt,
> +		const void *src)
> +{
> +	return 0;
> +}
> +
> +#endif /* CONFIG_SPL_BUILD */
> +
> +#endif /* _MMC_PRIVATE_H_ */
> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
> new file mode 100644
> index 0000000..dde5cf2
> --- /dev/null
> +++ b/drivers/mmc/mmc_write.c
> @@ -0,0 +1,189 @@
> +/*
> + * Copyright 2008, Freescale Semiconductor, Inc
> + * Andy Fleming
> + *
> + * Based vaguely on the Linux code
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <part.h>
> +#include "mmc_private.h"
> +
> +static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
> +{
> +	struct mmc_cmd cmd;
> +	ulong end;
> +	int err, start_cmd, end_cmd;
> +
> +	if (mmc->high_capacity) {
> +		end = start + blkcnt - 1;
> +	} else {
> +		end = (start + blkcnt - 1) * mmc->write_bl_len;
> +		start *= mmc->write_bl_len;
> +	}
> +
> +	if (IS_SD(mmc)) {
> +		start_cmd = SD_CMD_ERASE_WR_BLK_START;
> +		end_cmd = SD_CMD_ERASE_WR_BLK_END;
> +	} else {
> +		start_cmd = MMC_CMD_ERASE_GROUP_START;
> +		end_cmd = MMC_CMD_ERASE_GROUP_END;
> +	}
> +
> +	cmd.cmdidx = start_cmd;
> +	cmd.cmdarg = start;
> +	cmd.resp_type = MMC_RSP_R1;
> +
> +	err = mmc_send_cmd(mmc, &cmd, NULL);
> +	if (err)
> +		goto err_out;
> +
> +	cmd.cmdidx = end_cmd;
> +	cmd.cmdarg = end;
> +
> +	err = mmc_send_cmd(mmc, &cmd, NULL);
> +	if (err)
> +		goto err_out;
> +
> +	cmd.cmdidx = MMC_CMD_ERASE;
> +	cmd.cmdarg = SECURE_ERASE;
> +	cmd.resp_type = MMC_RSP_R1b;
> +
> +	err = mmc_send_cmd(mmc, &cmd, NULL);
> +	if (err)
> +		goto err_out;
> +
> +	return 0;
> +
> +err_out:
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> +	puts("mmc erase failed\n");
> +#endif
> +	return err;
> +}
> +
> +unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
> +{
> +	int err = 0;
> +	struct mmc *mmc = find_mmc_device(dev_num);
> +	lbaint_t blk = 0, blk_r = 0;
> +	int timeout = 1000;
> +
> +	if (!mmc)
> +		return -1;
> +
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> +	if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
> +		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
> +		       "The erase range would be change to "
> +		       "0x" LBAF "~0x" LBAF "\n\n",
> +		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
> +		       ((start + blkcnt + mmc->erase_grp_size)
> +		       & ~(mmc->erase_grp_size - 1)) - 1);
> +#endif
> +
> +	while (blk < blkcnt) {
> +		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
> +			mmc->erase_grp_size : (blkcnt - blk);
> +		err = mmc_erase_t(mmc, start + blk, blk_r);
> +		if (err)
> +			break;
> +
> +		blk += blk_r;
> +
> +		/* Waiting for the ready status */
> +		if (mmc_send_status(mmc, timeout))
> +			return 0;
> +	}
> +
> +	return blk;
> +}
> +
> +static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
> +		lbaint_t blkcnt, const void *src)
> +{
> +	struct mmc_cmd cmd;
> +	struct mmc_data data;
> +	int timeout = 1000;
> +
> +	if ((start + blkcnt) > mmc->block_dev.lba) {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> +		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
> +		       start + blkcnt, mmc->block_dev.lba);
> +#endif
> +		return 0;
> +	}
> +
> +	if (blkcnt == 0)
> +		return 0;
> +	else if (blkcnt == 1)
> +		cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK;
> +	else
> +		cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK;
> +
> +	if (mmc->high_capacity)
> +		cmd.cmdarg = start;
> +	else
> +		cmd.cmdarg = start * mmc->write_bl_len;
> +
> +	cmd.resp_type = MMC_RSP_R1;
> +
> +	data.src = src;
> +	data.blocks = blkcnt;
> +	data.blocksize = mmc->write_bl_len;
> +	data.flags = MMC_DATA_WRITE;
> +
> +	if (mmc_send_cmd(mmc, &cmd, &data)) {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> +		printf("mmc write failed\n");
> +#endif
> +		return 0;
> +	}
> +
> +	/* SPI multiblock writes terminate using a special
> +	 * token, not a STOP_TRANSMISSION request.
> +	 */
> +	if (!mmc_host_is_spi(mmc) && blkcnt > 1) {
> +		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
> +		cmd.cmdarg = 0;
> +		cmd.resp_type = MMC_RSP_R1b;
> +		if (mmc_send_cmd(mmc, &cmd, NULL)) {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> +			printf("mmc fail to send stop cmd\n");
> +#endif
> +			return 0;
> +		}
> +	}
> +
> +	/* Waiting for the ready status */
> +	if (mmc_send_status(mmc, timeout))
> +		return 0;
> +
> +	return blkcnt;
> +}
> +
> +ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void *src)
> +{
> +	lbaint_t cur, blocks_todo = blkcnt;
> +
> +	struct mmc *mmc = find_mmc_device(dev_num);
> +	if (!mmc)
> +		return 0;
> +
> +	if (mmc_set_blocklen(mmc, mmc->write_bl_len))
> +		return 0;
> +
> +	do {
> +		cur = (blocks_todo > mmc->b_max) ?  mmc->b_max : blocks_todo;
> +		if (mmc_write_blocks(mmc, start, cur, src) != cur)
> +			return 0;
> +		blocks_todo -= cur;
> +		start += cur;
> +		src += cur * mmc->write_bl_len;
> +	} while (blocks_todo > 0);
> +
> +	return blkcnt;
> +}
> -- 
> 1.8.4
> 
> 

Acked-by: Pantelis Antoniou <panto@antoniou-consulting.com>

Looks good to me; let's hear what other people say too.

Tom?

Regards

-- Pantelis

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH v2] mmc: don't support write & erase for SPL builds
  2013-09-06 13:43     ` [U-Boot] [PATCH v2] " Paul Burton
  2013-09-06 13:54       ` Pantelis Antoniou
@ 2013-09-08  8:48       ` Andreas Bießmann
  2013-09-09  8:14         ` Paul Burton
  1 sibling, 1 reply; 17+ messages in thread
From: Andreas Bießmann @ 2013-09-08  8:48 UTC (permalink / raw)
  To: u-boot

Dear Paul Burton,

On 06.09.13 15:43, Paul Burton wrote:
> For SPL builds this is just dead code since we'll only need to read.
> Eliminating it results in a significant size reduction for the SPL
> binary, which may be critical for certain platforms where the binary
> size is highly constrained.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> ---
> Changes in v2:
>   - Move the mmc_bwrite & mmc_berase functions to a new mmc_write.c
>     file which is only compiled for non-SPL builds, as per a request
>     from Pantelis Antoniou. This requires that a few formerly static
>     functions in mmc.c be accessible to the new file, so they are
>     declared in a new mmc_private.h header along with the write &
>     erase functions. For what it's worth I prefered v1, but hey ho.
> ---
>  drivers/mmc/Makefile      |   2 +
>  drivers/mmc/mmc.c         | 186 +--------------------------------------------
>  drivers/mmc/mmc_private.h |  45 +++++++++++
>  drivers/mmc/mmc_write.c   | 189 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 240 insertions(+), 182 deletions(-)
>  create mode 100644 drivers/mmc/mmc_private.h
>  create mode 100644 drivers/mmc/mmc_write.c

<snip>

> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
> new file mode 100644
> index 0000000..dde5cf2
> --- /dev/null
> +++ b/drivers/mmc/mmc_write.c
> @@ -0,0 +1,189 @@
> +/*
> + * Copyright 2008, Freescale Semiconductor, Inc
> + * Andy Fleming
> + *
> + * Based vaguely on the Linux code
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <part.h>
> +#include "mmc_private.h"
> +
> +static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
> +{
> +	struct mmc_cmd cmd;
> +	ulong end;
> +	int err, start_cmd, end_cmd;
> +
> +	if (mmc->high_capacity) {
> +		end = start + blkcnt - 1;
> +	} else {
> +		end = (start + blkcnt - 1) * mmc->write_bl_len;
> +		start *= mmc->write_bl_len;
> +	}
> +
> +	if (IS_SD(mmc)) {
> +		start_cmd = SD_CMD_ERASE_WR_BLK_START;
> +		end_cmd = SD_CMD_ERASE_WR_BLK_END;
> +	} else {
> +		start_cmd = MMC_CMD_ERASE_GROUP_START;
> +		end_cmd = MMC_CMD_ERASE_GROUP_END;
> +	}
> +
> +	cmd.cmdidx = start_cmd;
> +	cmd.cmdarg = start;
> +	cmd.resp_type = MMC_RSP_R1;
> +
> +	err = mmc_send_cmd(mmc, &cmd, NULL);
> +	if (err)
> +		goto err_out;
> +
> +	cmd.cmdidx = end_cmd;
> +	cmd.cmdarg = end;
> +
> +	err = mmc_send_cmd(mmc, &cmd, NULL);
> +	if (err)
> +		goto err_out;
> +
> +	cmd.cmdidx = MMC_CMD_ERASE;
> +	cmd.cmdarg = SECURE_ERASE;
> +	cmd.resp_type = MMC_RSP_R1b;
> +
> +	err = mmc_send_cmd(mmc, &cmd, NULL);
> +	if (err)
> +		goto err_out;
> +
> +	return 0;
> +
> +err_out:
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> +	puts("mmc erase failed\n");
> +#endif

this conditional compile in of puts/printf for SPL are no longer
required, I'd prefere to remove them globally in mmc_write.c.

> +	return err;
> +}

Rest of this patch looks good to me.

Best regards

Andreas Bie?mann

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH v2] mmc: don't support write & erase for SPL builds
  2013-09-08  8:48       ` Andreas Bießmann
@ 2013-09-09  8:14         ` Paul Burton
  2013-09-09  8:27           ` Pantelis Antoniou
  2013-09-09 14:30           ` [U-Boot] [PATCH v3] " Paul Burton
  0 siblings, 2 replies; 17+ messages in thread
From: Paul Burton @ 2013-09-09  8:14 UTC (permalink / raw)
  To: u-boot

On Sun 08 Sep 2013 09:48:20 BST, Andreas Bie?mann wrote:
>
> Dear Paul Burton,
>
> On 06.09.13 15:43, Paul Burton wrote:
>>
>> For SPL builds this is just dead code since we'll only need to read.
>> Eliminating it results in a significant size reduction for the SPL
>> binary, which may be critical for certain platforms where the binary
>> size is highly constrained.
>>
>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>> ---
>> Changes in v2:
>> - Move the mmc_bwrite & mmc_berase functions to a new mmc_write.c
>> file which is only compiled for non-SPL builds, as per a request
>> from Pantelis Antoniou. This requires that a few formerly static
>> functions in mmc.c be accessible to the new file, so they are
>> declared in a new mmc_private.h header along with the write &
>> erase functions. For what it's worth I prefered v1, but hey ho.
>> ---
>> drivers/mmc/Makefile | 2 +
>> drivers/mmc/mmc.c | 186 +--------------------------------------------
>> drivers/mmc/mmc_private.h | 45 +++++++++++
>> drivers/mmc/mmc_write.c | 189 
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 240 insertions(+), 182 deletions(-)
>> create mode 100644 drivers/mmc/mmc_private.h
>> create mode 100644 drivers/mmc/mmc_write.c
>
>
> <snip>
>
>>
>> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
>> new file mode 100644
>> index 0000000..dde5cf2
>> --- /dev/null
>> +++ b/drivers/mmc/mmc_write.c
>> @@ -0,0 +1,189 @@
>> +/*
>> + * Copyright 2008, Freescale Semiconductor, Inc
>> + * Andy Fleming
>> + *
>> + * Based vaguely on the Linux code
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <config.h>
>> +#include <common.h>
>> +#include <part.h>
>> +#include "mmc_private.h"
>> +
>> +static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
>> +{
>> + struct mmc_cmd cmd;
>> + ulong end;
>> + int err, start_cmd, end_cmd;
>> +
>> + if (mmc->high_capacity) {
>> + end = start + blkcnt - 1;
>> + } else {
>> + end = (start + blkcnt - 1) * mmc->write_bl_len;
>> + start *= mmc->write_bl_len;
>> + }
>> +
>> + if (IS_SD(mmc)) {
>> + start_cmd = SD_CMD_ERASE_WR_BLK_START;
>> + end_cmd = SD_CMD_ERASE_WR_BLK_END;
>> + } else {
>> + start_cmd = MMC_CMD_ERASE_GROUP_START;
>> + end_cmd = MMC_CMD_ERASE_GROUP_END;
>> + }
>> +
>> + cmd.cmdidx = start_cmd;
>> + cmd.cmdarg = start;
>> + cmd.resp_type = MMC_RSP_R1;
>> +
>> + err = mmc_send_cmd(mmc, &cmd, NULL);
>> + if (err)
>> + goto err_out;
>> +
>> + cmd.cmdidx = end_cmd;
>> + cmd.cmdarg = end;
>> +
>> + err = mmc_send_cmd(mmc, &cmd, NULL);
>> + if (err)
>> + goto err_out;
>> +
>> + cmd.cmdidx = MMC_CMD_ERASE;
>> + cmd.cmdarg = SECURE_ERASE;
>> + cmd.resp_type = MMC_RSP_R1b;
>> +
>> + err = mmc_send_cmd(mmc, &cmd, NULL);
>> + if (err)
>> + goto err_out;
>> +
>> + return 0;
>> +
>> +err_out:
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> + puts("mmc erase failed\n");
>> +#endif
>
>
> this conditional compile in of puts/printf for SPL are no longer
> required, I'd prefere to remove them globally in mmc_write.c.
Ah, yes good point, I'll remove that.
>
>>
>> + return err;
>> +}
>
>
> Rest of this patch looks good to me.
>
> Best regards
>
> Andreas Bie?mann
Thanks for looking at it!

Paul

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH v2] mmc: don't support write & erase for SPL builds
  2013-09-09  8:14         ` Paul Burton
@ 2013-09-09  8:27           ` Pantelis Antoniou
  2013-09-09 14:30           ` [U-Boot] [PATCH v3] " Paul Burton
  1 sibling, 0 replies; 17+ messages in thread
From: Pantelis Antoniou @ 2013-09-09  8:27 UTC (permalink / raw)
  To: u-boot

Hi Paul,

On Sep 9, 2013, at 11:14 AM, Paul Burton wrote:

> On Sun 08 Sep 2013 09:48:20 BST, Andreas Bie?mann wrote:
>> 
>> Dear Paul Burton,
>> 
>> On 06.09.13 15:43, Paul Burton wrote:
>>> 
>>> For SPL builds this is just dead code since we'll only need to read.
>>> Eliminating it results in a significant size reduction for the SPL
>>> binary, which may be critical for certain platforms where the binary
>>> size is highly constrained.
>>> 
>>> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>>> ---
>>> Changes in v2:
>>> - Move the mmc_bwrite & mmc_berase functions to a new mmc_write.c
>>> file which is only compiled for non-SPL builds, as per a request
>>> from Pantelis Antoniou. This requires that a few formerly static
>>> functions in mmc.c be accessible to the new file, so they are
>>> declared in a new mmc_private.h header along with the write &
>>> erase functions. For what it's worth I prefered v1, but hey ho.
>>> ---
>>> drivers/mmc/Makefile | 2 +
>>> drivers/mmc/mmc.c | 186 +--------------------------------------------
>>> drivers/mmc/mmc_private.h | 45 +++++++++++
>>> drivers/mmc/mmc_write.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 240 insertions(+), 182 deletions(-)
>>> create mode 100644 drivers/mmc/mmc_private.h
>>> create mode 100644 drivers/mmc/mmc_write.c
>> 
>> 
>> <snip>
>> 
>>> 
>>> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
>>> new file mode 100644
>>> index 0000000..dde5cf2
>>> --- /dev/null
>>> +++ b/drivers/mmc/mmc_write.c
>>> @@ -0,0 +1,189 @@
>>> +/*
>>> + * Copyright 2008, Freescale Semiconductor, Inc
>>> + * Andy Fleming
>>> + *
>>> + * Based vaguely on the Linux code
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#include <config.h>
>>> +#include <common.h>
>>> +#include <part.h>
>>> +#include "mmc_private.h"
>>> +
>>> +static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
>>> +{
>>> + struct mmc_cmd cmd;
>>> + ulong end;
>>> + int err, start_cmd, end_cmd;
>>> +
>>> + if (mmc->high_capacity) {
>>> + end = start + blkcnt - 1;
>>> + } else {
>>> + end = (start + blkcnt - 1) * mmc->write_bl_len;
>>> + start *= mmc->write_bl_len;
>>> + }
>>> +
>>> + if (IS_SD(mmc)) {
>>> + start_cmd = SD_CMD_ERASE_WR_BLK_START;
>>> + end_cmd = SD_CMD_ERASE_WR_BLK_END;
>>> + } else {
>>> + start_cmd = MMC_CMD_ERASE_GROUP_START;
>>> + end_cmd = MMC_CMD_ERASE_GROUP_END;
>>> + }
>>> +
>>> + cmd.cmdidx = start_cmd;
>>> + cmd.cmdarg = start;
>>> + cmd.resp_type = MMC_RSP_R1;
>>> +
>>> + err = mmc_send_cmd(mmc, &cmd, NULL);
>>> + if (err)
>>> + goto err_out;
>>> +
>>> + cmd.cmdidx = end_cmd;
>>> + cmd.cmdarg = end;
>>> +
>>> + err = mmc_send_cmd(mmc, &cmd, NULL);
>>> + if (err)
>>> + goto err_out;
>>> +
>>> + cmd.cmdidx = MMC_CMD_ERASE;
>>> + cmd.cmdarg = SECURE_ERASE;
>>> + cmd.resp_type = MMC_RSP_R1b;
>>> +
>>> + err = mmc_send_cmd(mmc, &cmd, NULL);
>>> + if (err)
>>> + goto err_out;
>>> +
>>> + return 0;
>>> +
>>> +err_out:
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> + puts("mmc erase failed\n");
>>> +#endif
>> 
>> 
>> this conditional compile in of puts/printf for SPL are no longer
>> required, I'd prefere to remove them globally in mmc_write.c.
> Ah, yes good point, I'll remove that.
>> 
>>> 
>>> + return err;
>>> +}
>> 
>> 
>> Rest of this patch looks good to me.
>> 
>> Best regards
>> 
>> Andreas Bie?mann
> Thanks for looking at it!
> 
> Paul
> 

Seem good to me too. I'll give it a spin later this week and make sure nothing breaks.

Regards

-- Pantelis

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH v3] mmc: don't support write & erase for SPL builds
  2013-09-09  8:14         ` Paul Burton
  2013-09-09  8:27           ` Pantelis Antoniou
@ 2013-09-09 14:30           ` Paul Burton
  1 sibling, 0 replies; 17+ messages in thread
From: Paul Burton @ 2013-09-09 14:30 UTC (permalink / raw)
  To: u-boot

For SPL builds this is just dead code since we'll only need to read.
Eliminating it results in a significant size reduction for the SPL
binary, which may be critical for certain platforms where the binary
size is highly constrained.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Acked-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
Changes in v3:
  - Remove conditional compilation of prints/puts calls in mmc_write.c
    since this file is only compiled for non-SPL builds making the
    conditionals superfluous. As pointed out by Andreas Bie?mann.

Changes in v2:
  - Move the mmc_bwrite & mmc_berase functions to a new mmc_write.c
    file which is only compiled for non-SPL builds, as per a request
    from Pantelis Antoniou. This requires that a few formerly static
    functions in mmc.c be accessible to the new file, so they are
    declared in a new mmc_private.h header along with the write &
    erase functions. For what it's worth I prefered v1, but hey ho.
---
 drivers/mmc/Makefile      |   2 +
 drivers/mmc/mmc.c         | 186 +---------------------------------------------
 drivers/mmc/mmc_private.h |  45 +++++++++++
 drivers/mmc/mmc_write.c   | 179 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 230 insertions(+), 182 deletions(-)
 create mode 100644 drivers/mmc/mmc_private.h
 create mode 100644 drivers/mmc/mmc_write.c

diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
index bedf833..06280d1 100644
--- a/drivers/mmc/Makefile
+++ b/drivers/mmc/Makefile
@@ -34,6 +34,8 @@ COBJS-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
 COBJS-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
 ifdef CONFIG_SPL_BUILD
 COBJS-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o
+else
+COBJS-$(CONFIG_GENERIC_MMC) += mmc_write.o
 endif
 
 COBJS	:= $(COBJS-y)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 30a985b..666f77b 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -15,6 +15,7 @@
 #include <malloc.h>
 #include <linux/list.h>
 #include <div64.h>
+#include "mmc_private.h"
 
 /* Set block count limit because of 16 bit register limit on some hardware*/
 #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
@@ -52,8 +53,7 @@ int __board_mmc_getcd(struct mmc *mmc) {
 int board_mmc_getcd(struct mmc *mmc)__attribute__((weak,
 	alias("__board_mmc_getcd")));
 
-static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
-			struct mmc_data *data)
+int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 {
 	struct mmc_data backup;
 	int ret;
@@ -114,7 +114,7 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	return ret;
 }
 
-static int mmc_send_status(struct mmc *mmc, int timeout)
+int mmc_send_status(struct mmc *mmc, int timeout)
 {
 	struct mmc_cmd cmd;
 	int err, retries = 5;
@@ -162,7 +162,7 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
 	return 0;
 }
 
-static int mmc_set_blocklen(struct mmc *mmc, int len)
+int mmc_set_blocklen(struct mmc *mmc, int len)
 {
 	struct mmc_cmd cmd;
 
@@ -192,184 +192,6 @@ struct mmc *find_mmc_device(int dev_num)
 	return NULL;
 }
 
-static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
-{
-	struct mmc_cmd cmd;
-	ulong end;
-	int err, start_cmd, end_cmd;
-
-	if (mmc->high_capacity)
-		end = start + blkcnt - 1;
-	else {
-		end = (start + blkcnt - 1) * mmc->write_bl_len;
-		start *= mmc->write_bl_len;
-	}
-
-	if (IS_SD(mmc)) {
-		start_cmd = SD_CMD_ERASE_WR_BLK_START;
-		end_cmd = SD_CMD_ERASE_WR_BLK_END;
-	} else {
-		start_cmd = MMC_CMD_ERASE_GROUP_START;
-		end_cmd = MMC_CMD_ERASE_GROUP_END;
-	}
-
-	cmd.cmdidx = start_cmd;
-	cmd.cmdarg = start;
-	cmd.resp_type = MMC_RSP_R1;
-
-	err = mmc_send_cmd(mmc, &cmd, NULL);
-	if (err)
-		goto err_out;
-
-	cmd.cmdidx = end_cmd;
-	cmd.cmdarg = end;
-
-	err = mmc_send_cmd(mmc, &cmd, NULL);
-	if (err)
-		goto err_out;
-
-	cmd.cmdidx = MMC_CMD_ERASE;
-	cmd.cmdarg = SECURE_ERASE;
-	cmd.resp_type = MMC_RSP_R1b;
-
-	err = mmc_send_cmd(mmc, &cmd, NULL);
-	if (err)
-		goto err_out;
-
-	return 0;
-
-err_out:
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-	puts("mmc erase failed\n");
-#endif
-	return err;
-}
-
-static unsigned long
-mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
-{
-	int err = 0;
-	struct mmc *mmc = find_mmc_device(dev_num);
-	lbaint_t blk = 0, blk_r = 0;
-	int timeout = 1000;
-
-	if (!mmc)
-		return -1;
-
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-	if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
-		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
-		       "The erase range would be change to "
-		       "0x" LBAF "~0x" LBAF "\n\n",
-		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
-		       ((start + blkcnt + mmc->erase_grp_size)
-		       & ~(mmc->erase_grp_size - 1)) - 1);
-#endif
-
-	while (blk < blkcnt) {
-		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
-			mmc->erase_grp_size : (blkcnt - blk);
-		err = mmc_erase_t(mmc, start + blk, blk_r);
-		if (err)
-			break;
-
-		blk += blk_r;
-
-		/* Waiting for the ready status */
-		if (mmc_send_status(mmc, timeout))
-			return 0;
-	}
-
-	return blk;
-}
-
-static ulong
-mmc_write_blocks(struct mmc *mmc, lbaint_t start, lbaint_t blkcnt, const void*src)
-{
-	struct mmc_cmd cmd;
-	struct mmc_data data;
-	int timeout = 1000;
-
-	if ((start + blkcnt) > mmc->block_dev.lba) {
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
-			start + blkcnt, mmc->block_dev.lba);
-#endif
-		return 0;
-	}
-
-	if (blkcnt == 0)
-		return 0;
-	else if (blkcnt == 1)
-		cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK;
-	else
-		cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK;
-
-	if (mmc->high_capacity)
-		cmd.cmdarg = start;
-	else
-		cmd.cmdarg = start * mmc->write_bl_len;
-
-	cmd.resp_type = MMC_RSP_R1;
-
-	data.src = src;
-	data.blocks = blkcnt;
-	data.blocksize = mmc->write_bl_len;
-	data.flags = MMC_DATA_WRITE;
-
-	if (mmc_send_cmd(mmc, &cmd, &data)) {
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-		printf("mmc write failed\n");
-#endif
-		return 0;
-	}
-
-	/* SPI multiblock writes terminate using a special
-	 * token, not a STOP_TRANSMISSION request.
-	 */
-	if (!mmc_host_is_spi(mmc) && blkcnt > 1) {
-		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
-		cmd.cmdarg = 0;
-		cmd.resp_type = MMC_RSP_R1b;
-		if (mmc_send_cmd(mmc, &cmd, NULL)) {
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-			printf("mmc fail to send stop cmd\n");
-#endif
-			return 0;
-		}
-	}
-
-	/* Waiting for the ready status */
-	if (mmc_send_status(mmc, timeout))
-		return 0;
-
-	return blkcnt;
-}
-
-static ulong
-mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void*src)
-{
-	lbaint_t cur, blocks_todo = blkcnt;
-
-	struct mmc *mmc = find_mmc_device(dev_num);
-	if (!mmc)
-		return 0;
-
-	if (mmc_set_blocklen(mmc, mmc->write_bl_len))
-		return 0;
-
-	do {
-		cur = (blocks_todo > mmc->b_max) ?  mmc->b_max : blocks_todo;
-		if(mmc_write_blocks(mmc, start, cur, src) != cur)
-			return 0;
-		blocks_todo -= cur;
-		start += cur;
-		src += cur * mmc->write_bl_len;
-	} while (blocks_todo > 0);
-
-	return blkcnt;
-}
-
 static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start,
 			   lbaint_t blkcnt)
 {
diff --git a/drivers/mmc/mmc_private.h b/drivers/mmc/mmc_private.h
new file mode 100644
index 0000000..16dcf9f
--- /dev/null
+++ b/drivers/mmc/mmc_private.h
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2008,2010 Freescale Semiconductor, Inc
+ * Andy Fleming
+ *
+ * Based (loosely) on the Linux code
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _MMC_PRIVATE_H_
+#define _MMC_PRIVATE_H_
+
+#include <mmc.h>
+
+extern int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
+			struct mmc_data *data);
+extern int mmc_send_status(struct mmc *mmc, int timeout);
+extern int mmc_set_blocklen(struct mmc *mmc, int len);
+
+#ifndef CONFIG_SPL_BUILD
+
+extern unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt);
+
+extern ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt,
+		const void *src);
+
+#else /* CONFIG_SPL_BUILD */
+
+/* SPL will never write or erase, declare dummies to reduce code size. */
+
+static inline unsigned long mmc_berase(int dev_num, lbaint_t start,
+		lbaint_t blkcnt)
+{
+	return 0;
+}
+
+static inline ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt,
+		const void *src)
+{
+	return 0;
+}
+
+#endif /* CONFIG_SPL_BUILD */
+
+#endif /* _MMC_PRIVATE_H_ */
diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
new file mode 100644
index 0000000..aa2fdef
--- /dev/null
+++ b/drivers/mmc/mmc_write.c
@@ -0,0 +1,179 @@
+/*
+ * Copyright 2008, Freescale Semiconductor, Inc
+ * Andy Fleming
+ *
+ * Based vaguely on the Linux code
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <config.h>
+#include <common.h>
+#include <part.h>
+#include "mmc_private.h"
+
+static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
+{
+	struct mmc_cmd cmd;
+	ulong end;
+	int err, start_cmd, end_cmd;
+
+	if (mmc->high_capacity) {
+		end = start + blkcnt - 1;
+	} else {
+		end = (start + blkcnt - 1) * mmc->write_bl_len;
+		start *= mmc->write_bl_len;
+	}
+
+	if (IS_SD(mmc)) {
+		start_cmd = SD_CMD_ERASE_WR_BLK_START;
+		end_cmd = SD_CMD_ERASE_WR_BLK_END;
+	} else {
+		start_cmd = MMC_CMD_ERASE_GROUP_START;
+		end_cmd = MMC_CMD_ERASE_GROUP_END;
+	}
+
+	cmd.cmdidx = start_cmd;
+	cmd.cmdarg = start;
+	cmd.resp_type = MMC_RSP_R1;
+
+	err = mmc_send_cmd(mmc, &cmd, NULL);
+	if (err)
+		goto err_out;
+
+	cmd.cmdidx = end_cmd;
+	cmd.cmdarg = end;
+
+	err = mmc_send_cmd(mmc, &cmd, NULL);
+	if (err)
+		goto err_out;
+
+	cmd.cmdidx = MMC_CMD_ERASE;
+	cmd.cmdarg = SECURE_ERASE;
+	cmd.resp_type = MMC_RSP_R1b;
+
+	err = mmc_send_cmd(mmc, &cmd, NULL);
+	if (err)
+		goto err_out;
+
+	return 0;
+
+err_out:
+	puts("mmc erase failed\n");
+	return err;
+}
+
+unsigned long mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
+{
+	int err = 0;
+	struct mmc *mmc = find_mmc_device(dev_num);
+	lbaint_t blk = 0, blk_r = 0;
+	int timeout = 1000;
+
+	if (!mmc)
+		return -1;
+
+	if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
+		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
+		       "The erase range would be change to "
+		       "0x" LBAF "~0x" LBAF "\n\n",
+		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
+		       ((start + blkcnt + mmc->erase_grp_size)
+		       & ~(mmc->erase_grp_size - 1)) - 1);
+
+	while (blk < blkcnt) {
+		blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
+			mmc->erase_grp_size : (blkcnt - blk);
+		err = mmc_erase_t(mmc, start + blk, blk_r);
+		if (err)
+			break;
+
+		blk += blk_r;
+
+		/* Waiting for the ready status */
+		if (mmc_send_status(mmc, timeout))
+			return 0;
+	}
+
+	return blk;
+}
+
+static ulong mmc_write_blocks(struct mmc *mmc, lbaint_t start,
+		lbaint_t blkcnt, const void *src)
+{
+	struct mmc_cmd cmd;
+	struct mmc_data data;
+	int timeout = 1000;
+
+	if ((start + blkcnt) > mmc->block_dev.lba) {
+		printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
+		       start + blkcnt, mmc->block_dev.lba);
+		return 0;
+	}
+
+	if (blkcnt == 0)
+		return 0;
+	else if (blkcnt == 1)
+		cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK;
+	else
+		cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK;
+
+	if (mmc->high_capacity)
+		cmd.cmdarg = start;
+	else
+		cmd.cmdarg = start * mmc->write_bl_len;
+
+	cmd.resp_type = MMC_RSP_R1;
+
+	data.src = src;
+	data.blocks = blkcnt;
+	data.blocksize = mmc->write_bl_len;
+	data.flags = MMC_DATA_WRITE;
+
+	if (mmc_send_cmd(mmc, &cmd, &data)) {
+		printf("mmc write failed\n");
+		return 0;
+	}
+
+	/* SPI multiblock writes terminate using a special
+	 * token, not a STOP_TRANSMISSION request.
+	 */
+	if (!mmc_host_is_spi(mmc) && blkcnt > 1) {
+		cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
+		cmd.cmdarg = 0;
+		cmd.resp_type = MMC_RSP_R1b;
+		if (mmc_send_cmd(mmc, &cmd, NULL)) {
+			printf("mmc fail to send stop cmd\n");
+			return 0;
+		}
+	}
+
+	/* Waiting for the ready status */
+	if (mmc_send_status(mmc, timeout))
+		return 0;
+
+	return blkcnt;
+}
+
+ulong mmc_bwrite(int dev_num, lbaint_t start, lbaint_t blkcnt, const void *src)
+{
+	lbaint_t cur, blocks_todo = blkcnt;
+
+	struct mmc *mmc = find_mmc_device(dev_num);
+	if (!mmc)
+		return 0;
+
+	if (mmc_set_blocklen(mmc, mmc->write_bl_len))
+		return 0;
+
+	do {
+		cur = (blocks_todo > mmc->b_max) ?  mmc->b_max : blocks_todo;
+		if (mmc_write_blocks(mmc, start, cur, src) != cur)
+			return 0;
+		blocks_todo -= cur;
+		start += cur;
+		src += cur * mmc->write_bl_len;
+	} while (blocks_todo > 0);
+
+	return blkcnt;
+}
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-09-09 14:30 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-04 15:12 [U-Boot] [PATCH 0/5] SPL/MMC size fixes Paul Burton
2013-09-04 15:12 ` [U-Boot] [PATCH 1/5] spl: remove unnecessary (& ARM specific) include of asm/utils.h Paul Burton
2013-09-04 15:12 ` [U-Boot] [PATCH 2/5] spl_mmc: only call printf or puts with CONFIG_SPL_LIBCOMMON_SUPPORT Paul Burton
2013-09-04 15:12 ` [U-Boot] [PATCH 3/5] mmc: don't call *printf or puts when SPL & !CONFIG_SPL_LIBCOMMON_SUPPORT Paul Burton
2013-09-06 12:48   ` Pantelis Antoniou
2013-09-06 12:51     ` Paul Burton
2013-09-06 12:55       ` Pantelis Antoniou
2013-09-04 15:12 ` [U-Boot] [PATCH 4/5] mmc: size optimization when !CONFIG_MMC_SPI Paul Burton
2013-09-04 15:14 ` [U-Boot] [PATCH 5/5] mmc: don't support write & erase for SPL builds Paul Burton
2013-09-06 12:56   ` Pantelis Antoniou
2013-09-06 13:43     ` [U-Boot] [PATCH v2] " Paul Burton
2013-09-06 13:54       ` Pantelis Antoniou
2013-09-08  8:48       ` Andreas Bießmann
2013-09-09  8:14         ` Paul Burton
2013-09-09  8:27           ` Pantelis Antoniou
2013-09-09 14:30           ` [U-Boot] [PATCH v3] " Paul Burton
2013-09-04 15:15 ` [U-Boot] [PATCH 0/5] SPL/MMC size fixes Paul Burton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox