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.
Patch 1 introduces a script to parse the kernel source using Coccinelle
and extract all compatibles that can be matched by 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.
Even though there's already scripts/dtc/dt-extract-compatibles that does
a similar job, it didn't seem to find all compatibles, returning ~3k,
while Coccinelle found ~11k. Besides that, Coccinelle actually parses
the C files, so it should be a more robust solution than relying on
regexes.
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.
Feedback is very much welcome.
Thanks,
Nícolas
[1] https://github.com/kernelci/bootrr
Nícolas F. R. A. Prado (2):
scripts/dtc: Add script to extract matchable DT compatibles
kselftest: Add Devicetree unprobed devices test
scripts/dtc/extract-matchable-dt-compatibles | 33 +++++++++++
scripts/dtc/matchable_dt_compatibles.cocci | 58 +++++++++++++++++++
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/dt/.gitignore | 1 +
tools/testing/selftests/dt/Makefile | 17 ++++++
.../selftests/dt/compatible_ignore_list | 3 +
.../selftests/dt/test_unprobed_devices.sh | 58 +++++++++++++++++++
7 files changed, 171 insertions(+)
create mode 100755 scripts/dtc/extract-matchable-dt-compatibles
create mode 100644 scripts/dtc/matchable_dt_compatibles.cocci
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 100755 tools/testing/selftests/dt/test_unprobed_devices.sh
--
2.41.0
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 from a script that parses
the kernel source using Coccinelle, and will be run as part of building
this test, therefore Coccinelle is a build-time dependency for 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 <[email protected]>
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/dt/.gitignore | 1 +
tools/testing/selftests/dt/Makefile | 17 ++++++
.../selftests/dt/compatible_ignore_list | 3 +
.../selftests/dt/test_unprobed_devices.sh | 58 +++++++++++++++++++
5 files changed, 80 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 100755 tools/testing/selftests/dt/test_unprobed_devices.sh
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 8dca8acdb671..2fe992ca9294 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -16,6 +16,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..fa5f3c12a659
--- /dev/null
+++ b/tools/testing/selftests/dt/Makefile
@@ -0,0 +1,17 @@
+COCCI = $(shell which spatch 2>/dev/null)
+
+ifneq ($(COCCI),)
+TEST_PROGS := test_unprobed_devices.sh
+TEST_GEN_FILES := compatible_list
+TEST_FILES := compatible_ignore_list
+
+include ../lib.mk
+
+$(OUTPUT)/compatible_list:
+ cd $(top_srcdir) && ./scripts/dtc/extract-matchable-dt-compatibles > $(OUTPUT)/compatible_list
+
+else
+
+all:
+
+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..5d7fc6229428
--- /dev/null
+++ b/tools/testing/selftests/dt/compatible_ignore_list
@@ -0,0 +1,3 @@
+fixed-factor-clock
+fixed-clock
+simple-mfd
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..4741bedefd1f
--- /dev/null
+++ b/tools/testing/selftests/dt/test_unprobed_devices.sh
@@ -0,0 +1,58 @@
+#!/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
+#
+PDT=/proc/device-tree/
+COMPAT_LIST=compatible_list
+IGNORE_LIST=compatible_ignore_list
+
+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
+ )
+
+retval=0
+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
+ echo "DEBUG: Ignoring " "${node}"
+ continue
+ fi
+
+ if grep -x -q "${compatible}" "$COMPAT_LIST"; then
+ echo "BROKEN: " "${node}"
+ retval=1
+ continue 2
+ fi
+ done
+ echo "DEBUG: Skipping " "${node}"
+ fi
+done
+
+exit $retval
--
2.41.0
On Thu, Aug 10, 2023 at 04:23:49PM -0400, N?colas F. R. A. Prado wrote:
>
> 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.
>
> Patch 1 introduces a script to parse the kernel source using Coccinelle
> and extract all compatibles that can be matched by 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.
>
> Even though there's already scripts/dtc/dt-extract-compatibles that does
> a similar job, it didn't seem to find all compatibles, returning ~3k,
> while Coccinelle found ~11k. Besides that, Coccinelle actually parses
> the C files, so it should be a more robust solution than relying on
> regexes.
I just sent a patch[1] last week fixing missing a bunch. I only looked
at the change in count of undocumented (by schema) though.
In any case, I'm happy if we have a better solution, but really we
should only have 1. So your script would need to replace the existing
one.
I'd be interested in a performance comparison. IME, coccinelle is
fairly slow. Slower is okay to a point though.
>
> 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.
I've been down this route. I had another implementation using gdb to
extract all of_device_id objects from a built kernel, but besides the
build time, it was really slow.
Rob
[1] https://lore.kernel.org/all/[email protected]/
On Thu, Aug 10, 2023 at 03:43:09PM -0600, Rob Herring wrote:
> On Thu, Aug 10, 2023 at 04:23:49PM -0400, N?colas F. R. A. Prado wrote:
> >
> > 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.
> >
> > Patch 1 introduces a script to parse the kernel source using Coccinelle
> > and extract all compatibles that can be matched by 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.
> >
> > Even though there's already scripts/dtc/dt-extract-compatibles that does
> > a similar job, it didn't seem to find all compatibles, returning ~3k,
> > while Coccinelle found ~11k. Besides that, Coccinelle actually parses
> > the C files, so it should be a more robust solution than relying on
> > regexes.
>
> I just sent a patch[1] last week fixing missing a bunch. I only looked
> at the change in count of undocumented (by schema) though.
With the existing script, I get 11761 compatibles and 12916 with my fix.
So how are you getting only 3k?
Rob
Introduce a script to extract all compatibles that can match a device
described on the Devicetree to a driver.
The compatible extraction is done by running Coccinelle with a newly
introduced Coccinelle file that detects compatibles listed inside a
of_device_id table which is referenced by the of_match_table of a
driver struct.
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---
scripts/dtc/extract-matchable-dt-compatibles | 33 +++++++++++
scripts/dtc/matchable_dt_compatibles.cocci | 58 ++++++++++++++++++++
2 files changed, 91 insertions(+)
create mode 100755 scripts/dtc/extract-matchable-dt-compatibles
create mode 100644 scripts/dtc/matchable_dt_compatibles.cocci
diff --git a/scripts/dtc/extract-matchable-dt-compatibles b/scripts/dtc/extract-matchable-dt-compatibles
new file mode 100755
index 000000000000..b5e96c7ba42e
--- /dev/null
+++ b/scripts/dtc/extract-matchable-dt-compatibles
@@ -0,0 +1,33 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Copyright (c) 2023 Collabora Ltd
+#
+# Based on coccicheck script.
+#
+# This script uses Coccinelle to extract all compatibles that can be matched by
+# Devicetree devices from the kernel source.
+
+DIR="$(dirname $(readlink -f $0))"
+SPATCH=`which ${SPATCH:=spatch}`
+COCCI_FILE=$DIR/matchable_dt_compatibles.cocci
+
+if [ ! -x "$SPATCH" ]; then
+ echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/'
+ exit 1
+fi
+
+# Use only one thread per core by default if hyperthreading is enabled
+THREADS_PER_CORE=$(LANG=C lscpu | grep "Thread(s) per core: " | tr -cd "[:digit:]")
+if [ -z "$J" ]; then
+ NPROC=$(getconf _NPROCESSORS_ONLN)
+ if [ $THREADS_PER_CORE -gt 1 -a $NPROC -gt 4 ] ; then
+ NPROC=$((NPROC/2))
+ fi
+else
+ NPROC="$J"
+fi
+
+"$SPATCH" --cocci-file "$COCCI_FILE" --dir . --jobs "$NPROC" --chunksize 1 \
+ --no-show-diff --no-includes --include-headers --very-quiet \
+ | grep '"' | sed -e 's/"//g;s/^ *\([^ ]*\) *$/\1/'
diff --git a/scripts/dtc/matchable_dt_compatibles.cocci b/scripts/dtc/matchable_dt_compatibles.cocci
new file mode 100644
index 000000000000..ecd3705681aa
--- /dev/null
+++ b/scripts/dtc/matchable_dt_compatibles.cocci
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright: (C) 2023 Collabora Ltd
+@rule1@
+identifier table;
+type drv;
+identifier drvname;
+identifier drvfield;
+identifier drvfield1;
+identifier drvfield2;
+@@
+
+(
+drv drvname = {
+ .drvfield = {
+ .of_match_table = table,
+ },
+};
+|
+drv drvname = {
+ .drvfield = {
+ .of_match_table = of_match_ptr(table),
+ },
+};
+|
+// Accounts for mdio and spimem drivers
+drv drvname = {
+ .drvfield1 = {
+ .drvfield2 = {
+ .of_match_table = table,
+ },
+ },
+};
+|
+drv drvname = {
+ .drvfield1 = {
+ .drvfield2 = {
+ .of_match_table = of_match_ptr(table),
+ },
+ },
+};
+)
+
+@rule2@
+identifier rule1.table;
+expression compat;
+@@
+
+struct of_device_id table[] = {
+...,
+{ .compatible = compat, },
+...
+};
+
+@script:python@
+compat << rule2.compat;
+@@
+
+print("%s" % compat)
--
2.41.0
On Thu, Aug 10, 2023 at 04:23:51PM -0400, N?colas F. R. A. Prado 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 from a script that parses
> the kernel source using Coccinelle, and will be run as part of building
> this test, therefore Coccinelle is a build-time dependency for 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.
This doesn't appear to produce KTAP output which is going to make it
less useful for generic kselftest runners.
On Fri, Aug 11, 2023 at 02:54:56PM +0100, Mark Brown wrote:
> On Thu, Aug 10, 2023 at 04:23:51PM -0400, N?colas F. R. A. Prado 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 from a script that parses
> > the kernel source using Coccinelle, and will be run as part of building
> > this test, therefore Coccinelle is a build-time dependency for 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.
>
> This doesn't appear to produce KTAP output which is going to make it
> less useful for generic kselftest runners.
Right, I'm going to need to rewrite it in C for that, but since I already had
the shell script done, I decided to send it as is for the RFC, since I wanted to
get feedback on the general approach more than anything.
Thanks,
N?colas
On Thu, Aug 10, 2023 at 03:43:09PM -0600, Rob Herring wrote:
> On Thu, Aug 10, 2023 at 04:23:49PM -0400, N?colas F. R. A. Prado wrote:
> >
> > 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.
> >
> > Patch 1 introduces a script to parse the kernel source using Coccinelle
> > and extract all compatibles that can be matched by 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.
> >
> > Even though there's already scripts/dtc/dt-extract-compatibles that does
> > a similar job, it didn't seem to find all compatibles, returning ~3k,
> > while Coccinelle found ~11k. Besides that, Coccinelle actually parses
> > the C files, so it should be a more robust solution than relying on
> > regexes.
>
> I just sent a patch[1] last week fixing missing a bunch. I only looked
> at the change in count of undocumented (by schema) though.
Ah, looks like I mixed up the output from the dt-extract-compatibles script and
the output from the make dt_compatible_check. The python script does list
practically (*) all compatibles that Coccinelle found. So I'll look into
extending it for the purposes of this test next.
(*) it misses 3 compatibles in .h files, and fsl,mpc5200-gpt-gpio because the
comment above it has ';'. Those are easy to fix though, either on the regex or
on the driver's code.
>
> In any case, I'm happy if we have a better solution, but really we
> should only have 1. So your script would need to replace the existing
> one.
Agreed.
>
> I'd be interested in a performance comparison. IME, coccinelle is
> fairly slow. Slower is okay to a point though.
Yes, Coccinelle seems to be quite a bit slower. I can provide a comparison after
I've tweaked the python script to get the same matches as Coccinelle so it is a
fair comparison.
>
> >
> > 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.
>
> I've been down this route. I had another implementation using gdb to
> extract all of_device_id objects from a built kernel, but besides the
> build time, it was really slow.
Interesting to know, that's another option that I'd considered.
Thanks,
N?colas
On Thu, Aug 17, 2023 at 12:17:38PM -0400, N?colas F. R. A. Prado wrote:
> On Thu, Aug 17, 2023 at 01:26:05PM +0100, Mark Brown wrote:
> > I'm not clear why KTAP would require C?
> When going through the documentation there was only mention of the C headers for
> the kselftest framework which outputs using the KTAP format, so I thought that
> was the only acceptable option.
> But in the meantime while looking through the other tests I saw you've recently
> added ktap output to ftrace/ftracetest. The newly added test in
> net/mptcp/mptcp_lib.sh also has its own helpers for outputting in KTAP. There
> are also a couple other cases of this in python.
> So I can definitely do the same for this test.
Right, KTAP is a format specification for what appears on stdout - how
that output is generated is immaterial. It's just that most tests are
written in C so that's where the helpers are.
On Fri, Aug 11, 2023 at 10:16:52AM -0400, N?colas F. R. A. Prado wrote:
> On Fri, Aug 11, 2023 at 02:54:56PM +0100, Mark Brown wrote:
> > This doesn't appear to produce KTAP output which is going to make it
> > less useful for generic kselftest runners.
> Right, I'm going to need to rewrite it in C for that, but since I already had
> the shell script done, I decided to send it as is for the RFC, since I wanted to
> get feedback on the general approach more than anything.
I'm not clear why KTAP would require C?