Hi Heiko,
I'm also CCing Pantelis to the loop.
> Hello Marek,
>
> Am 12.03.2014 12:43, schrieb Marek Vasut:
> > On Wednesday, March 12, 2014 at 11:01:19 AM, Heiko Schocher wrote:
> >> move the flushing code into an extra function dfu_flush(),
> >> so it can be used from other code.
> >>
> >> Signed-off-by: Heiko Schocher<h...@denx.de>
> >> Cc: Lukasz Majewski<l.majew...@samsung.com>
> >> Cc: Kyungmin Park<kyungmin.p...@samsung.com>
> >> Cc: Marek Vasut<ma...@denx.de>
> >> ---
> >> drivers/dfu/dfu.c | 46
> >> ++++++++++++++++++++++++++-------------------- include/dfu.h
> >> | 1 + 2 files changed, 27 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> >> index 56e69fd..193e047 100644
> >> --- a/drivers/dfu/dfu.c
> >> +++ b/drivers/dfu/dfu.c
> >> @@ -127,6 +127,31 @@ static int dfu_write_buffer_drain(struct
> >> dfu_entity *dfu) return ret;
> >> }
> >>
> >> +int dfu_flush(struct dfu_entity *dfu, void *buf, int size, int
> >> blk_seq_num) +{
> >> + int ret = 0;
> >> +
> >> + /* end? */
> >
> > What does this comment mean ? I don't understand it ...
>
> Comes from original code... Thinking about it, it seems better to
> let this comment and the below "if" in the dfu_write function...
This comment doesn't bring any meaningful information, so can be
removed.
>
> >> + if (size == 0) {
> >
> > You can check this like so:
> >
> > if (size)
> > return;
>
> ... as when moving this "if" back to dfu_write(), this "if" kindly
> disappears in the patch 2/3 of this series, as calling dfu_flush()
> is only in the new dfuMANIFEST state necessary, which is called at
> the end of the dfu transfer, so no need for checking, if end of
> size is reached!
So, in my opinion, this if can be safely removed.
>
> >
> > code
> > code
> > code
> >
> > This would cut down the indent, no ?
>
> Yes. Did the above change and this "if" is no longer necessary ;-)
>
> I will wait for comments from Lukasz before posting v2.
>
> > Please let's fix this when this popped up, either in separate patch
> > or wrap it into this one.
> >
> >> + /* Now try and flush to the medium if needed. */
> >> + if (dfu->flush_medium)
> >> + ret = dfu->flush_medium(dfu);
> >> + printf("\nDFU complete CRC32: 0x%08x\n",
> >> dfu->crc); +
> >> + /* clear everything */
> >> + dfu_free_buf();
> >> + dfu->crc = 0;
> >> + dfu->offset = 0;
> >> + dfu->i_blk_seq_num = 0;
> >> + dfu->i_buf_start = dfu_buf;
> >> + dfu->i_buf_end = dfu_buf;
> >> + dfu->i_buf = dfu->i_buf_start;
> >> +
> >> + dfu->inited = 0;
> >> + }
> >> + return ret;
> >> +}
> > [...]
>
> Thanks!
>
> bye,
> Heiko
BTW: Thanks for proper extension of the DFU state machine in u-boot :-).
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
