| Message ID | 20250116-pre-ict-jaguar-v2-1-157d319004fc@cherry.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests | expand |
On 16/01/2025 15:47, Quentin Schulz wrote: > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile > index 86cc418a2255cdc22f1d503e9519d2d9572d4e9d..3f92d1a9d0b6efeee46ad4aff8c2b8ed3cb83d73 100644 > --- a/arch/arm64/boot/dts/rockchip/Makefile > +++ b/arch/arm64/boot/dts/rockchip/Makefile > @@ -134,7 +134,6 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-w3.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-coolpi-cm5-evb.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-coolpi-cm5-genbook.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-io.dtb > -dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtbo > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6b-io.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-evb1-v10.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-friendlyelec-cm3588-nas.dtb > @@ -163,3 +162,8 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5b.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5a.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5c.dtb > + > +# Overlays > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtb > + > +rk3588-edgeble-neu6a-wifi-dtbs := rk3588-edgeble-neu6a-io.dtb rk3588-edgeble-neu6a-wifi.dtbo I think usually these two lines are reversed, so first you define what is rk3588-edgeble-neu6a-wifi.dtb and then use it in dtb-rockchip. But that might be just convention, so this is also fine for me: Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
Hi Quentin, On 1/16/25 15:47, Quentin Schulz wrote: > From: Quentin Schulz <quentin.schulz@cherry.de> > > The Edgeble NCM6A can have WiFi modules connected and this is handled > via an overlay (commit 951d6aaa37fe ("arm64: dts: rockchip: Add Edgeble > NCM6A WiFi6 Overlay")). > > In order to make sure the overlay is still valid in the future, let's > add a validation test by applying the overlay on top of the main base > at build time. > > Fixes: 951d6aaa37fe ("arm64: dts: rockchip: Add Edgeble NCM6A WiFi6 Overlay") > Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> > --- > arch/arm64/boot/dts/rockchip/Makefile | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile > index 86cc418a2255cdc22f1d503e9519d2d9572d4e9d..3f92d1a9d0b6efeee46ad4aff8c2b8ed3cb83d73 100644 > --- a/arch/arm64/boot/dts/rockchip/Makefile > +++ b/arch/arm64/boot/dts/rockchip/Makefile > @@ -134,7 +134,6 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-w3.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-coolpi-cm5-evb.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-coolpi-cm5-genbook.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-io.dtb > -dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtbo > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6b-io.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-evb1-v10.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-friendlyelec-cm3588-nas.dtb > @@ -163,3 +162,8 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5b.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5a.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5c.dtb > + > +# Overlays > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtb > + Maybe open a new section "# Compile time tests" or something like that? > +rk3588-edgeble-neu6a-wifi-dtbs := rk3588-edgeble-neu6a-io.dtb rk3588-edgeble-neu6a-wifi.dtbo > Thanks and regards, Michael
Hi Michael, On 1/20/25 10:07 AM, Michael Riesch wrote: > Hi Quentin, > > On 1/16/25 15:47, Quentin Schulz wrote: >> From: Quentin Schulz <quentin.schulz@cherry.de> >> >> The Edgeble NCM6A can have WiFi modules connected and this is handled >> via an overlay (commit 951d6aaa37fe ("arm64: dts: rockchip: Add Edgeble >> NCM6A WiFi6 Overlay")). >> >> In order to make sure the overlay is still valid in the future, let's >> add a validation test by applying the overlay on top of the main base >> at build time. >> >> Fixes: 951d6aaa37fe ("arm64: dts: rockchip: Add Edgeble NCM6A WiFi6 Overlay") >> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> >> --- >> arch/arm64/boot/dts/rockchip/Makefile | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile >> index 86cc418a2255cdc22f1d503e9519d2d9572d4e9d..3f92d1a9d0b6efeee46ad4aff8c2b8ed3cb83d73 100644 >> --- a/arch/arm64/boot/dts/rockchip/Makefile >> +++ b/arch/arm64/boot/dts/rockchip/Makefile >> @@ -134,7 +134,6 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-w3.dtb >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-coolpi-cm5-evb.dtb >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-coolpi-cm5-genbook.dtb >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-io.dtb >> -dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtbo >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6b-io.dtb >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-evb1-v10.dtb >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-friendlyelec-cm3588-nas.dtb >> @@ -163,3 +162,8 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5.dtb >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5b.dtb >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5a.dtb >> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5c.dtb >> + >> +# Overlays >> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtb >> + > > Maybe open a new section "# Compile time tests" or something like that? > The above line is to compile the build-time test of overlay application (notice the missing o in the extension). This points at the target below (which ends with -dtbs), which does require the dtbo to exist. So essentially, they are both for the build-time test of applying (and generating) DTBO. I feel like this comment/section would add to the confusion? I may have misunderstood what you are suggesting, can you provide an example? Cheers, Quentin
Hi Quentin, On 1/20/25 10:23, Quentin Schulz wrote: > Hi Michael, > > On 1/20/25 10:07 AM, Michael Riesch wrote: >> Hi Quentin, >> >> On 1/16/25 15:47, Quentin Schulz wrote: >>> From: Quentin Schulz <quentin.schulz@cherry.de> >>> >>> The Edgeble NCM6A can have WiFi modules connected and this is handled >>> via an overlay (commit 951d6aaa37fe ("arm64: dts: rockchip: Add Edgeble >>> NCM6A WiFi6 Overlay")). >>> >>> In order to make sure the overlay is still valid in the future, let's >>> add a validation test by applying the overlay on top of the main base >>> at build time. >>> >>> Fixes: 951d6aaa37fe ("arm64: dts: rockchip: Add Edgeble NCM6A WiFi6 >>> Overlay") >>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> >>> --- >>> arch/arm64/boot/dts/rockchip/Makefile | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm64/boot/dts/rockchip/Makefile >>> b/arch/arm64/boot/dts/rockchip/Makefile >>> index >>> 86cc418a2255cdc22f1d503e9519d2d9572d4e9d..3f92d1a9d0b6efeee46ad4aff8c2b8ed3cb83d73 100644 >>> --- a/arch/arm64/boot/dts/rockchip/Makefile >>> +++ b/arch/arm64/boot/dts/rockchip/Makefile >>> @@ -134,7 +134,6 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-w3.dtb >>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-coolpi-cm5-evb.dtb >>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-coolpi-cm5-genbook.dtb >>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-io.dtb >>> -dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtbo >>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6b-io.dtb >>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-evb1-v10.dtb >>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-friendlyelec-cm3588-nas.dtb >>> @@ -163,3 +162,8 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += >>> rk3588s-orangepi-5.dtb >>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5b.dtb >>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5a.dtb >>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5c.dtb >>> + >>> +# Overlays >>> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtb >>> + >> >> Maybe open a new section "# Compile time tests" or something like that? >> > > The above line is to compile the build-time test of overlay application > (notice the missing o in the extension). This points at the target below > (which ends with -dtbs), which does require the dtbo to exist. So > essentially, they are both for the build-time test of applying (and > generating) DTBO. I feel like this comment/section would add to the > confusion? I may have misunderstood what you are suggesting, can you > provide an example? Thanks for the explanation. At the beginning I was wondering what the point of this line was, and thought that a comment that explains the purpose of it would be beneficial. Maybe it makes sense to provide a section so that other contributors know where to sort in their tests, so maybe # Overlays dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtb [...] # Compile-time tests for overlays (and combinations thereof) rk3588-edgeble-neu6a-wifi-dtbs := rk3588-edgeble-neu6a-io.dtb rk3588-edgeble-neu6a-wifi.dtbo [...] But this is simply a recommendation. Regards, Michael
Am Montag, 20. Januar 2025, 11:34:25 CET schrieb Michael Riesch: > >> Maybe open a new section "# Compile time tests" or something like that? > >> > > > > The above line is to compile the build-time test of overlay application > > (notice the missing o in the extension). This points at the target below > > (which ends with -dtbs), which does require the dtbo to exist. So > > essentially, they are both for the build-time test of applying (and > > generating) DTBO. I feel like this comment/section would add to the > > confusion? I may have misunderstood what you are suggesting, can you > > provide an example? > > Thanks for the explanation. At the beginning I was wondering what the > point of this line was, and thought that a comment that explains the > purpose of it would be beneficial. > > Maybe it makes sense to provide a section so that other contributors > know where to sort in their tests, so maybe > > # Overlays > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtb > [...] > > # Compile-time tests for overlays (and combinations thereof) > rk3588-edgeble-neu6a-wifi-dtbs := rk3588-edgeble-neu6a-io.dtb > rk3588-edgeble-neu6a-wifi.dtbo > [...] I do feel that both parts belong to each other, and we're reading from top, so personally I'd go with Krzysztof's suggestion. # Overlays rk3588-edgeble-neu6a-wifi-dtbs := rk3588-edgeble-neu6a-io.dtb rk3588-edgeble-neu6a-wifi.dtbo dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtb Having separate blocks for overlays and the description of the building blocks just causes the reader to jump up and down between sections, especially once those parts become larger, so please keep things together. Heiko
diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile index 86cc418a2255cdc22f1d503e9519d2d9572d4e9d..3f92d1a9d0b6efeee46ad4aff8c2b8ed3cb83d73 100644 --- a/arch/arm64/boot/dts/rockchip/Makefile +++ b/arch/arm64/boot/dts/rockchip/Makefile @@ -134,7 +134,6 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-w3.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-coolpi-cm5-evb.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-coolpi-cm5-genbook.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-io.dtb -dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtbo dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6b-io.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-evb1-v10.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-friendlyelec-cm3588-nas.dtb @@ -163,3 +162,8 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5b.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5a.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5c.dtb + +# Overlays +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtb + +rk3588-edgeble-neu6a-wifi-dtbs := rk3588-edgeble-neu6a-io.dtb rk3588-edgeble-neu6a-wifi.dtbo