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