[PATCH v2 7/7] smegw01: Enable EMMC boot from multiple partitions

[Pali Rohár]

On Friday 21 April 2023 07:56:50 Fabio Estevam wrote:
> +uint mmc_get_env_part(struct mmc *mmc)
> +{
> +	uint part;
> +
> +	if (mmc->part_config == MMCPART_NOAVAILABLE) {
> +		part = 0;
> +	} else {
> +		switch (EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config)) {
> +		case 0: /* Booting from this eMMC device is disabled */
> +			printf("Error - Booting from this eMMC device is disabled\n");
> +			printf("Hint: Use 'mmc partconf' command to choose boot partition\n");
> +			return -ENODEV;

This does not look to be correct. Return type of the function is uint
but here you are trying to return negative number.

I think that there is some layering or API issue. Caller of this
function probably does not expect any failure and use returned value as
partition number. But you are reading partition number from the source
which may return failure (as the source does not have to contain it).

I'm not sure what is the correct way how to handle these kind of error.
I hope that this is something which you would know.

Which partition want to choose? Same from which was u-boot/spl loaded at
runtime? If yes then maybe this my patch series for mvebu may be
interested for you:
https://lore.kernel.org/u-boot/20230413205750.10641-1-pali@kernel.org/t/#u

I added there ability to store emmc partition from which was bootloader
loaded into access bits of mmc->part_config variable and then via macro
EXT_CSD_EXTRACT_PARTITION_ACCESS() it can be extracted. See function
spl_mmc_emmc_boot_partition() in that mvebu patch series.

But I'm not sure how reliable access bits of mmc->part_config are on
other platforms. As currently only mvebu in that patch series is doing
to use it.

> +		case 1: /* Boot partition 1 is used for booting */
> +			part = 1;
> +			break;
> +		case 2: /* Boot partition 2 is used for booting */
> +			part = 2;
> +			break;
> +		case 7: /* User area is used for booting */
> +			part = 0;
> +			break;
> +		default: /* Other values are reserved / unsupported */
> +			printf("Error - This eMMC device has configured Reserved boot option\n");
> +			printf("Hint: Use 'mmc partconf' command to choose boot partition\n");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	return part;
> +}