2023-09-20 15:13:12

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] Add a test to catch unprobed Devicetree devices

On Mon, Aug 28, 2023 at 05:13:09PM -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.
>
> 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 v3:
> - Added DT selftest path to MAINTAINERS
> - Enabled device probe test for nodes with 'status = "ok"'
> - Added pass/fail/skip totals to end of test output
>
> 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
>
> MAINTAINERS | 1 +
> 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 | 70 ++++++++++++++++
> .../selftests/dt/test_unprobed_devices.sh | 83 +++++++++++++++++++
> 8 files changed, 236 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

Hi Rob,

gentle ping on this series.

I take it you'll be merging this through your tree, so I've added Shuah's R-b
that she supplied on v2 for the kselftest patch.

Thanks,
N?colas


2023-09-20 20:10:57

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] Add a test to catch unprobed Devicetree devices

On Wed, Sep 20, 2023 at 10:03:06AM -0400, N?colas F. R. A. Prado wrote:
> On Mon, Aug 28, 2023 at 05:13:09PM -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.
> >
> > 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 v3:
> > - Added DT selftest path to MAINTAINERS
> > - Enabled device probe test for nodes with 'status = "ok"'
> > - Added pass/fail/skip totals to end of test output
> >
> > 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
> >
> > MAINTAINERS | 1 +
> > 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 | 70 ++++++++++++++++
> > .../selftests/dt/test_unprobed_devices.sh | 83 +++++++++++++++++++
> > 8 files changed, 236 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
>
> Hi Rob,
>
> gentle ping on this series.
>
> I take it you'll be merging this through your tree, so I've added Shuah's R-b
> that she supplied on v2 for the kselftest patch.

Sorry, now applied.

If you send something before or in the merge window, it is best to
rebase and resend after rc1 comes out.

Rob

2023-09-21 01:46:25

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] Add a test to catch unprobed Devicetree devices

On Wed, Sep 20, 2023 at 02:56:29PM -0500, Rob Herring wrote:
> On Wed, Sep 20, 2023 at 10:03:06AM -0400, N?colas F. R. A. Prado wrote:
> > On Mon, Aug 28, 2023 at 05:13:09PM -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.
> > >
> > > 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 v3:
> > > - Added DT selftest path to MAINTAINERS
> > > - Enabled device probe test for nodes with 'status = "ok"'
> > > - Added pass/fail/skip totals to end of test output
> > >
> > > 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
> > >
> > > MAINTAINERS | 1 +
> > > 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 | 70 ++++++++++++++++
> > > .../selftests/dt/test_unprobed_devices.sh | 83 +++++++++++++++++++
> > > 8 files changed, 236 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
> >
> > Hi Rob,
> >
> > gentle ping on this series.
> >
> > I take it you'll be merging this through your tree, so I've added Shuah's R-b
> > that she supplied on v2 for the kselftest patch.
>
> Sorry, now applied.
>
> If you send something before or in the merge window, it is best to
> rebase and resend after rc1 comes out.

Ah didn't know about that, will keep it in mind for the future, thanks!

N?colas