On Tue, 2020-04-14 at 15:15 +0200, Joerg Roedel wrote: > From: Joerg Roedel <jroe...@suse.de> > > Convert the Mediatek-v1 IOMMU driver to use the probe_device() and > release_device() call-backs of iommu_ops, so that the iommu core code > does the group and sysfs setup. > > Signed-off-by: Joerg Roedel <jroe...@suse.de> > --- > drivers/iommu/mtk_iommu_v1.c | 50 +++++++++++++++--------------------- > 1 file changed, 20 insertions(+), 30 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c > index a31be05601c9..7bdd74c7cb9f 100644 > --- a/drivers/iommu/mtk_iommu_v1.c > +++ b/drivers/iommu/mtk_iommu_v1.c > @@ -416,14 +416,12 @@ static int mtk_iommu_create_mapping(struct device *dev, > return 0; > } > > -static int mtk_iommu_add_device(struct device *dev) > +static struct iommu_device *mtk_iommu_probe_device(struct device *dev) > { > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > - struct dma_iommu_mapping *mtk_mapping; > struct of_phandle_args iommu_spec; > struct of_phandle_iterator it; > struct mtk_iommu_data *data; > - struct iommu_group *group; > int err; > > of_for_each_phandle(&it, err, dev->of_node, "iommus", > @@ -442,35 +440,28 @@ static int mtk_iommu_add_device(struct device *dev) > } > > if (!fwspec || fwspec->ops != &mtk_iommu_ops) > - return -ENODEV; /* Not a iommu client device */ > + return ERR_PTR(-ENODEV); /* Not a iommu client device */ > > - /* > - * This is a short-term bodge because the ARM DMA code doesn't > - * understand multi-device groups, but we have to call into it > - * successfully (and not just rely on a normal IOMMU API attach > - * here) in order to set the correct DMA API ops on @dev. > - */ > - group = iommu_group_alloc(); > - if (IS_ERR(group)) > - return PTR_ERR(group); > + data = dev_iommu_priv_get(dev); > > - err = iommu_group_add_device(group, dev); > - iommu_group_put(group); > - if (err) > - return err; > + return &data->iommu; > +} > > - data = dev_iommu_priv_get(dev); > +static void mtk_iommu_probe_finalize(struct device *dev) > +{ > + struct dma_iommu_mapping *mtk_mapping; > + struct mtk_iommu_data *data; > + int err; > + > + data = dev_iommu_priv_get(dev); > mtk_mapping = data->dev->archdata.iommu; > - err = arm_iommu_attach_device(dev, mtk_mapping); > - if (err) { > - iommu_group_remove_device(dev); > - return err; > - } > > - return iommu_device_link(&data->iommu, dev); > + err = arm_iommu_attach_device(dev, mtk_mapping); > + if (err) > + dev_err(dev, "Can't create IOMMU mapping - DMA-OPS will not > work\n");
Hi Joerg, Thanks very much for this patch. This arm_iommu_attach_device is called just as we expected. But it will fail in this callstack as the group->mutex was tried to be re-locked... [<c0938e8c>] (iommu_attach_device) from [<c0317590>] (__arm_iommu_attach_device+0x34/0x90) [<c0317590>] (__arm_iommu_attach_device) from [<c03175f8>] (arm_iommu_attach_device+0xc/0x20) [<c03175f8>] (arm_iommu_attach_device) from [<c09432cc>] (mtk_iommu_probe_finalize+0x34/0x50) [<c09432cc>] (mtk_iommu_probe_finalize) from [<c093a8ac>] (bus_iommu_probe+0x2a8/0x2c4) [<c093a8ac>] (bus_iommu_probe) from [<c093a950>] (bus_set_iommu +0x88/0xd4) [<c093a950>] (bus_set_iommu) from [<c0943c74>] (mtk_iommu_probe +0x2f8/0x364) > } > > -static void mtk_iommu_remove_device(struct device *dev) > +static void mtk_iommu_release_device(struct device *dev) > { > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > struct mtk_iommu_data *data; > @@ -479,9 +470,6 @@ static void mtk_iommu_remove_device(struct device *dev) > return; > > data = dev_iommu_priv_get(dev); > - iommu_device_unlink(&data->iommu, dev); > - > - iommu_group_remove_device(dev); > iommu_fwspec_free(dev); > } > > @@ -534,8 +522,10 @@ static const struct iommu_ops mtk_iommu_ops = { > .map = mtk_iommu_map, > .unmap = mtk_iommu_unmap, > .iova_to_phys = mtk_iommu_iova_to_phys, > - .add_device = mtk_iommu_add_device, > - .remove_device = mtk_iommu_remove_device, > + .probe_device = mtk_iommu_probe_device, > + .probe_finalize = mtk_iommu_probe_finalize, > + .release_device = mtk_iommu_release_device, > + .device_group = generic_device_group, > .pgsize_bitmap = ~0UL << MT2701_IOMMU_PAGE_SHIFT, > }; >