diff mbox series

[v3,3/3] kselftest: Add new test for detecting unprobed Devicetree devices

Message ID 20230828211424.2964562-4-nfraprado@collabora.com (mailing list archive)
State Accepted
Commit 14571ab1ad213de59b3726a40aea7ca0365bf445
Headers show
Series Add a test to catch unprobed Devicetree devices | expand

Commit Message

Nícolas F. R. A. Prado Aug. 28, 2023, 9:13 p.m. UTC
Introduce a new kselftest to detect devices that were declared in the
Devicetree, and are expected to be probed by a driver, but weren't.

The test uses two lists: a list of compatibles that can match a
Devicetree device to a driver, and a list of compatibles that should be
ignored. The first is automatically generated by the
dt-extract-compatibles script, and is run as part of building this test.
The list of compatibles to ignore is a hand-crafted list to capture the
few exceptions of compatibles that are expected to match a driver but
not be bound to it.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

---

Changes in v3:
- Added DT selftest path to MAINTAINERS
- Enabled test for nodes with 'status = "ok"'
- Added pass/fail/skip totals to end of test output

Changes in v2:
- Switched output to be in KTAP format
- Changed Makefile to make use of the dt-extract-compatibles instead of
  the Coccinelle script
- Dropped compatibles from compatible_ignore_list that are now already
  filtered out by extraction script
- Added early exit if /proc/device-tree is not present

 MAINTAINERS                                   |  1 +
 tools/testing/selftests/Makefile              |  1 +
 tools/testing/selftests/dt/.gitignore         |  1 +
 tools/testing/selftests/dt/Makefile           | 21 +++++
 .../selftests/dt/compatible_ignore_list       |  1 +
 tools/testing/selftests/dt/ktap_helpers.sh    | 70 ++++++++++++++++
 .../selftests/dt/test_unprobed_devices.sh     | 83 +++++++++++++++++++
 7 files changed, 178 insertions(+)
 create mode 100644 tools/testing/selftests/dt/.gitignore
 create mode 100644 tools/testing/selftests/dt/Makefile
 create mode 100644 tools/testing/selftests/dt/compatible_ignore_list
 create mode 100644 tools/testing/selftests/dt/ktap_helpers.sh
 create mode 100755 tools/testing/selftests/dt/test_unprobed_devices.sh

Comments

Aishwarya TCV Nov. 2, 2023, 12:11 p.m. UTC | #1
On 28/08/2023 22:13, Nícolas F. R. A. Prado wrote:
> Introduce a new kselftest to detect devices that were declared in the
> Devicetree, and are expected to be probed by a driver, but weren't.
> 
> The test uses two lists: a list of compatibles that can match a
> Devicetree device to a driver, and a list of compatibles that should be
> ignored. The first is automatically generated by the
> dt-extract-compatibles script, and is run as part of building this test.
> The list of compatibles to ignore is a hand-crafted list to capture the
> few exceptions of compatibles that are expected to match a driver but
> not be bound to it.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
> 

Hi Nicolas,

Currently when building kselftest against next-master and
mainline-master the below build error is observed. A bisect (full log
below) identified this patch as introducing the failure.

Full log from a failure:

https://storage.kernelci.org/mainline/master/v6.6-9152-gdeefd5024f07/arm64/defconfig%2Bkselftest/gcc-10/logs/kselftest.log

make[4]: Entering directory '/tmp/kci/linux/tools/testing/selftests/dt'
/tmp/kci/linux/tools/testing/selftests/../../../scripts/dtc/dt-extract-compatibles
-d /tmp/kci/linux/tools/testing/selftests/../../.. >
/tmp/kci/linux/build/kselftest/dt/compatible_list
Traceback (most recent call last):
  File
"/tmp/kci/linux/tools/testing/selftests/../../../scripts/dtc/dt-extract-compatibles",
line 107, in <module>
    compat_ignore_list.extend(parse_compatibles_to_ignore(f))
  File
"/tmp/kci/linux/tools/testing/selftests/../../../scripts/dtc/dt-extract-compatibles",
line 67, in parse_compatibles_to_ignore
    with open(file, 'r', encoding='utf-8') as f:
OSError: [Errno 40] Too many levels of symbolic links:
'/tmp/kci/linux/tools/testing/selftests/../../../build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/tools/testing/selftests/powerpc/vphn/vphn.c'
make[4]: *** [Makefile:12:
/tmp/kci/linux/build/kselftest/dt/compatible_list] Error 1
make[4]: Leaving directory '/tmp/kci/linux/tools/testing/selftests/dt'


The bisect log:

git bisect start
# good: [f9a7eda4d73d44dc1d17d05cdc9aeb9fc5660740] Merge tag
'hwmon-for-v6.7' of
git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging
git bisect good f9a7eda4d73d44dc1d17d05cdc9aeb9fc5660740
# bad: [8bc9e6515183935fa0cccaf67455c439afe4982b] Merge tag
'devicetree-for-6.7' of
git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
git bisect bad 8bc9e6515183935fa0cccaf67455c439afe4982b
# good: [0a6d7f8275f255eda823c0f0b61d024f6f5b483d] Merge branch
'clk-cleanup' into clk-next
git bisect good 0a6d7f8275f255eda823c0f0b61d024f6f5b483d
# good: [fe4ae2fab00b4751265580c5865fdf23b62d80b3] Merge tag
'clk-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux
git bisect good fe4ae2fab00b4751265580c5865fdf23b62d80b3
# good: [fc7b34ae1347f4eb36f065458e53d6065cd85928] power: supply:
wm831x_backup: Convert to platform remove callback returning void
git bisect good fc7b34ae1347f4eb36f065458e53d6065cd85928
# bad: [f2147371a83c6de1128093c163dc17bc61096362] dt-bindings: soc: fsl:
cpm_qe: cpm1-scc-qmc: Fix example property name
git bisect bad f2147371a83c6de1128093c163dc17bc61096362
# bad: [22c3888e55bf5c86be536a3d9e06a50e7bf3a39f] dt-bindings: watchdog:
atmel,at91rm9200-wdt: convert txt to yaml
git bisect bad 22c3888e55bf5c86be536a3d9e06a50e7bf3a39f
# bad: [780967feb626c6f4efa1e4b3532f1be83884cd76] dt-bindings: Add
Marantec vendor prefix
git bisect bad 780967feb626c6f4efa1e4b3532f1be83884cd76
# bad: [bc17fd92c1eb7589f1f3df1893e9f62bb35b8cc8] dt-bindings:
interrupt-controller: qcom,pdc: document qcom,sm4450-pdc
git bisect bad bc17fd92c1eb7589f1f3df1893e9f62bb35b8cc8
# good: [365ba0c7a73cce407bf40cdf9900b86b945d4acb] dt:
dt-extract-compatibles: Add flag for driver matching compatibles
git bisect good 365ba0c7a73cce407bf40cdf9900b86b945d4acb
# bad: [14571ab1ad213de59b3726a40aea7ca0365bf445] kselftest: Add new
test for detecting unprobed Devicetree devices
git bisect bad 14571ab1ad213de59b3726a40aea7ca0365bf445
# first bad commit: [14571ab1ad213de59b3726a40aea7ca0365bf445]
kselftest: Add new test for detecting unprobed Devicetree devices


Thanks,
Aishwarya
Naresh Kamboju Nov. 2, 2023, 1:45 p.m. UTC | #2
Hi Aishwarya,

On Thu, 2 Nov 2023 at 17:41, Aishwarya TCV <aishwarya.tcv@arm.com> wrote:
>
>
>
> On 28/08/2023 22:13, Nícolas F. R. A. Prado wrote:
> > Introduce a new kselftest to detect devices that were declared in the
> > Devicetree, and are expected to be probed by a driver, but weren't.
> >
> > The test uses two lists: a list of compatibles that can match a
> > Devicetree device to a driver, and a list of compatibles that should be
> > ignored. The first is automatically generated by the
> > dt-extract-compatibles script, and is run as part of building this test.
> > The list of compatibles to ignore is a hand-crafted list to capture the
> > few exceptions of compatibles that are expected to match a driver but
> > not be bound to it.
> >
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
> >
>
> Hi Nicolas,
>
> Currently when building kselftest against next-master and
> mainline-master the below build error is observed. A bisect (full log
> below) identified this patch as introducing the failure.
>
> Full log from a failure:
>
> https://storage.kernelci.org/mainline/master/v6.6-9152-gdeefd5024f07/arm64/defconfig%2Bkselftest/gcc-10/logs/kselftest.log
>
> make[4]: Entering directory '/tmp/kci/linux/tools/testing/selftests/dt'
> /tmp/kci/linux/tools/testing/selftests/../../../scripts/dtc/dt-extract-compatibles
> -d /tmp/kci/linux/tools/testing/selftests/../../.. >
> /tmp/kci/linux/build/kselftest/dt/compatible_list
> Traceback (most recent call last):
>   File
> "/tmp/kci/linux/tools/testing/selftests/../../../scripts/dtc/dt-extract-compatibles",
> line 107, in <module>
>     compat_ignore_list.extend(parse_compatibles_to_ignore(f))
>   File
> "/tmp/kci/linux/tools/testing/selftests/../../../scripts/dtc/dt-extract-compatibles",
> line 67, in parse_compatibles_to_ignore
>     with open(file, 'r', encoding='utf-8') as f:
> OSError: [Errno 40] Too many levels of symbolic links:

OSError: [Errno 40] Too many levels of symbolic links:
This is not related to selftests/dt tests build.

May be due to, A loop of symlinks that are pointing to self / same files ?

> '/tmp/kci/linux/tools/testing/selftests/../../../build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/tools/testing/selftests/powerpc/vphn/vphn.c'
> make[4]: *** [Makefile:12:
> /tmp/kci/linux/build/kselftest/dt/compatible_list] Error 1
> make[4]: Leaving directory '/tmp/kci/linux/tools/testing/selftests/dt'

Here is the log showing selftests/dt build pass for arm64 build from
Linux next master branch.

Links to the successful build and kselftest.tar file shared below [1].

Build log:
========
make[4]: Entering directory 'tools/testing/selftests/dt'
tools/testing/selftests/../../../scripts/dtc/dt-extract-compatibles -d
tools/testing/selftests/../../.. >
/home/tuxbuild/.cache/tuxmake/builds/1/build/kselftest/dt/compatible_list
make[4]: Leaving directory 'tools/testing/selftests/dt

Please build by using tuxmake and validate builds are working.

steps to reproduce:
======
#!/bin/sh

# TuxMake is a command line tool and Python library that provides
# portable and repeatable Linux kernel builds across a variety of
# architectures, toolchains, kernel configurations, and make targets.
#
# TuxMake supports the concept of runtimes.
# See https://docs.tuxmake.org/runtimes/, for that to work it requires
# that you install podman or docker on your system.
#
# To install tuxmake to your home directory at ~/.local/bin:
# pip3 install -U --user tuxmake
#
# Or install a deb/rpm depending on the running distribution
# See https://tuxmake.org/install-deb/ or
# https://tuxmake.org/install-rpm/
#
# See https://docs.tuxmake.org/ for complete documentation.
# Original tuxmake command with fragments listed below.
# tuxmake --runtime podman --target-arch arm64 --toolchain gcc-13
--kconfig defconfig --kconfig-add
https://raw.githubusercontent.com/Linaro/meta-lkft/kirkstone/meta/recipes-kernel/linux/files/systemd.config
--kconfig-add CONFIG_BCMGENET=y --kconfig-add
tools/testing/selftests/cgroup/config --kconfig-add
tools/testing/selftests/cpufreq/config --kconfig-add
tools/testing/selftests/efivarfs/config --kconfig-add
tools/testing/selftests/filesystems/binderfs/config --kconfig-add
tools/testing/selftests/filesystems/fat/config --kconfig-add
tools/testing/selftests/firmware/config --kconfig-add
tools/testing/selftests/ftrace/config --kconfig-add
tools/testing/selftests/gpio/config --kconfig-add
tools/testing/selftests/ipc/config --kconfig-add
tools/testing/selftests/memfd/config dtbs dtbs-legacy headers kernel
kselftest modules


tuxmake --runtime podman --target-arch arm64 --toolchain gcc-13
--kconfig https://storage.tuxsuite.com/public/linaro/lkft/builds/2XYjd2yxHiay3gVALCGpAch4G8o/config
dtbs dtbs-legacy headers kernel kselftest modules


Build links:
 [1] https://storage.tuxsuite.com/public/linaro/lkft/builds/2XYjd2yxHiay3gVALCGpAch4G8o/

- Naresh
Mark Brown Nov. 2, 2023, 5:36 p.m. UTC | #3
On Thu, Nov 02, 2023 at 07:15:58PM +0530, Naresh Kamboju wrote:
> On Thu, 2 Nov 2023 at 17:41, Aishwarya TCV <aishwarya.tcv@arm.com> wrote:

> > https://storage.kernelci.org/mainline/master/v6.6-9152-gdeefd5024f07/arm64/defconfig%2Bkselftest/gcc-10/logs/kselftest.log

...

> May be due to, A loop of symlinks that are pointing to self / same files ?

Right, it does look like something bad is going on with symlinks:

> > '/tmp/kci/linux/tools/testing/selftests/../../../build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/tools/testing/selftests/powerpc/vphn/vphn.c'

> Please build by using tuxmake and validate builds are working.

Note that tuxmake does an in tree build of kselftest:

  make --silent --keep-going --jobs=8 O=/home/tuxbuild/.cache/tuxmake/builds/1/build INSTALL_PATH=/home/tuxbuild/.cache/tuxmake/builds/1/build/kselftest_install ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- 'CC=sccache aarch64-linux-gnu-gcc' 'HOSTCC=sccache gcc' kselftest-install

and does it's own tarball build too, whereas kernelci does an out of
tree build and uses kselftest-gen_tar:

  make KBUILD_BUILD_USER=KernelCI FORMAT=.xz ARCH=arm64 HOSTCC=gcc CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- CC="ccache aarch64-linux-gnu-gcc" O=/tmp/kci/linux/build -C/tmp/kci/linux -j10 kselftest-gen_tar

and that the error is in the dt-extract-compatibles program which is
part of the kernel (well, imported into the kernel from dtc upstream):

  File "/tmp/kci/linux/tools/testing/selftests/../../../scripts/dtc/dt-extract-compatibles", line 107, in <module>
    compat_ignore_list.extend(parse_compatibles_to_ignore(f))

This all suggests that something to do with how the build is set up is
resulting in the source symlink that gets created for out of tree builds
blowing up, I guess it's not specifically the DT stuff that's blowing it
up but rather that it's tripping over an existing bug.  Really does look
like a legitimate bug though, the source link is set up by the in tree
kernel build infrastructure.

I did poke a bit at reproducing outside of the KernelCI scripts but
didn't manage to yet.
Rob Herring Nov. 6, 2023, 5:09 p.m. UTC | #4
On Thu, Nov 2, 2023 at 12:36 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Nov 02, 2023 at 07:15:58PM +0530, Naresh Kamboju wrote:
> > On Thu, 2 Nov 2023 at 17:41, Aishwarya TCV <aishwarya.tcv@arm.com> wrote:
>
> > > https://storage.kernelci.org/mainline/master/v6.6-9152-gdeefd5024f07/arm64/defconfig%2Bkselftest/gcc-10/logs/kselftest.log
>
> ...
>
> > May be due to, A loop of symlinks that are pointing to self / same files ?
>
> Right, it does look like something bad is going on with symlinks:
>
> > > '/tmp/kci/linux/tools/testing/selftests/../../../build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/tools/testing/selftests/powerpc/vphn/vphn.c'
>
> > Please build by using tuxmake and validate builds are working.
>
> Note that tuxmake does an in tree build of kselftest:
>
>   make --silent --keep-going --jobs=8 O=/home/tuxbuild/.cache/tuxmake/builds/1/build INSTALL_PATH=/home/tuxbuild/.cache/tuxmake/builds/1/build/kselftest_install ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- 'CC=sccache aarch64-linux-gnu-gcc' 'HOSTCC=sccache gcc' kselftest-install
>
> and does it's own tarball build too, whereas kernelci does an out of
> tree build and uses kselftest-gen_tar:
>
>   make KBUILD_BUILD_USER=KernelCI FORMAT=.xz ARCH=arm64 HOSTCC=gcc CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- CC="ccache aarch64-linux-gnu-gcc" O=/tmp/kci/linux/build -C/tmp/kci/linux -j10 kselftest-gen_tar
>
> and that the error is in the dt-extract-compatibles program which is
> part of the kernel (well, imported into the kernel from dtc upstream):
>
>   File "/tmp/kci/linux/tools/testing/selftests/../../../scripts/dtc/dt-extract-compatibles", line 107, in <module>
>     compat_ignore_list.extend(parse_compatibles_to_ignore(f))
>
> This all suggests that something to do with how the build is set up is
> resulting in the source symlink that gets created for out of tree builds
> blowing up, I guess it's not specifically the DT stuff that's blowing it
> up but rather that it's tripping over an existing bug.  Really does look
> like a legitimate bug though, the source link is set up by the in tree
> kernel build infrastructure.
>
> I did poke a bit at reproducing outside of the KernelCI scripts but
> didn't manage to yet.

I can repro with "make dt_compatible_check". The problem is with an
'out of tree' build within the tree. That's my normal setup, but the
difference is I have ".build" directories. If I use "build" instead,
then I can repro. The issue is the iglob will recurse into "build" but
not hidden directories (by default). There's no option to not follow
symlinks which would solve this (there is an open python issue since
2017 to add it). I don't see a simple solution in python other than
getting a full list with glob(), convert to absolute paths, and remove
duplicates. I imagine that will be somewhat slow.

A simple solution would be instead of passing the source tree root to
dt-extract-compatibles, pass 'arch', 'drivers', and 'sound' instead.
There shouldn't be compatibles anywhere else.

Rob
Mark Brown Nov. 7, 2023, 2:36 p.m. UTC | #5
On Mon, Nov 06, 2023 at 11:09:44AM -0600, Rob Herring wrote:

> A simple solution would be instead of passing the source tree root to
> dt-extract-compatibles, pass 'arch', 'drivers', and 'sound' instead.
> There shouldn't be compatibles anywhere else.

This does seem like a reasonable quick fix that avoids the issue for
now - nothing would stop someone implementing a more complete solution
later.
Nícolas F. R. A. Prado Nov. 7, 2023, 11:02 p.m. UTC | #6
On Mon, Nov 06, 2023 at 11:09:44AM -0600, Rob Herring wrote:
> On Thu, Nov 2, 2023 at 12:36 PM Mark Brown <broonie@kernel.org> wrote:
> >
> > On Thu, Nov 02, 2023 at 07:15:58PM +0530, Naresh Kamboju wrote:
> > > On Thu, 2 Nov 2023 at 17:41, Aishwarya TCV <aishwarya.tcv@arm.com> wrote:
> >
> > > > https://storage.kernelci.org/mainline/master/v6.6-9152-gdeefd5024f07/arm64/defconfig%2Bkselftest/gcc-10/logs/kselftest.log
> >
> > ...
> >
> > > May be due to, A loop of symlinks that are pointing to self / same files ?
> >
> > Right, it does look like something bad is going on with symlinks:
> >
> > > > '/tmp/kci/linux/tools/testing/selftests/../../../build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/build/source/tools/testing/selftests/powerpc/vphn/vphn.c'
> >
> > > Please build by using tuxmake and validate builds are working.
> >
> > Note that tuxmake does an in tree build of kselftest:
> >
> >   make --silent --keep-going --jobs=8 O=/home/tuxbuild/.cache/tuxmake/builds/1/build INSTALL_PATH=/home/tuxbuild/.cache/tuxmake/builds/1/build/kselftest_install ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- 'CC=sccache aarch64-linux-gnu-gcc' 'HOSTCC=sccache gcc' kselftest-install
> >
> > and does it's own tarball build too, whereas kernelci does an out of
> > tree build and uses kselftest-gen_tar:
> >
> >   make KBUILD_BUILD_USER=KernelCI FORMAT=.xz ARCH=arm64 HOSTCC=gcc CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- CC="ccache aarch64-linux-gnu-gcc" O=/tmp/kci/linux/build -C/tmp/kci/linux -j10 kselftest-gen_tar
> >
> > and that the error is in the dt-extract-compatibles program which is
> > part of the kernel (well, imported into the kernel from dtc upstream):
> >
> >   File "/tmp/kci/linux/tools/testing/selftests/../../../scripts/dtc/dt-extract-compatibles", line 107, in <module>
> >     compat_ignore_list.extend(parse_compatibles_to_ignore(f))
> >
> > This all suggests that something to do with how the build is set up is
> > resulting in the source symlink that gets created for out of tree builds
> > blowing up, I guess it's not specifically the DT stuff that's blowing it
> > up but rather that it's tripping over an existing bug.  Really does look
> > like a legitimate bug though, the source link is set up by the in tree
> > kernel build infrastructure.
> >
> > I did poke a bit at reproducing outside of the KernelCI scripts but
> > didn't manage to yet.
> 
> I can repro with "make dt_compatible_check". The problem is with an
> 'out of tree' build within the tree. That's my normal setup, but the
> difference is I have ".build" directories. If I use "build" instead,
> then I can repro. The issue is the iglob will recurse into "build" but
> not hidden directories (by default). There's no option to not follow
> symlinks which would solve this (there is an open python issue since
> 2017 to add it). I don't see a simple solution in python other than
> getting a full list with glob(), convert to absolute paths, and remove
> duplicates. I imagine that will be somewhat slow.

Hi, sorry for the delay, I was on vacation last week.

I was able to reproduce the issue the way you described. And I also suspected
an alternative approach would be slower, but after trying it out it ran just as
fast as the current one, even on cold cache, so I sent it out:

https://lore.kernel.org/all/20231107225624.9811-1-nfraprado@collabora.com

Let me know your thoughts there.

Thanks,
Nícolas
Mark Brown Dec. 7, 2023, 8:18 p.m. UTC | #7
On Mon, Aug 28, 2023 at 05:13:12PM -0400, Nícolas F. R. A. Prado wrote:
> Introduce a new kselftest to detect devices that were declared in the
> Devicetree, and are expected to be probed by a driver, but weren't.

I've been running this in my personal CI for a little while now and I'm
finding it's pretty marginal for the 45 second default timeout in
kselftest on some platforms, especially BeagleBone Black though it's not
just that.  BBB is both slow and has a comprehensive DT which won't help
matters, there's 253 devices.

I'm running it from nfsroot which is going to be part of the problem but
shouldn't be too bad since we're mainly dealing with proc and sysfs and
hopefully mostly running cached binaries, I'm also using a serial
console to get the output which is going to add overhead especially with
a large number odevices with length names.  I'm not sure what the best
solution is here - a quick glance at the code doesn't ring any alarm
bells for me, this may just be a reasonable runtime for the test.
Nícolas F. R. A. Prado Dec. 8, 2023, 1:45 p.m. UTC | #8
On Thu, Dec 07, 2023 at 08:18:49PM +0000, Mark Brown wrote:
> On Mon, Aug 28, 2023 at 05:13:12PM -0400, Nícolas F. R. A. Prado wrote:
> > Introduce a new kselftest to detect devices that were declared in the
> > Devicetree, and are expected to be probed by a driver, but weren't.
> 
> I've been running this in my personal CI for a little while now and I'm
> finding it's pretty marginal for the 45 second default timeout in
> kselftest on some platforms, especially BeagleBone Black though it's not
> just that.  BBB is both slow and has a comprehensive DT which won't help
> matters, there's 253 devices.
> 
> I'm running it from nfsroot which is going to be part of the problem but
> shouldn't be too bad since we're mainly dealing with proc and sysfs and
> hopefully mostly running cached binaries, I'm also using a serial
> console to get the output which is going to add overhead especially with
> a large number odevices with length names.  I'm not sure what the best
> solution is here - a quick glance at the code doesn't ring any alarm
> bells for me, this may just be a reasonable runtime for the test.

Thanks for reporting this. I've experimented a bit and was able to find an
effective optimization and CC'ed it to you [1]. Hopefully it is as effective for
your board and setup as it is for mine. Let me know there.

[1] https://lore.kernel.org/all/20231208133955.483851-1-nfraprado@collabora.com

Thanks,
Nícolas
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index ff1f273b4f36..4f5c13349eb8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15969,6 +15969,7 @@  F:	Documentation/ABI/testing/sysfs-firmware-ofw
 F:	drivers/of/
 F:	include/linux/of*.h
 F:	scripts/dtc/
+F:	tools/testing/selftests/dt/
 K:	of_overlay_notifier_
 K:	of_overlay_fdt_apply
 K:	of_overlay_remove
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 42806add0114..e8823097698c 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -18,6 +18,7 @@  TARGETS += drivers/dma-buf
 TARGETS += drivers/s390x/uvdevice
 TARGETS += drivers/net/bonding
 TARGETS += drivers/net/team
+TARGETS += dt
 TARGETS += efivarfs
 TARGETS += exec
 TARGETS += fchmodat2
diff --git a/tools/testing/selftests/dt/.gitignore b/tools/testing/selftests/dt/.gitignore
new file mode 100644
index 000000000000..f6476c9f2884
--- /dev/null
+++ b/tools/testing/selftests/dt/.gitignore
@@ -0,0 +1 @@ 
+compatible_list
diff --git a/tools/testing/selftests/dt/Makefile b/tools/testing/selftests/dt/Makefile
new file mode 100644
index 000000000000..62dc00ee4978
--- /dev/null
+++ b/tools/testing/selftests/dt/Makefile
@@ -0,0 +1,21 @@ 
+PY3 = $(shell which python3 2>/dev/null)
+
+ifneq ($(PY3),)
+
+TEST_PROGS := test_unprobed_devices.sh
+TEST_GEN_FILES := compatible_list
+TEST_FILES := compatible_ignore_list ktap_helpers.sh
+
+include ../lib.mk
+
+$(OUTPUT)/compatible_list:
+	$(top_srcdir)/scripts/dtc/dt-extract-compatibles -d $(top_srcdir) > $@
+
+else
+
+all: no_py3_warning
+
+no_py3_warning:
+	@echo "Missing python3. This test will be skipped."
+
+endif
diff --git a/tools/testing/selftests/dt/compatible_ignore_list b/tools/testing/selftests/dt/compatible_ignore_list
new file mode 100644
index 000000000000..1323903feca9
--- /dev/null
+++ b/tools/testing/selftests/dt/compatible_ignore_list
@@ -0,0 +1 @@ 
+simple-mfd
diff --git a/tools/testing/selftests/dt/ktap_helpers.sh b/tools/testing/selftests/dt/ktap_helpers.sh
new file mode 100644
index 000000000000..8dfae51bb4e2
--- /dev/null
+++ b/tools/testing/selftests/dt/ktap_helpers.sh
@@ -0,0 +1,70 @@ 
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (c) 2023 Collabora Ltd
+#
+# Helpers for outputting in KTAP format
+#
+KTAP_TESTNO=1
+KTAP_CNT_PASS=0
+KTAP_CNT_FAIL=0
+KTAP_CNT_SKIP=0
+
+ktap_print_header() {
+	echo "TAP version 13"
+}
+
+ktap_set_plan() {
+	num_tests="$1"
+
+	echo "1..$num_tests"
+}
+
+ktap_skip_all() {
+	echo -n "1..0 # SKIP "
+	echo $@
+}
+
+__ktap_test() {
+	result="$1"
+	description="$2"
+	directive="$3" # optional
+
+	local directive_str=
+	[[ ! -z "$directive" ]] && directive_str="# $directive"
+
+	echo $result $KTAP_TESTNO $description $directive_str
+
+	KTAP_TESTNO=$((KTAP_TESTNO+1))
+}
+
+ktap_test_pass() {
+	description="$1"
+
+	result="ok"
+	__ktap_test "$result" "$description"
+
+	KTAP_CNT_PASS=$((KTAP_CNT_PASS+1))
+}
+
+ktap_test_skip() {
+	description="$1"
+
+	result="ok"
+	directive="SKIP"
+	__ktap_test "$result" "$description" "$directive"
+
+	KTAP_CNT_SKIP=$((KTAP_CNT_SKIP+1))
+}
+
+ktap_test_fail() {
+	description="$1"
+
+	result="not ok"
+	__ktap_test "$result" "$description"
+
+	KTAP_CNT_FAIL=$((KTAP_CNT_FAIL+1))
+}
+
+ktap_print_totals() {
+	echo "# Totals: pass:$KTAP_CNT_PASS fail:$KTAP_CNT_FAIL xfail:0 xpass:0 skip:$KTAP_CNT_SKIP error:0"
+}
diff --git a/tools/testing/selftests/dt/test_unprobed_devices.sh b/tools/testing/selftests/dt/test_unprobed_devices.sh
new file mode 100755
index 000000000000..b07af2a4c4de
--- /dev/null
+++ b/tools/testing/selftests/dt/test_unprobed_devices.sh
@@ -0,0 +1,83 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (c) 2023 Collabora Ltd
+#
+# Based on Frank Rowand's dt_stat script.
+#
+# This script tests for devices that were declared on the Devicetree and are
+# expected to bind to a driver, but didn't.
+#
+# To achieve this, two lists are used:
+# * a list of the compatibles that can be matched by a Devicetree node
+# * a list of compatibles that should be ignored
+#
+
+DIR="$(dirname $(readlink -f "$0"))"
+
+source "${DIR}"/ktap_helpers.sh
+
+PDT=/proc/device-tree/
+COMPAT_LIST="${DIR}"/compatible_list
+IGNORE_LIST="${DIR}"/compatible_ignore_list
+
+KSFT_PASS=0
+KSFT_FAIL=1
+KSFT_SKIP=4
+
+ktap_print_header
+
+if [[ ! -d "${PDT}" ]]; then
+	ktap_skip_all "${PDT} doesn't exist."
+	exit "${KSFT_SKIP}"
+fi
+
+nodes_compatible=$(
+	for node_compat in $(find ${PDT} -name compatible); do
+		node=$(dirname "${node_compat}")
+		# Check if node is available
+		if [[ -e "${node}"/status ]]; then
+			status=$(tr -d '\000' < "${node}"/status)
+			[[ "${status}" != "okay" && "${status}" != "ok" ]] && continue
+		fi
+		echo "${node}" | sed -e 's|\/proc\/device-tree||'
+	done | sort
+	)
+
+nodes_dev_bound=$(
+	IFS=$'\n'
+	for uevent in $(find /sys/devices -name uevent); do
+		if [[ -d "$(dirname "${uevent}")"/driver ]]; then
+			grep '^OF_FULLNAME=' "${uevent}" | sed -e 's|OF_FULLNAME=||'
+		fi
+	done
+	)
+
+num_tests=$(echo ${nodes_compatible} | wc -w)
+ktap_set_plan "${num_tests}"
+
+retval="${KSFT_PASS}"
+for node in ${nodes_compatible}; do
+	if ! echo "${nodes_dev_bound}" | grep -E -q "(^| )${node}( |\$)"; then
+		compatibles=$(tr '\000' '\n' < "${PDT}"/"${node}"/compatible)
+
+		for compatible in ${compatibles}; do
+			if grep -x -q "${compatible}" "${IGNORE_LIST}"; then
+				continue
+			fi
+
+			if grep -x -q "${compatible}" "${COMPAT_LIST}"; then
+				ktap_test_fail "${node}"
+				retval="${KSFT_FAIL}"
+				continue 2
+			fi
+		done
+		ktap_test_skip "${node}"
+	else
+		ktap_test_pass "${node}"
+	fi
+
+done
+
+ktap_print_totals
+exit "${retval}"