[RFC PATCH] tools: env: Add an option to have an empty default environment
When building envtools via tools-only_defconfig the builtin defaults are based on options in the defconfig. For example:
bootcmd=bootp; setenv bootargs root=/dev/nfs nfsroot=${serverip}:${rootpath} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; bootm bootdelay=2 baudrate=115200 stdin=serial,cros-ec-keyb,usbkbd stdout=serial,vidconsole stderr=serial,vidconsole ethaddr=00:00:11:22:33:44 eth3addr=00:00:11:22:33:45 eth5addr=00:00:11:22:33:46 eth6addr=00:00:11:22:33:47 ipaddr=1.2.3.4 bootm_size=0x10000000 kernel_addr_r=0x1000000 fdt_addr_r=0xc00000 ramdisk_addr_r=0x2000000 scriptaddr=0x1000 pxefile_addr_r=0x2000
These may or may not be sensible for any particular target. Rather than trying to have a set of defaults that work for every target add a config option to make the default environment completely empty.
Signed-off-by: Chris Packham judge.packham@gmail.com ---
configs/tools-only_defconfig | 1 + env/Kconfig | 7 +++++++ include/env_default.h | 2 ++ 3 files changed, 10 insertions(+)
diff --git a/configs/tools-only_defconfig b/configs/tools-only_defconfig index a853abf2b8fc..0ddd0518ef9a 100644 --- a/configs/tools-only_defconfig +++ b/configs/tools-only_defconfig @@ -12,6 +12,7 @@ CONFIG_BOOTP_DNS2=y CONFIG_OF_CONTROL=y CONFIG_OF_HOSTFILE=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y +CONFIG_EMPTY_DEFAULT_ENV=y CONFIG_BOOTP_SEND_HOSTNAME=y CONFIG_IP_DEFRAG=y # CONFIG_ACPIGEN is not set diff --git a/env/Kconfig b/env/Kconfig index af4d9cbaa4d8..f7860f01cc16 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -614,6 +614,13 @@ config DEFAULT_ENV_FILE containing key=value pairs, blank lines and lines beginning with # are ignored.
+config EMPTY_DEFAULT_ENV + bool "Create an empty default environment" + help + Create an empty default environment. This is intended to be + used when building generic target tools and no sensible + default that can be included. + config ENV_VARS_UBOOT_RUNTIME_CONFIG bool "Add run-time information to the environment" help diff --git a/include/env_default.h b/include/env_default.h index 8a0c3057f0aa..859188a8e7c9 100644 --- a/include/env_default.h +++ b/include/env_default.h @@ -23,6 +23,7 @@ static char default_environment[] = { const uchar default_environment[] = { #endif #ifndef CONFIG_USE_DEFAULT_ENV_FILE +#ifndef CONFIG_EMPTY_DEFAULT_ENV #ifdef CONFIG_ENV_CALLBACK_LIST_DEFAULT ENV_CALLBACK_VAR "=" CONFIG_ENV_CALLBACK_LIST_DEFAULT "\0" #endif @@ -107,6 +108,7 @@ const uchar default_environment[] = { #endif #ifdef CONFIG_EXTRA_ENV_SETTINGS CONFIG_EXTRA_ENV_SETTINGS +#endif #endif "\0" #else /* CONFIG_USE_DEFAULT_ENV_FILE */
On Wed, Aug 12, 2020 at 8:37 PM Chris Packham judge.packham@gmail.com wrote:
When building envtools via tools-only_defconfig the builtin defaults are based on options in the defconfig. For example:
bootcmd=bootp; setenv bootargs root=/dev/nfs nfsroot=${serverip}:${rootpath} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; bootm bootdelay=2 baudrate=115200 stdin=serial,cros-ec-keyb,usbkbd stdout=serial,vidconsole stderr=serial,vidconsole ethaddr=00:00:11:22:33:44 eth3addr=00:00:11:22:33:45 eth5addr=00:00:11:22:33:46 eth6addr=00:00:11:22:33:47 ipaddr=1.2.3.4 bootm_size=0x10000000 kernel_addr_r=0x1000000 fdt_addr_r=0xc00000 ramdisk_addr_r=0x2000000 scriptaddr=0x1000 pxefile_addr_r=0x2000
These may or may not be sensible for any particular target. Rather than trying to have a set of defaults that work for every target add a config option to make the default environment completely empty.
Signed-off-by: Chris Packham judge.packham@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Dear Chris Packham,
In message 20200813013727.8186-1-judge.packham@gmail.com you wrote:
When building envtools via tools-only_defconfig the builtin defaults are based on options in the defconfig. For example:
bootcmd=bootp; setenv bootargs root=/dev/nfs nfsroot=${serverip}:${rootpath} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; bootm bootdelay=2 baudrate=115200 stdin=serial,cros-ec-keyb,usbkbd stdout=serial,vidconsole stderr=serial,vidconsole ethaddr=00:00:11:22:33:44 eth3addr=00:00:11:22:33:45 eth5addr=00:00:11:22:33:46 eth6addr=00:00:11:22:33:47
Having any specific MAC addresses set in the default environment has always been a strict No-Go as it is just begging for trouble.
If this was a real example, that code should be fixed!!
These may or may not be sensible for any particular target. Rather than trying to have a set of defaults that work for every target add a config option to make the default environment completely empty.
An empty default environment is even worse, as you are missing vital settings like baudrate, bootdelay etc. There are good chances to brick your board with such settings.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Tue, 18 Aug 2020, 10:27 PM Wolfgang Denk, wd@denx.de wrote:
Dear Chris Packham,
In message 20200813013727.8186-1-judge.packham@gmail.com you wrote:
When building envtools via tools-only_defconfig the builtin defaults are based on options in the defconfig. For example:
bootcmd=bootp; setenv bootargs root=/dev/nfs
nfsroot=${serverip}:${rootpath} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; bootm
bootdelay=2 baudrate=115200 stdin=serial,cros-ec-keyb,usbkbd stdout=serial,vidconsole stderr=serial,vidconsole ethaddr=00:00:11:22:33:44 eth3addr=00:00:11:22:33:45 eth5addr=00:00:11:22:33:46 eth6addr=00:00:11:22:33:47
Having any specific MAC addresses set in the default environment has always been a strict No-Go as it is just begging for trouble.
If this was a real example, that code should be fixed!!
Yes it's what you get if you build tools-only_defconfig as of 2020.07. There's also various addresses that are bad for the boards I deal with.
These may or may not be sensible for any particular target. Rather than trying to have a set of defaults that work for every target add a config option to make the default environment completely empty.
An empty default environment is even worse, as you are missing vital settings like baudrate, bootdelay etc. There are good chances to brick your board with such settings.
I believe the baudrate has build time default that is suitable if nothing is set. But yes there are things necessary for a board to boot an OS that are undone by a blank environment. But the same also applies to the wrong value. The current defaults render my boards expensive bricks.
What I think might be the best path forward is being able to supply defaults via a file at run time. That way we can have a generic fw_setenv but supply board specific defaults via a config file.
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de The word "fit", as I understand it, means "appropriate to a purpose", and I would say the body of the Dean is supremely appropriate to the purpose of sitting around all day and eating big heavy meals. - Terry Pratchett, _Moving Pictures_
Dear Chris,
In message CAFOYHZBz3gYXH=a9bj1BEnM4ztVgxjDUYzQGCsJy47Exggb0TA@mail.gmail.com you wrote:
What I think might be the best path forward is being able to supply defaults via a file at run time. That way we can have a generic fw_setenv but supply board specific defaults via a config file.
This would indeed make sense (though I would not call this any longer "default" then).
Patches welcome!
Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
And please - stop sending HTML to the list! Thanks.
Best regards,
Wolfgang Denk
Hi Chris,
On 13.08.20 03:37, Chris Packham wrote:
When building envtools via tools-only_defconfig the builtin defaults are based on options in the defconfig. For example:
bootcmd=bootp; setenv bootargs root=/dev/nfs nfsroot=${serverip}:${rootpath} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; bootm bootdelay=2 baudrate=115200 stdin=serial,cros-ec-keyb,usbkbd stdout=serial,vidconsole stderr=serial,vidconsole ethaddr=00:00:11:22:33:44 eth3addr=00:00:11:22:33:45 eth5addr=00:00:11:22:33:46 eth6addr=00:00:11:22:33:47 ipaddr=1.2.3.4 bootm_size=0x10000000 kernel_addr_r=0x1000000 fdt_addr_r=0xc00000 ramdisk_addr_r=0x2000000 scriptaddr=0x1000 pxefile_addr_r=0x2000
These may or may not be sensible for any particular target. Rather than trying to have a set of defaults that work for every target add a config option to make the default environment completely empty.
Signed-off-by: Chris Packham judge.packham@gmail.com
Which is the added value compared to use CONFIG_USE_DEFAULT_ENV_FILE=y and adding your file with no variables ?
Best regards, Stefano
configs/tools-only_defconfig | 1 + env/Kconfig | 7 +++++++ include/env_default.h | 2 ++ 3 files changed, 10 insertions(+)
diff --git a/configs/tools-only_defconfig b/configs/tools-only_defconfig index a853abf2b8fc..0ddd0518ef9a 100644 --- a/configs/tools-only_defconfig +++ b/configs/tools-only_defconfig @@ -12,6 +12,7 @@ CONFIG_BOOTP_DNS2=y CONFIG_OF_CONTROL=y CONFIG_OF_HOSTFILE=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y +CONFIG_EMPTY_DEFAULT_ENV=y CONFIG_BOOTP_SEND_HOSTNAME=y CONFIG_IP_DEFRAG=y # CONFIG_ACPIGEN is not set diff --git a/env/Kconfig b/env/Kconfig index af4d9cbaa4d8..f7860f01cc16 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -614,6 +614,13 @@ config DEFAULT_ENV_FILE containing key=value pairs, blank lines and lines beginning with # are ignored.
+config EMPTY_DEFAULT_ENV
- bool "Create an empty default environment"
- help
Create an empty default environment. This is intended to beused when building generic target tools and no sensibledefault that can be included.config ENV_VARS_UBOOT_RUNTIME_CONFIG bool "Add run-time information to the environment" help diff --git a/include/env_default.h b/include/env_default.h index 8a0c3057f0aa..859188a8e7c9 100644 --- a/include/env_default.h +++ b/include/env_default.h @@ -23,6 +23,7 @@ static char default_environment[] = { const uchar default_environment[] = { #endif #ifndef CONFIG_USE_DEFAULT_ENV_FILE +#ifndef CONFIG_EMPTY_DEFAULT_ENV #ifdef CONFIG_ENV_CALLBACK_LIST_DEFAULT ENV_CALLBACK_VAR "=" CONFIG_ENV_CALLBACK_LIST_DEFAULT "\0" #endif @@ -107,6 +108,7 @@ const uchar default_environment[] = { #endif #ifdef CONFIG_EXTRA_ENV_SETTINGS CONFIG_EXTRA_ENV_SETTINGS +#endif #endif "\0" #else /* CONFIG_USE_DEFAULT_ENV_FILE */
On Tue, 18 Aug 2020, 11:02 PM Stefano Babic, sbabic@denx.de wrote:
Hi Chris,
On 13.08.20 03:37, Chris Packham wrote:
When building envtools via tools-only_defconfig the builtin defaults are based on options in the defconfig. For example:
bootcmd=bootp; setenv bootargs root=/dev/nfs
nfsroot=${serverip}:${rootpath} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; bootm
bootdelay=2 baudrate=115200 stdin=serial,cros-ec-keyb,usbkbd stdout=serial,vidconsole stderr=serial,vidconsole ethaddr=00:00:11:22:33:44 eth3addr=00:00:11:22:33:45 eth5addr=00:00:11:22:33:46 eth6addr=00:00:11:22:33:47 ipaddr=1.2.3.4 bootm_size=0x10000000 kernel_addr_r=0x1000000 fdt_addr_r=0xc00000 ramdisk_addr_r=0x2000000 scriptaddr=0x1000 pxefile_addr_r=0x2000
These may or may not be sensible for any particular target. Rather than trying to have a set of defaults that work for every target add a config option to make the default environment completely empty.
Signed-off-by: Chris Packham judge.packham@gmail.com
Which is the added value compared to use CONFIG_USE_DEFAULT_ENV_FILE=y and adding your file with no variables ?
Long story short that was what I tried first but I was having trouble making it work with tools-only_defconfig. But I think the point remains, there's just no sane set of defaults that can be built-in to a generic fw_setenv tool.
Best regards, Stefano
configs/tools-only_defconfig | 1 + env/Kconfig | 7 +++++++ include/env_default.h | 2 ++ 3 files changed, 10 insertions(+)
diff --git a/configs/tools-only_defconfig b/configs/tools-only_defconfig index a853abf2b8fc..0ddd0518ef9a 100644 --- a/configs/tools-only_defconfig +++ b/configs/tools-only_defconfig @@ -12,6 +12,7 @@ CONFIG_BOOTP_DNS2=y CONFIG_OF_CONTROL=y CONFIG_OF_HOSTFILE=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y +CONFIG_EMPTY_DEFAULT_ENV=y CONFIG_BOOTP_SEND_HOSTNAME=y CONFIG_IP_DEFRAG=y # CONFIG_ACPIGEN is not set diff --git a/env/Kconfig b/env/Kconfig index af4d9cbaa4d8..f7860f01cc16 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -614,6 +614,13 @@ config DEFAULT_ENV_FILE containing key=value pairs, blank lines and lines beginning with # are ignored.
+config EMPTY_DEFAULT_ENV
bool "Create an empty default environment"helpCreate an empty default environment. This is intended to beused when building generic target tools and no sensibledefault that can be included.config ENV_VARS_UBOOT_RUNTIME_CONFIG bool "Add run-time information to the environment" help diff --git a/include/env_default.h b/include/env_default.h index 8a0c3057f0aa..859188a8e7c9 100644 --- a/include/env_default.h +++ b/include/env_default.h @@ -23,6 +23,7 @@ static char default_environment[] = { const uchar default_environment[] = { #endif #ifndef CONFIG_USE_DEFAULT_ENV_FILE +#ifndef CONFIG_EMPTY_DEFAULT_ENV #ifdef CONFIG_ENV_CALLBACK_LIST_DEFAULT ENV_CALLBACK_VAR "=" CONFIG_ENV_CALLBACK_LIST_DEFAULT "\0" #endif @@ -107,6 +108,7 @@ const uchar default_environment[] = { #endif #ifdef CONFIG_EXTRA_ENV_SETTINGS CONFIG_EXTRA_ENV_SETTINGS +#endif #endif "\0" #else /* CONFIG_USE_DEFAULT_ENV_FILE */
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de =====================================================================
participants (4)
-
Chris Packham -
Joe Hershberger -
Stefano Babic -
Wolfgang Denk