| Message ID | 20230902-bcm53xx-dir890l-fix-v1-1-287f074c3fed@linaro.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | bcm53xx: Unconditionally build U-Boot for DIR-890L | expand |
On 2023-09-02 18:33, Linus Walleij wrote: > Building the DIR-890L image requires U-Boot to be built > so list it as a device package. > > Reported-by: Arınç ÜNAL <arinc.unal@arinc9.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > Missed to enforce U-Boot build. Mea Culpa. > --- > target/linux/bcm53xx/image/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/linux/bcm53xx/image/Makefile > b/target/linux/bcm53xx/image/Makefile > index 39b7efbef789..9483812facca 100644 > --- a/target/linux/bcm53xx/image/Makefile > +++ b/target/linux/bcm53xx/image/Makefile > @@ -283,7 +283,7 @@ TARGET_DEVICES += dlink_dir-885l > define Device/dlink_dir-890l > DEVICE_VENDOR := D-Link > DEVICE_MODEL := DIR-890L > - DEVICE_PACKAGES := $(BRCMFMAC_43602A1) $(USB2_PACKAGES) > $(USB3_PACKAGES) > + DEVICE_PACKAGES := uboot-bcm53xx $(BRCMFMAC_43602A1) > $(USB2_PACKAGES) $(USB3_PACKAGES) > # Layout: U-boot (128kb max) followed by kernel and appended DTB. > # This is done because the boot loader will only read the first 2 MB > # from the flash and decompress the LZMA it finds there after the This may work as a workaround (I didn't try it) but it doesn't seem like a proper solution. DEVICE_PACKAGES should be used for listing packages to be included in device's rootfs. I don't see anything incorrect with Linus's original patch. Maybe we just have some dependency handling issue in u-boot generic .mk code? Is this issue bcm53xx specific? I think other targets use U-Boot in a similar way.
On Mon, Sep 4, 2023 at 12:03 PM Rafał Miłecki <rafal@milecki.pl> wrote: > I don't see anything incorrect with Linus's original patch. Maybe we > just have some dependency handling issue in u-boot generic .mk code? I have transient problem with the dependencies at times, then I just chose another target (entirely different!) in menuconfig and then back to the intended target. Problem fixed, dependency selected. It feels a bit shaky though, but usually all autobuilds work fine. Yours, Linus Walleij
On 4.09.2023 23:58, Linus Walleij wrote: > On Mon, Sep 4, 2023 at 12:03 PM Rafał Miłecki <rafal@milecki.pl> wrote: > >> I don't see anything incorrect with Linus's original patch. Maybe we >> just have some dependency handling issue in u-boot generic .mk code? > > I have transient problem with the dependencies at times, then > I just chose another target (entirely different!) in menuconfig and > then back to the intended target. Problem fixed, dependency > selected. It feels a bit shaky though, but usually all autobuilds > work fine. Perhaps I wasn't clear enough to explain the issue. As Rafał has already said, there's nothing wrong with enabling the package when DIR-890L is selected as the device. The problem is when another device is selected. OpenWrt SDK shouldn't compile an image for the devices that are not selected on menuconfig. Yet it does anyway. This is the first time on the bcm53xx target that compiling the image for a device requires a package to be built first. This has exposed an underlying issue. When a device that is not DIR-890L is selected, the u-boot package won't be enabled. An image for DIR-890L will be attempted to be compiled which will fail because the u-boot package is not enabled. To make it clear, what needs fixing is making OpenWrt SDK not compile an image for the devices that are not selected on menuconfig. I don't know GNU Make very well to figure out why this happens on the bcm53xx target. I don't see this happening on the mt7621 subtarget of the ramips target. Arınç
On Mon, 4 Sept 2023 at 23:00, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Mon, Sep 4, 2023 at 12:03 PM Rafał Miłecki <rafal@milecki.pl> wrote: > > > I don't see anything incorrect with Linus's original patch. Maybe we > > just have some dependency handling issue in u-boot generic .mk code? > > I have transient problem with the dependencies at times, then > I just chose another target (entirely different!) in menuconfig and > then back to the intended target. Problem fixed, dependency > selected. It feels a bit shaky though, but usually all autobuilds > work fine. You might want to set HIDDEN for the u-boot packages. Then they aren't a user selectable package (config symbol) anymore, and their selection state will follow the defaults automatically. Should avoid the u-boot packages (not) being built/selected when changing the device. Best Regards, Jonas
On Tue, Sep 5, 2023 at 10:27 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote: > To make it clear, what needs fixing is making OpenWrt SDK not compile an > image for the devices that are not selected on menuconfig. You're right it does, that's really weird! The BMIPS target doesn't behave like this at all. Yours, Linus Walleij
On Sat, Sep 2, 2023 at 6:33 PM Linus Walleij <linus.walleij@linaro.org> wrote: > Building the DIR-890L image requires U-Boot to be built > so list it as a device package. > > Reported-by: Arınç ÜNAL <arinc.unal@arinc9.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Scratch this, Rani has made a proper solution to the problem, see: https://github.com/openwrt/openwrt/pull/13577 Please review/merge. Yours, Linus Walleij
diff --git a/target/linux/bcm53xx/image/Makefile b/target/linux/bcm53xx/image/Makefile index 39b7efbef789..9483812facca 100644 --- a/target/linux/bcm53xx/image/Makefile +++ b/target/linux/bcm53xx/image/Makefile @@ -283,7 +283,7 @@ TARGET_DEVICES += dlink_dir-885l define Device/dlink_dir-890l DEVICE_VENDOR := D-Link DEVICE_MODEL := DIR-890L - DEVICE_PACKAGES := $(BRCMFMAC_43602A1) $(USB2_PACKAGES) $(USB3_PACKAGES) + DEVICE_PACKAGES := uboot-bcm53xx $(BRCMFMAC_43602A1) $(USB2_PACKAGES) $(USB3_PACKAGES) # Layout: U-boot (128kb max) followed by kernel and appended DTB. # This is done because the boot loader will only read the first 2 MB # from the flash and decompress the LZMA it finds there after the
Building the DIR-890L image requires U-Boot to be built so list it as a device package. Reported-by: Arınç ÜNAL <arinc.unal@arinc9.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- Missed to enforce U-Boot build. Mea Culpa. --- target/linux/bcm53xx/image/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: 02214ab8dce59ee6b599f8dfdacb0297dc5dcc24 change-id: 20230902-bcm53xx-dir890l-fix-6fec290afa97 Best regards,