[PATCH 09/14] spl: mmc: Handle error codes consistently

Sean Anderson seanga2 at gmail.com
Sat Jul 20 17:58:40 CEST 2024


On 7/20/24 02:17, Simon Glass wrote:
> Use 'ret' as the return code, since it may not be an error and this is
> the common name in U-Boot. Make sure to return the error code when
> given, rather than transforming it into -1 (-EPERM).

Does this transformation affect code size?

> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
>   common/spl/spl_mmc.c | 129 ++++++++++++++++++++++---------------------
>   1 file changed, 65 insertions(+), 64 deletions(-)
> 
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index ddc85aaeda7..70e376c0ed8 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -52,7 +52,7 @@ int mmc_load_image_raw_sector(struct spl_image_info *spl_image,
>   	ret = spl_load(spl_image, bootdev, &load, 0, sector << bd->log2blksz);
>   	if (ret) {
>   		puts("mmc_load_image_raw_sector: mmc block read error\n");
> -		return -1;
> +		return ret;
>   	}
>   
>   	return 0;
> @@ -75,27 +75,27 @@ static int spl_mmc_get_device_index(u32 boot_device)
>   
>   static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)
>   {
> -	int err, mmc_dev;
> +	int ret, mmc_dev;
>   
>   	mmc_dev = spl_mmc_get_device_index(boot_device);
>   	if (mmc_dev < 0)
>   		return mmc_dev;
>   
>   #if CONFIG_IS_ENABLED(DM_MMC)
> -	err = mmc_init_device(mmc_dev);
> +	ret = mmc_init_device(mmc_dev);
>   #else
> -	err = mmc_initialize(NULL);
> +	ret = mmc_initialize(NULL);
>   #endif /* DM_MMC */
> -	if (err) {
> -		printf("spl: could not initialize mmc. error: %d\n", err);
> -		return err;
> +	if (ret) {
> +		printf("spl: could not initialize mmc. error: %d\n", ret);
> +		return ret;
>   	}
>   	*mmcp = find_mmc_device(mmc_dev);
> -	err = *mmcp ? 0 : -ENODEV;
> -	if (err) {
> +	ret = *mmcp ? 0 : -ENODEV;
> +	if (ret) {
>   		printf("spl: could not find mmc device %d. error: %d\n",
> -		       mmc_dev, err);
> -		return err;
> +		       mmc_dev, ret);
> +		return ret;
>   	}
>   
>   	return 0;
> @@ -108,14 +108,14 @@ static int mmc_load_image_raw_partition(struct spl_image_info *spl_image,
>   					unsigned long sector)
>   {
>   	struct disk_partition info;
> -	int err;
> +	int ret;
>   
>   #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
>   	int type_part;
>   	/* Only support MBR so DOS_ENTRY_NUMBERS */
>   	for (type_part = 1; type_part <= DOS_ENTRY_NUMBERS; type_part++) {
> -		err = part_get_info(mmc_get_blk_desc(mmc), type_part, &info);
> -		if (err)
> +		ret = part_get_info(mmc_get_blk_desc(mmc), type_part, &info);
> +		if (ret)
>   			continue;
>   		if (info.sys_ind ==
>   			CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE) {
> @@ -125,10 +125,10 @@ static int mmc_load_image_raw_partition(struct spl_image_info *spl_image,
>   	}
>   #endif
>   
> -	err = part_get_info(mmc_get_blk_desc(mmc), partition, &info);
> -	if (err) {
> +	ret = part_get_info(mmc_get_blk_desc(mmc), partition, &info);
> +	if (ret) {
>   		puts("spl: partition error\n");
> -		return -err;
> +		return ret;
>   	}
>   
>   #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
> @@ -155,7 +155,7 @@ static int mmc_load_image_raw_os(struct spl_image_info *spl_image,
>   		(void *)CONFIG_SPL_PAYLOAD_ARGS_ADDR);
>   	if (count != CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS) {
>   		puts("mmc_load_image_raw_os: mmc block read error\n");
> -		return -1;
> +		return -EIO;
>   	}
>   #endif	/* CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTOR */
>   
> @@ -193,7 +193,7 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image,
>   			      struct mmc *mmc,
>   			      const char *filename)
>   {
> -	int err = -ENOSYS;
> +	int ret = -ENOSYS;
>   
>   	__maybe_unused int partition = CONFIG_SYS_MMCSD_FS_BOOT_PARTITION;
>   
> @@ -202,8 +202,8 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image,
>   		struct disk_partition info;
>   		debug("Checking for the first MBR bootable partition\n");
>   		for (int type_part = 1; type_part <= DOS_ENTRY_NUMBERS; type_part++) {
> -			err = part_get_info(mmc_get_blk_desc(mmc), type_part, &info);
> -			if (err)
> +			ret = part_get_info(mmc_get_blk_desc(mmc), type_part, &info);
> +			if (ret)
>   				continue;
>   			debug("Partition %d is of type %d and bootable=%d\n", type_part, info.sys_ind, info.bootable);
>   			if (info.bootable != 0) {
> @@ -221,40 +221,40 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image,
>   
>   #ifdef CONFIG_SPL_FS_FAT
>   	if (!spl_start_uboot()) {
> -		err = spl_load_image_fat_os(spl_image, bootdev, mmc_get_blk_desc(mmc),
> -			partition);
> -		if (!err)
> -			return err;
> +		ret = spl_load_image_fat_os(spl_image, bootdev, mmc_get_blk_desc(mmc),
> +					    partition);
> +		if (!ret)
> +			return ret;

Should be return 0 for clarity. And the same for the other instances of this pattern.

>   	}
>   #ifdef CONFIG_SPL_FS_LOAD_PAYLOAD_NAME
> -	err = spl_load_image_fat(spl_image, bootdev, mmc_get_blk_desc(mmc),
> +	ret = spl_load_image_fat(spl_image, bootdev, mmc_get_blk_desc(mmc),
>   				 partition,
>   				 filename);
> -	if (!err)
> -		return err;
> +	if (!ret)
> +		return ret;
>   #endif
>   #endif
>   #ifdef CONFIG_SPL_FS_EXT4
>   	if (!spl_start_uboot()) {
> -		err = spl_load_image_ext_os(spl_image, bootdev, mmc_get_blk_desc(mmc),
> -			partition);
> -		if (!err)
> -			return err;
> +		ret = spl_load_image_ext_os(spl_image, bootdev, mmc_get_blk_desc(mmc),
> +					    partition);
> +		if (!ret)
> +			return ret;
>   	}
>   #ifdef CONFIG_SPL_FS_LOAD_PAYLOAD_NAME
> -	err = spl_load_image_ext(spl_image, bootdev, mmc_get_blk_desc(mmc),
> +	ret = spl_load_image_ext(spl_image, bootdev, mmc_get_blk_desc(mmc),
>   				 partition,
>   				 filename);
> -	if (!err)
> -		return err;
> +	if (!ret)
> +		return ret;
>   #endif
>   #endif
>   
>   #if defined(CONFIG_SPL_FS_FAT) || defined(CONFIG_SPL_FS_EXT4)
> -	err = -ENOENT;
> +	ret = -ENOENT;
>   #endif
>   
> -	return err;
> +	return ret;
>   }
>   #endif
>   
> @@ -342,73 +342,74 @@ int spl_mmc_load(struct spl_image_info *spl_image,
>   		 unsigned long raw_sect)
>   {
>   	u32 boot_mode;
> -	int err = 0;
> +	int ret = 0;
>   	__maybe_unused int part = 0;
>   	int mmc_dev;
>   
>   	/* Perform peripheral init only once for an mmc device */
>   	mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
>   	if (!mmc || spl_mmc_get_mmc_devnum(mmc) != mmc_dev) {
> -		err = spl_mmc_find_device(&mmc, bootdev->boot_device);
> -		if (err)
> -			return err;
> +		ret = spl_mmc_find_device(&mmc, bootdev->boot_device);
> +		if (ret)
> +			return ret;
>   
> -		err = mmc_init(mmc);
> -		if (err) {
> +		ret = mmc_init(mmc);
> +		if (ret) {
>   			mmc = NULL;
> -			printf("spl: mmc init failed with error: %d\n", err);
> -			return err;
> +			printf("spl: mmc init failed with error: %d\n", ret);
> +			return ret;
>   		}
>   	}
>   
>   	boot_mode = spl_mmc_boot_mode(mmc, bootdev->boot_device);
> -	err = -EINVAL;
> +	ret = -EINVAL;
>   	switch (boot_mode) {
>   	case MMCSD_MODE_EMMCBOOT:
>   		part = spl_mmc_emmc_boot_partition(mmc);
>   
>   		if (CONFIG_IS_ENABLED(MMC_TINY))
> -			err = mmc_switch_part(mmc, part);
> +			ret = mmc_switch_part(mmc, part);
>   		else
> -			err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
> +			ret = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
>   
> -		if (err) {
> +		if (ret) {
>   			puts("spl: mmc partition switch failed\n");
> -			return err;
> +			return ret;
>   		}
>   		/* Fall through */
>   	case MMCSD_MODE_RAW:
>   		debug("spl: mmc boot mode: raw\n");
>   
>   		if (!spl_start_uboot()) {
> -			err = mmc_load_image_raw_os(spl_image, bootdev, mmc);
> -			if (!err)
> -				return err;
> +			ret = mmc_load_image_raw_os(spl_image, bootdev, mmc);
> +			if (!ret)
> +				return ret;
>   		}
>   
>   		raw_sect = spl_mmc_get_uboot_raw_sector(mmc, raw_sect);
>   
>   #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
> -		err = mmc_load_image_raw_partition(spl_image, bootdev,
> +		ret = mmc_load_image_raw_partition(spl_image, bootdev,
>   						   mmc, raw_part,
>   						   raw_sect);
> -		if (!err)
> -			return err;
> +		if (!ret)
> +			return ret;
>   #endif
>   #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
> -		err = mmc_load_image_raw_sector(spl_image, bootdev, mmc,
> -				raw_sect + spl_mmc_raw_uboot_offset(part));
> -		if (!err)
> -			return err;
> +		ret = mmc_load_image_raw_sector(spl_image, bootdev, mmc,
> +						raw_sect +
> +						spl_mmc_raw_uboot_offset(part));
> +		if (!ret)
> +			return ret;
>   #endif
>   		/* If RAW mode fails, try FS mode. */
>   #ifdef CONFIG_SYS_MMCSD_FS_BOOT
>   	case MMCSD_MODE_FS:
>   		debug("spl: mmc boot mode: fs\n");
>   
> -		err = spl_mmc_do_fs_boot(spl_image, bootdev, mmc, filename);
> -		if (!err)
> -			return err;
> +		ret = spl_mmc_do_fs_boot(spl_image, bootdev, mmc, filename);
> +		if (!ret)
> +			return ret;
>   
>   		break;
>   #endif
> @@ -416,7 +417,7 @@ int spl_mmc_load(struct spl_image_info *spl_image,
>   		puts("spl: mmc: wrong boot mode\n");
>   	}
>   
> -	return err;
> +	return ret;
>   }
>   
>   int spl_mmc_load_image(struct spl_image_info *spl_image,



More information about the U-Boot mailing list