[OpenWrt-Devel] [PATCH] build: reflect DEVICE_TYPE to top level config

mail at adrianschmutzler.de mail at adrianschmutzler.de
Fri May 29 13:39:55 EDT 2020


> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Linus Walleij
> Sent: Freitag, 29. Mai 2020 14:21
> To: openwrt-devel at lists.openwrt.org
> Cc: Linus Walleij <linus.walleij at linaro.org>
> Subject: [OpenWrt-Devel] [PATCH] build: reflect DEVICE_TYPE to top level
> config

Hi again,

sorry for intruding into this subject, but this has annoyed me for a long time already.

I've just sent a small patchset to tidy up the existing situation.

> 
> I made a patch to select a tool inside busybox by default on NAS type boxes,
> but this does not properly work because the package and image build
> processes are cleanly separate entities.
> 
> I also noticed that this becomes a problem if you build multiple profiles:
> maybe one of them is NAS and one of them is a router. You still want the
> least common denominator to decide: if you selected both NAS:es and
> routers, build packages that will be suitable for both NAS and routers.
> 
> To achieve this I reflect the DEVICE_TYPE up to the Kconfig level and define
> two Kconfig symbols:
> 
> config DEVICE_TYPE_ROUTER
>        bool
> 
> config DEVICE_TYPE_NAS
>        bool
> 
> These will be set from the DEVICE_TYPE of each profile and it is possible to
> select both.

Unfortunately, this doesn't seem to work like (at least) I would expect it to:

For the "Default Profile", the CONFIG_DEVICE_TYPE_* is set based on the target/subtarget Makefile.
If you select an individual device as target profile, the setting in the (sub)target Makefile is ignored (!), and the default ("router") is used. If the device has a DEVICE_TYPE in the device definition (which is wrong based on the initial concept, see my patch 1/3), then this value will be used.
If you select "Multiple devices", then CONFIG_DEVICE_TYPE_* won't be set at all.

As you stated earlier, it's just not so easy to connect the target and device scopes with each other. At the moment, I see two ways out of this:

1. We just live with the fact that the switch between router/nas/basic is per subtarget and adjust the code based on that.
2. We make the DEVICE_TYPE a real device-dependent variable and move it from target.mk to image.mk. Then, we could still set default values per target, but would have to adjust DEVICE_PACKAGES instead of DEFAULT_PACKAGES, which would lead to problems when building the Default Profile, but would make it much easier to deal with the individual devices.

Anyway, thanks for stirring this topic up again. Unfortunately, I don't think this will come cheap.

Best

Adrian

> 
> I then modify the busybox config to take this into account and conditionally
> build hdparm for CONFIG_DEVICE_TYPE_NAS.
> 
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
> ---
>  include/image.mk               |  1 +
>  include/target.mk              |  1 +
>  package/utils/busybox/Makefile |  2 +-
>  scripts/metadata.pm            |  2 ++
>  scripts/target-metadata.pl     | 12 ++++++++++++
>  5 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/include/image.mk b/include/image.mk index
> 984b64fb9c73..8104c040a1f7 100644
> --- a/include/image.mk
> +++ b/include/image.mk
> @@ -634,6 +634,7 @@ endef
>  define Device/DumpInfo
>  Target-Profile: DEVICE_$(1)
>  Target-Profile-Name: $(DEVICE_DISPLAY)
> +Target-Profile-Devicetype: $(DEVICE_TYPE)
>  Target-Profile-Packages: $(DEVICE_PACKAGES)
>  Target-Profile-hasImageMetadata: $(if $(foreach
> image,$(IMAGES),$(findstring append-metadata,$(IMAGE/$(image)))),1,0)
>  Target-Profile-SupportedDevices: $(SUPPORTED_DEVICES) diff --git
> a/include/target.mk b/include/target.mk index 9bd4c14936c1..e6f26cbfdf3d
> 100644
> --- a/include/target.mk
> +++ b/include/target.mk
> @@ -73,6 +73,7 @@ define Profile
>  	echo "Target-Profile: $(1)"; \
>  	$(if $(PRIORITY), echo "Target-Profile-Priority: $(PRIORITY)"; ) \
>  	echo "Target-Profile-Name: $(NAME)"; \
> +	echo "Target-Profile-Devicetype: $(DEVICE_TYPE)"; \
>  	echo "Target-Profile-Packages: $(PACKAGES) $(call
> extra_packages,$(DEFAULT_PACKAGES) $(PACKAGES))"; \
>  	echo "Target-Profile-Description:"; \
>  	echo "$$$$$$$$$(call shvar,Profile/$(1)/Description)"; \ diff --git
> a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile index
> 01441d1e87d1..f504117f60f3 100644
> --- a/package/utils/busybox/Makefile
> +++ b/package/utils/busybox/Makefile
> @@ -94,7 +94,7 @@ endif
>  define Build/Configure
>  	rm -f $(PKG_BUILD_DIR)/.config
>  	touch $(PKG_BUILD_DIR)/.config
> -ifeq ($(DEVICE_TYPE),nas)
> +ifeq ($(CONFIG_DEVICE_TYPE_NAS),y)
>  	echo "CONFIG_HDPARM=y" >> $(PKG_BUILD_DIR)/.config  endif
>  	grep 'CONFIG_BUSYBOX_$(BUSYBOX_SYM)' $(TOPDIR)/.config | sed
> -e "s,\\(#
> \)\\?CONFIG_BUSYBOX_$(BUSYBOX_SYM)_\\(.*\\),\\1CONFIG_\\2,g" >>
> $(PKG_BUILD_DIR)/.config diff --git a/scripts/metadata.pm
> b/scripts/metadata.pm index 1826a040a116..5062dba37ec0 100644
> --- a/scripts/metadata.pm
> +++ b/scripts/metadata.pm
> @@ -140,6 +140,7 @@ sub parse_target_metadata($) {
>  			$profile = {
>  				id => $1,
>  				name => $1,
> +				device_type => "router",
>  				has_image_metadata => 0,
>  				supported_devices => [],
>  				priority => 999,
> @@ -150,6 +151,7 @@ sub parse_target_metadata($) {
>  			push @{$target->{profiles}}, $profile;
>  		};
>  		/^Target-Profile-Name:\s*(.+)\s*$/ and $profile->{name} =
> $1;
> +		/^Target-Profile-Devicetype:\s*(.+)\s*$/ and $profile-
> >{device_type}
> += $1;
>  		/^Target-Profile-hasImageMetadata:\s*(\d+)\s*$/ and
> $profile->{has_image_metadata} = $1;
>  		/^Target-Profile-SupportedDevices:\s*(.+)\s*$/ and $profile-
> >{supported_devices} = [ split(/\s+/, $1) ];
>  		/^Target-Profile-Priority:\s*(\d+)\s*$/ and do { diff --git
> a/scripts/target-metadata.pl b/scripts/target-metadata.pl index
> ee0ab5a71811..fbd9fa70c08b 100755
> --- a/scripts/target-metadata.pl
> +++ b/scripts/target-metadata.pl
> @@ -244,6 +244,12 @@ EOF
>  				print "\tselect DEFAULT_$pkg\n";
>  				$defaults{$pkg} = 1;
>  			}
> +			if ($profile->{device_type} =~ "router") {
> +				print "\tselect DEVICE_TYPE_ROUTER\n";
> +			}
> +			if ($profile->{device_type} =~ "nas") {
> +				print "\tselect DEVICE_TYPE_NAS\n";
> +			}
>  			my $help = $profile->{desc};
>  			if ($help =~ /\w+/) {
>  				$help =~ s/^\s*/\t  /mg;
> @@ -328,6 +334,12 @@ config HAS_SUBTARGETS  config HAS_DEVICES
>  	bool
> 
> +config DEVICE_TYPE_ROUTER
> +	bool
> +
> +config DEVICE_TYPE_NAS
> +	bool
> +
>  config TARGET_BOARD
>  	string
> 
> --
> 2.26.2
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20200529/7e6f477d/attachment.sig>
-------------- next part --------------
_______________________________________________
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