| Message ID | 20230810202413.1780286-1-nfraprado@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add a test to catch unprobed Devicetree devices | expand |
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/20230804190130.1936566-1-robh@kernel.org/
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
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