[U-Boot] [ANN] 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.
Thanks all!
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@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@pollux [ 7:24:31] ttbott>
Any ideas?
bye, Heiko
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@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@pollux [ 7:24:31] ttbott>
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:
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@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;...
but may this does not work correct and now pops up.
I try to find out more, thanks for the hint!
bye, Heiko
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@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;...
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:
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@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;...
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@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@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@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
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 stringand 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@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;...
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@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@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@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.
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 numberstring 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@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;...
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@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@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@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@denx.de
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@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;...
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@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@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@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
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@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;...
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@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@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@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@denx.de
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@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;... > > > > 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@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@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@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@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
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@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;... >> >> >> >> 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@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@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@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@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
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@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;... >>> >>> >>> >>> 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@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@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@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@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@denx.de
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 anempty 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@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@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@denx.de
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/ar...
Regards, Andreas
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/ar...
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
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/ar...
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
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/ar...
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
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/ar...
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)
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/ar...
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
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/ar...
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
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
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
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)
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
participants (7)
-
Andreas Färber -
Felipe Balbi -
Heiko Schocher -
Lukasz Majewski -
Marek Vasut -
Simon Glass -
Tom Rini