spl: Move check for SPL_LIBCOMMON support to header

PATCH New Andrew F. Davis 3 participants 9 messages Jan 6, 2017 – Jan 21, 2017 MMC, SD and SDIO

Share & tools

Andrew F. Davis

author

2017-01-06 19:35 UTC #0 lore raw

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 <[email protected]>

---
 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(-)

common/spl/spl.c +0 -2

index f7df834e32..bcf408ce8e 100644
@@ -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;
 	}

common/spl/spl_ext.c +0 -8

index 1b8e15e37d..6165f88d2b 100644
@@ -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;
 	}

common/spl/spl_fat.c +0 -6

index a14acceebb..aa5a39e4f7 100644
@@ -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;
 	}

common/spl/spl_fit.c +0 -4

index aae556f97d..37ce7cc35f 100644
@@ -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;
 }

common/spl/spl_mmc.c +0 ~~-18~~

index 85e3de8f23..af41afd726 100644
@@ -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;

common/spl/spl_sata.c +0 -2

index 5476206131..c34bce84eb 100644
@@ -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 */

common/spl/spl_usb.c +0 -2

index 567a450505..0bb599d1fa 100644
@@ -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;
 	}

drivers/mmc/mmc.c +0 ~~-10~~

index 9f8368a123..36b8d90429 100644
@@ -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;
 		}
 	}

drivers/mmc/mmc_legacy.c +0 -2

index bdf9d984dd..e7ca05725f 100644
@@ -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;
 }

include/common.h +1 -1

index a8d833b989..773b7f1710 100644
@@ -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, ...)

Signature · 1 line

2.11.0

Andrew F. Davis

author

2017-01-06 19:35 UTC #1 lore raw

PATCH 2/2 spl: Remove inline ifdef check for EXT and FAT support

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 <[email protected]>

---
 common/spl/spl_ext.c | 2 --
 common/spl/spl_fat.c | 2 --
 2 files changed, 4 deletions(-)

common/spl/spl_ext.c +0 -2

index 6165f88d2b..a58f7ef3fe 100644
@@ -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

common/spl/spl_fat.c +0 -2

index aa5a39e4f7..66917ffa4f 100644
@@ -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

Signature · 1 line

2.11.0

Tom Rini

2017-01-11 16:28 UTC #2 lore raw

PATCH 2/2 spl: Remove inline ifdef check for EXT and FAT support

Quoted 5 lines · Andrew F. Davis

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 <[email protected]>

Reviewed-by Tom Rini <[email protected]>

Signature · 8 lines

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/20170111/d524eed7/attachment.sig>

Simon Glass

2017-01-19 13:57 UTC #3 lore raw

PATCH 2/2 spl: Remove inline ifdef check for EXT and FAT support

Quoted 6 lines · Tom Rini

Quoted 4 lines · Andrew F. Davis
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.

Quoted 6 lines

Quoted 1 line
Signed-off-by Andrew F. Davis <[email protected]>
Reviewed-by Tom Rini <[email protected]>
--
Tom

Reviewed-by Simon Glass <[email protected]>

Tom Rini

2017-01-21 03:37 UTC #4 lore raw

PATCH 2/2 spl: Remove inline ifdef check for EXT and FAT support

In reply to Andrew F. Davis

Quoted 7 lines · Andrew F. Davis

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 <[email protected]>
Reviewed-by Tom Rini <[email protected]>
Reviewed-by Simon Glass <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines

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/20170120/1baf1cf6/attachment.sig>

Tom Rini

2017-01-11 16:28 UTC #5 lore raw

PATCH 1/2 spl: Move check for SPL_LIBCOMMON support to header

In reply to Andrew F. Davis

Quoted 5 lines · Andrew F. Davis

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 <[email protected]>

Reviewed-by Tom Rini <[email protected]>

Signature · 8 lines

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/20170111/bfd54164/attachment.sig>

Tom Rini

2017-01-20 16:25 UTC #6 lore raw

PATCH 1/2 spl: Move check for SPL_LIBCOMMON support to header

In reply to Andrew F. Davis

Quoted 6 lines · Andrew F. Davis

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 <[email protected]>
Reviewed-by Tom Rini <[email protected]>

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!

Signature · 8 lines

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/20170120/ab0e17ab/attachment.sig>

Andrew F. Davis

author

2017-01-20 16:38 UTC #7 lore raw

PATCH 1/2 spl: Move check for SPL_LIBCOMMON support to header

Quoted 9 lines · Tom Rini

Quoted 6 lines · Andrew F. Davis
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 <[email protected]>
Reviewed-by Tom Rini <[email protected]>

Quoted 14 lines

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

Tom Rini

2017-01-20 20:57 UTC #8 lore raw

PATCH 1/2 spl: Move check for SPL_LIBCOMMON support to header

Quoted 24 lines · Andrew F. Davis

Quoted 9 lines · Tom Rini
Quoted 6 lines · Andrew F. Davis
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 <[email protected]>
Reviewed-by Tom Rini <[email protected]>
Quoted 13 lines
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!

Quoted 3 lines

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.

Quoted 4 lines

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

Signature · 8 lines

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/20170120/34fbb6d9/attachment.sig>