[OpenWrt-Devel] [PATCH] ath79: introduces KERNEL_LZMA variable for common build sequence

Piotr Dymacz pepe2k at gmail.com
Sun Feb 2 13:31:45 EST 2020


Hi Adrian,

On 02.02.2020 18:20, mail at adrianschmutzler.de wrote:
> Hi Piotr,
> 
>> -----Original Message-----
>> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
>> On Behalf Of Piotr Dymacz
>> Sent: Sonntag, 2. Februar 2020 18:02
>> To: Adrian Schmutzler <freifunk at adrianschmutzler.de>; openwrt-
>> devel at lists.openwrt.org
>> Subject: Re: [OpenWrt-Devel] [PATCH] ath79: introduces KERNEL_LZMA
>> variable for common build sequence
>> 
>> Hi Adrian,
>> 
>> On 02.02.2020 13:48, Adrian Schmutzler wrote:
>> > This introduce a variable KERNEL_LZMA to replace the frequently used
>> > sequence "kernel-bin | append-dtb | lzma", similar to the KERNEL_DTB
>> > variable in ramips target.
>> 
>> So in results we will have:
>> 
>> ramips: KERNEL_DTB   = kernel-bin | append-dtb | lzma
>>   ath79: KERNEL_LZMA := kernel-bin | append-dtb | lzma
>> 
>> Is there any reason to use different var name in ath79?
> 
> In ath79 we have some cases where gzip is used instead of lzma, e.g.
> https://github.com/openwrt/openwrt/blob/master/target/linux/ath79/image/common-tp-link.mk#L42

Plus some where no compression is used.

> Thus, if we use KERNEL_DTB for the case with lzma, it would be misleading, as the name only tells us about the DTB.

One code base, two variables with same values using different names?
That looks wrong to me.

> So, we could either just use " KERNEL_DTB  := kernel-bin | append-dtb ", which is too short to be helpful IMO, or use a more indicative name for the VARIABLE, which I thought I found. Actually, I would consider it more correct to change the name for ramips, too.

'KERNEL_DTB := kernel-bin | append-dtb' would cover almost everything, 
including KERNEL_INITRAMFS, but I agree that wouldn't help much.

> We could also use KERNEL_DTB_LZMA of course, but I do not think this will really bring a benefit.

"There are only two hard things in Computer Science: cache invalidation 
and naming things." :)

I care more about consistency in naming variables carrying same values 
than the name itself.

-- 
Cheers,
Piotr

> 
> Best
> 
> Adrian
> 
>> 
>> Also, see: https://chris.beams.io/posts/git-commit/#imperative
>> 
>> --
>> Cheers,
>> Piotr
>> 
>> > Signed-off-by: Adrian Schmutzler <freifunk at adrianschmutzler.de>
>> > ---
>> >   target/linux/ath79/image/Makefile           |  5 +++--
>> >   target/linux/ath79/image/common-mikrotik.mk |  4 ++--
>> >   target/linux/ath79/image/common-tp-link.mk  | 14 ++++++--------
>> >   target/linux/ath79/image/generic-tp-link.mk |  5 ++---
>> >   target/linux/ath79/image/generic-ubnt.mk    |  2 +-
>> >   target/linux/ath79/image/generic.mk         | 10 +++++-----
>> >   6 files changed, 19 insertions(+), 21 deletions(-)
>> >
>> > diff --git a/target/linux/ath79/image/Makefile
>> > b/target/linux/ath79/image/Makefile
>> > index a28658ec0b..71ffa4b429 100644
>> > --- a/target/linux/ath79/image/Makefile
>> > +++ b/target/linux/ath79/image/Makefile
>> > @@ -52,6 +52,7 @@ define Build/relocate-kernel
>> >   	rm -rf $@.relocate
>> >   endef
>> >
>> > +KERNEL_LZMA := kernel-bin | append-dtb | lzma
>> >
>> >   define Device/Default
>> >     DEVICE_DTS_DIR := ../dts
>> > @@ -59,8 +60,8 @@ define Device/Default
>> >     PROFILES = Default
>> >     MTDPARTS :=
>> >     BLOCKSIZE := 64k
>> > -  KERNEL := kernel-bin | append-dtb | lzma | uImage lzma
>> > -  KERNEL_INITRAMFS := kernel-bin | append-dtb | lzma | uImage lzma
>> > +  KERNEL := $(KERNEL_LZMA) | uImage lzma  KERNEL_INITRAMFS :=
>> > + $(KERNEL_LZMA) | uImage lzma
>> >     COMPILE :=
>> >     SUPPORTED_DEVICES := $(subst _,$(comma),$(1))
>> >     IMAGES := sysupgrade.bin
>> > diff --git a/target/linux/ath79/image/common-mikrotik.mk
>> > b/target/linux/ath79/image/common-mikrotik.mk
>> > index 292237c76a..6531619fe9 100644
>> > --- a/target/linux/ath79/image/common-mikrotik.mk
>> > +++ b/target/linux/ath79/image/common-mikrotik.mk
>> > @@ -2,6 +2,6 @@ define Device/mikrotik
>> >   	DEVICE_VENDOR := MikroTik
>> >   	DEVICE_PACKAGES := rbextract rbcfg
>> >   	LOADER_TYPE := elf
>> > -	KERNEL := kernel-bin | append-dtb | lzma | loader-kernel
>> > -	KERNEL_INITRAMFS := kernel-bin | append-dtb | lzma | loader-
>> kernel
>> > +	KERNEL := $(KERNEL_LZMA) | loader-kernel
>> > +	KERNEL_INITRAMFS := $(KERNEL_LZMA) | loader-kernel
>> >   endef
>> > diff --git a/target/linux/ath79/image/common-tp-link.mk
>> > b/target/linux/ath79/image/common-tp-link.mk
>> > index a9fccd0fe6..abce4095c4 100644
>> > --- a/target/linux/ath79/image/common-tp-link.mk
>> > +++ b/target/linux/ath79/image/common-tp-link.mk
>> > @@ -14,8 +14,8 @@ define Device/tplink-v1
>> >     TPLINK_HWREV := 0x1
>> >     TPLINK_HEADER_VERSION := 1
>> >     LOADER_TYPE := gz
>> > -  KERNEL := kernel-bin | append-dtb | lzma
>> > -  KERNEL_INITRAMFS := kernel-bin | append-dtb | lzma |
>> > tplink-v1-header
>> > +  KERNEL := $(KERNEL_LZMA)
>> > +  KERNEL_INITRAMFS := $(KERNEL_LZMA) | tplink-v1-header
>> >     IMAGES += factory.bin
>> >     IMAGE/sysupgrade.bin := tplink-v1-image sysupgrade | append-
>> metadata
>> >     IMAGE/factory.bin := tplink-v1-image factory @@ -26,8 +26,7 @@
>> > define Device/tplink-nolzma
>> >     LOADER_FLASH_OFFS := 0x22000
>> >     COMPILE := loader-$(1).gz
>> >     COMPILE/loader-$(1).gz := loader-okli-compile
>> > -  KERNEL := kernel-bin | append-dtb | lzma | uImage lzma -M 0x4f4b4c49 |
>> \
>> > -	loader-okli $(1) 7680
>> > +  KERNEL := $(KERNEL_LZMA) | uImage lzma -M 0x4f4b4c49 | loader-okli
>> > + $(1) 7680
>> >     KERNEL_INITRAMFS := kernel-bin | append-dtb | gzip | tplink-v1-header
>> >   endef
>> >
>> > @@ -63,7 +62,7 @@ endef
>> >
>> >   define Device/tplink-safeloader
>> >     $(Device/tplink-v1)
>> > -  KERNEL := kernel-bin | append-dtb | lzma | tplink-v1-header -O
>> > +  KERNEL := $(KERNEL_LZMA) | tplink-v1-header -O
>> >     IMAGE/sysupgrade.bin := append-rootfs | tplink-safeloader sysupgrade
>> | \
>> >       append-metadata | check-size $$$$(IMAGE_SIZE)
>> >     IMAGE/factory.bin := append-rootfs | tplink-safeloader factory @@
>> > -71,7 +70,7 @@ endef
>> >
>> >   define Device/tplink-safeloader-uimage
>> >     $(Device/tplink-safeloader)
>> > -  KERNEL := kernel-bin | append-dtb | lzma | uImageArcher lzma
>> > +  KERNEL := $(KERNEL_LZMA) | uImageArcher lzma
>> >   endef
>> >
>> >   define Device/tplink-safeloader-okli @@ -80,6 +79,5 @@ define
>> > Device/tplink-safeloader-okli
>> >     LOADER_FLASH_OFFS := 0x43000
>> >     COMPILE := loader-$(1).elf
>> >     COMPILE/loader-$(1).elf := loader-okli-compile
>> > -  KERNEL := kernel-bin | append-dtb | lzma | uImage lzma -M 0x4f4b4c49 |
>> \
>> > -	loader-okli $(1) 12288
>> > +  KERNEL := $(KERNEL_LZMA) | uImage lzma -M 0x4f4b4c49 | loader-okli
>> > + $(1) 12288
>> >   endef
>> > diff --git a/target/linux/ath79/image/generic-tp-link.mk
>> > b/target/linux/ath79/image/generic-tp-link.mk
>> > index f1a603dc6d..30e6f979c4 100644
>> > --- a/target/linux/ath79/image/generic-tp-link.mk
>> > +++ b/target/linux/ath79/image/generic-tp-link.mk
>> > @@ -203,9 +203,8 @@ define Device/tplink_archer-d50-v1
>> >     TPLINK_FLASHLAYOUT := 8Mqca
>> >     TPLINK_HWREVADD := 0x00000000
>> >     TPLINK_HVERSION := 3
>> > -  KERNEL := kernel-bin | append-dtb | lzma
>> > -  KERNEL_INITRAMFS := kernel-bin | append-dtb | lzma | \
>> > -	tplink-v2-header -s -V "ver. 1.0"
>> > +  KERNEL := $(KERNEL_LZMA)
>> > +  KERNEL_INITRAMFS := $(KERNEL_LZMA) | tplink-v2-header -s -V "ver.
>> 1.0"
>> >     IMAGES := sysupgrade.bin
>> >     IMAGE/sysupgrade.bin := tplink-v2-image -s -V "ver. 2.0" | append-
>> metadata | \
>> >   	check-size $$$$(IMAGE_SIZE)
>> > diff --git a/target/linux/ath79/image/generic-ubnt.mk
>> > b/target/linux/ath79/image/generic-ubnt.mk
>> > index ffae83eda5..bbacd393cb 100644
>> > --- a/target/linux/ath79/image/generic-ubnt.mk
>> > +++ b/target/linux/ath79/image/generic-ubnt.mk
>> > @@ -220,7 +220,7 @@ define Device/ubnt_routerstation_common
>> >     IMAGES := factory.bin
>> >     IMAGE/factory.bin := append-rootfs | pad-rootfs | mkubntimage | \
>> >   	check-size $$$$(IMAGE_SIZE)
>> > -  KERNEL := kernel-bin | append-dtb | lzma | pad-to $$(BLOCKSIZE)
>> > +  KERNEL := $(KERNEL_LZMA) | pad-to $$(BLOCKSIZE)
>> >     KERNEL_INITRAMFS := kernel-bin | append-dtb
>> >   endef
>> >
>> > diff --git a/target/linux/ath79/image/generic.mk
>> > b/target/linux/ath79/image/generic.mk
>> > index 1bc7b2d68e..7555ce4024 100644
>> > --- a/target/linux/ath79/image/generic.mk
>> > +++ b/target/linux/ath79/image/generic.mk
>> > @@ -105,7 +105,7 @@ define Device/adtran_bsap1880
>> >     SOC := ar7161
>> >     DEVICE_VENDOR := Adtran/Bluesocket
>> >     DEVICE_PACKAGES += -swconfig -uboot-envtools fconfig
>> > -  KERNEL := kernel-bin | append-dtb | lzma | pad-to $$(BLOCKSIZE)
>> > +  KERNEL := $(KERNEL_LZMA) | pad-to $$(BLOCKSIZE)
>> >     KERNEL_INITRAMFS := kernel-bin | append-dtb
>> >     IMAGE_SIZE := 11200k
>> >     IMAGES += kernel.bin rootfs.bin
>> > @@ -151,7 +151,7 @@ define Device/avm_fritz300e
>> >     SOC := ar7242
>> >     DEVICE_VENDOR := AVM
>> >     DEVICE_MODEL := FRITZ!WLAN Repeater 300E
>> > -  KERNEL := kernel-bin | append-dtb | lzma | eva-image
>> > +  KERNEL := $(KERNEL_LZMA) | eva-image
>> >     KERNEL_INITRAMFS := $$(KERNEL)
>> >     IMAGE_SIZE := 15232k
>> >     IMAGE/sysupgrade.bin := append-kernel | pad-to 64k | \ @@ -166,7
>> > +166,7 @@ define Device/avm_fritz4020
>> >     DEVICE_VENDOR := AVM
>> >     DEVICE_MODEL := FRITZ!Box 4020
>> >     IMAGE_SIZE := 15232k
>> > -  KERNEL := kernel-bin | append-dtb | lzma | eva-image
>> > +  KERNEL := $(KERNEL_LZMA) | eva-image
>> >     KERNEL_INITRAMFS := $$(KERNEL)
>> >     IMAGE/sysupgrade.bin := append-kernel | pad-to 64k | \
>> >   	append-squashfs-fakeroot-be | pad-to 256 | append-rootfs |
>> > pad-rootfs | \ @@ -712,7 +712,7 @@ define Device/jjplus_ja76pf2
>> >     IMAGES := kernel.bin rootfs.bin
>> >     IMAGE/kernel.bin := append-kernel
>> >     IMAGE/rootfs.bin := append-rootfs | pad-rootfs
>> > -  KERNEL := kernel-bin | append-dtb | lzma | pad-to $$(BLOCKSIZE)
>> > +  KERNEL := $(KERNEL_LZMA) | pad-to $$(BLOCKSIZE)
>> >     KERNEL_INITRAMFS := kernel-bin | append-dtb
>> >     IMAGE_SIZE := 16000k
>> >     SUPPORTED_DEVICES += ja76pf2
>> > @@ -992,7 +992,7 @@ define Device/pisen_wmb001n
>> >     COMPILE/loader-$(1).bin := loader-okli-compile
>> >     COMPILE/loader-$(1).uImage := append-loader-okli $(1) | pad-to 64k |
>> lzma | \
>> >   	uImage lzma
>> > -  KERNEL := kernel-bin | append-dtb | lzma | uImage lzma -M
>> > 0x4f4b4c49
>> > +  KERNEL := $(KERNEL_LZMA) | uImage lzma -M 0x4f4b4c49
>> >     IMAGES += factory.bin
>> >     IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | pisen_wmb001n-
>> factory $(1)
>> >   endef
>> >
>> 
>> 
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel at lists.openwrt.org
>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> 


_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel



More information about the openwrt-devel mailing list