[U-Boot] [PATCH 1/2] spl: Move check for SPL_LIBCOMMON support to header
Print statements in SPL depend on lib/common support, so many such statements are ifdef'd, move the check to the common.h header and remove these inline checks.
Signed-off-by: Andrew F. Davis afd@ti.com --- common/spl/spl.c | 2 -- common/spl/spl_ext.c | 8 -------- common/spl/spl_fat.c | 6 ------ common/spl/spl_fit.c | 4 ---- common/spl/spl_mmc.c | 18 ------------------ common/spl/spl_sata.c | 2 -- common/spl/spl_usb.c | 2 -- drivers/mmc/mmc.c | 10 ---------- drivers/mmc/mmc_legacy.c | 2 -- include/common.h | 2 +- 10 files changed, 1 insertion(+), 55 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index f7df834e32..bcf408ce8e 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -314,12 +314,10 @@ static int boot_from_devices(struct spl_image_info *spl_image, struct spl_image_loader *loader;
loader = spl_ll_find_loader(spl_boot_list[i]); -#if defined(CONFIG_SPL_SERIAL_SUPPORT) && defined(CONFIG_SPL_LIBCOMMON_SUPPORT) if (loader) printf("Trying to boot from %s", loader->name); else puts("SPL: Unsupported Boot Device!\n"); -#endif if (loader && !spl_load_image(spl_image, loader)) return 0; } diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c index 1b8e15e37d..6165f88d2b 100644 --- a/common/spl/spl_ext.c +++ b/common/spl/spl_ext.c @@ -31,9 +31,7 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
err = ext4fs_mount(0); if (!err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("%s: ext4fs mount err - %d\n", __func__, err); -#endif goto end; }
@@ -57,11 +55,9 @@ int spl_load_image_ext(struct spl_image_info *spl_image, err = ext4fs_read((char *)spl_image->load_addr, 0, filelen, &actlen);
end: -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT if (err < 0) printf("%s: error reading image %s, err - %d\n", __func__, filename, err); -#endif
return err < 0; } @@ -84,9 +80,7 @@ int spl_load_image_ext_os(struct spl_image_info *spl_image,
err = ext4fs_mount(0); if (!err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("%s: ext4fs mount err - %d\n", __func__, err); -#endif return -1; } #if defined(CONFIG_SPL_ENV_SUPPORT) @@ -129,10 +123,8 @@ defaults:
err = ext4fs_read((void *)CONFIG_SYS_SPL_ARGS_ADDR, 0, filelen, &actlen); if (err < 0) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("%s: error reading image %s, err - %d\n", __func__, CONFIG_SPL_FS_LOAD_ARGS_NAME, err); -#endif return -1; }
diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c index a14acceebb..aa5a39e4f7 100644 --- a/common/spl/spl_fat.c +++ b/common/spl/spl_fat.c @@ -29,9 +29,7 @@ static int spl_register_fat_device(struct blk_desc *block_dev, int partition)
err = fat_register_device(block_dev, partition); if (err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("%s: fat register err - %d\n", __func__, err); -#endif return err; }
@@ -93,11 +91,9 @@ int spl_load_image_fat(struct spl_image_info *spl_image, }
end: -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT if (err <= 0) printf("%s: error reading image %s, err - %d\n", __func__, filename, err); -#endif
return (err <= 0); } @@ -143,10 +139,8 @@ defaults: err = file_fat_read(CONFIG_SPL_FS_LOAD_ARGS_NAME, (void *)CONFIG_SYS_SPL_ARGS_ADDR, 0); if (err <= 0) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("%s: error reading image %s, err - %d\n", __func__, CONFIG_SPL_FS_LOAD_ARGS_NAME, err); -#endif return -1; }
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index aae556f97d..37ce7cc35f 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -40,10 +40,8 @@ static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp) node = fdt_next_subnode(fdt, node)) { name = fdt_getprop(fdt, node, "description", &len); if (!name) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("%s: Missing FDT description in DTB\n", __func__); -#endif return -EINVAL; } if (board_fit_config_name_match(name)) @@ -72,7 +70,6 @@ static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp) return len; }
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("No matching DT out of these options:\n"); for (node = fdt_first_subnode(fdt, conf); node >= 0; @@ -80,7 +77,6 @@ static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp) name = fdt_getprop(fdt, node, "description", &len); printf(" %s\n", name); } -#endif
return -ENOENT; } diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index 85e3de8f23..af41afd726 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -87,9 +87,7 @@ static int mmc_load_image_raw_sector(struct spl_image_info *spl_image,
end: if (ret) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT puts("mmc_load_image_raw_sector: mmc block read error\n"); -#endif return -1; }
@@ -106,9 +104,7 @@ int spl_mmc_get_device_index(u32 boot_device) return 1; }
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: unsupported mmc boot device.\n"); -#endif
return -ENODEV; } @@ -126,9 +122,7 @@ static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)
err = mmc_initialize(NULL); if (err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: could not initialize mmc. error: %d\n", err); -#endif return err; }
@@ -141,9 +135,7 @@ static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) err = *mmcp ? 0 : -ENODEV; #endif if (err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: could not find mmc device. error: %d\n", err); -#endif return err; }
@@ -159,9 +151,7 @@ static int mmc_load_image_raw_partition(struct spl_image_info *spl_image,
err = part_get_info(mmc_get_blk_desc(mmc), partition, &info); if (err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT puts("spl: partition error\n"); -#endif return -1; }
@@ -193,9 +183,7 @@ static int mmc_load_image_raw_os(struct spl_image_info *spl_image, CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS, (void *) CONFIG_SYS_SPL_ARGS_ADDR); if (count == 0) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT puts("mmc_load_image_raw_os: mmc block read error\n"); -#endif return -1; }
@@ -286,9 +274,7 @@ int spl_mmc_load_image(struct spl_image_info *spl_image,
err = mmc_init(mmc); if (err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: mmc init failed with error: %d\n", err); -#endif return err; }
@@ -312,9 +298,7 @@ int spl_mmc_load_image(struct spl_image_info *spl_image, err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
if (err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT puts("spl: mmc partition switch failed\n"); -#endif return err; } /* Fall through */ @@ -346,10 +330,8 @@ int spl_mmc_load_image(struct spl_image_info *spl_image, return err;
break; -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT default: puts("spl: mmc: wrong boot mode\n"); -#endif }
return err; diff --git a/common/spl/spl_sata.c b/common/spl/spl_sata.c index 5476206131..c34bce84eb 100644 --- a/common/spl/spl_sata.c +++ b/common/spl/spl_sata.c @@ -28,9 +28,7 @@ static int spl_sata_load_image(struct spl_image_info *spl_image,
err = init_sata(CONFIG_SPL_SATA_BOOT_DEVICE); if (err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: sata init failed: err - %d\n", err); -#endif return err; } else { /* try to recognize storage devices immediately */ diff --git a/common/spl/spl_usb.c b/common/spl/spl_usb.c index 567a450505..0bb599d1fa 100644 --- a/common/spl/spl_usb.c +++ b/common/spl/spl_usb.c @@ -31,9 +31,7 @@ static int spl_usb_load_image(struct spl_image_info *spl_image, usb_stop(); err = usb_init(); if (err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("%s: usb init failed: err - %d\n", __func__, err); -#endif return err; }
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 9f8368a123..36b8d90429 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -180,10 +180,8 @@ int mmc_send_status(struct mmc *mmc, int timeout) MMC_STATE_PRG) break; else if (cmd.response[0] & MMC_STATUS_MASK) { -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) printf("Status Error: 0x%08X\n", cmd.response[0]); -#endif return -ECOMM; } } else if (--retries < 0) @@ -197,9 +195,7 @@ int mmc_send_status(struct mmc *mmc, int timeout)
mmc_trace_state(mmc, &cmd); if (timeout <= 0) { -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) printf("Timeout waiting card ready\n"); -#endif return -ETIMEDOUT; }
@@ -251,9 +247,7 @@ static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start, cmd.cmdarg = 0; cmd.resp_type = MMC_RSP_R1b; if (mmc_send_cmd(mmc, &cmd, NULL)) { -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) printf("mmc fail to send stop cmd\n"); -#endif return 0; } } @@ -1654,9 +1648,7 @@ int mmc_start_init(struct mmc *mmc) #endif if (no_card) { mmc->has_init = 0; -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) printf("MMC: no card present\n"); -#endif return -ENOMEDIUM; }
@@ -1702,9 +1694,7 @@ int mmc_start_init(struct mmc *mmc) err = mmc_send_op_cond(mmc);
if (err) { -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) printf("Card did not respond to voltage select!\n"); -#endif return -EOPNOTSUPP; } } diff --git a/drivers/mmc/mmc_legacy.c b/drivers/mmc/mmc_legacy.c index bdf9d984dd..e7ca05725f 100644 --- a/drivers/mmc/mmc_legacy.c +++ b/drivers/mmc/mmc_legacy.c @@ -26,9 +26,7 @@ struct mmc *find_mmc_device(int dev_num) return m; }
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) printf("MMC Device %d not found\n", dev_num); -#endif
return NULL; } diff --git a/include/common.h b/include/common.h index a8d833b989..773b7f1710 100644 --- a/include/common.h +++ b/include/common.h @@ -869,7 +869,7 @@ int tstc(void); #if !defined(CONFIG_SPL_BUILD) || \ (defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_SERIAL_SUPPORT)) || \ (defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD) && \ - defined(CONFIG_SPL_SERIAL_SUPPORT)) + defined(CONFIG_SPL_SERIAL_SUPPORT) && CONFIG_SPL_LIBCOMMON_SUPPORT) void putc(const char c); void puts(const char *s); int printf(const char *fmt, ...)
These files are only included for build by the make system when CONFIG_SPL_{EXT,FAT}_SUPPORT is enabled, remove the unneed checks for these in the source files.
Signed-off-by: Andrew F. Davis afd@ti.com --- common/spl/spl_ext.c | 2 -- common/spl/spl_fat.c | 2 -- 2 files changed, 4 deletions(-)
diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c index 6165f88d2b..a58f7ef3fe 100644 --- a/common/spl/spl_ext.c +++ b/common/spl/spl_ext.c @@ -9,7 +9,6 @@ #include <errno.h> #include <image.h>
-#ifdef CONFIG_SPL_EXT_SUPPORT int spl_load_image_ext(struct spl_image_info *spl_image, struct blk_desc *block_dev, int partition, const char *filename) @@ -138,4 +137,3 @@ int spl_load_image_ext_os(struct spl_image_info *spl_image, return -ENOSYS; } #endif -#endif diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c index aa5a39e4f7..66917ffa4f 100644 --- a/common/spl/spl_fat.c +++ b/common/spl/spl_fat.c @@ -19,7 +19,6 @@
static int fat_registered;
-#ifdef CONFIG_SPL_FAT_SUPPORT static int spl_register_fat_device(struct blk_desc *block_dev, int partition) { int err = 0; @@ -154,4 +153,3 @@ int spl_load_image_fat_os(struct spl_image_info *spl_image, return -ENOSYS; } #endif -#endif
On Fri, Jan 06, 2017 at 01:35:45PM -0600, Andrew F. Davis wrote:
These files are only included for build by the make system when CONFIG_SPL_{EXT,FAT}_SUPPORT is enabled, remove the unneed checks for these in the source files.
Signed-off-by: Andrew F. Davis afd@ti.com
Reviewed-by: Tom Rini trini@konsulko.com
On 11 January 2017 at 09:28, Tom Rini trini@konsulko.com wrote:
On Fri, Jan 06, 2017 at 01:35:45PM -0600, Andrew F. Davis wrote:
These files are only included for build by the make system when CONFIG_SPL_{EXT,FAT}_SUPPORT is enabled, remove the unneed checks for these in the source files.
Signed-off-by: Andrew F. Davis afd@ti.com
Reviewed-by: Tom Rini trini@konsulko.com
-- Tom
Reviewed-by: Simon Glass sjg@chromium.org
On Fri, Jan 06, 2017 at 01:35:45PM -0600, Andrew F. Davis wrote:
These files are only included for build by the make system when CONFIG_SPL_{EXT,FAT}_SUPPORT is enabled, remove the unneed checks for these in the source files.
Signed-off-by: Andrew F. Davis afd@ti.com Reviewed-by: Tom Rini trini@konsulko.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
On Fri, Jan 06, 2017 at 01:35:44PM -0600, Andrew F. Davis wrote:
Print statements in SPL depend on lib/common support, so many such statements are ifdef'd, move the check to the common.h header and remove these inline checks.
Signed-off-by: Andrew F. Davis afd@ti.com
Reviewed-by: Tom Rini trini@konsulko.com
On Fri, Jan 06, 2017 at 01:35:44PM -0600, Andrew F. Davis wrote:
Print statements in SPL depend on lib/common support, so many such statements are ifdef'd, move the check to the common.h header and remove these inline checks.
Signed-off-by: Andrew F. Davis afd@ti.com Reviewed-by: Tom Rini trini@konsulko.com
This patch is a good example of why travis-ci is useful, even if takes a few hours for the cycle to complete (kick it off and check the results in the morning :)). As is, it's broken on PowerPC (where CONFIG_SPL_INIT_MINIMAL is the conditional for puts/printf/etc), mx31pdk and evb-rk3036 (and this is an incomplete list). The ARM targets are harder to just fix as it shows an underlying problem. Today we have no single symbol that means "In SPL I want serial output" (and ditto TPL). We try and rely on SPL_SERIAL_SUPPORT but this misses the case where we use neither TINY_PRINTF nor LIBCOMMON but instead have only puts available in a more raw way. So before we can make the type of change you're doing here we need to introduce a symbol that means "I have output". That will also greatly reduce the logic needed in the tests in common.h for having puts/etc be real or do-while loops. Thanks!
On 01/20/2017 10:25 AM, Tom Rini wrote:
On Fri, Jan 06, 2017 at 01:35:44PM -0600, Andrew F. Davis wrote:
Print statements in SPL depend on lib/common support, so many such statements are ifdef'd, move the check to the common.h header and remove these inline checks.
Signed-off-by: Andrew F. Davis afd@ti.com Reviewed-by: Tom Rini trini@konsulko.com
This patch is a good example of why travis-ci is useful, even if takes a few hours for the cycle to complete (kick it off and check the results in the morning :)). As is, it's broken on PowerPC (where CONFIG_SPL_INIT_MINIMAL is the conditional for puts/printf/etc), mx31pdk and evb-rk3036 (and this is an incomplete list). The ARM targets are harder to just fix as it shows an underlying problem. Today we have no single symbol that means "In SPL I want serial output" (and ditto TPL). We try and rely on SPL_SERIAL_SUPPORT but this misses the case where we use neither TINY_PRINTF nor LIBCOMMON but instead have only puts available in a more raw way. So before we can make the type of change you're doing here we need to introduce a symbol that means "I have output". That will also greatly reduce the logic needed in the tests in common.h for having puts/etc be real or do-while loops. Thanks!
Ahh, I was expecting something like this happening with all the levels of ifdef logic involved. Could we keep this patch and add CONFIG_SPL_INIT_MINIMAL to the check in common.h?
Removing all these various ifdefs from files is still correct, all this ifdef logic should *not* be around calls to puts/etc in-line in the code, all we need is to figure out the logic for the function's definitions in common.h, which can be cleaned up in a later patch.
Andrew
On Fri, Jan 20, 2017 at 10:38:41AM -0600, Andrew F. Davis wrote:
On 01/20/2017 10:25 AM, Tom Rini wrote:
On Fri, Jan 06, 2017 at 01:35:44PM -0600, Andrew F. Davis wrote:
Print statements in SPL depend on lib/common support, so many such statements are ifdef'd, move the check to the common.h header and remove these inline checks.
Signed-off-by: Andrew F. Davis afd@ti.com Reviewed-by: Tom Rini trini@konsulko.com
This patch is a good example of why travis-ci is useful, even if takes a few hours for the cycle to complete (kick it off and check the results in the morning :)). As is, it's broken on PowerPC (where CONFIG_SPL_INIT_MINIMAL is the conditional for puts/printf/etc), mx31pdk and evb-rk3036 (and this is an incomplete list). The ARM targets are harder to just fix as it shows an underlying problem. Today we have no single symbol that means "In SPL I want serial output" (and ditto TPL). We try and rely on SPL_SERIAL_SUPPORT but this misses the case where we use neither TINY_PRINTF nor LIBCOMMON but instead have only puts available in a more raw way. So before we can make the type of change you're doing here we need to introduce a symbol that means "I have output". That will also greatly reduce the logic needed in the tests in common.h for having puts/etc be real or do-while loops. Thanks!
Ahh, I was expecting something like this happening with all the levels of ifdef logic involved. Could we keep this patch and add CONFIG_SPL_INIT_MINIMAL to the check in common.h?
I did that, but it does not address the problem of mx31pdk and rockchip and others.
Removing all these various ifdefs from files is still correct, all this ifdef logic should *not* be around calls to puts/etc in-line in the code, all we need is to figure out the logic for the function's definitions in common.h, which can be cleaned up in a later patch.
Well, thinking about this harder. What would be OK for now would be a v2 (that you've pushed through travis-ci) that just removes the tests as that should be OK. But... I wonder if we don't have those tests due to needing to #if out the prints due to the gcc bug[1] that's only fixed in gcc 6.x and later.
[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54303
participants (3)
-
Andrew F. Davis -
Simon Glass -
Tom Rini