[PATCH 4/4 dfu-util] main: Always retrieve the DFU functional descriptor

[Stefan Schmidt]

Hello.

On Sun, 2011-09-18 at 11:20, Tormod Volden wrote:
> On Sat, Sep 17, 2011 at 6:40 PM, Stefan Schmidt wrote:
> >
> > Sadly this seems not to be the case. I gave this one a spin and it
> > broke several devices for me. :(
> >
> > I don't have time to debug this the next days but should hopefully be
> > able to do it end of next week. So I applied all but this patch from
> > the series.
> 
> Thanks for testing! Interesting. So they do not provide a functional
> descriptor at all? Can you please send me the sudo lsusb -v output
> from these devices? Do you have access to their firmware code?

lsusb output is attched.

Its the Openmoko Freerunner again. Source for the DFU u-boot
implementation is available but device owners are not likely to
upgrade their bootloaders in the field. Smeels like we need to add
another quirk for this device.

Having this fixed in the U-boot implemtation is needed as well. I'm
working on getting a fully working DFU implementation into U-boot
mainline, but so far I'm only testing it with a beagleboard as
Freerunner support in mainline U-boot is pretty poor. Maybe we can
bring this back later...

The u-boot source with dfu for the freerunner can be found here
(Yeah, a bit of strange git usage in this repo...):
http://git.openmoko.org/?p=u-boot.git;a=shortlog;h=refs/heads/mokopatches

As for the second version of the patch I gave it a fast test and it
still fails here:

Opening DFU USB device... ID 1d50:5119
Warning: failed to retrieve complete configuration descriptor
Error obtaining DFU functional descriptor
Warning: Assuming DFU version 1.0
Run-time device DFU version 100
Claiming USB DFU Runtime Interface...
Cannot set alt interface

You don't have the Openmoko Freerunner so I need to dig into this
myself. I'm still short on time on doing so. Hopefully at the weekend.

> >> The program banner announces it only supports DFU 1.0 but the fact is
> >> that it uses a few 1.1-only features. We should try to cover both in
> >> the long run, and I think we should also follow the standard strictly,
> >> and rather add device-specific quirks if needed.
> >
> > I agree that we should support both in the long term. Could you
> > refresh my mind what 1.1 features we are using currently? I was under
> > the impression we don't.
> 
> "Features" was maybe an exaggeration. For example, we use values
> introduced in 1.1 to see if the device is in run-time or DFU mode.
> This usually goes fine anyway, because they will be thought to be
> run-time which is most often the case for a "cold" device.
> Additionally the state is verified by dfu_getstate requests and the
> code traps the misconception and recovers. But it is clear that this
> was written along the 1.1 spec or reverse-engineering a (partly?) 1.1
> device.

Interesting I thought the bitWillDetach was the only addition in the
1.1 spec. Seems I have to read it again to fresh up knowledge about
it. :)

> On the other hand, I don't see any way to read the run-time/dfu state
> from the 1.0 descriptor set, so there is not much we can do about it
> except making the code acknowledge this incapability and not confuse
> someone reading the code or using it.
> 
> In fact there is not much difference between 1.0 and 1.1 so we can
> safely say we support both. There is the added bitWillDetach in 1.1
> that we might want to incorporate, by extending or replacing the
> final_reset option handling at the end of main(). Our handling of the
> "Manifestation phase" could generally be improved, but it is not
> critical, as long as the actual flash writing is successful and the
> device can be reset by hand. I guess this is a weak point in most
> device implementations as well.

Agreed. Manual reset is not to bad, but we should honor and implement
bitWillDetach later on. Still some other bugs to get fixed before
this. :)

regards
Stefan Schmidt