[U-Boot] [PATCH v3 0/3] spl: dfu: misc fixes and reduce MLO foot print
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
v2: - added Kconfig option CONFIG_SPL_DFU_MMC and use of CONFIG_IS_ENABLED(DFU_MMC). - compile out cli_hush. and use of cli_simple_run_command() instead of run_command(). - SPL size reduced by ~4K.
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 | 1 - common/dfu.c | 2 +- common/spl/Kconfig | 5 +++++ drivers/dfu/Kconfig | 5 +++++ drivers/dfu/Makefile | 3 +++ drivers/dfu/dfu.c | 4 ++++ drivers/dfu/dfu_mmc.c | 3 ++- include/dfu.h | 2 +- 8 files changed, 21 insertions(+), 4 deletions(-)
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
On Thu, May 04, 2017 at 03:45:28PM +0530, B, Ravi 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
Applied to u-boot/master, thanks!
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)
On Thu, May 04, 2017 at 03:45:29PM +0530, B, Ravi wrote:
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
Applied to u-boot/master, thanks!
compile out cli_hush.c for spl/dfu and use cli_simple_run_command for dfu to reduce the spl-dfu memory foot print.
Adding CONFIG_SPL_DFU_MMC to Kconfig and use CONFIG_IS_ENABLED(DFU_MMC).
Signed-off-by: Ravi Babu ravibabu@ti.com --- --- common/Makefile | 1 - drivers/dfu/Kconfig | 5 +++++ drivers/dfu/Makefile | 3 +++ drivers/dfu/dfu_mmc.c | 5 +++++ include/dfu.h | 2 +- 5 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/common/Makefile b/common/Makefile index 86225f1..6e90078 100644 --- a/common/Makefile +++ b/common/Makefile @@ -90,7 +90,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 diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig index 56a98f5..99a0db1 100644 --- a/drivers/dfu/Kconfig +++ b/drivers/dfu/Kconfig @@ -17,6 +17,11 @@ config DFU_MMC help This option enables using DFU to read and write to MMC based storage.
+config SPL_DFU_MMC + bool "MMC back end for SPL-DFU" + help + This option enables DFU for SPL to read and write to MMC based storage. + config DFU_NAND bool "NAND back end for DFU" help diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile index 61f2b71..7060908 100644 --- a/drivers/dfu/Makefile +++ b/drivers/dfu/Makefile @@ -6,7 +6,10 @@ #
obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o +ifndef CONFIG_SPL_BUILD obj-$(CONFIG_DFU_MMC) += dfu_mmc.o +endif +obj-$(CONFIG_SPL_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 diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 926ccbd..ba509db 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -15,6 +15,7 @@ #include <ext4fs.h> #include <fat.h> #include <mmc.h> +#include <cli.h>
static unsigned char *dfu_file_buf; static long dfu_file_buf_len; @@ -154,7 +155,11 @@ static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu,
debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
+#if CONFIG_IS_ENABLED(DFU_MMC) + ret = cli_simple_run_command(cmd_buf, 0); +#else ret = run_command(cmd_buf, 0); +#endif if (ret) { puts("dfu: Read error!\n"); return ret; diff --git a/include/dfu.h b/include/dfu.h index f39d3f1..94d2a49 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 CONFIG_IS_ENABLED(DFU_MMC) 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,
On Thu, May 04, 2017 at 03:45:30PM +0530, Ravi Babu wrote:
compile out cli_hush.c for spl/dfu and use cli_simple_run_command for dfu to reduce the spl-dfu memory foot print.
Adding CONFIG_SPL_DFU_MMC to Kconfig and use CONFIG_IS_ENABLED(DFU_MMC).
Signed-off-by: Ravi Babu ravibabu@ti.com
[snip]
diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile index 61f2b71..7060908 100644 --- a/drivers/dfu/Makefile +++ b/drivers/dfu/Makefile @@ -6,7 +6,10 @@ #
obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o +ifndef CONFIG_SPL_BUILD obj-$(CONFIG_DFU_MMC) += dfu_mmc.o +endif +obj-$(CONFIG_SPL_DFU_MMC) += dfu_mmc.o
This becomes obj-$(CONFIG_$(SPL_)DFU_MMC) += dfu_mmc.o
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 926ccbd..ba509db 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c
[snip]
@@ -154,7 +155,11 @@ static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu,
debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
+#if CONFIG_IS_ENABLED(DFU_MMC)
- ret = cli_simple_run_command(cmd_buf, 0);
+#else ret = run_command(cmd_buf, 0); +#endif
This doesn't make sense. CONFIG_IS_ENABLED(DFU_MMC) is true for CONFIG_DFU_MMC or CONFIG_SPL_DFU_MMC. Thanks!
Hi Tom
Sorry for late response, some how missed this mail.
obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o +ifndef CONFIG_SPL_BUILD obj-$(CONFIG_DFU_MMC) += dfu_mmc.o +endif +obj-$(CONFIG_SPL_DFU_MMC) += dfu_mmc.o
This becomes obj-$(CONFIG_$(SPL_)DFU_MMC) += dfu_mmc.o
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 926ccbd..ba509db 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c
[snip]
@@ -154,7 +155,11 @@ static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu,
debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
+#if CONFIG_IS_ENABLED(DFU_MMC)
- ret = cli_simple_run_command(cmd_buf, 0); #else ret = run_command(cmd_buf, 0);
+#endif
This doesn't make sense. CONFIG_IS_ENABLED(DFU_MMC) is true for CONFIG_DFU_MMC or CONFIG_SPL_DFU_MMC. Thanks!
True, My bad, I realized it lately after posting the patch.
I will use run_command() only, which abstrace use of both simple_cli_xx() and hush_parser.
Since cli_hush.c is compile out for SPL to reduce the size. SPL must use simple_cli_xx(). Since by default CONFIG_HUSH_PARSER is defined for both SPL/U-BOOT, this leads to compile error. I need to fix this way.
diff --git a/common/cli.c b/common/cli.c index a433ef2..5053f8a 100644 --- a/common/cli.c +++ b/common/cli.c @@ -28,7 +28,7 @@ DECLARE_GLOBAL_DATA_PTR; */ int run_command(const char *cmd, int flag) { -#ifndef CONFIG_HUSH_PARSER +#if !defined(CONFIG_HUSH_PARSER) || defined(CONFIG_SPL_BUILD) /* * cli_run_command can return 0 or 1 for success, so clean up * its result.
Is it ok, any other option ?
Regards Ravi
On Fri, May 12, 2017 at 08:44:31AM +0000, B, Ravi wrote:
Hi Tom
Sorry for late response, some how missed this mail.
obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o +ifndef CONFIG_SPL_BUILD obj-$(CONFIG_DFU_MMC) += dfu_mmc.o +endif +obj-$(CONFIG_SPL_DFU_MMC) += dfu_mmc.o
This becomes obj-$(CONFIG_$(SPL_)DFU_MMC) += dfu_mmc.o
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 926ccbd..ba509db 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c
[snip]
@@ -154,7 +155,11 @@ static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu,
debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
+#if CONFIG_IS_ENABLED(DFU_MMC)
- ret = cli_simple_run_command(cmd_buf, 0); #else ret = run_command(cmd_buf, 0);
+#endif
This doesn't make sense. CONFIG_IS_ENABLED(DFU_MMC) is true for CONFIG_DFU_MMC or CONFIG_SPL_DFU_MMC. Thanks!
True, My bad, I realized it lately after posting the patch.
I will use run_command() only, which abstrace use of both simple_cli_xx() and hush_parser.
Since cli_hush.c is compile out for SPL to reduce the size. SPL must use simple_cli_xx(). Since by default CONFIG_HUSH_PARSER is defined for both SPL/U-BOOT, this leads to compile error. I need to fix this way.
We keep running into problems due to trying to whack in what to do in DFU via command rather than via API. Can you make an attempt to convert things over, in both SPL and not SPL, in DFU to using ABI instead, to see if we can get the size reduction here still, and not have to try and put fragile to other use cases ifdefs in common code? Thanks!
Hi Tom
obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o +ifndef CONFIG_SPL_BUILD obj-$(CONFIG_DFU_MMC) += dfu_mmc.o +endif +obj-$(CONFIG_SPL_DFU_MMC) += dfu_mmc.o
This becomes obj-$(CONFIG_$(SPL_)DFU_MMC) += dfu_mmc.o
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 926ccbd..ba509db 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c
[snip]
@@ -154,7 +155,11 @@ static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu,
debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
+#if CONFIG_IS_ENABLED(DFU_MMC)
- ret = cli_simple_run_command(cmd_buf, 0); #else ret = run_command(cmd_buf, 0);
+#endif
This doesn't make sense. CONFIG_IS_ENABLED(DFU_MMC) is true for CONFIG_DFU_MMC or CONFIG_SPL_DFU_MMC. Thanks!
True, My bad, I realized it lately after posting the patch.
I will use run_command() only, which abstrace use of both simple_cli_xx() and hush_parser.
Since cli_hush.c is compile out for SPL to reduce the size. SPL must use simple_cli_xx(). Since by default CONFIG_HUSH_PARSER is defined for both SPL/U-BOOT, this leads to compile error. I need to fix this way.
We keep running into problems due to trying to whack in what to do in DFU via command rather than via API. Can you make an attempt to convert things over, in both SPL and not SPL, in DFU to using ABI instead, to see if we can get the size reduction here still, and >not have to try and put fragile to other use cases ifdefs in common code? Thanks!
Use of direct mmc read/write API instead of using the run_command() is not feasible in this case. As the DFU does read/write to mmc through fat/ext4 files system, uses "fatload/fatwrite" or "ext4load/ext4write" command to perform read/write to specific partition of mmc device. I think use of run_command() is more appropriate.
Regards Ravi
On Wed, May 24, 2017 at 12:11:57PM +0000, B, Ravi wrote:
Hi Tom
obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o +ifndef CONFIG_SPL_BUILD obj-$(CONFIG_DFU_MMC) += dfu_mmc.o +endif +obj-$(CONFIG_SPL_DFU_MMC) += dfu_mmc.o
This becomes obj-$(CONFIG_$(SPL_)DFU_MMC) += dfu_mmc.o
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 926ccbd..ba509db 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c
[snip]
@@ -154,7 +155,11 @@ static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu,
debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
+#if CONFIG_IS_ENABLED(DFU_MMC)
- ret = cli_simple_run_command(cmd_buf, 0); #else ret = run_command(cmd_buf, 0);
+#endif
This doesn't make sense. CONFIG_IS_ENABLED(DFU_MMC) is true for CONFIG_DFU_MMC or CONFIG_SPL_DFU_MMC. Thanks!
True, My bad, I realized it lately after posting the patch.
I will use run_command() only, which abstrace use of both simple_cli_xx() and hush_parser.
Since cli_hush.c is compile out for SPL to reduce the size. SPL must use simple_cli_xx(). Since by default CONFIG_HUSH_PARSER is defined for both SPL/U-BOOT, this leads to compile error. I need to fix this way.
We keep running into problems due to trying to whack in what to do in DFU via command rather than via API. Can you make an attempt to convert things over, in both SPL and not SPL, in DFU to using ABI instead, to see if we can get the size reduction here still, and >not have to try and put fragile to other use cases ifdefs in common code? Thanks!
Use of direct mmc read/write API instead of using the run_command() is not feasible in this case. As the DFU does read/write to mmc through fat/ext4 files system, uses "fatload/fatwrite" or "ext4load/ext4write" command to perform read/write to specific partition of mmc device. I think use of run_command() is more appropriate.
OK. Can you go back to https://patchwork.ozlabs.org/patch/758485/ and clean that up as I was saying? We should be able to call cli_simple_run_command in all cases and then not need cli_hush.c, yes? And we can then move common/cli.c into being compiled only in the non-SPL case I think (but use travis-ci to confirm). Thanks!
Hi Tom
This doesn't make sense. CONFIG_IS_ENABLED(DFU_MMC) is true for CONFIG_DFU_MMC or CONFIG_SPL_DFU_MMC. Thanks!
True, My bad, I realized it lately after posting the patch.
I will use run_command() only, which abstrace use of both simple_cli_xx() and hush_parser.
Since cli_hush.c is compile out for SPL to reduce the size. SPL must use simple_cli_xx(). Since by default CONFIG_HUSH_PARSER is defined for both SPL/U-BOOT, this leads to compile error. I need to fix this way.
We keep running into problems due to trying to whack in what to do in DFU via command rather than via API. Can you make an attempt to convert things over, in both SPL and not SPL, in DFU to using ABI instead, to see if we can get the size reduction here still, >and >not have to try and put fragile to other use cases ifdefs in common code? Thanks!
Use of direct mmc read/write API instead of using the run_command() is not feasible in this case. As the DFU does read/write to mmc through fat/ext4 files system, uses "fatload/fatwrite" or "ext4load/ext4write" command to perform read/write to specific >partition of mmc device. I think use of run_command() is more appropriate.
OK. Can you go back to https://patchwork.ozlabs.org/patch/758485/ and clean that up as I was saying? We should be able to call cli_simple_run_command in all cases and then not need cli_hush.c, yes? And we can then move common/cli.c into being compiled only in the non-SPL case I think (but use travis-ci to confirm). Thanks!
Ok, will do. Thanks.
In latest mainline, dfu has broken due to commit 1a9a5f7a: usb: host: xhci-omap: fix double weak board_usb_init functions. Due to this DFU gadget mode fails, due to weak board_usb_init() gets called which does not initialize the usb controller. I will fix this as well and send the patch.
Regards Ravi
participants (3)
-
B, Ravi -
Ravi Babu
-
Tom Rini