[U-Boot] [PATCH] DFU: fix for raw data write

[U-Boot] [PATCH] DFU: fix for raw data write

[Mateusz Zalega]

This commit fixes DFU raw (performed w/o partition interface, via
mmc_block_op()) write.

Mateusz Zalega (1):
  DFU: fix for raw data write

 drivers/dfu/dfu.c     | 22 ++++++++++++++++++++++
 drivers/dfu/dfu_mmc.c |  3 +++
 include/dfu.h         |  7 ++-----
 3 files changed, 27 insertions(+), 5 deletions(-)

-- 
1.8.2.1

[U-Boot] [PATCH] DFU: fix for raw data write

[Mateusz Zalega]

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.

Signed-off-by: Mateusz Zalega <m.zalega@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Minkyu Kang <mk7.kang@samsung.com>
---
 drivers/dfu/dfu.c     | 22 ++++++++++++++++++++++
 drivers/dfu/dfu_mmc.c |  3 +++
 include/dfu.h         |  7 ++-----
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index d73d510..6979112 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -414,3 +414,25 @@ struct dfu_entity *dfu_get_entity(int alt)
 
 	return NULL;
 }
+
+int get_mmc_blk_size(int dev)
+{
+	struct mmc *mmc = find_mmc_device(dev);
+
+	if (mmc == NULL) {
+		error("Couldn't find MMC device no. %d.\n", dev);
+		return -ENODEV;
+	}
+	if (!mmc->has_init) {
+		if (!mmc_init(mmc)) {
+			if (!mmc->read_bl_len) {
+				error("invalid block length\n");
+				return -ENODEV;
+			}
+		} else {
+			error("Couldn't init MMC device.\n");
+			return -ENODEV;
+		}
+	}
+	return mmc->read_bl_len;
+}
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index 0871a77..c776e26 100644
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -235,5 +235,8 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
 	/* initial state */
 	dfu->inited = 0;
 
+	/* better safe than sorry */
+	assert(dfu->data.mmc.lba_blk_size > 0);
+
 	return 0;
 }
diff --git a/include/dfu.h b/include/dfu.h
index 1d4006d..67ca711 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -40,6 +40,8 @@ struct mmc_internal_data {
 	unsigned int part;
 };
 
+int get_mmc_blk_size(int dev);
+
 struct nand_internal_data {
 	/* RAW programming */
 	u64 start;
@@ -49,11 +51,6 @@ struct nand_internal_data {
 	unsigned int part;
 };
 
-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
-- 
1.8.2.1

[U-Boot] [PATCH] DFU: fix for raw data write

[Lukasz Majewski]

Hi Mateusz,


> 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.
> 
> Signed-off-by: Mateusz Zalega <m.zalega@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Minkyu Kang <mk7.kang@samsung.com>
> ---
>  drivers/dfu/dfu.c     | 22 ++++++++++++++++++++++
>  drivers/dfu/dfu_mmc.c |  3 +++
>  include/dfu.h         |  7 ++-----
>  3 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index d73d510..6979112 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c

The mmc (or any other medium) related code shall NOT be added to
generic dfu.c file.

> @@ -414,3 +414,25 @@ struct dfu_entity *dfu_get_entity(int alt)
>  
>  	return NULL;
>  }
> +
> +int get_mmc_blk_size(int dev)
> +{
> +	struct mmc *mmc = find_mmc_device(dev);
> +
> +	if (mmc == NULL) {
> +		error("Couldn't find MMC device no. %d.\n", dev);
> +		return -ENODEV;
> +	}
> +	if (!mmc->has_init) {
> +		if (!mmc_init(mmc)) {
> +			if (!mmc->read_bl_len) {
> +				error("invalid block length\n");
> +				return -ENODEV;
> +			}
> +		} else {
> +			error("Couldn't init MMC device.\n");
> +			return -ENODEV;
> +		}
> +	}
> +	return mmc->read_bl_len;
> +}

The above code fits better at dfu_mmc.c file. My proposition would be
to clean up the dfu_fill_entity_mmc() function and add the above code
there.


> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
> index 0871a77..c776e26 100644
> --- a/drivers/dfu/dfu_mmc.c
> +++ b/drivers/dfu/dfu_mmc.c
> @@ -235,5 +235,8 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu,
> char *s) /* initial state */
>  	dfu->inited = 0;
>  
> +	/* better safe than sorry */
> +	assert(dfu->data.mmc.lba_blk_size > 0);
> +
>  	return 0;
>  }
> diff --git a/include/dfu.h b/include/dfu.h
> index 1d4006d..67ca711 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -40,6 +40,8 @@ struct mmc_internal_data {
>  	unsigned int part;
>  };
>  
> +int get_mmc_blk_size(int dev);
> +
>  struct nand_internal_data {
>  	/* RAW programming */
>  	u64 start;
> @@ -49,11 +51,6 @@ struct nand_internal_data {
>  	unsigned int part;
>  };
>  
> -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



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

[U-Boot] [PATCH v2] dfu:mmc: raw data write fix

[Mateusz Zalega]

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.

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>

---
Changes for v2:
- code cleanup
- minor dfu_alt_info format change

---
 drivers/dfu/dfu_mmc.c        | 110 +++++++++++++++++++++++++++----------------
 include/configs/am335x_evm.h |   4 +-
 include/configs/s5p_goni.h   |   2 +-
 include/configs/trats.h      |   2 +-
 include/dfu.h                |   5 --
 5 files changed, 73 insertions(+), 50 deletions(-)

diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index 0871a77..a91793b 100644
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -173,66 +173,94 @@ 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")) {
-
-		dfu->layout = DFU_RAW_ADDR;
+	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;
+		}
+	}
 
-		dev = simple_strtoul(s, &s, 10);
-		s++;
-		part = simple_strtoul(s, &s, 10);
+	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);
 
-		mmc = find_mmc_device(dev);
-		if (mmc == NULL || mmc_init(mmc)) {
-			printf("%s: could not find mmc device #%d!\n",
-			       __func__, dev);
+	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;
+	}
+	if (!mmc->has_init) {
+		if (!mmc_init(mmc)) {
+			if (!mmc->read_bl_len) {
+				error("invalid block length\n");
+				return -ENODEV;
+			}
+		} else {
+			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/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index e0a87f8..aa036b5 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -188,8 +188,8 @@
 	"boot part 0 1;" \
 	"rootfs part 0 2;" \
 	"MLO fat 0 1;" \
-	"MLO.raw mmc 100 100;" \
-	"u-boot.img.raw mmc 300 400;" \
+	"MLO.raw mmc 0x100 0x100;" \
+	"u-boot.img.raw mmc 0x300 0x400;" \
 	"spl-os-args.raw mmc 80 80;" \
 	"spl-os-image.raw mmc 900 2000;" \
 	"spl-os-args fat 0 1;" \
diff --git a/include/configs/s5p_goni.h b/include/configs/s5p_goni.h
index ef5c421..8c21ab5 100644
--- a/include/configs/s5p_goni.h
+++ b/include/configs/s5p_goni.h
@@ -86,7 +86,7 @@
 #define CONFIG_ZERO_BOOTDELAY_CHECK
 
 #define CONFIG_DFU_ALT \
-	"u-boot mmc 80 400;" \
+	"u-boot raw 0x80 0x400;" \
 	"uImage fat 0 2\0" \
 
 /* partitions definitions */
diff --git a/include/configs/trats.h b/include/configs/trats.h
index 9b6aac9..d40e583 100644
--- a/include/configs/trats.h
+++ b/include/configs/trats.h
@@ -129,7 +129,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\0"
 
diff --git a/include/dfu.h b/include/dfu.h
index 1d4006d..ea8eb09 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -49,11 +49,6 @@ struct nand_internal_data {
 	unsigned int part;
 };
 
-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
-- 
1.8.2.1

[U-Boot] [PATCH v2] dfu:mmc: raw data write fix

[Mateusz Zalega]

On 08/21/13 13:00, 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.
> 
> 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>
> 
> ---
> Changes for v2:
> - code cleanup
> - minor dfu_alt_info format change

Changes depend on "[PATCH RESEND v3] arm:goni: Update GONI configuration".

-- 
Mateusz Zalega
Samsung R&D Institute Poland

[U-Boot] [PATCH v2] dfu:mmc: raw data write fix

[Tom Rini]

On Wed, Aug 21, 2013 at 01:00:01PM +0200, 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.
> 
> 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>

Acked-by: Tom Rini <trini@ti.com>

I poked Marek on IRC yesterday, and he was fine with me taking this into
master, but since it depends on some other samsung patches, I'm fine
with this going in via u-boot-samsung.

-- 
Tom