| Message ID | 20230817233635.2306377-4-nfraprado@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add a test to catch unprobed Devicetree devices | expand |
On Thu, Aug 17, 2023 at 07:35:27PM -0400, Nícolas F. R. A. Prado wrote: > --- /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 > +# These look generic so could be at the top level kselftest directory in case any other tests want to use them? The test itself looks good in so far as I can read shell.
On Thu, Aug 17, 2023 at 6:36 PM Nícolas 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ícolas 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/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 >
On 8/18/23 09:08, Nícolas F. R. A. Prado wrote: > On Fri, Aug 18, 2023 at 01:54:21PM +0100, Mark Brown wrote: >> On Thu, Aug 17, 2023 at 07:35:27PM -0400, Nícolas F. R. A. Prado wrote: >> >>> --- /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 >>> +# >> >> These look generic so could be at the top level kselftest directory in >> case any other tests want to use them? > > Yes, they're generic. And sure, we can move it up. The tests using it will need > to source it at run-time, so we can either update the kselftest Makefile to > always copy this helper when installing, or each test's Makefile can > make its own copy during build. > Moving this up would require the above changes. I prefer making these later after this test goes in to avoid conflicts with linux-kselftest next and Rob's dt as this one depends on patches 1&2 which aren't in my Inbox. I would like also to see a common solution that works for C and shell tests. Sourcing works just for shell tests. >> >> The test itself looks good in so far as I can read shell. > > Thanks for the feedback! > Rob, Are you planning to take this through your tree. If you do, here is my Reviewed-by Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> thanks, -- Shuah
On Fri, Aug 18, 2023 at 11:38 AM Nícolas F. R. A. Prado <nfraprado@collabora.com> wrote: > > On Fri, Aug 18, 2023 at 10:05:00AM -0500, Rob Herring wrote: > > On Thu, Aug 17, 2023 at 6:36 PM Nícolas F. R. A. Prado > > <nfraprado@collabora.com> wrote: > [..] > > > 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. > > OK. > > > > > > .../selftests/dt/compatible_ignore_list | 1 + > > > tools/testing/selftests/dt/ktap_helpers.sh | 57 +++++++++++++ > > > > As Mark said, looks common. > > Yes, I'll move it one folder up. > > > > [..] > > > --- /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? > > I've just double-checked that compatible_list does get removed with make clean. > Not because it is in this gitignore, but because it is listed in TEST_GEN_FILES > in the Makefile. > > > > [..] > > > --- /dev/null > > > +++ b/tools/testing/selftests/dt/test_unprobed_devices.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. > > The /sys/firmware/devicetree/* entry in > Documentation/ABI/testing/sysfs-firmware-ofw reads: > > Userspace must not use the /sys/firmware/devicetree/base > path directly, but instead should follow /proc/device-tree > symlink. It is possible that the absolute path will change > in the future, but the symlink is the stable ABI. > > So is this information outdated? No, my memory is just wrong... Rob
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..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/ +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 + 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}"
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> --- 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 +++++ .../selftests/dt/compatible_ignore_list | 1 + tools/testing/selftests/dt/ktap_helpers.sh | 57 +++++++++++++ .../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