Hi Niklas,
On 1/17/25 11:54 AM, Niklas Cassel wrote:
On Thu, Jan 16, 2025 at 03:47:11PM +0100, Quentin Schulz wrote:
From: Quentin Schulz <quentin.schulz@xxxxxxxxx>
According to commit 40658534756f ("arm64: dts: rockchip: Add rock5b
overlays for PCIe endpoint mode"), Rock 5B can operate in PCIe endpoint
mode. For that to work, the rk3588-rock-5b-pcie-ep.dtbo overlay needs to
be applied on Rock 5B base Device Tree. If that Rock 5B is connected to
another Rock 5B, the latter needs to apply the
rk3588-rock-5b-pcie-srns.dtbo overlay.
In order to make sure the overlays are still valid in the future, let's
add a validation test by applying the overlays on top of the main base
at build time.
Fixes: 40658534756f ("arm64: dts: rockchip: Add rock5b overlays for PCIe endpoint mode")
Not sure if I agree with the Fixes-tag.
I don't think there is anything broken with that commit, but I definitely
think that it is a good idea make sure that they don't break in the future.
That's fair. I actually think it'd be a good idea to backport this to
stable releases. It could be possible for stable right now to somehow
only backport changes to the base DT without modifying the DTO (or
vice-versa) and then break the overlay unknowingly.
I added the Fixes to make it a bit simpler to know up to when to
backport it, though I still haven't decided if I should have added Cc:
stable there.
So 1) what's your opinion on backporting this to stable
2) if backporting, shouldn't I still remove the Fixes?
@@ -145,8 +145,6 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-orangepi-5-plus.dtb
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-quartzpro64.dtb
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5-itx.dtb
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b.dtb
-dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-ep.dtbo
-dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-srns.dtbo
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-tiger-haikou.dtb
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-toybrick-x0.dtb
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-turing-rk1.dtb
@@ -165,5 +163,9 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5c.dtb
# Overlays
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-ep.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-srns.dtb
You moved these lines further down, but you changed the suffix from
.dtbo to .dtb, why? It seems a little confusing, is that really needed?
That was also confusing to me, but that's actually how it works.
rk3588-rock-5b-pcie-ep.dtb somehow points at rk3588-rock-5b-pcie-ep-dtbs
which is the overlay application test of rk3588-rock-5b-pcie-ep.dtbo
onto rk3588-rock-5b.dtb. For that to work, the .dtbo needs to be
compiled. The target must be auto-generated somewhere because that still
works. You can remove all *.dtb* from the tree, apply this patch and
compile with make dtbs and you'll see that the DTBO is generated and the
build time test of overlay application done as well (the log line starts
with OVL).
Not sure exactly how to make this a bit less confusing in the commit
log, as I myself do not really know why that is necessary or how it is
working. But "it works" (tm).
This matches what was done for ti/ as well.
Cheers,
Quentin
