[U-Boot] [PATCH v2 0/3] spl: dfu: misc fixes and reduce MLO foot print

[U-Boot] [PATCH v2 0/3] spl: dfu: misc fixes and reduce MLO foot print

[Ravi Babu]

The patch series spl-dfu fixes includes
        - select spl-dfu only spl-ram supported
        - ignore the dfu-reset for spl-dfu
        - reduce the spl-dfu MLO foot print

buildman ran for arm targets

Change history:

v1: 
	- Kconfig fix for dfu-reset for SPL-DFU
	- compile out CONFIG_DFU_MMC for SPL-DFU
	  

Ravi Babu (3):
  spl: Kconfig: dfu: spl-dfu depends on SPL_RAM_SUPPORT
  common: dfu: ignore reset for spl-dfu
  spl: dfu: reduce spl-dfu MLO size

 common/Makefile      | 3 +--
 common/dfu.c         | 2 +-
 common/spl/Kconfig   | 5 +++++
 drivers/dfu/Makefile | 2 ++
 drivers/dfu/dfu.c    | 4 ++++
 include/dfu.h        | 2 +-
 6 files changed, 14 insertions(+), 4 deletions(-)

-- 
1.9.1

[U-Boot] [PATCH v2 1/3] spl: Kconfig: dfu: spl-dfu depends on SPL_RAM_SUPPORT

[Ravi Babu]

Since SPL_DFU_SUPPORT is depends on SPL_RAM_SUPPORT,
hence select SPL_DFU_SUPPORT only when
SPL_RAM_SUPPORT is chosen.

Signed-off-by: Ravi Babu <ravibabu@ti.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
 common/spl/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index ea6fbb6..1231351 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -646,6 +646,7 @@ config SPL_USBETH_SUPPORT
 config SPL_DFU_SUPPORT
 	bool "Support DFU (Device Firmware Upgarde)"
 	select SPL_HASH_SUPPORT
+	depends on SPL_RAM_SUPPORT
 	help
 	  This feature enables the DFU (Device Firmware Upgarde) in SPL with
 	  RAM memory device support. The ROM code will load and execute
-- 
1.9.1

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[Ravi Babu]

The SPL-DFU feature enable to load and
execute u-boot over usb from PC using
dfu-util.
Hence dfu-reset should not be issued
when dfu-util -R switch is issued.

Signed-off-by: Ravi Babu <ravibabu@ti.com>
---
 common/dfu.c       | 2 +-
 common/spl/Kconfig | 4 ++++
 drivers/dfu/dfu.c  | 4 ++++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/common/dfu.c b/common/dfu.c
index 0e9f5f5..546a1ab 100644
--- a/common/dfu.c
+++ b/common/dfu.c
@@ -88,7 +88,7 @@ exit:
 	board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
 
 	if (dfu_reset)
-		run_command("reset", 0);
+		do_reset(NULL, 0, 0, NULL);
 
 	g_dnl_clear_detach();
 
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 1231351..f51ae2c 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -6,6 +6,9 @@ config SUPPORT_SPL
 config SUPPORT_TPL
 	bool
 
+config SPL_DFU_NO_RESET
+	bool
+
 config SPL
 	bool
 	depends on SUPPORT_SPL
@@ -646,6 +649,7 @@ config SPL_USBETH_SUPPORT
 config SPL_DFU_SUPPORT
 	bool "Support DFU (Device Firmware Upgarde)"
 	select SPL_HASH_SUPPORT
+	select SPL_DFU_NO_RESET
 	depends on SPL_RAM_SUPPORT
 	help
 	  This feature enables the DFU (Device Firmware Upgarde) in SPL with
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 8dacc1a..ceb33e3 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -35,7 +35,11 @@ static struct hash_algo *dfu_hash_algo;
  */
 __weak bool dfu_usb_get_reset(void)
 {
+#ifdef CONFIG_SPL_DFU_NO_RESET
+	return false;
+#else
 	return true;
+#endif
 }
 
 static int dfu_find_alt_num(const char *s)
-- 
1.9.1

[U-Boot] [PATCH v2 3/3] spl: dfu: reduce spl-dfu MLO size

[Ravi Babu]

Since spl-dfu does not dfu-reset, there is no need
of run_command_cli, hence compiling out cli.c and
cli_hush.c to reduce the spl-dfu memory foot print.

Signed-off-by: Ravi Babu <ravibabu@ti.com>
---
need better way for how to compile out CONFIG_DFU_MMC

 common/Makefile      | 3 +--
 drivers/dfu/Makefile | 2 ++
 include/dfu.h        | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/common/Makefile b/common/Makefile
index 86225f1..8976cbc 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -11,6 +11,7 @@ obj-y += init/
 obj-y += main.o
 obj-y += exports.o
 obj-y += hash.o
+obj-y += cli.o
 obj-$(CONFIG_HUSH_PARSER) += cli_hush.o
 obj-$(CONFIG_AUTOBOOT) += autoboot.o
 
@@ -90,7 +91,6 @@ endif # !CONFIG_SPL_BUILD
 
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_DFU_SUPPORT) += dfu.o
-obj-$(CONFIG_SPL_DFU_SUPPORT) += cli_hush.o
 obj-$(CONFIG_SPL_HASH_SUPPORT) += hash.o
 obj-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o
 obj-$(CONFIG_SPL_YMODEM_SUPPORT) += xyzModem.o
@@ -171,7 +171,6 @@ endif
 # We always have this since drivers/ddr/fs/interactive.c needs it
 obj-$(CONFIG_CMDLINE) += cli_simple.o
 
-obj-y += cli.o
 obj-$(CONFIG_CMDLINE) += cli_readline.o
 obj-$(CONFIG_CMD_DFU) += dfu.o
 obj-y += command.o
diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile
index 61f2b71..5628734 100644
--- a/drivers/dfu/Makefile
+++ b/drivers/dfu/Makefile
@@ -6,7 +6,9 @@
 #
 
 obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o
+ifndef CONFIG_SPL_BUILD
 obj-$(CONFIG_DFU_MMC) += dfu_mmc.o
+endif
 obj-$(CONFIG_DFU_NAND) += dfu_nand.o
 obj-$(CONFIG_DFU_RAM) += dfu_ram.o
 obj-$(CONFIG_DFU_SF) += dfu_sf.o
diff --git a/include/dfu.h b/include/dfu.h
index f39d3f1..70eeaef 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -203,7 +203,7 @@ static inline void dfu_set_defer_flush(struct dfu_entity *dfu)
 int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size);
 
 /* Device specific */
-#ifdef CONFIG_DFU_MMC
+#if defined(CONFIG_DFU_MMC) && !defined(CONFIG_SPL_BUILD)
 extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s);
 #else
 static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr,
-- 
1.9.1

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[Tom Rini]

On Thu, Apr 27, 2017 at 05:45:20PM +0530, Ravi Babu wrote:
> The SPL-DFU feature enable to load and
> execute u-boot over usb from PC using
> dfu-util.
> Hence dfu-reset should not be issued
> when dfu-util -R switch is issued.
> 
> Signed-off-by: Ravi Babu <ravibabu@ti.com>
> ---
>  common/dfu.c       | 2 +-
>  common/spl/Kconfig | 4 ++++
>  drivers/dfu/dfu.c  | 4 ++++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/common/dfu.c b/common/dfu.c
> index 0e9f5f5..546a1ab 100644
> --- a/common/dfu.c
> +++ b/common/dfu.c
> @@ -88,7 +88,7 @@ exit:
>  	board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
>  
>  	if (dfu_reset)
> -		run_command("reset", 0);
> +		do_reset(NULL, 0, 0, NULL);
>  
>  	g_dnl_clear_detach();

So this hunk drops out the need for cli stuff.

> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index 1231351..f51ae2c 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -6,6 +6,9 @@ config SUPPORT_SPL
>  config SUPPORT_TPL
>  	bool
>  
> +config SPL_DFU_NO_RESET
> +	bool
> +
>  config SPL
>  	bool
>  	depends on SUPPORT_SPL
> @@ -646,6 +649,7 @@ config SPL_USBETH_SUPPORT
>  config SPL_DFU_SUPPORT
>  	bool "Support DFU (Device Firmware Upgarde)"
>  	select SPL_HASH_SUPPORT
> +	select SPL_DFU_NO_RESET
>  	depends on SPL_RAM_SUPPORT
>  	help
>  	  This feature enables the DFU (Device Firmware Upgarde) in SPL with
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 8dacc1a..ceb33e3 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -35,7 +35,11 @@ static struct hash_algo *dfu_hash_algo;
>   */
>  __weak bool dfu_usb_get_reset(void)
>  {
> +#ifdef CONFIG_SPL_DFU_NO_RESET
> +	return false;
> +#else
>  	return true;
> +#endif
>  }
>  
>  static int dfu_find_alt_num(const char *s)

So do we still need the above, in order to save space?  How much are we
saving here even, now?  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/20170427/30881778/attachment.sig>

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[B, Ravi]

Hi Tom

>> 
>> diff --git a/common/dfu.c b/common/dfu.c index 0e9f5f5..546a1ab 100644
>> --- a/common/dfu.c
>> +++ b/common/dfu.c
>> @@ -88,7 +88,7 @@ exit:
>>  	board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
>>  
>>  	if (dfu_reset)
>> -		run_command("reset", 0);
>> +		do_reset(NULL, 0, 0, NULL);
>>  
>>  	g_dnl_clear_detach();

>So this hunk drops out the need for cli stuff.

Yes.

>> diff --git a/common/spl/Kconfig b/common/spl/Kconfig index 
>> 1231351..f51ae2c 100644
>> --- a/common/spl/Kconfig
>> +++ b/common/spl/Kconfig
>> @@ -6,6 +6,9 @@ config SUPPORT_SPL
>>  config SUPPORT_TPL
>>  	bool
>>  
>> +config SPL_DFU_NO_RESET
>> +	bool
>> +
>>  config SPL
>>  	bool
>>  	depends on SUPPORT_SPL
>> @@ -646,6 +649,7 @@ config SPL_USBETH_SUPPORT  config SPL_DFU_SUPPORT
>>  	bool "Support DFU (Device Firmware Upgarde)"
>>  	select SPL_HASH_SUPPORT
>> +	select SPL_DFU_NO_RESET
>>  	depends on SPL_RAM_SUPPORT
>>  	help
>>  	  This feature enables the DFU (Device Firmware Upgarde) in SPL with 
>> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 
>> 8dacc1a..ceb33e3 100644
>> --- a/drivers/dfu/dfu.c
>> +++ b/drivers/dfu/dfu.c
>> @@ -35,7 +35,11 @@ static struct hash_algo *dfu_hash_algo;
>>   */
>>  __weak bool dfu_usb_get_reset(void)
>  {
>> +#ifdef CONFIG_SPL_DFU_NO_RESET
>> +	return false;
>> +#else
>>  	return true;
>> +#endif
>>  }
>>  
>>  static int dfu_find_alt_num(const char *s)

>So do we still need the above, in order to save space?  How much are we saving here even, now?  Thanks!

I observed around 7K reduced. 

Regards
Ravi

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[Tom Rini]

On Thu, Apr 27, 2017 at 05:24:09PM +0000, B, Ravi wrote:
> Hi Tom
> 
> >> 
> >> diff --git a/common/dfu.c b/common/dfu.c index 0e9f5f5..546a1ab 100644
> >> --- a/common/dfu.c
> >> +++ b/common/dfu.c
> >> @@ -88,7 +88,7 @@ exit:
> >>  	board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
> >>  
> >>  	if (dfu_reset)
> >> -		run_command("reset", 0);
> >> +		do_reset(NULL, 0, 0, NULL);
> >>  
> >>  	g_dnl_clear_detach();
> 
> >So this hunk drops out the need for cli stuff.
> 
> Yes.
> 
> >> diff --git a/common/spl/Kconfig b/common/spl/Kconfig index 
> >> 1231351..f51ae2c 100644
> >> --- a/common/spl/Kconfig
> >> +++ b/common/spl/Kconfig
> >> @@ -6,6 +6,9 @@ config SUPPORT_SPL
> >>  config SUPPORT_TPL
> >>  	bool
> >>  
> >> +config SPL_DFU_NO_RESET
> >> +	bool
> >> +
> >>  config SPL
> >>  	bool
> >>  	depends on SUPPORT_SPL
> >> @@ -646,6 +649,7 @@ config SPL_USBETH_SUPPORT  config SPL_DFU_SUPPORT
> >>  	bool "Support DFU (Device Firmware Upgarde)"
> >>  	select SPL_HASH_SUPPORT
> >> +	select SPL_DFU_NO_RESET
> >>  	depends on SPL_RAM_SUPPORT
> >>  	help
> >>  	  This feature enables the DFU (Device Firmware Upgarde) in SPL with 
> >> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 
> >> 8dacc1a..ceb33e3 100644
> >> --- a/drivers/dfu/dfu.c
> >> +++ b/drivers/dfu/dfu.c
> >> @@ -35,7 +35,11 @@ static struct hash_algo *dfu_hash_algo;
> >>   */
> >>  __weak bool dfu_usb_get_reset(void)
> >  {
> >> +#ifdef CONFIG_SPL_DFU_NO_RESET
> >> +	return false;
> >> +#else
> >>  	return true;
> >> +#endif
> >>  }
> >>  
> >>  static int dfu_find_alt_num(const char *s)
> 
> >So do we still need the above, in order to save space?  How much are we saving here even, now?  Thanks!
> 
> I observed around 7K reduced. 

I don't just mean dropping out CLI, I mean after dropping out CLI but
leaving in the reset logic.  That's _still_ 7k?

-- 
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/20170427/0b162338/attachment.sig>

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[B, Ravi]

Tom

>> >>  
>> >>  static int dfu_find_alt_num(const char *s)
>> 
>> >So do we still need the above, in order to save space?  How much are we saving here even, now?  Thanks!
>> 
>> I observed around 7K reduced. 

>I don't just mean dropping out CLI, I mean after dropping out CLI but leaving in the reset logic.  That's _still_ 7k?

Without this fix, with cli_simple_run_command(), size of MLO with default dra7xx_evm_defconfig is 130K.
With this fix, compile out cli.c, the MLO size is 126K.
Around 4K is space saved.

Regards
Ravi  

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[B, Ravi]

>Tom

>>> >>  
>>> >>  static int dfu_find_alt_num(const char *s)
>>> 
>>> >So do we still need the above, in order to save space?  How much are we saving here even, now?  Thanks!
>>> 
>>> I observed around 7K reduced. 

Ignore 7K figure provided, that's wrong calculation.

>I don't just mean dropping out CLI, I mean after dropping out CLI but leaving in the reset logic.  That's _still_ 7k?

>Without this fix, with cli_simple_run_command(), size of MLO with default dra7xx_evm_defconfig is 130K.
>With this fix, compile out cli.c, the MLO size is 126K. 
>Around 4K is space saved.

do_reset() used instead of run_command() for dfu_reset().

Regards
Ravi  

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[Tom Rini]

On Tue, May 02, 2017 at 12:41:48PM +0000, B, Ravi wrote:
> Tom
> 
> >> >>  
> >> >>  static int dfu_find_alt_num(const char *s)
> >> 
> >> >So do we still need the above, in order to save space?  How much are we saving here even, now?  Thanks!
> >> 
> >> I observed around 7K reduced. 
> 
> >I don't just mean dropping out CLI, I mean after dropping out CLI but leaving in the reset logic.  That's _still_ 7k?
> 
> Without this fix, with cli_simple_run_command(), size of MLO with default dra7xx_evm_defconfig is 130K.
> With this fix, compile out cli.c, the MLO size is 126K.
> Around 4K is space saved.

OK.  And dropping out CLI and leaving in reset, unconditionally vs
dropping out CLI and also dropping reset via DFU, what is the savings
there?  Is that 3K?

-- 
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/20170502/deb11d91/attachment.sig>

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[B, Ravi]

Tom

>> >> 
>> >> I observed around 7K reduced. 
>> 
>> >I don't just mean dropping out CLI, I mean after dropping out CLI but leaving in the reset logic.  That's _still_ 7k?
>> 
>> Without this fix, with cli_simple_run_command(), size of MLO with default dra7xx_evm_defconfig is 130K.
>> With this fix, compile out cli.c, the MLO size is 126K.
>> Around 4K is space saved.

>OK.  And dropping out CLI and leaving in reset, unconditionally vs dropping out CLI and also dropping reset via DFU, what is the savings there?  Is that 3K?

7K provided earlier was wrong calculation. Sorry for confusion.

If unconditionally dropping CLI and use do_reset instead of run_command, I will save around 4K. (with this patch v2 series)
If unconditionally dropping CLI and dropping do_reset in SPL-DFU, I will save around 5K. (with this patch series + drop do_reset in SPL-DFU unconditionally)

Regards
Ravi  

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[Tom Rini]

On Tue, May 02, 2017 at 01:02:24PM +0000, B, Ravi wrote:
> Tom
> 
> >> >> 
> >> >> I observed around 7K reduced. 
> >> 
> >> >I don't just mean dropping out CLI, I mean after dropping out CLI but leaving in the reset logic.  That's _still_ 7k?
> >> 
> >> Without this fix, with cli_simple_run_command(), size of MLO with default dra7xx_evm_defconfig is 130K.
> >> With this fix, compile out cli.c, the MLO size is 126K.
> >> Around 4K is space saved.
> 
> >OK.  And dropping out CLI and leaving in reset, unconditionally vs dropping out CLI and also dropping reset via DFU, what is the savings there?  Is that 3K?
> 
> 7K provided earlier was wrong calculation. Sorry for confusion.

OK.

> If unconditionally dropping CLI and use do_reset instead of
> run_command, I will save around 4K. (with this patch v2 series)
> If unconditionally dropping CLI and dropping do_reset in SPL-DFU, I
> will save around 5K. (with this patch series + drop do_reset in
> SPL-DFU unconditionally)

Can you give the exact bytes saved in each case, with your specific
compiler?  I ask since I'm surprised it's more than a function being
dropped by the linker in this case.  diff'ing the u-boot-spl.map files
would also say what is dropped and I'd be interested in that.  (And yes,
I'm asking for more details to justify adding a Kconfig option here).
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/20170502/0f7c4174/attachment.sig>

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[B, Ravi]

Tom

>> >> >I don't just mean dropping out CLI, I mean after dropping out CLI but leaving in the reset logic.  That's _still_ 7k?
>> >> 
>> >> Without this fix, with cli_simple_run_command(), size of MLO with default dra7xx_evm_defconfig is 130K.
>> >> With this fix, compile out cli.c, the MLO size is 126K.
>> >> Around 4K is space saved.
>> 
>> >OK.  And dropping out CLI and leaving in reset, unconditionally vs dropping out CLI and also dropping reset via DFU, what is the savings there?  Is that 3K?
>> 
>> 7K provided earlier was wrong calculation. Sorry for confusion.

>OK.

>> If unconditionally dropping CLI and use do_reset instead of 
>> run_command, I will save around 4K. (with this patch v2 series) If 
>> unconditionally dropping CLI and dropping do_reset in SPL-DFU, I will 
>> save around 5K. (with this patch series + drop do_reset in SPL-DFU 
>> unconditionally)

>Can you give the exact bytes saved in each case, with your specific compiler?  I ask since I'm surprised it's more than a function being dropped by the linker in this case.  diff'ing the u-boot-spl.map files would also say what is dropped and I'd be interested in that.  (And >yes, I'm asking for more details to justify adding a Kconfig option here).
>Thanks

Compiler : arm-linu-gnueabihif-gcc, version: 6.2-2016.11)

1) default dra7xx_evm_defconfig and use cli_simple_runcommand - MLO size is 129998
2) default dra7xx_evm_defconfig and dropping CLI and use do_reset - MLO size is 126130, saving is ~4K (129998-126130 = 3878 bytes).
3) default dra7xx_evm_defconfig and dropping CLI and dropping do_reset - MLO size is 125298, saving is ~5K (129998-125298 = 4708 bytes).

Regards
Ravi

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[B, Ravi]

Tom

>>Can you give the exact bytes saved in each case, with your specific compiler?  I ask since I'm surprised it's more than a function being dropped by the linker in this case.  diff'ing the u-boot-spl.map files would also say what is dropped and I'd be interested in that.  > (And >yes, I'm asking for more details to justify adding a Kconfig option here).
>>Thanks

>Compiler : arm-linu-gnueabihif-gcc, version: 6.2-2016.11)

>1) default dra7xx_evm_defconfig and use cli_simple_runcommand - MLO size is 129998
This is with no patches.

>2) default dra7xx_evm_defconfig and dropping CLI and use do_reset - MLO size is 126130, saving is ~4K (129998-126130 = 3878 bytes).
This 4K saving is based on this V2 patches series (excludes only CONFIG_DFU_MMC in SPL-DFU).

>3) default dra7xx_evm_defconfig and dropping CLI and dropping do_reset - MLO size is 125298, saving is ~5K (129998-125298 = 4708 bytes).
(My bad, I changed to V1 initial series in between while taking this data)
This 5K saving is based on this V1 patches (basically, which excludes all CONFIG_DFU_<MMC/NAND/SF/TFTP> in SPL-DFU)
Dropping do_reset in SPL does not reduce the MLO size. I observe do_reset is always included in spl-uboot.map whether exclude and include.

Regards
Ravi

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[Tom Rini]

On Tue, May 02, 2017 at 01:56:45PM +0000, B, Ravi wrote:
> Tom
> 
> >>Can you give the exact bytes saved in each case, with your specific compiler?  I ask since I'm surprised it's more than a function being dropped by the linker in this case.  diff'ing the u-boot-spl.map files would also say what is dropped and I'd be interested in that.  > (And >yes, I'm asking for more details to justify adding a Kconfig option here).
> >>Thanks
> 
> >Compiler : arm-linu-gnueabihif-gcc, version: 6.2-2016.11)
> 
> >1) default dra7xx_evm_defconfig and use cli_simple_runcommand - MLO size is 129998
> This is with no patches.
> 
> >2) default dra7xx_evm_defconfig and dropping CLI and use do_reset - MLO size is 126130, saving is ~4K (129998-126130 = 3878 bytes).
> This 4K saving is based on this V2 patches series (excludes only CONFIG_DFU_MMC in SPL-DFU).
> 
> >3) default dra7xx_evm_defconfig and dropping CLI and dropping do_reset - MLO size is 125298, saving is ~5K (129998-125298 = 4708 bytes).
> (My bad, I changed to V1 initial series in between while taking this data)
> This 5K saving is based on this V1 patches (basically, which excludes all CONFIG_DFU_<MMC/NAND/SF/TFTP> in SPL-DFU)
> Dropping do_reset in SPL does not reduce the MLO size. I observe do_reset is always included in spl-uboot.map whether exclude and include.

So in other words, we don't save any space by making DFU-reset be
conditionally included?  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/20170502/defe39ac/attachment.sig>

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[B, Ravi]

>-----Original Message-----
>From: Tom Rini [mailto:trini at konsulko.com] 
>Sent: Tuesday, May 02, 2017 8:24 PM
>To: B, Ravi
>Cc: u-boot at lists.denx.de
>Subject: Re: [U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

>On Tue, May 02, 2017 at 01:56:45PM +0000, B, Ravi wrote:
>> Tom
>> 
>> >>Can you give the exact bytes saved in each case, with your specific compiler?  I ask since I'm surprised it's more than a function being dropped by the linker in this case.  diff'ing the u-boot-spl.map files would also say what is dropped and I'd be interested in that.  >> (And >yes, I'm asking for more details to justify adding a Kconfig option here).
>> >>Thanks
>> 
>> >Compiler : arm-linu-gnueabihif-gcc, version: 6.2-2016.11)
>> 
>> >1) default dra7xx_evm_defconfig and use cli_simple_runcommand - MLO 
>> >size is 129998
>> This is with no patches.
>> 
>> >2) default dra7xx_evm_defconfig and dropping CLI and use do_reset - MLO size is 126130, saving is ~4K (129998-126130 = 3878 bytes).
>> This 4K saving is based on this V2 patches series (excludes only CONFIG_DFU_MMC in SPL-DFU).
>> 
>> >3) default dra7xx_evm_defconfig and dropping CLI and dropping do_reset - MLO size is 125298, saving is ~5K (129998-125298 = 4708 bytes).
>> (My bad, I changed to V1 initial series in between while taking this 
>> data) This 5K saving is based on this V1 patches (basically, which 
>> excludes all CONFIG_DFU_<MMC/NAND/SF/TFTP> in SPL-DFU) Dropping do_reset in SPL does not reduce the MLO size. I observe do_reset is always included in spl-uboot.map whether exclude and include.

>So in other words, we don't save any space by making DFU-reset be conditionally included?  Thanks!

Yes, you are correct Tom.

Thanks & Regards
Ravi

[U-Boot] [PATCH v2 2/3] common: dfu: ignore reset for spl-dfu

[Ravi Babu]

The SPL-DFU feature enable to load and
execute u-boot from RAM over usb from
PC using dfu-util.
Hence dfu-reset should not be issued
when dfu-util -R switch is issued.

Signed-off-by: Ravi Babu <ravibabu@ti.com>
---
 common/dfu.c       | 2 +-
 common/spl/Kconfig | 4 ++++
 drivers/dfu/dfu.c  | 4 ++++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/common/dfu.c b/common/dfu.c
index 0e9f5f5..546a1ab 100644
--- a/common/dfu.c
+++ b/common/dfu.c
@@ -88,7 +88,7 @@ exit:
 	board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
 
 	if (dfu_reset)
-		run_command("reset", 0);
+		do_reset(NULL, 0, 0, NULL);
 
 	g_dnl_clear_detach();
 
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 1231351..f51ae2c 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -6,6 +6,9 @@ config SUPPORT_SPL
 config SUPPORT_TPL
 	bool
 
+config SPL_DFU_NO_RESET
+	bool
+
 config SPL
 	bool
 	depends on SUPPORT_SPL
@@ -646,6 +649,7 @@ config SPL_USBETH_SUPPORT
 config SPL_DFU_SUPPORT
 	bool "Support DFU (Device Firmware Upgarde)"
 	select SPL_HASH_SUPPORT
+	select SPL_DFU_NO_RESET
 	depends on SPL_RAM_SUPPORT
 	help
 	  This feature enables the DFU (Device Firmware Upgarde) in SPL with
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 8dacc1a..ceb33e3 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -35,7 +35,11 @@ static struct hash_algo *dfu_hash_algo;
  */
 __weak bool dfu_usb_get_reset(void)
 {
+#ifdef CONFIG_SPL_DFU_NO_RESET
+	return false;
+#else
 	return true;
+#endif
 }
 
 static int dfu_find_alt_num(const char *s)
-- 
1.9.1