Re: [PATCH v3] mmc: allow use of hardware partition names for mmc partconf

[Marek Vasut <marex@denx.de>]

On 5/13/24 10:52 PM, Tim Harvey wrote:
> On Mon, Apr 29, 2024 at 1:51 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 4/29/24 6:48 PM, Tim Harvey wrote:
>>> On Sat, Apr 27, 2024 at 4:20 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 4/27/24 2:11 AM, Tim Harvey wrote:
>>>>
>>>> [...]
>>>>
>>>>> diff --git a/include/mmc.h b/include/mmc.h
>>>>> index 4b8327f1f93b..7243bd761202 100644
>>>>> --- a/include/mmc.h
>>>>> +++ b/include/mmc.h
>>>>> @@ -381,6 +381,21 @@ enum mmc_voltage {
>>>>>     #define MMC_TIMING_MMC_HS200        9
>>>>>     #define MMC_TIMING_MMC_HS400        10
>>>>>
>>>>> +/* emmc hardware partition values */
>>>>> +enum emmc_hwpart {
>>>>> +     EMMC_HWPART_DEFAULT = 0,
>>>>
>>>> One more thing ... eMMC hardware partition 0 and 7 are both referring to
>>>> USER HW partition. Have a look at the git log, there have been patches
>>>> which handled this case in the MMC subsystem from about a year ago.
>>>
>>> Hi Marek,
>>>
>>> I can't find what you are referring to. I assume you are talking about
>>> calling the first name something other than 'user' as technically it's
>>> not (but it gets treated as user).
>>>
>>> Can you find the commit or discussion you are thinking about?
>>
>> It seems this whole thing is much older:
>>
>> 7dbe63bc950b ("SPL: Add CONFIG_SUPPORT_EMMC_BOOT support to
>> CONFIG_SPL_FRAMEWORK")
>>
>> +                * We need to check what the partition is configured to.
>> +                * 1 and 2 match up to boot0 / boot1 and 7 is user data
>> +                * which is the first physical partition (0).
>> +                */
>> +               int part = (mmc->part_config >> 3) & PART_ACCESS_MASK;
>> +
>> +               if (part == 7)
>> +                       part = 0;
> 
> Hi Marek,

Hello Tim,

> Sorry, I haven't been able to work on U-Boot for the past week or so
> and am just getting back to this.

No worries.

> I'm glad you pointed this out as it made me aware that there is a bit
> of a mixing of eMMC PARTITION_CONFIG (Ext CSD 179) fields
> BOOT_PARTITION_ENABLE and PARTITION_ACCESS in U-Boot currently.
> 
> I'm going to add an enumerated type for each and update the various
> places where a 'conversion' as above is done but I'm still a bit torn
> on naming conventions.
> 
> Consider the following:
> - the eMMC spec refers to boot partitions and gp partitions as 1 based
> - the Linux kernel device names for these are 0 based
> - U-Boot does not currently refer to the boot devices by names but
> does currently refer to the gp's by names using 1-based names (the
> 'mmc hwpartition' command)
> 
> Personally I would like to name the boot partitions 'boot0' and
> 'boot1' to match Linux but I think I should name the gp's
> 'gp1'...'gp4' to be backward compatible with the 'mmc hwpartition'
> command.
> 
> What do you think?

I agree.

Maybe you could also check this with Ulf (Linux MMC maintainer) and Avri 
(mmc-utils), both on To:, so we would be consistent and in sync ?