public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Lukasz Majewski <l.majewski@samsung.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] A proposal/hack for an efficient USB DFU for linux based boards
Date: Fri, 14 Mar 2014 09:52:34 +0100	[thread overview]
Message-ID: <20140314095234.476c4479@amdc2363> (raw)
In-Reply-To: <CACYw5mA+TwhJNRWBFk16YDzitMn5GBq-Jr2VEHaQ3qxtqpJFxQ@mail.gmail.com>

Hi Krishna,

> Hi,
> 
> Thanks for your time,
> 
>  Please find following comments:
> 
> >
> > 1. Current solution (u-boot):
> > - It is possible to specify your own "dfu_alt_info" environment
> >   variable when you call dfu command.
> >
> > 2. Speed improvement - section "Making it faster" from:
> > http://codelectron.wordpress.com/2014/02/28/flexible-firmware-upgrade/
> >
> > - I've noticed that you are referring to following dfu-util version:
> > dfu-util - (C) 2007 by Openmoko Inc.
> >
> > It seems to be a bit old one.
> >
> 
> I just used the content from my old documentation, But im referring to
> current implementation.
> 
> 
> > The newest GIT repo can be found at:
> > git://gitorious.org/dfu-util/dfu-util.git
> >
> > I'm referring to following SHA1 (master branch):
> > bda43a52a6c5e9dcd159236553b0d5c511616e77
> >
> > The code at dfuload_do_dnload() function is already rewritten to
> > only wait:
> > milli_sleep(dst.bwPollTimeout);
> >
> > which is correct, since non-zero values are explicitly specified in
> > u-boot (when e.g. target expects that eMMC data will be written).
> >
> > It seems that the proposed improvement is already implemented.
> >
> 
> I dont think its implemented, you can refer here, its the block of
> code what I am referring to,
> 
> https://gitorious.org/dfu-util/dfu-util/source/bda43a52a6c5e9dcd159236553b0d5c511616e77:src/dfu_load.c#L123
> https://gitorious.org/dfu-util/dfu-util/source/bda43a52a6c5e9dcd159236553b0d5c511616e77:src/dfu_load.c#L137

Ok, so there is the current code:

        do {
            ret = dfu_get_status(dif, &dst);
            if (ret < 0) {
                errx(EX_IOERR, "Error during download get_status");
                goto out_free;
            }

            if (dst.bState == DFU_STATE_dfuDNLOAD_IDLE ||
                    dst.bState == DFU_STATE_dfuERROR)
                break;

            /* Wait while device executes flashing */
            milli_sleep(dst.bwPollTimeout);
        } while (1);

And the code you refer in your webpage:
http://codelectron.wordpress.com/2014/02/28/flexible-firmware-upgrade/
	do {	
	ret = dfu_get_status(dif->dev_handle, dif->interface, &dst);
	if (ret < 0) {
		fprintf(stderr, ?Error during download get_status\n?);
		goto out_free;
	}
	if (dst.bState == DFU_STATE_dfuDNLOAD_IDLE ||
		dst.bState == DFU_STATE_dfuERROR)
		break;
	/* Wait while device executes flashing */
	if (quirks & QUIRK_POLLTIMEOUT)
		milli_sleep(DEFAULT_POLLTIMEOUT);
	else
		milli_sleep(dst.bwPollTimeout);
	} while (1);

I'm a bit confused here, since it looks like the
milli_sleep(DEFAULT_POLLTIMEOUT); is already removed.

Am I missing something?

> 
> 
> > As a side note:
> > To improve transmission speed we were even trying to increase the
> > EP0 packet size on HOST machine. Unfortunately it has some
> > limitations (4 KiB if I'm not wrong).
> >
> Yes I do think so about the limitation, as a standard EP0 should
> support atleast 64 bytes and DFU is based on that.
> 
> >
> > Anyway, I'm open for potential DFU transmission speed improvements.
> >
> >
> > 3. Flexibility improvement - section "Making it flexible"
> >
> > The idea of adding headers seems appealing - but there are as usual
> > some corner cases:
> >
> > - Backward compatibility - dfu-util and dfu use number of alt
> > settings to chose the memory region to be written. That is why you
> > observe a lot of alt=X when you type dfu-util -l. We would need
> > some extra switch in the dfu-util to support yours header based
> > approach.
> >
> > - In u-boot we would like to have some degree of control of where
> > and if we flash data. I personally like to specify beforehand (in
> > envs) what parts of memory are available for flashing.
> >
> 
> I developed it keeping in mind a situation in field where there can
> be a complete change in memory organisation.

Maybe some special alt setting could be defined for such a behavior in
u-boot? We can think about that if it solves a real problem.

> 
> 
> > - Some more complicated schemes shall be considered as well. I've
> > got some pending patches to support eMMC's bootparts flashing via
> > DFU.
> >
> > - Also the header itself only assumes NAND being the backing
> > memory. But we now also alter content of eMMC and RAM with DFU.
> > This is missing.
> >
> > - The information provided by headers is already stored at
> >   "dfu_alt_info" environment variable. It is also used by other
> > gadgets.
> >
> 
> It was an example code based on what I had implemented some time
> back. It was based on nand flash, but the idea will be the same for
> any other  type.
> 
> I just wanted to share the idea of a different type of implementation
> of USB DFU.

I'm happy, that you had shared this fresh view.

Do you have any other ideas for improvements?

> 
> 
> Krishna
> www.codelectron.com



-- 
Best regards,

Lukasz Majewski

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

  reply	other threads:[~2014-03-14  8:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-05 12:39 [U-Boot] A proposal/hack for an efficient USB DFU for linux based boards Krishna Pattabiraman
2014-03-06 10:31 ` Lukasz Majewski
2014-03-07 18:56   ` Krishna Pattabiraman
2014-03-14  8:52     ` Lukasz Majewski [this message]
2014-03-14 10:42       ` Krishna Pattabiraman
2014-03-15  8:56         ` Tormod Volden

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140314095234.476c4479@amdc2363 \
    --to=l.majewski@samsung.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox