openmoko dfu-util improvements (patch & questions)

[Sandro Giessl]

Hi,

thanks for your quick responses.

Am Sonntag, 4. April 2010 03:42:17 schrieb Harald Welte:
> On Sat, Apr 03, 2010 at 11:36:37PM +0200, Stefan Schmidt wrote:
> > > DFU_GETSTATUS ignores bwPollTimeout but does hardcoded usleep(5000)
> > > when doing DFU download:
> > > This is a show-stoper for us, since this delay is too short and results
> > > in libusb 'Broken pipe' errors.
> > > A patch implementing proper timeout handling is attached.
> >
> > Nice to see other people using it.
> 
> I sporadically get reports from other people using it for various
> different devices, too.
>
> > > My question is, do you see a chance in getting these things
> > > upstream?  Since some distros are shipping slightly different SVN
> > > revisions, do you plan a real dfu-util release any time soon?
> >
> > As Paul already pointed out the biggest problem is that we don't have
> > a real maintainer for it. I think Harald is way to busy with other
> > things and nobody stepped up to take care of fixing problems going
> > forward to a real release.
> 
> This is correct.  I don't mind bumping the version number and announcing
> something as an official release, if that is something people want me to
> do.  But I don't have all the various devices that people (want to) use
> dfu-util with, and thus it's hard for me to tell what works where.

It's what stated in the header of sam7dfu.c which motivates me to merge 
changes directly into dfu-util, instead doing some quick hacks and creating 
yet another dfu-programmer:
"This is supposed to be a "real" DFU implementation, just as specified in the 
USB DFU 1.0 Spec."

Since it's not feasible to test any device out in the wild, the focus should 
be set on a closer spec conformity + occasional quirks and special handling 
based on idVendor/idProduct/bcdDevice.

As long as it *is* an option to not only continue in OM-bug-fixing mode I'm 
more than happy.

> There are multiple different options:
> 
> 1) separate it from openmoko and create its own repository and
>    mailinglist, run it as a standalone project
> 
> 2) simply giving anyone who's stepping up to maintain it the committ
>    access in the openmoko repository and continue to point to this
>    mailing list for patch submission and discussion

If you really don't have the time I will be glad to look after it 
occasionally.  You may call it maintainership, but I would still prefer to 
have some of the OM developers looking over my shoulder, pointing out possible 
flaws or OM-incompatibilities.
It's up to you... I currently prefer 2) with the emphasis to be open for other 
parties (e.g. also their legacy device specific quirks) interested in DFU, too.

> > The download part is quite stable I think, at least on the Openmoko
> > phones and I already heard from people that it shoudl also detect
> > various BT devices. The last time I tested upload was broken though.
> 
> As far as I remember, this was a problem inside u-boots DFU
> implementation and not the host utility.
> 
> > Hmm, this is no longer only sam7 related. Seems like a historical
> > fragment from the pre-OM days. (Some of the code was already used on
> > the OpenPCD project).
> 
> yes, and somebody should probably test if it still works with openpcd at
> some point before making a release...
> 
> > Aehm, I don't think 2 lines of code change plus three lines comments
> > justify the copyright addition of two people...
> 
> For credits, we can start a CONTRIBUTERS file or something along those
> lines.  But I agree with Stefan. It is even legally wrong: A trivial
> change like this does not constitue a copyrightable work and thus there
> should be no copyright statement of the respective authors

Sure, you are right. Of course there are some bigger changes that need to be 
cleaned up and integrated some time, so I wasn't actually thinking about how 
ridiculous this might look in conjunction with this single usleep diff. :)

Since I can't draw the line myself, commit/maintainer guidelines are 
appreciated. I'm no expert in legal matters, but I believed that such trivial 
changes are well sufficient for example to update the notice of copyrighted 
years?

> > Did you test if bwPollTimeout is really filled from u-boot on the
> > Openmoko phones?
> 
> This is an important question, OM users are probably still our primary
> user base for the program, so we need to check it still works.  I don't
> have any OM phone on my holidays so I cannot test.
> 
> Looking at the usbdfu.c code in openmoko u-boot:
>         /* FIXME: set dstat->bwPollTimeout */
> does not seem promising.
> 
> There are two solutions to this:
> 
> 1) Check if bwPollTimeout == 0, and then revert to the old value of
>    5000 for usleep
> 
> 2) add something like device-specific kludges/fixups/quirks, similar to
>    how many linux kernel drivers handle this.  In this case, we would
>    have a QUIRK_POLL_TIMEOUT_5MS and define this in case we're working
>    on an openmoko device.
> 
> Both solutions are fine with me and I'm happy to apply them to the repo.

The second solution sounds good enough. I agree that OM users are the main 
user base, but since there doesn't exist any open source reference 
implementation for DFU, dfu-util might step in.

I don't have an OM phone for testing, therefore it would be neccessary to 
document its quirks and/or someone verifying that nothing breaks in case of 
openmoko/u-boot.

-- 
┌─────────────────────────────────┐
  niftylight GmbH

  Sandro Gießl
  Lämmerweide 12a
  D-82256 Fürstenfeldbruck
  Germany

  Tel.:    +49-8141-315 99 00
  Telefax: +49-8141-315 99 01
  Mobile:  +49-177-21 33 476
  WWW:     http://niftylight.de
└─────────────────────────────────┘

Geschäftsleitung: D.Hiepler / S.Gießl
Handelsregister: Amtsgericht München - HRB Nr. 175748