[PATCH] env: mmc: Use correct eMMC HW partition size when calculating end of HW partition

[Marek Vasut]

On 5/4/26 2:22 PM, Simon Glass wrote:

Hello Simon,

>> diff --git a/env/mmc.c b/env/mmc.c
>> @@ -239,8 +239,12 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr)
>>                return -ENOENT;
>>        }
>>
>> -     if (offset < 0)
>> -             offset += mmc->capacity;
>> +     if (offset < 0) {
>> +             if (CONFIG_ENV_MMC_EMMC_HW_PARTITION)
>> +                     offset += mmc->capacity_boot;
>> +             else
>> +                     offset += mmc->capacity;
>> +     }
> 
> Just to clarify the motivation: every caller of mmc_get_env_addr()
> goes through init_mmc_for_env(), which calls mmc_set_env_part_init()
> -> blk_select_hwpart_devnum() -> mmc_switch_part() ->
> mmc_set_capacity(). After that switch mmc->capacity should already
> equal mmc->capacity_boot when CONFIG_ENV_MMC_EMMC_HW_PARTITION is 1 or
> 2. Can you describe the path where mmc->capacity is still
> capacity_user here? Perhaps the fix belongs instead in the
> partition-switch path?

The mmc_get_env_addr() is a public function, you can not predict the 
caller, so it should return the correct offset no matter which eMMC HW 
partition is selected.

>> diff --git a/env/mmc.c b/env/mmc.c
>> @@ -239,8 +239,12 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr)
>> +             if (CONFIG_ENV_MMC_EMMC_HW_PARTITION)
>> +                     offset += mmc->capacity_boot;
>> +             else
>> +                     offset += mmc->capacity;
> 
> This seems to do the wrong thing for
> CONFIG_ENV_MMC_EMMC_HW_PARTITION=3 (RPMB)

I don't think we support env in eMMC RPMB . Also, 
CONFIG_ENV_MMC_EMMC_HW_PARTITION=3 would be GP0 , not RPMB, which we 
also do not support.

> - it picks capacity_boot
> when capacity_rpmb is meant. The Kconfig is a plain int with no upper
> bound, so this is reachable. If you want to keep the explicit
> dispatch, you could key it off mmc_get_blk_desc(mmc)->hwpart so all
> partition kinds (user/boot/rpmb) fall out correctly. What do you
> think?

I could special-case it this way:

"
         if (offset < 0) {
-               if (CONFIG_ENV_MMC_EMMC_HW_PARTITION)
+               if (CONFIG_ENV_MMC_EMMC_HW_PARTITION == 1 ||
+                   CONFIG_ENV_MMC_EMMC_HW_PARTITION == 2) {
                         offset += mmc->capacity_boot;
-               else
+               } else if (CONFIG_ENV_MMC_EMMC_HW_PARTITION >= 3 ||
+                          CONFIG_ENV_MMC_EMMC_HW_PARTITION <= 6) {
+                       offset += 
mmc->capacity_gp[CONFIG_ENV_MMC_EMMC_HW_PARTITION - 3];
+               } else {
                         offset += mmc->capacity;
+               }
         }
"
[...]