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

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

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/.

...

> I plan to update the code as follows:
> 
> struct virtio_spi_req *spi_req __free(kfree) = NULL;
> spi_req = kzalloc(sizeof(*spi_req), GFP_KERNEL);
> if(!spi_req)
>     return -ENOMEM;
> 
> This follows the pattern used in
> virtio_net(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/virtio_net.c?h=v6.17-rc4#n3746)
> 
> I'd like to check if this style is acceptable here, thanks.

The style is fine. The potential issue (not now and probably never) is that the
scope of the variable in this case is different which might lead to unexpected
side-effects. That said, You can go with it.

-- 
With Best Regards,
Andy Shevchenko