On Mon, Aug 14, 2023 at 02:44:50PM +0800, Baolu Lu wrote:
> On 2023/8/3 8:08, Jason Gunthorpe wrote:
> > Prior to commit 1b932ceddd19 ("iommu: Remove detach_dev callbacks") the
> > sun50i_iommu_detach_device() function was being called by
> > ops->detach_dev().
> >
> > This is an IDENTITY domain so convert sun50i_iommu_detach_device() into
> > sun50i_iommu_identity_attach() and a full IDENTITY domain and thus hook it
> > back up the same was as the old ops->detach_dev().
> >
> > Signed-off-by: Jason Gunthorpe<jgg@xxxxxxxxxx>
> > ---
> > drivers/iommu/sun50i-iommu.c | 26 +++++++++++++++++++-------
> > 1 file changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
> > index 74c5cb93e90027..0bf08b120cf105 100644
> > --- a/drivers/iommu/sun50i-iommu.c
> > +++ b/drivers/iommu/sun50i-iommu.c
> > @@ -757,21 +757,32 @@ static void sun50i_iommu_detach_domain(struct sun50i_iommu *iommu,
> > iommu->domain = NULL;
> > }
> > -static void sun50i_iommu_detach_device(struct iommu_domain *domain,
> > - struct device *dev)
> > +static int sun50i_iommu_identity_attach(struct iommu_domain *identity_domain,
> > + struct device *dev)
> > {
> > - struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
> > struct sun50i_iommu *iommu = dev_iommu_priv_get(dev);
> > + struct sun50i_iommu_domain *sun50i_domain;
> > dev_dbg(dev, "Detaching from IOMMU domain\n");
> > - if (iommu->domain != domain)
> > - return;
> > + if (iommu->domain == identity_domain)
> > + return 0;
> > + sun50i_domain = to_sun50i_domain(iommu->domain);
> > if (refcount_dec_and_test(&sun50i_domain->refcnt))
> > sun50i_iommu_detach_domain(iommu, sun50i_domain);
> > + return 0;
> > }
>
> Does iommu->domain need to be set to identity_domain before returning?
sun50i_iommu_detach_domain() does it.
This driver is confused because it uses generic_single_device_group
but also has this weird refcounting stuff. It should just make the
first attach alter the HW and have the remaining ones (eg new domain
== current domani) be NOPs. It should not refcount.
Jason
