[U-Boot] [PATCH 1/1] usb: gadget: g_dnl: Use serial# variable to set g_dnl_serial

Since 842778a09104 commit, "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, before checking g_dnl_serial, let's re-check if we have valid serial# value. And if so, let's set it as g_dnl_serial value.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org --- drivers/usb/gadget/g_dnl.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 0491a0eea9..e4d0289757 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -226,6 +226,15 @@ static int g_dnl_bind(struct usb_composite_dev *cdev)
g_dnl_bind_fixup(&device_desc, cdev->driver->name);
+ /* First try to obtain serial number from serial# variable */ + if (strlen(g_dnl_serial) == 0) { + const char *s; + + s = env_get("serial#"); + if (s) + g_dnl_set_serialnumber((char *)s); + } + if (strlen(g_dnl_serial)) { id = usb_string_id(cdev); if (id < 0)

On 31 August 2017 at 21:20, Sam Protsenko semen.protsenko@linaro.org wrote:
Since 842778a09104 commit, "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, before checking g_dnl_serial, let's re-check if we have valid serial# value. And if so, let's set it as g_dnl_serial value.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
drivers/usb/gadget/g_dnl.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 0491a0eea9..e4d0289757 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -226,6 +226,15 @@ static int g_dnl_bind(struct usb_composite_dev *cdev)
g_dnl_bind_fixup(&device_desc, cdev->driver->name);
/* First try to obtain serial number from serial# variable */
if (strlen(g_dnl_serial) == 0) {
const char *s;
s = env_get("serial#");
if (s)
g_dnl_set_serialnumber((char *)s);
}
if (strlen(g_dnl_serial)) { id = usb_string_id(cdev); if (id < 0)
-- 2.14.1
Similar commit: [1]. But I'm not sure that implementing g_dnl_bind_fixup() in board file is the best way to fix it.
[1] http://git.denx.de/?p=u-boot.git;a=commit;h=e91ead868b536d0a0dc49aa48f4d8c80...

On 08/31/2017 08:45 PM, Sam Protsenko wrote:
On 31 August 2017 at 21:20, Sam Protsenko semen.protsenko@linaro.org wrote:
Since 842778a09104 commit, "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, before checking g_dnl_serial, let's re-check if we have valid serial# value. And if so, let's set it as g_dnl_serial value.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
drivers/usb/gadget/g_dnl.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 0491a0eea9..e4d0289757 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -226,6 +226,15 @@ static int g_dnl_bind(struct usb_composite_dev *cdev)
g_dnl_bind_fixup(&device_desc, cdev->driver->name);
/* First try to obtain serial number from serial# variable */
if (strlen(g_dnl_serial) == 0) {
const char *s;
s = env_get("serial#");
if (s)
g_dnl_set_serialnumber((char *)s);
}
if (strlen(g_dnl_serial)) { id = usb_string_id(cdev); if (id < 0)
-- 2.14.1
Similar commit: [1]. But I'm not sure that implementing g_dnl_bind_fixup() in board file is the best way to fix it.
[1] http://git.denx.de/?p=u-boot.git;a=commit;h=e91ead868b536d0a0dc49aa48f4d8c80...
I would opt for having g_dnl_bind_fixup() since it is generally used for tweaking board specific data to gadget subsystem (like vid, pid, serial number).

On 08/31/2017 08:20 PM, Sam Protsenko wrote:
Since 842778a09104 commit, "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, before checking g_dnl_serial, let's re-check if we have valid serial# value. And if so, let's set it as g_dnl_serial value.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
Can't we use U_BOOT_ENV_CALLBACK() here instead and set the serial number when the environment variable is set ?
drivers/usb/gadget/g_dnl.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 0491a0eea9..e4d0289757 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -226,6 +226,15 @@ static int g_dnl_bind(struct usb_composite_dev *cdev)
g_dnl_bind_fixup(&device_desc, cdev->driver->name);
- /* First try to obtain serial number from serial# variable */
- if (strlen(g_dnl_serial) == 0) {
const char *s;
s = env_get("serial#");
if (s)
g_dnl_set_serialnumber((char *)s);
- }
- if (strlen(g_dnl_serial)) { id = usb_string_id(cdev); if (id < 0)

On 09/01/2017 10:19 AM, Marek Vasut wrote:
On 08/31/2017 08:20 PM, Sam Protsenko wrote:
Since 842778a09104 commit, "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, before checking g_dnl_serial, let's re-check if we have valid serial# value. And if so, let's set it as g_dnl_serial value.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
Can't we use U_BOOT_ENV_CALLBACK() here instead and set the serial number when the environment variable is set ?
Hmm... This might work.
However, I'm a bit concerned with having "bound" the one particular env variable ("serial#" in this case) to generic g_dnl.c code.
drivers/usb/gadget/g_dnl.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 0491a0eea9..e4d0289757 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -226,6 +226,15 @@ static int g_dnl_bind(struct usb_composite_dev *cdev)
g_dnl_bind_fixup(&device_desc, cdev->driver->name);
- /* First try to obtain serial number from serial# variable */
- if (strlen(g_dnl_serial) == 0) {
const char *s;
s = env_get("serial#");
if (s)
g_dnl_set_serialnumber((char *)s);
- }
- if (strlen(g_dnl_serial)) { id = usb_string_id(cdev); if (id < 0)

On 09/01/2017 10:36 AM, Łukasz Majewski wrote:
On 09/01/2017 10:19 AM, Marek Vasut wrote:
On 08/31/2017 08:20 PM, Sam Protsenko wrote:
Since 842778a09104 commit, "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, before checking g_dnl_serial, let's re-check if we have valid serial# value. And if so, let's set it as g_dnl_serial value.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
Can't we use U_BOOT_ENV_CALLBACK() here instead and set the serial number when the environment variable is set ?
Hmm... This might work.
However, I'm a bit concerned with having "bound" the one particular env variable ("serial#" in this case) to generic g_dnl.c code.
That's a separate problem though, we already have this serial# variable in place.

On 09/01/2017 11:51 AM, Marek Vasut wrote:
On 09/01/2017 10:36 AM, Łukasz Majewski wrote:
On 09/01/2017 10:19 AM, Marek Vasut wrote:
On 08/31/2017 08:20 PM, Sam Protsenko wrote:
Since 842778a09104 commit, "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, before checking g_dnl_serial, let's re-check if we have valid serial# value. And if so, let's set it as g_dnl_serial value.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
Can't we use U_BOOT_ENV_CALLBACK() here instead and set the serial number when the environment variable is set ?
Hmm... This might work.
However, I'm a bit concerned with having "bound" the one particular env variable ("serial#" in this case) to generic g_dnl.c code.
That's a separate problem though, we already have this serial# variable in place.
Do you mean that it is the same of "fixed" variable, as "ipaddr" is for network subsytem?

On 1 September 2017 at 11:19, Marek Vasut marex@denx.de wrote:
On 08/31/2017 08:20 PM, Sam Protsenko wrote:
Since 842778a09104 commit, "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, before checking g_dnl_serial, let's re-check if we have valid serial# value. And if so, let's set it as g_dnl_serial value.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
Can't we use U_BOOT_ENV_CALLBACK() here instead and set the serial number when the environment variable is set ?
Good point. I've tested this approach and it works fine. Will resend v2 shortly. Thanks for the hint!
drivers/usb/gadget/g_dnl.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 0491a0eea9..e4d0289757 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -226,6 +226,15 @@ static int g_dnl_bind(struct usb_composite_dev *cdev)
g_dnl_bind_fixup(&device_desc, cdev->driver->name);
/* First try to obtain serial number from serial# variable */
if (strlen(g_dnl_serial) == 0) {
const char *s;
s = env_get("serial#");
if (s)
g_dnl_set_serialnumber((char *)s);
}
if (strlen(g_dnl_serial)) { id = usb_string_id(cdev); if (id < 0)
-- Best regards, Marek Vasut

On 1 September 2017 at 14:58, Sam Protsenko semen.protsenko@linaro.org wrote:
On 1 September 2017 at 11:19, Marek Vasut marex@denx.de wrote:
On 08/31/2017 08:20 PM, Sam Protsenko wrote:
Since 842778a09104 commit, "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, before checking g_dnl_serial, let's re-check if we have valid serial# value. And if so, let's set it as g_dnl_serial value.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
Can't we use U_BOOT_ENV_CALLBACK() here instead and set the serial number when the environment variable is set ?
Good point. I've tested this approach and it works fine. Will resend v2 shortly. Thanks for the hint!
Sent v2, using U_BOOT_ENV_CALLBACK: [1]. Guys, please review.
[1] https://lists.denx.de/pipermail/u-boot/2017-September/304460.html
drivers/usb/gadget/g_dnl.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 0491a0eea9..e4d0289757 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -226,6 +226,15 @@ static int g_dnl_bind(struct usb_composite_dev *cdev)
g_dnl_bind_fixup(&device_desc, cdev->driver->name);
/* First try to obtain serial number from serial# variable */
if (strlen(g_dnl_serial) == 0) {
const char *s;
s = env_get("serial#");
if (s)
g_dnl_set_serialnumber((char *)s);
}
if (strlen(g_dnl_serial)) { id = usb_string_id(cdev); if (id < 0)
-- Best regards, Marek Vasut
participants (3)
-
Marek Vasut
-
Sam Protsenko
-
Łukasz Majewski