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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Wed, Sep 10, 2025 at 11:51:03AM +0800, Haixu Cui wrote:
> On 9/3/2025 6:27 PM, Andy Shevchenko wrote:
> > On Wed, Sep 03, 2025 at 05:04:46PM +0800, Haixu Cui wrote:
> > > On 9/1/2025 8:07 PM, Andy Shevchenko wrote:
> > > > On Thu, Aug 28, 2025 at 05:34:51PM +0800, Haixu Cui wrote:
> > > > > This is the virtio SPI Linux kernel driver.

...

> > > > > +#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