U-Boot v2017.05-rc2 released

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.

Hello Tom,

added Lukasz, Marek and Felipe,

Am 18.04.2017 um 00:22 schrieb Tom Rini:

Quoted 8 lines

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 <[email protected]>
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 <[email protected]>

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?

Quoted 34 lines

Hello Tom,

added Lukasz, Marek and Felipe,

Am 18.04.2017 um 00:22 schrieb Tom Rini:

Quoted 8 lines

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 <[email protected]>
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 <[email protected]>

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

Quoted 2 lines

Any ideas?

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

Hello Marek,

Am 19.04.2017 um 10:43 schrieb Marek Vasut:

Quoted 38 lines · Heiko Schocher

Quoted 37 lines

Hello Tom,

added Lukasz, Marek and Felipe,

Am 18.04.2017 um 00:22 schrieb Tom Rini:

Quoted 10 lines

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 <[email protected]>
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 <[email protected]>

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

[...]

Quoted 5 lines

Quoted 2 lines

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!

Quoted 53 lines

Hello Marek,

Am 19.04.2017 um 10:43 schrieb Marek Vasut:

Quoted 38 lines · Heiko Schocher

Quoted 37 lines

Hello Tom,

added Lukasz, Marek and Felipe,

Am 18.04.2017 um 00:22 schrieb Tom Rini:

Quoted 10 lines

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 <[email protected]>
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 <[email protected]>

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

[...]

Quoted 5 lines

Quoted 2 lines

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

Quoted 3 lines

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.

Hello Marek,

Am 19.04.2017 um 11:51 schrieb Marek Vasut:

Quoted 60 lines · Heiko Schocher

Quoted 59 lines

Hello Marek,

Am 19.04.2017 um 10:43 schrieb Marek Vasut:

Quoted 42 lines · Heiko Schocher

Quoted 41 lines

Hello Tom,

added Lukasz, Marek and Felipe,

Am 18.04.2017 um 00:22 schrieb Tom Rini:

Quoted 12 lines

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 <[email protected]>
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 <[email protected]>

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

[...]

Quoted 6 lines

Quoted 2 lines

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

Quoted 6 lines

Quoted 3 lines

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:

board/siemens/smartweb/smartweb.c +14 -0

index 78a7946..01a3dd2 100644
@@ -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:

drivers/usb/gadget/g_dnl.c +1 -1

index d4bee9b..813d4b8 100644
@@ -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?

Quoted 195 lines

Hello Marek,

Am 19.04.2017 um 11:51 schrieb Marek Vasut:

Quoted 64 lines · Heiko Schocher

Quoted 63 lines

Hello Marek,

Am 19.04.2017 um 10:43 schrieb Marek Vasut:

Quoted 46 lines · Heiko Schocher

Quoted 45 lines

Hello Tom,

added Lukasz, Marek and Felipe,

Am 18.04.2017 um 00:22 schrieb Tom Rini:

Quoted 13 lines

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 <[email protected]>
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 <[email protected]>

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

[...]

Quoted 6 lines

Quoted 2 lines

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

Quoted 6 lines

Quoted 3 lines

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:

board/siemens/smartweb/smartweb.c +3 -0

index 78a7946..01a3dd2 100644
@@ -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) +{

board/siemens/smartweb/smartweb.c +10 -0

+       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:

drivers/usb/gadget/g_dnl.c +1 -1

index d4bee9b..813d4b8 100644
@@ -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;

Quoted 4 lines

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.

Dear All,

Quoted 196 lines · Heiko Schocher

Quoted 195 lines

Hello Marek,

Am 19.04.2017 um 11:51 schrieb Marek Vasut:

Quoted 64 lines · Heiko Schocher

Quoted 63 lines

Hello Marek,

Am 19.04.2017 um 10:43 schrieb Marek Vasut:

Quoted 46 lines · Heiko Schocher

Quoted 45 lines

Hello Tom,

added Lukasz, Marek and Felipe,

Am 18.04.2017 um 00:22 schrieb Tom Rini:

Quoted 13 lines

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 <[email protected]>
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 <[email protected]>

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

[...]

Quoted 6 lines

Quoted 2 lines

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

Quoted 6 lines

Quoted 3 lines

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:

board/siemens/smartweb/smartweb.c +3 -0

index 78a7946..01a3dd2 100644
@@ -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) +{

board/siemens/smartweb/smartweb.c +10 -0

+       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:

drivers/usb/gadget/g_dnl.c +1 -1

index d4bee9b..813d4b8 100644
@@ -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;

Quoted 8 lines

Quoted 4 lines

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

Hello all,

Am 19.04.2017 um 14:07 schrieb Lukasz Majewski:

Quoted 223 lines

Dear All,

Quoted 197 lines · Heiko Schocher

Quoted 196 lines

Hello Marek,

Am 19.04.2017 um 11:51 schrieb Marek Vasut:

Quoted 64 lines · Heiko Schocher

Quoted 63 lines

Hello Marek,

Am 19.04.2017 um 10:43 schrieb Marek Vasut:

Quoted 46 lines · Heiko Schocher

Quoted 45 lines

Hello Tom,

added Lukasz, Marek and Felipe,

Am 18.04.2017 um 00:22 schrieb Tom Rini:

Quoted 13 lines

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 <[email protected]>
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
<[email protected]> 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

[...]

Quoted 6 lines

Quoted 2 lines

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

Quoted 6 lines

Quoted 3 lines

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:

board/siemens/smartweb/smartweb.c +3 -0

index 78a7946..01a3dd2 100644
@@ -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) +{

board/siemens/smartweb/smartweb.c +10 -0

+       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:

(quoted diff) +0 -0

b/drivers/usb/gadget/g_dnl.c index d4bee9b..813d4b8 100644

drivers/usb/gadget/g_dnl.c +1 -1

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

Quoted 8 lines

Quoted 4 lines

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?

Quoted 8 lines

[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

Quoted 188 lines

Quoted 188 lines

Quoted 169 lines

Quoted 169 lines

Quoted 40 lines

Quoted 40 lines

Quoted 26 lines

Quoted 26 lines

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 <[email protected]>
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
<[email protected]> 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

[...]

Quoted 6 lines

Quoted 2 lines

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

Quoted 6 lines

Quoted 3 lines

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:

board/siemens/smartweb/smartweb.c +3 -0

index 78a7946..01a3dd2 100644
@@ -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) +{

board/siemens/smartweb/smartweb.c +10 -0

+       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:

(quoted diff) +0 -0

b/drivers/usb/gadget/g_dnl.c index d4bee9b..813d4b8 100644

drivers/usb/gadget/g_dnl.c +1 -1

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

Quoted 8 lines

Quoted 4 lines

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.

Quoted 5 lines

Quoted 5 lines

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

Quoted 4 lines

Quoted 4 lines

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

Best regards,

Quoted 4 lines

Quoted 1 line

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.

Quoted 2 lines

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 <[email protected]>
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 <[email protected]>

---
 drivers/usb/gadget/g_dnl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/usb/gadget/g_dnl.c +2 -1

index d4bee9b..0491a0e 100644
@@ -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;

Signature · 1 line

2.1.4

Hi,

Quoted 188 lines · Lukasz Majewski

Quoted 188 lines

Quoted 188 lines

Quoted 169 lines

Quoted 169 lines

Quoted 40 lines

Quoted 40 lines

Quoted 26 lines

Quoted 26 lines

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 <[email protected]>
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
<[email protected]> 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

[...]

Quoted 6 lines

Quoted 2 lines

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

Quoted 6 lines

Quoted 3 lines

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:

board/siemens/smartweb/smartweb.c +3 -0

index 78a7946..01a3dd2 100644
@@ -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) +{

board/siemens/smartweb/smartweb.c +10 -0

+       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:

(quoted diff) +0 -0

b/drivers/usb/gadget/g_dnl.c index d4bee9b..813d4b8 100644

drivers/usb/gadget/g_dnl.c +1 -1

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

Quoted 8 lines

Quoted 4 lines

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.

Quoted 7 lines

Quoted 5 lines

Quoted 5 lines

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.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

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

Best regards,

Quoted 8 lines

Quoted 4 lines

Quoted 1 line

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

Quoted 30 lines

Quoted 2 lines

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 <[email protected]>
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).

Quoted 4 lines

This commit moves the g_dnl_bind_fixup() call before the g_dnl_serial length test.

Signed-off-by Lukasz Majewski <[email protected]>

looks good to me.

Quoted 29 lines

---
  drivers/usb/gadget/g_dnl.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

drivers/usb/gadget/g_dnl.c +2 -2

index d4bee9b..0491a0e 100644
@@ -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

Hello Felipe, Lukasz,

Am 26.06.2017 um 12:22 schrieb Felipe Balbi:

Quoted 256 lines

Hi,

Quoted 189 lines · Lukasz Majewski

Quoted 189 lines

Quoted 189 lines

Quoted 169 lines

Quoted 169 lines

Quoted 40 lines

Quoted 40 lines

Quoted 26 lines

Quoted 26 lines

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 <[email protected]>
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
<[email protected]> 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

[...]

Quoted 6 lines

Quoted 2 lines

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

Quoted 6 lines

Quoted 3 lines

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:

board/siemens/smartweb/smartweb.c +3 -0

index 78a7946..01a3dd2 100644
@@ -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) +{

board/siemens/smartweb/smartweb.c +10 -0

+       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:

(quoted diff) +0 -0

b/drivers/usb/gadget/g_dnl.c index d4bee9b..813d4b8 100644

drivers/usb/gadget/g_dnl.c +1 -1

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

Quoted 9 lines

Quoted 5 lines

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.

Quoted 7 lines

Quoted 5 lines

Quoted 5 lines

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.

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

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

Best regards,

Lukasz Majewski

Quoted 7 lines

Quoted 2 lines

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

Quoted 33 lines

Quoted 2 lines

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 <[email protected]>
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.

Quoted 3 lines

Quoted 1 line

Signed-off-by Lukasz Majewski <[email protected]>

looks good to me.

Acked-by Heiko Schocher <[email protected]>

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

Thanks!

bye,
Heiko

Quoted 30 lines

Quoted 29 lines

---
  drivers/usb/gadget/g_dnl.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/drivers/usb/gadget/g_dnl.c index d4bee9b..0491a0e 100644

drivers/usb/gadget/g_dnl.c +2 -2

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

Hello Tom,

added Lukasz, Marek and Felipe,

Am 18.04.2017 um 00:22 schrieb Tom Rini:

Quoted 9 lines

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
<[email protected]> 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 <[email protected]>

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

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

Quoted 7 lines

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.

Quoted 5 lines

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?

Hi Simon,

Am 21.04.2017 um 01:44 schrieb Simon Glass:

Quoted 8 lines · Andreas Färber

Quoted 7 lines

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.

Quoted 8 lines

Quoted 5 lines

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)

Am 21.04.2017 um 02:34 schrieb Andreas Färber:

Quoted 23 lines

Hi Simon,

Am 21.04.2017 um 01:44 schrieb Simon Glass:

Quoted 8 lines · Andreas Färber

Quoted 7 lines

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.

Quoted 8 lines

Quoted 5 lines

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:

Quoted 6 lines

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)

Quoted 36 lines

Am 21.04.2017 um 02:34 schrieb Andreas Färber:

Quoted 21 lines

Am 21.04.2017 um 01:44 schrieb Simon Glass:

Quoted 8 lines · Andreas Färber

Quoted 7 lines

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.

Quoted 8 lines

Quoted 5 lines

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:

Quoted 6 lines

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

Quoted 2 lines

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?

Hi Simon,

Am 21.04.2017 um 04:10 schrieb Simon Glass:

Quoted 40 lines · Andreas Färber

Quoted 38 lines

Am 21.04.2017 um 02:34 schrieb Andreas Färber:

Quoted 21 lines

Am 21.04.2017 um 01:44 schrieb Simon Glass:

Quoted 8 lines · Andreas Färber

Quoted 7 lines

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.

Quoted 8 lines

Quoted 5 lines

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:

Quoted 6 lines

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.

Quoted 2 lines

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.

Am 21.04.2017 um 04:24 schrieb Andreas Färber:

Quoted 11 lines

Hi Simon,

Am 21.04.2017 um 04:10 schrieb Simon Glass:

Quoted 2 lines

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

simon_config renamed from .config +2 -3

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

simon_config renamed from .config +2 -1

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

Am 21.04.2017 um 04:43 schrieb Andreas Färber:

Quoted 49 lines

Am 21.04.2017 um 04:24 schrieb Andreas Färber:

Quoted 11 lines

Hi Simon,

Am 21.04.2017 um 04:10 schrieb Simon Glass:

Quoted 2 lines

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

simon_config 2017-04-21 04:39:07.123727380 +0200 renamed from .config 2017-04-21 04:39:49.484215791 +0200 +2 -3

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

simon_config 2017-04-21 04:39:07.123727380 +0200 renamed from .config 2017-04-21 04:39:49.484215791 +0200 +2 -1

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

Quoted 1 line

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

simon_config 2017-04-21 04:39:07.123727380 +0200 renamed from .config 2017-04-21 04:47:11.165310486 +0200 +2 -4

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

Quoted 4 lines

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

Quoted 8 lines

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

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

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.