Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 11 Aug 2023 09:17:40 -0400
From:   =?utf-8?B?TsOtY29sYXMgRi4gUi4gQS4=?= Prado 
        <nfraprado@collabora.com>
To:     Rob Herring <robh@kernel.org>
Cc:     Frank Rowand <frowand.list@gmail.com>,
        Shuah Khan <shuah@kernel.org>, cocci@inria.fr,
        Mark Brown <broonie@kernel.org>,
        Nicolas Palix <nicolas.palix@imag.fr>,
        kernelci@lists.linux.dev, Julia Lawall <Julia.Lawall@inria.fr>,
        Bjorn Andersson <andersson@kernel.org>, kernel@collabora.com,
        Guenter Roeck <groeck@chromium.org>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org
Subject: Re: [RFC PATCH 0/2] Add a test to catch unprobed Devicetree devices
Message-ID: <7e915b52-c356-4a39-b286-27b31fa1abf9@notapiano>
References: <20230810202413.1780286-1-nfraprado@collabora.com>
 <20230810214309.GA1226583-robh@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20230810214309.GA1226583-robh@kernel.org>
Precedence: bulk

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