[PATCH dfu-util] main: Simplify --detach handling by reusing existing code

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH dfu-util] main: Simplify --detach handling by reusing existing code

Tormod Volden
From: Tormod Volden <[hidden email]>

Signed-off-by: Tormod Volden <[hidden email]>
---

Hi,

I have some issues with the recent --detach option addition:
* It duplicates the existing detach code
* It sends requests to an interface without claiming it first
* It resets the device without checking bitWillDetach

Can't we simply do it like this, use existing code instead?
I do not have any devices that do the mode transition correctly,
so I can not verify that it works.

If the main motivation for this option is to inquire about devices
which only show alternate interfaces after switching to DFU mode,
then an alternative would be to list those if the user has not
specified one of them with the -a option. This way we may avoid
adding the -e option altogether. Patch to follow.

Regards,
Tormod

 src/main.c |   18 +++++-------------
 1 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/src/main.c b/src/main.c
index 4c96f88..3624069 100644
--- a/src/main.c
+++ b/src/main.c
@@ -804,19 +804,6 @@ int main(int argc, char **argv)
  printf("Run-time device DFU version %04x\n",
        libusb_le16_to_cpu(func_dfu_rt.bcdDFUVersion));
 
- if (mode == MODE_DETACH) {
- if (dfu_detach(_rt_dif.dev_handle, _rt_dif.interface, 1000) < 0) {
- fprintf(stderr, "error detaching\n");
- exit(1);
- }
- libusb_release_interface(_rt_dif.dev_handle, _rt_dif.interface);
- ret = libusb_reset_device(_rt_dif.dev_handle);
- if (ret < 0 && ret != LIBUSB_ERROR_NOT_FOUND)
- fprintf(stderr, "error resetting "
- "after detach\n");
- exit(0);
- }
-
  /* Transition from run-Time mode to DFU mode */
  if (!(_rt_dif.flags & DFU_IFF_DFU)) {
  /* In the 'first round' during runtime mode, there can only be one
@@ -889,6 +876,11 @@ int main(int argc, char **argv)
  _rt_dif.interface);
  libusb_close(_rt_dif.dev_handle);
 
+ if (mode == MODE_DETACH) {
+ libusb_exit(ctx);
+ exit(0);
+ }
+
  /* now we need to re-scan the bus and locate our device */
 // if (usb_find_devices() < 2)
 // printf("not at least 2 device changes found ?!?\n");
--
1.7.5.4


_______________________________________________
devel mailing list
[hidden email]
https://lists.openmoko.org/mailman/listinfo/devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH dfu-util] main: Simplify --detach handling by reusing existing code

Stefan Schmidt-2
Hello.

On Sun, 2011-11-13 at 23:02, Tormod Volden wrote:
> From: Tormod Volden <[hidden email]>
>
> I have some issues with the recent --detach option addition:
> * It duplicates the existing detach code
> * It sends requests to an interface without claiming it first
> * It resets the device without checking bitWillDetach

Valid points in here. Maybe I was a bit to fast adding it. :)

> Can't we simply do it like this, use existing code instead?
> I do not have any devices that do the mode transition correctly,
> so I can not verify that it works.
>
> If the main motivation for this option is to inquire about devices
> which only show alternate interfaces after switching to DFU mode,
> then an alternative would be to list those if the user has not
> specified one of them with the -a option. This way we may avoid
> adding the -e option altogether. Patch to follow.

While listing the alternative interfaces is the major point behinf
this chnage it is also nice to have it around for debugging. Not sure
if it really has much of a value besides that.

I need to think about this for a moment and will test out your
patches.

regards
Stefan Schmidt

_______________________________________________
devel mailing list
[hidden email]
https://lists.openmoko.org/mailman/listinfo/devel