Thread (11 messages) 11 messages, 4 authors, 2025-09-11

Re: [PATCH v9 3/3] SPI: Add virtio SPI driver

From: Andy Shevchenko <hidden>
Date: 2025-09-11 07:48:54
Also in: linux-spi, lkml

On Wed, Sep 10, 2025 at 11:51:03AM +0800, Haixu Cui wrote:
On 9/3/2025 6:27 PM, Andy Shevchenko wrote:
quoted
On Wed, Sep 03, 2025 at 05:04:46PM +0800, Haixu Cui wrote:
quoted
On 9/1/2025 8:07 PM, Andy Shevchenko wrote:
quoted
On Thu, Aug 28, 2025 at 05:34:51PM +0800, Haixu Cui wrote:
quoted
This is the virtio SPI Linux kernel driver.
...
quoted
quoted
quoted
quoted
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/stddef.h>
A lot of headers are still missing. See below.
This driver compiles successfully, and I believe all required definitions
are resolved through indirect inclusion. For example, since I included
virtio.h, there is no need to explicitly include device.h, scatterlist.h or
types.h.

I avoided redundant #includes to keep the code clean and minimal.

If there are any essential headers I’ve overlooked, please feel free to
highlight them—I’ll gladly include them in the next revision.
The rationale is described on https://include-what-you-use.org/.
Thanks for your feedback and for pointing me to the iwyu guidelines.

I've experimented with the iwyu tool, and while for spi-virtio.c I noticed
that it recommends header that is not directly to the code - such as
vdso/cache.h - and occasionally suggests re-include header like
linux/spi/spi.h that is already present.
iwyu is a power tool expecially in application-level development for C++
projects where header dependencies are more straightforward. However it
seems iwyu may not yet be fully suited for analyzing Linux kernel due to its
complexity and conditional inclusions.
I was not talking about the tool, yes, this tool is not ready for Linux kernel.
Jonathan Cameron played with it to make it useful for the Linux kernel project,
but it seems the work is stalled.
Additionally, I’ve verified that the driver compiles successfully with both
gcc and clang, which indicates that all required definitions are either
directly or indirectly resolved.
The IWYU principle is against the includes that indirectly resolve types and
APIs. The problem that some headers are included in others and that relation
is guaranteed, but we do not have an official list of the guarantees, so this
is a tribal knowledge.

Basically the developer should know a bit more about the Linux kernel header
organisation to fulfill IWYU. And this is currently problematic.
I appreciate your guidance and will continue to refine the patch with
clarity and maintainability in mind.
-- 
With Best Regards,
Andy Shevchenko

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help