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

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

[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.

Thanks all!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170417/be0aab02/attachment.sig>

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

[Heiko Schocher]

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@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@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
# bad: [f6c1df44b815a08585e7fd3805a1db51a5955d09] Prepare v2017.05-rc2
git bisect bad f6c1df44b815a08585e7fd3805a1db51a5955d09
# good: [9963890b8be1d208035945abc5ba9f77637b6542] libfdt: fix build with Python 3
git bisect good 9963890b8be1d208035945abc5ba9f77637b6542
# good: [af1b7286d8b2712cff5779d8a1565afed9d9d8e6] Merge branch 'master' of git://git.denx.de/u-boot-mmc
git bisect good af1b7286d8b2712cff5779d8a1565afed9d9d8e6
# bad: [3fea95369850987de15a2a0ac009d05e13b90246] Merge branch 'master' of 
git://git.denx.de/u-boot-video
git bisect bad 3fea95369850987de15a2a0ac009d05e13b90246
# good: [c1a16c3ab541c014b029b42cc27cae496107e170] Merge branch 'master' of 
git://git.denx.de/u-boot-socfpga
git bisect good c1a16c3ab541c014b029b42cc27cae496107e170
# bad: [fbeb33752999e7317113199ef89873d6b6916814] buildman: Translate more strings to latin-1
git bisect bad fbeb33752999e7317113199ef89873d6b6916814
# bad: [b7bf4a95922c3e1a4974aa34ebb714ac2eb89937] usb: dwc3: ensure consistent types for 
dwc3_flush_cache
git bisect bad b7bf4a95922c3e1a4974aa34ebb714ac2eb89937
# good: [12d0b8f5f07b76d7447d60f7528d578233553909] usb: gadget: g_dnl: hold maximum string descriptor
git bisect good 12d0b8f5f07b76d7447d60f7528d578233553909
# bad: [207835b13feeae15db0555574d89352a4e5379a4] usb: gadget: g_dnl: don't set iProduct nor 
iSerialNumber
git bisect bad 207835b13feeae15db0555574d89352a4e5379a4
# bad: [842778a091047b0c868efa12229633959f711152] usb: gadget: g_dnl: only set iSerialNumber if we 
have a serial#
git bisect bad 842778a091047b0c868efa12229633959f711152
# first bad commit: [842778a091047b0c868efa12229633959f711152] usb: gadget: g_dnl: only set 
iSerialNumber if we have a serial#
hs at pollux [ 7:24:31] ttbott>

Any ideas?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[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@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@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
> # bad: [f6c1df44b815a08585e7fd3805a1db51a5955d09] Prepare v2017.05-rc2
> git bisect bad f6c1df44b815a08585e7fd3805a1db51a5955d09
> # good: [9963890b8be1d208035945abc5ba9f77637b6542] libfdt: fix build
> with Python 3
> git bisect good 9963890b8be1d208035945abc5ba9f77637b6542
> # good: [af1b7286d8b2712cff5779d8a1565afed9d9d8e6] Merge branch 'master'
> of git://git.denx.de/u-boot-mmc
> git bisect good af1b7286d8b2712cff5779d8a1565afed9d9d8e6
> # bad: [3fea95369850987de15a2a0ac009d05e13b90246] Merge branch 'master'
> of git://git.denx.de/u-boot-video
> git bisect bad 3fea95369850987de15a2a0ac009d05e13b90246
> # good: [c1a16c3ab541c014b029b42cc27cae496107e170] Merge branch 'master'
> of git://git.denx.de/u-boot-socfpga
> git bisect good c1a16c3ab541c014b029b42cc27cae496107e170
> # bad: [fbeb33752999e7317113199ef89873d6b6916814] buildman: Translate
> more strings to latin-1
> git bisect bad fbeb33752999e7317113199ef89873d6b6916814
> # bad: [b7bf4a95922c3e1a4974aa34ebb714ac2eb89937] usb: dwc3: ensure
> consistent types for dwc3_flush_cache
> git bisect bad b7bf4a95922c3e1a4974aa34ebb714ac2eb89937
> # good: [12d0b8f5f07b76d7447d60f7528d578233553909] usb: gadget: g_dnl:
> hold maximum string descriptor
> git bisect good 12d0b8f5f07b76d7447d60f7528d578233553909
> # bad: [207835b13feeae15db0555574d89352a4e5379a4] usb: gadget: g_dnl:
> don't set iProduct nor iSerialNumber
> git bisect bad 207835b13feeae15db0555574d89352a4e5379a4
> # bad: [842778a091047b0c868efa12229633959f711152] usb: gadget: g_dnl:
> only set iSerialNumber if we have a serial#
> git bisect bad 842778a091047b0c868efa12229633959f711152
> # first bad commit: [842778a091047b0c868efa12229633959f711152] usb:
> gadget: g_dnl: only set iSerialNumber if we have a serial#
> hs at pollux [ 7:24:31] ttbott>
> 
> Any ideas?

Is your board setting up the serial number with g_dnl_set_serialnumber()
correctly ?

-- 
Best regards,
Marek Vasut

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

[Heiko Schocher]

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@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@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!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[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@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@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.

-- 
Best regards,
Marek Vasut

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

[Heiko Schocher]

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@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@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?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Marek Vasut]

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@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@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.

-- 
Best regards,
Marek Vasut

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

[Lukasz Majewski]

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@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@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

[U-Boot] drivers: usb: dfu: set serial number from board code was: [ANN] U-Boot v2017.05-rc2 released

[Heiko Schocher]

Hello all,

Am 19.04.2017 um 14:07 schrieb Lukasz Majewski:
> 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@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@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

How to proceed here? If the current behaviour of U-Boot is correct,
then I simple adapt my tbot testcase ... but I think, currently we
have no way to set the serial number field, or?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] drivers: usb: dfu: set serial number from board code was: [ANN] U-Boot v2017.05-rc2 released

[Lukasz Majewski]

Hi Heiko,

> Hello all,
> 
> Am 19.04.2017 um 14:07 schrieb Lukasz Majewski:
> > 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@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@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?

I was waiting for Felipe answer.....

> >
> >
> >
> > [1] http://www.usb.org/developers/tools/usb20_tools/
> >
> > Best regards,
> >
> > Lukasz Majewski
> 
> How to proceed here? If the current behaviour of U-Boot is correct,
> then I simple adapt my tbot testcase ... 

... but none was given.

However, IMHO it is better to not expose the string when it is empty.

> but I think, currently we
> have no way to set the serial number field, or?

:-( Yes, this commit has introduced regression (the g_dnl_serial is
always NULL there because we setup the serial number in a latter
function:

g_dnl_bind_fixup @ for example board/siemens/common/factoryreset.c

which calls: g_dnl_set_serialnumber() and only there the g_dnl_serial
is set.

Please find attached patch (if it fixes siemens boards).

> 
> bye,
> Heiko




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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-usb-gadget-Call-g_dnl_bind_fixup-before-testing-g_dn.patch
Type: text/x-patch
Size: 1432 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170626/fb2a3862/attachment.bin>

[U-Boot] drivers: usb: dfu: set serial number from board code was: [ANN] U-Boot v2017.05-rc2 released

[Felipe Balbi]

Hi,

Lukasz Majewski <lukma@denx.de> writes:
>> >>>>>>> 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@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@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?
>
> I was waiting for Felipe answer.....

sorry, I completely missed this.

if iSerialNumber is set to a non-zero value, then the actual string
should exist. if the string is empty, then iSerialNumber should be
cleared to zero in the device descriptor.

>> > [1] http://www.usb.org/developers/tools/usb20_tools/
>> >
>> > Best regards,
>> >
>> > Lukasz Majewski
>> 
>> How to proceed here? If the current behaviour of U-Boot is correct,
>> then I simple adapt my tbot testcase ... 
>
> ... but none was given.
>
> However, IMHO it is better to not expose the string when it is empty.

right

>> but I think, currently we
>> have no way to set the serial number field, or?
>
> :-( Yes, this commit has introduced regression (the g_dnl_serial is
> always NULL there because we setup the serial number in a latter
> function:
>
> g_dnl_bind_fixup @ for example board/siemens/common/factoryreset.c
>
> which calls: g_dnl_set_serialnumber() and only there the g_dnl_serial
> is set.
>
> Please find attached patch (if it fixes siemens boards).

> 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
> From 5af562a7b43184ea5ab5bb5a18ff3b14f48b2475 Mon Sep 17 00:00:00 2001
> From: Lukasz Majewski <lukma@denx.de>
> Date: Mon, 26 Jun 2017 12:15:09 +0200
> Subject: [PATCH] usb: gadget: Call g_dnl_bind_fixup() before testing
>  g_dnl_serial length
>
> After the commit SHA1: 842778a091 - the serial number descriptor is only
> visible when we have non zero length of g_dnl_serial.
>
> However, on some platforms (e.g. Siemens) the serial number is set at
> g_dnl_bind_fixup(), so with the current code we will always omit the
> serial (since it is not set).
>
> This commit moves the g_dnl_bind_fixup() call before the g_dnl_serial
> length test.
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>

looks good to me.

> ---
>  drivers/usb/gadget/g_dnl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
> index d4bee9b..0491a0e 100644
> --- a/drivers/usb/gadget/g_dnl.c
> +++ b/drivers/usb/gadget/g_dnl.c
> @@ -224,6 +224,8 @@ 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 +235,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;
> -- 
> 2.1.4
>

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170626/47ce3c3d/attachment.sig>

[U-Boot] drivers: usb: dfu: set serial number from board code was: [ANN] U-Boot v2017.05-rc2 released

[Heiko Schocher]

Hello Felipe, Lukasz,

Am 26.06.2017 um 12:22 schrieb Felipe Balbi:
>
> Hi,
>
> Lukasz Majewski <lukma@denx.de> writes:
>>>>>>>>>> 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@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@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?
>>
>> I was waiting for Felipe answer.....
>
> sorry, I completely missed this.
>
> if iSerialNumber is set to a non-zero value, then the actual string
> should exist. if the string is empty, then iSerialNumber should be
> cleared to zero in the device descriptor.
>
>>>> [1] http://www.usb.org/developers/tools/usb20_tools/
>>>>
>>>> Best regards,
>>>>
>>>> Lukasz Majewski
>>>
>>> How to proceed here? If the current behaviour of U-Boot is correct,
>>> then I simple adapt my tbot testcase ...
>>
>> ... but none was given.
>>
>> However, IMHO it is better to not expose the string when it is empty.
>
> right
>
>>> but I think, currently we
>>> have no way to set the serial number field, or?
>>
>> :-( Yes, this commit has introduced regression (the g_dnl_serial is
>> always NULL there because we setup the serial number in a latter
>> function:
>>
>> g_dnl_bind_fixup @ for example board/siemens/common/factoryreset.c
>>
>> which calls: g_dnl_set_serialnumber() and only there the g_dnl_serial
>> is set.
>>
>> Please find attached patch (if it fixes siemens boards).
>
>> 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
>>  From 5af562a7b43184ea5ab5bb5a18ff3b14f48b2475 Mon Sep 17 00:00:00 2001
>> From: Lukasz Majewski <lukma@denx.de>
>> Date: Mon, 26 Jun 2017 12:15:09 +0200
>> Subject: [PATCH] usb: gadget: Call g_dnl_bind_fixup() before testing
>>   g_dnl_serial length
>>
>> After the commit SHA1: 842778a091 - the serial number descriptor is only
>> visible when we have non zero length of g_dnl_serial.
>>
>> However, on some platforms (e.g. Siemens) the serial number is set at
>> g_dnl_bind_fixup(), so with the current code we will always omit the
>> serial (since it is not set).
>>
>> This commit moves the g_dnl_bind_fixup() call before the g_dnl_serial
>> length test.
>>
>> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>
> looks good to me.

Acked-by: Heiko Schocher <hs@denx.de>

@Lukasz: Do you send this as a patch? (Or did I missed it?)

Thanks!

bye,
Heiko
>
>> ---
>>   drivers/usb/gadget/g_dnl.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
>> index d4bee9b..0491a0e 100644
>> --- a/drivers/usb/gadget/g_dnl.c
>> +++ b/drivers/usb/gadget/g_dnl.c
>> @@ -224,6 +224,8 @@ 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 +235,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;
>> --
>> 2.1.4
>>
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] drivers: usb: dfu: set serial number from board code was: [ANN] U-Boot v2017.05-rc2 released

[Lukasz Majewski]

On Mon, 26 Jun 2017 12:50:34 +0200
Heiko Schocher <hs@denx.de> wrote:

> Hello Felipe, Lukasz,
> 
> Am 26.06.2017 um 12:22 schrieb Felipe Balbi:
> >
> > Hi,
> >
> > Lukasz Majewski <lukma@denx.de> writes:
> >>>>>>>>>> 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@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@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?
> >>
> >> I was waiting for Felipe answer.....
> >
> > sorry, I completely missed this.
> >
> > if iSerialNumber is set to a non-zero value, then the actual string
> > should exist. if the string is empty, then iSerialNumber should be
> > cleared to zero in the device descriptor.
> >
> >>>> [1] http://www.usb.org/developers/tools/usb20_tools/
> >>>>
> >>>> Best regards,
> >>>>
> >>>> Lukasz Majewski
> >>>
> >>> How to proceed here? If the current behaviour of U-Boot is
> >>> correct, then I simple adapt my tbot testcase ...
> >>
> >> ... but none was given.
> >>
> >> However, IMHO it is better to not expose the string when it is
> >> empty.
> >
> > right
> >
> >>> but I think, currently we
> >>> have no way to set the serial number field, or?
> >>
> >> :-( Yes, this commit has introduced regression (the g_dnl_serial is
> >> always NULL there because we setup the serial number in a latter
> >> function:
> >>
> >> g_dnl_bind_fixup @ for example board/siemens/common/factoryreset.c
> >>
> >> which calls: g_dnl_set_serialnumber() and only there the
> >> g_dnl_serial is set.
> >>
> >> Please find attached patch (if it fixes siemens boards).
> >
> >> 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 From 5af562a7b43184ea5ab5bb5a18ff3b14f48b2475 Mon Sep
> >> 17 00:00:00 2001 From: Lukasz Majewski <lukma@denx.de>
> >> Date: Mon, 26 Jun 2017 12:15:09 +0200
> >> Subject: [PATCH] usb: gadget: Call g_dnl_bind_fixup() before
> >> testing g_dnl_serial length
> >>
> >> After the commit SHA1: 842778a091 - the serial number descriptor
> >> is only visible when we have non zero length of g_dnl_serial.
> >>
> >> However, on some platforms (e.g. Siemens) the serial number is set
> >> at g_dnl_bind_fixup(), so with the current code we will always
> >> omit the serial (since it is not set).
> >>
> >> This commit moves the g_dnl_bind_fixup() call before the
> >> g_dnl_serial length test.
> >>
> >> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> >
> > looks good to me.
> 
> Acked-by: Heiko Schocher <hs@denx.de>
> 
> @Lukasz: Do you send this as a patch? (Or did I missed it?)

I will send it immediately to ML :-)

> 
> Thanks!
> 
> bye,
> Heiko
> >
> >> ---
> >>   drivers/usb/gadget/g_dnl.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/gadget/g_dnl.c
> >> b/drivers/usb/gadget/g_dnl.c index d4bee9b..0491a0e 100644
> >> --- a/drivers/usb/gadget/g_dnl.c
> >> +++ b/drivers/usb/gadget/g_dnl.c
> >> @@ -224,6 +224,8 @@ 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 +235,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;
> >> --
> >> 2.1.4
> >>
> >
> 




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

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

[Lukasz Majewski]

Hi Heiko,

> 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@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.

This change was added recently to DFU tree. It seemed like the one,
which clean up things.

Moreover, I tested it on the "old" set of DFU tests (however, those
tests are not the USB compliance test suite).

Heiko, could you paste the error output? (from the host PC and the
board)?

> 
>      Signed-off-by: Felipe Balbi <felipe.balbi@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
> # bad: [f6c1df44b815a08585e7fd3805a1db51a5955d09] Prepare v2017.05-rc2
> git bisect bad f6c1df44b815a08585e7fd3805a1db51a5955d09
> # good: [9963890b8be1d208035945abc5ba9f77637b6542] libfdt: fix build
> with Python 3 git bisect good 9963890b8be1d208035945abc5ba9f77637b6542
> # good: [af1b7286d8b2712cff5779d8a1565afed9d9d8e6] Merge branch
> 'master' of git://git.denx.de/u-boot-mmc git bisect good
> af1b7286d8b2712cff5779d8a1565afed9d9d8e6 # bad:
> [3fea95369850987de15a2a0ac009d05e13b90246] Merge branch 'master' of
> git://git.denx.de/u-boot-video git bisect bad
> 3fea95369850987de15a2a0ac009d05e13b90246 # good:
> [c1a16c3ab541c014b029b42cc27cae496107e170] Merge branch 'master' of
> git://git.denx.de/u-boot-socfpga git bisect good
> c1a16c3ab541c014b029b42cc27cae496107e170 # bad:
> [fbeb33752999e7317113199ef89873d6b6916814] buildman: Translate more
> strings to latin-1 git bisect bad
> fbeb33752999e7317113199ef89873d6b6916814 # bad:
> [b7bf4a95922c3e1a4974aa34ebb714ac2eb89937] usb: dwc3: ensure
> consistent types for dwc3_flush_cache git bisect bad
> b7bf4a95922c3e1a4974aa34ebb714ac2eb89937 # good:
> [12d0b8f5f07b76d7447d60f7528d578233553909] usb: gadget: g_dnl: hold
> maximum string descriptor git bisect good
> 12d0b8f5f07b76d7447d60f7528d578233553909 # bad:
> [207835b13feeae15db0555574d89352a4e5379a4] usb: gadget: g_dnl: don't
> set iProduct nor iSerialNumber git bisect bad
> 207835b13feeae15db0555574d89352a4e5379a4 # bad:
> [842778a091047b0c868efa12229633959f711152] usb: gadget: g_dnl: only
> set iSerialNumber if we have a serial# git bisect bad
> 842778a091047b0c868efa12229633959f711152 # first bad commit:
> [842778a091047b0c868efa12229633959f711152] usb: gadget: g_dnl: only
> set iSerialNumber if we have a serial# hs at pollux [ 7:24:31] ttbott>
> 
> Any ideas?
> 
> bye,
> Heiko




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

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

[Andreas Färber]

Hi,

Using -rc2 with firefly-rk3288 defconfig I can only boot into SPL but
not into full U-Boot. I am using the old documented way of dd'ing to
sector 256 on SD (doc/README.rockchip).

Looking at include/configs/rk3288_common.h I also tried putting
u-boot.img on a FAT partition, to no effect.

v2017.03 doesn't even show SPL working.
v2017.01 worked okay.

Build log for -rc2:
https://build.opensuse.org/build/Base:System:Staging/openSUSE_Factory_ARM/armv7l/u-boot-firefly-rk3288/_log

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[Simon Glass]

Hi Andreas,

On 20 April 2017 at 17:23, Andreas Färber <afaerber@suse.de> wrote:
> Hi,
>
> Using -rc2 with firefly-rk3288 defconfig I can only boot into SPL but
> not into full U-Boot. I am using the old documented way of dd'ing to
> sector 256 on SD (doc/README.rockchip).
>
> Looking at include/configs/rk3288_common.h I also tried putting
> u-boot.img on a FAT partition, to no effect.
>
> v2017.03 doesn't even show SPL working.
> v2017.01 worked okay.
>
> Build log for -rc2:
> https://build.opensuse.org/build/Base:System:Staging/openSUSE_Factory_ARM/armv7l/u-boot-firefly-rk3288/_log

Can you try removing RETURN_TO_BROM (or imilsar) and adding
CONFIG_SPL_OF_PLATDATA?

If that works we should probably create a new board variant with that
configuration, or figure out a way to detect which to use.

Regards,
Simon

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

[Andreas Färber]

Hi Simon,

Am 21.04.2017 um 01:44 schrieb Simon Glass:
> On 20 April 2017 at 17:23, Andreas Färber <afaerber@suse.de> wrote:
>> Using -rc2 with firefly-rk3288 defconfig I can only boot into SPL but
>> not into full U-Boot. I am using the old documented way of dd'ing to
>> sector 256 on SD (doc/README.rockchip).
>>
>> Looking at include/configs/rk3288_common.h I also tried putting
>> u-boot.img on a FAT partition, to no effect.
>>
>> v2017.03 doesn't even show SPL working.
>> v2017.01 worked okay.
>>
>> Build log for -rc2:
>> https://build.opensuse.org/build/Base:System:Staging/openSUSE_Factory_ARM/armv7l/u-boot-firefly-rk3288/_log
> 
> Can you try removing RETURN_TO_BROM (or imilsar) and adding
> CONFIG_SPL_OF_PLATDATA?

As an interim update I can share that latest master behaves slightly
differently:

U-Boot SPL 2017.05-rc2-00053-g3c476d8 (Apr 21 2017 - 02:29:48)
Returning to boot ROM...

compared to:

U-Boot SPL 2017.05-rc2 (Apr 20 2017 - 14:33:20)

Will tweak options next.

Thanks,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[Andreas Färber]

Am 21.04.2017 um 02:34 schrieb Andreas Färber:
> Hi Simon,
> 
> Am 21.04.2017 um 01:44 schrieb Simon Glass:
>> On 20 April 2017 at 17:23, Andreas Färber <afaerber@suse.de> wrote:
>>> Using -rc2 with firefly-rk3288 defconfig I can only boot into SPL but
>>> not into full U-Boot. I am using the old documented way of dd'ing to
>>> sector 256 on SD (doc/README.rockchip).
>>>
>>> Looking at include/configs/rk3288_common.h I also tried putting
>>> u-boot.img on a FAT partition, to no effect.
>>>
>>> v2017.03 doesn't even show SPL working.
>>> v2017.01 worked okay.
>>>
>>> Build log for -rc2:
>>> https://build.opensuse.org/build/Base:System:Staging/openSUSE_Factory_ARM/armv7l/u-boot-firefly-rk3288/_log
>>
>> Can you try removing RETURN_TO_BROM (or imilsar) and adding
>> CONFIG_SPL_OF_PLATDATA?
> 
> As an interim update I can share that latest master behaves slightly
> differently:
> 
> U-Boot SPL 2017.05-rc2-00053-g3c476d8 (Apr 21 2017 - 02:29:48)
> Returning to boot ROM...
> 
> compared to:
> 
> U-Boot SPL 2017.05-rc2 (Apr 20 2017 - 14:33:20)

Disabling the ROCKCHIP_SPL_BACK_TO_BROM option I get:

U-Boot SPL 2017.05-rc2-00053-g3c476d8 (Apr 21 2017 - 02:38:52)
SPL: Unsupported Boot Device!
SPL: failed to boot from all boot devices
### ERROR ### Please RESET the board ###

If I additionally enable SPL_OF_PLATDATA then I am back to the v2017.03
state of no serial output.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[Simon Glass]

Hi Andreas,

On 20 April 2017 at 18:47, Andreas Färber <afaerber@suse.de> wrote:
> Am 21.04.2017 um 02:34 schrieb Andreas Färber:
>> Hi Simon,
>>
>> Am 21.04.2017 um 01:44 schrieb Simon Glass:
>>> On 20 April 2017 at 17:23, Andreas Färber <afaerber@suse.de> wrote:
>>>> Using -rc2 with firefly-rk3288 defconfig I can only boot into SPL but
>>>> not into full U-Boot. I am using the old documented way of dd'ing to
>>>> sector 256 on SD (doc/README.rockchip).
>>>>
>>>> Looking at include/configs/rk3288_common.h I also tried putting
>>>> u-boot.img on a FAT partition, to no effect.
>>>>
>>>> v2017.03 doesn't even show SPL working.
>>>> v2017.01 worked okay.
>>>>
>>>> Build log for -rc2:
>>>> https://build.opensuse.org/build/Base:System:Staging/openSUSE_Factory_ARM/armv7l/u-boot-firefly-rk3288/_log
>>>
>>> Can you try removing RETURN_TO_BROM (or imilsar) and adding
>>> CONFIG_SPL_OF_PLATDATA?
>>
>> As an interim update I can share that latest master behaves slightly
>> differently:
>>
>> U-Boot SPL 2017.05-rc2-00053-g3c476d8 (Apr 21 2017 - 02:29:48)
>> Returning to boot ROM...
>>
>> compared to:
>>
>> U-Boot SPL 2017.05-rc2 (Apr 20 2017 - 14:33:20)
>
> Disabling the ROCKCHIP_SPL_BACK_TO_BROM option I get:
>
> U-Boot SPL 2017.05-rc2-00053-g3c476d8 (Apr 21 2017 - 02:38:52)
> SPL: Unsupported Boot Device!
> SPL: failed to boot from all boot devices
> ### ERROR ### Please RESET the board ###
>
> If I additionally enable SPL_OF_PLATDATA then I am back to the v2017.03
> state of no serial output.

I just tested mainline with those two changes and it works for me. I
pushed my patch to u-boot-rockchip/firefly-working. Can you try again?

Regards,
Simon

>
> Regards,
> Andreas
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)

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

[Andreas Färber]

Hi Simon,

Am 21.04.2017 um 04:10 schrieb Simon Glass:
> On 20 April 2017 at 18:47, Andreas Färber <afaerber@suse.de> wrote:
>> Am 21.04.2017 um 02:34 schrieb Andreas Färber:
>>> Am 21.04.2017 um 01:44 schrieb Simon Glass:
>>>> On 20 April 2017 at 17:23, Andreas Färber <afaerber@suse.de> wrote:
>>>>> Using -rc2 with firefly-rk3288 defconfig I can only boot into SPL but
>>>>> not into full U-Boot. I am using the old documented way of dd'ing to
>>>>> sector 256 on SD (doc/README.rockchip).
>>>>>
>>>>> Looking at include/configs/rk3288_common.h I also tried putting
>>>>> u-boot.img on a FAT partition, to no effect.
>>>>>
>>>>> v2017.03 doesn't even show SPL working.
>>>>> v2017.01 worked okay.
>>>>>
>>>>> Build log for -rc2:
>>>>> https://build.opensuse.org/build/Base:System:Staging/openSUSE_Factory_ARM/armv7l/u-boot-firefly-rk3288/_log
>>>>
>>>> Can you try removing RETURN_TO_BROM (or imilsar) and adding
>>>> CONFIG_SPL_OF_PLATDATA?
>>>
>>> As an interim update I can share that latest master behaves slightly
>>> differently:
>>>
>>> U-Boot SPL 2017.05-rc2-00053-g3c476d8 (Apr 21 2017 - 02:29:48)
>>> Returning to boot ROM...
>>>
>>> compared to:
>>>
>>> U-Boot SPL 2017.05-rc2 (Apr 20 2017 - 14:33:20)
>>
>> Disabling the ROCKCHIP_SPL_BACK_TO_BROM option I get:
>>
>> U-Boot SPL 2017.05-rc2-00053-g3c476d8 (Apr 21 2017 - 02:38:52)
>> SPL: Unsupported Boot Device!
>> SPL: failed to boot from all boot devices
>> ### ERROR ### Please RESET the board ###
>>
>> If I additionally enable SPL_OF_PLATDATA then I am back to the v2017.03
>> state of no serial output.
> 
> I just tested mainline with those two changes and it works for me. I
> pushed my patch to u-boot-rockchip/firefly-working. Can you try again?

Confirming that with your defconfig it fully works again.

I had instead run menuconfig and manually (un)selected the options.
Maybe those options influence other defaults? I was at the same qoriq
merge commit your branch is based off.

Anyway, does this allow any conclusions for -rc3?

Cheers,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[Andreas Färber]

Am 21.04.2017 um 04:24 schrieb Andreas Färber:
> Hi Simon,
> 
> Am 21.04.2017 um 04:10 schrieb Simon Glass:
>> On 20 April 2017 at 18:47, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 21.04.2017 um 02:34 schrieb Andreas Färber:
>>>> Am 21.04.2017 um 01:44 schrieb Simon Glass:
>>>>> On 20 April 2017 at 17:23, Andreas Färber <afaerber@suse.de> wrote:
>>>>>> Using -rc2 with firefly-rk3288 defconfig I can only boot into SPL but
>>>>>> not into full U-Boot. I am using the old documented way of dd'ing to
>>>>>> sector 256 on SD (doc/README.rockchip).
>>>>>>
>>>>>> Looking at include/configs/rk3288_common.h I also tried putting
>>>>>> u-boot.img on a FAT partition, to no effect.
>>>>>>
>>>>>> v2017.03 doesn't even show SPL working.
>>>>>> v2017.01 worked okay.
>>>>>>
>>>>>> Build log for -rc2:
>>>>>> https://build.opensuse.org/build/Base:System:Staging/openSUSE_Factory_ARM/armv7l/u-boot-firefly-rk3288/_log
>>>>>
>>>>> Can you try removing RETURN_TO_BROM (or imilsar) and adding
>>>>> CONFIG_SPL_OF_PLATDATA?
>>>>
>>>> As an interim update I can share that latest master behaves slightly
>>>> differently:
>>>>
>>>> U-Boot SPL 2017.05-rc2-00053-g3c476d8 (Apr 21 2017 - 02:29:48)
>>>> Returning to boot ROM...
>>>>
>>>> compared to:
>>>>
>>>> U-Boot SPL 2017.05-rc2 (Apr 20 2017 - 14:33:20)
>>>
>>> Disabling the ROCKCHIP_SPL_BACK_TO_BROM option I get:
>>>
>>> U-Boot SPL 2017.05-rc2-00053-g3c476d8 (Apr 21 2017 - 02:38:52)
>>> SPL: Unsupported Boot Device!
>>> SPL: failed to boot from all boot devices
>>> ### ERROR ### Please RESET the board ###
>>>
>>> If I additionally enable SPL_OF_PLATDATA then I am back to the v2017.03
>>> state of no serial output.
>>
>> I just tested mainline with those two changes and it works for me. I
>> pushed my patch to u-boot-rockchip/firefly-working. Can you try again?
> 
> Confirming that with your defconfig it fully works again.
> 
> I had instead run menuconfig and manually (un)selected the options.
> Maybe those options influence other defaults? I was at the same qoriq
> merge commit your branch is based off.

Here's a quick diff against master:

diff -u firefly/.config firefly/simon_config
--- firefly/.config	2017-04-21 04:39:49.484215791 +0200
+++ firefly/simon_config	2017-04-21 04:39:07.123727380 +0200
@@ -169,9 +169,8 @@
 CONFIG_ROCKCHIP_RK3288=y
 # CONFIG_ROCKCHIP_RK3328 is not set
 # CONFIG_ROCKCHIP_RK3399 is not set
-CONFIG_ROCKCHIP_SPL_BACK_TO_BROM=y
-CONFIG_ROCKCHIP_BROM_HELPER=y
-# CONFIG_SPL_MMC_SUPPORT is not set
+# CONFIG_ROCKCHIP_SPL_BACK_TO_BROM is not set
+CONFIG_SPL_MMC_SUPPORT=y
 CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_BOARD_SPECIFIC_OPTIONS=y
 # CONFIG_TARGET_CHROMEBOOK_JERRY is not set
@@ -510,7 +509,7 @@
 CONFIG_OF_SEPARATE=y
 # CONFIG_OF_EMBED is not set
 CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names
interrupt-parent assigned-clocks assigned-clock-rates
assigned-clock-parents"
-# CONFIG_SPL_OF_PLATDATA is not set
+CONFIG_SPL_OF_PLATDATA=y
 CONFIG_NET=y
 CONFIG_NET_RANDOM_ETHADDR=y
 # CONFIG_NETCONSOLE is not set
@@ -848,6 +847,7 @@
 # CONFIG_FSL_LPUART is not set
 # CONFIG_MVEBU_A3700_UART is not set
 CONFIG_SYS_NS16550=y
+CONFIG_ROCKCHIP_SERIAL=y
 # CONFIG_MSM_SERIAL is not set
 # CONFIG_PXA_SERIAL is not set

At least SERIAL looks important...

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[Andreas Färber]

Am 21.04.2017 um 04:43 schrieb Andreas Färber:
> Am 21.04.2017 um 04:24 schrieb Andreas Färber:
>> Hi Simon,
>>
>> Am 21.04.2017 um 04:10 schrieb Simon Glass:
>>> I just tested mainline with those two changes and it works for me. I
>>> pushed my patch to u-boot-rockchip/firefly-working. Can you try again?
>>
>> Confirming that with your defconfig it fully works again.
>>
>> I had instead run menuconfig and manually (un)selected the options.
>> Maybe those options influence other defaults? I was at the same qoriq
>> merge commit your branch is based off.
> 
> Here's a quick diff against master:
> 
> diff -u firefly/.config firefly/simon_config
> --- firefly/.config	2017-04-21 04:39:49.484215791 +0200
> +++ firefly/simon_config	2017-04-21 04:39:07.123727380 +0200
> @@ -169,9 +169,8 @@
>  CONFIG_ROCKCHIP_RK3288=y
>  # CONFIG_ROCKCHIP_RK3328 is not set
>  # CONFIG_ROCKCHIP_RK3399 is not set
> -CONFIG_ROCKCHIP_SPL_BACK_TO_BROM=y
> -CONFIG_ROCKCHIP_BROM_HELPER=y
> -# CONFIG_SPL_MMC_SUPPORT is not set
> +# CONFIG_ROCKCHIP_SPL_BACK_TO_BROM is not set
> +CONFIG_SPL_MMC_SUPPORT=y
>  CONFIG_SPL_SERIAL_SUPPORT=y
>  CONFIG_BOARD_SPECIFIC_OPTIONS=y
>  # CONFIG_TARGET_CHROMEBOOK_JERRY is not set
> @@ -510,7 +509,7 @@
>  CONFIG_OF_SEPARATE=y
>  # CONFIG_OF_EMBED is not set
>  CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names
> interrupt-parent assigned-clocks assigned-clock-rates
> assigned-clock-parents"
> -# CONFIG_SPL_OF_PLATDATA is not set
> +CONFIG_SPL_OF_PLATDATA=y
>  CONFIG_NET=y
>  CONFIG_NET_RANDOM_ETHADDR=y
>  # CONFIG_NETCONSOLE is not set
> @@ -848,6 +847,7 @@
>  # CONFIG_FSL_LPUART is not set
>  # CONFIG_MVEBU_A3700_UART is not set
>  CONFIG_SYS_NS16550=y
> +CONFIG_ROCKCHIP_SERIAL=y
>  # CONFIG_MSM_SERIAL is not set
>  # CONFIG_PXA_SERIAL is not set
> 
> At least SERIAL looks important...

... but enabling SPL_OF_PLATDATA and ROCKCHIP_SERIAL on master (via
menuconfig) still sits at "Returning to boot ROM...".

Regards,
Andreas

diff -u firefly/.config firefly/simon_config
--- firefly/.config	2017-04-21 04:47:11.165310486 +0200
+++ firefly/simon_config	2017-04-21 04:39:07.123727380 +0200
@@ -169,9 +169,8 @@
 CONFIG_ROCKCHIP_RK3288=y
 # CONFIG_ROCKCHIP_RK3328 is not set
 # CONFIG_ROCKCHIP_RK3399 is not set
-CONFIG_ROCKCHIP_SPL_BACK_TO_BROM=y
-CONFIG_ROCKCHIP_BROM_HELPER=y
-# CONFIG_SPL_MMC_SUPPORT is not set
+# CONFIG_ROCKCHIP_SPL_BACK_TO_BROM is not set
+CONFIG_SPL_MMC_SUPPORT=y
 CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_BOARD_SPECIFIC_OPTIONS=y
 # CONFIG_TARGET_CHROMEBOOK_JERRY is not set

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[Simon Glass]

Hi Andreas,

On 20 April 2017 at 20:54, Andreas Färber <afaerber@suse.de> wrote:
> Am 21.04.2017 um 04:43 schrieb Andreas Färber:
>> Am 21.04.2017 um 04:24 schrieb Andreas Färber:
>>> Hi Simon,
>>>
>>> Am 21.04.2017 um 04:10 schrieb Simon Glass:
>>>> I just tested mainline with those two changes and it works for me. I
>>>> pushed my patch to u-boot-rockchip/firefly-working. Can you try again?
>>>
>>> Confirming that with your defconfig it fully works again.
>>>
>>> I had instead run menuconfig and manually (un)selected the options.
>>> Maybe those options influence other defaults? I was at the same qoriq
>>> merge commit your branch is based off.
>>
>> Here's a quick diff against master:
>>
>> diff -u firefly/.config firefly/simon_config
>> --- firefly/.config   2017-04-21 04:39:49.484215791 +0200
>> +++ firefly/simon_config      2017-04-21 04:39:07.123727380 +0200
>> @@ -169,9 +169,8 @@
>>  CONFIG_ROCKCHIP_RK3288=y
>>  # CONFIG_ROCKCHIP_RK3328 is not set
>>  # CONFIG_ROCKCHIP_RK3399 is not set
>> -CONFIG_ROCKCHIP_SPL_BACK_TO_BROM=y
>> -CONFIG_ROCKCHIP_BROM_HELPER=y
>> -# CONFIG_SPL_MMC_SUPPORT is not set
>> +# CONFIG_ROCKCHIP_SPL_BACK_TO_BROM is not set
>> +CONFIG_SPL_MMC_SUPPORT=y
>>  CONFIG_SPL_SERIAL_SUPPORT=y
>>  CONFIG_BOARD_SPECIFIC_OPTIONS=y
>>  # CONFIG_TARGET_CHROMEBOOK_JERRY is not set
>> @@ -510,7 +509,7 @@
>>  CONFIG_OF_SEPARATE=y
>>  # CONFIG_OF_EMBED is not set
>>  CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names
>> interrupt-parent assigned-clocks assigned-clock-rates
>> assigned-clock-parents"
>> -# CONFIG_SPL_OF_PLATDATA is not set
>> +CONFIG_SPL_OF_PLATDATA=y
>>  CONFIG_NET=y
>>  CONFIG_NET_RANDOM_ETHADDR=y
>>  # CONFIG_NETCONSOLE is not set
>> @@ -848,6 +847,7 @@
>>  # CONFIG_FSL_LPUART is not set
>>  # CONFIG_MVEBU_A3700_UART is not set
>>  CONFIG_SYS_NS16550=y
>> +CONFIG_ROCKCHIP_SERIAL=y
>>  # CONFIG_MSM_SERIAL is not set
>>  # CONFIG_PXA_SERIAL is not set
>>
>> At least SERIAL looks important...
>
> ... but enabling SPL_OF_PLATDATA and ROCKCHIP_SERIAL on master (via
> menuconfig) still sits at "Returning to boot ROM...".

Can you diff the resulting config? There might perhaps be a broken dependency.

>
> Regards,
> Andreas
>
> diff -u firefly/.config firefly/simon_config
> --- firefly/.config     2017-04-21 04:47:11.165310486 +0200
> +++ firefly/simon_config        2017-04-21 04:39:07.123727380 +0200
> @@ -169,9 +169,8 @@
>  CONFIG_ROCKCHIP_RK3288=y
>  # CONFIG_ROCKCHIP_RK3328 is not set
>  # CONFIG_ROCKCHIP_RK3399 is not set
> -CONFIG_ROCKCHIP_SPL_BACK_TO_BROM=y
> -CONFIG_ROCKCHIP_BROM_HELPER=y
> -# CONFIG_SPL_MMC_SUPPORT is not set
> +# CONFIG_ROCKCHIP_SPL_BACK_TO_BROM is not set
> +CONFIG_SPL_MMC_SUPPORT=y
>  CONFIG_SPL_SERIAL_SUPPORT=y
>  CONFIG_BOARD_SPECIFIC_OPTIONS=y
>  # CONFIG_TARGET_CHROMEBOOK_JERRY is not set
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)


Regards,
Simon

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

[Simon Glass]

Hi Andreas,

On Apr 20, 2017 18:34, "Andreas Färber" <afaerber@suse.de> wrote:

Hi Simon,

Am 21.04.2017 um 01:44 schrieb Simon Glass:
> On 20 April 2017 at 17:23, Andreas Färber <afaerber@suse.de> wrote:
>> Using -rc2 with firefly-rk3288 defconfig I can only boot into SPL but
>> not into full U-Boot. I am using the old documented way of dd'ing to
>> sector 256 on SD (doc/README.rockchip).
>>
>> Looking at include/configs/rk3288_common.h I also tried putting
>> u-boot.img on a FAT partition, to no effect.
>>
>> v2017.03 doesn't even show SPL working.
>> v2017.01 worked okay.
>>
>> Build log for -rc2:
>> https://build.opensuse.org/build/Base:System:Staging/openSUS
E_Factory_ARM/armv7l/u-boot-firefly-rk3288/_log
>
> Can you try removing RETURN_TO_BROM (or imilsar) and adding
> CONFIG_SPL_OF_PLATDATA?

As an interim update I can share that latest master behaves slightly
differently:

U-Boot SPL 2017.05-rc2-00053-g3c476d8 (Apr 21 2017 - 02:29:48)
Returning to boot ROM...

compared to:

U-Boot SPL 2017.05-rc2 (Apr 20 2017 - 14:33:20)

Will tweak options next.


Yes I got sick of it hanging with no message!

Regards,
Simon



Thanks,
Andreas

--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

[Andreas Färber]

Am 21.04.2017 um 01:23 schrieb Andreas Färber:
> Hi,
> 
> Using -rc2 with firefly-rk3288 defconfig I can only boot into SPL but
> not into full U-Boot. I am using the old documented way of dd'ing to
> sector 256 on SD (doc/README.rockchip).
> 
> Looking at include/configs/rk3288_common.h I also tried putting
> u-boot.img on a FAT partition, to no effect.
> 
> v2017.03 doesn't even show SPL working.
> v2017.01 worked okay.

Cutting a long thread short: U-Boot can only handle either BROM-style
booting or SPL-style booting. Since v2017.03 booting defaults to BROM,
which requires u-boot.bin immediately after the rksd, as opposed to
u-boot.img at sector 256 as before.

That works for -rc2, but still leaves v2017.03 without serial output.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)