diff mbox

[U-Boot,06/10] dfu: mmc: buffer file reads too

Message ID 1441425831-3441-6-git-send-email-swarren@wwwdotorg.org
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Stephen Warren Sept. 5, 2015, 4:03 a.m. UTC
From: Stephen Warren <swarren@nvidia.com>

When writing to files in a filesystem on MMC, dfu_mmc.c buffers up the
entire file content until the end of the transaction, at which point the
file is written in one go. This allows writing files larger than the USB
transfer size (CONFIG_SYS_DFU_DATA_BUF_SIZE); the maximum written file
size is CONFIG_SYS_DFU_MAX_FILE_SIZE (the size of the temporary buffer).

The current file reading code does not do any buffering, and so limits
the maximum read file size to the USB transfer size. Enhance the code to
do the same kind of buffering as the write path, so the same file size
limits apply.

Remove the size checking code from dfu_read() since all read paths now
support larger files than the USB transfer buffer.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/dfu/dfu.c     | 11 -----------
 drivers/dfu/dfu_mmc.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 12 deletions(-)

Comments

Ɓukasz Majewski Sept. 8, 2015, 12:57 p.m. UTC | #1
Hi Stephen,

> From: Stephen Warren <swarren@nvidia.com>
> 
> When writing to files in a filesystem on MMC, dfu_mmc.c buffers up the
> entire file content until the end of the transaction, at which point
> the file is written in one go. This allows writing files larger than
> the USB transfer size (CONFIG_SYS_DFU_DATA_BUF_SIZE); the maximum
> written file size is CONFIG_SYS_DFU_MAX_FILE_SIZE (the size of the
> temporary buffer).
> 
> The current file reading code does not do any buffering, and so limits
> the maximum read file size to the USB transfer size. Enhance the code
> to do the same kind of buffering as the write path, so the same file
> size limits apply.
> 
> Remove the size checking code from dfu_read() since all read paths now
> support larger files than the USB transfer buffer.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  drivers/dfu/dfu.c     | 11 -----------
>  drivers/dfu/dfu_mmc.c | 27 ++++++++++++++++++++++++++-
>  2 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index d85d3f507a7b..d1e465aa7695 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -338,17 +338,6 @@ int dfu_read(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) dfu->r_left = dfu->get_medium_size(dfu);
>  		if (dfu->r_left < 0)
>  			return dfu->r_left;
> -		switch (dfu->layout) {
> -		case DFU_RAW_ADDR:
> -		case DFU_RAM_ADDR:
> -			break;
> -		default:
> -			if (dfu->r_left > dfu_buf_size) {
> -				printf("%s: File too big for
> buffer\n",
> -				       __func__);
> -				return -EOVERFLOW;
> -			}
> -		}
>  
>  		debug("%s: %s %ld [B]\n", __func__, dfu->name,
> dfu->r_left); 
> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
> index 2a780f7b5d31..5a9fb4a6e247 100644
> --- a/drivers/dfu/dfu_mmc.c
> +++ b/drivers/dfu/dfu_mmc.c
> @@ -18,6 +18,7 @@
>  
>  static unsigned char *dfu_file_buf;
>  static long dfu_file_buf_len;
> +static long dfu_file_buf_filled;
>  
>  static int mmc_access_part(struct dfu_entity *dfu, struct mmc *mmc,
> int part) {
> @@ -230,9 +231,12 @@ long dfu_get_medium_size_mmc(struct dfu_entity
> *dfu) return dfu->data.mmc.lba_size * dfu->data.mmc.lba_blk_size;
>  	case DFU_FS_FAT:
>  	case DFU_FS_EXT4:
> +		dfu_file_buf_filled = -1;
>  		ret = mmc_file_op(DFU_OP_SIZE, dfu, NULL, &len);
>  		if (ret < 0)
>  			return ret;
> +		if (len > CONFIG_SYS_DFU_MAX_FILE_SIZE)
> +			return -1;
>  		return len;
>  	default:
>  		printf("%s: Layout (%s) not (yet) supported!\n",
> __func__, @@ -241,6 +245,27 @@ long dfu_get_medium_size_mmc(struct
> dfu_entity *dfu) }
>  }
>  
> +static int mmc_file_unbuffer(struct dfu_entity *dfu, u64 offset,
> void *buf,
> +			     long *len)
> +{
> +	int ret;
> +	long file_len;
> +
> +	if (dfu_file_buf_filled == -1) {
> +		ret = mmc_file_op(DFU_OP_READ, dfu, dfu_file_buf,
> &file_len);
> +		if (ret < 0)
> +			return ret;
> +		dfu_file_buf_filled = file_len;
> +	}
> +	if (offset + *len > dfu_file_buf_filled)
> +		return -EINVAL;
> +
> +	/* Add to the current buffer. */
> +	memcpy(buf, dfu_file_buf + offset, *len);
> +
> +	return 0;
> +}
> +
>  int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void
> *buf, long *len)
>  {
> @@ -252,7 +277,7 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu,
> u64 offset, void *buf, break;
>  	case DFU_FS_FAT:
>  	case DFU_FS_EXT4:
> -		ret = mmc_file_op(DFU_OP_READ, dfu, buf, len);
> +		ret = mmc_file_unbuffer(dfu, offset, buf, len);
>  		break;
>  	default:
>  		printf("%s: Layout (%s) not (yet) supported!\n",
> __func__,

Acked-by: Lukasz Majewski <l.majewski@samsung.com>

Tested-by: Lukasz Majewski <l.majewski@samsung.com>
Test HW: Odroid XU3 - Exynos5433
[DFU Tests]
Tom Rini Sept. 12, 2015, 12:51 p.m. UTC | #2
On Fri, Sep 04, 2015 at 10:03:47PM -0600, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> When writing to files in a filesystem on MMC, dfu_mmc.c buffers up the
> entire file content until the end of the transaction, at which point the
> file is written in one go. This allows writing files larger than the USB
> transfer size (CONFIG_SYS_DFU_DATA_BUF_SIZE); the maximum written file
> size is CONFIG_SYS_DFU_MAX_FILE_SIZE (the size of the temporary buffer).
> 
> The current file reading code does not do any buffering, and so limits
> the maximum read file size to the USB transfer size. Enhance the code to
> do the same kind of buffering as the write path, so the same file size
> limits apply.
> 
> Remove the size checking code from dfu_read() since all read paths now
> support larger files than the USB transfer buffer.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Acked-by: Lukasz Majewski <l.majewski@samsung.com>
> Tested-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot/master, thanks!
diff mbox

Patch

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index d85d3f507a7b..d1e465aa7695 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -338,17 +338,6 @@  int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 		dfu->r_left = dfu->get_medium_size(dfu);
 		if (dfu->r_left < 0)
 			return dfu->r_left;
-		switch (dfu->layout) {
-		case DFU_RAW_ADDR:
-		case DFU_RAM_ADDR:
-			break;
-		default:
-			if (dfu->r_left > dfu_buf_size) {
-				printf("%s: File too big for buffer\n",
-				       __func__);
-				return -EOVERFLOW;
-			}
-		}
 
 		debug("%s: %s %ld [B]\n", __func__, dfu->name, dfu->r_left);
 
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index 2a780f7b5d31..5a9fb4a6e247 100644
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -18,6 +18,7 @@ 
 
 static unsigned char *dfu_file_buf;
 static long dfu_file_buf_len;
+static long dfu_file_buf_filled;
 
 static int mmc_access_part(struct dfu_entity *dfu, struct mmc *mmc, int part)
 {
@@ -230,9 +231,12 @@  long dfu_get_medium_size_mmc(struct dfu_entity *dfu)
 		return dfu->data.mmc.lba_size * dfu->data.mmc.lba_blk_size;
 	case DFU_FS_FAT:
 	case DFU_FS_EXT4:
+		dfu_file_buf_filled = -1;
 		ret = mmc_file_op(DFU_OP_SIZE, dfu, NULL, &len);
 		if (ret < 0)
 			return ret;
+		if (len > CONFIG_SYS_DFU_MAX_FILE_SIZE)
+			return -1;
 		return len;
 	default:
 		printf("%s: Layout (%s) not (yet) supported!\n", __func__,
@@ -241,6 +245,27 @@  long dfu_get_medium_size_mmc(struct dfu_entity *dfu)
 	}
 }
 
+static int mmc_file_unbuffer(struct dfu_entity *dfu, u64 offset, void *buf,
+			     long *len)
+{
+	int ret;
+	long file_len;
+
+	if (dfu_file_buf_filled == -1) {
+		ret = mmc_file_op(DFU_OP_READ, dfu, dfu_file_buf, &file_len);
+		if (ret < 0)
+			return ret;
+		dfu_file_buf_filled = file_len;
+	}
+	if (offset + *len > dfu_file_buf_filled)
+		return -EINVAL;
+
+	/* Add to the current buffer. */
+	memcpy(buf, dfu_file_buf + offset, *len);
+
+	return 0;
+}
+
 int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf,
 		long *len)
 {
@@ -252,7 +277,7 @@  int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf,
 		break;
 	case DFU_FS_FAT:
 	case DFU_FS_EXT4:
-		ret = mmc_file_op(DFU_OP_READ, dfu, buf, len);
+		ret = mmc_file_unbuffer(dfu, offset, buf, len);
 		break;
 	default:
 		printf("%s: Layout (%s) not (yet) supported!\n", __func__,