[PATCH] Look for DFU functional descriptor in the configuration descriptors

Tormod Volden lists.tormod at gmail.com
Mon Nov 8 22:58:46 CET 2010


On Mon, Nov 8, 2010 at 12:21 PM, Stefan Schmidt wrote:
> First of all thanks for your patches to dfu-util. I'm not reading the
> openmoko-devel list that often anymore so a explicit cc might help getting me to
> see the patches earlier.
>

Hi Stefan,

Thanks for the fast response and a very helpful review. I will repost
the patches (with cc) addressing all the issues you have raised. Until
then, just a few quick comments.

> I prefer a commit message starting with the area or source file it touches to
> give the commit message a context.
>
> e.g. main: Look for DFU functional descriptor in the configuration descriptors

Yes, this is sometimes a difficult balance. I will try to follow your
preference.

>> Hi, I am looking at the dfu-util code and trying to make it work with
>> some devices I have (under Linux).
>
> What devices do you work with? I would like to compile a list with devices
> dfu-util can work with or has known problems.

I am actually working on enhancing dfu-util to support some
*cough*ST*cough* extensions. The protocol is different but there is so
much common code it would be stupid not to keep it together. I have
got it working, and the new code is mostly in a new file, but I would
like to work on it more before I suggest it for merging. There are
also parts of dfu-util (dfu suffix decoding, protocol detection etc) I
would like to have improved before my stuff can be cleanly integrated.

However, the present patches are free-standing from that work. From
the current dfu-util code it seems that the functional descriptor
retrieval issue is seen on other devices as well.

> I can life with this workaround. And if anybody is interested in porting it over
> to libusb 1.0 let me know. :)

I would (over longer term) be interested in this. I think too many
applications are based on the stagnated 0.1 version. The dfu-util code
should be relatively easy to port.

> +     int ret = 0;

> Do we really need the initial set to 0 here?

> +
> +     conflen = (usb_device(udev))->config->wTotalLength;
> +     cbuf = malloc(conflen);
> +     conflen = usb_get_descriptor(udev, USB_DT_CONFIG, index, cbuf, conflen);
> +     while (p + 1 < conflen) {

I was thinking of the possibility of usb_get_descriptor returning less
than 2, in which case the "while" block is not entered where ret is
set. Of course, in that case we probably have a bigger problem and
should bail out. I admit this is "compact" coding.

Tormod



More information about the devel mailing list