| Message ID | 20200131154620.25773-4-tomek_n@o2.pl |
|---|---|
| State | Rejected |
| Headers | show |
| Series | mvebu: Second round of clean-ups | expand |
Hi, > -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On > Behalf Of Tomasz Maciej Nowak > Sent: Freitag, 31. Januar 2020 16:46 > To: openwrt-devel@lists.openwrt.org > Subject: [OpenWrt-Devel] [PATCH 3/8] mvebu: image: drop empty variables from > default profile > > These variables are already initialized within DEVICE_VARS. Just move > DEVICE_VARS to make sure they are set before default profile. I've just played around with this in ath79 for some other reason, and found that DEVICE_VARS _won't_ set the variables to empty values. It will just make sure that the variables can be _set_ per device (e.g. DEVICE_DTS_DIR vs. DTS_DIR). If you do not set the variable for one device, it will just have the value from the previous device(!). So, setting the variables to "" in the default definition actually makes sure that they really are zero for those devices where they are not set. (Of course, typically those are the devices where they aren't evaluated anyway.) Best Adrian > > Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl> > --- > target/linux/mvebu/image/Makefile | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/target/linux/mvebu/image/Makefile > b/target/linux/mvebu/image/Makefile > index aeabffdca2..d9e4b1acce 100644 > --- a/target/linux/mvebu/image/Makefile > +++ b/target/linux/mvebu/image/Makefile > @@ -75,6 +75,7 @@ define Build/uDPU-firmware > (cd $@-fw; $(TAR) -cvzf $(KDIR_TMP)/$(IMAGE_PREFIX)-firmware.tgz .) > endef > > +DEVICE_VARS += BOOT_SCRIPT UBOOT > define Device/Default > PROFILES := Default > DEVICE_DTS = $$(SOC)-$(lastword $(subst _, ,$(1))) > @@ -86,10 +87,7 @@ define Device/Default > IMAGE/sysupgrade.bin := sysupgrade-tar | append-metadata > SUPPORTED_DEVICES = $(subst _,$(comma),$(1)) > UBINIZE_OPTS := -E 5 > - UBOOT := > - BOOT_SCRIPT := > endef > -DEVICE_VARS += BOOT_SCRIPT UBOOT > > define Device/Default-arm64 > BOOT_SCRIPT := generic-arm64 > -- > 2.25.0 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Hi Adrian. W dniu 31.01.2020 o 16:56, Adrian Schmutzler pisze: > Hi, > >> -----Original Message----- >> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On >> Behalf Of Tomasz Maciej Nowak >> Sent: Freitag, 31. Januar 2020 16:46 >> To: openwrt-devel@lists.openwrt.org >> Subject: [OpenWrt-Devel] [PATCH 3/8] mvebu: image: drop empty variables from >> default profile >> >> These variables are already initialized within DEVICE_VARS. Just move >> DEVICE_VARS to make sure they are set before default profile. > > I've just played around with this in ath79 for some other reason, and found that > DEVICE_VARS _won't_ set the variables to empty values. It will just make sure > that the variables can be _set_ per device (e.g. DEVICE_DTS_DIR vs. DTS_DIR). If > you do not set the variable for one device, it will just have the value from the > previous device(!). I saw similar behavior when variables were set but not added to DEVICE_VARS. From the tests I've done before sending, the produced images looked fine, but I'll re-test that to make sure. > So, setting the variables to "" in the default definition actually makes sure > that they really are zero for those devices where they are not set. (Of course, > typically those are the devices where they aren't evaluated anyway.) > > Best > > Adrian > Regards > > >> >> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl> >> --- >> target/linux/mvebu/image/Makefile | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/target/linux/mvebu/image/Makefile >> b/target/linux/mvebu/image/Makefile >> index aeabffdca2..d9e4b1acce 100644 >> --- a/target/linux/mvebu/image/Makefile >> +++ b/target/linux/mvebu/image/Makefile >> @@ -75,6 +75,7 @@ define Build/uDPU-firmware >> (cd $@-fw; $(TAR) -cvzf $(KDIR_TMP)/$(IMAGE_PREFIX)-firmware.tgz .) >> endef >> >> +DEVICE_VARS += BOOT_SCRIPT UBOOT >> define Device/Default >> PROFILES := Default >> DEVICE_DTS = $$(SOC)-$(lastword $(subst _, ,$(1))) >> @@ -86,10 +87,7 @@ define Device/Default >> IMAGE/sysupgrade.bin := sysupgrade-tar | append-metadata >> SUPPORTED_DEVICES = $(subst _,$(comma),$(1)) >> UBINIZE_OPTS := -E 5 >> - UBOOT := >> - BOOT_SCRIPT := >> endef >> -DEVICE_VARS += BOOT_SCRIPT UBOOT >> >> define Device/Default-arm64 >> BOOT_SCRIPT := generic-arm64 >> -- >> 2.25.0 >> >> >> _______________________________________________ >> openwrt-devel mailing list >> openwrt-devel@lists.openwrt.org >> https://lists.openwrt.org/mailman/listinfo/openwrt-devel >
Hi, > I saw similar behavior when variables were set but not added to DEVICE_VARS. > From the tests I've done before sending, the produced images looked fine, but > I'll re-test that to make sure. When variables are set, but are _not_ added to DEVICE_VARS, the variables will have _one_ single value for _all_ devices, i.e. the one set for the last device. When variables are _not_ set, but are added to DEVICE_VARS, the variables will have the last value set to any device before (i.e. the last device setting it). Note that Device/Default counts like an include to the current device there. I've tested this for a bunch of tplink-safeloader devices in ath79 by adding the following line to the Build/tplink-safeloader definition: printf "$(DEVICE_TITLE) l$(LOADER_TYPE) x$(TPLINK_HWID) y$(TPLINK_HWREV) z$(TPLINK_HWREVADD)'\n" >> /data/openwrt/safeloadercheck.txt Best Adrian > > > So, setting the variables to "" in the default definition actually makes sure > > that they really are zero for those devices where they are not set. (Of course, > > typically those are the devices where they aren't evaluated anyway.) > > > > Best > > > > Adrian > > > > Regards > > > > > > >> > >> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl> > >> --- > >> target/linux/mvebu/image/Makefile | 4 +--- > >> 1 file changed, 1 insertion(+), 3 deletions(-) > >> > >> diff --git a/target/linux/mvebu/image/Makefile > >> b/target/linux/mvebu/image/Makefile > >> index aeabffdca2..d9e4b1acce 100644 > >> --- a/target/linux/mvebu/image/Makefile > >> +++ b/target/linux/mvebu/image/Makefile > >> @@ -75,6 +75,7 @@ define Build/uDPU-firmware > >> (cd $@-fw; $(TAR) -cvzf $(KDIR_TMP)/$(IMAGE_PREFIX)-firmware.tgz .) > >> endef > >> > >> +DEVICE_VARS += BOOT_SCRIPT UBOOT > >> define Device/Default > >> PROFILES := Default > >> DEVICE_DTS = $$(SOC)-$(lastword $(subst _, ,$(1))) > >> @@ -86,10 +87,7 @@ define Device/Default > >> IMAGE/sysupgrade.bin := sysupgrade-tar | append-metadata > >> SUPPORTED_DEVICES = $(subst _,$(comma),$(1)) > >> UBINIZE_OPTS := -E 5 > >> - UBOOT := > >> - BOOT_SCRIPT := > >> endef > >> -DEVICE_VARS += BOOT_SCRIPT UBOOT > >> > >> define Device/Default-arm64 > >> BOOT_SCRIPT := generic-arm64 > >> -- > >> 2.25.0 > >> > >> > >> _______________________________________________ > >> openwrt-devel mailing list > >> openwrt-devel@lists.openwrt.org > >> https://lists.openwrt.org/mailman/listinfo/openwrt-devel > > > > > -- > TMN
> When variables are _not_ set, but are added to DEVICE_VARS, the variables will > have the last value set to any device before (i.e. the last device setting it). Note > that Device/Default counts like an include to the current device there. Still not precise enough here: When variables are _not_ set, but are added to DEVICE_VARS, the variables for each device will have the last value set to any device before the current device (i.e. the last device setting it). Note that Device/Default counts like an include to the current device there, so it effectively overrides the values taken over from the previous ones and thus creates the "expected" behavior.
W dniu 31.01.2020 o 19:33, Adrian Schmutzler pisze: > Hi, > >> I saw similar behavior when variables were set but not added to DEVICE_VARS. >> From the tests I've done before sending, the produced images looked fine, but >> I'll re-test that to make sure. > > When variables are set, but are _not_ added to DEVICE_VARS, the variables will have _one_ single value for _all_ devices, i.e. the one set for the last device. > > When variables are _not_ set, but are added to DEVICE_VARS, the variables will have the last value set to any device before (i.e. the last device setting it). Note that Device/Default counts like an include to the current device there. > Thanks, that makes it clear, after testing the DEVICE_DTS_DIR, I'll send v2 without this patch. > I've tested this for a bunch of tplink-safeloader devices in ath79 by adding the following line to the Build/tplink-safeloader definition: > printf "$(DEVICE_TITLE) l$(LOADER_TYPE) x$(TPLINK_HWID) y$(TPLINK_HWREV) z$(TPLINK_HWREVADD)'\n" >> /data/openwrt/safeloadercheck.txt > > Best > > Adrian > Regards >> >>> So, setting the variables to "" in the default definition actually makes sure >>> that they really are zero for those devices where they are not set. (Of course, >>> typically those are the devices where they aren't evaluated anyway.) >>> >>> Best >>> >>> Adrian >>> >> >> Regards >> >>> >>> >>>> >>>> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl> >>>> --- >>>> target/linux/mvebu/image/Makefile | 4 +--- >>>> 1 file changed, 1 insertion(+), 3 deletions(-) >>>> >>>> diff --git a/target/linux/mvebu/image/Makefile >>>> b/target/linux/mvebu/image/Makefile >>>> index aeabffdca2..d9e4b1acce 100644 >>>> --- a/target/linux/mvebu/image/Makefile >>>> +++ b/target/linux/mvebu/image/Makefile >>>> @@ -75,6 +75,7 @@ define Build/uDPU-firmware >>>> (cd $@-fw; $(TAR) -cvzf $(KDIR_TMP)/$(IMAGE_PREFIX)-firmware.tgz .) >>>> endef >>>> >>>> +DEVICE_VARS += BOOT_SCRIPT UBOOT >>>> define Device/Default >>>> PROFILES := Default >>>> DEVICE_DTS = $$(SOC)-$(lastword $(subst _, ,$(1))) >>>> @@ -86,10 +87,7 @@ define Device/Default >>>> IMAGE/sysupgrade.bin := sysupgrade-tar | append-metadata >>>> SUPPORTED_DEVICES = $(subst _,$(comma),$(1)) >>>> UBINIZE_OPTS := -E 5 >>>> - UBOOT := >>>> - BOOT_SCRIPT := >>>> endef >>>> -DEVICE_VARS += BOOT_SCRIPT UBOOT >>>> >>>> define Device/Default-arm64 >>>> BOOT_SCRIPT := generic-arm64 >>>> -- >>>> 2.25.0 >>>> >>>> >>>> _______________________________________________ >>>> openwrt-devel mailing list >>>> openwrt-devel@lists.openwrt.org >>>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel >>> >> >> >> -- >> TMN >
diff --git a/target/linux/mvebu/image/Makefile b/target/linux/mvebu/image/Makefile index aeabffdca2..d9e4b1acce 100644 --- a/target/linux/mvebu/image/Makefile +++ b/target/linux/mvebu/image/Makefile @@ -75,6 +75,7 @@ define Build/uDPU-firmware (cd $@-fw; $(TAR) -cvzf $(KDIR_TMP)/$(IMAGE_PREFIX)-firmware.tgz .) endef +DEVICE_VARS += BOOT_SCRIPT UBOOT define Device/Default PROFILES := Default DEVICE_DTS = $$(SOC)-$(lastword $(subst _, ,$(1))) @@ -86,10 +87,7 @@ define Device/Default IMAGE/sysupgrade.bin := sysupgrade-tar | append-metadata SUPPORTED_DEVICES = $(subst _,$(comma),$(1)) UBINIZE_OPTS := -E 5 - UBOOT := - BOOT_SCRIPT := endef -DEVICE_VARS += BOOT_SCRIPT UBOOT define Device/Default-arm64 BOOT_SCRIPT := generic-arm64
These variables are already initialized within DEVICE_VARS. Just move DEVICE_VARS to make sure they are set before default profile. Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl> --- target/linux/mvebu/image/Makefile | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)