[OpenWrt-Devel] ipq40xx: fritz4040 depends on uboot-fritz4040
diff mbox series

Message ID 20191109225217.19505-1-devel-sven@geroedel.de
State Changes Requested, archived
Headers show
Series
  • [OpenWrt-Devel] ipq40xx: fritz4040 depends on uboot-fritz4040
Related show

Commit Message

Sven Roederer Nov. 9, 2019, 10:52 p.m. UTC
The "append-uboot" macro is looking for the file "$(STAGING_DIR_IMAGE)/uboot-fritz4040.bin"
which is provided by the u-boot-fritz4040 package. If this is not build, image creation
will fail with "file not found".

Signed-off-by: Sven Roederer <devel-sven@geroedel.de>
---
 target/linux/ipq40xx/image/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

David Bauer Nov. 9, 2019, 11:20 p.m. UTC | #1
Hello Sven,

On 11/9/19 11:52 PM, Sven Roederer wrote:
> The "append-uboot" macro is looking for the file "$(STAGING_DIR_IMAGE)/uboot-fritz4040.bin"
> which is provided by the u-boot-fritz4040 package. If this is not build, image creation
> will fail with "file not found".

The dependency comes from the U-Boot package [0], so it should be pulled in automatically.
Maybe you are missing this commit [1]?

[0] https://github.com/openwrt/openwrt/blob/master/package/boot/uboot-fritz4040/Makefile#L31
[1] https://github.com/openwrt/openwrt/commit/40e3f660c1c0f400092cce09feb8c13bec97caeb

Best wishes
David

> 
> Signed-off-by: Sven Roederer <devel-sven@geroedel.de>
> ---
>  target/linux/ipq40xx/image/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/linux/ipq40xx/image/Makefile b/target/linux/ipq40xx/image/Makefile
> index a9c5e625af..7ae6f36baa 100644
> --- a/target/linux/ipq40xx/image/Makefile
> +++ b/target/linux/ipq40xx/image/Makefile
> @@ -130,7 +130,7 @@ define Device/avm_fritzbox-4040
>  	IMAGES = eva.bin sysupgrade.bin
>  	IMAGE/eva.bin := append-uboot | pad-to $$$$(UBOOT_PARTITION_SIZE) | append-kernel | append-rootfs | pad-rootfs
>  	IMAGE/sysupgrade.bin := append-kernel | append-rootfs | pad-rootfs | append-metadata
> -	DEVICE_PACKAGES := fritz-tffs fritz-caldata
> +	DEVICE_PACKAGES := fritz-tffs fritz-caldata u-boot-fritz4040
>  endef
>  TARGET_DEVICES += avm_fritzbox-4040
>  
>
Christian Lamparter Nov. 10, 2019, 12:10 a.m. UTC | #2
On Saturday, 9 November 2019 23:52:17 CET Sven Roederer wrote:
> The "append-uboot" macro is looking for the file "$(STAGING_DIR_IMAGE)/uboot-fritz4040.bin"
> which is provided by the u-boot-fritz4040 package. If this is not build, image creation
> will fail with "file not found".

The FRITZ!Box 4040 is a special case as the "eva.bin" image file needs the
u-boot binary to work as intended.

In OpenWrt's u-boot.mk, the u-boots for a target are usually get built/enabled
because <https://github.com/openwrt/openwrt/blob/master/include/u-boot.mk#L63>
adds something like this to the u-boot packages:
|Default: y if (TARGET_ipq40xx_generic_Default || TARGET_ipq40xx_generic_DEVICE_avm_fritzbox-4040 || TARGET_DEVICE_ipq40xx_generic_DEVICE_avm_fritzbox-4040)

This causes the needed u-boot package to be included automatically.
Unless the package is either deselected actively, or accidentally.
(This can happen when a existing .config in which the device was
previously disabled, is grandfathered in. Because the existing
"not set" will take preference [the diffconfig.pl will catch this]).

Technically, the eva.bin isn't necessary. It allows for an easier
installation, but nobody apart from the 4040 uses the append-uboot
and there have been issues in the past with this as well.

Question is: should we add that dependency, ditch the eva.bin image
(or make it so that it's optional - this requires some changes to
the builtsystem) or decide that "this is a bug elsewhere"?

Cheers,
Christian
> ---
>  target/linux/ipq40xx/image/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/linux/ipq40xx/image/Makefile b/target/linux/ipq40xx/image/Makefile
> index a9c5e625af..7ae6f36baa 100644
> --- a/target/linux/ipq40xx/image/Makefile
> +++ b/target/linux/ipq40xx/image/Makefile
> @@ -130,7 +130,7 @@ define Device/avm_fritzbox-4040
>  	IMAGES = eva.bin sysupgrade.bin
>  	IMAGE/eva.bin := append-uboot | pad-to $$$$(UBOOT_PARTITION_SIZE) | append-kernel | append-rootfs | pad-rootfs
>  	IMAGE/sysupgrade.bin := append-kernel | append-rootfs | pad-rootfs | append-metadata
> -	DEVICE_PACKAGES := fritz-tffs fritz-caldata
> +	DEVICE_PACKAGES := fritz-tffs fritz-caldata u-boot-fritz4040
David Bauer Nov. 10, 2019, 8:25 a.m. UTC | #3
Hello Christian,

On 11/10/19 1:10 AM, Christian Lamparter wrote:
> Technically, the eva.bin isn't necessary. It allows for an easier
> installation, but nobody apart from the 4040 uses the append-uboot
> and there have been issues in the past with this as well.
> 
> Question is: should we add that dependency, ditch the eva.bin image
> (or make it so that it's optional - this requires some changes to
> the builtsystem) or decide that "this is a bug elsewhere"?

I definitely wouldn't ditch it, as the installation process is greatly
simplified with it. Regarding the dependency of the U-Boot on the device -
I'm not quite sure if this is the way to take, as we originally moved it
from there to the bootloader itself. But maybe Sven can give us a hint how
he stumbled upon this issue ;)

Best wishes
David

> 
> Cheers,
> Christian
>> ---
>>  target/linux/ipq40xx/image/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/linux/ipq40xx/image/Makefile b/target/linux/ipq40xx/image/Makefile
>> index a9c5e625af..7ae6f36baa 100644
>> --- a/target/linux/ipq40xx/image/Makefile
>> +++ b/target/linux/ipq40xx/image/Makefile
>> @@ -130,7 +130,7 @@ define Device/avm_fritzbox-4040
>>  	IMAGES = eva.bin sysupgrade.bin
>>  	IMAGE/eva.bin := append-uboot | pad-to $$$$(UBOOT_PARTITION_SIZE) | append-kernel | append-rootfs | pad-rootfs
>>  	IMAGE/sysupgrade.bin := append-kernel | append-rootfs | pad-rootfs | append-metadata
>> -	DEVICE_PACKAGES := fritz-tffs fritz-caldata
>> +	DEVICE_PACKAGES := fritz-tffs fritz-caldata u-boot-fritz4040
> 
>

Patch
diff mbox series

diff --git a/target/linux/ipq40xx/image/Makefile b/target/linux/ipq40xx/image/Makefile
index a9c5e625af..7ae6f36baa 100644
--- a/target/linux/ipq40xx/image/Makefile
+++ b/target/linux/ipq40xx/image/Makefile
@@ -130,7 +130,7 @@  define Device/avm_fritzbox-4040
 	IMAGES = eva.bin sysupgrade.bin
 	IMAGE/eva.bin := append-uboot | pad-to $$$$(UBOOT_PARTITION_SIZE) | append-kernel | append-rootfs | pad-rootfs
 	IMAGE/sysupgrade.bin := append-kernel | append-rootfs | pad-rootfs | append-metadata
-	DEVICE_PACKAGES := fritz-tffs fritz-caldata
+	DEVICE_PACKAGES := fritz-tffs fritz-caldata u-boot-fritz4040
 endef
 TARGET_DEVICES += avm_fritzbox-4040