Hi Dragan,
On 2/4/25 2:35 PM, Dragan Simic wrote:
Hello Quentin,
On 2025-02-04 13:20, Quentin Schulz wrote:
On 2/4/25 12:22 PM, Dragan Simic wrote:
> On 2025-01-31 11:40, Quentin Schulz wrote:
Not discussing CONFIG_OF_ALL_DTBS relevancy wrt hiding overlay tests
behind, unrelated to this series I believe :)
[...]
With the above-proposed changes in place, and with CONFIG_OF_ALL_DTBS
selected, the relevant part of the "make dtbs" output looks like this:
DTC arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtb
DTC arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtbo
DTC arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-srns.dtbo
OVL arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-ep.dtb
OVL arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-srns.dtb
No more "phony targets" in the produced output. :)
Funnily enough, I would prefer to see OVL for overlays rather than
DTC, but I guess it's just one more occurrence of developers
disagreeing on how to name things :)
I actually agree with that, just like I prefer to see .dtbo files
as additions to dtb-$(CONFIG_ARCH_XYZ). It's all about the overlays,
so they should be both specified and echoed back.
Moreover, we currently also have additional .dtb files with applied
overlays left after the build and installed afterwards, which doesn't
make much sense to me. To me, those additional .dtb files should be
deleted as build artefacts and not installed.
I **think** it could be useful for systems without overlay support. Then
you have a dtb which is the result of an overlay applied on top of the
base dtb and you can replace your previous dtb with that one, and voilà.
What I don't like is that it's difficult to differentiate them from the
"normal" base DTB or even from the DTBO (simple base DTB + overlay test
is usually named after the overlay, and in the case of the Rock 5B test:
rk3588-rock-5b-pcie-srns.dtbo and
arch/arm64/boot/dts/rockchip/rk3588-rock-5b-pcie-srns.dtb), easy to pick
the wrong one. Though that is on **me** as I could pick another name for
the overlay test and e.g. prepend "test-ovl_" to the filename for example.
[...]
I won't be too difficult to convince here, just want some "authority"
or a piece of history about CONFIG_OF_ALL_DTBS that would go your
direction, before doing the change. I believe automated build tests
without needing to enable a symbol, and that taking DTB and DTBO from
the build output and apply DTBO on top of DTB works without having to
go through some length to get the symbols, are good reasons to keep it
the way it is in this patch series.
I'd like the most to perform the above-proposed "divorcing" of the DT
overlay tests from CONFIG_OF_ALL_DTBS, so we don't have to enable any
additional options to have the overlay tests run automatically, but
to keep .dtbo filenames in dtb-$(CONFIG_ARCH_XYZ). I think that would
bring the best of both worlds, so to speak.
So, just to recap:
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5.dtb
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-display-vz.dtbo
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo
stays and I add:
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-vz-2-uhd.dtb
rk3568-wolfvision-pf5-vz-2-uhd-dtbs := rk3568-wolfvision-pf5.dtb \
rk3568-wolfvision-pf5-display-vz.dtbo \
rk3568-wolfvision-pf5-io-expander.dtbo
at the bottom of the Makefile. I specifically do NOT want to make this
depend on CONFIG_OF_ALL_DTBS (by using dtb- like in ti/), so that the
base DTB will always have the symbols in, regardless of CONFIG_OF_ALL_DTBS.
I think the redundancy is unnecessary but I guess it's worth getting
away from implicit rules.
I can compromise on that :)
@Heiko does this work for you?
Cheers,
Quentin
