[U-Boot] [PATCH v2 5/9] dfu: tftp: update: Add dfu_write_from_mem_addr() function
Simon Glass
sjg at chromium.org
Mon Aug 3 00:30:34 CEST 2015
Hi Lukasz,
On 25 July 2015 at 02:11, Lukasz Majewski <l.majewski at majess.pl> wrote:
> This function allows writing via DFU data stored from fixed buffer address
> (like e.g. loadaddr env variable).
>
> Such predefined buffers are used in the update_tftp() code. In fact this
> function is a wrapper on the dfu_write() and dfu_flush().
>
> Signed-off-by: Lukasz Majewski <l.majewski at majess.pl>
> ---
> Changes for v2:
> - Use min() macro instead of comparison
> - Change definition of left variable to be unsigned long - this allowed
> safe usage of min() macro
> ---
> drivers/dfu/dfu.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/dfu.h | 1 +
> 2 files changed, 49 insertions(+)
Reviewed-by: Simon Glass <sjg at chromium.org>
But see nits below.
>
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 2267dbf..c83ee41 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -568,3 +568,51 @@ int dfu_get_alt(char *name)
>
> return -ENODEV;
> }
> +
> +/**
> + * dfu_write_from_mem_addr - this function adds support for writing data
> + * starting from fixed memory address (like $loadaddr)
> + * to dfu managed medium (e.g. NAND, MMC)
I think it's better to start with a direct one-line description, like:
+ * dfu_write_from_mem_addr - wite data to dfu-managed medium
+ *
+ * More detail goes here ...
> + *
> + * @param dfu - dfu entity to which we want to store data
> + * @param buf - fixed memory addres from where data starts
> + * @param size - number of bytes to write
> + *
> + * @return - 0 on success, other value on failure
> + */
> +int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size)
const void *buf? My understanding is that this buffer is not changed.
> +{
> + unsigned long dfu_buf_size, write, left = size;
> + int i, ret = 0;
> + void *dp = buf;
> +
> + /*
> + * Here we must call dfu_get_buf(dfu) first to be sure that dfu_buf_size
> + * has been properly initialized - e.g. if "dfu_bufsiz" has been taken
> + * into account.
> + */
> + dfu_get_buf(dfu);
> + dfu_buf_size = dfu_get_buf_size();
> + debug("%s: dfu buf size: %lu\n", __func__, dfu_buf_size);
> +
> + for (i = 0; left > 0; i++) {
> + write = min(dfu_buf_size, left);
> +
> + debug("%s: dp: 0x%p left: %lu write: %lu\n", __func__,
> + dp, left, write);
> + ret = dfu_write(dfu, dp, write, i);
> + if (ret) {
> + error("DFU write failed\n");
> + return ret;
> + }
> +
> + dp += write;
> + left -= write;
> + }
> +
> + ret = dfu_flush(dfu, NULL, 0, i);
> + if (ret)
> + error("DFU flush failed!");
> +
> + return ret;
> +}
> diff --git a/include/dfu.h b/include/dfu.h
> index 7f78cc2..e624c41 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -162,6 +162,7 @@ bool dfu_usb_get_reset(void);
> int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
> int dfu_write(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
> int dfu_flush(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
Please can you put your function comment here in the header?
> +int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size);
> /* Device specific */
> #ifdef CONFIG_DFU_MMC
> extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s);
> --
> 2.1.4
>
Regards,
Simon
More information about the U-Boot
mailing list