[PATCH] spl: Move check for SPL_LIBCOMMON support to header

From: "Andrew F. Davis" afd@ti.com
Print statements in SPL depend on lib/common support, due to this many such print statements are ifdef'd. Instead of checking at each call site move the check to the common.h header and remove these inline checks.
Signed-off-by: Andrew F. Davis afd@ti.com Reviewed-by: Simon Glass sjg@chromium.org --- boot/common_fit.c | 2 -- common/spl/spl.c | 4 +--- common/spl/spl_ext.c | 8 -------- common/spl/spl_fat.c | 6 ------ common/spl/spl_mmc.c | 18 ------------------ common/spl/spl_usb.c | 2 -- drivers/mmc/mmc-uclass.c | 2 -- drivers/mmc/mmc.c | 12 ------------ drivers/mmc/mmc_legacy.c | 2 -- include/stdio.h | 5 ++--- 10 files changed, 3 insertions(+), 58 deletions(-)
diff --git a/boot/common_fit.c b/boot/common_fit.c index cde2dc45e9..f2e1425075 100644 --- a/boot/common_fit.c +++ b/boot/common_fit.c @@ -54,10 +54,8 @@ int fit_find_config_node(const void *fdt) 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; }
diff --git a/common/spl/spl.c b/common/spl/spl.c index 29e0898f03..af962b6857 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -682,9 +682,7 @@ static int boot_from_devices(struct spl_image_info *spl_image, if (CONFIG_IS_ENABLED(SHOW_ERRORS)) ret = -ENXIO; loader = spl_ll_find_loader(bootdev); - if (CONFIG_IS_ENABLED(SERIAL) && - CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT) && - !IS_ENABLED(CONFIG_SILENT_CONSOLE)) { + if (!IS_ENABLED(CONFIG_SILENT_CONSOLE)) { if (loader) printf("Trying to boot from %s\n", spl_loader_name(loader)); diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c index ebd914c492..4d557b40b4 100644 --- a/common/spl/spl_ext.c +++ b/common/spl/spl_ext.c @@ -30,9 +30,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 return -1; }
@@ -56,11 +54,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 5b270541fc..29f7cdce8c 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; }
@@ -104,11 +102,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); } @@ -155,10 +151,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_mmc.c b/common/spl/spl_mmc.c index f66147477e..550d17dbb2 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -124,9 +124,7 @@ 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; }
@@ -143,9 +141,7 @@ static 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; } @@ -164,18 +160,14 @@ static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) err = mmc_initialize(NULL); #endif /* DM_MMC */ if (err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: could not initialize mmc. error: %d\n", err); -#endif return err; } *mmcp = find_mmc_device(mmc_dev); err = *mmcp ? 0 : -ENODEV; if (err) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: could not find mmc device %d. error: %d\n", mmc_dev, err); -#endif return err; }
@@ -208,9 +200,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; }
@@ -237,9 +227,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 != CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS) { -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT puts("mmc_load_image_raw_os: mmc block read error\n"); -#endif return -1; } #endif /* CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTOR */ @@ -418,9 +406,7 @@ int spl_mmc_load(struct spl_image_info *spl_image, err = mmc_init(mmc); if (err) { mmc = NULL; -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: mmc init failed with error: %d\n", err); -#endif return err; } } @@ -437,9 +423,7 @@ int spl_mmc_load(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 */ @@ -476,10 +460,8 @@ int spl_mmc_load(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_usb.c b/common/spl/spl_usb.c index ccf01c8276..2cab8af404 100644 --- a/common/spl/spl_usb.c +++ b/common/spl/spl_usb.c @@ -33,9 +33,7 @@ int spl_usb_load(struct spl_image_info *spl_image, }
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-uclass.c b/drivers/mmc/mmc-uclass.c index 688bdc06d4..95d6ffa00b 100644 --- a/drivers/mmc/mmc-uclass.c +++ b/drivers/mmc/mmc-uclass.c @@ -293,9 +293,7 @@ struct mmc *find_mmc_device(int dev_num) ret = blk_find_device(IF_TYPE_MMC, dev_num, &dev);
if (ret) { -#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/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 12d29da528..7e88bf60c1 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -291,9 +291,7 @@ int mmc_poll_for_busy(struct mmc *mmc, int timeout_ms) break;
if (status & MMC_STATUS_MASK) { -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) pr_err("Status Error: 0x%08x\n", status); -#endif return -ECOMM; }
@@ -304,9 +302,7 @@ int mmc_poll_for_busy(struct mmc *mmc, int timeout_ms) }
if (timeout_ms <= 0) { -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) pr_err("Timeout waiting card ready\n"); -#endif return -ETIMEDOUT; }
@@ -429,9 +425,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) pr_err("mmc fail to send stop cmd\n"); -#endif return 0; } } @@ -480,10 +474,8 @@ ulong mmc_bread(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt, return 0;
if ((start + blkcnt) > block_dev->lba) { -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) pr_err("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n", start + blkcnt, block_dev->lba); -#endif return 0; }
@@ -2846,10 +2838,8 @@ retry: err = mmc_send_op_cond(mmc);
if (err) { -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) if (!quiet) pr_err("Card did not respond to voltage select! : %d\n", err); -#endif return -EOPNOTSUPP; } } @@ -2900,9 +2890,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) pr_err("MMC: no card present\n"); -#endif return -ENOMEDIUM; }
diff --git a/drivers/mmc/mmc_legacy.c b/drivers/mmc/mmc_legacy.c index a05da6c2e8..f41ef562ec 100644 --- a/drivers/mmc/mmc_legacy.c +++ b/drivers/mmc/mmc_legacy.c @@ -45,9 +45,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/stdio.h b/include/stdio.h index 1939a48f0f..2fbc3da5f9 100644 --- a/include/stdio.h +++ b/include/stdio.h @@ -10,9 +10,8 @@ int tstc(void);
/* stdout */ #if !defined(CONFIG_SPL_BUILD) || \ - (defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_SERIAL)) || \ - (defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD) && \ - defined(CONFIG_SPL_SERIAL)) + (defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_SERIAL) && defined(CONFIG_TPL_LIBCOMMON_SUPPORT)) || \ + (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_SERIAL) && defined(CONFIG_SPL_LIBCOMMON_SUPPORT) && !defined(CONFIG_TPL_BUILD)) void putc(const char c); void puts(const char *s); int __printf(1, 2) printf(const char *fmt, ...);

Hi Andrew,
On Fri, 15 Jul 2022 at 09:35, Andrew Davis afd@ti.com wrote:
From: "Andrew F. Davis" afd@ti.com
Print statements in SPL depend on lib/common support, due to this many such print statements are ifdef'd. Instead of checking at each call site move the check to the common.h header and remove these inline checks.
nit: stdio.h I think?
Signed-off-by: Andrew F. Davis afd@ti.com Reviewed-by: Simon Glass sjg@chromium.org
boot/common_fit.c | 2 -- common/spl/spl.c | 4 +--- common/spl/spl_ext.c | 8 -------- common/spl/spl_fat.c | 6 ------ common/spl/spl_mmc.c | 18 ------------------ common/spl/spl_usb.c | 2 -- drivers/mmc/mmc-uclass.c | 2 -- drivers/mmc/mmc.c | 12 ------------ drivers/mmc/mmc_legacy.c | 2 -- include/stdio.h | 5 ++--- 10 files changed, 3 insertions(+), 58 deletions(-)
Regards, Simon

On 7/16/22 7:00 AM, Simon Glass wrote:
Hi Andrew,
On Fri, 15 Jul 2022 at 09:35, Andrew Davis afd@ti.com wrote:
From: "Andrew F. Davis" afd@ti.com
Print statements in SPL depend on lib/common support, due to this many such print statements are ifdef'd. Instead of checking at each call site move the check to the common.h header and remove these inline checks.
nit: stdio.h I think?
Ah yes, looks like this header got renamed since I wrote this patch. Will fix if v2 is needed.
Thanks, Andrew
Signed-off-by: Andrew F. Davis afd@ti.com Reviewed-by: Simon Glass sjg@chromium.org
boot/common_fit.c | 2 -- common/spl/spl.c | 4 +--- common/spl/spl_ext.c | 8 -------- common/spl/spl_fat.c | 6 ------ common/spl/spl_mmc.c | 18 ------------------ common/spl/spl_usb.c | 2 -- drivers/mmc/mmc-uclass.c | 2 -- drivers/mmc/mmc.c | 12 ------------ drivers/mmc/mmc_legacy.c | 2 -- include/stdio.h | 5 ++--- 10 files changed, 3 insertions(+), 58 deletions(-)
Regards, Simon

On Fri, Jul 15, 2022 at 10:35:00AM -0500, Andrew Davis wrote:
From: "Andrew F. Davis" afd@ti.com
Print statements in SPL depend on lib/common support, due to this many such print statements are ifdef'd. Instead of checking at each call site move the check to the common.h header and remove these inline checks.
Signed-off-by: Andrew F. Davis afd@ti.com Reviewed-by: Simon Glass sjg@chromium.org
I thought we had this already, but I guess not. That said:
[snip]
diff --git a/include/stdio.h b/include/stdio.h index 1939a48f0f..2fbc3da5f9 100644 --- a/include/stdio.h +++ b/include/stdio.h @@ -10,9 +10,8 @@ int tstc(void);
/* stdout */ #if !defined(CONFIG_SPL_BUILD) || \
- (defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_SERIAL)) || \
- (defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD) && \
defined(CONFIG_SPL_SERIAL))
- (defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_SERIAL) && defined(CONFIG_TPL_LIBCOMMON_SUPPORT)) || \
- (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_SERIAL) && defined(CONFIG_SPL_LIBCOMMON_SUPPORT) && !defined(CONFIG_TPL_BUILD))
Can we write this as something like: #if !defined(CONFIG_SPL_BUILD) || \ (CONFIG_IS_ENABLED(BUILD) && CONFIG_IS_ENABLED(SERIAL) && CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT))
? I'm not 100% sure it will work for CONFIG_SPL_BUILD / CONFIG_TPL_BUILD but it should for the rest and mean we can slightly clean it up.

On Fri, Jul 15, 2022 at 10:35:00AM -0500, Andrew Davis wrote:
From: "Andrew F. Davis" afd@ti.com
Print statements in SPL depend on lib/common support, due to this many such print statements are ifdef'd. Instead of checking at each call site move the check to the common.h header and remove these inline checks.
Signed-off-by: Andrew F. Davis afd@ti.com Reviewed-by: Simon Glass sjg@chromium.org
This leads to a number of different cases of build failure. Looking at evb-px30 and P1010RDB-PA_36BIT_NAND show I think two different ones. Using the public CI infrastructure to ensure a clean world build is strongly advised for the next iteration of this, thanks!
participants (3)
-
Andrew Davis
-
Simon Glass
-
Tom Rini