[PATCH v2 0/3] Add a test to catch unprobed Devicetree devices

[PATCH v2 0/3] Add a test to catch unprobed Devicetree devices

[Nícolas F. R. A. Prado]

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

[PATCH v2 1/3] dt: dt-extract-compatibles: Handle cfile arguments in generator function

[Nícolas F. R. A. Prado]

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

[PATCH v2 2/3] dt: dt-extract-compatibles: Add flag for driver matching compatibles

[Nícolas F. R. A. Prado]

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

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

[Nícolas F. R. A. Prado]

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

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

[Mark Brown]

[-- 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 --]

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

[Rob Herring]

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
>

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

[Nícolas F. R. A. Prado]

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

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

[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

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

[Nícolas F. R. A. Prado]

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

> 
[..]

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

[Rob Herring]

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