* [U-Boot] [PATCH 0/3] spl: dfu: misc fixes and reduce MLO foot print
@ 2017-04-26 13:14 Ravi Babu
2017-04-26 13:14 ` [U-Boot] [PATCH 1/3] spl: Kconfig: dfu: spl-dfu depends on SPL_RAM_SUPPORT Ravi Babu
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: Ravi Babu @ 2017-04-26 13:14 UTC (permalink / raw)
To: u-boot
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
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 | 3 +++
common/spl/Kconfig | 1 +
drivers/dfu/Makefile | 4 +++-
include/dfu.h | 8 ++++----
5 files changed, 12 insertions(+), 7 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 27+ messages in thread* [U-Boot] [PATCH 1/3] spl: Kconfig: dfu: spl-dfu depends on SPL_RAM_SUPPORT 2017-04-26 13:14 [U-Boot] [PATCH 0/3] spl: dfu: misc fixes and reduce MLO foot print Ravi Babu @ 2017-04-26 13:14 ` Ravi Babu 2017-04-26 13:36 ` Tom Rini 2017-04-26 13:14 ` [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu Ravi Babu 2017-04-26 13:14 ` [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size Ravi Babu 2 siblings, 1 reply; 27+ messages in thread From: Ravi Babu @ 2017-04-26 13:14 UTC (permalink / raw) To: u-boot 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> --- 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 1/3] spl: Kconfig: dfu: spl-dfu depends on SPL_RAM_SUPPORT 2017-04-26 13:14 ` [U-Boot] [PATCH 1/3] spl: Kconfig: dfu: spl-dfu depends on SPL_RAM_SUPPORT Ravi Babu @ 2017-04-26 13:36 ` Tom Rini 0 siblings, 0 replies; 27+ messages in thread From: Tom Rini @ 2017-04-26 13:36 UTC (permalink / raw) To: u-boot On Wed, Apr 26, 2017 at 06:44:07PM +0530, Ravi Babu wrote: > 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> -- 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/20170426/fa77d648/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-26 13:14 [U-Boot] [PATCH 0/3] spl: dfu: misc fixes and reduce MLO foot print Ravi Babu 2017-04-26 13:14 ` [U-Boot] [PATCH 1/3] spl: Kconfig: dfu: spl-dfu depends on SPL_RAM_SUPPORT Ravi Babu @ 2017-04-26 13:14 ` Ravi Babu 2017-04-26 13:40 ` Tom Rini 2017-04-26 13:14 ` [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size Ravi Babu 2 siblings, 1 reply; 27+ messages in thread From: Ravi Babu @ 2017-04-26 13:14 UTC (permalink / raw) To: u-boot 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 | 3 +++ 1 file changed, 3 insertions(+) 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 if (dfu_reset) run_command("reset", 0); -- 1.9.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-26 13:14 ` [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu Ravi Babu @ 2017-04-26 13:40 ` Tom Rini 2017-04-26 15:58 ` B, Ravi 0 siblings, 1 reply; 27+ messages in thread From: Tom Rini @ 2017-04-26 13:40 UTC (permalink / raw) To: u-boot On Wed, Apr 26, 2017 at 06:44:08PM +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 | 3 +++ > 1 file changed, 3 insertions(+) > > 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 > if (dfu_reset) > run_command("reset", 0); So we "fix" some of the problems we see by saying that you can't reset the board in SPL via DFU. I think maybe we should instead drop run_command here and make reset-via-DFU call do_reset() directly like some other small-size-required cases do. This will let us drop the command requirement here but still allow for "use DFU to flash and reset the board with just SPL" as a use-case. 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/20170426/b0bf380c/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-26 13:40 ` Tom Rini @ 2017-04-26 15:58 ` B, Ravi 2017-04-26 16:24 ` Tom Rini 0 siblings, 1 reply; 27+ messages in thread From: B, Ravi @ 2017-04-26 15:58 UTC (permalink / raw) To: u-boot Hi Tom >> 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 | 3 +++ >> 1 file changed, 3 insertions(+) >> >> 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 >> if (dfu_reset) >> run_command("reset", 0); >So we "fix" some of the problems we see by saying that you can't reset the board in SPL via DFU. > I think maybe we should instead drop run_command here and make reset-via-DFU call do_reset() directly like some other small-size-required cases do. This will let us drop the command >requirement here but still allow for "use DFU to flash and reset the board with just SPL" as a use-case. Thanks! The SPL-DFU will load and execute u-boot.img from RAM. If we issue dfu-reset (-R switch), this leads to cpu-reset and we lost the purpose of SPL-DFU itself. Hence dfu-reset issue shall not be issued for SPL-DFU. I agree, the dfu-reset is needed in u-boot, after flashing images to QSPI/eMMC/SD using the DFU to execute newly loaded image. So, dfu-reset is needed for u-boot, but not required for SPL-DFU. For u-boot, we can continue to use run_command() for dfu-reset. Regards Ravi ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-26 15:58 ` B, Ravi @ 2017-04-26 16:24 ` Tom Rini 2017-04-26 16:25 ` B, Ravi 2017-04-27 8:06 ` Lukasz Majewski 0 siblings, 2 replies; 27+ messages in thread From: Tom Rini @ 2017-04-26 16:24 UTC (permalink / raw) To: u-boot On Wed, Apr 26, 2017 at 03:58:27PM +0000, B, Ravi wrote: > Hi Tom > > >> 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 | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> 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 > >> if (dfu_reset) > >> run_command("reset", 0); > > >So we "fix" some of the problems we see by saying that you can't > >reset the board in SPL via DFU. I think maybe we should instead drop > >run_command here and make reset-via-DFU call do_reset() directly like > >some other small-size-required cases do. This will let us drop the > >command >requirement here but still allow for "use DFU to flash and > >reset the board with just SPL" as a use-case. Thanks! > > The SPL-DFU will load and execute u-boot.img from RAM. If we issue > dfu-reset (-R switch), this leads to cpu-reset and we lost the purpose > of SPL-DFU itself. Hence dfu-reset issue shall not be issued for > SPL-DFU. > > I agree, the dfu-reset is needed in u-boot, after flashing images to > QSPI/eMMC/SD using the DFU to execute newly loaded image. So, > dfu-reset is needed for u-boot, but not required for SPL-DFU. > > For u-boot, we can continue to use run_command() for dfu-reset. OK. I guess if someone else wants to try and use SPL for DFU flashing that requires more work and they can address the above then, thanks! Reviewed-by: Tom Rini <trini@konsulko.com> -- 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/20170426/f8088a03/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-26 16:24 ` Tom Rini @ 2017-04-26 16:25 ` B, Ravi 2017-04-27 8:06 ` Lukasz Majewski 1 sibling, 0 replies; 27+ messages in thread From: B, Ravi @ 2017-04-26 16:25 UTC (permalink / raw) To: u-boot Hi Tom >> >> The SPL-DFU will load and execute u-boot.img from RAM. If we issue >> dfu-reset (-R switch), this leads to cpu-reset and we lost the purpose >> of SPL-DFU itself. Hence dfu-reset issue shall not be issued for >> SPL-DFU. >> >> I agree, the dfu-reset is needed in u-boot, after flashing images to >> QSPI/eMMC/SD using the DFU to execute newly loaded image. So, >> dfu-reset is needed for u-boot, but not required for SPL-DFU. >> >> For u-boot, we can continue to use run_command() for dfu-reset. >OK. I guess if someone else wants to try and use SPL for DFU flashing that requires more work and they can address the above then, thanks! >Reviewed-by: Tom Rini <trini@konsulko.com> Thanks. Any comments on [PATCH 3/3]? Regards Ravi ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-26 16:24 ` Tom Rini 2017-04-26 16:25 ` B, Ravi @ 2017-04-27 8:06 ` Lukasz Majewski 2017-04-27 8:37 ` B, Ravi 2017-04-27 8:37 ` B, Ravi 1 sibling, 2 replies; 27+ messages in thread From: Lukasz Majewski @ 2017-04-27 8:06 UTC (permalink / raw) To: u-boot On Wed, 26 Apr 2017 12:24:06 -0400 Tom Rini <trini@konsulko.com> wrote: > On Wed, Apr 26, 2017 at 03:58:27PM +0000, B, Ravi wrote: > > Hi Tom > > > > >> 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 | 3 +++ > > >> 1 file changed, 3 insertions(+) > > >> > > >> 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 > > >> if (dfu_reset) > > >> run_command("reset", 0); > > > > >So we "fix" some of the problems we see by saying that you can't > > >reset the board in SPL via DFU. I think maybe we should instead > > >drop run_command here and make reset-via-DFU call do_reset() > > >directly like some other small-size-required cases do. This will > > >let us drop the command >requirement here but still allow for "use > > >DFU to flash and reset the board with just SPL" as a use-case. > > >Thanks! > > > > The SPL-DFU will load and execute u-boot.img from RAM. If we issue > > dfu-reset (-R switch), this leads to cpu-reset and we lost the > > purpose of SPL-DFU itself. Hence dfu-reset issue shall not be > > issued for SPL-DFU. It seems like a valid use case - maybe it would be beneficial to add Kconfig option (CONFIG_DFU_SPL_NO_RESET) to give the user possibility to decide (and in this way document it?). > > > > I agree, the dfu-reset is needed in u-boot, after flashing images to > > QSPI/eMMC/SD using the DFU to execute newly loaded image. So, > > dfu-reset is needed for u-boot, but not required for SPL-DFU. > > > > For u-boot, we can continue to use run_command() for dfu-reset. > > OK. I guess if someone else wants to try and use SPL for DFU flashing > that requires more work and they can address the above then, thanks! > > Reviewed-by: Tom Rini <trini@konsulko.com> > 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 181 bytes Desc: OpenPGP digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170427/f2c24af4/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-27 8:06 ` Lukasz Majewski @ 2017-04-27 8:37 ` B, Ravi 2017-04-27 8:37 ` B, Ravi 1 sibling, 0 replies; 27+ messages in thread From: B, Ravi @ 2017-04-27 8:37 UTC (permalink / raw) To: u-boot Lukasz >> > >> > The SPL-DFU will load and execute u-boot.img from RAM. If we issue >> > dfu-reset (-R switch), this leads to cpu-reset and we lost the >> > purpose of SPL-DFU itself. Hence dfu-reset issue shall not be >> > issued for SPL-DFU. >It seems like a valid use case - maybe it would be beneficial to add Kconfig option (CONFIG_DFU_SPL_NO_RESET) to give the user possibility to decide (and in this way document it?). Yes, make sense, to differentiate dfu-reset for SPL-DFU. Ok, I will include CONFIG_SPL_DFU_NO_RESET in next version of patch. Thanks. >> > >> > I agree, the dfu-reset is needed in u-boot, after flashing images to >> > QSPI/eMMC/SD using the DFU to execute newly loaded image. So, >> > dfu-reset is needed for u-boot, but not required for SPL-DFU. >> > >> > For u-boot, we can continue to use run_command() for dfu-reset. >> >> OK. I guess if someone else wants to try and use SPL for DFU flashing >> that requires more work and they can address the above then, thanks! >> >> Reviewed-by: Tom Rini <trini@konsulko.com> Regards Ravi ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-27 8:06 ` Lukasz Majewski 2017-04-27 8:37 ` B, Ravi @ 2017-04-27 8:37 ` B, Ravi 1 sibling, 0 replies; 27+ messages in thread From: B, Ravi @ 2017-04-27 8:37 UTC (permalink / raw) To: u-boot Lukasz >> > >> > The SPL-DFU will load and execute u-boot.img from RAM. If we issue >> > dfu-reset (-R switch), this leads to cpu-reset and we lost the >> > purpose of SPL-DFU itself. Hence dfu-reset issue shall not be >> > issued for SPL-DFU. >It seems like a valid use case - maybe it would be beneficial to add Kconfig option (CONFIG_DFU_SPL_NO_RESET) to give the user possibility to decide (and in this way document it?). Yes, make sense, to differentiate dfu-reset for SPL-DFU. Ok, I will include CONFIG_SPL_DFU_NO_RESET in next version of patch. Thanks. >> > >> > I agree, the dfu-reset is needed in u-boot, after flashing images >> > to QSPI/eMMC/SD using the DFU to execute newly loaded image. So, >> > dfu-reset is needed for u-boot, but not required for SPL-DFU. >> > >> > For u-boot, we can continue to use run_command() for dfu-reset. >> >> OK. I guess if someone else wants to try and use SPL for DFU >> flashing that requires more work and they can address the above then, thanks! >> >> Reviewed-by: Tom Rini <trini@konsulko.com> Regards Ravi ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size 2017-04-26 13:14 [U-Boot] [PATCH 0/3] spl: dfu: misc fixes and reduce MLO foot print Ravi Babu 2017-04-26 13:14 ` [U-Boot] [PATCH 1/3] spl: Kconfig: dfu: spl-dfu depends on SPL_RAM_SUPPORT Ravi Babu 2017-04-26 13:14 ` [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu Ravi Babu @ 2017-04-26 13:14 ` Ravi Babu 2017-04-26 13:35 ` Tom Rini 2 siblings, 1 reply; 27+ messages in thread From: Ravi Babu @ 2017-04-26 13:14 UTC (permalink / raw) To: u-boot 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> --- common/Makefile | 3 +-- drivers/dfu/Makefile | 4 +++- include/dfu.h | 8 ++++---- 3 files changed, 8 insertions(+), 7 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..ef48f36 100644 --- a/drivers/dfu/Makefile +++ b/drivers/dfu/Makefile @@ -6,8 +6,10 @@ # obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o +ifndef CONFIG_SPL_BUILD obj-$(CONFIG_DFU_MMC) += dfu_mmc.o obj-$(CONFIG_DFU_NAND) += dfu_nand.o -obj-$(CONFIG_DFU_RAM) += dfu_ram.o obj-$(CONFIG_DFU_SF) += dfu_sf.o obj-$(CONFIG_DFU_TFTP) += dfu_tftp.o +endif +obj-$(CONFIG_DFU_RAM) += dfu_ram.o diff --git a/include/dfu.h b/include/dfu.h index f39d3f1..b53ae80 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, @@ -214,7 +214,7 @@ static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, } #endif -#ifdef CONFIG_DFU_NAND +#if defined(CONFIG_DFU_NAND) && !defined(CONFIG_SPL_BUILD) extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s); #else static inline int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, @@ -236,7 +236,7 @@ static inline int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, } #endif -#ifdef CONFIG_DFU_SF +#if defined(CONFIG_DFU_SF) && !defined(CONFIG_SPL_BUILD) extern int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s); #else static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, @@ -260,7 +260,7 @@ static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, * * @return 0 on success, otherwise error code */ -#ifdef CONFIG_DFU_TFTP +#if defined(CONFIG_DFU_TFTP) && !defined(CONFIG_SPL_BUILD) int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len, char *interface, char *devstring); #else -- 1.9.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size 2017-04-26 13:14 ` [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size Ravi Babu @ 2017-04-26 13:35 ` Tom Rini 2017-04-27 7:22 ` B, Ravi 0 siblings, 1 reply; 27+ messages in thread From: Tom Rini @ 2017-04-26 13:35 UTC (permalink / raw) To: u-boot On Wed, Apr 26, 2017 at 06:44:09PM +0530, Ravi Babu wrote: > 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> [snip] > diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile > index 61f2b71..ef48f36 100644 > --- a/drivers/dfu/Makefile > +++ b/drivers/dfu/Makefile > @@ -6,8 +6,10 @@ > # > > obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o > +ifndef CONFIG_SPL_BUILD > obj-$(CONFIG_DFU_MMC) += dfu_mmc.o > obj-$(CONFIG_DFU_NAND) += dfu_nand.o > -obj-$(CONFIG_DFU_RAM) += dfu_ram.o > obj-$(CONFIG_DFU_SF) += dfu_sf.o > obj-$(CONFIG_DFU_TFTP) += dfu_tftp.o > +endif > +obj-$(CONFIG_DFU_RAM) += dfu_ram.o We should discard at link time the unreachable parts here in SPL, no? > diff --git a/include/dfu.h b/include/dfu.h > index f39d3f1..b53ae80 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) I don't like the initial condition we have here, adding the !SPL_BUILD test makes this even worse, lets not do that. -- 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/20170426/0c1fef39/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size 2017-04-26 13:35 ` Tom Rini @ 2017-04-27 7:22 ` B, Ravi 2017-04-27 12:31 ` Tom Rini 0 siblings, 1 reply; 27+ messages in thread From: B, Ravi @ 2017-04-27 7:22 UTC (permalink / raw) To: u-boot Hi Tom >> 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> [snip] >> diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile index >> 61f2b71..ef48f36 100644 >> --- a/drivers/dfu/Makefile >> +++ b/drivers/dfu/Makefile >> @@ -6,8 +6,10 @@ >> # >> >> obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o >> +ifndef CONFIG_SPL_BUILD >> obj-$(CONFIG_DFU_MMC) += dfu_mmc.o >> obj-$(CONFIG_DFU_NAND) += dfu_nand.o >> -obj-$(CONFIG_DFU_RAM) += dfu_ram.o >> obj-$(CONFIG_DFU_SF) += dfu_sf.o >> obj-$(CONFIG_DFU_TFTP) += dfu_tftp.o >> +endif >> +obj-$(CONFIG_DFU_RAM) += dfu_ram.o >We should discard at link time the unreachable parts here in SPL, no? Yes you are correct. But what is happening here is, the CONFIG_DFU_<MMC/NAND/SF/TFTP> selected through Kconfig/Menuconfig is applicable for both SPL and U-Boot. Hence CONFIG_DFU_MMC/NAND/SF gets compiled for SPL as well, which needs run_command(). Actually CONFIG_DFU_MMC/NAND/etc is not scoped for SPL-DFU. As we have aligned, not to increase the SPL size, user shall use SPL-DFU feature to boot to u-boot, then utilize the full featured DFU to flash MMC/NAND/SF. I get undefined reference to common function run_command(), "dfu_fill_entitiy_<mmc/nand/sf>" in driver/dfu/dfu.c. The dfu.c is common for both SPL-DFU and U-boot. >> diff --git a/include/dfu.h b/include/dfu.h index f39d3f1..b53ae80 >> 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) >I don't like the initial condition we have here, adding the !SPL_BUILD test makes this even worse, lets not do that. I did not find better solution to reduce the SPL size by removing cli.c/cli_hush.c and compiling out CONFIG_DFU_<MMC/NAND/SF> for SPL altogether. Regards Ravi ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size 2017-04-27 7:22 ` B, Ravi @ 2017-04-27 12:31 ` Tom Rini 2017-04-27 17:25 ` B, Ravi 2017-05-03 8:36 ` B, Ravi 0 siblings, 2 replies; 27+ messages in thread From: Tom Rini @ 2017-04-27 12:31 UTC (permalink / raw) To: u-boot On Thu, Apr 27, 2017 at 07:22:29AM +0000, B, Ravi wrote: > Hi Tom > > >> 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> > [snip] > >> diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile index > >> 61f2b71..ef48f36 100644 > >> --- a/drivers/dfu/Makefile > >> +++ b/drivers/dfu/Makefile > >> @@ -6,8 +6,10 @@ > >> # > >> > >> obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o > >> +ifndef CONFIG_SPL_BUILD > >> obj-$(CONFIG_DFU_MMC) += dfu_mmc.o > >> obj-$(CONFIG_DFU_NAND) += dfu_nand.o > >> -obj-$(CONFIG_DFU_RAM) += dfu_ram.o > >> obj-$(CONFIG_DFU_SF) += dfu_sf.o > >> obj-$(CONFIG_DFU_TFTP) += dfu_tftp.o > >> +endif > >> +obj-$(CONFIG_DFU_RAM) += dfu_ram.o > > >We should discard at link time the unreachable parts here in SPL, no? > > Yes you are correct. > But what is happening here is, the CONFIG_DFU_<MMC/NAND/SF/TFTP> selected through Kconfig/Menuconfig is applicable for both SPL and U-Boot. > Hence CONFIG_DFU_MMC/NAND/SF gets compiled for SPL as well, which needs run_command(). Actually CONFIG_DFU_MMC/NAND/etc is not scoped for SPL-DFU. > As we have aligned, not to increase the SPL size, user shall use SPL-DFU feature to boot to u-boot, then utilize the full featured DFU to flash MMC/NAND/SF. > > I get undefined reference to common function run_command(), "dfu_fill_entitiy_<mmc/nand/sf>" in driver/dfu/dfu.c. > The dfu.c is common for both SPL-DFU and U-boot. OK. I think we need to introduce SPL_DFU_xxx Kconfig options, and use CONFIG_IS_ENABLED(DFU_xxx) so that we will get things enabled/disabled (and discarded) as needed. -- 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/615ba049/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size 2017-04-27 12:31 ` Tom Rini @ 2017-04-27 17:25 ` B, Ravi 2017-05-03 8:36 ` B, Ravi 1 sibling, 0 replies; 27+ messages in thread From: B, Ravi @ 2017-04-27 17:25 UTC (permalink / raw) To: u-boot Hi Tom >> >> Yes you are correct. >> But what is happening here is, the CONFIG_DFU_<MMC/NAND/SF/TFTP> selected through Kconfig/Menuconfig is applicable for both SPL and U-Boot. >> Hence CONFIG_DFU_MMC/NAND/SF gets compiled for SPL as well, which needs run_command(). Actually CONFIG_DFU_MMC/NAND/etc is not scoped for SPL-DFU. >> As we have aligned, not to increase the SPL size, user shall use SPL-DFU feature to boot to u-boot, then utilize the full featured DFU to flash MMC/NAND/SF. >> >> I get undefined reference to common function run_command(), "dfu_fill_entitiy_<mmc/nand/sf>" in driver/dfu/dfu.c. >> The dfu.c is common for both SPL-DFU and U-boot. >OK. I think we need to introduce SPL_DFU_xxx Kconfig options, and use >CONFIG_IS_ENABLED(DFU_xxx) so that we will get things enabled/disabled (and discarded) as needed. Ok, will do. Regards Ravi ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size 2017-04-27 12:31 ` Tom Rini 2017-04-27 17:25 ` B, Ravi @ 2017-05-03 8:36 ` B, Ravi 2017-05-03 12:42 ` Tom Rini 1 sibling, 1 reply; 27+ messages in thread From: B, Ravi @ 2017-05-03 8:36 UTC (permalink / raw) To: u-boot Tom >>> >>> Yes you are correct. >>> But what is happening here is, the CONFIG_DFU_<MMC/NAND/SF/TFTP> selected through Kconfig/Menuconfig is applicable for both SPL and U-Boot. >>> Hence CONFIG_DFU_MMC/NA >ND/SF gets compiled for SPL as well, which needs run_command(). Actually CONFIG_DFU_MMC/NAND/etc is not scoped for SPL-DFU. >>> As we have aligned, not to increase the SPL size, user shall use SPL-DFU feature to boot to u-boot, then utilize the full featured DFU to flash MMC/NAND/SF. >>> >>> I get undefined reference to common function run_command(), "dfu_fill_entitiy_<mmc/nand/sf>" in driver/dfu/dfu.c. >>> The dfu.c is common for both SPL-DFU and U-boot. >>OK. I think we need to introduce SPL_DFU_xxx Kconfig options, and use >>CONFIG_IS_ENABLED(DFU_xxx) so that we will get things enabled/disabled (and discarded) as needed. >Ok, will do. Correct me if I am wrong, I need understand if we introduce say SPL_DFU_MMC Kconfig options, then whether need to support DFU_MMC in SPL ? Again this will increase the SPL-size, and also DFU_MMC uses run_command() again, there is dependency of cli.c, hush etc. Regards Ravi ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size 2017-05-03 8:36 ` B, Ravi @ 2017-05-03 12:42 ` Tom Rini 2017-05-03 12:45 ` B, Ravi 2017-05-03 20:48 ` Lukasz Majewski 0 siblings, 2 replies; 27+ messages in thread From: Tom Rini @ 2017-05-03 12:42 UTC (permalink / raw) To: u-boot On Wed, May 03, 2017 at 08:36:31AM +0000, B, Ravi wrote: > Tom > > >>> > >>> Yes you are correct. > >>> But what is happening here is, the CONFIG_DFU_<MMC/NAND/SF/TFTP> selected through Kconfig/Menuconfig is applicable for both SPL and U-Boot. > >>> Hence CONFIG_DFU_MMC/NA > >ND/SF gets compiled for SPL as well, which needs run_command(). Actually CONFIG_DFU_MMC/NAND/etc is not scoped for SPL-DFU. > >>> As we have aligned, not to increase the SPL size, user shall use SPL-DFU feature to boot to u-boot, then utilize the full featured DFU to flash MMC/NAND/SF. > >>> > >>> I get undefined reference to common function run_command(), "dfu_fill_entitiy_<mmc/nand/sf>" in driver/dfu/dfu.c. > >>> The dfu.c is common for both SPL-DFU and U-boot. > > >>OK. I think we need to introduce SPL_DFU_xxx Kconfig options, and use > >>CONFIG_IS_ENABLED(DFU_xxx) so that we will get things enabled/disabled (and discarded) as needed. > > >Ok, will do. > > Correct me if I am wrong, I need understand if we introduce say > SPL_DFU_MMC Kconfig options, then whether need to support DFU_MMC in > SPL ? > Again this will increase the SPL-size, and also DFU_MMC uses > run_command() again, there is dependency of cli.c, hush etc. SPL_DFU_MMC will only increase the size of SPL if it's enabled. Being able to switch to testing with CONFIG_IS_ENABLED(DFU_xxx) means that we'll be able to keep the space savings while also not making various parts of the code harder to read with more #ifdef tests. -- 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/20170503/f66352f4/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size 2017-05-03 12:42 ` Tom Rini @ 2017-05-03 12:45 ` B, Ravi 2017-05-03 20:53 ` Lukasz Majewski 2017-05-03 20:48 ` Lukasz Majewski 1 sibling, 1 reply; 27+ messages in thread From: B, Ravi @ 2017-05-03 12:45 UTC (permalink / raw) To: u-boot Hi Tom >> >> >>OK. I think we need to introduce SPL_DFU_xxx Kconfig options, and >> >>use >> >>CONFIG_IS_ENABLED(DFU_xxx) so that we will get things enabled/disabled (and discarded) as needed. >> >> >Ok, will do. >> >> Correct me if I am wrong, I need understand if we introduce say >> SPL_DFU_MMC Kconfig options, then whether need to support DFU_MMC in >> SPL ? >> Again this will increase the SPL-size, and also DFU_MMC uses >> run_command() again, there is dependency of cli.c, hush etc. >SPL_DFU_MMC will only increase the size of SPL if it's enabled. Being able to switch to testing with CONFIG_IS_ENABLED(DFU_xxx) means that we'll be able to keep the space savings while also not making various parts of the code harder to read with more #ifdef tests. Ok, since SPL_DFU_MMC will be added in Kconfig, if SPL_DFU_MMC is selected it will increase the SPL size, it means SPL_DFU_MMC dependency code shall be included (like cli,c, hush etc). When SPL_DFU_MMC is not selected then automatically SPL size will be reduced. Thanks. Regards Ravi ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size 2017-05-03 12:45 ` B, Ravi @ 2017-05-03 20:53 ` Lukasz Majewski 0 siblings, 0 replies; 27+ messages in thread From: Lukasz Majewski @ 2017-05-03 20:53 UTC (permalink / raw) To: u-boot On Wed, 3 May 2017 12:45:10 +0000 "B, Ravi" <ravibabu@ti.com> wrote: > Hi Tom > > >> > >> >>OK. I think we need to introduce SPL_DFU_xxx Kconfig options, > >> >>and use > >> >>CONFIG_IS_ENABLED(DFU_xxx) so that we will get things > >> >>enabled/disabled (and discarded) as needed. > >> > >> >Ok, will do. > >> > >> Correct me if I am wrong, I need understand if we introduce say > >> SPL_DFU_MMC Kconfig options, then whether need to support DFU_MMC > >> in SPL ? > >> Again this will increase the SPL-size, and also DFU_MMC uses > >> run_command() again, there is dependency of cli.c, hush etc. > > >SPL_DFU_MMC will only increase the size of SPL if it's enabled. > >Being able to switch to testing with CONFIG_IS_ENABLED(DFU_xxx) > >means that we'll be able to keep the space savings while also not > >making various parts of the code harder to read with more #ifdef > >tests. > > Ok, since SPL_DFU_MMC will be added in Kconfig, if SPL_DFU_MMC is > selected it will increase the SPL size, it means SPL_DFU_MMC > dependency code shall be included (like cli,c, hush etc). When > SPL_DFU_MMC is not selected then automatically SPL size will be > reduced. My impression here is that CONFIG_IS_ENABLED() will help us to make the code looking better and document changes by Kconfig variables. And to be honest - I do not feel like adding hush to SPL is a good conceptual solution. > > Thanks. > > 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 at denx.de ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size 2017-05-03 12:42 ` Tom Rini 2017-05-03 12:45 ` B, Ravi @ 2017-05-03 20:48 ` Lukasz Majewski 1 sibling, 0 replies; 27+ messages in thread From: Lukasz Majewski @ 2017-05-03 20:48 UTC (permalink / raw) To: u-boot Hi Tom, > On Wed, May 03, 2017 at 08:36:31AM +0000, B, Ravi wrote: > > Tom > > > > >>> > > >>> Yes you are correct. > > >>> But what is happening here is, the > > >>> CONFIG_DFU_<MMC/NAND/SF/TFTP> selected through > > >>> Kconfig/Menuconfig is applicable for both SPL and U-Boot. Hence > > >>> CONFIG_DFU_MMC/NA > > >ND/SF gets compiled for SPL as well, which needs run_command(). > > >Actually CONFIG_DFU_MMC/NAND/etc is not scoped for SPL-DFU. > > >>> As we have aligned, not to increase the SPL size, user shall > > >>> use SPL-DFU feature to boot to u-boot, then utilize the full > > >>> featured DFU to flash MMC/NAND/SF. > > >>> > > >>> I get undefined reference to common function run_command(), > > >>> "dfu_fill_entitiy_<mmc/nand/sf>" in driver/dfu/dfu.c. The dfu.c > > >>> is common for both SPL-DFU and U-boot. > > > > >>OK. I think we need to introduce SPL_DFU_xxx Kconfig options, > > >>and use CONFIG_IS_ENABLED(DFU_xxx) so that we will get things > > >>enabled/disabled (and discarded) as needed. > > > > >Ok, will do. > > > > Correct me if I am wrong, I need understand if we introduce say > > SPL_DFU_MMC Kconfig options, then whether need to support DFU_MMC in > > SPL ? > > Again this will increase the SPL-size, and also DFU_MMC uses > > run_command() again, there is dependency of cli.c, hush etc. > > SPL_DFU_MMC will only increase the size of SPL if it's enabled. Being > able to switch to testing with CONFIG_IS_ENABLED(DFU_xxx) means that > we'll be able to keep the space savings while also not making various > parts of the code harder to read with more #ifdef tests. > +1 (and I would like to see the code soon :-) ) 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 181 bytes Desc: OpenPGP digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170503/00f0a85e/attachment.sig> ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <1493212325-31879-1-git-send-email-ravibabu@ti.com>]
[parent not found: <1493212325-31879-3-git-send-email-ravibabu@ti.com>]
[parent not found: <20170427095527.2a3992fe@jawa>]
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu [not found] ` <20170427095527.2a3992fe@jawa> @ 2017-04-27 8:26 ` B, Ravi 2017-04-27 10:33 ` Lukasz Majewski 0 siblings, 1 reply; 27+ messages in thread From: B, Ravi @ 2017-04-27 8:26 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-27 8:26 ` [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu B, Ravi @ 2017-04-27 10:33 ` Lukasz Majewski 2017-04-27 10:34 ` Lukasz Majewski 0 siblings, 1 reply; 27+ messages in thread From: Lukasz Majewski @ 2017-04-27 10:33 UTC (permalink / raw) To: u-boot 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 at denx.de ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-27 10:33 ` Lukasz Majewski @ 2017-04-27 10:34 ` Lukasz Majewski 2017-04-27 11:19 ` B, Ravi 0 siblings, 1 reply; 27+ messages in thread From: Lukasz Majewski @ 2017-04-27 10:34 UTC (permalink / raw) To: u-boot 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 at 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 at denx.de ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-27 10:34 ` Lukasz Majewski @ 2017-04-27 11:19 ` B, Ravi 2017-04-27 13:09 ` Lukasz Majewski 0 siblings, 1 reply; 27+ messages in thread From: B, Ravi @ 2017-04-27 11:19 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-27 11:19 ` B, Ravi @ 2017-04-27 13:09 ` Lukasz Majewski 2017-04-27 17:30 ` B, Ravi 0 siblings, 1 reply; 27+ messages in thread From: Lukasz Majewski @ 2017-04-27 13:09 UTC (permalink / raw) To: u-boot 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 at denx.de ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu 2017-04-27 13:09 ` Lukasz Majewski @ 2017-04-27 17:30 ` B, Ravi 0 siblings, 0 replies; 27+ messages in thread From: B, Ravi @ 2017-04-27 17:30 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2017-05-03 20:53 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-26 13:14 [U-Boot] [PATCH 0/3] spl: dfu: misc fixes and reduce MLO foot print Ravi Babu
2017-04-26 13:14 ` [U-Boot] [PATCH 1/3] spl: Kconfig: dfu: spl-dfu depends on SPL_RAM_SUPPORT Ravi Babu
2017-04-26 13:36 ` Tom Rini
2017-04-26 13:14 ` [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu Ravi Babu
2017-04-26 13:40 ` Tom Rini
2017-04-26 15:58 ` B, Ravi
2017-04-26 16:24 ` Tom Rini
2017-04-26 16:25 ` B, Ravi
2017-04-27 8:06 ` Lukasz Majewski
2017-04-27 8:37 ` B, Ravi
2017-04-27 8:37 ` B, Ravi
2017-04-26 13:14 ` [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size Ravi Babu
2017-04-26 13:35 ` Tom Rini
2017-04-27 7:22 ` B, Ravi
2017-04-27 12:31 ` Tom Rini
2017-04-27 17:25 ` B, Ravi
2017-05-03 8:36 ` B, Ravi
2017-05-03 12:42 ` Tom Rini
2017-05-03 12:45 ` B, Ravi
2017-05-03 20:53 ` Lukasz Majewski
2017-05-03 20:48 ` Lukasz Majewski
[not found] <1493212325-31879-1-git-send-email-ravibabu@ti.com>
[not found] ` <1493212325-31879-3-git-send-email-ravibabu@ti.com>
[not found] ` <20170427095527.2a3992fe@jawa>
2017-04-27 8:26 ` [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu B, Ravi
2017-04-27 10:33 ` Lukasz Majewski
2017-04-27 10:34 ` Lukasz Majewski
2017-04-27 11:19 ` B, Ravi
2017-04-27 13:09 ` Lukasz Majewski
2017-04-27 17:30 ` B, Ravi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox