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
