[U-Boot] A proposal/hack for an efficient USB DFU for linux based boards

[U-Boot] A proposal/hack for an efficient USB DFU for linux based boards

[Krishna Pattabiraman]

Hi All,

I was working with dfu-utill couple of years ago as part of my thesis. My
task was to provide a firmware upgrade mechanism using USB. I had an open
moko so I thought of implementing a solution based on it. The board I used
is SAM9G45 http://www.atmel.com/devices/sam9g45.aspx.

My observation in the current implementation was that, the partitions are
getting listed as DFU capable devices with fixed address and  to change
their size or address, you need to change the u-boot-env. Considering the
requirement I tweaked the protocol and some code to make a fast and
flexible upgrade.

I have explained about it here
http://codelectron.wordpress.com/2014/02/28/flexible-firmware-upgrade/
Header source code here https://github.com/codelectron/DFU-Tweak/

Let me know your comments and feedback about this idea?.

Thanks,
Krishna

[U-Boot] A proposal/hack for an efficient USB DFU for linux based boards

[Lukasz Majewski]

Hi Krishna,

Thanks for attempt to improve DFU.

I've CCed Tormod Volden, who is involved in dfu-util development.

> Hi All,
> 
> I was working with dfu-utill couple of years ago as part of my
> thesis. My task was to provide a firmware upgrade mechanism using
> USB. I had an open moko so I thought of implementing a solution based
> on it. The board I used is SAM9G45
> http://www.atmel.com/devices/sam9g45.aspx.
> 
> My observation in the current implementation was that, the partitions
> are getting listed as DFU capable devices with fixed address and  to
> change their size or address, you need to change the u-boot-env.
> Considering the requirement I tweaked the protocol and some code to
> make a fast and flexible upgrade.
> 
> I have explained about it here
> http://codelectron.wordpress.com/2014/02/28/flexible-firmware-upgrade/
> Header source code here https://github.com/codelectron/DFU-Tweak/
> 
> Let me know your comments and feedback about this idea?.

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.

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.

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).

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.

- 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.


> 
> Thanks,
> Krishna
> 


-- 
Best regards,

Lukasz Majewski

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

[U-Boot] A proposal/hack for an efficient USB DFU for linux based boards

[Krishna Pattabiraman]

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


> 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.


> - 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.


Krishna
www.codelectron.com

[U-Boot] A proposal/hack for an efficient USB DFU for linux based boards

[Lukasz Majewski]

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

[U-Boot] A proposal/hack for an efficient USB DFU for linux based boards

[Krishna Pattabiraman]

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?
>
No not in the code. Yes they are different code but what I am speaking
about in the blog  is the removal of the whole block and removing the
corresponding states in u-boot also. I have also given the reasons for it
in the blog.


>
> 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.
>
Its a difference between a partition table style vs free style. My reason
for choosing the latter was
1. The partition table is actually inside the linux kernel and to change
partition, we need upgrade to u-boot-env as well as Linux kernel. Risk of
bricking was higher.
Like here
http://lxr.free-electrons.com/source/arch/arm/mach-omap2/board-omap3beagle.c?v=2.6.36#L53

2. I used SAM9G45 and for normal booting of Linux and U-boot for upgrade
and other maintanence purpose.
http://free-electrons.com/blog/at91bootstrap-linux/ .

I haven't considered other types of storage devices and boards, but I think
that the concept is very same for other devices too.


>
> I'm happy, that you had shared this fresh view.
>
> Do you have any other ideas for improvements?
>

Yes, When you plug the DFU enabled device to PC as per standard it should
show as Device Firware Updrade in the Device class. Device manager in
windows and any similar appplication in other OS will show that. I haven't
checked the current implementation in U-boot. It can be done by changing
the configuration descriptor in the USB driver. The class is FE and sub
class is 01 you can refer here
http://www.usb.org/developers/defined_class/#BaseClassFEh .

Thank you for your time.

Krishna
www.codelectron.com

[U-Boot] A proposal/hack for an efficient USB DFU for linux based boards

[Tormod Volden]

[Cc'ing the dfu-util list instead of myself, full thread on
http://u-boot.10912.n7.nabble.com/A-proposal-hack-for-an-efficient-USB-DFU-for-linux-based-boards-td175319.html#a175947]

On Fri, Mar 14, 2014 at 11:42 AM, Krishna Pattabiraman wrote:
>
>On Fri, Mar 14, 2014 at 9:52 AM, Lukasz Majewski wrote:
>> 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?

The DEFAULT_POLLTIMEOUT is now applied inside dfu_get_status() so that
is as before. However this default value is only used for a few
devices with explicit quirks applied in dfu-util. For u-boot the
bwPollTimeout value returned by the boot loader is used.

>
> No not in the code. Yes they are different code but what I am speaking about
> in the blog  is the removal of the whole block and removing the
> corresponding states in u-boot also. I have also given the reasons for it in
> the blog.

Then it would not be DFU any longer. The DFU standard mandates a
DFU_GETSTATUS request after the DFU_DNLOAD request, and that the host
should wait bwPollTimeout milliseconds before sending requests to the
device again*. If u-boot does not need any time on its own here, it
should return a zero bwPollTimeout.

*) The standard says "wait between the status phase of the next
DFU_DNLOAD and the subsequent solicitation of the device's status via
DFU_GETSTATUS", however many devices only initiates the actual
flashing/erasing/etc after the DFU_GETSTATUS requests and can
therefore set this value specifically for the current request.

Regards,
Tormod

>
>
>>
>> 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.
>
> Its a difference between a partition table style vs free style. My reason
> for choosing the latter was
> 1. The partition table is actually inside the linux kernel and to change
> partition, we need upgrade to u-boot-env as well as Linux kernel. Risk of
> bricking was higher.
> Like here
> http://lxr.free-electrons.com/source/arch/arm/mach-omap2/board-omap3beagle.c?v=2.6.36#L53
>
> 2. I used SAM9G45 and for normal booting of Linux and U-boot for upgrade and
> other maintanence purpose.
> http://free-electrons.com/blog/at91bootstrap-linux/ .
>
> I haven't considered other types of storage devices and boards, but I think
> that the concept is very same for other devices too.
>
>>
>>
>> I'm happy, that you had shared this fresh view.
>>
>> Do you have any other ideas for improvements?
>
>
> Yes, When you plug the DFU enabled device to PC as per standard it should
> show as Device Firware Updrade in the Device class. Device manager in
> windows and any similar appplication in other OS will show that. I haven't
> checked the current implementation in U-boot. It can be done by changing the
> configuration descriptor in the USB driver. The class is FE and sub class is
> 01 you can refer here
> http://www.usb.org/developers/defined_class/#BaseClassFEh .
>
> Thank you for your time.
>
> Krishna
> www.codelectron.com
>