2023-08-19 12:00:44

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices

On Thu, Aug 17, 2023 at 6:36 PM Nícolas F. R. A. Prado
<[email protected]> 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 <[email protected]>
>
> ---
>
> 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/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

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

> 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..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=1
> +
> +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_test_skip() {
> + description="$1"
> +
> + result="ok"
> + directive="SKIP"
> + __ktap_test "$result" "$description" "$directive"
> +}
> +
> +ktap_test_fail() {
> + description="$1"
> +
> + result="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="$(dirname $(readlink -f "$0"))"
> +
> +source "${DIR}"/ktap_helpers.sh
> +
> +PDT=/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="${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
> + [[ -e "${node}"/status && $(tr -d '\000' < "${node}"/status) != "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=$(
> + 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
> +
> +exit "${retval}"
> --
> 2.41.0
>