diff mbox

[U-Boot] usb:composite:fix Provide function data when addressing device with only one interface

Message ID 1362148218-22392-1-git-send-email-l.majewski@samsung.com
State Accepted
Delegated to: Marek Vasut
Headers show

Commit Message

Łukasz Majewski March 1, 2013, 2:30 p.m. UTC
This commit fixes problems with some non-standard requests send with
device address instead of interface address (bmRequestType.Receipent field).

This happens with dfu-util (debian version: 0.5), which address non standard
requests (like w_value=0x21 and bRequest=GET_DESCRIPTOR) to device.
Without this fix, the above request is STALLED, and hence causes dfu-util
to assume some standard configuration (packet size = 1024B instead of 4096B)
In turn it displays following errors:
Error obtaining DFU functional descriptor
Warning: Assuming DFU version 1.0
Warning: Transfer size can not be detected
...
Warning: Trying default transfer size 1024

This fix allows passing non-standard request to function setup code, where
it shall be handled.

Tested at: 	Trats (exynos4210)
Tested with:DFU and UMS gadgets

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
This patch fixes problem pointed by Pantelis Antoniou.
It is supposed to replace following commit added to u-boot-usb repository:

"dfu: Send correct DFU response from composite_setup"
SHA1: fef31049fa06e516945114195eb14ac3549d0ad2
---
 drivers/usb/gadget/composite.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Marek Vasut March 1, 2013, 2:34 p.m. UTC | #1
Dear Lukasz Majewski,

> This commit fixes problems with some non-standard requests send with
> device address instead of interface address (bmRequestType.Receipent
> field).
> 
> This happens with dfu-util (debian version: 0.5), which address non
> standard requests (like w_value=0x21 and bRequest=GET_DESCRIPTOR) to
> device. Without this fix, the above request is STALLED, and hence causes
> dfu-util to assume some standard configuration (packet size = 1024B
> instead of 4096B) In turn it displays following errors:
> Error obtaining DFU functional descriptor
> Warning: Assuming DFU version 1.0
> Warning: Transfer size can not be detected
> ...
> Warning: Trying default transfer size 1024
> 
> This fix allows passing non-standard request to function setup code, where
> it shall be handled.
> 
> Tested at: 	Trats (exynos4210)
> Tested with:DFU and UMS gadgets

Can you please repost all the patches you want applied on u-boot-usb ? I'm 
completely lost in all this DFU stuff.

Best regards,
Marek Vasut
Łukasz Majewski March 1, 2013, 3:34 p.m. UTC | #2
Hi Marek,

> Dear Lukasz Majewski,
> 
> > This commit fixes problems with some non-standard requests send with
> > device address instead of interface address (bmRequestType.Receipent
> > field).
> > 
> > This happens with dfu-util (debian version: 0.5), which address non
> > standard requests (like w_value=0x21 and bRequest=GET_DESCRIPTOR) to
> > device. Without this fix, the above request is STALLED, and hence
> > causes dfu-util to assume some standard configuration (packet size
> > = 1024B instead of 4096B) In turn it displays following errors:
> > Error obtaining DFU functional descriptor
> > Warning: Assuming DFU version 1.0
> > Warning: Transfer size can not be detected
> > ...
> > Warning: Trying default transfer size 1024
> > 
> > This fix allows passing non-standard request to function setup
> > code, where it shall be handled.
> > 
> > Tested at: 	Trats (exynos4210)
> > Tested with:DFU and UMS gadgets
> 
> Can you please repost all the patches you want applied on
> u-boot-usb ? I'm completely lost in all this DFU stuff.

This is only one patch to replace patch done by Pantelis:

Patch to replace (revert):
dfu: Send correct DFU response from composite_setup
SHA1: fef31049fa06e516945114195eb14ac3549d0ad2

And this shall be replaced with posted patch.

Another patches (with UMS gadget support - done by Piotr Wilczek), also
shall be added to u-boot-usb tree:

http://patchwork.ozlabs.org/patch/219744/
http://patchwork.ozlabs.org/patch/219746/
http://patchwork.ozlabs.org/patch/219745/



> 
> Best regards,
> Marek Vasut
Tom Rini March 1, 2013, 4:46 p.m. UTC | #3
On Fri, Mar 01, 2013 at 03:30:18PM +0100, Lukasz Majewski wrote:

> This commit fixes problems with some non-standard requests send with
> device address instead of interface address (bmRequestType.Receipent field).
> 
> This happens with dfu-util (debian version: 0.5), which address non standard
> requests (like w_value=0x21 and bRequest=GET_DESCRIPTOR) to device.
> Without this fix, the above request is STALLED, and hence causes dfu-util
> to assume some standard configuration (packet size = 1024B instead of 4096B)
> In turn it displays following errors:
> Error obtaining DFU functional descriptor
> Warning: Assuming DFU version 1.0
> Warning: Transfer size can not be detected
> ...
> Warning: Trying default transfer size 1024
> 
> This fix allows passing non-standard request to function setup code, where
> it shall be handled.
> 
> Tested at: 	Trats (exynos4210)
> Tested with:DFU and UMS gadgets
> 
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Tested-by: Tom Rini <trini@ti.com>
on am335x_evm with this patch replacing Pantelis' patch, in the stack of
things required for DFU on this board.
Marek Vasut March 5, 2013, 11:24 p.m. UTC | #4
Dear Lukasz Majewski,

> Hi Marek,
> 
> > Dear Lukasz Majewski,
> > 
> > > This commit fixes problems with some non-standard requests send with
> > > device address instead of interface address (bmRequestType.Receipent
> > > field).
> > > 
> > > This happens with dfu-util (debian version: 0.5), which address non
> > > standard requests (like w_value=0x21 and bRequest=GET_DESCRIPTOR) to
> > > device. Without this fix, the above request is STALLED, and hence
> > > causes dfu-util to assume some standard configuration (packet size
> > > = 1024B instead of 4096B) In turn it displays following errors:
> > > Error obtaining DFU functional descriptor
> > > Warning: Assuming DFU version 1.0
> > > Warning: Transfer size can not be detected
> > > ...
> > > Warning: Trying default transfer size 1024
> > > 
> > > This fix allows passing non-standard request to function setup
> > > code, where it shall be handled.
> > > 
> > > Tested at: 	Trats (exynos4210)
> > > Tested with:DFU and UMS gadgets
> > 
> > Can you please repost all the patches you want applied on
> > u-boot-usb ? I'm completely lost in all this DFU stuff.
> 
> This is only one patch to replace patch done by Pantelis:
> 
> Patch to replace (revert):
> dfu: Send correct DFU response from composite_setup
> SHA1: fef31049fa06e516945114195eb14ac3549d0ad2
> 
> And this shall be replaced with posted patch.
> 
> Another patches (with UMS gadget support - done by Piotr Wilczek), also
> shall be added to u-boot-usb tree:
> 
> http://patchwork.ozlabs.org/patch/219744/
> http://patchwork.ozlabs.org/patch/219746/
> http://patchwork.ozlabs.org/patch/219745/

Ok, please check the u-boot-usb ... if it's OK, I will send Tom a pullrq 
(finally). Tom, do you still accept those for .04 ?

Best regards,
Marek Vasut
Tom Rini March 5, 2013, 11:44 p.m. UTC | #5
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/05/2013 06:24 PM, Marek Vasut wrote:
> Dear Lukasz Majewski,
> 
>> Hi Marek,
>> 
>>> Dear Lukasz Majewski,
>>> 
>>>> This commit fixes problems with some non-standard requests
>>>> send with device address instead of interface address
>>>> (bmRequestType.Receipent field).
>>>> 
>>>> This happens with dfu-util (debian version: 0.5), which
>>>> address non standard requests (like w_value=0x21 and
>>>> bRequest=GET_DESCRIPTOR) to device. Without this fix, the
>>>> above request is STALLED, and hence causes dfu-util to assume
>>>> some standard configuration (packet size = 1024B instead of
>>>> 4096B) In turn it displays following errors: Error obtaining
>>>> DFU functional descriptor Warning: Assuming DFU version 1.0 
>>>> Warning: Transfer size can not be detected ... Warning:
>>>> Trying default transfer size 1024
>>>> 
>>>> This fix allows passing non-standard request to function
>>>> setup code, where it shall be handled.
>>>> 
>>>> Tested at: 	Trats (exynos4210) Tested with:DFU and UMS
>>>> gadgets
>>> 
>>> Can you please repost all the patches you want applied on 
>>> u-boot-usb ? I'm completely lost in all this DFU stuff.
>> 
>> This is only one patch to replace patch done by Pantelis:
>> 
>> Patch to replace (revert): dfu: Send correct DFU response from
>> composite_setup SHA1: fef31049fa06e516945114195eb14ac3549d0ad2
>> 
>> And this shall be replaced with posted patch.
>> 
>> Another patches (with UMS gadget support - done by Piotr
>> Wilczek), also shall be added to u-boot-usb tree:
>> 
>> http://patchwork.ozlabs.org/patch/219744/ 
>> http://patchwork.ozlabs.org/patch/219746/ 
>> http://patchwork.ozlabs.org/patch/219745/
> 
> Ok, please check the u-boot-usb ... if it's OK, I will send Tom a
> pullrq (finally). Tom, do you still accept those for .04 ?

As of earlier in the day it still had Pantelis' patch for sending
files larger than memory which breaks all filesystem-based writes, so
that needs to go.  Lukasz and I are discussing how to deal with that.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRNoNSAAoJENk4IS6UOR1WnDkP+QE7ihcrFz0u5v2wAhkjwtaZ
etpl6oL/rB7jA6jK9zDYkyoc/Bf9A9/ffxsfGruP8kzjK2BTwHizs/Hz4I8Z90k6
6g4S7DUY4kx9t20Iyyswwuo8Bs0MXuEyDSS1a3kWXsB1110eUXd6xBsQoOKv5BeK
1UO29ZFMRJAnIpKBSrrGNE0e8xG8fnF1y8/Wos8OAk240DKoMPTF1lZEB5TEkbtN
7xePk4XLGClePj8CH8iIBj2YDo5kjRPUCZ2zSd6niayVrp5On5as+Fn9XTvWiQYG
UjLDbeAhNinzRJx9q8sB2dxXq2sSh+8ehtKZgnQCwOuj0bV0Ok4um0FP7DKN1Z+K
COvJdKC629rZ9cyc/kLfXtJzEYBJwK+OsLjLHOf7YuuhQpcPcugZWmdc8LZt6QMA
aL61JGTCfF80PVO8jAjWkJjniTHTSQRL+Adt91wej9D8e246/SbzFMqjjpTWQvVB
RcR6fUwKdcMwT+RKAJybYH8z3tqwiRQXleBPs6NqcRYV7E8AHzpeMUh5v7m2Gz/P
8y+ea4v8Nglq79UnYmeqCHmvMO0ZqUpb8/+hB35xfkyuIHstx3J784NHOaQ7IZqj
iJZ/Stf3De+ASddDFeAldoyc3EdT2k98gYUKpitExTOB5EbWKrK781JOEC0q+iSb
+xUyux8npF6Rhhf55Rm7
=DAuM
-----END PGP SIGNATURE-----
Marek Vasut March 5, 2013, 11:54 p.m. UTC | #6
Dear Tom Rini,

> On 03/05/2013 06:24 PM, Marek Vasut wrote:
> > Dear Lukasz Majewski,
> > 
> >> Hi Marek,
> >> 
> >>> Dear Lukasz Majewski,
> >>> 
> >>>> This commit fixes problems with some non-standard requests
> >>>> send with device address instead of interface address
> >>>> (bmRequestType.Receipent field).
> >>>> 
> >>>> This happens with dfu-util (debian version: 0.5), which
> >>>> address non standard requests (like w_value=0x21 and
> >>>> bRequest=GET_DESCRIPTOR) to device. Without this fix, the
> >>>> above request is STALLED, and hence causes dfu-util to assume
> >>>> some standard configuration (packet size = 1024B instead of
> >>>> 4096B) In turn it displays following errors: Error obtaining
> >>>> DFU functional descriptor Warning: Assuming DFU version 1.0
> >>>> Warning: Transfer size can not be detected ... Warning:
> >>>> Trying default transfer size 1024
> >>>> 
> >>>> This fix allows passing non-standard request to function
> >>>> setup code, where it shall be handled.
> >>>> 
> >>>> Tested at: 	Trats (exynos4210) Tested with:DFU and UMS
> >>>> gadgets
> >>> 
> >>> Can you please repost all the patches you want applied on
> >>> u-boot-usb ? I'm completely lost in all this DFU stuff.
> >> 
> >> This is only one patch to replace patch done by Pantelis:
> >> 
> >> Patch to replace (revert): dfu: Send correct DFU response from
> >> composite_setup SHA1: fef31049fa06e516945114195eb14ac3549d0ad2
> >> 
> >> And this shall be replaced with posted patch.
> >> 
> >> Another patches (with UMS gadget support - done by Piotr
> >> Wilczek), also shall be added to u-boot-usb tree:
> >> 
> >> http://patchwork.ozlabs.org/patch/219744/
> >> http://patchwork.ozlabs.org/patch/219746/
> >> http://patchwork.ozlabs.org/patch/219745/
> > 
> > Ok, please check the u-boot-usb ... if it's OK, I will send Tom a
> > pullrq (finally). Tom, do you still accept those for .04 ?
> 
> As of earlier in the day it still had Pantelis' patch for sending
> files larger than memory which breaks all filesystem-based writes, so
> that needs to go.  Lukasz and I are discussing how to deal with that.

IIRC I killed that patch.

Best regards,
Marek Vasut
Tom Rini March 6, 2013, 12:26 a.m. UTC | #7
On Wed, Mar 06, 2013 at 12:54:45AM +0100, Marek Vasut wrote:
> Dear Tom Rini,
> 
> > On 03/05/2013 06:24 PM, Marek Vasut wrote:
> > > Dear Lukasz Majewski,
> > > 
> > >> Hi Marek,
> > >> 
> > >>> Dear Lukasz Majewski,
> > >>> 
> > >>>> This commit fixes problems with some non-standard requests
> > >>>> send with device address instead of interface address
> > >>>> (bmRequestType.Receipent field).
> > >>>> 
> > >>>> This happens with dfu-util (debian version: 0.5), which
> > >>>> address non standard requests (like w_value=0x21 and
> > >>>> bRequest=GET_DESCRIPTOR) to device. Without this fix, the
> > >>>> above request is STALLED, and hence causes dfu-util to assume
> > >>>> some standard configuration (packet size = 1024B instead of
> > >>>> 4096B) In turn it displays following errors: Error obtaining
> > >>>> DFU functional descriptor Warning: Assuming DFU version 1.0
> > >>>> Warning: Transfer size can not be detected ... Warning:
> > >>>> Trying default transfer size 1024
> > >>>> 
> > >>>> This fix allows passing non-standard request to function
> > >>>> setup code, where it shall be handled.
> > >>>> 
> > >>>> Tested at: 	Trats (exynos4210) Tested with:DFU and UMS
> > >>>> gadgets
> > >>> 
> > >>> Can you please repost all the patches you want applied on
> > >>> u-boot-usb ? I'm completely lost in all this DFU stuff.
> > >> 
> > >> This is only one patch to replace patch done by Pantelis:
> > >> 
> > >> Patch to replace (revert): dfu: Send correct DFU response from
> > >> composite_setup SHA1: fef31049fa06e516945114195eb14ac3549d0ad2
> > >> 
> > >> And this shall be replaced with posted patch.
> > >> 
> > >> Another patches (with UMS gadget support - done by Piotr
> > >> Wilczek), also shall be added to u-boot-usb tree:
> > >> 
> > >> http://patchwork.ozlabs.org/patch/219744/
> > >> http://patchwork.ozlabs.org/patch/219746/
> > >> http://patchwork.ozlabs.org/patch/219745/
> > > 
> > > Ok, please check the u-boot-usb ... if it's OK, I will send Tom a
> > > pullrq (finally). Tom, do you still accept those for .04 ?
> > 
> > As of earlier in the day it still had Pantelis' patch for sending
> > files larger than memory which breaks all filesystem-based writes, so
> > that needs to go.  Lukasz and I are discussing how to deal with that.
> 
> IIRC I killed that patch.

In u-boot-usb/master currently:
commit a09f955db6ad7031bd715a615fb8b90d671a14d4
Author: Pantelis Antoniou <panto@antoniou-consulting.com>
Date:   Fri Nov 30 08:01:12 2012 +0000

    dfu: Support larger than memory transfers.

Is one of the two that needed to go.  The patch in this thread replaced
the other one that you did drop.
Marek Vasut March 6, 2013, 12:55 a.m. UTC | #8
Dear Tom Rini,

> On Wed, Mar 06, 2013 at 12:54:45AM +0100, Marek Vasut wrote:
> > Dear Tom Rini,
> > 
> > > On 03/05/2013 06:24 PM, Marek Vasut wrote:
> > > > Dear Lukasz Majewski,
> > > > 
> > > >> Hi Marek,
> > > >> 
> > > >>> Dear Lukasz Majewski,
> > > >>> 
> > > >>>> This commit fixes problems with some non-standard requests
> > > >>>> send with device address instead of interface address
> > > >>>> (bmRequestType.Receipent field).
> > > >>>> 
> > > >>>> This happens with dfu-util (debian version: 0.5), which
> > > >>>> address non standard requests (like w_value=0x21 and
> > > >>>> bRequest=GET_DESCRIPTOR) to device. Without this fix, the
> > > >>>> above request is STALLED, and hence causes dfu-util to assume
> > > >>>> some standard configuration (packet size = 1024B instead of
> > > >>>> 4096B) In turn it displays following errors: Error obtaining
> > > >>>> DFU functional descriptor Warning: Assuming DFU version 1.0
> > > >>>> Warning: Transfer size can not be detected ... Warning:
> > > >>>> Trying default transfer size 1024
> > > >>>> 
> > > >>>> This fix allows passing non-standard request to function
> > > >>>> setup code, where it shall be handled.
> > > >>>> 
> > > >>>> Tested at: 	Trats (exynos4210) Tested with:DFU and UMS
> > > >>>> gadgets
> > > >>> 
> > > >>> Can you please repost all the patches you want applied on
> > > >>> u-boot-usb ? I'm completely lost in all this DFU stuff.
> > > >> 
> > > >> This is only one patch to replace patch done by Pantelis:
> > > >> 
> > > >> Patch to replace (revert): dfu: Send correct DFU response from
> > > >> composite_setup SHA1: fef31049fa06e516945114195eb14ac3549d0ad2
> > > >> 
> > > >> And this shall be replaced with posted patch.
> > > >> 
> > > >> Another patches (with UMS gadget support - done by Piotr
> > > >> Wilczek), also shall be added to u-boot-usb tree:
> > > >> 
> > > >> http://patchwork.ozlabs.org/patch/219744/
> > > >> http://patchwork.ozlabs.org/patch/219746/
> > > >> http://patchwork.ozlabs.org/patch/219745/
> > > > 
> > > > Ok, please check the u-boot-usb ... if it's OK, I will send Tom a
> > > > pullrq (finally). Tom, do you still accept those for .04 ?
> > > 
> > > As of earlier in the day it still had Pantelis' patch for sending
> > > files larger than memory which breaks all filesystem-based writes, so
> > > that needs to go.  Lukasz and I are discussing how to deal with that.
> > 
> > IIRC I killed that patch.
> 
> In u-boot-usb/master currently:
> commit a09f955db6ad7031bd715a615fb8b90d671a14d4
> Author: Pantelis Antoniou <panto@antoniou-consulting.com>
> Date:   Fri Nov 30 08:01:12 2012 +0000
> 
>     dfu: Support larger than memory transfers.
> 
> Is one of the two that needed to go.  The patch in this thread replaced
> the other one that you did drop.

True, we now have the proper one applied to my understanding.

Best regards,
Marek Vasut
Tom Rini March 6, 2013, 1:38 a.m. UTC | #9
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/05/2013 07:55 PM, Marek Vasut wrote:
> Dear Tom Rini,
> 
>> On Wed, Mar 06, 2013 at 12:54:45AM +0100, Marek Vasut wrote:
>>> Dear Tom Rini,
>>> 
>>>> On 03/05/2013 06:24 PM, Marek Vasut wrote:
>>>>> Dear Lukasz Majewski,
>>>>> 
>>>>>> Hi Marek,
>>>>>> 
>>>>>>> Dear Lukasz Majewski,
>>>>>>> 
>>>>>>>> This commit fixes problems with some non-standard
>>>>>>>> requests send with device address instead of
>>>>>>>> interface address (bmRequestType.Receipent field).
>>>>>>>> 
>>>>>>>> This happens with dfu-util (debian version: 0.5),
>>>>>>>> which address non standard requests (like
>>>>>>>> w_value=0x21 and bRequest=GET_DESCRIPTOR) to device.
>>>>>>>> Without this fix, the above request is STALLED, and
>>>>>>>> hence causes dfu-util to assume some standard
>>>>>>>> configuration (packet size = 1024B instead of 4096B)
>>>>>>>> In turn it displays following errors: Error
>>>>>>>> obtaining DFU functional descriptor Warning: Assuming
>>>>>>>> DFU version 1.0 Warning: Transfer size can not be
>>>>>>>> detected ... Warning: Trying default transfer size
>>>>>>>> 1024
>>>>>>>> 
>>>>>>>> This fix allows passing non-standard request to
>>>>>>>> function setup code, where it shall be handled.
>>>>>>>> 
>>>>>>>> Tested at: 	Trats (exynos4210) Tested with:DFU and
>>>>>>>> UMS gadgets
>>>>>>> 
>>>>>>> Can you please repost all the patches you want applied
>>>>>>> on u-boot-usb ? I'm completely lost in all this DFU
>>>>>>> stuff.
>>>>>> 
>>>>>> This is only one patch to replace patch done by
>>>>>> Pantelis:
>>>>>> 
>>>>>> Patch to replace (revert): dfu: Send correct DFU response
>>>>>> from composite_setup SHA1:
>>>>>> fef31049fa06e516945114195eb14ac3549d0ad2
>>>>>> 
>>>>>> And this shall be replaced with posted patch.
>>>>>> 
>>>>>> Another patches (with UMS gadget support - done by Piotr 
>>>>>> Wilczek), also shall be added to u-boot-usb tree:
>>>>>> 
>>>>>> http://patchwork.ozlabs.org/patch/219744/ 
>>>>>> http://patchwork.ozlabs.org/patch/219746/ 
>>>>>> http://patchwork.ozlabs.org/patch/219745/
>>>>> 
>>>>> Ok, please check the u-boot-usb ... if it's OK, I will send
>>>>> Tom a pullrq (finally). Tom, do you still accept those for
>>>>> .04 ?
>>>> 
>>>> As of earlier in the day it still had Pantelis' patch for
>>>> sending files larger than memory which breaks all
>>>> filesystem-based writes, so that needs to go.  Lukasz and I
>>>> are discussing how to deal with that.
>>> 
>>> IIRC I killed that patch.
>> 
>> In u-boot-usb/master currently: commit
>> a09f955db6ad7031bd715a615fb8b90d671a14d4 Author: Pantelis
>> Antoniou <panto@antoniou-consulting.com> Date:   Fri Nov 30
>> 08:01:12 2012 +0000
>> 
>> dfu: Support larger than memory transfers.
>> 
>> Is one of the two that needed to go.  The patch in this thread
>> replaced the other one that you did drop.
> 
> True, we now have the proper one applied to my understanding.

No, the concept needs re-working as non-filesystem can work in chunks
but files must be done all at once.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRNp4EAAoJENk4IS6UOR1W61YQAJutQ0MUjAY5uw/P7YrY9o5C
KPiuxLrTgaTEl6cNeeGWS30YxqX7cz7w/SQd8igKno5sqeoNncggc0CwkAumpxRe
baBxb1VVePijS9NFoQC/zMMzJ8+H4Csx9OLYW1EqlYL30DDpNqS7T2aYznQ7hsth
wyOA7oasGm5KY6tgsi8e8Qfw/dNER//OWgCx+6KqsUJemg5MPSZCABAKQmtNz6yU
jgaXC/z5bx3bfrAa/yf8zaNnbtMLhjcM8Bwhd3oiOsaFgU0eGS/IFJe6gDK9Gpfw
NrehYEBmA3xKXROi7ph8c4yHUaPqA62nAPO0s60uTOQ+K6rXeh5eZ1OPe5Y99raG
Ka0Dx+GT2zT4PJcMTML8xWcF5o7ugD3kG8YbVyWM8OfT7deuZscvdsHaL6k7KOtM
pVrM/lfurHs2wgR4wCLHXCxoRYioHHl4ZgSFTI0RwX/fTphYokX3hloVEMlbRLtR
2QTknQIq7lwRvAOem0CrFczTfVMXtawZ9UDQ6qQ/jgIyQbfyUSyywsTWs2Ts8WSL
PTfOgRrTK9cXk5PCT+pKVv1160mYUCu7c0YYCImqmq9T2qw5QvACpc9FXoSpo/9n
JJ9tt5awX+2nZhrVGqc+d+cfM957xqMJ2ViD3M7Tw8wwy/076FOBvtDFbvA73pV+
HHh8svg50YA8O0dI9n5F
=s8vQ
-----END PGP SIGNATURE-----
Łukasz Majewski March 6, 2013, 7:42 a.m. UTC | #10
Dear All,

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 03/05/2013 06:24 PM, Marek Vasut wrote:
> > Dear Lukasz Majewski,
> > 
> >> Hi Marek,
> >> 
> >>> Dear Lukasz Majewski,
> >>> 
> >>>> This commit fixes problems with some non-standard requests
> >>>> send with device address instead of interface address
> >>>> (bmRequestType.Receipent field).
> >>>> 
> >>>> This happens with dfu-util (debian version: 0.5), which
> >>>> address non standard requests (like w_value=0x21 and
> >>>> bRequest=GET_DESCRIPTOR) to device. Without this fix, the
> >>>> above request is STALLED, and hence causes dfu-util to assume
> >>>> some standard configuration (packet size = 1024B instead of
> >>>> 4096B) In turn it displays following errors: Error obtaining
> >>>> DFU functional descriptor Warning: Assuming DFU version 1.0 
> >>>> Warning: Transfer size can not be detected ... Warning:
> >>>> Trying default transfer size 1024
> >>>> 
> >>>> This fix allows passing non-standard request to function
> >>>> setup code, where it shall be handled.
> >>>> 
> >>>> Tested at: 	Trats (exynos4210) Tested with:DFU and UMS
> >>>> gadgets
> >>> 
> >>> Can you please repost all the patches you want applied on 
> >>> u-boot-usb ? I'm completely lost in all this DFU stuff.
> >> 
> >> This is only one patch to replace patch done by Pantelis:
> >> 
> >> Patch to replace (revert): dfu: Send correct DFU response from
> >> composite_setup SHA1: fef31049fa06e516945114195eb14ac3549d0ad2
> >> 
> >> And this shall be replaced with posted patch.
> >> 
> >> Another patches (with UMS gadget support - done by Piotr
> >> Wilczek), also shall be added to u-boot-usb tree:
> >> 
> >> http://patchwork.ozlabs.org/patch/219744/ 
> >> http://patchwork.ozlabs.org/patch/219746/ 
> >> http://patchwork.ozlabs.org/patch/219745/
> > 
> > Ok, please check the u-boot-usb ... if it's OK, I will send Tom a
> > pullrq (finally). Tom, do you still accept those for .04 ?

Marek, thanks for pulling those patches.

> 
> As of earlier in the day it still had Pantelis' patch for sending
> files larger than memory which breaks all filesystem-based writes, so
> that needs to go.  Lukasz and I are discussing how to deal with that.
> 

The patch about which Tom is speaking is:
"dfu: Support larger than memory transfers."
SHA1: a09f955db6ad7031bd715a615fb8b90d671a14d4

This patch is causing regression (not only on Trats).

I think, that it shall be dropped and replaced by new patch on which
Tom (with my little help) is working now.

This patch is not connected to the one, which you have already removed
from u-boot-usb tree.
You have removed patch about composite.c DFU fix (which was fixed by:
"usb:composite:fix Provide function data when addressing device with
only one interface").



> - -- 
> Tom
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
> 
> iQIcBAEBAgAGBQJRNoNSAAoJENk4IS6UOR1WnDkP+QE7ihcrFz0u5v2wAhkjwtaZ
> etpl6oL/rB7jA6jK9zDYkyoc/Bf9A9/ffxsfGruP8kzjK2BTwHizs/Hz4I8Z90k6
> 6g4S7DUY4kx9t20Iyyswwuo8Bs0MXuEyDSS1a3kWXsB1110eUXd6xBsQoOKv5BeK
> 1UO29ZFMRJAnIpKBSrrGNE0e8xG8fnF1y8/Wos8OAk240DKoMPTF1lZEB5TEkbtN
> 7xePk4XLGClePj8CH8iIBj2YDo5kjRPUCZ2zSd6niayVrp5On5as+Fn9XTvWiQYG
> UjLDbeAhNinzRJx9q8sB2dxXq2sSh+8ehtKZgnQCwOuj0bV0Ok4um0FP7DKN1Z+K
> COvJdKC629rZ9cyc/kLfXtJzEYBJwK+OsLjLHOf7YuuhQpcPcugZWmdc8LZt6QMA
> aL61JGTCfF80PVO8jAjWkJjniTHTSQRL+Adt91wej9D8e246/SbzFMqjjpTWQvVB
> RcR6fUwKdcMwT+RKAJybYH8z3tqwiRQXleBPs6NqcRYV7E8AHzpeMUh5v7m2Gz/P
> 8y+ea4v8Nglq79UnYmeqCHmvMO0ZqUpb8/+hB35xfkyuIHstx3J784NHOaQ7IZqj
> iJZ/Stf3De+ASddDFeAldoyc3EdT2k98gYUKpitExTOB5EbWKrK781JOEC0q+iSb
> +xUyux8npF6Rhhf55Rm7
> =DAuM
> -----END PGP SIGNATURE-----
Pantelis Antoniou March 6, 2013, 7:45 a.m. UTC | #11
Hi Lukasz,

On Mar 6, 2013, at 9:42 AM, Lukasz Majewski wrote:

> Dear All,
> 
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>> 
>> On 03/05/2013 06:24 PM, Marek Vasut wrote:
>>> Dear Lukasz Majewski,
>>> 
>>>> Hi Marek,
>>>> 
>>>>> Dear Lukasz Majewski,
>>>>> 
>>>>>> This commit fixes problems with some non-standard requests
>>>>>> send with device address instead of interface address
>>>>>> (bmRequestType.Receipent field).
>>>>>> 
>>>>>> This happens with dfu-util (debian version: 0.5), which
>>>>>> address non standard requests (like w_value=0x21 and
>>>>>> bRequest=GET_DESCRIPTOR) to device. Without this fix, the
>>>>>> above request is STALLED, and hence causes dfu-util to assume
>>>>>> some standard configuration (packet size = 1024B instead of
>>>>>> 4096B) In turn it displays following errors: Error obtaining
>>>>>> DFU functional descriptor Warning: Assuming DFU version 1.0 
>>>>>> Warning: Transfer size can not be detected ... Warning:
>>>>>> Trying default transfer size 1024
>>>>>> 
>>>>>> This fix allows passing non-standard request to function
>>>>>> setup code, where it shall be handled.
>>>>>> 
>>>>>> Tested at: 	Trats (exynos4210) Tested with:DFU and UMS
>>>>>> gadgets
>>>>> 
>>>>> Can you please repost all the patches you want applied on 
>>>>> u-boot-usb ? I'm completely lost in all this DFU stuff.
>>>> 
>>>> This is only one patch to replace patch done by Pantelis:
>>>> 
>>>> Patch to replace (revert): dfu: Send correct DFU response from
>>>> composite_setup SHA1: fef31049fa06e516945114195eb14ac3549d0ad2
>>>> 
>>>> And this shall be replaced with posted patch.
>>>> 
>>>> Another patches (with UMS gadget support - done by Piotr
>>>> Wilczek), also shall be added to u-boot-usb tree:
>>>> 
>>>> http://patchwork.ozlabs.org/patch/219744/ 
>>>> http://patchwork.ozlabs.org/patch/219746/ 
>>>> http://patchwork.ozlabs.org/patch/219745/
>>> 
>>> Ok, please check the u-boot-usb ... if it's OK, I will send Tom a
>>> pullrq (finally). Tom, do you still accept those for .04 ?
> 
> Marek, thanks for pulling those patches.
> 
>> 
>> As of earlier in the day it still had Pantelis' patch for sending
>> files larger than memory which breaks all filesystem-based writes, so
>> that needs to go.  Lukasz and I are discussing how to deal with that.
>> 
> 
> The patch about which Tom is speaking is:
> "dfu: Support larger than memory transfers."
> SHA1: a09f955db6ad7031bd715a615fb8b90d671a14d4
> 
> This patch is causing regression (not only on Trats).
> 
> I think, that it shall be dropped and replaced by new patch on which
> Tom (with my little help) is working now.
> 
> This patch is not connected to the one, which you have already removed
> from u-boot-usb tree.
> You have removed patch about composite.c DFU fix (which was fixed by:
> "usb:composite:fix Provide function data when addressing device with
> only one interface").
> 

Sorry about missing this thread. I've been busy with other stuff.

What kind of regressions are we talking about?

I'm trying to get around to looking into all of this, but this shouldn't
be causing all that trouble.

Regards

-- Pantelis


> 
> 
>> - -- 
>> Tom
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1.4.11 (GNU/Linux)
>> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>> 
>> iQIcBAEBAgAGBQJRNoNSAAoJENk4IS6UOR1WnDkP+QE7ihcrFz0u5v2wAhkjwtaZ
>> etpl6oL/rB7jA6jK9zDYkyoc/Bf9A9/ffxsfGruP8kzjK2BTwHizs/Hz4I8Z90k6
>> 6g4S7DUY4kx9t20Iyyswwuo8Bs0MXuEyDSS1a3kWXsB1110eUXd6xBsQoOKv5BeK
>> 1UO29ZFMRJAnIpKBSrrGNE0e8xG8fnF1y8/Wos8OAk240DKoMPTF1lZEB5TEkbtN
>> 7xePk4XLGClePj8CH8iIBj2YDo5kjRPUCZ2zSd6niayVrp5On5as+Fn9XTvWiQYG
>> UjLDbeAhNinzRJx9q8sB2dxXq2sSh+8ehtKZgnQCwOuj0bV0Ok4um0FP7DKN1Z+K
>> COvJdKC629rZ9cyc/kLfXtJzEYBJwK+OsLjLHOf7YuuhQpcPcugZWmdc8LZt6QMA
>> aL61JGTCfF80PVO8jAjWkJjniTHTSQRL+Adt91wej9D8e246/SbzFMqjjpTWQvVB
>> RcR6fUwKdcMwT+RKAJybYH8z3tqwiRQXleBPs6NqcRYV7E8AHzpeMUh5v7m2Gz/P
>> 8y+ea4v8Nglq79UnYmeqCHmvMO0ZqUpb8/+hB35xfkyuIHstx3J784NHOaQ7IZqj
>> iJZ/Stf3De+ASddDFeAldoyc3EdT2k98gYUKpitExTOB5EbWKrK781JOEC0q+iSb
>> +xUyux8npF6Rhhf55Rm7
>> =DAuM
>> -----END PGP SIGNATURE-----
> 
> 
> 
> -- 
> Best regards,
> 
> Lukasz Majewski
> 
> Samsung R&D Poland (SRPOL) | Linux Platform Group
Tom Rini March 6, 2013, 1:03 p.m. UTC | #12
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/06/2013 02:45 AM, Pantelis Antoniou wrote:

[snip]
>> Sorry about missing this thread. I've been busy with other
>> stuff.
> 
>> What kind of regressions are we talking about?
> 
>> I'm trying to get around to looking into all of this, but this
>> shouldn't be causing all that trouble.

There's two problems.  The first big problem is that file writes
become impossible as we don't support appending to files at an offset
today.  The second problem is that Lukasz is seeing a regression with
regards to raw block operations (I think akin to how we have to
account for bad blocks, on eMMC we have to deal with lba block sizes).

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRNz6dAAoJENk4IS6UOR1WsuAQAKZ+tXycZbgIvwa2QnKpU11g
eaZiD0JcWUbi1xa1Novy4qLxgnzz+x1cMFE2GtExWywPbrF5K+tdgvPp2H6O04aS
PUfw1EdgoFMANkEHfuQx7NjDiS1ye25Ik6SFvVCRxUjprRJ7nyOxRWpzvMpLU3zb
al8ByraUiGhciz73tcT2wKBQNAfuwcauW5omJgwSVdhMfxG25HD7DOv+CiBFugs1
g6Gawls5lr4e9C7dSAmI8H8TbC99YbxZ5eBYQbkYWwbH2kd6/7VYfMJQHeqAsgcu
Qb/gpXt3pFHgq+zQz0k4RdbN1m1WXcp1o72PV3sKA4ElNsnUzRc1ddJ4NLL+UsfS
mVVYkLIK46nnoKsOlEP/g4T0BLi0R0DNkBg4TZ/l2EDRG3MOkpT9OwCcxkYW5Rs1
3PVsajYaMOmYk4LLHK33hY6Xe0cQv8ieQ1VM45djlGdkGX5PjxRyrE82Ewm0jwyS
y1B0+naXlvCe15A7eFmYZyjDN/WndRECaAWshyXjHGThXhzoCULQcSUqhItzCPWG
50zcFi4GZz2KRLHOoJkl3AwC4ItMUAIAbIeWR5k2Vh4jQYJc5oFKAQu6pQxT9NEG
IQdks3nRkHylXENR5lStTm4lmI6uuSgfOnALOft5vJCDPPlY7Mkb+Jjoy2v4oFMR
WpxIAgeJ/4fJrYWUo9kv
=9/Lh
-----END PGP SIGNATURE-----
Tormod Volden June 27, 2013, 9:36 p.m. UTC | #13
Lukasz Majewski writes:
> 
> This commit fixes problems with some non-standard requests send with
> device address instead of interface address (bmRequestType.Receipent field).
> 
> This happens with dfu-util (debian version: 0.5), which address non standard
> requests (like w_value=0x21 and bRequest=GET_DESCRIPTOR) to device.


Dear u-boot developers,

The above comment seems to imply that dfu-util is sending non-standard
requests. For the record, that would be wrong. dfu-util is sending a
standard "get descriptor" request. These always go to the device, not to an
interface. The descriptor it is asking for is, although not mandatory per
the core USB standard, a class-specific descriptor which is part of the DFU
1.0 standard. So this is all standard.

While on this topic, I would recommend that you include this descriptor in
the configuration descriptor set, that is, in the complete configuration
descriptor that the host OS usually requests during enumeration. Also this
class-specific descriptor belongs here. In that case, the OS and libusb will
keep a copy of this descriptor, and dfu-util will not have to send an
explicit request (through libusb_get_descriptor()) to retrieve it.


> Without this fix, the above request is STALLED, and hence causes dfu-util
> to assume some standard configuration (packet size = 1024B instead of 4096B)

Note that this number is the "Maximum number of bytes that the device can
accept per control-write transaction."

The DFU transactions should work fine if a smaller packet size is used - as
long as it is larger or equal to the bMaxPacketSize0. Of course, smaller
packets will cause more overhead and be less efficient, but you must make
sure that your device can handle this. I am not sure from these comments
whether the smaller packet size caused errors in your implementation, or if
the problem was just reduced performance.

Best regards,
Tormod Volden
Łukasz Majewski July 1, 2013, 6:28 a.m. UTC | #14
On Thu, 27 Jun 2013 21:36:09 +0000 (UTC), Tormod Volden wrote:

Hi Tormod,

> Lukasz Majewski writes:
> > 
> > This commit fixes problems with some non-standard requests send with
> > device address instead of interface address
> > (bmRequestType.Receipent field).
> > 
> > This happens with dfu-util (debian version: 0.5), which address non
> > standard requests (like w_value=0x21 and bRequest=GET_DESCRIPTOR)
> > to device.
> 
> 
> Dear u-boot developers,
> 
> The above comment seems to imply that dfu-util is sending non-standard
> requests. For the record, that would be wrong. dfu-util is sending a
> standard "get descriptor" request. These always go to the device, not
> to an interface. The descriptor it is asking for is, although not
> mandatory per the core USB standard, a class-specific descriptor
> which is part of the DFU 1.0 standard. So this is all standard.
> 
> While on this topic, I would recommend that you include this
> descriptor in the configuration descriptor set, that is, in the
> complete configuration descriptor that the host OS usually requests
> during enumeration. Also this class-specific descriptor belongs here.
> In that case, the OS and libusb will keep a copy of this descriptor,
> and dfu-util will not have to send an explicit request (through
> libusb_get_descriptor()) to retrieve it.

Thanks for explanation. The commit about which we are discussing was a
(reasonable) fix for the problem.  

I need to look into this and came up with proper solution.

> 
> 
> > Without this fix, the above request is STALLED, and hence causes
> > dfu-util to assume some standard configuration (packet size = 1024B
> > instead of 4096B)
> 
> Note that this number is the "Maximum number of bytes that the device
> can accept per control-write transaction."

I need to check this, but AFAIR there was a problem with dfu-util (and
probably libusb under the hood) version 0.1 (shipped with "ancient"
old stable debian) and 0.5 which is currently in use.

> 
> The DFU transactions should work fine if a smaller packet size is
> used - as long as it is larger or equal to the bMaxPacketSize0. Of
> course, smaller packets will cause more overhead and be less
> efficient, but you must make sure that your device can handle this. I
> am not sure from these comments whether the smaller packet size
> caused errors in your implementation, or if the problem was just
> reduced performance.

The change was driven by the need to improve performance, when
downloading large files (with roofts) and to silent warning on the
dfu-util side.

> 
> Best regards,
> Tormod Volden
> 
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
diff mbox

Patch

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index ebb5131..2c5600e 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -859,6 +859,25 @@  unknown:
 			if (&f->list == &cdev->config->functions)
 				f = NULL;
 			break;
+		/*
+		 * dfu-util (version 0.5) sets bmRequestType.Receipent = Device
+		 * for non-standard request (w_value = 0x21,
+		 * bRequest = GET_DESCRIPTOR in this case).
+		 * When only one interface is registered (as it is done now),
+		 * then this request shall be handled as it was requested for
+		 * interface.
+		 *
+		 * In the below code it is checked if only one interface is
+		 * present and proper function for it is extracted. Due to that
+		 * function's setup (f->setup) is called to handle this
+		 * special non-standard request.
+		 */
+		case USB_RECIP_DEVICE:
+			debug("cdev->config->next_interface_id: %d intf: %d\n",
+			       cdev->config->next_interface_id, intf);
+			if (cdev->config->next_interface_id == 1)
+				f = cdev->config->interface[intf];
+			break;
 		}
 
 		if (f && f->setup)