dfu: mmc: Replace calls to u-boot commands with native mmc API

PATCH New Lukasz Majewski 3 participants 13 messages Feb 21, 2014 – Mar 17, 2014

Share & tools

Lukasz Majewski

author

2014-02-21 07:23 UTC #0 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.

Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.

Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.

Signed-off-by Lukasz Majewski <[email protected]>

---
 drivers/dfu/dfu_mmc.c |   31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

drivers/dfu/dfu_mmc.c +24 -7

index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];

@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
 static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 			u64 offset, void *buf, long *len)
 {
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
 
 	/*
 	 * We must ensure that we work in lba_blk_size chunks, so ALIGN

@@ -38,12 +39,28 @@ static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 		return -EINVAL;
 	}
 
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n", __func__,
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE", dfu->dev_num,
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num, blk_start,
+					      blk_count, buf);
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num, blk_start,
+					       blk_count, buf);
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
 }
 
 static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len)

Signature · 1 line

1.7.10.4

Pantelis Antoniou

2014-02-21 13:20 UTC #1 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

Hi Lukasz,

Looks fine to me. Let me run a few tests over the weekend and I'll apply.

Thanks

-- Pantelis

Quoted 74 lines · Lukasz Majewski

For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.

Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.

Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.

Signed-off-by Lukasz Majewski <[email protected]>

---
drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++-------

1 file changed, 24 insertions(+), 7 deletions(-)

drivers/dfu/dfu_mmc.c +24 -7

index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];

@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
 static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 			u64 offset, void *buf, long *len)
 {
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
 
 	/*
 	 * We must ensure that we work in lba_blk_size chunks, so ALIGN

@@ -38,12 +39,28 @@ static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 		return -EINVAL;
 	}
 
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n", __func__,
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE", dfu->dev_num,
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num, blk_start,
+					      blk_count, buf);
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num, blk_start,
+					       blk_count, buf);
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
 }
 
 static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len)

Signature · 1 line

1.7.10.4

Lukasz Majewski

2014-02-22 12:03 UTC #2 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

Hi Pantelis,

Quoted 5 lines

Hi Lukasz,
Looks fine to me.
Let me run a few tests over the weekend

I was wondering if we could share tests for DFU?

Will it be suitable to put it in the ./test/dfu?

Quoted 85 lines

and I'll
apply.
Thanks
-- Pantelis
Quoted 76 lines · Lukasz Majewski
For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.
Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.
Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.
Signed-off-by Lukasz Majewski <[email protected]>
---
drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
drivers/dfu/dfu_mmc.c +3 -2
index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];
@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
 static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 			u64 offset, void *buf, long *len)
 {
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
 
 	/*
 	 * We must ensure that we work in lba_blk_size chunks, so
ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum dfu_op op,
struct dfu_entity *dfu, return -EINVAL;
	}
drivers/dfu/dfu_mmc.c +1 -3
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",
__func__,
drivers/dfu/dfu_mmc.c +1 -0
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",
dfu->dev_num,
drivers/dfu/dfu_mmc.c +4 -0
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +4 -0
+					      blk_count, buf);
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +11 -2
+					       blk_count, buf);
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
}

Quoted 5 lines

Quoted 4 lines
static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) --
1.7.10.4

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140222/081f2a8d/attachment.pgp>

Lukasz Majewski

author

2014-02-27 08:36 UTC #3 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

In reply to Pantelis Antoniou

Hi Pantelis,

Quoted 4 lines

Hi Lukasz,
Looks fine to me. Let me run a few tests over the weekend and I'll apply.

Have the run test gone smoothly?

Quoted 88 lines

Thanks
-- Pantelis
Quoted 80 lines · Lukasz Majewski
For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.
Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.
Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.
Signed-off-by Lukasz Majewski <[email protected]>
---
drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
drivers/dfu/dfu_mmc.c +3 -2
index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];
@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
 static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 			u64 offset, void *buf, long *len)
 {
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
 
 	/*
 	 * We must ensure that we work in lba_blk_size chunks, so
ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum dfu_op op,
struct dfu_entity *dfu, return -EINVAL;
	}
drivers/dfu/dfu_mmc.c +1 -3
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",
__func__,
drivers/dfu/dfu_mmc.c +1 -0
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",
dfu->dev_num,
drivers/dfu/dfu_mmc.c +4 -0
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +4 -0
+					      blk_count, buf);
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +11 -2
+					       blk_count, buf);
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
}
static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) --
1.7.10.4

Signature · 5 lines

Best regards,

Lukasz Majewski

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

Pantelis Antoniou

2014-02-27 08:39 UTC #4 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

Hi Lukasz,

Quoted 9 lines · Lukasz Majewski

Hi Pantelis,
Quoted 4 lines
Hi Lukasz,
Looks fine to me. Let me run a few tests over the weekend and I'll apply.
Have the run test gone smoothly?

In the middle of reworking some core stuff in mmc, will test when that's done.

Regards

-- Pantelis

Quoted 97 lines

Quoted 88 lines
Thanks
-- Pantelis
Quoted 80 lines · Lukasz Majewski
For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.
Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.
Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.
Signed-off-by Lukasz Majewski <[email protected]>
---
drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
drivers/dfu/dfu_mmc.c +3 -2
index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];
@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
 static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 			u64 offset, void *buf, long *len)
 {
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
 
 	/*
 	 * We must ensure that we work in lba_blk_size chunks, so
ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum dfu_op op,
struct dfu_entity *dfu, return -EINVAL;
	}
drivers/dfu/dfu_mmc.c +1 -3
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",
__func__,
drivers/dfu/dfu_mmc.c +1 -0
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",
dfu->dev_num,
drivers/dfu/dfu_mmc.c +4 -0
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +4 -0
+					      blk_count, buf);
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +11 -2
+					       blk_count, buf);
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
}
static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) --
1.7.10.4
Signature · 5 lines
Best regards,

Lukasz Majewski

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

Lukasz Majewski

author

2014-02-27 09:21 UTC #5 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

Hi Pantelis,

Quoted 9 lines

Hi Lukasz,
Quoted 4 lines · Lukasz Majewski
Hi Pantelis,
Quoted 2 lines
Hi Lukasz,

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
Looks fine to me. Let me run a few tests over the weekend and I'll apply.
Have the run test gone smoothly?
In the middle of reworking some core stuff in mmc, will test when that's done.

Does your rework intend to change the MMC API?

I'm regarding this patch as one suitable for v2014.04, since it replaces cumbersome sprintf() approach with the native mmc API.

Those operations are done at dfu_mmc.c, not any core mmc file.

Quoted 103 lines

Regards
-- Pantelis
Quoted 97 lines
Quoted 88 lines
Thanks
-- Pantelis
Quoted 80 lines · Lukasz Majewski
For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.
Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.
Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.
Signed-off-by Lukasz Majewski <[email protected]>
---
drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
drivers/dfu/dfu_mmc.c +3 -2
index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];
@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
 static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 			u64 offset, void *buf, long *len)
 {
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
 
 	/*
 	 * We must ensure that we work in lba_blk_size chunks, so
ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum dfu_op op,
struct dfu_entity *dfu, return -EINVAL;
	}
drivers/dfu/dfu_mmc.c +1 -3
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",
__func__,
drivers/dfu/dfu_mmc.c +1 -0
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",
dfu->dev_num,
drivers/dfu/dfu_mmc.c +4 -0
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +4 -0
+					      blk_count, buf);
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +11 -2
+					       blk_count, buf);
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
}
static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) --
1.7.10.4
Signature · 5 lines
Best regards,

Lukasz Majewski

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

Signature · 5 lines

Best regards,

Lukasz Majewski

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

Pantelis Antoniou

2014-02-27 09:22 UTC #6 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

Hi Lukasz,

Quoted 20 lines · Lukasz Majewski

Hi Pantelis,
Quoted 9 lines
Hi Lukasz,
Quoted 4 lines · Lukasz Majewski
Hi Pantelis,
Quoted 2 lines
Hi Lukasz,
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Looks fine to me. Let me run a few tests over the weekend and I'll apply.
Have the run test gone smoothly?
In the middle of reworking some core stuff in mmc, will test when that's done.

Quoted 7 lines

Does your rework intend to change the MMC API?
I'm regarding this patch as one suitable for v2014.04, since it replaces cumbersome sprintf() approach with the native mmc API.
Those operations are done at dfu_mmc.c, not any core mmc file.

Your point is valid; I'll try to give it a spin today.

Quoted 112 lines

Quoted 103 lines
Regards
-- Pantelis
Quoted 97 lines
Quoted 88 lines
Thanks
-- Pantelis
Quoted 80 lines · Lukasz Majewski
For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.
Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.
Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.
Signed-off-by Lukasz Majewski <[email protected]>
---
drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
drivers/dfu/dfu_mmc.c +3 -2
index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];
@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
 static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 			u64 offset, void *buf, long *len)
 {
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
 
 	/*
 	 * We must ensure that we work in lba_blk_size chunks, so
ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum dfu_op op,
struct dfu_entity *dfu, return -EINVAL;
	}
drivers/dfu/dfu_mmc.c +1 -3
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",
__func__,
drivers/dfu/dfu_mmc.c +1 -0
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",
dfu->dev_num,
drivers/dfu/dfu_mmc.c +4 -0
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +4 -0
+					      blk_count, buf);
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +11 -2
+					       blk_count, buf);
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
}
static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) --
1.7.10.4
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Signature · 5 lines
Best regards,

Lukasz Majewski

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

Lukasz Majewski

author

2014-03-06 12:08 UTC #7 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

Hi Pantelis,

Quoted 26 lines

Hi Lukasz,
Quoted 22 lines · Lukasz Majewski
Hi Pantelis,
Quoted 9 lines
Hi Lukasz,
Quoted 4 lines · Lukasz Majewski
Hi Pantelis,
Quoted 2 lines
Hi Lukasz,
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Looks fine to me. Let me run a few tests over the weekend and I'll apply.
Have the run test gone smoothly?
In the middle of reworking some core stuff in mmc, will test when that's done.
Does your rework intend to change the MMC API?

Quoted 7 lines

Quoted 5 lines
I'm regarding this patch as one suitable for v2014.04, since it replaces cumbersome sprintf() approach with the native mmc API.
Those operations are done at dfu_mmc.c, not any core mmc file.
Your point is valid; I'll try to give it a spin today.

Ping.

Quoted 113 lines

Quoted 113 lines
Quoted 104 lines
Regards
-- Pantelis
Quoted 97 lines
Quoted 88 lines
Thanks
-- Pantelis
Quoted 80 lines · Lukasz Majewski
For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.
Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.
Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.
Signed-off-by Lukasz Majewski <[email protected]>
---
drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
drivers/dfu/dfu_mmc.c +3 -2
index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];
@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
 static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 			u64 offset, void *buf, long *len)
 {
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
 
 	/*
 	 * We must ensure that we work in lba_blk_size chunks,
so ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum
dfu_op op, struct dfu_entity *dfu, return -EINVAL;
	}
drivers/dfu/dfu_mmc.c +1 -3
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",
__func__,
drivers/dfu/dfu_mmc.c +1 -0
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",
dfu->dev_num,
drivers/dfu/dfu_mmc.c +4 -0
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +4 -0
+					      blk_count, buf);
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +11 -2
+					       blk_count, buf);
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
}
static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) --
1.7.10.4
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Signature · 5 lines
Best regards,

Lukasz Majewski

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

Signature · 5 lines

Best regards,

Lukasz Majewski

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

Lukasz Majewski

author

2014-03-12 08:06 UTC #8 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

In reply to Pantelis Antoniou

Hi Tom, Pantelis

Quoted 26 lines

Hi Lukasz,
Quoted 22 lines · Lukasz Majewski
Hi Pantelis,
Quoted 9 lines
Hi Lukasz,
Quoted 4 lines · Lukasz Majewski
Hi Pantelis,
Quoted 2 lines
Hi Lukasz,
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Looks fine to me. Let me run a few tests over the weekend and I'll apply.
Have the run test gone smoothly?
In the middle of reworking some core stuff in mmc, will test when that's done.
Does your rework intend to change the MMC API?

Quoted 7 lines

Quoted 5 lines
I'm regarding this patch as one suitable for v2014.04, since it replaces cumbersome sprintf() approach with the native mmc API.
Those operations are done at dfu_mmc.c, not any core mmc file.
Your point is valid; I'll try to give it a spin today.

Tom, Pantelis would it be possible to add this patch to the v2014.04? This patch was initially posted some time ago.

http://patchwork.ozlabs.org/patch/322450/

Replacing at DFU core the sprintf() with call to native eMMC API seems appealing :-).

Quoted 114 lines

Quoted 113 lines
Quoted 104 lines
Regards
-- Pantelis
Quoted 97 lines
Quoted 88 lines
Thanks
-- Pantelis
Quoted 80 lines · Lukasz Majewski
For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.
Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.
Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.
Signed-off-by Lukasz Majewski <[email protected]>
---
drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
drivers/dfu/dfu_mmc.c +3 -2
index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];
@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
 static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 			u64 offset, void *buf, long *len)
 {
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
 
 	/*
 	 * We must ensure that we work in lba_blk_size chunks,
so ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum
dfu_op op, struct dfu_entity *dfu, return -EINVAL;
	}
drivers/dfu/dfu_mmc.c +1 -3
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",
__func__,
drivers/dfu/dfu_mmc.c +1 -0
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",
dfu->dev_num,
drivers/dfu/dfu_mmc.c +4 -0
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +4 -0
+					      blk_count, buf);
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +11 -2
+					       blk_count, buf);
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
}
static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) --
1.7.10.4
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Signature · 5 lines
Best regards,

Lukasz Majewski

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

Signature · 5 lines

Best regards,

Lukasz Majewski

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

Pantelis Antoniou

2014-03-12 10:12 UTC #9 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

Hi Lukasz,

Quoted 36 lines · Lukasz Majewski

Hi Tom, Pantelis
Quoted 26 lines
Hi Lukasz,
Quoted 22 lines · Lukasz Majewski
Hi Pantelis,
Quoted 9 lines
Hi Lukasz,
Quoted 4 lines · Lukasz Majewski
Hi Pantelis,
Quoted 2 lines
Hi Lukasz,
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Looks fine to me. Let me run a few tests over the weekend and I'll apply.
Have the run test gone smoothly?
In the middle of reworking some core stuff in mmc, will test when that's done.
Does your rework intend to change the MMC API?
Quoted 7 lines
Quoted 5 lines
I'm regarding this patch as one suitable for v2014.04, since it replaces cumbersome sprintf() approach with the native mmc API.
Those operations are done at dfu_mmc.c, not any core mmc file.
Your point is valid; I'll try to give it a spin today.

Quoted 8 lines

Tom, Pantelis would it be possible to add this patch to the v2014.04? This patch was initially posted some time ago.
http://patchwork.ozlabs.org/patch/322450/
Replacing at DFU core the sprintf() with call to native eMMC API seems appealing :-).

Yes, it is :)

I'm going to include it; just busy with some mmc reorg right now.

Regards

-- Pantelis

Quoted 121 lines

Quoted 114 lines
Quoted 113 lines
Quoted 104 lines
Regards
-- Pantelis
Quoted 97 lines
Quoted 88 lines
Thanks
-- Pantelis
Quoted 80 lines · Lukasz Majewski
For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.
Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.
Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.
Signed-off-by Lukasz Majewski <[email protected]>
---
drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
drivers/dfu/dfu_mmc.c +3 -2
index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];
@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
 static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 			u64 offset, void *buf, long *len)
 {
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
 
 	/*
 	 * We must ensure that we work in lba_blk_size chunks,
so ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum
dfu_op op, struct dfu_entity *dfu, return -EINVAL;
	}
drivers/dfu/dfu_mmc.c +1 -3
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",
__func__,
drivers/dfu/dfu_mmc.c +1 -0
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",
dfu->dev_num,
drivers/dfu/dfu_mmc.c +4 -0
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +4 -0
+					      blk_count, buf);
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +11 -2
+					       blk_count, buf);
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
}
static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) --
1.7.10.4
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Signature · 5 lines
Best regards,

Lukasz Majewski

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

Lukasz Majewski

author

2014-03-12 11:12 UTC #10 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

Hi Pantelis, Tom,

Quoted 45 lines

Hi Lukasz,
Quoted 41 lines · Lukasz Majewski
Hi Tom, Pantelis
Quoted 26 lines
Hi Lukasz,
Quoted 22 lines · Lukasz Majewski
Hi Pantelis,
Quoted 9 lines
Hi Lukasz,
Quoted 4 lines · Lukasz Majewski
Hi Pantelis,
Quoted 2 lines
Hi Lukasz,
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Looks fine to me. Let me run a few tests over the weekend and I'll apply.
Have the run test gone smoothly?
In the middle of reworking some core stuff in mmc, will test when that's done.
Does your rework intend to change the MMC API?
Quoted 7 lines
Quoted 5 lines
I'm regarding this patch as one suitable for v2014.04, since it replaces cumbersome sprintf() approach with the native mmc API.
Those operations are done at dfu_mmc.c, not any core mmc file.
Your point is valid; I'll try to give it a spin today.
Tom, Pantelis would it be possible to add this patch to the v2014.04? This patch was initially posted some time ago.
http://patchwork.ozlabs.org/patch/322450/

Quoted 7 lines

Quoted 3 lines
Replacing at DFU core the sprintf() with call to native eMMC API seems appealing :-).
Yes, it is :)
I'm going to include it; just busy with some mmc reorg right now.

I think that, it would be better to ask Tom to pull it directly, since it is changing the code in the DFU not mmc.

Quoted 129 lines

Regards
-- Pantelis
Quoted 123 lines
Quoted 116 lines
Quoted 115 lines
Quoted 106 lines
Regards
-- Pantelis
Quoted 99 lines
Quoted 90 lines
Thanks
-- Pantelis
Quoted 82 lines · Lukasz Majewski
For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.
Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.
Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.
Signed-off-by Lukasz Majewski <[email protected]>
---
drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
drivers/dfu/dfu_mmc.c +1 -0
index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];
@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu, u64 offset, void *buf, long *len)
{
drivers/dfu/dfu_mmc.c +2 -2
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
	/*
	 * We must ensure that we work in lba_blk_size chunks,
so ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum
dfu_op op, struct dfu_entity *dfu, return -EINVAL;
	}
drivers/dfu/dfu_mmc.c +1 -3
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",
__func__,
drivers/dfu/dfu_mmc.c +1 -0
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",
dfu->dev_num,
drivers/dfu/dfu_mmc.c +4 -0
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +1 -0
+					      blk_count,
buf);
drivers/dfu/dfu_mmc.c +3 -0
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +1 -0
+					       blk_count,
buf);
drivers/dfu/dfu_mmc.c +10 -2
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
}
static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) --
1.7.10.4
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Signature · 5 lines
Best regards,

Lukasz Majewski

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

Signature · 5 lines

Best regards,

Lukasz Majewski

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

Pantelis Antoniou

2014-03-12 11:15 UTC #11 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

Quoted 51 lines · Lukasz Majewski

Hi Pantelis, Tom,
Quoted 49 lines
Hi Lukasz,
Quoted 44 lines · Lukasz Majewski
Hi Tom, Pantelis
Quoted 26 lines
Hi Lukasz,
Quoted 22 lines · Lukasz Majewski
Hi Pantelis,
Quoted 9 lines
Hi Lukasz,
Quoted 4 lines · Lukasz Majewski
Hi Pantelis,
Quoted 2 lines
Hi Lukasz,
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Looks fine to me. Let me run a few tests over the weekend and I'll apply.
Have the run test gone smoothly?
In the middle of reworking some core stuff in mmc, will test when that's done.
Does your rework intend to change the MMC API?
Quoted 7 lines
Quoted 5 lines
I'm regarding this patch as one suitable for v2014.04, since it replaces cumbersome sprintf() approach with the native mmc API.
Those operations are done at dfu_mmc.c, not any core mmc file.
Your point is valid; I'll try to give it a spin today.
Tom, Pantelis would it be possible to add this patch to the v2014.04? This patch was initially posted some time ago.
http://patchwork.ozlabs.org/patch/322450/
Replacing at DFU core the sprintf() with call to native eMMC API seems appealing :-).

Quoted 7 lines

Quoted 3 lines
Yes, it is :)
I'm going to include it; just busy with some mmc reorg right now.
I think that, it would be better to ask Tom to pull it directly, since it is changing the code in the DFU not mmc.

FWIW your patch applies cleanly even after the changes.

It will be part of the next mmc pull request, but I have no objection to Tom picking it up directly.

Regards

-- Pantelis

Quoted 139 lines

Quoted 130 lines
Regards
-- Pantelis
Quoted 123 lines
Quoted 116 lines
Quoted 115 lines
Quoted 106 lines
Regards
-- Pantelis
Quoted 99 lines
Quoted 90 lines
Thanks
-- Pantelis
Quoted 82 lines · Lukasz Majewski
For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.
Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.
Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.
Signed-off-by Lukasz Majewski <[email protected]>
---
drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
drivers/dfu/dfu_mmc.c +1 -0
index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];
@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu, u64 offset, void *buf, long *len)
{
drivers/dfu/dfu_mmc.c +2 -2
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
	/*
	 * We must ensure that we work in lba_blk_size chunks,
so ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum
dfu_op op, struct dfu_entity *dfu, return -EINVAL;
	}
drivers/dfu/dfu_mmc.c +1 -3
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",
__func__,
drivers/dfu/dfu_mmc.c +1 -0
+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",
dfu->dev_num,
drivers/dfu/dfu_mmc.c +4 -0
+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +1 -0
+					      blk_count,
buf);
drivers/dfu/dfu_mmc.c +3 -0
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num,
blk_start,
drivers/dfu/dfu_mmc.c +1 -0
+					       blk_count,
buf);
drivers/dfu/dfu_mmc.c +10 -2
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
}
static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) --
1.7.10.4
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Signature · 5 lines
Best regards,

Lukasz Majewski

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

Lukasz Majewski

author

2014-03-17 13:43 UTC #12 lore raw

PATCH 1/1 dfu: mmc: Replace calls to u-boot commands with native mmc API

In reply to Lukasz Majewski

Hi Lukasz,

Quoted 76 lines

For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone.

Switch to "native" mmc subsystem API would allow better type checking and shall improve speed.

Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused.

Signed-off-by Lukasz Majewski <[email protected]>

---
 drivers/dfu/dfu_mmc.c |   31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

drivers/dfu/dfu_mmc.c +3 -2

index 0816f46..651cfff 100644
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];

@@ -20,8 +21,8 @@ static long dfu_file_buf_len;
 static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
 			u64 offset, void *buf, long *len)
 {
-	char cmd_buf[DFU_CMD_BUF_SIZE];
-	u32 blk_start, blk_count;
+	struct mmc *mmc = find_mmc_device(dfu->dev_num);
+	u32 blk_start, blk_count, n = 0;
 
 	/*
 	 * We must ensure that we work in lba_blk_size chunks, so

ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu, return -EINVAL;

drivers/dfu/dfu_mmc.c +1 -3

 	}
 
-	sprintf(cmd_buf, "mmc %s %p %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		 buf, blk_start, blk_count);
+	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",

__func__,

drivers/dfu/dfu_mmc.c +1 -0

+	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",

dfu->dev_num,

drivers/dfu/dfu_mmc.c +4 -0

+	      blk_start, blk_count, buf);
+	switch (op) {
+	case DFU_OP_READ:
+		n = mmc->block_dev.block_read(dfu->dev_num,

blk_start,

drivers/dfu/dfu_mmc.c +4 -0

+					      blk_count, buf);
+		break;
+	case DFU_OP_WRITE:
+		n = mmc->block_dev.block_write(dfu->dev_num,

blk_start,

drivers/dfu/dfu_mmc.c +11 -2

+					       blk_count, buf);
+		break;
+	default:
+		error("Operation not supported\n");
+	}
 
-	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
-	return run_command(cmd_buf, 0);
+	if (n != blk_count) {
+		error("MMC operation failed");
+		return -EIO;
+	}
+
+	return 0;
 }
 
 static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long

Quoted 1 line

*len)

Applied to u-boot-dfu.

Signature · 5 lines

Best regards,

Lukasz Majewski

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