Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230817233635.2306377-1-nfraprado@collabora.com> <20230817233635.2306377-4-nfraprado@collabora.com>
In-Reply-To: <20230817233635.2306377-4-nfraprado@collabora.com>
From:   Rob Herring <robh+dt@kernel.org>
Date:   Fri, 18 Aug 2023 10:05:00 -0500
Message-ID: <CAL_JsqLyimRvcnL_O8sMB8SnezXv3Z431VckMC-26w-aT1nYmQ@mail.gmail.com>
Subject: Re: [PATCH v2 3/3] kselftest: Add new test for detecting unprobed
 Devicetree devices
To:     =?UTF-8?B?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= 
        <nfraprado@collabora.com>
Cc:     Frank Rowand <frowand.list@gmail.com>,
        Shuah Khan <shuah@kernel.org>, Mark Brown <broonie@kernel.org>,
        kernelci@lists.linux.dev, Guenter Roeck <groeck@chromium.org>,
        kernel@collabora.com, Bjorn Andersson <andersson@kernel.org>,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Aug 17, 2023 at 6:36=E2=80=AFPM N=C3=ADcolas F. R. A. Prado
<nfraprado@collabora.com> 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=C3=ADcolas F. R. A. Prado <nfraprado@collabora.com>
>
> ---
>
> 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
>
>  tools/testing/selftests/Makefile              |  1 +
>  tools/testing/selftests/dt/.gitignore         |  1 +
>  tools/testing/selftests/dt/Makefile           | 21 +++++

Please add this path to DT maintainers entry.

>  .../selftests/dt/compatible_ignore_list       |  1 +
>  tools/testing/selftests/dt/ktap_helpers.sh    | 57 +++++++++++++

As Mark said, looks common.

>  .../selftests/dt/test_unprobed_devices.sh     | 79 +++++++++++++++++++
>  6 files changed, 160 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
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/M=
akefile
> index 42806add0114..e8823097698c 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -18,6 +18,7 @@ TARGETS +=3D drivers/dma-buf
>  TARGETS +=3D drivers/s390x/uvdevice
>  TARGETS +=3D drivers/net/bonding
>  TARGETS +=3D drivers/net/team
> +TARGETS +=3D dt
>  TARGETS +=3D efivarfs
>  TARGETS +=3D exec
>  TARGETS +=3D fchmodat2
> diff --git a/tools/testing/selftests/dt/.gitignore b/tools/testing/selfte=
sts/dt/.gitignore
> new file mode 100644
> index 000000000000..f6476c9f2884
> --- /dev/null
> +++ b/tools/testing/selftests/dt/.gitignore
> @@ -0,0 +1 @@
> +compatible_list

Not sure on the selftests, but is this enough that it gets cleaned?

> diff --git a/tools/testing/selftests/dt/Makefile b/tools/testing/selftest=
s/dt/Makefile
> new file mode 100644
> index 000000000000..62dc00ee4978
> --- /dev/null
> +++ b/tools/testing/selftests/dt/Makefile
> @@ -0,0 +1,21 @@
> +PY3 =3D $(shell which python3 2>/dev/null)
> +
> +ifneq ($(PY3),)
> +
> +TEST_PROGS :=3D test_unprobed_devices.sh
> +TEST_GEN_FILES :=3D compatible_list
> +TEST_FILES :=3D 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/te=
sting/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/s=
elftests/dt/ktap_helpers.sh
> new file mode 100644
> index 000000000000..27e89a31e602
> --- /dev/null
> +++ b/tools/testing/selftests/dt/ktap_helpers.sh
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (c) 2023 Collabora Ltd
> +#
> +# Helpers for outputting in KTAP format
> +#
> +KTAP_TESTNO=3D1
> +
> +ktap_print_header() {
> +       echo "TAP version 13"
> +}
> +
> +ktap_set_plan() {
> +       num_tests=3D"$1"
> +
> +       echo "1..$num_tests"
> +}
> +
> +ktap_skip_all() {
> +       echo -n "1..0 # SKIP "
> +       echo $@
> +}
> +
> +__ktap_test() {
> +       result=3D"$1"
> +       description=3D"$2"
> +       directive=3D"$3" # optional
> +
> +       local directive_str=3D
> +       [[ ! -z "$directive" ]] && directive_str=3D"# $directive"
> +
> +       echo $result $KTAP_TESTNO $description $directive_str
> +
> +       KTAP_TESTNO=3D$((KTAP_TESTNO+1))
> +}
> +
> +ktap_test_pass() {
> +       description=3D"$1"
> +
> +       result=3D"ok"
> +       __ktap_test "$result" "$description"
> +}
> +
> +ktap_test_skip() {
> +       description=3D"$1"
> +
> +       result=3D"ok"
> +       directive=3D"SKIP"
> +       __ktap_test "$result" "$description" "$directive"
> +}
> +
> +ktap_test_fail() {
> +       description=3D"$1"
> +
> +       result=3D"not ok"
> +       __ktap_test "$result" "$description"
> +}
> 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..b523767cdbfb
> --- /dev/null
> +++ b/tools/testing/selftests/dt/test_unprobed_devices.sh
> @@ -0,0 +1,79 @@
> +#!/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=3D"$(dirname $(readlink -f "$0"))"
> +
> +source "${DIR}"/ktap_helpers.sh
> +
> +PDT=3D/proc/device-tree/

This is considered the legacy path though we will probably never get
rid of it. Use the sysfs path instead.

> +COMPAT_LIST=3D"${DIR}"/compatible_list
> +IGNORE_LIST=3D"${DIR}"/compatible_ignore_list
> +
> +KSFT_PASS=3D0
> +KSFT_FAIL=3D1
> +KSFT_SKIP=3D4
> +
> +ktap_print_header
> +
> +if [[ ! -d "${PDT}" ]]; then
> +       ktap_skip_all "${PDT} doesn't exist."
> +       exit "${KSFT_SKIP}"
> +fi
> +
> +nodes_compatible=3D$(
> +       for node_compat in $(find ${PDT} -name compatible); do
> +               node=3D$(dirname "${node_compat}")
> +               # Check if node is available
> +               [[ -e "${node}"/status && $(tr -d '\000' < "${node}"/stat=
us) !=3D "okay" ]] && continue

Note that "ok" is accepted by the kernel and does show up some. But
for your use, probably okay as is.

> +               echo "${node}" | sed -e 's|\/proc\/device-tree||'
> +       done | sort
> +       )
> +
> +nodes_dev_bound=3D$(
> +       IFS=3D$'\n'
> +       for uevent in $(find /sys/devices -name uevent); do
> +               if [[ -d "$(dirname "${uevent}")"/driver ]]; then
> +                       grep '^OF_FULLNAME=3D' "${uevent}" | sed -e 's|OF=
_FULLNAME=3D||'
> +               fi
> +       done
> +       )
> +
> +num_tests=3D$(echo ${nodes_compatible} | wc -w)
> +ktap_set_plan "${num_tests}"
> +
> +retval=3D"${KSFT_PASS}"
> +for node in ${nodes_compatible}; do
> +       if ! echo "${nodes_dev_bound}" | grep -E -q "(^| )${node}( |\$)";=
 then
> +               compatibles=3D$(tr '\000' '\n' < "${PDT}"/"${node}"/compa=
tible)
> +
> +               for compatible in ${compatibles}; do
> +                       if grep -x -q "${compatible}" "${IGNORE_LIST}"; t=
hen
> +                               continue
> +                       fi
> +
> +                       if grep -x -q "${compatible}" "${COMPAT_LIST}"; t=
hen
> +                               ktap_test_fail "${node}"
> +                               retval=3D"${KSFT_FAIL}"
> +                               continue 2
> +                       fi
> +               done
> +               ktap_test_skip "${node}"
> +       else
> +               ktap_test_pass "${node}"
> +       fi
> +
> +done
> +
> +exit "${retval}"
> --
> 2.41.0
>