Re: [PATCH v2 1/2] env: mmc: Clean up macro usage

[Marek Vasut <marex@denx.de>]

On 2/23/23 17:32, Tom Rini wrote:
> On Thu, Feb 23, 2023 at 02:22:51PM +0100, Marek Vasut wrote:
>> On 2/23/23 11:41, Patrick DELAUNAY wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> On 2/9/23 13:30, Marek Vasut wrote:
>>>> Consistently use 'if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID))' instead of
>>>> mix of ifdef.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> ---
>>>> Cc: Patrice Chotard <patrice.chotard@foss.st.com>
>>>> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
>>>> Cc: Tom Rini <trini@konsulko.com>
>>>> ---
>>>> V2: Replace CONFIG_IS_ENABLED(PARTITION_TYPE_GUID) with
>>>> IS_ENABLED(CONFIG_PARTITION_TYPE_GUID)
>>>> ---
>>>>    env/mmc.c | 4 +---
>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/env/mmc.c b/env/mmc.c
>>>> index 5b01f657a7a..d51a5579128 100644
>>>> --- a/env/mmc.c
>>>> +++ b/env/mmc.c
>>>> @@ -73,8 +73,7 @@ static inline int mmc_offset_try_partition(const
>>>> char *str, int copy, s64 *val)
>>>>            if (str && !strncmp((const char *)info.name, str,
>>>> sizeof(info.name)))
>>>>                break;
>>>> -#ifdef CONFIG_PARTITION_TYPE_GUID
>>>> -        if (!str) {
>>>> +        if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID) && !str) {
>>>>                const efi_guid_t env_guid = PARTITION_U_BOOT_ENVIRONMENT;
>>>>                efi_guid_t type_guid;
>>>> @@ -82,7 +81,6 @@ static inline int mmc_offset_try_partition(const
>>>> char *str, int copy, s64 *val)
>>>>                if (!memcmp(&env_guid, &type_guid, sizeof(efi_guid_t)))
>>>>                    break;
>>>>            }
>>>> -#endif
>>>>        }
>>>>        /* round up to info.blksz */
>>>
>>>
>>> If I remenber, I try this test with IS_ENABLED when I propose my patch
>>>
>>> and I have compilation issue on next line
>>>
>>>
>>> +            uuid_str_to_bin(info.type_guid, type_guid.b,
>>> UUID_STR_FORMAT_GUID);
>>>
>>>
>>> because "info.type_guid" don't exist in struct disk_partition
>>>
>>> see ./include/part.h:59
>>>
>>>
>>> struct disk_partition {
>>>       lbaint_t    start;    /* # of first block in partition    */
>>> ...
>>> #ifdef CONFIG_PARTITION_TYPE_GUID
>>>       char    type_guid[UUID_STR_LEN + 1];    /* type GUID as string, if
>>> exists    */
>>> #endif
>>> ...
>>> };
>>
>> Uh, which defconfig triggers this ?
>>
>> Also, is there a way to deal with this failure without reinstating the ifdef
>> ? Tom ?
> 
> It's likely on one of the platforms that disables EFI_LOADER, where this
> ends up being a fail to build.  I don't recall which, but I've seen it
> before. So we probably don't end up converting this to a macro check.

Can you pick the 2/2 for starters ? It is a bugfix and applies cleanly, 
so let's not block that one. I'll keep 1/2 in my queue and revisit it, I 
see the failure in CI now.