Re: [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
Lukasz
diff --git a/common/dfu.c b/common/dfu.c index 0e9f5f5..fa77526 100644 --- a/common/dfu.c +++ b/common/dfu.c @@ -87,6 +87,9 @@ exit: g_dnl_unregister(); board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
+#ifdef CONFIG_SPL_BUILD
- dfu_reset = 0;
+#endif
Why do you only ifdef this part? What problem does this solve?
Common/dfu.c is common code for SPL and U-boot, for SPL-DFU dfu_reset should not be given. This is must fix. Also this avoid use of run_command for SPL-DFU altogether, SPL size also will reduce by removing cli.c/cli_hush.c
Regards Ravi
On Thu, 27 Apr 2017 08:26:57 +0000 "B, Ravi" ravibabu@ti.com wrote:
Lukasz
diff --git a/common/dfu.c b/common/dfu.c index 0e9f5f5..fa77526 100644 --- a/common/dfu.c +++ b/common/dfu.c @@ -87,6 +87,9 @@ exit: g_dnl_unregister(); board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
+#ifdef CONFIG_SPL_BUILD
- dfu_reset = 0;
+#endif
Why do you only ifdef this part? What problem does this solve?
Common/dfu.c is common code for SPL and U-boot, for SPL-DFU dfu_reset should not be given. This is must fix. Also this avoid use of run_command for SPL-DFU altogether, SPL size also will reduce by removing cli.c/cli_hush.c
As I've metioned in the other mail. Kconfig option would be OK.
Please look into the dfu_usb_get_reset() __weak function definition.
It is by default set to true.
You can extend this function to take into account a Kconfig option to return false during SPL builds.
Regards Ravi
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,
On Thu, 27 Apr 2017 08:26:57 +0000 "B, Ravi" ravibabu@ti.com wrote:
Lukasz
diff --git a/common/dfu.c b/common/dfu.c index 0e9f5f5..fa77526 100644 --- a/common/dfu.c +++ b/common/dfu.c @@ -87,6 +87,9 @@ exit: g_dnl_unregister(); board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
+#ifdef CONFIG_SPL_BUILD
- dfu_reset = 0;
+#endif
Why do you only ifdef this part? What problem does this solve?
Common/dfu.c is common code for SPL and U-boot, for SPL-DFU dfu_reset should not be given. This is must fix. Also this avoid use of run_command for SPL-DFU altogether, SPL size also will reduce by removing cli.c/cli_hush.c
As I've metioned in the other mail. Kconfig option would be OK.
And this Kconfig should be _only_ enabled for your SPL-DFU support enabled (also in Kconfig).
Please look into the dfu_usb_get_reset() __weak function definition.
It is by default set to true.
You can extend this function to take into account a Kconfig option to return false during SPL builds.
Regards Ravi
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
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
+#ifdef CONFIG_SPL_BUILD
- dfu_reset = 0;
+#endif
Why do you only ifdef this part? What problem does this solve?
Common/dfu.c is common code for SPL and U-boot, for SPL-DFU dfu_reset should not be given. This is must fix. Also this avoid use of run_command for SPL-DFU altogether, SPL size also will reduce by removing cli.c/cli_hush.c
As I've metioned in the other mail. Kconfig option would be OK.
And this Kconfig should be _only_ enabled for your SPL-DFU support enabled (also in Kconfig).
I have already added CONFIG_SPL_DFU_NO_RESET option for SPL-DFU in Kconfig.
Please look into the dfu_usb_get_reset() __weak function definition.
It is by default set to true.
You can extend this function to take into account a Kconfig option to return false during SPL builds.
As suggested by you, I feel this is best option.
I have test verified this option, will post in next patch version.
__weak bool dfu_usb_get_reset(void) { +#ifdef CONFIG_SPL_DFU_NO_RESET + return false +#else return true; +#endif }
Also changing run_command() to do_reset().
If (dfu_reset) do_reset(NULL, 0, 0, NULL);
Regards Ravi
Hi Ravi,
Hi Lukasz
+#ifdef CONFIG_SPL_BUILD
- dfu_reset = 0;
+#endif
Why do you only ifdef this part? What problem does this solve?
Common/dfu.c is common code for SPL and U-boot, for SPL-DFU dfu_reset should not be given. This is must fix. Also this avoid use of run_command for SPL-DFU altogether, SPL size also will reduce by removing cli.c/cli_hush.c
As I've metioned in the other mail. Kconfig option would be OK.
And this Kconfig should be _only_ enabled for your SPL-DFU support enabled (also in Kconfig).
I have already added CONFIG_SPL_DFU_NO_RESET option for SPL-DFU in Kconfig.
Please look into the dfu_usb_get_reset() __weak function definition.
It is by default set to true.
You can extend this function to take into account a Kconfig option to return false during SPL builds.
As suggested by you, I feel this is best option.
I have test verified this option, will post in next patch version.
__weak bool dfu_usb_get_reset(void) { +#ifdef CONFIG_SPL_DFU_NO_RESET
- return false
+#else return true; +#endif }
Also changing run_command() to do_reset().
If (dfu_reset) do_reset(NULL, 0, 0, NULL);
+1
One question - could you write some numbers before SPL dfu tinification and afterwards?
I'm just curious how much we can save up.
Regards Ravi
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,
Also changing run_command() to do_reset().
If (dfu_reset) do_reset(NULL, 0, 0, NULL);
+1
One question - could you write some numbers before SPL dfu tinification and afterwards?
I'm just curious how much we can save up.
I am OOO till May 2nd. Will provide more details once I am back.
Regards Ravi
participants (2)
-
B, Ravi -
Lukasz Majewski