[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