* [PATCH v2 0/3] Add a test to catch unprobed Devicetree devices
@ 2023-08-17 23:35 Nícolas F. R. A. Prado
2023-08-17 23:35 ` [PATCH v2 1/3] dt: dt-extract-compatibles: Handle cfile arguments in generator function Nícolas F. R. A. Prado
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Nícolas F. R. A. Prado @ 2023-08-17 23:35 UTC (permalink / raw)
To: Rob Herring, Frank Rowand, Shuah Khan
Cc: Mark Brown, kernelci, Guenter Roeck, kernel, Bjorn Andersson,
Nícolas F. R. A. Prado, devicetree, linux-kernel,
linux-kselftest
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
--
2.41.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 1/3] dt: dt-extract-compatibles: Handle cfile arguments in generator function 2023-08-17 23:35 [PATCH v2 0/3] Add a test to catch unprobed Devicetree devices Nícolas F. R. A. Prado @ 2023-08-17 23:35 ` Nícolas F. R. A. Prado 2023-08-17 23:35 ` [PATCH v2 2/3] dt: dt-extract-compatibles: Add flag for driver matching compatibles Nícolas F. R. A. Prado 2023-08-17 23:35 ` [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices Nícolas F. R. A. Prado 2 siblings, 0 replies; 10+ messages in thread From: Nícolas F. R. A. Prado @ 2023-08-17 23:35 UTC (permalink / raw) To: Rob Herring, Frank Rowand, Shuah Khan Cc: Mark Brown, kernelci, Guenter Roeck, kernel, Bjorn Andersson, Nícolas F. R. A. Prado, devicetree, linux-kernel Move the handling of the cfile arguments to a separate generator function to avoid redundancy. Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- Changes in v2: - Added this commit scripts/dtc/dt-extract-compatibles | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/scripts/dtc/dt-extract-compatibles b/scripts/dtc/dt-extract-compatibles index 9df9f1face83..2b6d228602e8 100755 --- a/scripts/dtc/dt-extract-compatibles +++ b/scripts/dtc/dt-extract-compatibles @@ -49,6 +49,14 @@ def print_compat(filename, compatibles): else: print(*compatibles, sep='\n') +def files_to_parse(path_args): + for f in path_args: + if os.path.isdir(f): + for filename in glob.iglob(f + "/**/*.c", recursive=True): + yield filename + else: + yield f + show_filename = False if __name__ == "__main__": @@ -59,11 +67,6 @@ if __name__ == "__main__": show_filename = args.with_filename - for f in args.cfile: - if os.path.isdir(f): - for filename in glob.iglob(f + "/**/*.c", recursive=True): - compat_list = parse_compatibles(filename) - print_compat(filename, compat_list) - else: - compat_list = parse_compatibles(f) - print_compat(f, compat_list) + for f in files_to_parse(args.cfile): + compat_list = parse_compatibles(f) + print_compat(f, compat_list) -- 2.41.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] dt: dt-extract-compatibles: Add flag for driver matching compatibles 2023-08-17 23:35 [PATCH v2 0/3] Add a test to catch unprobed Devicetree devices Nícolas F. R. A. Prado 2023-08-17 23:35 ` [PATCH v2 1/3] dt: dt-extract-compatibles: Handle cfile arguments in generator function Nícolas F. R. A. Prado @ 2023-08-17 23:35 ` Nícolas F. R. A. Prado 2023-08-17 23:35 ` [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices Nícolas F. R. A. Prado 2 siblings, 0 replies; 10+ messages in thread From: Nícolas F. R. A. Prado @ 2023-08-17 23:35 UTC (permalink / raw) To: Rob Herring, Frank Rowand, Shuah Khan Cc: Mark Brown, kernelci, Guenter Roeck, kernel, Bjorn Andersson, Nícolas F. R. A. Prado, devicetree, linux-kernel Add a new flag, '--driver-match', to the dt-extract-compatibles script that causes it to only print out compatibles that are expected to match a driver. This output can then be used by tests to detect device probe failures. In order to filter the compatibles down to only ones that will match to a driver, the following is considered: - A compatible needs to show up in a driver's of_match_table for it to be matched to a driver - Compatibles that are used in both of_match_table and OF_DECLARE type macros can't be expected to match to a driver and so are ignored. One exception is CLK_OF_DECLARE_DRIVER, since it indicates that a driver will also later probe, so compatibles in this macro are not ignored. Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- Changes in v2: - Added this commit scripts/dtc/dt-extract-compatibles | 57 +++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/scripts/dtc/dt-extract-compatibles b/scripts/dtc/dt-extract-compatibles index 2b6d228602e8..bd07477dd144 100755 --- a/scripts/dtc/dt-extract-compatibles +++ b/scripts/dtc/dt-extract-compatibles @@ -7,11 +7,15 @@ import re import argparse -def parse_of_declare_macros(data): +def parse_of_declare_macros(data, include_driver_macros=True): """ Find all compatible strings in OF_DECLARE() style macros """ compat_list = [] # CPU_METHOD_OF_DECLARE does not have a compatible string - for m in re.finditer(r'(?<!CPU_METHOD_)(IRQCHIP|OF)_(DECLARE|MATCH)(_DRIVER)?\(.*?\)', data): + if include_driver_macros: + re_macros = r'(?<!CPU_METHOD_)(IRQCHIP|OF)_(DECLARE|MATCH)(_DRIVER)?\(.*?\)' + else: + re_macros = r'(?<!CPU_METHOD_)(IRQCHIP|OF)_(DECLARE|MATCH)\(.*?\)' + for m in re.finditer(re_macros, data): try: compat = re.search(r'"(.*?)"', m[0])[1] except: @@ -22,24 +26,52 @@ def parse_of_declare_macros(data): return compat_list -def parse_of_device_id(data): +def parse_of_device_id(data, match_table_list=None): """ Find all compatible strings in of_device_id structs """ compat_list = [] - for m in re.finditer(r'of_device_id(\s+\S+)?\s+\S+\[\](\s+\S+)?\s*=\s*({.*?);', data): - compat_list += re.findall(r'\.compatible\s+=\s+"(\S+)"', m[3]) + for m in re.finditer(r'of_device_id(\s+\S+)?\s+(\S+)\[\](\s+\S+)?\s*=\s*({.*?);', data): + if match_table_list is not None and m[2] not in match_table_list: + continue + compat_list += re.findall(r'\.compatible\s+=\s+"(\S+)"', m[4]) return compat_list -def parse_compatibles(file): +def parse_of_match_table(data): + """ Find all driver's of_match_table """ + match_table_list = [] + for m in re.finditer(r'\.of_match_table\s+=\s+(of_match_ptr\()?([a-zA-Z0-9_-]+)', data): + match_table_list.append(m[2]) + + return match_table_list + + +def parse_compatibles(file, compat_ignore_list): with open(file, 'r', encoding='utf-8') as f: data = f.read().replace('\n', '') - compat_list = parse_of_declare_macros(data) - compat_list += parse_of_device_id(data) + if compat_ignore_list is not None: + # For a compatible in the DT to be matched to a driver it needs to show + # up in a driver's of_match_table + match_table_list = parse_of_match_table(data) + compat_list = parse_of_device_id(data, match_table_list) + + compat_list = [compat for compat in compat_list if compat not in compat_ignore_list] + else: + compat_list = parse_of_declare_macros(data) + compat_list += parse_of_device_id(data) return compat_list +def parse_compatibles_to_ignore(file): + with open(file, 'r', encoding='utf-8') as f: + data = f.read().replace('\n', '') + + # Compatibles that show up in OF_DECLARE macros can't be expected to + # match a driver, except for the _DRIVER ones. + return parse_of_declare_macros(data, include_driver_macros=False) + + def print_compat(filename, compatibles): if not compatibles: return @@ -63,10 +95,17 @@ if __name__ == "__main__": ap = argparse.ArgumentParser() ap.add_argument("cfile", type=str, nargs='*', help="C source files or directories to parse") ap.add_argument('-H', '--with-filename', help="Print filename with compatibles", action="store_true") + ap.add_argument('-d', '--driver-match', help="Only print compatibles that should match to a driver", action="store_true") args = ap.parse_args() show_filename = args.with_filename + compat_ignore_list = None + + if args.driver_match: + compat_ignore_list = [] + for f in files_to_parse(args.cfile): + compat_ignore_list.extend(parse_compatibles_to_ignore(f)) for f in files_to_parse(args.cfile): - compat_list = parse_compatibles(f) + compat_list = parse_compatibles(f, compat_ignore_list) print_compat(f, compat_list) -- 2.41.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices 2023-08-17 23:35 [PATCH v2 0/3] Add a test to catch unprobed Devicetree devices Nícolas F. R. A. Prado 2023-08-17 23:35 ` [PATCH v2 1/3] dt: dt-extract-compatibles: Handle cfile arguments in generator function Nícolas F. R. A. Prado 2023-08-17 23:35 ` [PATCH v2 2/3] dt: dt-extract-compatibles: Add flag for driver matching compatibles Nícolas F. R. A. Prado @ 2023-08-17 23:35 ` Nícolas F. R. A. Prado 2023-08-18 12:54 ` Mark Brown 2023-08-18 15:05 ` Rob Herring 2 siblings, 2 replies; 10+ messages in thread From: Nícolas F. R. A. Prado @ 2023-08-17 23:35 UTC (permalink / raw) To: Rob Herring, Frank Rowand, Shuah Khan Cc: Mark Brown, kernelci, Guenter Roeck, kernel, Bjorn Andersson, Nícolas F. R. A. Prado, linux-kernel, linux-kselftest 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}" -- 2.41.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices 2023-08-17 23:35 ` [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices Nícolas F. R. A. Prado @ 2023-08-18 12:54 ` Mark Brown 2023-08-18 15:08 ` Nícolas F. R. A. Prado 2023-08-18 15:05 ` Rob Herring 1 sibling, 1 reply; 10+ messages in thread From: Mark Brown @ 2023-08-18 12:54 UTC (permalink / raw) To: Nícolas F. R. A. Prado Cc: Rob Herring, Frank Rowand, Shuah Khan, kernelci, Guenter Roeck, kernel, Bjorn Andersson, linux-kernel, linux-kselftest [-- Attachment #1: Type: text/plain, Size: 479 bytes --] 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. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices 2023-08-18 12:54 ` Mark Brown @ 2023-08-18 15:08 ` Nícolas F. R. A. Prado 2023-08-18 16:11 ` Shuah Khan 0 siblings, 1 reply; 10+ messages in thread From: Nícolas F. R. A. Prado @ 2023-08-18 15:08 UTC (permalink / raw) To: Mark Brown Cc: Rob Herring, Frank Rowand, Shuah Khan, kernelci, Guenter Roeck, kernel, Bjorn Andersson, linux-kernel, linux-kselftest 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices 2023-08-18 15:08 ` Nícolas F. R. A. Prado @ 2023-08-18 16:11 ` Shuah Khan 0 siblings, 0 replies; 10+ messages in thread From: Shuah Khan @ 2023-08-18 16:11 UTC (permalink / raw) To: Nícolas F. R. A. Prado, Mark Brown, Rob Herring Cc: Frank Rowand, Shuah Khan, kernelci, Guenter Roeck, kernel, Bjorn Andersson, linux-kernel, linux-kselftest, Shuah Khan 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices 2023-08-17 23:35 ` [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices Nícolas F. R. A. Prado 2023-08-18 12:54 ` Mark Brown @ 2023-08-18 15:05 ` Rob Herring 2023-08-18 16:38 ` Nícolas F. R. A. Prado 1 sibling, 1 reply; 10+ messages in thread From: Rob Herring @ 2023-08-18 15:05 UTC (permalink / raw) To: Nícolas F. R. A. Prado Cc: Frank Rowand, Shuah Khan, Mark Brown, kernelci, Guenter Roeck, kernel, Bjorn Andersson, linux-kernel, linux-kselftest 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices 2023-08-18 15:05 ` Rob Herring @ 2023-08-18 16:38 ` Nícolas F. R. A. Prado 2023-08-28 16:37 ` Rob Herring 0 siblings, 1 reply; 10+ messages in thread From: Nícolas F. R. A. Prado @ 2023-08-18 16:38 UTC (permalink / raw) To: Rob Herring Cc: Frank Rowand, Shuah Khan, Mark Brown, kernelci, Guenter Roeck, kernel, Bjorn Andersson, linux-kernel, linux-kselftest 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 > [..] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices 2023-08-18 16:38 ` Nícolas F. R. A. Prado @ 2023-08-28 16:37 ` Rob Herring 0 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2023-08-28 16:37 UTC (permalink / raw) To: Nícolas F. R. A. Prado Cc: Frank Rowand, Shuah Khan, Mark Brown, kernelci, Guenter Roeck, kernel, Bjorn Andersson, linux-kernel, linux-kselftest 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-08-28 16:38 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-17 23:35 [PATCH v2 0/3] Add a test to catch unprobed Devicetree devices Nícolas F. R. A. Prado 2023-08-17 23:35 ` [PATCH v2 1/3] dt: dt-extract-compatibles: Handle cfile arguments in generator function Nícolas F. R. A. Prado 2023-08-17 23:35 ` [PATCH v2 2/3] dt: dt-extract-compatibles: Add flag for driver matching compatibles Nícolas F. R. A. Prado 2023-08-17 23:35 ` [PATCH v2 3/3] kselftest: Add new test for detecting unprobed Devicetree devices Nícolas F. R. A. Prado 2023-08-18 12:54 ` Mark Brown 2023-08-18 15:08 ` Nícolas F. R. A. Prado 2023-08-18 16:11 ` Shuah Khan 2023-08-18 15:05 ` Rob Herring 2023-08-18 16:38 ` Nícolas F. R. A. Prado 2023-08-28 16:37 ` Rob Herring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®