Patchwork [U-Boot,v3,7/9] dfu:mmc: raw data write fix

login
register
mail settings
Submitter Mateusz Zalega
Date Jan. 9, 2014, 2:31 p.m.
Message ID <1389277919-15279-7-git-send-email-m.zalega@samsung.com>
Download mbox | patch
Permalink /patch/308827/
State Changes Requested
Delegated to: Minkyu Kang
Headers show

Comments

Mateusz Zalega - Jan. 9, 2014, 2:31 p.m.
When user attempted to perform a raw write using DFU (vide
dfu_fill_entity_mmc) with MMC interface not initialized before,
get_mmc_blk_size() reported invalid (zero) block size - it wasn't
possible to write ie. a new u-boot image.

This commit fixes that by initializing device in get_mmc_blk_size() when
needed.

Tested on Samsung Goni.

v2 changes:
- code cleanup
- minor dfu_alt_info format change

v3 changes:
- moved invalid block length check to mmc core
- removed redundant 'has_init' check

Change-Id: Icb50bb9f805a9a78848acd19f682fad474cb9082
Signed-off-by: Mateusz Zalega <m.zalega@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
Cc: Minkyu Kang <mk7.kang@samsung.com>
---
 drivers/dfu/dfu_mmc.c        | 106 ++++++++++++++++++++++++++-----------------
 drivers/mmc/mmc.c            |  13 ++++--
 include/configs/am335x_evm.h |   8 ++--
 include/configs/trats.h      |   2 +-
 include/configs/trats2.h     |   2 +-
 include/dfu.h                |   5 --
 6 files changed, 80 insertions(+), 56 deletions(-)
Jaehoon Chung - Jan. 10, 2014, 5:03 a.m.
Hi, Mateusz,

This patch should be separated with dfu and mmc.

On 01/09/2014 11:31 PM, Mateusz Zalega wrote:
> When user attempted to perform a raw write using DFU (vide
> dfu_fill_entity_mmc) with MMC interface not initialized before,
> get_mmc_blk_size() reported invalid (zero) block size - it wasn't
> possible to write ie. a new u-boot image.
> 
> This commit fixes that by initializing device in get_mmc_blk_size() when
> needed.
> 
> Tested on Samsung Goni.
> 
> v2 changes:
> - code cleanup
> - minor dfu_alt_info format change
> 
> v3 changes:
> - moved invalid block length check to mmc core
> - removed redundant 'has_init' check
> 
> Change-Id: Icb50bb9f805a9a78848acd19f682fad474cb9082
> Signed-off-by: Mateusz Zalega <m.zalega@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Minkyu Kang <mk7.kang@samsung.com>
> ---
>  drivers/dfu/dfu_mmc.c        | 106 ++++++++++++++++++++++++++-----------------
>  drivers/mmc/mmc.c            |  13 ++++--
>  include/configs/am335x_evm.h |   8 ++--
>  include/configs/trats.h      |   2 +-
>  include/configs/trats2.h     |   2 +-
>  include/dfu.h                |   5 --
>  6 files changed, 80 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
> index f942758..075e4cd 100644
> --- a/drivers/dfu/dfu_mmc.c
> +++ b/drivers/dfu/dfu_mmc.c
> @@ -168,66 +168,88 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf,
>  	return ret;
>  }
>  
> +/*
> + * @param s Parameter string containing space-separated arguments:
> + *	1st:
> + *		raw	(raw read/write)
> + *		fat	(files)
> + *		ext4	(^)
> + *		part	(partition image)
> + *	2nd and 3rd:
> + *		lba_start and lba_size, for raw write
> + *		mmc_dev and mmc_part, for filesystems and part
> + */
>  int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
>  {
> -	int dev, part;
> -	struct mmc *mmc;
> -	block_dev_desc_t *blk_dev;
> -	disk_partition_t partinfo;
> -	char *st;
> -
> -	dfu->dev_type = DFU_DEV_MMC;
> -	st = strsep(&s, " ");
> -	if (!strcmp(st, "mmc")) {
> -		dfu->layout = DFU_RAW_ADDR;
> -		dfu->data.mmc.lba_start = simple_strtoul(s, &s, 16);
> -		dfu->data.mmc.lba_size = simple_strtoul(++s, &s, 16);
> -		dfu->data.mmc.lba_blk_size = get_mmc_blk_size(dfu->dev_num);
> -	} else if (!strcmp(st, "fat")) {
> -		dfu->layout = DFU_FS_FAT;
> -	} else if (!strcmp(st, "ext4")) {
> -		dfu->layout = DFU_FS_EXT4;
> -	} else if (!strcmp(st, "part")) {
> +	const char *argv[3];
> +	const char **parg = argv;
> +	for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) {
> +		*parg = strsep(&s, " ");
> +		if (*parg == NULL) {
> +			error("Invalid number of arguments.\n");
> +			return -ENODEV;
> +		}
> +	}
>  
> -		dfu->layout = DFU_RAW_ADDR;
> +	const char *entity_type = argv[0];
> +	/*
> +	 * Base 0 means we'll accept (prefixed with 0x or 0) base 16, 8,
> +	 * with default 10.
> +	 */
> +	size_t second_arg = simple_strtoul(argv[1], NULL, 0);
> +	size_t third_arg = simple_strtoul(argv[2], NULL, 0);
>  
> -		dev = simple_strtoul(s, &s, 10);
> -		s++;
> -		part = simple_strtoul(s, &s, 10);
> +	struct mmc *mmc = find_mmc_device(dfu->dev_num);
> +	if (mmc == NULL) {
> +		error("Couldn't find MMC device no. %d.\n", dfu->dev_num);
> +		return -ENODEV;
> +	}
>  
> -		mmc = find_mmc_device(dev);
> -		if (mmc == NULL || mmc_init(mmc)) {
> -			printf("%s: could not find mmc device #%d!\n",
> -			       __func__, dev);
> -			return -ENODEV;
> -		}
> +	if (mmc_init(mmc)) {
> +		error("Couldn't init MMC device.\n");
> +		return -ENODEV;
> +	}
>  
> -		blk_dev = &mmc->block_dev;
> -		if (get_partition_info(blk_dev, part, &partinfo) != 0) {
> -			printf("%s: could not find partition #%d on mmc device #%d!\n",
> -			       __func__, part, dev);
> +	if (!strcmp(entity_type, "raw")) {
> +		dfu->layout			= DFU_RAW_ADDR;
> +		dfu->data.mmc.lba_start		= second_arg;
> +		dfu->data.mmc.lba_size		= third_arg;
> +		dfu->data.mmc.lba_blk_size	= mmc->read_bl_len;
> +	} else if (!strcmp(entity_type, "part")) {
> +		disk_partition_t partinfo;
> +		block_dev_desc_t *blk_dev = &mmc->block_dev;
> +		int mmcdev = second_arg;
> +		int mmcpart = third_arg;
> +
> +		if (get_partition_info(blk_dev, mmcpart, &partinfo) != 0) {
> +			error("Couldn't find part #%d on mmc device #%d\n",
> +			      mmcpart, mmcdev);
>  			return -ENODEV;
>  		}
>  
> -		dfu->data.mmc.lba_start = partinfo.start;
> -		dfu->data.mmc.lba_size = partinfo.size;
> -		dfu->data.mmc.lba_blk_size = partinfo.blksz;
> -
> +		dfu->layout			= DFU_RAW_ADDR;
> +		dfu->data.mmc.lba_start		= partinfo.start;
> +		dfu->data.mmc.lba_size		= partinfo.size;
> +		dfu->data.mmc.lba_blk_size	= partinfo.blksz;
> +	} else if (!strcmp(entity_type, "fat")) {
> +		dfu->layout = DFU_FS_FAT;
> +	} else if (!strcmp(entity_type, "ext4")) {
> +		dfu->layout = DFU_FS_EXT4;
>  	} else {
> -		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
> +		error("Memory layout (%s) not supported!\n", entity_type);
>  		return -ENODEV;
>  	}
>  
> -	if (dfu->layout == DFU_FS_EXT4 || dfu->layout == DFU_FS_FAT) {
> -		dfu->data.mmc.dev = simple_strtoul(s, &s, 10);
> -		dfu->data.mmc.part = simple_strtoul(++s, &s, 10);
> +	/* if it's NOT a raw write */
> +	if (strcmp(entity_type, "raw")) {
> +		dfu->data.mmc.dev = second_arg;
> +		dfu->data.mmc.part = third_arg;
>  	}
>  
> +	dfu->dev_type = DFU_DEV_MMC;
>  	dfu->read_medium = dfu_read_medium_mmc;
>  	dfu->write_medium = dfu_write_medium_mmc;
>  	dfu->flush_medium = dfu_flush_medium_mmc;
> -
> -	/* initial state */
>  	dfu->inited = 0;
>  
>  	return 0;
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index e1461a9..f2fa230 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 <errno.h>
>  #include "mmc_private.h"
>  
>  /* Set block count limit because of 16 bit register limit on some hardware*/
> @@ -1266,17 +1267,23 @@ static int mmc_complete_init(struct mmc *mmc)
>  
>  int mmc_init(struct mmc *mmc)
>  {
> +	if (mmc->has_init)
> +		return 0;
> +
What difference before?

>  	int err = IN_PROGRESS;
>  	unsigned start = get_timer(0);
>  
> -	if (mmc->has_init)
> -		return 0;
>  	if (!mmc->init_in_progress)
>  		err = mmc_start_init(mmc);
> -
Need not to change.

>  	if (!err || err == IN_PROGRESS)
>  		err = mmc_complete_init(mmc);
> +
Ditto.

>  	debug("%s: %d, time %lu\n", __func__, err, get_timer(start));
> +
> +	if (!mmc->read_bl_len || !mmc->write_bl_len) {
> +		error("invalid block length\n");
> +		return -ENODEV;
> +	}
I know mmc->read_bl_len and write_bl_len is set at init time.
Why this condition needs?

>  	return err;
>  }
>  
> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
> index 8af4d6a..d76962f 100644
> --- a/include/configs/am335x_evm.h
> +++ b/include/configs/am335x_evm.h
> @@ -312,10 +312,10 @@
>  	"boot part 0 1;" \
>  	"rootfs part 0 2;" \
>  	"MLO fat 0 1;" \
> -	"MLO.raw mmc 100 100;" \
> -	"u-boot.img.raw mmc 300 400;" \
> -	"spl-os-args.raw mmc 80 80;" \
> -	"spl-os-image.raw mmc 900 2000;" \
> +	"MLO.raw mmc 0x100 0x100;" \
> +	"u-boot.img.raw mmc 0x300 0x400;" \
> +	"spl-os-args.raw mmc 0x80 0x80;" \
> +	"spl-os-image.raw mmc 0x900 0x2000;" \
>  	"spl-os-args fat 0 1;" \
>  	"spl-os-image fat 0 1;" \
>  	"u-boot.img fat 0 1;" \
> diff --git a/include/configs/trats.h b/include/configs/trats.h
> index 6cd15c2..ed3b278 100644
> --- a/include/configs/trats.h
> +++ b/include/configs/trats.h
> @@ -140,7 +140,7 @@
>  	"name="PARTS_UMS",size=-,uuid=${uuid_gpt_"PARTS_UMS"}\0" \
>  
>  #define CONFIG_DFU_ALT \
> -	"u-boot mmc 80 400;" \
> +	"u-boot raw 0x80 0x400;" \
>  	"uImage ext4 0 2;" \
>  	"exynos4210-trats.dtb ext4 0 2;" \
>  	""PARTS_ROOT" part 0 5\0"
> diff --git a/include/configs/trats2.h b/include/configs/trats2.h
> index 5d86a3d..a22be63 100644
> --- a/include/configs/trats2.h
> +++ b/include/configs/trats2.h
> @@ -169,7 +169,7 @@
>  	"name="PARTS_UMS",size=-,uuid=${uuid_gpt_"PARTS_UMS"}\0" \
>  
>  #define CONFIG_DFU_ALT \
> -	"u-boot mmc 80 800;" \
> +	"u-boot mmc 0x80 0x800;" \

u-boot mmc? u-boot raw? what's correct?

Best Regards,
Jaehoon Chung
>  	"uImage ext4 0 2;" \
>  	"exynos4412-trats2.dtb ext4 0 2;" \
>  	""PARTS_ROOT" part 0 5\0"
> diff --git a/include/dfu.h b/include/dfu.h
> index f973426..f2e83db 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -64,11 +64,6 @@ struct ram_internal_data {
>  	unsigned int	size;
>  };
>  
> -static inline unsigned int get_mmc_blk_size(int dev)
> -{
> -	return find_mmc_device(dev)->read_bl_len;
> -}
> -
>  #define DFU_NAME_SIZE			32
>  #define DFU_CMD_BUF_SIZE		128
>  #ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE
>
Mateusz Zalega - Jan. 13, 2014, 1:34 p.m.
On 01/10/14 06:03, Jaehoon Chung wrote:
> This patch should be separated with dfu and mmc.

ACK, because we're going to remove the bl_len assertion, see below.

> On 01/09/2014 11:31 PM, Mateusz Zalega wrote:
>> When user attempted to perform a raw write using DFU (vide
>> dfu_fill_entity_mmc) with MMC interface not initialized before,
>> get_mmc_blk_size() reported invalid (zero) block size - it wasn't
>> possible to write ie. a new u-boot image.
>>
>> This commit fixes that by initializing device in get_mmc_blk_size() when
>> needed.
>>
>> Tested on Samsung Goni.
>>
>> v2 changes:
>> - code cleanup
>> - minor dfu_alt_info format change
>>
>> v3 changes:
>> - moved invalid block length check to mmc core
>> - removed redundant 'has_init' check
>>
>> Change-Id: Icb50bb9f805a9a78848acd19f682fad474cb9082
>> Signed-off-by: Mateusz Zalega <m.zalega@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
>> Cc: Minkyu Kang <mk7.kang@samsung.com>
>> ---
>>  drivers/dfu/dfu_mmc.c        | 106 ++++++++++++++++++++++++++-----------------
>>  drivers/mmc/mmc.c            |  13 ++++--
>>  include/configs/am335x_evm.h |   8 ++--
>>  include/configs/trats.h      |   2 +-
>>  include/configs/trats2.h     |   2 +-
>>  include/dfu.h                |   5 --
>>  6 files changed, 80 insertions(+), 56 deletions(-)
>>
>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
(...)
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index e1461a9..f2fa230 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 <errno.h>
>>  #include "mmc_private.h"
>>  
>>  /* Set block count limit because of 16 bit register limit on some hardware*/
>> @@ -1266,17 +1267,23 @@ static int mmc_complete_init(struct mmc *mmc)
>>  
>>  int mmc_init(struct mmc *mmc)
>>  {
>> +	if (mmc->has_init)
>> +		return 0;
>> +
> What difference before?

It doesn't have to go through get_timer(). The effect, although
negligible, saves some cycles.

>>  	int err = IN_PROGRESS;
>>  	unsigned start = get_timer(0);
>>  
>> -	if (mmc->has_init)
>> -		return 0;
>>  	if (!mmc->init_in_progress)
>>  		err = mmc_start_init(mmc);
>> -
> Need not to change.
> 
>>  	if (!err || err == IN_PROGRESS)
>>  		err = mmc_complete_init(mmc);
>> +
> Ditto.

NAK, it improves code readability.

>>  	debug("%s: %d, time %lu\n", __func__, err, get_timer(start));
>> +
>> +	if (!mmc->read_bl_len || !mmc->write_bl_len) {
>> +		error("invalid block length\n");
>> +		return -ENODEV;
>> +	}
> I know mmc->read_bl_len and write_bl_len is set at init time.
> Why this condition needs?

I added it as a countermeasure after fixing the bug and mistook its
purpose when writing a late update to this patch, my bad.

Given the circumstances it might be a sound assertion, but we shouldn't
clobber the codebase that we aim to optimize for size, should we.

ACK, will remove.

>>  	return err;
>>  }
>>  
>> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
>> index 8af4d6a..d76962f 100644
>> --- a/include/configs/am335x_evm.h
>> +++ b/include/configs/am335x_evm.h
>> @@ -312,10 +312,10 @@
>>  	"boot part 0 1;" \
>>  	"rootfs part 0 2;" \
>>  	"MLO fat 0 1;" \
>> -	"MLO.raw mmc 100 100;" \
>> -	"u-boot.img.raw mmc 300 400;" \
>> -	"spl-os-args.raw mmc 80 80;" \
>> -	"spl-os-image.raw mmc 900 2000;" \
>> +	"MLO.raw mmc 0x100 0x100;" \
>> +	"u-boot.img.raw mmc 0x300 0x400;" \
>> +	"spl-os-args.raw mmc 0x80 0x80;" \
>> +	"spl-os-image.raw mmc 0x900 0x2000;" \
>>  	"spl-os-args fat 0 1;" \
>>  	"spl-os-image fat 0 1;" \
>>  	"u-boot.img fat 0 1;" \
>> diff --git a/include/configs/trats.h b/include/configs/trats.h
>> index 6cd15c2..ed3b278 100644
>> --- a/include/configs/trats.h
>> +++ b/include/configs/trats.h
>> @@ -140,7 +140,7 @@
>>  	"name="PARTS_UMS",size=-,uuid=${uuid_gpt_"PARTS_UMS"}\0" \
>>  
>>  #define CONFIG_DFU_ALT \
>> -	"u-boot mmc 80 400;" \
>> +	"u-boot raw 0x80 0x400;" \
>>  	"uImage ext4 0 2;" \
>>  	"exynos4210-trats.dtb ext4 0 2;" \
>>  	""PARTS_ROOT" part 0 5\0"
>> diff --git a/include/configs/trats2.h b/include/configs/trats2.h
>> index 5d86a3d..a22be63 100644
>> --- a/include/configs/trats2.h
>> +++ b/include/configs/trats2.h
>> @@ -169,7 +169,7 @@
>>  	"name="PARTS_UMS",size=-,uuid=${uuid_gpt_"PARTS_UMS"}\0" \
>>  
>>  #define CONFIG_DFU_ALT \
>> -	"u-boot mmc 80 800;" \
>> +	"u-boot mmc 0x80 0x800;" \
> 
> u-boot mmc? u-boot raw? what's correct?

raw - ACK

>>  	"uImage ext4 0 2;" \
>>  	"exynos4412-trats2.dtb ext4 0 2;" \
>>  	""PARTS_ROOT" part 0 5\0"
>> diff --git a/include/dfu.h b/include/dfu.h
>> index f973426..f2e83db 100644
>> --- a/include/dfu.h
>> +++ b/include/dfu.h
>> @@ -64,11 +64,6 @@ struct ram_internal_data {
>>  	unsigned int	size;
>>  };
>>  
>> -static inline unsigned int get_mmc_blk_size(int dev)
>> -{
>> -	return find_mmc_device(dev)->read_bl_len;
>> -}
>> -
>>  #define DFU_NAME_SIZE			32
>>  #define DFU_CMD_BUF_SIZE		128
>>  #ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE
>>
> 
>

Patch

diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index f942758..075e4cd 100644
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -168,66 +168,88 @@  int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf,
 	return ret;
 }
 
+/*
+ * @param s Parameter string containing space-separated arguments:
+ *	1st:
+ *		raw	(raw read/write)
+ *		fat	(files)
+ *		ext4	(^)
+ *		part	(partition image)
+ *	2nd and 3rd:
+ *		lba_start and lba_size, for raw write
+ *		mmc_dev and mmc_part, for filesystems and part
+ */
 int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
 {
-	int dev, part;
-	struct mmc *mmc;
-	block_dev_desc_t *blk_dev;
-	disk_partition_t partinfo;
-	char *st;
-
-	dfu->dev_type = DFU_DEV_MMC;
-	st = strsep(&s, " ");
-	if (!strcmp(st, "mmc")) {
-		dfu->layout = DFU_RAW_ADDR;
-		dfu->data.mmc.lba_start = simple_strtoul(s, &s, 16);
-		dfu->data.mmc.lba_size = simple_strtoul(++s, &s, 16);
-		dfu->data.mmc.lba_blk_size = get_mmc_blk_size(dfu->dev_num);
-	} else if (!strcmp(st, "fat")) {
-		dfu->layout = DFU_FS_FAT;
-	} else if (!strcmp(st, "ext4")) {
-		dfu->layout = DFU_FS_EXT4;
-	} else if (!strcmp(st, "part")) {
+	const char *argv[3];
+	const char **parg = argv;
+	for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) {
+		*parg = strsep(&s, " ");
+		if (*parg == NULL) {
+			error("Invalid number of arguments.\n");
+			return -ENODEV;
+		}
+	}
 
-		dfu->layout = DFU_RAW_ADDR;
+	const char *entity_type = argv[0];
+	/*
+	 * Base 0 means we'll accept (prefixed with 0x or 0) base 16, 8,
+	 * with default 10.
+	 */
+	size_t second_arg = simple_strtoul(argv[1], NULL, 0);
+	size_t third_arg = simple_strtoul(argv[2], NULL, 0);
 
-		dev = simple_strtoul(s, &s, 10);
-		s++;
-		part = simple_strtoul(s, &s, 10);
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	if (mmc == NULL) {
+		error("Couldn't find MMC device no. %d.\n", dfu->dev_num);
+		return -ENODEV;
+	}
 
-		mmc = find_mmc_device(dev);
-		if (mmc == NULL || mmc_init(mmc)) {
-			printf("%s: could not find mmc device #%d!\n",
-			       __func__, dev);
-			return -ENODEV;
-		}
+	if (mmc_init(mmc)) {
+		error("Couldn't init MMC device.\n");
+		return -ENODEV;
+	}
 
-		blk_dev = &mmc->block_dev;
-		if (get_partition_info(blk_dev, part, &partinfo) != 0) {
-			printf("%s: could not find partition #%d on mmc device #%d!\n",
-			       __func__, part, dev);
+	if (!strcmp(entity_type, "raw")) {
+		dfu->layout			= DFU_RAW_ADDR;
+		dfu->data.mmc.lba_start		= second_arg;
+		dfu->data.mmc.lba_size		= third_arg;
+		dfu->data.mmc.lba_blk_size	= mmc->read_bl_len;
+	} else if (!strcmp(entity_type, "part")) {
+		disk_partition_t partinfo;
+		block_dev_desc_t *blk_dev = &mmc->block_dev;
+		int mmcdev = second_arg;
+		int mmcpart = third_arg;
+
+		if (get_partition_info(blk_dev, mmcpart, &partinfo) != 0) {
+			error("Couldn't find part #%d on mmc device #%d\n",
+			      mmcpart, mmcdev);
 			return -ENODEV;
 		}
 
-		dfu->data.mmc.lba_start = partinfo.start;
-		dfu->data.mmc.lba_size = partinfo.size;
-		dfu->data.mmc.lba_blk_size = partinfo.blksz;
-
+		dfu->layout			= DFU_RAW_ADDR;
+		dfu->data.mmc.lba_start		= partinfo.start;
+		dfu->data.mmc.lba_size		= partinfo.size;
+		dfu->data.mmc.lba_blk_size	= partinfo.blksz;
+	} else if (!strcmp(entity_type, "fat")) {
+		dfu->layout = DFU_FS_FAT;
+	} else if (!strcmp(entity_type, "ext4")) {
+		dfu->layout = DFU_FS_EXT4;
 	} else {
-		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+		error("Memory layout (%s) not supported!\n", entity_type);
 		return -ENODEV;
 	}
 
-	if (dfu->layout == DFU_FS_EXT4 || dfu->layout == DFU_FS_FAT) {
-		dfu->data.mmc.dev = simple_strtoul(s, &s, 10);
-		dfu->data.mmc.part = simple_strtoul(++s, &s, 10);
+	/* if it's NOT a raw write */
+	if (strcmp(entity_type, "raw")) {
+		dfu->data.mmc.dev = second_arg;
+		dfu->data.mmc.part = third_arg;
 	}
 
+	dfu->dev_type = DFU_DEV_MMC;
 	dfu->read_medium = dfu_read_medium_mmc;
 	dfu->write_medium = dfu_write_medium_mmc;
 	dfu->flush_medium = dfu_flush_medium_mmc;
-
-	/* initial state */
 	dfu->inited = 0;
 
 	return 0;
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index e1461a9..f2fa230 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 <errno.h>
 #include "mmc_private.h"
 
 /* Set block count limit because of 16 bit register limit on some hardware*/
@@ -1266,17 +1267,23 @@  static int mmc_complete_init(struct mmc *mmc)
 
 int mmc_init(struct mmc *mmc)
 {
+	if (mmc->has_init)
+		return 0;
+
 	int err = IN_PROGRESS;
 	unsigned start = get_timer(0);
 
-	if (mmc->has_init)
-		return 0;
 	if (!mmc->init_in_progress)
 		err = mmc_start_init(mmc);
-
 	if (!err || err == IN_PROGRESS)
 		err = mmc_complete_init(mmc);
+
 	debug("%s: %d, time %lu\n", __func__, err, get_timer(start));
+
+	if (!mmc->read_bl_len || !mmc->write_bl_len) {
+		error("invalid block length\n");
+		return -ENODEV;
+	}
 	return err;
 }
 
diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index 8af4d6a..d76962f 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -312,10 +312,10 @@ 
 	"boot part 0 1;" \
 	"rootfs part 0 2;" \
 	"MLO fat 0 1;" \
-	"MLO.raw mmc 100 100;" \
-	"u-boot.img.raw mmc 300 400;" \
-	"spl-os-args.raw mmc 80 80;" \
-	"spl-os-image.raw mmc 900 2000;" \
+	"MLO.raw mmc 0x100 0x100;" \
+	"u-boot.img.raw mmc 0x300 0x400;" \
+	"spl-os-args.raw mmc 0x80 0x80;" \
+	"spl-os-image.raw mmc 0x900 0x2000;" \
 	"spl-os-args fat 0 1;" \
 	"spl-os-image fat 0 1;" \
 	"u-boot.img fat 0 1;" \
diff --git a/include/configs/trats.h b/include/configs/trats.h
index 6cd15c2..ed3b278 100644
--- a/include/configs/trats.h
+++ b/include/configs/trats.h
@@ -140,7 +140,7 @@ 
 	"name="PARTS_UMS",size=-,uuid=${uuid_gpt_"PARTS_UMS"}\0" \
 
 #define CONFIG_DFU_ALT \
-	"u-boot mmc 80 400;" \
+	"u-boot raw 0x80 0x400;" \
 	"uImage ext4 0 2;" \
 	"exynos4210-trats.dtb ext4 0 2;" \
 	""PARTS_ROOT" part 0 5\0"
diff --git a/include/configs/trats2.h b/include/configs/trats2.h
index 5d86a3d..a22be63 100644
--- a/include/configs/trats2.h
+++ b/include/configs/trats2.h
@@ -169,7 +169,7 @@ 
 	"name="PARTS_UMS",size=-,uuid=${uuid_gpt_"PARTS_UMS"}\0" \
 
 #define CONFIG_DFU_ALT \
-	"u-boot mmc 80 800;" \
+	"u-boot mmc 0x80 0x800;" \
 	"uImage ext4 0 2;" \
 	"exynos4412-trats2.dtb ext4 0 2;" \
 	""PARTS_ROOT" part 0 5\0"
diff --git a/include/dfu.h b/include/dfu.h
index f973426..f2e83db 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -64,11 +64,6 @@  struct ram_internal_data {
 	unsigned int	size;
 };
 
-static inline unsigned int get_mmc_blk_size(int dev)
-{
-	return find_mmc_device(dev)->read_bl_len;
-}
-
 #define DFU_NAME_SIZE			32
 #define DFU_CMD_BUF_SIZE		128
 #ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE