diff mbox

[U-Boot] dfu: using dfu-util -l shows different output

Message ID 51C97DEF.90705@denx.de
State RFC
Headers show

Commit Message

Heiko Schocher June 25, 2013, 11:24 a.m. UTC 
Hello Lukasz,

Am 25.06.2013 12:29, schrieb Lukasz Majewski:
> Hi Heiko,
> 
>> Hello Lukasz,
>>
>> Am 25.06.2013 10:44, schrieb Lukasz Majewski:
>>> Hi Heiko,
>>>
>>>> Hello Pantelis,
>>>>
>>>> Am 25.06.2013 10:16, schrieb Pantelis Antoniou:
>>>>> Heiko,
>>>>>
>>>>> I don't think the gadget is initialized before you issue
>>>>> a dfu call.
>>>>>
>>>>> So that makes sense.
>>>>
>>>> ?
>>>>
>>>> I call from the host "dfu-util -l" so the gadget on the board
>>>> should do the answer ... or?
>>>
>>> The gadget is not initialized here (so the dfu-util -l shows no
>>> output).
>>>
>>> After transfer it shows information about proper alt settings.
>>>
>>> This is correct behaviour, but I had discussion about this with Tom
>>> and we agreed, that it would be nice to have "dfu-util -l"
>>> exporting from very beginning the alt setting information.
>>
>> Yes, I vote for this too. Connecting to a board, I first
>> would do a "dfu-util -l" to look, what is possible to
>> update here ...
>>
>>> Frankly, I had too much other work to implement it.
>>>
>>> However, I think, that it would be a very useful feature (e.g. to
>>> get alt settings layout at HOST to facilitate automated flashing).
>>
>> Hmm... I digged into the code, but not really found a place
>> where I have to start. Can you give me a hint where to start?
>> So maybe, I can do it?
> 
> The problem here is to switch:
>   f_dfu->usb_function.strings = dfu_strings | NULL;
>   f_dfu->usb_function.hs_descriptors = f_dfu->function |
>   	dfu_runtime_descs;
> 
> The trick here is to properly fill in hs USB descriptors to embed the
> alt settings strings.
> 
> 
> dfu-util -l can read the alt settings when following patch is applied:
> 
> @@ -655,6 +656,7 @@ static int dfu_bind(struct usb_configuration *c,
> struct usb_function *f) 
>         stringtab_dfu.strings = f_dfu->strings;
>  
> +       to_dfu_mode(f_dfu);
>         cdev->req->context = f_dfu;
>  
>  error:
> 
> Unfortunately this breaks normal dfu operation, since board hangs at
> "dfu mode".

What do you mean with "normal dfu operation" ?

I just tried the following patch, and "dfu-util -l" and
"dfu-util -a ... -D ..." works ...


log on the host, where I use dfu-util:

after restart of the board:

[root@ts8 ~]# dfu-util -l
dfu-util 0.5

(C) 2005-2008 by Weston Schmidt, Harald Welte and OpenMoko Inc.
(C) 2010-2011 Tormod Volden (DfuSe support)
This program is Free Software and has ABSOLUTELY NO WARRANTY

dfu-util does currently only support DFU version 1.0

Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=0, name="SPL"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=1, name="SPL.backup1"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=2, name="SPL.backup2"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=3, name="SPL.backup3"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=4, name="u-boot"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=5, name="kernel_a"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=6, name="kernel_b"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=7, name="rootfs"
[root@ts8 ~]# dfu-util -a rootfs -D dxr2-org/dxr2.xx-release-image-UNKNOWN-dxr2.ubi
dfu-util 0.5

(C) 2005-2008 by Weston Schmidt, Harald Welte and OpenMoko Inc.
(C) 2010-2011 Tormod Volden (DfuSe support)
This program is Free Software and has ABSOLUTELY NO WARRANTY

dfu-util does currently only support DFU version 1.0

Opening DFU USB device... ID 0525:4a47
WARNING: Can not find cached DFU functional descriptor
Warning: Assuming DFU version 1.0
Run-time device DFU version 0100
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=7, name="rootfs"
Claiming USB DFU Interface...
Setting Alternate Setting #7 ...
Determining device status: state = dfuIDLE, status = 0
dfuIDLE, continuing
Error obtaining cached DFU functional descriptor
DFU mode device DFU version 0110
Device returned transfer size 4096
No valid DFU suffix signature
Warning: File has no DFU suffix
bytes_per_hash=576716
Copying data from PC to DFU device
Starting download: [##################################################] finished!
state(2) = dfuIDLE, status(0) = No error condition is present
Done!
[root@ts8 ~]# dfu-util -l
dfu-util 0.5

(C) 2005-2008 by Weston Schmidt, Harald Welte and OpenMoko Inc.
(C) 2010-2011 Tormod Volden (DfuSe support)
This program is Free Software and has ABSOLUTELY NO WARRANTY

dfu-util does currently only support DFU version 1.0

Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=0, name="SPL"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=1, name="SPL.backup1"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=2, name="SPL.backup2"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=3, name="SPL.backup3"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=4, name="u-boot"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=5, name="kernel_a"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=6, name="kernel_b"
Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=7, name="rootfs"
[root@ts8 ~]#

seems to me, that we should add

"f_dfu->dfu_state = DFU_STATE_dfuIDLE"
into to_dfu_mode() as, after all places where to_dfu_mode() is called
f_dfu->dfu_state = DFU_STATE_dfuIDLE is set ...

bye,
Heiko

Comments

Łukasz Majewski June 25, 2013, 11:35 a.m. UTC | #1 
Hi Heiko,

> Hello Lukasz,
> 
> Am 25.06.2013 12:29, schrieb Lukasz Majewski:
> > Hi Heiko,
> > 
> >> Hello Lukasz,
> >>
> >> Am 25.06.2013 10:44, schrieb Lukasz Majewski:
> >>> Hi Heiko,
> >>>
> >>>> Hello Pantelis,
> >>>>
> >>>> Am 25.06.2013 10:16, schrieb Pantelis Antoniou:
> >>>>> Heiko,
> >>>>>
> >>>>> I don't think the gadget is initialized before you issue
> >>>>> a dfu call.
> >>>>>
> >>>>> So that makes sense.
> >>>>
> >>>> ?
> >>>>
> >>>> I call from the host "dfu-util -l" so the gadget on the board
> >>>> should do the answer ... or?
> >>>
> >>> The gadget is not initialized here (so the dfu-util -l shows no
> >>> output).
> >>>
> >>> After transfer it shows information about proper alt settings.
> >>>
> >>> This is correct behaviour, but I had discussion about this with
> >>> Tom and we agreed, that it would be nice to have "dfu-util -l"
> >>> exporting from very beginning the alt setting information.
> >>
> >> Yes, I vote for this too. Connecting to a board, I first
> >> would do a "dfu-util -l" to look, what is possible to
> >> update here ...
> >>
> >>> Frankly, I had too much other work to implement it.
> >>>
> >>> However, I think, that it would be a very useful feature (e.g. to
> >>> get alt settings layout at HOST to facilitate automated flashing).
> >>
> >> Hmm... I digged into the code, but not really found a place
> >> where I have to start. Can you give me a hint where to start?
> >> So maybe, I can do it?
> > 
> > The problem here is to switch:
> >   f_dfu->usb_function.strings = dfu_strings | NULL;
> >   f_dfu->usb_function.hs_descriptors = f_dfu->function |
> >   	dfu_runtime_descs;
> > 
> > The trick here is to properly fill in hs USB descriptors to embed
> > the alt settings strings.
> > 
> > 
> > dfu-util -l can read the alt settings when following patch is
> > applied:
> > 
> > @@ -655,6 +656,7 @@ static int dfu_bind(struct usb_configuration *c,
> > struct usb_function *f) 
> >         stringtab_dfu.strings = f_dfu->strings;
> >  
> > +       to_dfu_mode(f_dfu);
> >         cdev->req->context = f_dfu;
> >  
> >  error:
> > 
> > Unfortunately this breaks normal dfu operation, since board hangs at
> > "dfu mode".
> 
> What do you mean with "normal dfu operation" ?
> 
> I just tried the following patch, and "dfu-util -l" and
> "dfu-util -a ... -D ..." works ...
> 
> diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
> index a322ae5..0334d9f 100644
> --- a/drivers/usb/gadget/f_dfu.c
> +++ b/drivers/usb/gadget/f_dfu.c
> @@ -653,6 +653,9 @@ static int dfu_bind(struct usb_configuration *c,
> struct usb_function *f) ->iInterface = id;
>         }
> 
> +       to_dfu_mode(f_dfu);
> +       f_dfu->dfu_state = DFU_STATE_dfuIDLE;
		^^^^^ = this is what I was missing.

> +
>         stringtab_dfu.strings = f_dfu->strings;
> 
>         cdev->req->context = f_dfu;
> 
> log on the host, where I use dfu-util:
> 
> after restart of the board:
> 
> [root@ts8 ~]# dfu-util -l
> dfu-util 0.5
> 
> (C) 2005-2008 by Weston Schmidt, Harald Welte and OpenMoko Inc.
> (C) 2010-2011 Tormod Volden (DfuSe support)
> This program is Free Software and has ABSOLUTELY NO WARRANTY
> 
> dfu-util does currently only support DFU version 1.0
> 
> Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=0, name="SPL"
> Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=1,
> name="SPL.backup1" Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0,
> alt=2, name="SPL.backup2" Found DFU: [0525:4a47] devnum=0, cfg=2,
> intf=0, alt=3, name="SPL.backup3" Found DFU: [0525:4a47] devnum=0,
> cfg=2, intf=0, alt=4, name="u-boot" Found DFU: [0525:4a47] devnum=0,
> cfg=2, intf=0, alt=5, name="kernel_a" Found DFU: [0525:4a47]
> devnum=0, cfg=2, intf=0, alt=6, name="kernel_b" Found DFU:
> [0525:4a47] devnum=0, cfg=2, intf=0, alt=7, name="rootfs" [root@ts8
> ~]# dfu-util -a rootfs -D
> dxr2-org/dxr2.xx-release-image-UNKNOWN-dxr2.ubi dfu-util 0.5
> 
> (C) 2005-2008 by Weston Schmidt, Harald Welte and OpenMoko Inc.
> (C) 2010-2011 Tormod Volden (DfuSe support)
> This program is Free Software and has ABSOLUTELY NO WARRANTY
> 
> dfu-util does currently only support DFU version 1.0
> 
> Opening DFU USB device... ID 0525:4a47
> WARNING: Can not find cached DFU functional descriptor
> Warning: Assuming DFU version 1.0
> Run-time device DFU version 0100
> Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=7, name="rootfs"
> Claiming USB DFU Interface...
> Setting Alternate Setting #7 ...
> Determining device status: state = dfuIDLE, status = 0
> dfuIDLE, continuing
> Error obtaining cached DFU functional descriptor
> DFU mode device DFU version 0110
> Device returned transfer size 4096
> No valid DFU suffix signature
> Warning: File has no DFU suffix
> bytes_per_hash=576716
> Copying data from PC to DFU device
> Starting download:
> [##################################################] finished!
> state(2) = dfuIDLE, status(0) = No error condition is present Done!
> [root@ts8 ~]# dfu-util -l
> dfu-util 0.5
> 
> (C) 2005-2008 by Weston Schmidt, Harald Welte and OpenMoko Inc.
> (C) 2010-2011 Tormod Volden (DfuSe support)
> This program is Free Software and has ABSOLUTELY NO WARRANTY
> 
> dfu-util does currently only support DFU version 1.0
> 
> Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=0, name="SPL"
> Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0, alt=1,
> name="SPL.backup1" Found DFU: [0525:4a47] devnum=0, cfg=2, intf=0,
> alt=2, name="SPL.backup2" Found DFU: [0525:4a47] devnum=0, cfg=2,
> intf=0, alt=3, name="SPL.backup3" Found DFU: [0525:4a47] devnum=0,
> cfg=2, intf=0, alt=4, name="u-boot" Found DFU: [0525:4a47] devnum=0,
> cfg=2, intf=0, alt=5, name="kernel_a" Found DFU: [0525:4a47]
> devnum=0, cfg=2, intf=0, alt=6, name="kernel_b" Found DFU:
> [0525:4a47] devnum=0, cfg=2, intf=0, alt=7, name="rootfs" [root@ts8
> ~]#
> 
> seems to me, that we should add
> 
> "f_dfu->dfu_state = DFU_STATE_dfuIDLE"
> into to_dfu_mode() as, after all places where to_dfu_mode() is called
> f_dfu->dfu_state = DFU_STATE_dfuIDLE is set ...

Yes, indeed. This seems as a good solution. 

> 
> bye,
> Heiko
diff mbox

Patch

diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
index a322ae5..0334d9f 100644
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -653,6 +653,9 @@  static int dfu_bind(struct usb_configuration *c, struct usb_function *f)
                        ->iInterface = id;
        }

+       to_dfu_mode(f_dfu);
+       f_dfu->dfu_state = DFU_STATE_dfuIDLE;
+
        stringtab_dfu.strings = f_dfu->strings;

        cdev->req->context = f_dfu;