[PATCH/RFC] dma-mapping: Provide dummy set_dma_ops() for NO_DMA=y

[PATCH/RFC] dma-mapping: Provide dummy set_dma_ops() for NO_DMA=y

[Geert Uytterhoeven]

Adding a dummy for set_dma_ops() allows to compile (sub)drivers that
don't actually use the DMA API, but propagate DMA ops configuration to a
second driver that may or may not use the DMA API.  Of course the second
driver does have to depend on HAS_DMA if it uses the DMA API.

An example is commit 5567e989198b5a8d ("fsl/fman: propagate dma_ops").

This allows to revert commit 85688d9adf685572 ("fsl/fman: add dependency
on HAS_DMA").

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 include/linux/dma-mapping.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 843ab866e0f487c2..0ab244b954418e2b 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -194,6 +194,8 @@ static inline const struct dma_map_ops *get_dma_ops(struct device *dev)
 {
 	return &bad_dma_ops;
 }
+static inline void set_dma_ops(struct device *dev,
+			       const struct dma_map_ops *dma_ops) {}
 #endif
 
 static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
-- 
2.7.4

Re: [PATCH/RFC] dma-mapping: Provide dummy set_dma_ops() for NO_DMA=y

[Arnd Bergmann]

On Sun, Jul 9, 2017 at 9:33 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Adding a dummy for set_dma_ops() allows to compile (sub)drivers that
> don't actually use the DMA API, but propagate DMA ops configuration to a
> second driver that may or may not use the DMA API.  Of course the second
> driver does have to depend on HAS_DMA if it uses the DMA API.
>
> An example is commit 5567e989198b5a8d ("fsl/fman: propagate dma_ops").
>
> This allows to revert commit 85688d9adf685572 ("fsl/fman: add dependency
> on HAS_DMA").
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

I can't think of any correct use case for this new helper. IMO a device
driver should never set the dma_map_ops for any device, and we should
instead for the fman driver correctly.

       Arnd

Re: [PATCH/RFC] dma-mapping: Provide dummy set_dma_ops() for NO_DMA=y

[Christoph Hellwig]

This looks reasonable to me, I'd be happy to pick it up.  Can you send
it as a series with the reverts?

Re: [PATCH/RFC] dma-mapping: Provide dummy set_dma_ops() for NO_DMA=y

[Robin Murphy]

On 10/07/17 15:56, Christoph Hellwig wrote:
> This looks reasonable to me, I'd be happy to pick it up.  Can you send
> it as a series with the reverts?

The fact remains that the FSL driver is still doing the wrong thing
though - set_dma_ops(dev1, get_dma_ops(dev2)) is just a hack which
happens to work on platforms which don't keep other arch-internal DMA
info as well. I did start writing a patch somewhere to fix this thing to
actually do proper DMA configuration (originally in the context of not
abusing arch_setup_dma_ops()), but Arnd's fix is probably simpler.

I don't think it makes an awful lot of sense for code without a DMA API
dependency to be calling set_dma_ops() - AFAIU the only reason it's
available to drivers at all is for particular RDMA cases which know that
their "DMA" is actually done by CPU threads, and want to optimise for that.

Robin.

Re: [PATCH/RFC] dma-mapping: Provide dummy set_dma_ops() for NO_DMA=y

[Christoph Hellwig]

On Mon, Jul 10, 2017 at 04:31:54PM +0100, Robin Murphy wrote:
> On 10/07/17 15:56, Christoph Hellwig wrote:
> > This looks reasonable to me, I'd be happy to pick it up.  Can you send
> > it as a series with the reverts?
> 
> The fact remains that the FSL driver is still doing the wrong thing
> though - set_dma_ops(dev1, get_dma_ops(dev2)) is just a hack which
> happens to work on platforms which don't keep other arch-internal DMA
> info as well. I did start writing a patch somewhere to fix this thing to
> actually do proper DMA configuration (originally in the context of not
> abusing arch_setup_dma_ops()), but Arnd's fix is probably simpler.
> 
> I don't think it makes an awful lot of sense for code without a DMA API
> dependency to be calling set_dma_ops() - AFAIU the only reason it's
> available to drivers at all is for particular RDMA cases which know that
> their "DMA" is actually done by CPU threads, and want to optimise for that.

Even that code should require the DMA API.  So yes, maybe we should
fix the code to propagate the settings by another means.