[PATCH] quirks: Wait until device is ready after DFU status requests

Stefan Schmidt stefan at datenfreihafen.org
Sun Nov 14 18:19:56 CET 2010


Hello.

On Sun, 2010-11-14 at 01:32, Tormod Volden wrote:
> From: Tormod Volden <debian.tormod at gmail.com>
> 
> DFU devices tell the host in the status request answer for how long to
> wait before another request.
> 
> The previous hardcoded wait of 5 ms while executing the flashing is too
> short for many devices, causing (intermittent) failures.
> 
> The OpenMoko returns invalid (and random) values, so quirks are needed.
> Add a simple quirk system matching vendor/product.

The way you implemented the quirk system is easy enough for the beginning.
Depending on how much of these quirks we need in the future I would think about
changing it in a way that avoid all kind of if (...QUIRK...) checks. But for now
it is fine.

The set_quirks call was done to early. With the following incremental patch it
works reliable for my openmoko devices.

diff --git a/src/main.c b/src/main.c
index 3367a0a..5d14c94 100644
--- a/src/main.c
+++ b/src/main.c
@@ -611,14 +611,14 @@ int main(int argc, char **argv)
                exit(1);
        }
 
-       /* find set of quirks for this device */
-       set_quirks(dif->vendor, dif->product);
-
        /* try to find first DFU interface of device */
        memcpy(&_rt_dif, dif, sizeof(_rt_dif));
        if (!get_first_dfu_if(&_rt_dif))
                exit(1);
 
+       /* find set of quirks for this device */
+       set_quirks(_rt_dif.vendor, _rt_dif.product);
+
        if (!_rt_dif.flags & DFU_IFF_DFU) {

Harald, does the sam7dfu bootloader send out correct values for bwPollTimeout or
does it also rely on a fixed timeout in dfu-util? If it is the later we need to
add the USB IDs to the quirk.

If this patch does not break OpenPCD/OpenPICC I would prepare a 0.2 release next
week.

[Full email kept for Haralds reference]

regards
Stefan Schmidt

> 
>  src/Makefile.am |    3 ++-
>  src/dfu_load.c  |   11 ++++++++++-
>  src/main.c      |   10 ++++++++++
>  src/quirks.c    |   29 +++++++++++++++++++++++++++++
>  src/quirks.h    |   16 ++++++++++++++++
>  5 files changed, 67 insertions(+), 2 deletions(-)
>  create mode 100644 src/quirks.c
>  create mode 100644 src/quirks.h
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index eb1fa94..d844f30 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -12,6 +12,7 @@ dfu_util_SOURCES = main.c \
>  		dfu_load.h \
>  		dfu.c \
>  		dfu.h \
> -		usb_dfu.h
> +		usb_dfu.h \
> +		quirks.c
>  
>  EXTRA_DIST = dfu-version.h
> diff --git a/src/dfu_load.c b/src/dfu_load.c
> index 24f784b..2425d17 100644
> --- a/src/dfu_load.c
> +++ b/src/dfu_load.c
> @@ -34,6 +34,8 @@
>  #include "config.h"
>  #include "dfu.h"
>  #include "usb_dfu.h"
> +#include "dfu_load.h"
> +#include "quirks.h"
>  
>  /* ugly hack for Win32 */
>  #ifndef O_BINARY
> @@ -161,7 +163,12 @@ int dfuload_do_dnload(struct usb_dev_handle *usb_handle, int interface,
>  				fprintf(stderr, "Error during download get_status\n");
>  				goto out_close;
>  			}
> -			usleep(5000);
> +			/* Wait while device executes flashing */
> +			if (quirks & QUIRK_POLLTIMEOUT)
> +				usleep(DEFAULT_POLLTIMEOUT * 1000);
> +			else
> +				usleep(dst.bwPollTimeout * 1000);
> +
>  		} while (dst.bState != DFU_STATE_dfuDNLOAD_IDLE &&
>  			 dst.bState != DFU_STATE_dfuERROR);
>  		if (dst.bStatus != DFU_STATUS_OK) {
> @@ -200,6 +207,8 @@ get_status:
>  	printf("state(%u) = %s, status(%u) = %s\n", dst.bState,
>  		dfu_state_to_string(dst.bState), dst.bStatus,
>  		dfu_status_to_string(dst.bStatus));
> +	if (!(quirks & QUIRK_POLLTIMEOUT))
> +		usleep(dst.bwPollTimeout * 1000);
>  
>  	/* FIXME: deal correctly with ManifestationTolerant=0 / WillDetach bits */
>  	switch (dst.bState) {
> diff --git a/src/main.c b/src/main.c
> index 264b553..3367a0a 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -33,6 +33,7 @@
>  #include "usb_dfu.h"
>  #include "dfu_load.h"
>  #include "dfu-version.h"
> +#include "quirks.h"
>  #ifdef HAVE_CONFIG_H
>  #include "config.h"
>  #endif
> @@ -610,6 +611,9 @@ int main(int argc, char **argv)
>  		exit(1);
>  	}
>  
> +	/* find set of quirks for this device */
> +	set_quirks(dif->vendor, dif->product);
> +
>  	/* try to find first DFU interface of device */
>  	memcpy(&_rt_dif, dif, sizeof(_rt_dif));
>  	if (!get_first_dfu_if(&_rt_dif))
> @@ -643,6 +647,8 @@ int main(int argc, char **argv)
>  		}
>  		printf("state = %s, status = %d\n", 
>  		       dfu_state_to_string(status.bState), status.bStatus);
> +		if (!(quirks & QUIRK_POLLTIMEOUT))
> +			usleep(status.bwPollTimeout * 1000);
>  
>  		switch (status.bState) {
>  		case DFU_STATE_appIDLE:
> @@ -791,6 +797,8 @@ status_again:
>  	}
>  	printf("state = %s, status = %d\n",
>  	       dfu_state_to_string(status.bState), status.bStatus);
> +	if (!(quirks & QUIRK_POLLTIMEOUT))
> +		usleep(status.bwPollTimeout * 1000);
>  
>  	switch (status.bState) {
>  	case DFU_STATE_appIDLE:
> @@ -863,6 +871,8 @@ status_again:
>  			fprintf(stderr, "Error: %d\n", status.bStatus);
>  			exit(1);
>  		}
> +		if (!(quirks & QUIRK_POLLTIMEOUT))
> +			usleep(status.bwPollTimeout * 1000);
>          }
>  
>  	switch (mode) {
> diff --git a/src/quirks.c b/src/quirks.c
> new file mode 100644
> index 0000000..28302d2
> --- /dev/null
> +++ b/src/quirks.c
> @@ -0,0 +1,29 @@
> +/*  Simple quirk system for dfu-util
> + *  Copyright 2010 Tormod Volden
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#include "quirks.h"
> +
> +int quirks = 0;
> +
> +void set_quirks(unsigned long vendor, unsigned long product)
> +{
> +	/* Device returns bogus bwPollTimeout values */
> +	if (vendor == VENDOR_OPENMOKO ||
> +	    vendor == VENDOR_FIC)
> +		quirks |= QUIRK_POLLTIMEOUT;
> +}
> diff --git a/src/quirks.h b/src/quirks.h
> new file mode 100644
> index 0000000..042b19e
> --- /dev/null
> +++ b/src/quirks.h
> @@ -0,0 +1,16 @@
> +#ifndef DFU_QUIRKS_H
> +#define DFU_QUIRKS_H
> +
> +#define VENDOR_OPENMOKO 0x1d50
> +#define VENDOR_FIC      0x1457
> +
> +#define QUIRK_POLLTIMEOUT  (1<<0)
> +
> +/* Fallback value, works for OpenMoko */
> +#define DEFAULT_POLLTIMEOUT  5
> +
> +extern int quirks;
> +
> +void set_quirks(unsigned long vendor, unsigned long product);
> +
> +#endif /* DFU_QUIRKS_H */
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> devel mailing list
> devel at lists.openmoko.org
> https://lists.openmoko.org/mailman/listinfo/devel



More information about the devel mailing list