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

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@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,