Regressions that cause a device to no longer be probed by a driver can have a big impact on the platform's functionality, and despite being relatively common there isn't currently any generic test to detect them. As an example, bootrr [1] does test for device probe, but it requires defining the expected probed devices for each platform.
Given that the Devicetree already provides a static description of devices on the system, it is a good basis for building such a test on top.
This series introduces a test to catch regressions that prevent devices from probing.
Patches 1 and 2 extend the existing dt-extract-compatibles to be able to output only the compatibles that can be expected to match a Devicetree node to a driver. Patch 2 adds a kselftest that walks over the Devicetree nodes on the current platform and compares the compatibles to the ones on the list, and on an ignore list, to point out devices that failed to be probed.
A compatible list is needed because not all compatibles that can show up in a Devicetree node can be used to match to a driver, for example the code for that compatible might use "OF_DECLARE" type macros and avoid the driver framework, or the node might be controlled by a driver that was bound to a different node.
An ignore list is needed for the few cases where it's common for a driver to match a device but not probe, like for the "simple-mfd" compatible, where the driver only probes if that compatible is the node's first compatible.
The reason for parsing the kernel source instead of relying on information exposed by the kernel at runtime (say, looking at modaliases or introducing some other mechanism), is to be able to catch issues where a config was renamed or a driver moved across configs, and the .config used by the kernel not updated accordingly. We need to parse the source to find all compatibles present in the kernel independent of the current config being run.
[1] https://github.com/kernelci/bootrr
Changes in v2: - Extended dt-extract-compatibles script to be able to extract driver matching compatibles, instead of adding a new one in Coccinelle - Made kselftest output in the KTAP format
Nícolas F. R. A. Prado (3): dt: dt-extract-compatibles: Handle cfile arguments in generator function dt: dt-extract-compatibles: Add flag for driver matching compatibles kselftest: Add new test for detecting unprobed Devicetree devices
scripts/dtc/dt-extract-compatibles | 74 +++++++++++++---- 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 +++++++++++++++++++ 7 files changed, 218 insertions(+), 16 deletions(-) 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
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
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}"
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 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 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" # optionallocal directive_str=[[ ! -z "$directive" ]] && directive_str="# $directive"echo $result $KTAP_TESTNO $description $directive_strKTAP_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); donode=$(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); doif [[ -d "$(dirname "${uevent}")"/driver ]]; thengrep '^OF_FULLNAME=' "${uevent}" | sed -e 's|OF_FULLNAME=||'fidone)+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}( |\$)"; thencompatibles=$(tr '\000' '\n' < "${PDT}"/"${node}"/compatible)for compatible in ${compatibles}; doif grep -x -q "${compatible}" "${IGNORE_LIST}"; thencontinuefiif grep -x -q "${compatible}" "${COMPAT_LIST}"; thenktap_test_fail "${node}"retval="${KSFT_FAIL}"continue 2fidonektap_test_skip "${node}"elsektap_test_pass "${node}"fi+done
+exit "${retval}"
2.41.0
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); donode=$(dirname "${node_compat}")# Check if node is available[[ -e "${node}"/status && $(tr -d '\000' < "${node}"/status) != "okay" ]] && continueNote 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
linux-kselftest-mirror@lists.linaro.org
-
Mark Brown -
Nícolas F. R. A. Prado -
Rob Herring -
Shuah Khan