| Message ID | 1499628825-16260-1-git-send-email-geert@linux-m68k.org |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
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
This looks reasonable to me, I'd be happy to pick it up. Can you send it as a series with the reverts?
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.
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.
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,
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(+)