* [Buildroot] [PATCH] boot/uboot: fix Kconfig warning for DEFAULT_ENV_FILE
@ 2024-07-18 15:45 Brandon Maier via buildroot
2024-07-18 17:13 ` Luca Ceresoli via buildroot
0 siblings, 1 reply; 5+ messages in thread
From: Brandon Maier via buildroot @ 2024-07-18 15:45 UTC (permalink / raw)
To: buildroot; +Cc: Luca Ceresoli, Brandon Maier
The following warning occurs from commit "boot/uboot: allow taking the
entire default environment from a text file".
boot/uboot/Config.in:141:warning: config symbol
'BR2_TARGET_UBOOT_DEFAULT_ENV_FILE' uses select, but is not bool or
tristate
Add a 'bool' config so we can enable dependencies.
In addition, the DEFAULT_ENV_FILE works by setting Kconfig options,
therefore we also need to depend on
BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG.
Signed-off-by: Brandon Maier <brandon.maier@collins.com>
---
boot/uboot/Config.in | 10 +++++++++-
boot/uboot/uboot.mk | 2 +-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
index 57aac06e20..92e7bcc20a 100644
--- a/boot/uboot/Config.in
+++ b/boot/uboot/Config.in
@@ -136,9 +136,15 @@ config BR2_TARGET_UBOOT_CONFIG_FRAGMENT_FILES
that will be merged to the main U-Boot configuration file.
endif
+config BR2_TARGET_UBOOT_DEFAULT_ENV
+ bool "Use default environment file"
+ depends on BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG
+ select BR2_TARGET_UBOOT_NEEDS_XXD
+
+if BR2_TARGET_UBOOT_DEFAULT_ENV
+
config BR2_TARGET_UBOOT_DEFAULT_ENV_FILE
string "Text file with default environment"
- select BR2_TARGET_UBOOT_NEEDS_XXD
help
Text file containing the variables to be used as the default
environment in U-Boot.
@@ -165,6 +171,8 @@ config BR2_TARGET_UBOOT_DEFAULT_ENV_FILE
Requires U-Boot >= v2018.05.
+endif
+
config BR2_TARGET_UBOOT_NEEDS_DTC
bool "U-Boot needs dtc"
select BR2_PACKAGE_HOST_DTC
diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
index 6cc87b35d3..df53fb28c1 100644
--- a/boot/uboot/uboot.mk
+++ b/boot/uboot/uboot.mk
@@ -391,8 +391,8 @@ UBOOT_KCONFIG_EDITORS = menuconfig xconfig gconfig nconfig
# time, so use HOSTCC_NOCCACHE.
UBOOT_KCONFIG_OPTS = $(UBOOT_MAKE_OPTS) HOSTCC="$(HOSTCC_NOCCACHE)" HOSTLDFLAGS=""
+ifeq ($(BR2_TARGET_UBOOT_DEFAULT_ENV),y)
UBOOT_DEFAULT_ENV_FILE = $(call qstrip,$(BR2_TARGET_UBOOT_DEFAULT_ENV_FILE))
-ifneq ($(UBOOT_DEFAULT_ENV_FILE),)
define UBOOT_KCONFIG_DEFAULT_ENV_FILE
$(call KCONFIG_SET_OPT,CONFIG_USE_DEFAULT_ENV_FILE,y)
$(call KCONFIG_SET_OPT,CONFIG_DEFAULT_ENV_FILE,"$(shell readlink -f $(UBOOT_DEFAULT_ENV_FILE))")
---
base-commit: c624eee120b6ee0c81e636bd47abe92452610cf7
change-id: 20240718-boot-uboot-env-select-2f98051c7af9
Best regards,
--
Brandon Maier <brandon.maier@collins.com>
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCH] boot/uboot: fix Kconfig warning for DEFAULT_ENV_FILE
2024-07-18 15:45 [Buildroot] [PATCH] boot/uboot: fix Kconfig warning for DEFAULT_ENV_FILE Brandon Maier via buildroot
@ 2024-07-18 17:13 ` Luca Ceresoli via buildroot
2024-07-18 20:02 ` Thomas Petazzoni via buildroot
2024-07-18 20:32 ` Arnout Vandecappelle via buildroot
0 siblings, 2 replies; 5+ messages in thread
From: Luca Ceresoli via buildroot @ 2024-07-18 17:13 UTC (permalink / raw)
To: Brandon Maier; +Cc: buildroot
Hello Brandon, Arnout,
On Thu, 18 Jul 2024 15:45:45 +0000
Brandon Maier <brandon.maier@collins.com> wrote:
> The following warning occurs from commit "boot/uboot: allow taking the
> entire default environment from a text file".
>
> boot/uboot/Config.in:141:warning: config symbol
> 'BR2_TARGET_UBOOT_DEFAULT_ENV_FILE' uses select, but is not bool or
> tristate
Hum, apologies, didn't notice this, at least when working on v2 (see
below).
> Add a 'bool' config so we can enable dependencies.
That's basically what I did in v1 [0], possibly for this same reason,
but I'm afraid I don't remember exactly.
Then in his review Arnout suggested to have only a string option, and I
did not recall any good reason to not do so, and so v2 came without the
bool.
> In addition, the DEFAULT_ENV_FILE works by setting Kconfig options,
> therefore we also need to depend on
> BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG.
Arnout, do you think the issues reported by Brandon can be fixed while
keeping a single string option?
If they can't, I think it's worth resurrecting the help text from the
two options as I had written them initially in [0]. Otherwise one would
have a bool without help text, and woudl have to enable it just to make
the string option (with help text) visible. Kinda chicken-egg.
[0]
https://lore.kernel.org/buildroot/20240617-uboot-default-env-v1-1-9ac88f0e1789@bootlin.com/
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCH] boot/uboot: fix Kconfig warning for DEFAULT_ENV_FILE
2024-07-18 17:13 ` Luca Ceresoli via buildroot
@ 2024-07-18 20:02 ` Thomas Petazzoni via buildroot
2024-07-18 20:32 ` Arnout Vandecappelle via buildroot
1 sibling, 0 replies; 5+ messages in thread
From: Thomas Petazzoni via buildroot @ 2024-07-18 20:02 UTC (permalink / raw)
To: Luca Ceresoli via buildroot; +Cc: Luca Ceresoli, Brandon Maier
Hello,
On Thu, 18 Jul 2024 19:13:55 +0200
Luca Ceresoli via buildroot <buildroot@buildroot.org> wrote:
> > Add a 'bool' config so we can enable dependencies.
>
> That's basically what I did in v1 [0], possibly for this same reason,
> but I'm afraid I don't remember exactly.
>
> Then in his review Arnout suggested to have only a string option, and I
> did not recall any good reason to not do so, and so v2 came without the
> bool.
>
> > In addition, the DEFAULT_ENV_FILE works by setting Kconfig options,
> > therefore we also need to depend on
> > BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG.
>
> Arnout, do you think the issues reported by Brandon can be fixed while
> keeping a single string option?
>
> If they can't, I think it's worth resurrecting the help text from the
> two options as I had written them initially in [0]. Otherwise one would
> have a bool without help text, and woudl have to enable it just to make
> the string option (with help text) visible. Kinda chicken-egg.
Here is something that seems to work:
diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
index 57aac06e20..aff9f01ea4 100644
--- a/boot/uboot/Config.in
+++ b/boot/uboot/Config.in
@@ -136,9 +136,13 @@ config BR2_TARGET_UBOOT_CONFIG_FRAGMENT_FILES
that will be merged to the main U-Boot configuration file.
endif
+config BR2_TARGET_UBOOT_DEFAULT_ENV_FILE_HIDDEN
+ bool
+ default y if BR2_TARGET_UBOOT_DEFAULT_ENV_FILE != ""
+ select BR2_TARGET_UBOOT_NEEDS_XXD
+
config BR2_TARGET_UBOOT_DEFAULT_ENV_FILE
string "Text file with default environment"
- select BR2_TARGET_UBOOT_NEEDS_XXD
help
Text file containing the variables to be used as the default
environment in U-Boot.
Thomas
--
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Buildroot] [PATCH] boot/uboot: fix Kconfig warning for DEFAULT_ENV_FILE
2024-07-18 17:13 ` Luca Ceresoli via buildroot
2024-07-18 20:02 ` Thomas Petazzoni via buildroot
@ 2024-07-18 20:32 ` Arnout Vandecappelle via buildroot
2024-07-19 7:51 ` Luca Ceresoli via buildroot
1 sibling, 1 reply; 5+ messages in thread
From: Arnout Vandecappelle via buildroot @ 2024-07-18 20:32 UTC (permalink / raw)
To: Luca Ceresoli, Brandon Maier; +Cc: buildroot
On 18/07/2024 19:13, Luca Ceresoli wrote:
> Hello Brandon, Arnout,
>
> On Thu, 18 Jul 2024 15:45:45 +0000
> Brandon Maier <brandon.maier@collins.com> wrote:
>
>> The following warning occurs from commit "boot/uboot: allow taking the
>> entire default environment from a text file".
>>
>> boot/uboot/Config.in:141:warning: config symbol
>> 'BR2_TARGET_UBOOT_DEFAULT_ENV_FILE' uses select, but is not bool or
>> tristate
Oh right, Kconfig is too stupid for that. You need a helper config but it can
be blind. Like:
config BR2_TARGET_UBOOT_USE_DEFAULT_ENV_FILE
bool
default y
depends on BR2_TARGET_UBOOT_DEFAULT_ENV_FILE != ""
select BR2_TARGET_UBOOT_NEEDS_XXD
> Hum, apologies, didn't notice this, at least when working on v2 (see
> below).
>
>> Add a 'bool' config so we can enable dependencies.
>
> That's basically what I did in v1 [0], possibly for this same reason,
> but I'm afraid I don't remember exactly.
>
> Then in his review Arnout suggested to have only a string option, and I
> did not recall any good reason to not do so, and so v2 came without the
> bool.
>
>> In addition, the DEFAULT_ENV_FILE works by setting Kconfig options,
>> therefore we also need to depend on
>> BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG.
Oh yes, but that's a simple depends on in the
BR2_TARGET_UBOOT_DEFAULT_ENV_FILE option.
>
> Arnout, do you think the issues reported by Brandon can be fixed while
> keeping a single string option?
Yes I think so. But it needs to be tested so I'd prefer is _someone_ <grin>
would do that.
Regards,
Arnout
> If they can't, I think it's worth resurrecting the help text from the
> two options as I had written them initially in [0]. Otherwise one would
> have a bool without help text, and woudl have to enable it just to make
> the string option (with help text) visible. Kinda chicken-egg.
>
> [0]
> https://lore.kernel.org/buildroot/20240617-uboot-default-env-v1-1-9ac88f0e1789@bootlin.com/
>
> Luca
>
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCH] boot/uboot: fix Kconfig warning for DEFAULT_ENV_FILE
2024-07-18 20:32 ` Arnout Vandecappelle via buildroot
@ 2024-07-19 7:51 ` Luca Ceresoli via buildroot
0 siblings, 0 replies; 5+ messages in thread
From: Luca Ceresoli via buildroot @ 2024-07-19 7:51 UTC (permalink / raw)
To: Arnout Vandecappelle; +Cc: Thomas Petazzoni, Brandon Maier, buildroot
Hi Arnout, Thomas, Brandon,
On Thu, 18 Jul 2024 22:32:21 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:
> On 18/07/2024 19:13, Luca Ceresoli wrote:
> > Hello Brandon, Arnout,
> >
> > On Thu, 18 Jul 2024 15:45:45 +0000
> > Brandon Maier <brandon.maier@collins.com> wrote:
> >
> >> The following warning occurs from commit "boot/uboot: allow taking the
> >> entire default environment from a text file".
> >>
> >> boot/uboot/Config.in:141:warning: config symbol
> >> 'BR2_TARGET_UBOOT_DEFAULT_ENV_FILE' uses select, but is not bool or
> >> tristate
>
> Oh right, Kconfig is too stupid for that. You need a helper config but it can
> be blind. Like:
>
> config BR2_TARGET_UBOOT_USE_DEFAULT_ENV_FILE
> bool
> default y
> depends on BR2_TARGET_UBOOT_DEFAULT_ENV_FILE != ""
> select BR2_TARGET_UBOOT_NEEDS_XXD
Ah, right! And it's similar to what Thomas proposed. Thanks both.
> > Hum, apologies, didn't notice this, at least when working on v2 (see
> > below).
> >
> >> Add a 'bool' config so we can enable dependencies.
> >
> > That's basically what I did in v1 [0], possibly for this same reason,
> > but I'm afraid I don't remember exactly.
> >
> > Then in his review Arnout suggested to have only a string option, and I
> > did not recall any good reason to not do so, and so v2 came without the
> > bool.
> >
> >> In addition, the DEFAULT_ENV_FILE works by setting Kconfig options,
> >> therefore we also need to depend on
> >> BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG.
>
> Oh yes, but that's a simple depends on in the
> BR2_TARGET_UBOOT_DEFAULT_ENV_FILE option.
Indeed it has to be added.
> > Arnout, do you think the issues reported by Brandon can be fixed while
> > keeping a single string option?
>
> Yes I think so. But it needs to be tested so I'd prefer is _someone_ <grin>
> would do that.
Sure, _someone_ will. :) And that will be me, perhaps next week however,
unless Brandon wants to send a patch before I do.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-07-19 7:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-18 15:45 [Buildroot] [PATCH] boot/uboot: fix Kconfig warning for DEFAULT_ENV_FILE Brandon Maier via buildroot
2024-07-18 17:13 ` Luca Ceresoli via buildroot
2024-07-18 20:02 ` Thomas Petazzoni via buildroot
2024-07-18 20:32 ` Arnout Vandecappelle via buildroot
2024-07-19 7:51 ` Luca Ceresoli via buildroot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox