| Message ID | 20230817233635.2306377-4-nfraprado@collabora.com (mailing list archive) |
|---|---|
| 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 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. > > The test itself looks good in so far as I can read shell. Thanks for the feedback! Thanks, Nícolas
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 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? > [..] > > +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. Right, good point. Actually I think we should probably check for "ok" here as well. Even though it's not valid based on the spec and not used in any upstream Devicetree anymore, it's still supported by the kernel. If this test was to be run with older or downstream Devicetrees, it should still detect that a node with 'status = "ok"' is not having its driver probed, rather than ignore it. Thanks, Nícolas > [..]
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