[1/2] bsp: harmonize settings across targets

This patch harmonizes the options in the OpenWrt .config files
across targets. Many of them have evolved somewhat independently,
and unifying them should make maintenance easier in the future.

The most important change is the consistent per-device build applied:
When building devices with OpenWrt, you have the option to either
build a default image for the (sub)target, an image for a single
device, or images for multiple devices. This is controlled by
CONFIG_TARGET_MULTI_PROFILE. In addition, the option
CONFIG_TARGET_PER_DEVICE_ROOTFS will toggle whether packages
are selected per-device or per-target.

When we build only a single device per target, setting these options
will only have minor effect. As soon as a second device is added
though, impact will be heavy, and devices may end up with no/wrong
packages.

Thus, this patch sets both options to "y" on all targets, which
essentially eliminates this problem for the future (and corresponds
to what the OpenWrt build bots do).

The only option not harmonized is the CONFIG_TARGET_SQUASHFS_BLOCK_SIZE.

All ath10k settings are target dependent as well.

Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
---
 bsp/ipq806x/.config | 17 +++++++++++++++--
 bsp/mt7621/.config  | 24 ++++++++++++++++++------
 bsp/wdr4900/.config |  9 +++++----
 3 files changed, 38 insertions(+), 12 deletions(-)

Moin,

sieht gut aus. Aber können wir die "is not set" Zeilen nicht weglassen?

On 01.04.20 12:47, Adrian Schmutzler wrote:
> This patch harmonizes the options in the OpenWrt .config files
> across targets. Many of them have evolved somewhat independently,
> and unifying them should make maintenance easier in the future.
> 
> The most important change is the consistent per-device build applied:
> When building devices with OpenWrt, you have the option to either
> build a default image for the (sub)target, an image for a single
> device, or images for multiple devices. This is controlled by
> CONFIG_TARGET_MULTI_PROFILE. In addition, the option
> CONFIG_TARGET_PER_DEVICE_ROOTFS will toggle whether packages
> are selected per-device or per-target.
> 
> When we build only a single device per target, setting these options
> will only have minor effect. As soon as a second device is added
> though, impact will be heavy, and devices may end up with no/wrong
> packages.
> 
> Thus, this patch sets both options to "y" on all targets, which
> essentially eliminates this problem for the future (and corresponds
> to what the OpenWrt build bots do).
> 
> The only option not harmonized is the CONFIG_TARGET_SQUASHFS_BLOCK_SIZE.
> 
> All ath10k settings are target dependent as well.
> 
> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> ---
>  bsp/ipq806x/.config | 17 +++++++++++++++--
>  bsp/mt7621/.config  | 24 ++++++++++++++++++------
>  bsp/wdr4900/.config |  9 +++++----
>  3 files changed, 38 insertions(+), 12 deletions(-)
> 
> diff --git a/bsp/ipq806x/.config b/bsp/ipq806x/.config
> index a02f76fa..d4997bfe 100644
> --- a/bsp/ipq806x/.config
> +++ b/bsp/ipq806x/.config
> @@ -6,10 +6,23 @@ CONFIG_TARGET_ipq806x_generic=y
>  CONFIG_TARGET_MULTI_PROFILE=y
>  CONFIG_TARGET_DEVICE_ipq806x_generic_DEVICE_tplink_c2600=y
>  CONFIG_TARGET_DEVICE_PACKAGES_ipq806x_generic_DEVICE_tplink_c2600="-ath10k-firmware-qca99x0-ct ath10k-firmware-qca99x0"
> -CONFIG_TARGET_PER_DEVICE_ROOTFS=y
>  CONFIG_BUSYBOX_CUSTOM=y
> +CONFIG_TARGET_PER_DEVICE_ROOTFS=y
> +# CONFIG_BUSYBOX_CONFIG_BRCTL is not set
> +# CONFIG_BUSYBOX_CONFIG_CROND is not set
> +# CONFIG_BUSYBOX_CONFIG_CRONTAB is not set
> +# CONFIG_BUSYBOX_CONFIG_FEATURE_FAST_TOP is not set
> +# CONFIG_BUSYBOX_CONFIG_FEATURE_NTPD_SERVER is not set
>  CONFIG_CLEAN_IPKG=y
> -CONFIG_PACKAGE_kmod-hwmon-core=m
> +# CONFIG_DROPBEAR_CURVE25519 is not set
> +# CONFIG_FASTD_ENABLE_CIPHER_SALSA2012 is not set
> +# CONFIG_FASTD_ENABLE_MAC_GHASH is not set
> +# CONFIG_FASTD_ENABLE_MAC_UHASH is not set
> +# CONFIG_FASTD_ENABLE_METHOD_COMPOSED_GMAC is not set
> +# CONFIG_FASTD_ENABLE_METHOD_COMPOSED_UMAC is not set
> +# CONFIG_FASTD_ENABLE_METHOD_GENERIC_GMAC is not set
> +# CONFIG_FASTD_ENABLE_METHOD_GENERIC_UMAC is not set
> +# CONFIG_PACKAGE_ALFRED_VIS is not set
>  CONFIG_PACKAGE_opkg=m
>  CONFIG_PACKAGE_ath10k-firmware-qca99x0=m
>  CONFIG_PACKAGE_kmod-ath10k=y
> diff --git a/bsp/mt7621/.config b/bsp/mt7621/.config
> index 45925362..ee581085 100644
> --- a/bsp/mt7621/.config
> +++ b/bsp/mt7621/.config
> @@ -5,14 +5,26 @@ CONFIG_TARGET_ramips=y
>  CONFIG_TARGET_ramips_mt7621=y
>  CONFIG_TARGET_MULTI_PROFILE=y
>  CONFIG_TARGET_DEVICE_ramips_mt7621_DEVICE_ubnt-erx=y
> +CONFIG_TARGET_DEVICE_PACKAGES_ramips_mt7621_DEVICE_ubnt-erx=""
>  CONFIG_TARGET_DEVICE_ramips_mt7621_DEVICE_ubnt-erx-sfp=y
> +CONFIG_TARGET_DEVICE_PACKAGES_ramips_mt7621_DEVICE_ubnt-erx-sfp=""
>  CONFIG_BUSYBOX_CUSTOM=y
> +CONFIG_TARGET_PER_DEVICE_ROOTFS=y
> +# CONFIG_BUSYBOX_CONFIG_BRCTL is not set
> +# CONFIG_BUSYBOX_CONFIG_CROND is not set
> +# CONFIG_BUSYBOX_CONFIG_CRONTAB is not set
> +# CONFIG_BUSYBOX_CONFIG_FEATURE_FAST_TOP is not set
> +# CONFIG_BUSYBOX_CONFIG_FEATURE_NTPD_SERVER is not set
>  CONFIG_CLEAN_IPKG=y
> -CONFIG_PACKAGE_kmod-ifb=y
> -CONFIG_PACKAGE_kmod-ledtrig-heartbeat=y
> -CONFIG_PACKAGE_kmod-ledtrig-netdev=y
> -CONFIG_PACKAGE_kmod-sched-connmark=y
> -CONFIG_PACKAGE_qos-scripts=y
> -CONFIG_PACKAGE_tc=y
> +# CONFIG_DROPBEAR_CURVE25519 is not set
> +# CONFIG_FASTD_ENABLE_CIPHER_SALSA2012 is not set
> +# CONFIG_FASTD_ENABLE_MAC_GHASH is not set
> +# CONFIG_FASTD_ENABLE_MAC_UHASH is not set
> +# CONFIG_FASTD_ENABLE_METHOD_COMPOSED_GMAC is not set
> +# CONFIG_FASTD_ENABLE_METHOD_COMPOSED_UMAC is not set
> +# CONFIG_FASTD_ENABLE_METHOD_GENERIC_GMAC is not set
> +# CONFIG_FASTD_ENABLE_METHOD_GENERIC_UMAC is not set
> +# CONFIG_PACKAGE_ALFRED_VIS is not set
> +CONFIG_PACKAGE_opkg=m
>  CONFIG_STRIP_KERNEL_EXPORTS=y
>  CONFIG_TARGET_SQUASHFS_BLOCK_SIZE=512
> diff --git a/bsp/wdr4900/.config b/bsp/wdr4900/.config
> index 29774c60..abdc06e5 100644
> --- a/bsp/wdr4900/.config
> +++ b/bsp/wdr4900/.config
> @@ -3,8 +3,11 @@
>  #
>  CONFIG_TARGET_mpc85xx=y
>  CONFIG_TARGET_mpc85xx_generic=y
> -CONFIG_TARGET_mpc85xx_generic_Default=y
> +CONFIG_TARGET_MULTI_PROFILE=y
> +CONFIG_TARGET_DEVICE_mpc85xx_generic_DEVICE_tl-wdr4900-v1=y
> +CONFIG_TARGET_DEVICE_PACKAGES_mpc85xx_generic_DEVICE_tl-wdr4900-v1=""
>  CONFIG_BUSYBOX_CUSTOM=y
> +CONFIG_TARGET_PER_DEVICE_ROOTFS=y
>  # CONFIG_BUSYBOX_CONFIG_BRCTL is not set
>  # CONFIG_BUSYBOX_CONFIG_CROND is not set
>  # CONFIG_BUSYBOX_CONFIG_CRONTAB is not set
> @@ -19,9 +22,7 @@ CONFIG_CLEAN_IPKG=y
>  # CONFIG_FASTD_ENABLE_METHOD_COMPOSED_UMAC is not set
>  # CONFIG_FASTD_ENABLE_METHOD_GENERIC_GMAC is not set
>  # CONFIG_FASTD_ENABLE_METHOD_GENERIC_UMAC is not set
> -# CONFIG_KERNEL_DEBUG_INFO is not set
> -# CONFIG_KERNEL_DEBUG_KERNEL is not set
>  # CONFIG_PACKAGE_ALFRED_VIS is not set
> -# CONFIG_PACKAGE_opkg is not set
> +CONFIG_PACKAGE_opkg=m
>  CONFIG_STRIP_KERNEL_EXPORTS=y
>  CONFIG_TARGET_SQUASHFS_BLOCK_SIZE=512
>

> sieht gut aus. Aber können wir die "is not set" Zeilen nicht weglassen? 

Ich habe das jetzt nicht konkret geprüft, bitte aber folgendes zu beachten (falls dir das nicht bewusst ist):

Die .config Dateien orientieren sich an der Kernel config. Es gibt daher für echte config parameter zwei Optionen:

enabled:
CONFIG=y

disabled:
# CONFIG_xy is not set

Letzteres bedeutet also _nicht_ ein no-change-use-default, sondern meint effectiv "CONFIG=n", was es in dieser Syntax nicht gibt.
Damit schaltet man also effektiv Dinge aktiv aus.

Bei Packages gibt es dann zusätzlich noch das "m".

Ich weiß jetzt nicht, ob wir diese CONFIGs tatsächlich ausschalten müssen (da müsste man mal bei fastd etc. nachschauen, ob die standardmäßig an sind). Ich habe sie nur von ar71xx kopiert.
Sind diese aber tatsächlich per default an, schalten wir sie mit diesen Zeilen aus. Würden wir die Zeilen weglassen, würde wir hingegen den Default verwenden.

Du kannst das auch selbst mit make menuconfig ausprobieren, du musst nur darauf achten, dass du echte Optionen erwischst. Bei Packages, die einmal selektiert wurden, kommt man in der Regel nur auf "m" zurück.

Grüße

Adrian

Hi,

wenn es sich am Kernel orientiert findet man dazu z.B. bei tldp.org:

"An option will either indicate some driver is built into the kernel
("=y") or will be built as a module ("=m") or is not selected. The
unselected state can either be indicated by a line starting with "#"
(e.g. "# CONFIG_SCSI is not set") or by the absence of the relevant line
from the .config file."

Ich verstehe das so, dass die Zeilen auch weg können.

Robert

Am 01.04.20 um 13:12 schrieb mail@adrianschmutzler.de:
>> sieht gut aus. Aber können wir die "is not set" Zeilen nicht weglassen? 
> Ich habe das jetzt nicht konkret geprüft, bitte aber folgendes zu beachten (falls dir das nicht bewusst ist):
>
> Die .config Dateien orientieren sich an der Kernel config. Es gibt daher für echte config parameter zwei Optionen:
>
> enabled:
> CONFIG=y
>
> disabled:
> # CONFIG_xy is not set
>
> Letzteres bedeutet also _nicht_ ein no-change-use-default, sondern meint effectiv "CONFIG=n", was es in dieser Syntax nicht gibt.
> Damit schaltet man also effektiv Dinge aktiv aus.
>
> Bei Packages gibt es dann zusätzlich noch das "m".
>
> Ich weiß jetzt nicht, ob wir diese CONFIGs tatsächlich ausschalten müssen (da müsste man mal bei fastd etc. nachschauen, ob die standardmäßig an sind). Ich habe sie nur von ar71xx kopiert.
> Sind diese aber tatsächlich per default an, schalten wir sie mit diesen Zeilen aus. Würden wir die Zeilen weglassen, würde wir hingegen den Default verwenden.
>
> Du kannst das auch selbst mit make menuconfig ausprobieren, du musst nur darauf achten, dass du echte Optionen erwischst. Bei Packages, die einmal selektiert wurden, kommt man in der Regel nur auf "m" zurück.
>
> Grüße
>
> Adrian

Hi Robert,

das wäre so, wenn die Konfiguration nur nach build/.config kopiert werden würde.

Unsere Konfigurationen sollen aber lediglich den Unterschied zur OpenWrt Standardkonfiguration abbilden.
Daher werden diese mit ./scripts/diffconfig.sh erzeugt und beim Anwenden von einem "make defconfig" zu den Defaults ergänzt.

"make defconfig" setzt dann für alle nicht existierenden Einstellungen die Defaults.
Siehe auch: https://openwrt.org/docs/guide-developer/build-system/use-buildsystem#defconfig

Gruß
Fabian

On 01.04.20 13:48, Robert Langhammer wrote:
> Hi,
> 
> wenn es sich am Kernel orientiert findet man dazu z.B. bei tldp.org:
> 
> "An option will either indicate some driver is built into the kernel ("=y") or will be built as a module ("=m") or is not selected. The unselected state can either be indicated by a line starting with "#" (e.g. "# CONFIG_SCSI is not set") or by the absence of the relevant line from the .config file."
> 
> Ich verstehe das so, dass die Zeilen auch weg können.
> 
> Robert
> 
> Am 01.04.20 um 13:12 schrieb mail@adrianschmutzler.de:
>>> sieht gut aus. Aber können wir die "is not set" Zeilen nicht weglassen? 
>> Ich habe das jetzt nicht konkret geprüft, bitte aber folgendes zu beachten (falls dir das nicht bewusst ist):
>>
>> Die .config Dateien orientieren sich an der Kernel config. Es gibt daher für echte config parameter zwei Optionen:
>>
>> enabled:
>> CONFIG=y
>>
>> disabled:
>> # CONFIG_xy is not set
>>
>> Letzteres bedeutet also _nicht_ ein no-change-use-default, sondern meint effectiv "CONFIG=n", was es in dieser Syntax nicht gibt.
>> Damit schaltet man also effektiv Dinge aktiv aus.
>>
>> Bei Packages gibt es dann zusätzlich noch das "m".
>>
>> Ich weiß jetzt nicht, ob wir diese CONFIGs tatsächlich ausschalten müssen (da müsste man mal bei fastd etc. nachschauen, ob die standardmäßig an sind). Ich habe sie nur von ar71xx kopiert.
>> Sind diese aber tatsächlich per default an, schalten wir sie mit diesen Zeilen aus. Würden wir die Zeilen weglassen, würde wir hingegen den Default verwenden.
>>
>> Du kannst das auch selbst mit make menuconfig ausprobieren, du musst nur darauf achten, dass du echte Optionen erwischst. Bei Packages, die einmal selektiert wurden, kommt man in der Regel nur auf "m" zurück.
>>
>> Grüße
>>
>> Adrian

Tatsächlich scheint das alles _nicht_ der default zu sein.
Dann bin ich erstmal dafür, das so zu belassen (aka. diesen Patch so zu applien), bis wir uns dazu entschieden haben, ob und wie wir Geräte mit 4MB Flash gesondert behandeln möchten.

Hier noch der Hinweis am Rand, dass kmod-ath10k-ct und die ganzen ct Firmwares per Default auch "m" sind und daher theoretisch noch weg könnten.

Reviewed-by: Fabian Bläse <fabian@blaese.de>

Gruß
Fabian

On 01.04.20 13:12, mail@adrianschmutzler.de wrote:
>> sieht gut aus. Aber können wir die "is not set" Zeilen nicht weglassen? 
> 
> Ich habe das jetzt nicht konkret geprüft, bitte aber folgendes zu beachten (falls dir das nicht bewusst ist):
> 
> Die .config Dateien orientieren sich an der Kernel config. Es gibt daher für echte config parameter zwei Optionen:
> 
> enabled:
> CONFIG=y
> 
> disabled:
> # CONFIG_xy is not set
> 
> Letzteres bedeutet also _nicht_ ein no-change-use-default, sondern meint effectiv "CONFIG=n", was es in dieser Syntax nicht gibt.
> Damit schaltet man also effektiv Dinge aktiv aus.
> 
> Bei Packages gibt es dann zusätzlich noch das "m".
> 
> Ich weiß jetzt nicht, ob wir diese CONFIGs tatsächlich ausschalten müssen (da müsste man mal bei fastd etc. nachschauen, ob die standardmäßig an sind). Ich habe sie nur von ar71xx kopiert.
> Sind diese aber tatsächlich per default an, schalten wir sie mit diesen Zeilen aus. Würden wir die Zeilen weglassen, würde wir hingegen den Default verwenden.
> 
> Du kannst das auch selbst mit make menuconfig ausprobieren, du musst nur darauf achten, dass du echte Optionen erwischst. Bei Packages, die einmal selektiert wurden, kommt man in der Regel nur auf "m" zurück.
> 
> Grüße
> 
> Adrian
>

Tatsächlich scheint das alles _nicht_ der default zu sein.
Dann bin ich erstmal dafür, das so zu belassen (aka. diesen Patch so zu applien), bis wir uns dazu entschieden haben, ob und wie wir Geräte mit 4MB Flash gesondert behandeln möchten.

Hier noch der Hinweis am Rand, dass kmod-ath10k-ct und die ganzen ct Firmwares per Default auch "m" sind und daher theoretisch noch weg könnten.

Reviewed-by: Fabian Bläse <fabian@blaese.de>

Gruß
Fabian

On 01.04.20 13:12, mail@adrianschmutzler.de wrote:
>> sieht gut aus. Aber können wir die "is not set" Zeilen nicht weglassen? 
> 
> Ich habe das jetzt nicht konkret geprüft, bitte aber folgendes zu beachten (falls dir das nicht bewusst ist):
> 
> Die .config Dateien orientieren sich an der Kernel config. Es gibt daher für echte config parameter zwei Optionen:
> 
> enabled:
> CONFIG=y
> 
> disabled:
> # CONFIG_xy is not set
> 
> Letzteres bedeutet also _nicht_ ein no-change-use-default, sondern meint effectiv "CONFIG=n", was es in dieser Syntax nicht gibt.
> Damit schaltet man also effektiv Dinge aktiv aus.
> 
> Bei Packages gibt es dann zusätzlich noch das "m".
> 
> Ich weiß jetzt nicht, ob wir diese CONFIGs tatsächlich ausschalten müssen (da müsste man mal bei fastd etc. nachschauen, ob die standardmäßig an sind). Ich habe sie nur von ar71xx kopiert.
> Sind diese aber tatsächlich per default an, schalten wir sie mit diesen Zeilen aus. Würden wir die Zeilen weglassen, würde wir hingegen den Default verwenden.
> 
> Du kannst das auch selbst mit make menuconfig ausprobieren, du musst nur darauf achten, dass du echte Optionen erwischst. Bei Packages, die einmal selektiert wurden, kommt man in der Regel nur auf "m" zurück.
> 
> Grüße
> 
> Adrian
>

> Hier noch der Hinweis am Rand, dass kmod-ath10k-ct und die ganzen ct Firmwares per Default auch "m" sind und daher theoretisch noch weg könnten.

Das habe ich zunächst auch gedacht, tatsächlich wird das aber durch die Selektion als DEVICE_PACKAGES überschrieben.
Aus der Sicht eines einzelnen betroffenen Gerätes ist dann also die Config trotzdem =y und muss zusätzlich noch mit dem Config-Parameter auf y gesetzt werden.
Zumindest war das bei meinen Build-Tests immer so.

Grüße

Adrian

Das dürfte eigentlich nicht sein, da wir die Device Packages ja nochmal explizit überschreiben.
Ich kann mich aber auch täuschen.

Ich würde diesen Patch dafür jetzt auf jeden Fall nicht verändern.

Gruß
Fabian

On 01.04.20 14:23, mail@adrianschmutzler.de wrote:
>> Hier noch der Hinweis am Rand, dass kmod-ath10k-ct und die ganzen ct Firmwares per Default auch "m" sind und daher theoretisch noch weg könnten.
> 
> Das habe ich zunächst auch gedacht, tatsächlich wird das aber durch die Selektion als DEVICE_PACKAGES überschrieben.
> Aus der Sicht eines einzelnen betroffenen Gerätes ist dann also die Config trotzdem =y und muss zusätzlich noch mit dem Config-Parameter auf y gesetzt werden.
> Zumindest war das bei meinen Build-Tests immer so.
> 
> Grüße
> 
> Adrian 
>

> Das dürfte eigentlich nicht sein, da wir die Device Packages ja nochmal explizit überschreiben. 
> Ich kann mich aber auch täuschen. 

Meine Vermutung ist, dass wir hier nichts "überschreiben", sondern die Änderungen (-packageA packageB) nachträglich angewandt werden.

Applied (nur 1/2).