* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API @ 2014-02-21 7:23 Lukasz Majewski 2014-02-21 13:20 ` Pantelis Antoniou 2014-03-17 13:43 ` Lukasz Majewski 0 siblings, 2 replies; 13+ messages in thread From: Lukasz Majewski @ 2014-02-21 7:23 UTC (permalink / raw) To: u-boot 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 <l.majewski@samsung.com> --- drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 0816f46..651cfff 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -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) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-02-21 7:23 [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API Lukasz Majewski @ 2014-02-21 13:20 ` Pantelis Antoniou 2014-02-22 12:03 ` Lukasz Majewski 2014-02-27 8:36 ` Lukasz Majewski 2014-03-17 13:43 ` Lukasz Majewski 1 sibling, 2 replies; 13+ messages in thread From: Pantelis Antoniou @ 2014-02-21 13:20 UTC (permalink / raw) To: u-boot Hi Lukasz, Looks fine to me. Let me run a few tests over the weekend and I'll apply. Thanks -- Pantelis On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote: > 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 <l.majewski@samsung.com> > --- > drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c > index 0816f46..651cfff 100644 > --- a/drivers/dfu/dfu_mmc.c > +++ b/drivers/dfu/dfu_mmc.c > @@ -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) > -- > 1.7.10.4 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-02-21 13:20 ` Pantelis Antoniou @ 2014-02-22 12:03 ` Lukasz Majewski 2014-02-27 8:36 ` Lukasz Majewski 1 sibling, 0 replies; 13+ messages in thread From: Lukasz Majewski @ 2014-02-22 12:03 UTC (permalink / raw) To: u-boot Hi Pantelis, > 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? > and I'll > apply. > > Thanks > > -- Pantelis > > On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote: > > > 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 <l.majewski@samsung.com> > > --- > > drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- > > 1 file changed, 24 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c > > index 0816f46..651cfff 100644 > > --- a/drivers/dfu/dfu_mmc.c > > +++ b/drivers/dfu/dfu_mmc.c > > @@ -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) -- > > 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> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-02-21 13:20 ` Pantelis Antoniou 2014-02-22 12:03 ` Lukasz Majewski @ 2014-02-27 8:36 ` Lukasz Majewski 2014-02-27 8:39 ` Pantelis Antoniou 1 sibling, 1 reply; 13+ messages in thread From: Lukasz Majewski @ 2014-02-27 8:36 UTC (permalink / raw) To: u-boot Hi Pantelis, > 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? > > Thanks > > -- Pantelis > > On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote: > > > 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 <l.majewski@samsung.com> > > --- > > drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- > > 1 file changed, 24 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c > > index 0816f46..651cfff 100644 > > --- a/drivers/dfu/dfu_mmc.c > > +++ b/drivers/dfu/dfu_mmc.c > > @@ -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) -- > > 1.7.10.4 > > > -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-02-27 8:36 ` Lukasz Majewski @ 2014-02-27 8:39 ` Pantelis Antoniou 2014-02-27 9:21 ` Lukasz Majewski 0 siblings, 1 reply; 13+ messages in thread From: Pantelis Antoniou @ 2014-02-27 8:39 UTC (permalink / raw) To: u-boot Hi Lukasz, On Feb 27, 2014, at 10:36 AM, Lukasz Majewski wrote: > Hi Pantelis, > >> 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 >> >> Thanks >> >> -- Pantelis >> >> On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote: >> >>> 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 <l.majewski@samsung.com> >>> --- >>> drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- >>> 1 file changed, 24 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c >>> index 0816f46..651cfff 100644 >>> --- a/drivers/dfu/dfu_mmc.c >>> +++ b/drivers/dfu/dfu_mmc.c >>> @@ -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) -- >>> 1.7.10.4 >>> >> > > > > -- > Best regards, > > Lukasz Majewski > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-02-27 8:39 ` Pantelis Antoniou @ 2014-02-27 9:21 ` Lukasz Majewski 2014-02-27 9:22 ` Pantelis Antoniou 0 siblings, 1 reply; 13+ messages in thread From: Lukasz Majewski @ 2014-02-27 9:21 UTC (permalink / raw) To: u-boot Hi Pantelis, > Hi Lukasz, > > > On Feb 27, 2014, at 10:36 AM, Lukasz Majewski wrote: > > > Hi Pantelis, > > > >> 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. 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. > > Regards > > -- Pantelis > > >> > >> Thanks > >> > >> -- Pantelis > >> > >> On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote: > >> > >>> 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 <l.majewski@samsung.com> > >>> --- > >>> drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- > >>> 1 file changed, 24 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c > >>> index 0816f46..651cfff 100644 > >>> --- a/drivers/dfu/dfu_mmc.c > >>> +++ b/drivers/dfu/dfu_mmc.c > >>> @@ -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) -- > >>> 1.7.10.4 > >>> > >> > > > > > > > > -- > > Best regards, > > > > Lukasz Majewski > > > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group > -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-02-27 9:21 ` Lukasz Majewski @ 2014-02-27 9:22 ` Pantelis Antoniou 2014-03-06 12:08 ` Lukasz Majewski 2014-03-12 8:06 ` Lukasz Majewski 0 siblings, 2 replies; 13+ messages in thread From: Pantelis Antoniou @ 2014-02-27 9:22 UTC (permalink / raw) To: u-boot Hi Lukasz, On Feb 27, 2014, at 11:21 AM, Lukasz Majewski wrote: > Hi Pantelis, > >> Hi Lukasz, >> >> >> On Feb 27, 2014, at 10:36 AM, Lukasz Majewski wrote: >> >>> Hi Pantelis, >>> >>>> 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. > > 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. >> >> Regards >> >> -- Pantelis >> >>>> >>>> Thanks >>>> >>>> -- Pantelis >>>> >>>> On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote: >>>> >>>>> 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 <l.majewski@samsung.com> >>>>> --- >>>>> drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- >>>>> 1 file changed, 24 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c >>>>> index 0816f46..651cfff 100644 >>>>> --- a/drivers/dfu/dfu_mmc.c >>>>> +++ b/drivers/dfu/dfu_mmc.c >>>>> @@ -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) -- >>>>> 1.7.10.4 >>>>> >>>> >>> >>> >>> >>> -- >>> Best regards, >>> >>> Lukasz Majewski >>> >>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group >> > > > > -- > Best regards, > > Lukasz Majewski > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-02-27 9:22 ` Pantelis Antoniou @ 2014-03-06 12:08 ` Lukasz Majewski 2014-03-12 8:06 ` Lukasz Majewski 1 sibling, 0 replies; 13+ messages in thread From: Lukasz Majewski @ 2014-03-06 12:08 UTC (permalink / raw) To: u-boot Hi Pantelis, > Hi Lukasz, > > On Feb 27, 2014, at 11:21 AM, Lukasz Majewski wrote: > > > Hi Pantelis, > > > >> Hi Lukasz, > >> > >> > >> On Feb 27, 2014, at 10:36 AM, Lukasz Majewski wrote: > >> > >>> Hi Pantelis, > >>> > >>>> 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. > > > > 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. Ping. > > >> > >> Regards > >> > >> -- Pantelis > >> > >>>> > >>>> Thanks > >>>> > >>>> -- Pantelis > >>>> > >>>> On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote: > >>>> > >>>>> 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 <l.majewski@samsung.com> > >>>>> --- > >>>>> drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- > >>>>> 1 file changed, 24 insertions(+), 7 deletions(-) > >>>>> > >>>>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c > >>>>> index 0816f46..651cfff 100644 > >>>>> --- a/drivers/dfu/dfu_mmc.c > >>>>> +++ b/drivers/dfu/dfu_mmc.c > >>>>> @@ -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) -- > >>>>> 1.7.10.4 > >>>>> > >>>> > >>> > >>> > >>> > >>> -- > >>> Best regards, > >>> > >>> Lukasz Majewski > >>> > >>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group > >> > > > > > > > > -- > > Best regards, > > > > Lukasz Majewski > > > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-02-27 9:22 ` Pantelis Antoniou 2014-03-06 12:08 ` Lukasz Majewski @ 2014-03-12 8:06 ` Lukasz Majewski 2014-03-12 10:12 ` Pantelis Antoniou 1 sibling, 1 reply; 13+ messages in thread From: Lukasz Majewski @ 2014-03-12 8:06 UTC (permalink / raw) To: u-boot Hi Tom, Pantelis > Hi Lukasz, > > On Feb 27, 2014, at 11:21 AM, Lukasz Majewski wrote: > > > Hi Pantelis, > > > >> Hi Lukasz, > >> > >> > >> On Feb 27, 2014, at 10:36 AM, Lukasz Majewski wrote: > >> > >>> Hi Pantelis, > >>> > >>>> 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. > > > > 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. 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 :-). > > >> > >> Regards > >> > >> -- Pantelis > >> > >>>> > >>>> Thanks > >>>> > >>>> -- Pantelis > >>>> > >>>> On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote: > >>>> > >>>>> 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 <l.majewski@samsung.com> > >>>>> --- > >>>>> drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- > >>>>> 1 file changed, 24 insertions(+), 7 deletions(-) > >>>>> > >>>>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c > >>>>> index 0816f46..651cfff 100644 > >>>>> --- a/drivers/dfu/dfu_mmc.c > >>>>> +++ b/drivers/dfu/dfu_mmc.c > >>>>> @@ -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) -- > >>>>> 1.7.10.4 > >>>>> > >>>> > >>> > >>> > >>> > >>> -- > >>> Best regards, > >>> > >>> Lukasz Majewski > >>> > >>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group > >> > > > > > > > > -- > > Best regards, > > > > Lukasz Majewski > > > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group > -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-03-12 8:06 ` Lukasz Majewski @ 2014-03-12 10:12 ` Pantelis Antoniou 2014-03-12 11:12 ` Lukasz Majewski 0 siblings, 1 reply; 13+ messages in thread From: Pantelis Antoniou @ 2014-03-12 10:12 UTC (permalink / raw) To: u-boot Hi Lukasz, On Mar 12, 2014, at 10:06 AM, Lukasz Majewski wrote: > Hi Tom, Pantelis > >> Hi Lukasz, >> >> On Feb 27, 2014, at 11:21 AM, Lukasz Majewski wrote: >> >>> Hi Pantelis, >>> >>>> Hi Lukasz, >>>> >>>> >>>> On Feb 27, 2014, at 10:36 AM, Lukasz Majewski wrote: >>>> >>>>> Hi Pantelis, >>>>> >>>>>> 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. >>> >>> 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. > > 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 >> >>>> >>>> Regards >>>> >>>> -- Pantelis >>>> >>>>>> >>>>>> Thanks >>>>>> >>>>>> -- Pantelis >>>>>> >>>>>> On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote: >>>>>> >>>>>>> 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 <l.majewski@samsung.com> >>>>>>> --- >>>>>>> drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- >>>>>>> 1 file changed, 24 insertions(+), 7 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c >>>>>>> index 0816f46..651cfff 100644 >>>>>>> --- a/drivers/dfu/dfu_mmc.c >>>>>>> +++ b/drivers/dfu/dfu_mmc.c >>>>>>> @@ -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) -- >>>>>>> 1.7.10.4 >>>>>>> >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Best regards, >>>>> >>>>> Lukasz Majewski >>>>> >>>>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group >>>> >>> >>> >>> >>> -- >>> Best regards, >>> >>> Lukasz Majewski >>> >>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group >> > > -- > Best regards, > > Lukasz Majewski > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-03-12 10:12 ` Pantelis Antoniou @ 2014-03-12 11:12 ` Lukasz Majewski 2014-03-12 11:15 ` Pantelis Antoniou 0 siblings, 1 reply; 13+ messages in thread From: Lukasz Majewski @ 2014-03-12 11:12 UTC (permalink / raw) To: u-boot Hi Pantelis, Tom, > Hi Lukasz, > > On Mar 12, 2014, at 10:06 AM, Lukasz Majewski wrote: > > > Hi Tom, Pantelis > > > >> Hi Lukasz, > >> > >> On Feb 27, 2014, at 11:21 AM, Lukasz Majewski wrote: > >> > >>> Hi Pantelis, > >>> > >>>> Hi Lukasz, > >>>> > >>>> > >>>> On Feb 27, 2014, at 10:36 AM, Lukasz Majewski wrote: > >>>> > >>>>> Hi Pantelis, > >>>>> > >>>>>> 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. > >>> > >>> 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. > > > > 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. 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. > > Regards > > -- Pantelis > > >> > >>>> > >>>> Regards > >>>> > >>>> -- Pantelis > >>>> > >>>>>> > >>>>>> Thanks > >>>>>> > >>>>>> -- Pantelis > >>>>>> > >>>>>> On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote: > >>>>>> > >>>>>>> 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 <l.majewski@samsung.com> > >>>>>>> --- > >>>>>>> drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- > >>>>>>> 1 file changed, 24 insertions(+), 7 deletions(-) > >>>>>>> > >>>>>>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c > >>>>>>> index 0816f46..651cfff 100644 > >>>>>>> --- a/drivers/dfu/dfu_mmc.c > >>>>>>> +++ b/drivers/dfu/dfu_mmc.c > >>>>>>> @@ -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) -- > >>>>>>> 1.7.10.4 > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> Best regards, > >>>>> > >>>>> Lukasz Majewski > >>>>> > >>>>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group > >>>> > >>> > >>> > >>> > >>> -- > >>> Best regards, > >>> > >>> Lukasz Majewski > >>> > >>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group > >> > > > > -- > > Best regards, > > > > Lukasz Majewski > > > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group > -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-03-12 11:12 ` Lukasz Majewski @ 2014-03-12 11:15 ` Pantelis Antoniou 0 siblings, 0 replies; 13+ messages in thread From: Pantelis Antoniou @ 2014-03-12 11:15 UTC (permalink / raw) To: u-boot On Mar 12, 2014, at 1:12 PM, Lukasz Majewski wrote: > Hi Pantelis, Tom, > >> Hi Lukasz, >> >> On Mar 12, 2014, at 10:06 AM, Lukasz Majewski wrote: >> >>> Hi Tom, Pantelis >>> >>>> Hi Lukasz, >>>> >>>> On Feb 27, 2014, at 11:21 AM, Lukasz Majewski wrote: >>>> >>>>> Hi Pantelis, >>>>> >>>>>> Hi Lukasz, >>>>>> >>>>>> >>>>>> On Feb 27, 2014, at 10:36 AM, Lukasz Majewski wrote: >>>>>> >>>>>>> Hi Pantelis, >>>>>>> >>>>>>>> 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. >>>>> >>>>> 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. >>> >>> 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. > > 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 > >> >> Regards >> >> -- Pantelis >> >>>> >>>>>> >>>>>> Regards >>>>>> >>>>>> -- Pantelis >>>>>> >>>>>>>> >>>>>>>> Thanks >>>>>>>> >>>>>>>> -- Pantelis >>>>>>>> >>>>>>>> On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote: >>>>>>>> >>>>>>>>> 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 <l.majewski@samsung.com> >>>>>>>>> --- >>>>>>>>> drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- >>>>>>>>> 1 file changed, 24 insertions(+), 7 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c >>>>>>>>> index 0816f46..651cfff 100644 >>>>>>>>> --- a/drivers/dfu/dfu_mmc.c >>>>>>>>> +++ b/drivers/dfu/dfu_mmc.c >>>>>>>>> @@ -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) -- >>>>>>>>> 1.7.10.4 >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Best regards, >>>>>>> >>>>>>> Lukasz Majewski >>>>>>> >>>>>>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Best regards, >>>>> >>>>> Lukasz Majewski >>>>> >>>>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group >>>> >>> >>> -- >>> Best regards, >>> >>> Lukasz Majewski >>> >>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group >> > > > > -- > Best regards, > > Lukasz Majewski > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API 2014-02-21 7:23 [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API Lukasz Majewski 2014-02-21 13:20 ` Pantelis Antoniou @ 2014-03-17 13:43 ` Lukasz Majewski 1 sibling, 0 replies; 13+ messages in thread From: Lukasz Majewski @ 2014-03-17 13:43 UTC (permalink / raw) To: u-boot Hi Lukasz, > 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 <l.majewski@samsung.com> > --- > drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c > index 0816f46..651cfff 100644 > --- a/drivers/dfu/dfu_mmc.c > +++ b/drivers/dfu/dfu_mmc.c > @@ -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) Applied to u-boot-dfu. -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-03-17 13:43 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-21 7:23 [U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API Lukasz Majewski 2014-02-21 13:20 ` Pantelis Antoniou 2014-02-22 12:03 ` Lukasz Majewski 2014-02-27 8:36 ` Lukasz Majewski 2014-02-27 8:39 ` Pantelis Antoniou 2014-02-27 9:21 ` Lukasz Majewski 2014-02-27 9:22 ` Pantelis Antoniou 2014-03-06 12:08 ` Lukasz Majewski 2014-03-12 8:06 ` Lukasz Majewski 2014-03-12 10:12 ` Pantelis Antoniou 2014-03-12 11:12 ` Lukasz Majewski 2014-03-12 11:15 ` Pantelis Antoniou 2014-03-17 13:43 ` Lukasz Majewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox