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
