Re: [PATCH v3 3/5] env: scsi: Add support for partition type GUID based environment

[David Lechner <dlechner@baylibre.com>]

On 4/19/26 5:24 AM, Balaji Selvanathan wrote:
> Add support for locating SCSI environment partition using GPT type
> GUID instead of unique UUID. This enables the saveenv command to
> work with partitions identified by their type rather than unique
> identifiers, providing flexibility for systems where partition
> UUIDs may vary across devices but types remain constant.
> 
> Introduce a Kconfig choice statement to select between three partition
> lookup methods. The choice provides mutually exclusive options:
> ENV_SCSI_PART_USE_UUID (default), ENV_SCSI_PART_USE_TYPE_GUID, and
> ENV_SCSI_PART_USE_HW. The corresponding string configs depend on their
> respective selection method, creating a clear configuration structure.
> 
> Introduce CONFIG_ENV_SCSI_PART_TYPE_GUID configuration option that
> allows specifying a partition type GUID for environment storage.
> When SCSI_ENV_PART_USE_TYPE_GUID is enabled, the environment subsystem
> uses the type GUID based lookup method via
> scsi_get_blk_by_type_guid() to find the first matching partition.
> 
> Refactor env/scsi.c to use compile-time preprocessor conditionals
> instead of runtime string checks. Replace the previous logic that
> checked if CONFIG_ENV_SCSI_PART_UUID was empty with explicit #if/#elif
> branches for each method.

Might be a bit cleaner to split the refactor and adding the new option
into two patches. This is quite a bit going on in one patch.

> 
> Elevate hardware partition selection from an implicit fallback to an
> explicit choice (ENV_SCSI_PART_USE_HW), improving configuration clarity.
> Move ENV_SCSI_HW_PARTITION to depend on this new option.
> 
> Signed-off-by: Balaji Selvanathan <balaji.selvanathan@oss.qualcomm.com>
> ---
> Changes in v3:
> - Introduce a new choice config: ENV_SCSI_PART_USE_HW for
>   ENV_SCSI_HW_PARTITION
> - Refactor env_scsi_get_part and env_scsi_load functions based
>   on the choice configs
> 
> Changes in v2:
> - Introduce a Kconfig choice config to select between UUID-based
> and type GUID-based partition lookup methods.
> ---
>  env/Kconfig | 51 +++++++++++++++++++++++++++++++++++++++++++--------
>  env/scsi.c  | 45 +++++++++++++++++++++++++++------------------
>  2 files changed, 70 insertions(+), 26 deletions(-)
> 
> diff --git a/env/Kconfig b/env/Kconfig
> index 7abd82ab6f3..d5d956cb4ce 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -780,10 +780,51 @@ config ENV_MMC_USE_DT
>  	  The 2 defines CONFIG_ENV_OFFSET, CONFIG_ENV_OFFSET_REDUND
>  	  are not used as fallback.
>  

We should update the help of `config ENV_IS_IN_SCSI` as well since it
mentions these options below.

> +choice
> +	prompt "SCSI partition selection method"
> +	depends on ENV_IS_IN_SCSI
> +	default ENV_SCSI_PART_USE_UUID
> +	help
> +	  Select the method to identify the SCSI partition for environment storage.
> +

I wish I would have thought of using `choice` in the recent changes I
did. It is much cleaner to explicitly select which one you want. :-)

> +config ENV_SCSI_PART_USE_UUID
> +	bool "Use partition UUID"
> +	help
> +	  Use the partition's unique UUID to identify the SCSI partition
> +	  for environment storage.
> +
> +config ENV_SCSI_PART_USE_TYPE_GUID
> +	bool "Use partition type GUID"
> +	help
> +	  Use the partition type GUID to identify the SCSI partition
> +	  for environment storage. The first partition matching the
> +	  specified type GUID will be used.
> +
> +config ENV_SCSI_PART_USE_HW
> +	bool "Use hardware partition number"
> +	help
> +	  Use the hardware device number to identify the SCSI device
> +	  for environment storage.
> +
> +endchoice
> +
> +config ENV_SCSI_PART_UUID
> +	string "SCSI partition UUID for saving environment"
> +	depends on ENV_SCSI_PART_USE_UUID
> +	help
> +	  UUID of the SCSI partition that you want to store the environment in.
> +
> +config ENV_SCSI_PART_TYPE_GUID
> +	string "SCSI partition type GUID for saving environment"
> +	depends on ENV_SCSI_PART_USE_TYPE_GUID
> +	help
> +	  Type GUID of the SCSI partition to store the environment in.
> +	  Uses the first partition matching this type GUID.
> +
>  config ENV_SCSI_HW_PARTITION
>  	string "SCSI hardware partition number"
> -	depends on ENV_IS_IN_SCSI
> -	default 0
> +	depends on ENV_SCSI_PART_USE_HW
> +	default "0"

Fixing the quotes here is technically an unrelated change, so could be
a patch on it's own as well.

>  	help
>  	  SCSI hardware partition device number on the platform where the
>  	  environment is stored.  Note that this is not related to any software
> @@ -791,12 +832,6 @@ config ENV_SCSI_HW_PARTITION
>  	  partition 0 or the first boot partition, which is 1 or some other defined
>  	  partition.
>  
> -config ENV_SCSI_PART_UUID
> -	string "SCSI partition UUID for saving environment"
> -	depends on ENV_IS_IN_SCSI
> -	help
> -	  UUID of the SCSI partition that you want to store the environment in.
> -
>  config ENV_USE_DEFAULT_ENV_TEXT_FILE
>  	bool "Create default environment from file"
>  	depends on !COMPILE_TEST
> diff --git a/env/scsi.c b/env/scsi.c
> index 91a6c430302..abb8b0a1dfd 100644
> --- a/env/scsi.c
> +++ b/env/scsi.c
> @@ -41,14 +41,19 @@ static inline struct env_scsi_info *env_scsi_get_part(void)
>  		is_scsi_scanned = true;
>  	}
>  
> -	if (CONFIG_ENV_SCSI_PART_UUID[0] == '\0') {
> -		if (blk_get_device_part_str("scsi", CONFIG_ENV_SCSI_HW_PARTITION,
> -					    &ep->blk, &ep->part, true))
> -			return NULL;
> -	} else {
> -		if (scsi_get_blk_by_uuid(CONFIG_ENV_SCSI_PART_UUID, &ep->blk, &ep->part))
> -			return NULL;
> -	}
> +#if defined(CONFIG_ENV_SCSI_PART_USE_TYPE_GUID)
> +	if (scsi_get_blk_by_type_guid(CONFIG_ENV_SCSI_PART_TYPE_GUID, &ep->blk, &ep->part))
> +		return NULL;
> +
> +#elif defined(CONFIG_ENV_SCSI_PART_USE_UUID)
> +	if (scsi_get_blk_by_uuid(CONFIG_ENV_SCSI_PART_UUID, &ep->blk, &ep->part))
> +		return NULL;
> +
> +#elif defined(CONFIG_ENV_SCSI_PART_USE_HW)
> +	if (blk_get_device_part_str("scsi", CONFIG_ENV_SCSI_HW_PARTITION,
> +				    &ep->blk, &ep->part, true))
> +		return NULL;
> +#endif
>  
>  	ep->count = CONFIG_ENV_SIZE / ep->part.blksz;
>  
> @@ -95,20 +100,24 @@ static int env_scsi_load(void)
>  	int ret;
>  
>  	if (!ep) {
> -		if (CONFIG_ENV_SCSI_PART_UUID[0] == '\0')
> -			env_set_default("SCSI partition " CONFIG_ENV_SCSI_HW_PARTITION " not found", 0);
> -		else
> -			env_set_default(CONFIG_ENV_SCSI_PART_UUID " partition not found", 0);
> -
> +#if defined(CONFIG_ENV_SCSI_PART_USE_TYPE_GUID)
> +		env_set_default(CONFIG_ENV_SCSI_PART_TYPE_GUID " partition not found", 0);

It could be a bit more clear to have a different message for
this one. e.g. "partition type " CONFIG_ENV_SCSI_PART_TYPE_GUID " not found".

> +#elif defined(CONFIG_ENV_SCSI_PART_USE_UUID)
> +		env_set_default(CONFIG_ENV_SCSI_PART_UUID " partition not found", 0);
> +#elif defined(CONFIG_ENV_SCSI_PART_USE_HW)
> +		env_set_default("SCSI partition " CONFIG_ENV_SCSI_HW_PARTITION " not found", 0);
> +#endif
>  		return -ENOENT;
>  	}
>  
>  	if (blk_dread(ep->blk, ep->part.start, ep->count, &envbuf) != ep->count) {
> -		if (CONFIG_ENV_SCSI_PART_UUID[0] == '\0')
> -			env_set_default("SCSI partition " CONFIG_ENV_SCSI_HW_PARTITION " read failed", 0);
> -		else
> -			env_set_default(CONFIG_ENV_SCSI_PART_UUID " partition read failed", 0);
> -
> +#if defined(CONFIG_ENV_SCSI_PART_USE_TYPE_GUID)
> +		env_set_default(CONFIG_ENV_SCSI_PART_TYPE_GUID " partition read failed", 0);

similar as above

> +#elif defined(CONFIG_ENV_SCSI_PART_USE_UUID)
> +		env_set_default(CONFIG_ENV_SCSI_PART_UUID " partition read failed", 0);
> +#elif defined(CONFIG_ENV_SCSI_PART_USE_HW)
> +		env_set_default("SCSI partition " CONFIG_ENV_SCSI_HW_PARTITION " read failed", 0);
> +#endif
>  		return -EIO;
>  	}
>  
>