* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.