Re: [PATCH 1/1] rapidio/tsi721: include dmaengine private header

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rapidio/tsi721: ineffective dmaengine.h include after linux/rio.h
--

commit 74c6f7d411b15b0789cd2ddd5bc0e36eadfcaa9a
Author: Frank Li <Frank.Li@nxp.com>

rapidio/tsi721: include dmaengine private header

This commit includes the internal DMA engine header ../../dma/dmaengine.h in
drivers/rapidio/devices/tsi721.c in preparation for making struct dma_device
private to the DMA engine provider header.

> diff --git a/drivers/rapidio/devices/tsi721.c b/drivers/rapidio/devices/tsi721.c
> index 71b87bf8c31d8..0953a39459584 100644
> --- a/drivers/rapidio/devices/tsi721.c
> +++ b/drivers/rapidio/devices/tsi721.c
> @@ -21,6 +21,7 @@
>  #include <linux/kfifo.h>
>  #include <linux/delay.h>
>  
> +#include "../../dma/dmaengine.h"

[Severity: Low]
Will including ../../dma/dmaengine.h here actually work once struct dma_device
is moved out of include/linux/dmaengine.h?

Neither drivers/rapidio/devices/tsi721.c nor drivers/rapidio/devices/tsi721.h
directly references struct dma_device or any other symbol from
drivers/dma/dmaengine.h (all DMA engine provider code is in
drivers/rapidio/devices/tsi721_dma.c). Instead, struct tsi721_device embeds
struct rio_mport, which embeds struct dma_device by value in
include/linux/rio.h when CONFIG_RAPIDIO_DMA_ENGINE is enabled:

include/linux/rio.h:
struct rio_mport {
	...
#ifdef CONFIG_RAPIDIO_DMA_ENGINE
	struct dma_device	dma;
#endif
	...
};

In drivers/rapidio/devices/tsi721.c (as well as in
drivers/rapidio/devices/tsi721_dma.c), linux/rio.h is included before
../../dma/dmaengine.h:

drivers/rapidio/devices/tsi721.c:
#include <linux/pci.h>
#include <linux/rio.h>
#include <linux/rio_drv.h>
#include <linux/dma-mapping.h>
#include <linux/interrupt.h>
#include <linux/kfifo.h>
#include <linux/delay.h>

#include "../../dma/dmaengine.h"

If struct dma_device is made private to drivers/dma/dmaengine.h while struct
rio_mport still embeds it by value, wouldn't struct dma_device still be an
incomplete type at the point where include/linux/rio.h is parsed? Conversely,
if struct rio_mport is converted so it no longer embeds struct dma_device by
value, does drivers/rapidio/devices/tsi721.c need to include
../../dma/dmaengine.h at all?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105170511.1262005-1-Frank.Li@oss.nxp.com?part=1