[U-Boot] [PATCH v3 1/1] usb: gadget: g_dnl: Sync internal SN variable with env

[U-Boot] [PATCH v3 1/1] usb: gadget: g_dnl: Sync internal SN variable with env

[Sam Protsenko]

Since commit 842778a09104 ("usb: gadget: g_dnl: only set iSerialNumber
if we have a serial#") "fastboot devices" stopped to show correct device
serial number for TI boards, showing this line instead:

    ????????????	fastboot

This is because serial# env variable could be set after g_dnl gadget was
initialized (e.g. by using env_set() in the board file).

To fix this, let's update internal serial number variable (g_dnl_serial)
when "serial#" env var is changed.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
---
 drivers/usb/gadget/g_dnl.c | 15 +++++++++++++++
 include/env_callback.h     |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
index 0491a0eea9..039331a5af 100644
--- a/drivers/usb/gadget/g_dnl.c
+++ b/drivers/usb/gadget/g_dnl.c
@@ -19,6 +19,8 @@
 #include <dfu.h>
 #include <thor.h>
 
+#include <env_callback.h>
+
 #include "gadget_chips.h"
 #include "composite.c"
 
@@ -202,6 +204,19 @@ static int g_dnl_get_bcd_device_number(struct usb_composite_dev *cdev)
 	return g_dnl_get_board_bcd_device_number(gcnum);
 }
 
+/**
+ * Update internal serial number variable when the "serial#" env var changes.
+ *
+ * Handle all cases, even when flags == H_PROGRAMMATIC or op == env_op_delete.
+ */
+static int on_serialno(const char *name, const char *value, enum env_op op,
+		int flags)
+{
+	g_dnl_set_serialnumber((char *)value);
+	return 0;
+}
+U_BOOT_ENV_CALLBACK(serialno, on_serialno);
+
 static int g_dnl_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget *gadget = cdev->gadget;
diff --git a/include/env_callback.h b/include/env_callback.h
index 90b95b5e66..5c4a30c2de 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -72,6 +72,7 @@
 	SILENT_CALLBACK \
 	SPLASHIMAGE_CALLBACK \
 	"stdin:console,stdout:console,stderr:console," \
+	"serial#:serialno," \
 	CONFIG_ENV_CALLBACK_LIST_STATIC
 
 struct env_clbk_tbl {
-- 
2.14.1

[U-Boot] [PATCH v3 1/1] usb: gadget: g_dnl: Sync internal SN variable with env

[Łukasz Majewski]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 2477 bytes --]

Hi Heiko,

Would you find some time and run this patch through your test setup?

Thanks in advance.

Best regards,
Łukasz


> Since commit 842778a09104 ("usb: gadget: g_dnl: only set iSerialNumber
> if we have a serial#") "fastboot devices" stopped to show correct device
> serial number for TI boards, showing this line instead:
>
>     ????????????	fastboot
>
> This is because serial# env variable could be set after g_dnl gadget was
> initialized (e.g. by using env_set() in the board file).
>
> To fix this, let's update internal serial number variable (g_dnl_serial)
> when "serial#" env var is changed.
>
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> ---
>  drivers/usb/gadget/g_dnl.c | 15 +++++++++++++++
>  include/env_callback.h     |  1 +
>  2 files changed, 16 insertions(+)
>
> diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
> index 0491a0eea9..039331a5af 100644
> --- a/drivers/usb/gadget/g_dnl.c
> +++ b/drivers/usb/gadget/g_dnl.c
> @@ -19,6 +19,8 @@
>  #include <dfu.h>
>  #include <thor.h>
>
> +#include <env_callback.h>
> +
>  #include "gadget_chips.h"
>  #include "composite.c"
>
> @@ -202,6 +204,19 @@ static int g_dnl_get_bcd_device_number(struct usb_composite_dev *cdev)
>  	return g_dnl_get_board_bcd_device_number(gcnum);
>  }
>
> +/**
> + * Update internal serial number variable when the "serial#" env var changes.
> + *
> + * Handle all cases, even when flags == H_PROGRAMMATIC or op == env_op_delete.
> + */
> +static int on_serialno(const char *name, const char *value, enum env_op op,
> +		int flags)
> +{
> +	g_dnl_set_serialnumber((char *)value);
> +	return 0;
> +}
> +U_BOOT_ENV_CALLBACK(serialno, on_serialno);
> +
>  static int g_dnl_bind(struct usb_composite_dev *cdev)
>  {
>  	struct usb_gadget *gadget = cdev->gadget;
> diff --git a/include/env_callback.h b/include/env_callback.h
> index 90b95b5e66..5c4a30c2de 100644
> --- a/include/env_callback.h
> +++ b/include/env_callback.h
> @@ -72,6 +72,7 @@
>  	SILENT_CALLBACK \
>  	SPLASHIMAGE_CALLBACK \
>  	"stdin:console,stdout:console,stderr:console," \
> +	"serial#:serialno," \
>  	CONFIG_ENV_CALLBACK_LIST_STATIC
>
>  struct env_clbk_tbl {
>


-- 
Best regards,

Lukasz Majewski

--

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

[U-Boot] [PATCH v3 1/1] usb: gadget: g_dnl: Sync internal SN variable with env

[Heiko Schocher]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 2093 bytes --]

Hello Lukasz,

Am 02.09.2017 um 13:08 schrieb Łukasz Majewski:
> Hi Heiko,
>
> Would you find some time and run this patch through your test setup?
>
> Thanks in advance.

Of course, as it is automated ;-)

Okay, I had to add an oneliner, to say tbot, which patchwork patch it
has to download... for the records see [1].

Here the results for the smartweb board (in short, my dfu test are
working with this patch):

http://xeidos.ddns.net/tests/test_db_auslesen.php#408

Logs:
http://xeidos.ddns.net/tbot/id_408/html_log.html

Especially the steps:

(download patchwork patch, check with checkpatch, apply to mainline):
http://xeidos.ddns.net/tbot/id_408/html_log.html#92
http://xeidos.ddns.net/tbot/id_408/html_log.html#95
http://xeidos.ddns.net/tbot/id_408/html_log.html#98

and the dfu test:

http://xeidos.ddns.net/tbot/id_408/html_log.html#398
http://xeidos.ddns.net/tbot/id_408/html_log.html#409
http://xeidos.ddns.net/tbot/id_408/html_log.html#413
http://xeidos.ddns.net/tbot/id_408/html_log.html#417

My browser (firefox) seems to have problems with jumping to the correct
"id"s in the html file ...

Hope you can find the logs, if not, look into the raw logfile:

http://xeidos.ddns.net/tbot/id_408/tbot.txt

but this is not very comfortable ...

So, please add my:

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

bye,
Heiko

[1] tbot patch for adding a patchworkpatch

root at raspberrypi:/home/pi/data/tbot# git diff
diff --git a/config/smartweb.py b/config/smartweb.py
index 72a7972..98c8e0f 100644
--- a/config/smartweb.py
+++ b/config/smartweb.py
@@ -37,6 +37,8 @@ tc_workfd_apply_patchwork_patches_list_hand = [
]
tc_workfd_apply_patchwork_patches_blacklist = ['204183', '561384']

+tc_workfd_apply_patchwork_patches_list = ['808671']
+
uboot_get_parameter_file_list = ['.config', 'include/configs/smartweb.h', 
'arch/arm/mach-at91/include/mach/at91sam9260.h']

tc_workfd_set_toolchain_arch = 'arm'
root at raspberrypi:/home/pi/data/tbot#

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

[U-Boot] [PATCH v3 1/1] usb: gadget: g_dnl: Sync internal SN variable with env

[Sam Protsenko]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 2467 bytes --]

On 4 September 2017 at 08:36, Heiko Schocher <hs@denx.de> wrote:
> Hello Lukasz,
>
> Am 02.09.2017 um 13:08 schrieb Łukasz Majewski:
>>
>> Hi Heiko,
>>
>> Would you find some time and run this patch through your test setup?
>>
>> Thanks in advance.
>
>
> Of course, as it is automated ;-)
>
> Okay, I had to add an oneliner, to say tbot, which patchwork patch it
> has to download... for the records see [1].
>
> Here the results for the smartweb board (in short, my dfu test are
> working with this patch):
>
> http://xeidos.ddns.net/tests/test_db_auslesen.php#408
>
> Logs:
> http://xeidos.ddns.net/tbot/id_408/html_log.html
>
> Especially the steps:
>
> (download patchwork patch, check with checkpatch, apply to mainline):
> http://xeidos.ddns.net/tbot/id_408/html_log.html#92
> http://xeidos.ddns.net/tbot/id_408/html_log.html#95
> http://xeidos.ddns.net/tbot/id_408/html_log.html#98
>
> and the dfu test:
>
> http://xeidos.ddns.net/tbot/id_408/html_log.html#398
> http://xeidos.ddns.net/tbot/id_408/html_log.html#409
> http://xeidos.ddns.net/tbot/id_408/html_log.html#413
> http://xeidos.ddns.net/tbot/id_408/html_log.html#417
>
> My browser (firefox) seems to have problems with jumping to the correct
> "id"s in the html file ...
>
> Hope you can find the logs, if not, look into the raw logfile:
>
> http://xeidos.ddns.net/tbot/id_408/tbot.txt
>
> but this is not very comfortable ...
>
> So, please add my:
>
> Tested-by: Heiko Schocher <hs@denx.de>
>

Hi Lukasz,

Now that testing is done (thanks Heiko!), can you please Ack or Review
this patch? I really want it to go in v2017.09 release, as it's
critical bug fix for us.

Thanks!

> bye,
> Heiko
>
> [1] tbot patch for adding a patchworkpatch
>
> root at raspberrypi:/home/pi/data/tbot# git diff
> diff --git a/config/smartweb.py b/config/smartweb.py
> index 72a7972..98c8e0f 100644
> --- a/config/smartweb.py
> +++ b/config/smartweb.py
> @@ -37,6 +37,8 @@ tc_workfd_apply_patchwork_patches_list_hand = [
> ]
> tc_workfd_apply_patchwork_patches_blacklist = ['204183', '561384']
>
> +tc_workfd_apply_patchwork_patches_list = ['808671']
> +
> uboot_get_parameter_file_list = ['.config', 'include/configs/smartweb.h',
> 'arch/arm/mach-at91/include/mach/at91sam9260.h']
>
> tc_workfd_set_toolchain_arch = 'arm'
> root at raspberrypi:/home/pi/data/tbot#
>
>
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH v3 1/1] usb: gadget: g_dnl: Sync internal SN variable with env

[Łukasz Majewski]

Hi Sam,

> On 4 September 2017 at 08:36, Heiko Schocher <hs@denx.de> wrote:
>> Hello Lukasz,
>>
>> Am 02.09.2017 um 13:08 schrieb Łukasz Majewski:
>>>
>>> Hi Heiko,
>>>
>>> Would you find some time and run this patch through your test setup?
>>>
>>> Thanks in advance.
>>
>>
>> Of course, as it is automated ;-)
>>
>> Okay, I had to add an oneliner, to say tbot, which patchwork patch it
>> has to download... for the records see [1].
>>
>> Here the results for the smartweb board (in short, my dfu test are
>> working with this patch):
>>
>> http://xeidos.ddns.net/tests/test_db_auslesen.php#408
>>
>> Logs:
>> http://xeidos.ddns.net/tbot/id_408/html_log.html
>>
>> Especially the steps:
>>
>> (download patchwork patch, check with checkpatch, apply to mainline):
>> http://xeidos.ddns.net/tbot/id_408/html_log.html#92
>> http://xeidos.ddns.net/tbot/id_408/html_log.html#95
>> http://xeidos.ddns.net/tbot/id_408/html_log.html#98
>>
>> and the dfu test:
>>
>> http://xeidos.ddns.net/tbot/id_408/html_log.html#398
>> http://xeidos.ddns.net/tbot/id_408/html_log.html#409
>> http://xeidos.ddns.net/tbot/id_408/html_log.html#413
>> http://xeidos.ddns.net/tbot/id_408/html_log.html#417
>>
>> My browser (firefox) seems to have problems with jumping to the correct
>> "id"s in the html file ...
>>
>> Hope you can find the logs, if not, look into the raw logfile:
>>
>> http://xeidos.ddns.net/tbot/id_408/tbot.txt
>>
>> but this is not very comfortable ...
>>
>> So, please add my:
>>
>> Tested-by: Heiko Schocher <hs@denx.de>
>>
> 
> Hi Lukasz,
> 
> Now that testing is done (thanks Heiko!), can you please Ack or Review
> this patch? I really want it to go in v2017.09 release, as it's
> critical bug fix for us.

No problem from my side:

Acked-by: Łukasz Majewski <lukma@denx.de>


If Marek don't mind - I would like to ask Tom to apply it directly to 
-master branch (because of the patch importance).


> 
> Thanks!
> 
>> bye,
>> Heiko
>>
>> [1] tbot patch for adding a patchworkpatch
>>
>> root at raspberrypi:/home/pi/data/tbot# git diff
>> diff --git a/config/smartweb.py b/config/smartweb.py
>> index 72a7972..98c8e0f 100644
>> --- a/config/smartweb.py
>> +++ b/config/smartweb.py
>> @@ -37,6 +37,8 @@ tc_workfd_apply_patchwork_patches_list_hand = [
>> ]
>> tc_workfd_apply_patchwork_patches_blacklist = ['204183', '561384']
>>
>> +tc_workfd_apply_patchwork_patches_list = ['808671']
>> +
>> uboot_get_parameter_file_list = ['.config', 'include/configs/smartweb.h',
>> 'arch/arm/mach-at91/include/mach/at91sam9260.h']
>>
>> tc_workfd_set_toolchain_arch = 'arm'
>> root at raspberrypi:/home/pi/data/tbot#
>>
>>
>> --
>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> 


-- 
Best regards,

Lukasz Majewski

--

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

[U-Boot] [PATCH v3 1/1] usb: gadget: g_dnl: Sync internal SN variable with env

[Marek Vasut]

On 09/01/2017 02:42 PM, Sam Protsenko wrote:
> Since commit 842778a09104 ("usb: gadget: g_dnl: only set iSerialNumber
> if we have a serial#") "fastboot devices" stopped to show correct device
> serial number for TI boards, showing this line instead:
> 
>     ????????????	fastboot
> 
> This is because serial# env variable could be set after g_dnl gadget was
> initialized (e.g. by using env_set() in the board file).
> 
> To fix this, let's update internal serial number variable (g_dnl_serial)
> when "serial#" env var is changed.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>

Reviewed-by: Marek Vasut <marex@denx.de>

-- 
Best regards,
Marek Vasut

[U-Boot] [U-Boot, v3, 1/1] usb: gadget: g_dnl: Sync internal SN variable with env

[Tom Rini]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1099 bytes --]

On Fri, Sep 01, 2017 at 03:42:03PM +0300, Sam Protsenko wrote:

> Since commit 842778a09104 ("usb: gadget: g_dnl: only set iSerialNumber
> if we have a serial#") "fastboot devices" stopped to show correct device
> serial number for TI boards, showing this line instead:
> 
>     ????????????	fastboot
> 
> This is because serial# env variable could be set after g_dnl gadget was
> initialized (e.g. by using env_set() in the board file).
> 
> To fix this, let's update internal serial number variable (g_dnl_serial)
> when "serial#" env var is changed.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Tested-by: Heiko Schocher <hs@denx.de>
> Acked-by: Łukasz Majewski <lukma@denx.de>
> Reviewed-by: Marek Vasut <marex@denx.de>

Applied to u-boot/master, thanks!

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