[PATCH v2 0/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests

[PATCH v2 0/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests

[Quentin Schulz]

This adds minimal support for the Pre-ICT tester adapter for RK3588
Jaguar.
GPIO3A3, GPIO3A4, GPIO3B2 and GPIO3D2 to GPIO3D5 are all routed to power
rails and can only be used as input and their bias are important to be
able to properly detect soldering issues.

Additionally, this adds build-time overlay application tests for (some)
Rockchip overlays to try to avoid future regressions.

Notably, the Device Trees from Wolfvision PF5 aren't migrated (but
should) as I do not own the device and couldn't figure out from the
introducing commit logs what the possible valid combinations are.
+Cc Michael Riesch for awareness

I'm wondering if we shouldn't backport patches 1 and 2 to stable? In
which case, it would make sense to try to have the Wolfvision PF5
overlay tests merged before the addition of the Pre-ICT tester adapter
support for RK3588 Jaguar as that one won't be backported to stable and
backporting the Wolfvision overlay test would incur an unnecessary
(though not difficult) git conflict to resolve.

I also do not know what kind of tests we should have when overlay
combinations are possible (let's say there are A, B and C overlays that
can all be applied, should we have base + A, base + B, base + C,
base + A + B, base + A + C, base + B + C and base + A + B + C tests?
maybe even base + B + A, base + C + B, base A + C + B, base + B + A + C,
base + B + C + A, base + C + B + A and base + C + A + B tests?).

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
Changes in v2:
- add overlay application tests for Edgeble NCM6A WiFi and Rock 5B PCIe
  Endpoint+SNRS
- add overlay application test for RK3588 Jaguar + Pre-ICT tester
  adapter,
- Link to v1: https://lore.kernel.org/r/20241206-pre-ict-jaguar-v1-1-7f660bd4b70c@cherry.de

---
Quentin Schulz (3):
      arm64: dts: rockchip: add overlay test for Edgeble NCM6A
      arm64: dts: rockchip: add overlay tests for Rock 5B PCIe overlays
      arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar

 arch/arm64/boot/dts/rockchip/Makefile              |  14 +-
 .../dts/rockchip/rk3588-jaguar-pre-ict-tester.dtso | 171 +++++++++++++++++++++
 2 files changed, 182 insertions(+), 3 deletions(-)
---
base-commit: 619f0b6fad524f08d493a98d55bac9ab8895e3a6
change-id: 20241206-pre-ict-jaguar-b90fafee8bd8

Best regards,
-- 
Quentin Schulz <quentin.schulz@cherry.de>

[PATCH v2 1/3] arm64: dts: rockchip: add overlay test for Edgeble NCM6A

[Quentin Schulz]

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
+
+rk3588-edgeble-neu6a-wifi-dtbs := rk3588-edgeble-neu6a-io.dtb rk3588-edgeble-neu6a-wifi.dtbo

-- 
2.48.0

Re: [PATCH v2 1/3] arm64: dts: rockchip: add overlay test for Edgeble NCM6A

[Krzysztof Kozlowski]

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

Re: [PATCH v2 1/3] arm64: dts: rockchip: add overlay test for Edgeble NCM6A

[Michael Riesch]

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

Re: [PATCH v2 1/3] arm64: dts: rockchip: add overlay test for Edgeble NCM6A

[Quentin Schulz]

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

Re: [PATCH v2 1/3] arm64: dts: rockchip: add overlay test for Edgeble NCM6A

[Michael Riesch]

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

Re: [PATCH v2 1/3] arm64: dts: rockchip: add overlay test for Edgeble NCM6A

[Heiko Stübner]

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

[PATCH v2 2/3] arm64: dts: rockchip: add overlay tests for Rock 5B PCIe overlays

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

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")
Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 arch/arm64/boot/dts/rockchip/Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index 3f92d1a9d0b6efeee46ad4aff8c2b8ed3cb83d73..72f8666b42400a2f462421f5db6b3cb0cf369611 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -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
 
 rk3588-edgeble-neu6a-wifi-dtbs := rk3588-edgeble-neu6a-io.dtb rk3588-edgeble-neu6a-wifi.dtbo
+rk3588-rock-5b-pcie-ep-dtbs := rk3588-rock-5b.dtb rk3588-rock-5b-pcie-ep.dtbo
+rk3588-rock-5b-pcie-srns-dtbs := rk3588-rock-5b.dtb rk3588-rock-5b-pcie-srns.dtbo

-- 
2.48.0

Re: [PATCH v2 2/3] arm64: dts: rockchip: add overlay tests for Rock 5B PCIe overlays

[Niklas Cassel]

On Thu, Jan 16, 2025 at 03:47:11PM +0100, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> 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.


> @@ -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?


Kind regards,
Niklas

>  
>  rk3588-edgeble-neu6a-wifi-dtbs := rk3588-edgeble-neu6a-io.dtb rk3588-edgeble-neu6a-wifi.dtbo
> +rk3588-rock-5b-pcie-ep-dtbs := rk3588-rock-5b.dtb rk3588-rock-5b-pcie-ep.dtbo
> +rk3588-rock-5b-pcie-srns-dtbs := rk3588-rock-5b.dtb rk3588-rock-5b-pcie-srns.dtbo
> 
> -- 
> 2.48.0
> 

Re: [PATCH v2 2/3] arm64: dts: rockchip: add overlay tests for Rock 5B PCIe overlays

[Quentin Schulz]

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@cherry.de>
>>
>> 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

Re: [PATCH v2 2/3] arm64: dts: rockchip: add overlay tests for Rock 5B PCIe overlays

[Niklas Cassel]

Hello Quentin,

On Fri, Jan 17, 2025 at 12:50:52PM +0100, Quentin Schulz wrote:
> 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@cherry.de>
> > > 
> > > 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.

I know what Greg would have said:
https://lore.kernel.org/linux-pci/2025011651-stubborn-certified-b22f@gregkh/


> 
> So 1) what's your opinion on backporting this to stable
> 2) if backporting, shouldn't I still remove the Fixes?

My personal opinion is that a lot of companies are way too greedy, just
taking the work of upstream, but never giving anything back by actually
employing people working on upstream, all while sometimes having hundreds
of employees working on downstream.

I wish I could stop marking patches with CC: stable just to not make their
lives easier. If they want the good stuff, at least they should spend time
upgrading from their 4.14 kernel... Note that I do still (reluctantly) mark
my fixes with CC: stable.


> 
> > 
> > > @@ -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.

As long as the files still get generated with the .dtbo offset, I'm happy.


Kind regards,
Niklas

Re: [PATCH v2 2/3] arm64: dts: rockchip: add overlay tests for Rock 5B PCIe overlays

[Krzysztof Kozlowski]

On 16/01/2025 15:47, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> 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.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 2/3] arm64: dts: rockchip: add overlay tests for Rock 5B PCIe overlays

[Niklas Cassel]

On Thu, Jan 16, 2025 at 03:47:11PM +0100, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> 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")
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---

Reviewed-by: Niklas Cassel <cassel@kernel.org>

[PATCH v2 3/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

The Pre-ICT tester adapter connects to RK3588 Jaguar SBC through its
proprietary Mezzanine connector.

It exposes a PCIe Gen2 1x M.2 connector and two proprietary camera
connectors. Support for the latter will come once the rest of the camera
stack is supported.

Additionally, the adapter loops some GPIOs together as well as route
some GPIOs to power rails.

This adapter is used for manufacturing RK3588 Jaguar to be able to test
the Mezzanine connector is properly soldered.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 arch/arm64/boot/dts/rockchip/Makefile              |   2 +
 .../dts/rockchip/rk3588-jaguar-pre-ict-tester.dtso | 171 +++++++++++++++++++++
 2 files changed, 173 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index 72f8666b42400a2f462421f5db6b3cb0cf369611..deeb96609461847d4039e144beb36522333ca398 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -163,9 +163,11 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5c.dtb
 
 # Overlays
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-wifi.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-jaguar-pre-ict-tester.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-ep.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-srns.dtb
 
 rk3588-edgeble-neu6a-wifi-dtbs := rk3588-edgeble-neu6a-io.dtb rk3588-edgeble-neu6a-wifi.dtbo
+rk3588-jaguar-pre-ict-tester-dtbs     := rk3588-jaguar.dtb rk3588-jaguar-pre-ict-tester.dtbo
 rk3588-rock-5b-pcie-ep-dtbs := rk3588-rock-5b.dtb rk3588-rock-5b-pcie-ep.dtbo
 rk3588-rock-5b-pcie-srns-dtbs := rk3588-rock-5b.dtb rk3588-rock-5b-pcie-srns.dtbo
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar-pre-ict-tester.dtso b/arch/arm64/boot/dts/rockchip/rk3588-jaguar-pre-ict-tester.dtso
new file mode 100644
index 0000000000000000000000000000000000000000..9d44dfe2f30d793accb994a230c58038f0c3daad
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar-pre-ict-tester.dtso
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT)
+/*
+ * Copyright (c) 2024 Cherry Embedded Solutions GmbH
+ *
+ * Device Tree Overlay for the Pre-ICT tester adapter for the Mezzanine
+ * connector on RK3588 Jaguar.
+ *
+ * This adapter has a PCIe Gen2 x1 M.2 M-Key connector and two proprietary
+ * camera connectors (each their own I2C bus, clock, reset and PWM lines as well
+ * as 2-lane CSI).
+ *
+ * This adapter routes some GPIOs to power rails and loops together some other
+ * GPIOs.
+ *
+ * This adapter is used during manufacturing for validating proper soldering of
+ * the mezzanine connector.
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pinctrl/rockchip.h>
+
+&{/} {
+	pre_ict_tester_vcc_1v2: regulator-pre-ict-tester-vcc-1v2 {
+		compatible = "regulator-fixed";
+		regulator-name = "pre_ict_tester_vcc_1v2";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <1200000>;
+		regulator-max-microvolt = <1200000>;
+		vin-supply = <&vcc_3v3_s3>;
+	};
+
+	pre_ict_tester_vcc_2v8: regulator-pre-ict-tester-vcc-2v8 {
+		compatible = "regulator-fixed";
+		regulator-name = "pre_ict_tester_vcc_2v8";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		vin-supply = <&vcc_3v3_s3>;
+	};
+};
+
+&combphy0_ps {
+	status = "okay";
+};
+
+&gpio3 {
+	pinctrl-0 = <&pre_ict_pwr2gpio>;
+	pinctrl-names = "default";
+};
+
+&pcie2x1l2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie2x1l2_perstn_m0>;
+	reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>; /* PCIE20X1_2_PERSTN_M0 */
+	vpcie3v3-supply = <&vcc_3v3_s3>;
+	status = "okay";
+};
+
+&pinctrl {
+	pcie2x1l2 {
+		pcie2x1l2_perstn_m0: pcie2x1l2-perstn-m0 {
+			rockchip,pins = <3 RK_PD1 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pre-ict-tester {
+		pre_ict_pwr2gpio: pre-ict-pwr2gpio-pins {
+			rockchip,pins =
+			/*
+			 * GPIO3_A3 requires two power rails to be properly
+			 * routed to the mezzanine connector to report a proper
+			 * value: VCC_1V8_S0_1 and VCC_IN_2. It may report an
+			 * incorrect value if VCC_1V8_S0_1 isn't properly routed,
+			 * but GPIO3_C6 would catch this HW soldering issue.
+			 * If VCC_IN_2 is properly routed, GPIO3_A3 should be
+			 * LOW. The signal shall not read HIGH in the event
+			 * GPIO3_A3 isn't properly routed due to soldering
+			 * issue. Therefore, let's enforce a pull-up (which is
+			 * the SoC default for this pin).
+			 */
+				<3 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>,
+			/*
+			 * GPIO3_A4 is directly routed to VCC_1V8_S0_2 power
+			 * rail. It should be HIGH if all is properly soldered.
+			 * To guarantee that, a pull-down is enforced (which is
+			 * the SoC default for this pin) so that LOW is read if
+			 * the loop doesn't exist on HW (soldering issue on
+			 * either signals).
+			 */
+				<3 RK_PA4 RK_FUNC_GPIO &pcfg_pull_down>,
+			/*
+			 * GPIO3_B2 requires two power rails to be properly
+			 * routed to the mezzanine connector to report a proper
+			 * value: VCC_1V8_S0_1 and VCC_IN_1. It may report an
+			 * incorrect value if VCC_1V8_S0_1 isn't properly routed,
+			 * but GPIO3_C6 would catch this HW soldering issue.
+			 * If VCC_IN_1 is properly routed, GPIO3_B2 should be
+			 * LOW. This is an issue if GPIO3_B2 isn't properly
+			 * routed due to soldering issue, because GPIO3_B2
+			 * default bias is pull-down therefore being LOW. So
+			 * the worst case scenario and the pass scenario expect
+			 * the same value. Make GPIO3_B2 a pull-up so that a
+			 * soldering issue on GPIO3_B2 reports HIGH but proper
+			 * soldering reports LOW.
+			 */
+				<3 RK_PB2 RK_FUNC_GPIO &pcfg_pull_up>,
+			/*
+			 * GPIO3_C6 is directly routed to VCC_1V8_S0_1 power
+			 * rail. It should be HIGH if all is properly soldered.
+			 * This is an issue if GPIO3_C6 or VCC_1V8_S0_1 isn't
+			 * properly routed due to soldering issue, because
+			 * GPIO3_C6 default bias is pull-up therefore being HIGH
+			 * in all cases:
+			 *  - GPIO3_C6 is floating (so HIGH) if GPIO3_C6 is not
+			 *    routed properly,
+			 *  - GPIO3_C6 is floating (so HIGH) if VCC_1V8_S0_1 is
+			 *    not routed properly,
+			 *  - GPIO3_C6 is HIGH if everything is proper,
+			 * Make GPIO3_C6 a pull-down so that a soldering issue
+			 * on GPIO3_C6 or VCC_1V8_S0_1 reports LOW but proper
+			 * soldering reports HIGH.
+			 */
+				<3 RK_PC6 RK_FUNC_GPIO &pcfg_pull_down>,
+			/*
+			 * GPIO3_D2 is routed to VCC_5V0_1 power rail through a
+			 * voltage divider on the adapter.
+			 * It should be HIGH if all is properly soldered.
+			 * To guarantee that, a pull-down is enforced (which is
+			 * the SoC default for this pin) so that LOW is read if
+			 * the loop doesn't exist on HW (soldering issue on
+			 * either signals).
+			 */
+				<3 RK_PD2 RK_FUNC_GPIO &pcfg_pull_down>,
+			/*
+			 * GPIO3_D3 is routed to VCC_5V0_2 power rail through a
+			 * voltage divider on the adapter.
+			 * It should be HIGH if all is properly soldered.
+			 * To guarantee that, a pull-down is enforced (which is
+			 * the SoC default for this pin) so that LOW is read if
+			 * the loop doesn't exist on HW (soldering issue on
+			 * either signals).
+			 */
+				<3 RK_PD3 RK_FUNC_GPIO &pcfg_pull_down>,
+			/*
+			 * GPIO3_D4 is routed to VCC_3V3_S3_1 power rail through
+			 * a voltage divider on the adapter.
+			 * It should be HIGH if all is properly soldered.
+			 * To guarantee that, a pull-down is enforced (which is
+			 * the SoC default for this pin) so that LOW is read if
+			 * the loop doesn't exist on HW (soldering issue on
+			 * either signals).
+			 */
+				<3 RK_PD4 RK_FUNC_GPIO &pcfg_pull_down>,
+			/*
+			 * GPIO3_D5 is routed to VCC_3V3_S3_2 power rail through
+			 * a voltage divider on the adapter.
+			 * It should be HIGH if all is properly soldered.
+			 * To guarantee that, a pull-down is enforced (which is
+			 * the SoC default for this pin) so that LOW is read if
+			 * the loop doesn't exist on HW (soldering issue on
+			 * either signals).
+			 */
+				<3 RK_PD5 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
+};

-- 
2.48.0

Re: [PATCH v2 3/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar

[Krzysztof Kozlowski]

On 16/01/2025 15:47, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> The Pre-ICT tester adapter connects to RK3588 Jaguar SBC through its
> proprietary Mezzanine connector.
> 
> It exposes a PCIe Gen2 1x M.2 connector and two proprietary camera
> connectors. Support for the latter will come once the rest of the camera
> stack is supported.
> 
> Additionally, the adapter loops some GPIOs together as well as route
> some GPIOs to power rails.
> 
> This adapter is used for manufacturing RK3588 Jaguar to be able to test
> the Mezzanine connector is properly soldered.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 0/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests

[Michael Riesch]

Hi Quentin,

On 1/16/25 15:47, Quentin Schulz wrote:
> This adds minimal support for the Pre-ICT tester adapter for RK3588
> Jaguar.
> GPIO3A3, GPIO3A4, GPIO3B2 and GPIO3D2 to GPIO3D5 are all routed to power
> rails and can only be used as input and their bias are important to be
> able to properly detect soldering issues.
> 
> Additionally, this adds build-time overlay application tests for (some)
> Rockchip overlays to try to avoid future regressions.
> 
> Notably, the Device Trees from Wolfvision PF5 aren't migrated (but
> should) as I do not own the device and couldn't figure out from the
> introducing commit logs what the possible valid combinations are.
> +Cc Michael Riesch for awareness

Thanks for the heads up!

Just to make sure I understood correctly: By migrated you mean that the
overlay entries are moved to a separate section in the Makefile and
there are explicit combinations of base DTS and overlays for
compile-time testing purposes? If so, I don't consider the PF5 migration
as *that* urgent. I don't think you can break anything on our side. Or
am I missing something?

Maybe you can move the lines

  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-display-vz.dtbo
  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo

to the overlay section as well? This should not cause any functional
changes.

> I'm wondering if we shouldn't backport patches 1 and 2 to stable? In
> which case, it would make sense to try to have the Wolfvision PF5
> overlay tests merged before the addition of the Pre-ICT tester adapter
> support for RK3588 Jaguar as that one won't be backported to stable and
> backporting the Wolfvision overlay test would incur an unnecessary
> (though not difficult) git conflict to resolve.
> 
> I also do not know what kind of tests we should have when overlay
> combinations are possible (let's say there are A, B and C overlays that
> can all be applied, should we have base + A, base + B, base + C,
> base + A + B, base + A + C, base + B + C and base + A + B + C tests?
> maybe even base + B + A, base + C + B, base A + C + B, base + B + A + C,
> base + B + C + A, base + C + B + A and base + C + A + B tests?).

I have never been good at combinatorics, but I feel this has the
potential to explode :-) My two cents: The overlays *should* be
orthogonal to each other, i.e., no dependencies between them in the
sense that overlay A creates a node that is used by overlay B and that
sort of thing. Then
 - Permutation can be ignored. (base + A + C = base + C + A)
 - Composition (base + A + B + C) may be ignored in favor of individual
   tests.
 - Individual tests may be ignored in favor of (a) composition(s) that
   cover(s) all individual tests.

But of course this is likely to vary from case to case. In our case, in
the composition

  rk3568-wolfvision-pf5-vz-2-uhd := rk3568-wolfvision-pf5.dtb \
    rk3568-wolfvision-pf5-io-expander.dtbo \
    rk3568-wolfvision-pf5-display-vz.dtbo

would do the trick.

Thanks and regards,
Michael

> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
> Changes in v2:
> - add overlay application tests for Edgeble NCM6A WiFi and Rock 5B PCIe
>   Endpoint+SNRS
> - add overlay application test for RK3588 Jaguar + Pre-ICT tester
>   adapter,
> - Link to v1: https://lore.kernel.org/r/20241206-pre-ict-jaguar-v1-1-7f660bd4b70c@cherry.de
> 
> ---
> Quentin Schulz (3):
>       arm64: dts: rockchip: add overlay test for Edgeble NCM6A
>       arm64: dts: rockchip: add overlay tests for Rock 5B PCIe overlays
>       arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar
> 
>  arch/arm64/boot/dts/rockchip/Makefile              |  14 +-
>  .../dts/rockchip/rk3588-jaguar-pre-ict-tester.dtso | 171 +++++++++++++++++++++
>  2 files changed, 182 insertions(+), 3 deletions(-)
> ---
> base-commit: 619f0b6fad524f08d493a98d55bac9ab8895e3a6
> change-id: 20241206-pre-ict-jaguar-b90fafee8bd8
> 
> Best regards,

Re: [PATCH v2 0/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests

[Quentin Schulz]

Hi Michael,

On 1/20/25 10:06 AM, Michael Riesch wrote:
> Hi Quentin,
> 
> On 1/16/25 15:47, Quentin Schulz wrote:
>> This adds minimal support for the Pre-ICT tester adapter for RK3588
>> Jaguar.
>> GPIO3A3, GPIO3A4, GPIO3B2 and GPIO3D2 to GPIO3D5 are all routed to power
>> rails and can only be used as input and their bias are important to be
>> able to properly detect soldering issues.
>>
>> Additionally, this adds build-time overlay application tests for (some)
>> Rockchip overlays to try to avoid future regressions.
>>
>> Notably, the Device Trees from Wolfvision PF5 aren't migrated (but
>> should) as I do not own the device and couldn't figure out from the
>> introducing commit logs what the possible valid combinations are.
>> +Cc Michael Riesch for awareness
> 
> Thanks for the heads up!
> 
> Just to make sure I understood correctly: By migrated you mean that the
> overlay entries are moved to a separate section in the Makefile and
> there are explicit combinations of base DTS and overlays for
> compile-time testing purposes? If so, I don't consider the PF5 migration
> as *that* urgent. I don't think you can break anything on our side. Or
> am I missing something?
> 

I think it makes sense to backport the patches in this series (except 
the one adding support for the Pre-ICT tester adapter on RK3588 Jaguar). 
Because we cannot backport the patch for Pre-ICT tester adapter (it's an 
addition, not a bug fix), any patch that is applied after that one will 
result in a git conflict when backporting to stable releases.

I believe it makes sense to backport build time overlay application tests.

The git conflict will likely be no big deal but if we can avoid it, 
better avoid it :)

> Maybe you can move the lines
> 
>    dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-display-vz.dtbo
>    dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo
> 
> to the overlay section as well? This should not cause any functional
> changes.
> 

The overlay section would only support the syntax which allows build 
time testing. I would like to avoid confusion around what to do when 
adding a new overlay. Specifically note the missing o in the extension 
with the build time tests.

>> I'm wondering if we shouldn't backport patches 1 and 2 to stable? In
>> which case, it would make sense to try to have the Wolfvision PF5
>> overlay tests merged before the addition of the Pre-ICT tester adapter
>> support for RK3588 Jaguar as that one won't be backported to stable and
>> backporting the Wolfvision overlay test would incur an unnecessary
>> (though not difficult) git conflict to resolve.
>>
>> I also do not know what kind of tests we should have when overlay
>> combinations are possible (let's say there are A, B and C overlays that
>> can all be applied, should we have base + A, base + B, base + C,
>> base + A + B, base + A + C, base + B + C and base + A + B + C tests?
>> maybe even base + B + A, base + C + B, base A + C + B, base + B + A + C,
>> base + B + C + A, base + C + B + A and base + C + A + B tests?).
> 
> I have never been good at combinatorics, but I feel this has the
> potential to explode :-) My two cents: The overlays *should* be
> orthogonal to each other, i.e., no dependencies between them in the
> sense that overlay A creates a node that is used by overlay B and that
> sort of thing. Then

I disagree. I can already tell you the following usecase:

our Pre-ICT tester adapter for RK3588 Jaguar has two proprietary camera 
connectors. We already have two camera modules working with it, so the 
following would make sense:

pre-ict-tester.dtbo
pre-ict-tester-con1-camX.dtbo
pre-ict-tester-con1-camY.dtbo
pre-ict-tester-con2-camX.dtbo
pre-ict-tester-con2-camY.dtbo

You would then apply pre-ict-tester.dtbo followed by one or two cam 
dtbos. The pre-ict-tester can be used without the camera modules (c.f. 
this patch :) ).

>   - Permutation can be ignored. (base + A + C = base + C + A)

I think that's fair. It would anyway be an issue with dtso which are 
using /delete-node/ or /delete-property/.

>   - Composition (base + A + B + C) may be ignored in favor of individual
>     tests.

Not sure this is ideal either.

Our RK3588 Jaguar main PCBA also has two proprietary camera connectors. 
It would make sense to test that applying a dtso for main PCBA is not 
messing with applying a dtso for the camera module on the adapter.

This is a bit theoretical at the moment though since there's no camera 
stack available for RK3588.

>   - Individual tests may be ignored in favor of (a) composition(s) that
>     cover(s) all individual tests.
> 
> But of course this is likely to vary from case to case. In our case, in
> the composition
> 
>    rk3568-wolfvision-pf5-vz-2-uhd := rk3568-wolfvision-pf5.dtb \
>      rk3568-wolfvision-pf5-io-expander.dtbo \
>      rk3568-wolfvision-pf5-display-vz.dtbo
> 
> would do the trick.
> 

Thanks, will send a patch for that in v3.

Cheers,
Quentin

Re: [PATCH v2 0/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests

[Michael Riesch]

Hi Quentin,

On 1/20/25 10:20, Quentin Schulz wrote:
> Hi Michael,
> 
> On 1/20/25 10:06 AM, Michael Riesch wrote:
>> Hi Quentin,
>>
>> On 1/16/25 15:47, Quentin Schulz wrote:
>>> This adds minimal support for the Pre-ICT tester adapter for RK3588
>>> Jaguar.
>>> GPIO3A3, GPIO3A4, GPIO3B2 and GPIO3D2 to GPIO3D5 are all routed to power
>>> rails and can only be used as input and their bias are important to be
>>> able to properly detect soldering issues.
>>>
>>> Additionally, this adds build-time overlay application tests for (some)
>>> Rockchip overlays to try to avoid future regressions.
>>>
>>> Notably, the Device Trees from Wolfvision PF5 aren't migrated (but
>>> should) as I do not own the device and couldn't figure out from the
>>> introducing commit logs what the possible valid combinations are.
>>> +Cc Michael Riesch for awareness
>>
>> Thanks for the heads up!
>>
>> Just to make sure I understood correctly: By migrated you mean that the
>> overlay entries are moved to a separate section in the Makefile and
>> there are explicit combinations of base DTS and overlays for
>> compile-time testing purposes? If so, I don't consider the PF5 migration
>> as *that* urgent. I don't think you can break anything on our side. Or
>> am I missing something?
>>
> 
> I think it makes sense to backport the patches in this series (except
> the one adding support for the Pre-ICT tester adapter on RK3588 Jaguar).
> Because we cannot backport the patch for Pre-ICT tester adapter (it's an
> addition, not a bug fix), any patch that is applied after that one will
> result in a git conflict when backporting to stable releases.
> 
> I believe it makes sense to backport build time overlay application tests.
> 
> The git conflict will likely be no big deal but if we can avoid it,
> better avoid it :)

OK.

> 
>> Maybe you can move the lines
>>
>>    dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-display-vz.dtbo
>>    dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo
>>
>> to the overlay section as well? This should not cause any functional
>> changes.
>>
> 
> The overlay section would only support the syntax which allows build
> time testing. I would like to avoid confusion around what to do when
> adding a new overlay. Specifically note the missing o in the extension
> with the build time tests.
> 
>>> I'm wondering if we shouldn't backport patches 1 and 2 to stable? In
>>> which case, it would make sense to try to have the Wolfvision PF5
>>> overlay tests merged before the addition of the Pre-ICT tester adapter
>>> support for RK3588 Jaguar as that one won't be backported to stable and
>>> backporting the Wolfvision overlay test would incur an unnecessary
>>> (though not difficult) git conflict to resolve.
>>>
>>> I also do not know what kind of tests we should have when overlay
>>> combinations are possible (let's say there are A, B and C overlays that
>>> can all be applied, should we have base + A, base + B, base + C,
>>> base + A + B, base + A + C, base + B + C and base + A + B + C tests?
>>> maybe even base + B + A, base + C + B, base A + C + B, base + B + A + C,
>>> base + B + C + A, base + C + B + A and base + C + A + B tests?).
>>
>> I have never been good at combinatorics, but I feel this has the
>> potential to explode :-) My two cents: The overlays *should* be
>> orthogonal to each other, i.e., no dependencies between them in the
>> sense that overlay A creates a node that is used by overlay B and that
>> sort of thing. Then
> 
> I disagree. I can already tell you the following usecase:
> 
> our Pre-ICT tester adapter for RK3588 Jaguar has two proprietary camera
> connectors. We already have two camera modules working with it, so the
> following would make sense:
> 
> pre-ict-tester.dtbo
> pre-ict-tester-con1-camX.dtbo
> pre-ict-tester-con1-camY.dtbo
> pre-ict-tester-con2-camX.dtbo
> pre-ict-tester-con2-camY.dtbo
> 
> You would then apply pre-ict-tester.dtbo followed by one or two cam
> dtbos. The pre-ict-tester can be used without the camera modules (c.f.
> this patch :) ).

Most probably there is no general answer. In the end the use cases
decide what tests do make sense.

> 
>>   - Permutation can be ignored. (base + A + C = base + C + A)
> 
> I think that's fair. It would anyway be an issue with dtso which are
> using /delete-node/ or /delete-property/.
> 
>>   - Composition (base + A + B + C) may be ignored in favor of individual
>>     tests.
> 
> Not sure this is ideal either.
> 
> Our RK3588 Jaguar main PCBA also has two proprietary camera connectors.
> It would make sense to test that applying a dtso for main PCBA is not
> messing with applying a dtso for the camera module on the adapter.
> 
> This is a bit theoretical at the moment though since there's no camera
> stack available for RK3588.
> 
>>   - Individual tests may be ignored in favor of (a) composition(s) that
>>     cover(s) all individual tests.
>>
>> But of course this is likely to vary from case to case. In our case, in
>> the composition
>>
>>    rk3568-wolfvision-pf5-vz-2-uhd := rk3568-wolfvision-pf5.dtb \
>>      rk3568-wolfvision-pf5-io-expander.dtbo \
>>      rk3568-wolfvision-pf5-display-vz.dtbo
>>
>> would do the trick.
>>
> 
> Thanks, will send a patch for that in v3.

That'd be great, thanks!

Regards, Michael

> 
> Cheers,
> Quentin

Re: [PATCH v2 0/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests

[Quentin Schulz]

Hi all,

On 1/16/25 3:47 PM, Quentin Schulz wrote:
> This adds minimal support for the Pre-ICT tester adapter for RK3588
> Jaguar.
> GPIO3A3, GPIO3A4, GPIO3B2 and GPIO3D2 to GPIO3D5 are all routed to power
> rails and can only be used as input and their bias are important to be
> able to properly detect soldering issues.
> 
> Additionally, this adds build-time overlay application tests for (some)
> Rockchip overlays to try to avoid future regressions.
> 
> Notably, the Device Trees from Wolfvision PF5 aren't migrated (but
> should) as I do not own the device and couldn't figure out from the
> introducing commit logs what the possible valid combinations are.
> +Cc Michael Riesch for awareness
> 
> I'm wondering if we shouldn't backport patches 1 and 2 to stable? In
> which case, it would make sense to try to have the Wolfvision PF5
> overlay tests merged before the addition of the Pre-ICT tester adapter
> support for RK3588 Jaguar as that one won't be backported to stable and
> backporting the Wolfvision overlay test would incur an unnecessary
> (though not difficult) git conflict to resolve.
> 
> I also do not know what kind of tests we should have when overlay
> combinations are possible (let's say there are A, B and C overlays that
> can all be applied, should we have base + A, base + B, base + C,
> base + A + B, base + A + C, base + B + C and base + A + B + C tests?
> maybe even base + B + A, base + C + B, base A + C + B, base + B + A + C,
> base + B + C + A, base + C + B + A and base + C + A + B tests?).
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
> Changes in v2:
> - add overlay application tests for Edgeble NCM6A WiFi and Rock 5B PCIe
>    Endpoint+SNRS
> - add overlay application test for RK3588 Jaguar + Pre-ICT tester
>    adapter,

This actually has a side effect.

Rockchip DTBs are compiled without symbols today for backward 
compatibility reasons. Indeed, having symbols increases the size of the 
DTB and by a rather non-negligible amount.

With this series applied (plus the change for the wolvision that was 
intended for v3):

115K arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5.dtb
162K arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-io.dtb
165K arch/arm64/boot/dts/rockchip/rk3588-jaguar.dtb
165K arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtb

Without it:
57K arch/arm64/boot/dts/rockchip/rk3568-wolfvision-pf5.dtb
83K arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-io.dtb
87K arch/arm64/boot/dts/rockchip/rk3588-jaguar.dtb
86K arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtb

As far as I remember, the issue is that we want to make sure that such a 
bloated binary is not going to break things. Considering that U-Boot 
passes the full DTB to TF-A and that upstream TF-A has support for 
loading it on Rockchip since v2.1, this would be one piece of software 
potentially impacted by the size increase.

TF-A v2.1 to v2.3 (both included) only had 64KiB available for loading 
the DTB passed by the previous stage (U-Boot most likely). 
https://review.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/7029e806833b94f729d9117bd35d488476b0e27e%5E%21/#F0 
the commit introducing support for parsing the FDT, see the static array 
of 0x10000 bytes (64KiB).

The buffer size got increased to 0x20000 bytes (128KiB) in v2.4, c.f.
https://review.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/8109f738ffa79a63735cba29da26e7c2859977b5%5E%21/#F0

Additionally, before v2.4, passing a DT too big would result in TF-A 
crashing, c.f. 
https://review.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/e7b586987c0a46660aa8402f19d626a5489fe449%5E%21/#F0

Unfortunately, Rockchip has seemingly decided v2.2 will be forever their 
base version for their blobs. This means that we are forced to pass a DT 
below 64KiB in size at the risk of crashing TF-A otherwise. Considering 
that a memory misalignment can also make fdt_open_into() fail and thus 
crash TF-A <= 2.3, c.f. 
https://review.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/621acbd055d712ab8bf79054911155598fdb74d0%5E%21/#F0, 
there's essentially too much risk to use DT with TF-A <= 2.3.

Rockchip being stuck on v2.2 for the binary blob is the reason why most 
Rockchip boards supported by U-Boot do NOT actually pass the DT to TF-A, 
c.f. SPL_ATF_NO_PLATFORM_PARAM symbol: 
https://elixir.bootlin.com/u-boot/v2024.10/source/common/spl/Kconfig#L1446

So essentially, if SPL_ATF_NO_PLATFORM_PARAM is selected (the default 
for RK356x, RK3588, forced on on RK3308, enabled for the majority of 
RK3399 boards, enabled for all RK3328 boards) the DT won't be passed to 
TF-A so no issue in terms of size on that side.
If it is not selected, for TF-A < 2.4 (released 20201117, 4 years ago), 
a DTB bigger than 64KiB will crash TF-A.
If it is not selected, for TF-A >= 2.4, a DTB bigger than 128KiB will 
result in TF-A not being able to read the DTB (for Rockchip, that means 
not being able to derive the UART settings (controller and baudrate) to 
use, and will use the compile-time default instead).

RPi seems to be loading it into a 1MiB buffer, Xilinx into a 2MiB 
buffer, 64KiB for ARM FPGA targets and Allwinner.

We could/should increase the size of the buffer for the DTB passed to 
TF-A but there's a limit. Indeed, there are many assumptions all over 
U-Boot that TF-A only operates in the first 2MiB of DRAM and reserves it 
for that purpose.

If I didn't misread the code, it seems 
PX30/RK3328/RK3368/RK3399/RK356x/RK3588 upstream support only uses the 
last 768KiB of the first 1MiB of DRAM, c.f. BL31_BASE and BL31_LIMIT. No 
clue if that's a proper interpretation of the code or if I missed 
something, that's a rather odd choice, considering Rockchip is adamant 
we need to reserve 2MiB for their downstream blob.

Regardless of what can be done for TF-A in the future, the fact is that 
it currently is limited to 128KiB in the best case scenario. This limit 
is already reached by adding symbols to the DTB, which is a thing that 
this patch series does.

Note that U-Boot typically does not use the kernel's DTB as its own, at 
least for Rockchip, for now. It does compile from the same source (+ 
some additions in arch/arm/dts/*-u-boot.dtsi files) but without symbols 
except if OF_OVERLAY_LIST symbol is defined, in which case the DTB 
symbols are kept. This symbol is currently not enabled by any Rockchip 
board.

In short, I don't know where to go with that additional piece of 
information, but this is a bit bigger than simply moving things around 
and adding compile-time tests for overlay application.

Cheers,
Quentin

Re: [PATCH v2 0/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests

[Niklas Cassel]

On Wed, Jan 22, 2025 at 04:38:16PM +0100, Quentin Schulz wrote:
> So essentially, if SPL_ATF_NO_PLATFORM_PARAM is selected (the default for
> RK356x, RK3588, forced on on RK3308, enabled for the majority of RK3399
> boards, enabled for all RK3328 boards) the DT won't be passed to TF-A so no
> issue in terms of size on that side.
> If it is not selected, for TF-A < 2.4 (released 20201117, 4 years ago), a
> DTB bigger than 64KiB will crash TF-A.
> If it is not selected, for TF-A >= 2.4, a DTB bigger than 128KiB will result
> in TF-A not being able to read the DTB (for Rockchip, that means not being
> able to derive the UART settings (controller and baudrate) to use, and will
> use the compile-time default instead).

Not everyone is using binary blobs from Rockchip.
On my rock5b (rk3588), I'm building the bootloader using buildroot,
which is using upstream TrustedFirmware-A (v2.12).


> In short, I don't know where to go with that additional piece of
> information, but this is a bit bigger than simply moving things around and
> adding compile-time tests for overlay application.

This is significant information indeed.


Kind regards,
Niklas

Re: [PATCH v2 0/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests

[Heiko Stübner]

Am Mittwoch, 22. Januar 2025, 17:12:26 CET schrieb Niklas Cassel:
> On Wed, Jan 22, 2025 at 04:38:16PM +0100, Quentin Schulz wrote:
> > So essentially, if SPL_ATF_NO_PLATFORM_PARAM is selected (the default for
> > RK356x, RK3588, forced on on RK3308, enabled for the majority of RK3399
> > boards, enabled for all RK3328 boards) the DT won't be passed to TF-A so no
> > issue in terms of size on that side.
> > If it is not selected, for TF-A < 2.4 (released 20201117, 4 years ago), a
> > DTB bigger than 64KiB will crash TF-A.
> > If it is not selected, for TF-A >= 2.4, a DTB bigger than 128KiB will result
> > in TF-A not being able to read the DTB (for Rockchip, that means not being
> > able to derive the UART settings (controller and baudrate) to use, and will
> > use the compile-time default instead).
> 
> Not everyone is using binary blobs from Rockchip.
> On my rock5b (rk3588), I'm building the bootloader using buildroot,
> which is using upstream TrustedFirmware-A (v2.12).
> 
> 
> > In short, I don't know where to go with that additional piece of
> > information, but this is a bit bigger than simply moving things around and
> > adding compile-time tests for overlay application.
> 
> This is significant information indeed.

I guess the question is, can this hurt existing devices?

As Quentin mentioned, this only affects DTs that get handed over from
U-Boot to TF-A (and maybe OP-TEE).

So the whole range of things loading their DT from extlinux.conf or
whatever are not really affected.


DTs U-Boot can hand over are 2 types,
(1) built from within u-boot and
(2) stored somewhere centrally (SPI flash).


Case (1) is again not affected, as U-Boot (and other bootloaders) may
very well sync the DTS files, but generally not the build-system, so if
U-Boot (or any other bootloader) creates DTBs with symbols is completely
their own choice.


And for case (2) I see the manufacturer being responsible. Having the DT
in central storage makes it somewhat part of a "bios"-level in the hirarchy
and the general guarantee is that new software _will work_ with older DTs,
but the other way around is more a nice to have (old SW with new DTB).

So if some manufacturer has a centrally located DTB this does not matter
until they upgrade, and when that happens I do expect testing to happen
at the manufacturers side, before rolling out a "bios update"


Heiko

Re: [PATCH v2 0/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests

[Niklas Cassel]

On Thu, Jan 23, 2025 at 03:13:01PM +0100, Heiko Stübner wrote:
> Am Mittwoch, 22. Januar 2025, 17:12:26 CET schrieb Niklas Cassel:
> > On Wed, Jan 22, 2025 at 04:38:16PM +0100, Quentin Schulz wrote:
> > > So essentially, if SPL_ATF_NO_PLATFORM_PARAM is selected (the default for
> > > RK356x, RK3588, forced on on RK3308, enabled for the majority of RK3399
> > > boards, enabled for all RK3328 boards) the DT won't be passed to TF-A so no
> > > issue in terms of size on that side.
> > > If it is not selected, for TF-A < 2.4 (released 20201117, 4 years ago), a
> > > DTB bigger than 64KiB will crash TF-A.
> > > If it is not selected, for TF-A >= 2.4, a DTB bigger than 128KiB will result
> > > in TF-A not being able to read the DTB (for Rockchip, that means not being
> > > able to derive the UART settings (controller and baudrate) to use, and will
> > > use the compile-time default instead).
> > 
> > Not everyone is using binary blobs from Rockchip.
> > On my rock5b (rk3588), I'm building the bootloader using buildroot,
> > which is using upstream TrustedFirmware-A (v2.12).
> > 
> > 
> > > In short, I don't know where to go with that additional piece of
> > > information, but this is a bit bigger than simply moving things around and
> > > adding compile-time tests for overlay application.
> > 
> > This is significant information indeed.
> 
> I guess the question is, can this hurt existing devices?
> 
> As Quentin mentioned, this only affects DTs that get handed over from
> U-Boot to TF-A (and maybe OP-TEE).
> 
> So the whole range of things loading their DT from extlinux.conf or
> whatever are not really affected.
> 
> 
> DTs U-Boot can hand over are 2 types,
> (1) built from within u-boot and
> (2) stored somewhere centrally (SPI flash).
> 
> 
> Case (1) is again not affected, as U-Boot (and other bootloaders) may
> very well sync the DTS files, but generally not the build-system, so if
> U-Boot (or any other bootloader) creates DTBs with symbols is completely
> their own choice.
> 
> 
> And for case (2) I see the manufacturer being responsible. Having the DT
> in central storage makes it somewhat part of a "bios"-level in the hirarchy
> and the general guarantee is that new software _will work_ with older DTs,
> but the other way around is more a nice to have (old SW with new DTB).
> 
> So if some manufacturer has a centrally located DTB this does not matter
> until they upgrade, and when that happens I do expect testing to happen
> at the manufacturers side, before rolling out a "bios update"

Personally, I'm all for letting the kernel build the DTBs with symbols.

(I have a patch that I keep rebasing on my tree only for that purpose.
Sure, I could modify my build scripts to build the DTB separately,
but with this patch, I do not need to do anything since the kernel
builds the DTBs already.)

Other platforms, e.g. TI already build many boards with symbols:
https://github.com/torvalds/linux/blob/v6.13/arch/arm64/boot/dts/ti/Makefile#L242-L261


You seems to have been against enabling symbols before:
https://lore.kernel.org/linux-rockchip/171941553475.921128.9467465539299233735.b4-ty@sntech.de/
https://lore.kernel.org/linux-rockchip/1952472.6tgchFWduM@diego/

But if you have changed you mind, and you are no longer concerned about
doing so, then in my own self-interest I'm all for it :)


Kind regards,
Niklas

Re: [PATCH v2 0/3] arm64: dts: rockchip: minimal support for Pre-ICT tester adapter for RK3588 Jaguar + add overlay tests

[Heiko Stübner]

Am Freitag, 24. Januar 2025, 11:21:00 CET schrieb Niklas Cassel:
> On Thu, Jan 23, 2025 at 03:13:01PM +0100, Heiko Stübner wrote:
> > Am Mittwoch, 22. Januar 2025, 17:12:26 CET schrieb Niklas Cassel:
> > > On Wed, Jan 22, 2025 at 04:38:16PM +0100, Quentin Schulz wrote:
> > > > So essentially, if SPL_ATF_NO_PLATFORM_PARAM is selected (the default for
> > > > RK356x, RK3588, forced on on RK3308, enabled for the majority of RK3399
> > > > boards, enabled for all RK3328 boards) the DT won't be passed to TF-A so no
> > > > issue in terms of size on that side.
> > > > If it is not selected, for TF-A < 2.4 (released 20201117, 4 years ago), a
> > > > DTB bigger than 64KiB will crash TF-A.
> > > > If it is not selected, for TF-A >= 2.4, a DTB bigger than 128KiB will result
> > > > in TF-A not being able to read the DTB (for Rockchip, that means not being
> > > > able to derive the UART settings (controller and baudrate) to use, and will
> > > > use the compile-time default instead).
> > > 
> > > Not everyone is using binary blobs from Rockchip.
> > > On my rock5b (rk3588), I'm building the bootloader using buildroot,
> > > which is using upstream TrustedFirmware-A (v2.12).
> > > 
> > > 
> > > > In short, I don't know where to go with that additional piece of
> > > > information, but this is a bit bigger than simply moving things around and
> > > > adding compile-time tests for overlay application.
> > > 
> > > This is significant information indeed.
> > 
> > I guess the question is, can this hurt existing devices?
> > 
> > As Quentin mentioned, this only affects DTs that get handed over from
> > U-Boot to TF-A (and maybe OP-TEE).
> > 
> > So the whole range of things loading their DT from extlinux.conf or
> > whatever are not really affected.
> > 
> > 
> > DTs U-Boot can hand over are 2 types,
> > (1) built from within u-boot and
> > (2) stored somewhere centrally (SPI flash).
> > 
> > 
> > Case (1) is again not affected, as U-Boot (and other bootloaders) may
> > very well sync the DTS files, but generally not the build-system, so if
> > U-Boot (or any other bootloader) creates DTBs with symbols is completely
> > their own choice.
> > 
> > 
> > And for case (2) I see the manufacturer being responsible. Having the DT
> > in central storage makes it somewhat part of a "bios"-level in the hirarchy
> > and the general guarantee is that new software _will work_ with older DTs,
> > but the other way around is more a nice to have (old SW with new DTB).
> > 
> > So if some manufacturer has a centrally located DTB this does not matter
> > until they upgrade, and when that happens I do expect testing to happen
> > at the manufacturers side, before rolling out a "bios update"
> 
> Personally, I'm all for letting the kernel build the DTBs with symbols.
> 
> (I have a patch that I keep rebasing on my tree only for that purpose.
> Sure, I could modify my build scripts to build the DTB separately,
> but with this patch, I do not need to do anything since the kernel
> builds the DTBs already.)
> 
> Other platforms, e.g. TI already build many boards with symbols:
> https://github.com/torvalds/linux/blob/v6.13/arch/arm64/boot/dts/ti/Makefile#L242-L261
> 
> 
> You seems to have been against enabling symbols before:
> https://lore.kernel.org/linux-rockchip/171941553475.921128.9467465539299233735.b4-ty@sntech.de/
> https://lore.kernel.org/linux-rockchip/1952472.6tgchFWduM@diego/
> 
> But if you have changed you mind, and you are no longer concerned about
> doing so, then in my own self-interest I'm all for it :)

I'm all for keeping compatibility as good as possible and that issue came
on the table way too often already ;-) . In the past it was essentially easy
to go with "just don't enable symbols" and not go down the nitty-gritty
detail route - because that whole mesh of different firmware combinations
gives me a headache ;-) [0]

So finally going through those possible affected variants gave me those
thoughts of "is there even an actual problem with existing boards?".
Especially wrt forward<->backwards compatibility.

Outcome is, I'm definitly not sure about myself, but also could not come
up with an actual scenario. But that compile-time testing of applying
DTBOs is way too great to pass up on :-)


Heiko


[0] If just some vendor would directly work on upstream TF-A from the
beginning, instead of hacking up some half-decade old ATF  ... ;-)