[U-Boot] [PATCH] dfu, usb: set usb descriptor to values defined in dfu spec 1.1

[Lukasz Majewski]

Hi Heiko,

> Hello Marek,
> 
> Am 05.09.2014 00:42, schrieb Marek Vasut:
> > On Thursday, September 04, 2014 at 12:21:40 PM, Heiko Schocher
> > wrote:
> >
> > Hi Heiko,
> >
> > I'll just rant a bit, but please wait until Lukasz does a proper
> > runthrough.
> >
> >> set bDeviceClass, bDeviceSubClass and bcdUSB to the values
> >> defined in dfu spec 1.1 chapter 4.2.1 found here:
> >>
> >> http://www.usb.org/developers/devclass_docs/DFU_1.1.pdf
> >
> > This URL gives me a website with:
> > "
> > Resource not found.
> > Please check the URL and try again.
> > "
> 
> :-( Maybe it is deleted currently there?
> Hmm.. there is a search field if you open the above URL ... and if
> you type DFU and press search, you get:
> 
> DFU
> File Format: PDF/Adobe Acrobat
> Aug 5, 2004 ... USB Device Firmware Upgrade Specification, Revision
> 1.1. 2. Intellectual Property Disclaimer. THIS SPECIFICATION IS
> PROVIDED “AS IS” ... www.usb.org/developers/devclass_docs/DFU_1.1.pdf
> 
> This is exactly the above link ...
> 
> This link is also referenced from openmoko:
> 
> http://wiki.openmoko.org/wiki/USB_DFU_-_The_USB_Device_Firmware_Upgrade_standard

I can confirm that the original link is dead. Maybe we can add above
information to the commit message?

> 
> >> Signed-off-by: Heiko Schocher<hs at denx.de>
> >> Cc: Lukasz Majewski<l.majewski at samsung.com>
> >> Cc: Marek Vasut<marex at denx.de>
> >> Cc: Roger Meier<r.meier at siemens.com>
> >>
> >> ---
> >>
> >> before this patch, "dfu-util -l" showed:
> >>
> >> Bus 002 Device 114: ID 0908:02c5 Siemens AG
> >> Device Descriptor:
> >>    bLength                18
> >>    bDescriptorType         1
> >>    bcdUSB               2.00
> >>    bDeviceClass            2 Communications
> >>    bDeviceSubClass         2 Abstract (modem)
> >>    bDeviceProtocol         0 None
> >>    bMaxPacketSize0        64
> >>    idVendor           0x0908 Siemens AG
> >>    idProduct          0x02c5
> >>    bcdDevice            0.00
> >>    iManufacturer           1 Siemens AG
> >>
> >> with this patch:
> >>
> >> Bus 002 Device 089: ID 0908:02c5 Siemens AG
> >> Device Descriptor:
> >>    bLength                18
> >>    bDescriptorType         1
> >>    bcdUSB               1.10
> >>    bDeviceClass            0 (Defined at Interface level)
> >>    bDeviceSubClass         0
> >>    bDeviceProtocol         0
> >>    bMaxPacketSize0        64
> >>    idVendor           0x0908 Siemens AG
> >>    idProduct          0x02c5
> >>    bcdDevice            0.00
> >>    iManufacturer           1 Siemens AG
> >>    iProduct                2 USB download gadget
> >>
> >> which fits for "bcdUSB", "bDeviceClass" and "bDeviceSubClass" with
> >> the DFU spezifikation.
> >
> > This patch doesn't seem fully right, let me explain below please.
> >
> >> ---
> >>   drivers/usb/gadget/f_dfu.c | 5 +++--
> >>   drivers/usb/gadget/g_dnl.c | 6 +++---
> >>   2 files changed, 6 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/usb/gadget/f_dfu.c
> >> b/drivers/usb/gadget/f_dfu.c index 9128add..9429e50 100644
> >> --- a/drivers/usb/gadget/f_dfu.c
> >> +++ b/drivers/usb/gadget/f_dfu.c
> >> @@ -83,8 +83,9 @@ static struct usb_descriptor_header
> >> *dfu_runtime_descs[] = { static const struct
> >> usb_qualifier_descriptor dev_qualifier = { .bLength
> >> =		sizeof dev_qualifier, .bDescriptorType =
> >> USB_DT_DEVICE_QUALIFIER,
> >> -	.bcdUSB =		__constant_cpu_to_le16(0x0200),
> >> -	.bDeviceClass =		USB_CLASS_VENDOR_SPEC,
> >> +	.bcdUSB =		__constant_cpu_to_le16(0x0110),
> >
> > See [1] and look for first and second instance of 'bcdUsb' there
> > please. Quote of the second one:
> >
> > '
> > The bcdUSB field reports the highest version of USB the device
> > supports. The value is in binary coded decimal with a format of
> > 0xJJMN where JJ is the major version number, M is the minor version
> > number and N is the sub minor version number. e.g. USB 2.0 is
> > reported as 0x0200, USB 1.1 as 0x0110 and USB 1.0 as 0x0100.
> > '
> >
> > So this should be configured dynamically based on what the highest
> > mode the controller supports is, no?
> 
> Good question ... the DFU 1.1 spezification says in chapter
> 4.2.1 DFU Mode Device Descriptor:
> 
> bcdUSB = 0100  USB specification release number in binary coded
> decimal. (which seems wrong, as the spez is version 1.1, see [2])
> bDeviceClass = 0
> bDeviceSubClass = 0
> 
> ... ?
> 
> I discussed this change with Lukasz, see:
> 
> [2] http://lists.denx.de/pipermail/u-boot/2014-August/186918.html
> 

I'm more than happy to announce that you both may be right :-).

In the DFU 1.1 spec the point 4.2.1 describes the DFU MODE DEVICE
DESCRIPTOR. 

According to information from point 4.- "Enumeration Phase":

"It is very important to note that the device exposes two distinct and
independent descriptor sets, one each at the appropriate time:
- Run-time descriptor (the one from [1] pointed out by Marek) 
- DFU mode descriptor (the one from 4.2.1. - fixed by Heiko)

It seems to me that the first descriptor is defined at g_dnl.c and the
second, which I guess should be changed, at f_dfu.c

However, to be 100% sure I need to examine this with USB analizer. I'm
going to setup it today or on Monday.

> >
> >> +	.bDeviceClass =		USB_CLASS_PER_INTERFACE,
> >
> > This might be OK.
> >
> >> +	.bDeviceSubClass =	0x00,
> >
> > But this is not, the device definitelly isn't CDC-serial.
> >
> > [...]
> >
> > [1] http://www.beyondlogic.org/usbnutshell/usb5.shtml
> 
> bye,
> Heiko


-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group