diff mbox series

[v2,1/2] uboot-envtools: add support for multiple config partitions

Message ID 20201213190735.31978-2-bjorn@mork.no
State Superseded
Headers show
Series uboot-envtools: support for multiple config partitions | expand

Commit Message

Bjørn Mork Dec. 13, 2020, 7:07 p.m. UTC
Most (all?) of the realtek devices have two u-boot config partitions
with a different set of variables in each. The U-Boot shell provides
two sets of apps to manipulate these:

 printenv- print environment variables
 printsys- printsys - print system information variables
 saveenv - save environment variables to persistent storage
 savesys - savesys - save system information variables to persistent storage
 setenv  - set environment variables
 setsys  - setsys  - set system information variables

Add support for multiple ubootenv configuration types, allowing
more than one configuration file.

Section names are not suitable for naming the different
configurations since each file can be the result of multiple sections
in case of backup partitions.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 package/boot/uboot-envtools/Makefile          |  1 +
 package/boot/uboot-envtools/files/realtek     |  8 +++-
 .../uboot-envtools/files/uboot-envtools.sh    | 38 ++++++++++++-------
 3 files changed, 32 insertions(+), 15 deletions(-)

Comments

Adrian Schmutzler Dec. 16, 2020, 2:34 p.m. UTC | #1
Hi,

just a taste nitpick:

> --- a/package/boot/uboot-envtools/files/realtek
> +++ b/package/boot/uboot-envtools/files/realtek
> @@ -15,15 +15,21 @@ zyxel,gs1900-10hp)
>  	idx="$(find_mtd_index u-boot-env)"
>  	[ -n "$idx" ] && \
>  		ubootenv_add_uci_config "/dev/mtd$idx" "0x0" "0x400"
> "0x10000"
> +	idx="$(find_mtd_index u-boot-env2)"
> +	[ -n "$idx" ] && \
> +		ubootenv_add_uci_sys_config "/dev/mtd$idx" "0x0"

I'd personally use a different variable name here, e.g. idx2, so it's clearly separated.

BTW, if you only need the variable once, you can directly use logic on the assignment:

+	idx2="$(find_mtd_index u-boot-env2)" &&
+		ubootenv_add_uci_sys_config "/dev/mtd$idx2" "0x0"

Best

Adrian
Bjørn Mork April 6, 2021, 9:41 a.m. UTC | #2
"Adrian Schmutzler" <mail@adrianschmutzler.de> writes:

>> --- a/package/boot/uboot-envtools/files/realtek
>> +++ b/package/boot/uboot-envtools/files/realtek
>> @@ -15,15 +15,21 @@ zyxel,gs1900-10hp)
>>  	idx="$(find_mtd_index u-boot-env)"
>>  	[ -n "$idx" ] && \
>>  		ubootenv_add_uci_config "/dev/mtd$idx" "0x0" "0x400"
>> "0x10000"
>> +	idx="$(find_mtd_index u-boot-env2)"
>> +	[ -n "$idx" ] && \
>> +		ubootenv_add_uci_sys_config "/dev/mtd$idx" "0x0"
>
> I'd personally use a different variable name here, e.g. idx2, so it's clearly separated.

fixing up for v3

> BTW, if you only need the variable once, you can directly use logic on the assignment:
>
> +	idx2="$(find_mtd_index u-boot-env2)" &&
> +		ubootenv_add_uci_sys_config "/dev/mtd$idx2" "0x0"


Hmm, this doesn't work for me...  And it doesn't match existing patterns
in the package.  This is not the same:

 $ idx2="";[ -n "$idx2" ] && echo "idx2=$idx2"
 $ idx2="" && echo "idx2=$idx2"
 idx2=

Am I misunderstanding your comment?



Bjørn
diff mbox series

Patch

diff --git a/package/boot/uboot-envtools/Makefile b/package/boot/uboot-envtools/Makefile
index 590e38d8831a..601627011d56 100644
--- a/package/boot/uboot-envtools/Makefile
+++ b/package/boot/uboot-envtools/Makefile
@@ -61,6 +61,7 @@  MAKE_FLAGS += \
 define Package/uboot-envtools/conffiles
 /etc/config/ubootenv
 /etc/fw_env.config
+/etc/fw_sys.config
 endef
 
 define Package/uboot-envtools/install
diff --git a/package/boot/uboot-envtools/files/realtek b/package/boot/uboot-envtools/files/realtek
index cce0628ffcbb..b64bb23b0747 100644
--- a/package/boot/uboot-envtools/files/realtek
+++ b/package/boot/uboot-envtools/files/realtek
@@ -15,15 +15,21 @@  zyxel,gs1900-10hp)
 	idx="$(find_mtd_index u-boot-env)"
 	[ -n "$idx" ] && \
 		ubootenv_add_uci_config "/dev/mtd$idx" "0x0" "0x400" "0x10000"
+	idx="$(find_mtd_index u-boot-env2)"
+	[ -n "$idx" ] && \
+		ubootenv_add_uci_sys_config "/dev/mtd$idx" "0x0" "0x1000" "0x10000"
 	;;
 *)
 	idx="$(find_mtd_index u-boot-env)"
 	[ -n "$idx" ] && \
 		ubootenv_add_uci_config "/dev/mtd$idx" "0x0" "0x10000" "0x10000"
+	idx="$(find_mtd_index u-boot-env2)"
+	[ -n "$idx" ] && \
+		ubootenv_add_uci_sys_config "/dev/mtd$idx" "0x0" "0x1000" "0x10000"
 	;;
 esac
 
 config_load ubootenv
-config_foreach ubootenv_add_app_config ubootenv
+config_foreach ubootenv_add_app_config
 
 exit 0
diff --git a/package/boot/uboot-envtools/files/uboot-envtools.sh b/package/boot/uboot-envtools/files/uboot-envtools.sh
index 9218bc4e3912..980c9962b17c 100644
--- a/package/boot/uboot-envtools/files/uboot-envtools.sh
+++ b/package/boot/uboot-envtools/files/uboot-envtools.sh
@@ -3,34 +3,44 @@ 
 # Copyright (C) 2011-2012 OpenWrt.org
 #
 
-ubootenv_add_uci_config() {
-	local dev=$1
-	local offset=$2
-	local envsize=$3
-	local secsize=$4
-	local numsec=$5
+_ubootenv_add_uci_config() {
+	local cfgtype=$1
+	local dev=$2
+	local offset=$3
+	local envsize=$4
+	local secsize=$5
+	local numsec=$6
 	uci batch <<EOF
-add ubootenv ubootenv
-set ubootenv.@ubootenv[-1].dev='$dev'
-set ubootenv.@ubootenv[-1].offset='$offset'
-set ubootenv.@ubootenv[-1].envsize='$envsize'
-set ubootenv.@ubootenv[-1].secsize='$secsize'
-set ubootenv.@ubootenv[-1].numsec='$numsec'
+add ubootenv $cfgtype
+set ubootenv.@$cfgtype[-1].dev='$dev'
+set ubootenv.@$cfgtype[-1].offset='$offset'
+set ubootenv.@$cfgtype[-1].envsize='$envsize'
+set ubootenv.@$cfgtype[-1].secsize='$secsize'
+set ubootenv.@$cfgtype[-1].numsec='$numsec'
 EOF
 	uci commit ubootenv
 }
 
+ubootenv_add_uci_config() {
+	_ubootenv_add_uci_config "ubootenv" "$@"
+}
+
+ubootenv_add_uci_sys_config() {
+	_ubootenv_add_uci_config "ubootsys" "$@"
+}
+
 ubootenv_add_app_config() {
+	local cfgtype
 	local dev
 	local offset
 	local envsize
 	local secsize
 	local numsec
+	config_get cfgtype "$1" TYPE
 	config_get dev "$1" dev
 	config_get offset "$1" offset
 	config_get envsize "$1" envsize
 	config_get secsize "$1" secsize
 	config_get numsec "$1" numsec
-	grep -q "^[[:space:]]*${dev}[[:space:]]*${offset}" /etc/fw_env.config || echo "$dev $offset $envsize $secsize $numsec" >>/etc/fw_env.config
+	grep -q "^[[:space:]]*${dev}[[:space:]]*${offset}" "/etc/fw_${cfgtype#uboot}.config" || echo "$dev $offset $envsize $secsize $numsec" >>"/etc/fw_${cfgtype#uboot}.config"
 }
-