Re: [PATCH v2 1/1] iommu/dma: Use DMA ops setter instead of direct assignment
On Tue, Feb 08, 2022 at 11:06:56PM -0800, Christoph Hellwig wrote: > On Mon, Feb 07, 2022 at 03:55:32PM +, Robin Murphy wrote: > > On 2022-02-07 14:13, Andy Shevchenko wrote: > > > Use DMA ops setter instead of direct assignment. Even we know that > > > this module doesn't perform access to the dma_ops member of struct device, > > > it's better to use setter to avoid potential problems in the future. > > > > What potential problems are you imagining? This whole file is a DMA ops > > implementation, not driver code (and definitely not a module); if anyone > > removes the "select DMA_OPS" from CONFIG_IOMMU_DMA they deserve whatever > > breakage they get. > > > > I concur that there's no major harm in using the helper here, but I also see > > no point in pretending that there's any value to abstracting the operation > > in this particular context. > > Yeah. Killing off the the wrapper is actually on my todo list, mostly > because it leads to people doing completely broken things like the VDPA > private dma ops that should not exist. Let's abandon this change. (I see that it's kinda 50/50 of the users with API and without) -- With Best Regards, Andy Shevchenko ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/1] iommu/dma: Use DMA ops setter instead of direct assignment
On Mon, Feb 07, 2022 at 03:55:32PM +, Robin Murphy wrote: > On 2022-02-07 14:13, Andy Shevchenko wrote: > > Use DMA ops setter instead of direct assignment. Even we know that > > this module doesn't perform access to the dma_ops member of struct device, > > it's better to use setter to avoid potential problems in the future. > > What potential problems are you imagining? This whole file is a DMA ops > implementation, not driver code (and definitely not a module); if anyone > removes the "select DMA_OPS" from CONFIG_IOMMU_DMA they deserve whatever > breakage they get. > > I concur that there's no major harm in using the helper here, but I also see > no point in pretending that there's any value to abstracting the operation > in this particular context. Yeah. Killing off the the wrapper is actually on my todo list, mostly because it leads to people doing completely broken things like the VDPA private dma ops that should not exist. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/1] iommu/dma: Use DMA ops setter instead of direct assignment
On 2022-02-07 14:13, Andy Shevchenko wrote:
Use DMA ops setter instead of direct assignment. Even we know that
this module doesn't perform access to the dma_ops member of struct device,
it's better to use setter to avoid potential problems in the future.
What potential problems are you imagining? This whole file is a DMA ops
implementation, not driver code (and definitely not a module); if anyone
removes the "select DMA_OPS" from CONFIG_IOMMU_DMA they deserve whatever
breakage they get.
I concur that there's no major harm in using the helper here, but I also
see no point in pretending that there's any value to abstracting the
operation in this particular context.
Thanks,
Robin.
Signed-off-by: Andy Shevchenko
---
v2: rebased on top of the latest codebase
drivers/iommu/dma-iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d85d54f2b549..b585a3fdbc56 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1482,7 +1482,7 @@ void iommu_setup_dma_ops(struct device *dev, u64
dma_base, u64 dma_limit)
if (iommu_is_dma_domain(domain)) {
if (iommu_dma_init_domain(domain, dma_base, dma_limit, dev))
goto out_err;
- dev->dma_ops = _dma_ops;
+ set_dma_ops(dev, _dma_ops);
}
return;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
