* [PATCH v2 2/2] env: mmc: Apply GPT only on eMMC user HW partition
2023-02-09 12:30 ` [PATCH v2 1/2] env: mmc: Clean up macro usage Marek Vasut
@ 2023-02-09 12:30 ` Marek Vasut
2023-02-09 15:27 ` Tom Rini
2023-02-24 14:43 ` Tom Rini
2023-02-09 15:27 ` [PATCH v2 1/2] env: mmc: Clean up macro usage Tom Rini
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Marek Vasut @ 2023-02-09 12:30 UTC (permalink / raw)
To: u-boot; +Cc: Marek Vasut, Patrice Chotard, Patrick Delaunay, Tom Rini
Apply the GPT U-Boot environment GUID type look up only on eMMC user
HW partition, do not apply the look up on eMMC boot HW partitions as
mmc_offset_try_partition() assumes either SD partitions or eMMC user
HW partition.
This fixes environment operation on systems where CONFIG_SYS_MMC_ENV_PART
is non-zero and CONFIG_SYS_REDUNDAND_ENVIRONMENT is set.
Fixes: 80105d8fd52 ("env: mmc: select GPT env partition by type guid")
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: Rebase on changes in 1/2
---
env/mmc.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/env/mmc.c b/env/mmc.c
index d51a5579128..88f8a9a8978 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -92,7 +92,7 @@ static inline int mmc_offset_try_partition(const char *str, int copy, s64 *val)
return 0;
}
-static inline s64 mmc_offset(int copy)
+static inline s64 mmc_offset(struct mmc *mmc, int copy)
{
const struct {
const char *offset_redund;
@@ -106,8 +106,12 @@ static inline s64 mmc_offset(int copy)
s64 val = 0, defvalue;
const char *propname;
const char *str;
+ int hwpart = 0;
int err;
+ if (IS_ENABLED(CONFIG_SYS_MMC_ENV_PART))
+ hwpart = mmc_get_env_part(mmc);
+
/* look for the partition in mmc CONFIG_SYS_MMC_ENV_DEV */
str = ofnode_conf_read_str(dt_prop.partition);
if (str) {
@@ -119,7 +123,7 @@ static inline s64 mmc_offset(int copy)
}
/* try the GPT partition with "U-Boot ENV" TYPE GUID */
- if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID)) {
+ if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID) && hwpart == 0) {
err = mmc_offset_try_partition(NULL, copy, &val);
if (!err)
return val;
@@ -136,7 +140,7 @@ static inline s64 mmc_offset(int copy)
return ofnode_conf_read_int(propname, defvalue);
}
#else
-static inline s64 mmc_offset(int copy)
+static inline s64 mmc_offset(struct mmc *mmc, int copy)
{
s64 offset = ENV_MMC_OFFSET;
@@ -149,7 +153,7 @@ static inline s64 mmc_offset(int copy)
__weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr)
{
- s64 offset = mmc_offset(copy);
+ s64 offset = mmc_offset(mmc, copy);
if (offset == ENV_MMC_INVALID_OFFSET) {
printf("Invalid ENV offset in MMC, copy=%d\n", copy);
--
2.39.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/2] env: mmc: Apply GPT only on eMMC user HW partition
2023-02-09 12:30 ` [PATCH v2 2/2] env: mmc: Apply GPT only on eMMC user HW partition Marek Vasut
@ 2023-02-09 15:27 ` Tom Rini
2023-02-09 16:33 ` Marek Vasut
2023-02-24 14:43 ` Tom Rini
1 sibling, 1 reply; 13+ messages in thread
From: Tom Rini @ 2023-02-09 15:27 UTC (permalink / raw)
To: Marek Vasut; +Cc: u-boot, Patrice Chotard, Patrick Delaunay
[-- Attachment #1: Type: text/plain, Size: 897 bytes --]
On Thu, Feb 09, 2023 at 01:30:10PM +0100, Marek Vasut wrote:
> Apply the GPT U-Boot environment GUID type look up only on eMMC user
> HW partition, do not apply the look up on eMMC boot HW partitions as
> mmc_offset_try_partition() assumes either SD partitions or eMMC user
> HW partition.
>
> This fixes environment operation on systems where CONFIG_SYS_MMC_ENV_PART
> is non-zero and CONFIG_SYS_REDUNDAND_ENVIRONMENT is set.
>
> Fixes: 80105d8fd52 ("env: mmc: select GPT env partition by type guid")
> Signed-off-by: Marek Vasut <marex@denx.de>
So you're saying that today, you cannot make use of this feature and put
the environment on one of the boot partitions, as it doesn't work? If
so, then yes, this is the first step forward and someone else with that
wants the case I described here can fix things further.
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/2] env: mmc: Apply GPT only on eMMC user HW partition
2023-02-09 15:27 ` Tom Rini
@ 2023-02-09 16:33 ` Marek Vasut
0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2023-02-09 16:33 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot, Patrice Chotard, Patrick Delaunay
On 2/9/23 16:27, Tom Rini wrote:
> On Thu, Feb 09, 2023 at 01:30:10PM +0100, Marek Vasut wrote:
>
>> Apply the GPT U-Boot environment GUID type look up only on eMMC user
>> HW partition, do not apply the look up on eMMC boot HW partitions as
>> mmc_offset_try_partition() assumes either SD partitions or eMMC user
>> HW partition.
>>
>> This fixes environment operation on systems where CONFIG_SYS_MMC_ENV_PART
>> is non-zero and CONFIG_SYS_REDUNDAND_ENVIRONMENT is set.
>>
>> Fixes: 80105d8fd52 ("env: mmc: select GPT env partition by type guid")
>> Signed-off-by: Marek Vasut <marex@denx.de>
>
> So you're saying that today, you cannot make use of this feature and put
> the environment on one of the boot partitions, as it doesn't work?
Today you cannot place env into GPT which is located in eMMC BOOT HW
partitions, that only works for GPT which is located in eMMC USER HW
partition.
> If
> so, then yes, this is the first step forward and someone else with that
> wants the case I described here can fix things further.
I'm not even sure whether it makes sense to have 4 MiB or so GPT in eMMC
BOOT HW partitions in the first place, but sure, if someone needs that
at some later point, why not.
> Reviewed-by: Tom Rini <trini@konsulko.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] env: mmc: Apply GPT only on eMMC user HW partition
2023-02-09 12:30 ` [PATCH v2 2/2] env: mmc: Apply GPT only on eMMC user HW partition Marek Vasut
2023-02-09 15:27 ` Tom Rini
@ 2023-02-24 14:43 ` Tom Rini
1 sibling, 0 replies; 13+ messages in thread
From: Tom Rini @ 2023-02-24 14:43 UTC (permalink / raw)
To: Marek Vasut; +Cc: u-boot, Patrice Chotard, Patrick Delaunay
[-- Attachment #1: Type: text/plain, Size: 657 bytes --]
On Thu, Feb 09, 2023 at 01:30:10PM +0100, Marek Vasut wrote:
> Apply the GPT U-Boot environment GUID type look up only on eMMC user
> HW partition, do not apply the look up on eMMC boot HW partitions as
> mmc_offset_try_partition() assumes either SD partitions or eMMC user
> HW partition.
>
> This fixes environment operation on systems where CONFIG_SYS_MMC_ENV_PART
> is non-zero and CONFIG_SYS_REDUNDAND_ENVIRONMENT is set.
>
> Fixes: 80105d8fd52 ("env: mmc: select GPT env partition by type guid")
> Signed-off-by: Marek Vasut <marex@denx.de>
> Reviewed-by: Tom Rini <trini@konsulko.com>
Applied to u-boot/master, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] env: mmc: Clean up macro usage
2023-02-09 12:30 ` [PATCH v2 1/2] env: mmc: Clean up macro usage Marek Vasut
2023-02-09 12:30 ` [PATCH v2 2/2] env: mmc: Apply GPT only on eMMC user HW partition Marek Vasut
@ 2023-02-09 15:27 ` Tom Rini
2023-02-09 20:13 ` Simon Glass
2023-02-10 0:01 ` Jaehoon Chung
2023-02-23 10:41 ` Patrick DELAUNAY
3 siblings, 1 reply; 13+ messages in thread
From: Tom Rini @ 2023-02-09 15:27 UTC (permalink / raw)
To: Marek Vasut; +Cc: u-boot, Patrice Chotard, Patrick Delaunay
[-- Attachment #1: Type: text/plain, Size: 266 bytes --]
On Thu, Feb 09, 2023 at 01:30:09PM +0100, 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>
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 1/2] env: mmc: Clean up macro usage
2023-02-09 12:30 ` [PATCH v2 1/2] env: mmc: Clean up macro usage Marek Vasut
2023-02-09 12:30 ` [PATCH v2 2/2] env: mmc: Apply GPT only on eMMC user HW partition Marek Vasut
2023-02-09 15:27 ` [PATCH v2 1/2] env: mmc: Clean up macro usage Tom Rini
@ 2023-02-10 0:01 ` Jaehoon Chung
2023-02-23 10:41 ` Patrick DELAUNAY
3 siblings, 0 replies; 13+ messages in thread
From: Jaehoon Chung @ 2023-02-10 0:01 UTC (permalink / raw)
To: 'Marek Vasut', u-boot
Cc: 'Patrice Chotard', 'Patrick Delaunay',
'Tom Rini'
> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Marek Vasut
> Sent: Thursday, February 9, 2023 9:30 PM
> To: u-boot@lists.denx.de
> Cc: Marek Vasut <marex@denx.de>; Patrice Chotard <patrice.chotard@foss.st.com>; Patrick Delaunay
> <patrick.delaunay@foss.st.com>; Tom Rini <trini@konsulko.com>
> Subject: [PATCH v2 1/2] env: mmc: Clean up macro usage
>
> Consistently use 'if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID))' instead of
> mix of ifdef.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Best Regards,
Jaehoon Chung
> ---
> 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 */
> --
> 2.39.1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/2] env: mmc: Clean up macro usage
2023-02-09 12:30 ` [PATCH v2 1/2] env: mmc: Clean up macro usage Marek Vasut
` (2 preceding siblings ...)
2023-02-10 0:01 ` Jaehoon Chung
@ 2023-02-23 10:41 ` Patrick DELAUNAY
2023-02-23 13:22 ` Marek Vasut
3 siblings, 1 reply; 13+ messages in thread
From: Patrick DELAUNAY @ 2023-02-23 10:41 UTC (permalink / raw)
To: Marek Vasut, u-boot; +Cc: Patrice Chotard, Tom Rini
Hi Marek,
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
...
};
Regards
Patrick
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/2] env: mmc: Clean up macro usage
2023-02-23 10:41 ` Patrick DELAUNAY
@ 2023-02-23 13:22 ` Marek Vasut
2023-02-23 16:32 ` Tom Rini
2023-02-23 17:17 ` Simon Glass
0 siblings, 2 replies; 13+ messages in thread
From: Marek Vasut @ 2023-02-23 13:22 UTC (permalink / raw)
To: Patrick DELAUNAY, u-boot; +Cc: Patrice Chotard, Tom Rini
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 ?
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/2] env: mmc: Clean up macro usage
2023-02-23 13:22 ` Marek Vasut
@ 2023-02-23 16:32 ` Tom Rini
2023-02-23 18:24 ` Marek Vasut
2023-02-23 17:17 ` Simon Glass
1 sibling, 1 reply; 13+ messages in thread
From: Tom Rini @ 2023-02-23 16:32 UTC (permalink / raw)
To: Marek Vasut; +Cc: Patrick DELAUNAY, u-boot, Patrice Chotard
[-- Attachment #1: Type: text/plain, Size: 2681 bytes --]
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.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/2] env: mmc: Clean up macro usage
2023-02-23 16:32 ` Tom Rini
@ 2023-02-23 18:24 ` Marek Vasut
0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2023-02-23 18:24 UTC (permalink / raw)
To: Tom Rini; +Cc: Patrick DELAUNAY, u-boot, Patrice Chotard
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] env: mmc: Clean up macro usage
2023-02-23 13:22 ` Marek Vasut
2023-02-23 16:32 ` Tom Rini
@ 2023-02-23 17:17 ` Simon Glass
1 sibling, 0 replies; 13+ messages in thread
From: Simon Glass @ 2023-02-23 17:17 UTC (permalink / raw)
To: Marek Vasut; +Cc: Patrick DELAUNAY, u-boot, Patrice Chotard, Tom Rini
Hi,
On Thu, 23 Feb 2023 at 06:23, Marek Vasut <marex@denx.de> 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 ?
We can create static inline accessors as we have done with some things
in global_data.h
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread