[U-Boot] [ANN] U-Boot v2017.05-rc2 released

Wed Apr 19 12:07:33 UTC 2017

Dear All,

> On 04/19/2017 12:39 PM, Heiko Schocher wrote:
> > Hello Marek,
> > 
> > Am 19.04.2017 um 11:51 schrieb Marek Vasut:
> >> On 04/19/2017 11:46 AM, Heiko Schocher wrote:
> >>> Hello Marek,
> >>>
> >>> Am 19.04.2017 um 10:43 schrieb Marek Vasut:
> >>>> On 04/19/2017 07:29 AM, Heiko Schocher wrote:
> >>>>> Hello Tom,
> >>>>>
> >>>>> added Lukasz, Marek and Felipe,
> >>>>>
> >>>>> Am 18.04.2017 um 00:22 schrieb Tom Rini:
> >>>>>> Hey all,
> >>>>>>
> >>>>>> It's release day and v2017.05-rc2 is out.  I think my patchwork
> >>>>>> queue is
> >>>>>> looking good currently.  I have some outstanding removal
> >>>>>> patches to take
> >>>>>> from Masahiro related to architectures that I removed as
> >>>>>> promised.  The
> >>>>>> release is bigger than I really wanted, but since I was on
> >>>>>> vacation for
> >>>>>> most of the normal -rc1 window, stuff came in that would have
> >>>>>> come in then now, instead.  Things are on track for -rc3 on
> >>>>>> the 1st.
> >>>>>
> >>>>> My weekly dfu test on the siemens smartweb board failed with
> >>>>> current HEAD.
> >>>>>
> >>>>> I started an automated git bisect with tbot, and found:
> >>>>>
> >>>>> 2017-04-19 07:24:30,717:CON    :tbotlib   # tb_ctrl: git bisect
> >>>>> visualize
> >>>>> 2017-04-19 07:24:30,783:CON    :tbotlib   # tb_ctrl: commit
> >>>>> 842778a091047b0c868efa12229633959f711152
> >>>>> Author: Felipe Balbi <felipe.balbi at linux.intel.com>
> >>>>> Date:   Wed Feb 22 17:12:40 2017 +0200
> >>>>>       usb: gadget: g_dnl: only set iSerialNumber if we have a
> >>>>> serial#
> >>>>>
> >>>>>       We don't want to claim that we support a serial number
> >>>>> string and
> >>>>>       later return nothing. Because of that, if g_dnl_serial is
> >>>>> an empty
> >>>>>       string, let's skip setting iSerialNumber to a valid
> >>>>> number.
> >>>>>
> >>>>>       Signed-off-by: Felipe Balbi <felipe.balbi at linux.intel.com>
> >>>>> hs at pollux [ 7:24:30] ttbott>
> >>>>> 2017-04-19 07:24:31,769:CON    :tbotlib   # tb_ctrl: git bisect
> >>>>> log 2017-04-19 07:24:31,836:CON    :tbotlib   # tb_ctrl: git
> >>>>> bisect start
> >>> [...]
> >>>>>
> >>>>> Any ideas?
> >>>>
> >>>> Is your board setting up the serial number with
> >>>> g_dnl_set_serialnumber()
> >>>> correctly ?
> >>>
> >>> Hmm.. good question ... its done here:
> >>>
> >>> http://git.denx.de/?p=u-boot.git;a=blob;f=board/siemens/common/factoryset.c;h=6c869ed2b035a0e9f840e1f6f960fe0e6ac824e5;hb=f6c1df44b815a08585e7fd3805a1db51a5955d09#l313
> >>>
> >>>
> >>>
> >>> but may this does not work correct and now pops up.
> >>>
> >>> I try to find out more, thanks for the hint!
> >>
> >> Just check if you're not passing in NULL or empty string, that
> >> might be it. Otherwise the USB code is botched.
> > 
> > Hmm... OK, on the smartweb board there is no factory set, so never
> > calling g_dnl_set_serialnumber()
> > 
> > :-(
> > 
> > why did this worked before commit 842778a091?
> > 
> > So, I added for a fast dirty test:
> > 
> > diff --git a/board/siemens/smartweb/smartweb.c
> > b/board/siemens/smartweb/smartweb.c
> > index 78a7946..01a3dd2 100644
> > --- a/board/siemens/smartweb/smartweb.c
> > +++ b/board/siemens/smartweb/smartweb.c
> > @@ -34,6 +34,7 @@
> >  #ifndef CONFIG_DM_ETH
> >  # include <netdev.h>
> >  #endif
> > +#include <g_dnl.h>
> > 
> >  DECLARE_GLOBAL_DATA_PTR;
> > 
> > @@ -265,3 +266,17 @@ U_BOOT_DEVICE(at91sam9260_serial) = {
> >         .name   = "serial_atmel",
> >         .platdata = &at91sam9260_serial_plat,
> >  };
> > +
> > +int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char
> > *name) +{
> > +       printf("%s: *********\n", __func__);
> > +       g_dnl_set_serialnumber("0123456789");
> > +
> > +       return 0;
> > +}
> > +
> > +int g_dnl_get_board_bcd_device_number(int gcnum)
> > +{
> > +       return 0;
> > +}
> > 
> > Now I see this printf:
> > (also enabled debug in ./drivers/usb/gadget/g_dnl.c)
> > 
> > dfu 0 nand 0
> > using id 'nand0,4'
> > g_dnl_register: g_dnl_driver.name = usb_dnl_dfu
> > g_dnl_bind: gadget: 0x23adf6c0 cdev: 0x23b262d0
> > g_dnl_bind_fixup: *********
> > g_dnl_do_config: configuration: 0x23b263c0 composite dev: 0x23b262d0
> > g_dnl_bind: calling usb_gadget_connect for controller 'at91_udc'
> > 
> > but result is the same:
> > # ./src/dfu-util -l
> > dfu-util 0.7
> > 
> > Copyright 2005-2008 Weston Schmidt, Harald Welte and OpenMoko Inc.
> > Copyright 2010-2012 Tormod Volden and Stefan Schmidt
> > This program is Free Software and has ABSOLUTELY NO WARRANTY
> > Please report bugs to dfu-util at lists.gnumonks.org
> > tb_ctrl: Found DFU: [0908:02d2] ver=0000, devnum=0, cfg=1, intf=0,
> > alt=0, name="Linux", serial="UNDEFINED"
> > 
> > reverting commit 842778a091 and it works as before ... console
> > output for this case:
> > 
> > ./src/dfu-util -l
> > dfu-util 0.7
> > 
> > Copyright 2005-2008 Weston Schmidt, Harald Welte and OpenMoko Inc.
> > Copyright 2010-2012 Tormod Volden and Stefan Schmidt
> > This program is Free Software and has ABSOLUTELY NO WARRANTY
> > Please report bugs to dfu-util at lists.gnumonks.org
> > tb_ctrl: Found DFU: [0908:02d2] ver=0000, devnum=0, cfg=1, intf=0,
> > alt=0, name="Linux", serial="0123456789"
> > 
> > Ok, before commit 842778a091 is in mainline I had the follwoing
> > output:
> > 
> > # tb_ctrl: ./src/dfu-util -l
> > # tb_ctrl: dfu-util 0.7
> > 
> > Copyright 2005-2008 Weston Schmidt, Harald Welte and OpenMoko Inc.
> > Copyright 2010-2012 Tormod Volden and Stefan Schmidt
> > This program is Free Software and has ABSOLUTELY NO WARRANTY
> > Please report bugs to dfu-util at lists.gnumonks.org
> > 
> > Found DFU: [0908:02d2] ver=0212, devnum=0, cfg=1, intf=0, alt=0,
> >     name="Linux", serial=""
> > 
> > serial is an empty string ... It seems to me, that commit 842778a091
> > broke here something fundamental ...
> > 
> > Hmm ... looking into drivers/usb/gadget/g_dnl.c g_dnl_bind()
> > 
> > if (strlen(g_dnl_serial)) {
> > 
> > is *before* g_dnl_bind_fixup() is called ... ?
> > 
> > Yup, with patch:
> > 
> > diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
> > index d4bee9b..813d4b8 100644
> > --- a/drivers/usb/gadget/g_dnl.c
> > +++ b/drivers/usb/gadget/g_dnl.c
> > @@ -224,6 +224,7 @@ static int g_dnl_bind(struct usb_composite_dev
> > *cdev) g_dnl_string_defs[1].id = id;
> >         device_desc.iProduct = id;
> > 
> > +       g_dnl_bind_fixup(&device_desc, cdev->driver->name);
> >         if (strlen(g_dnl_serial)) {
> >                 id = usb_string_id(cdev);
> >                 if (id < 0)
> > @@ -233,7 +234,6 @@ static int g_dnl_bind(struct usb_composite_dev
> > *cdev) device_desc.iSerialNumber = id;
> >         }
> > 
> > -       g_dnl_bind_fixup(&device_desc, cdev->driver->name);
> >         ret = g_dnl_config_register(cdev);
> >         if (ret)
> >                 goto error;
> > 
> > and adding g_dnl_bind_fixup() in board/siemens/smartweb/smartweb.c
> > dfu work again for the smartweb board ... is this an valid fix?
> > 
> > BTW: is an empty serial string not allowed?
> 
> This is Lukasz's domain of expertise, so I'll pull out of this
> discussion and wait for a PR from him.
> 

The problem is with providing "iSerialNumber" visible (at USB
descriptor) only when it is defined.

Before the Felipe commit (SHA1: 842778a091)  we had exposed it
unconditionally - even when we had a zeroed char table (which was the
"" string)

Now we do not have the iStringNumber field defined in descriptor when
the "serial" string size is zero.

I'm wondering which behavior is correct - i.e. comply with the USB
standard.

Felipe - have you tried to run the USB compliance tests [1] (Windows
one) after applying this patch?

[1] http://www.usb.org/developers/tools/usb20_tools/

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de