Messages in this thread | | | Date | Fri, 24 Mar 2023 13:24:47 +0000 | Subject | Re: [PATCH v2] iommu/rockchip: Add missing set_platform_dma_ops callback | From | Steven Price <> |
| |
On 24/03/2023 13:16, Baolu Lu wrote: > On 2023/3/24 19:11, Steven Price wrote: >> Similar to exynos, we need a set_platform_dma_ops() callback for proper >> operation on ARM 32 bit after recent changes in the IOMMU framework >> (detach ops removal). But also the use of a NULL domain is confusing. >> >> Rework the code to have a singleton rk_identity_domain which is assigned >> to domain when using an identity mapping rather than "detaching". This >> makes the code easier to reason about. >> >> Signed-off-by: Steven Price <steven.price@arm.com> >> --- >> Changes since v1[1]: >> >> * Reworked the code to avoid a NULL domain, instead a singleton >> rk_identity_domain is used instead. The 'detach' language is no >> longer used. >> >> [1] >> https://lore.kernel.org/r/20230315164152.333251-1-steven.price%40arm.com >> >> drivers/iommu/rockchip-iommu.c | 50 ++++++++++++++++++++++++++-------- >> 1 file changed, 39 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/iommu/rockchip-iommu.c >> b/drivers/iommu/rockchip-iommu.c >> index f30db22ea5d7..437541004994 100644 >> --- a/drivers/iommu/rockchip-iommu.c >> +++ b/drivers/iommu/rockchip-iommu.c >> @@ -124,6 +124,7 @@ struct rk_iommudata { >> static struct device *dma_dev; >> static const struct rk_iommu_ops *rk_ops; >> +static struct iommu_domain rk_identity_domain; >> static inline void rk_table_flush(struct rk_iommu_domain *dom, >> dma_addr_t dma, >> unsigned int count) >> @@ -980,26 +981,27 @@ static int rk_iommu_enable(struct rk_iommu *iommu) >> return ret; >> } >> -static void rk_iommu_detach_device(struct iommu_domain *domain, >> - struct device *dev) >> +static int rk_iommu_identity_attach(struct iommu_domain >> *identity_domain, >> + struct device *dev) >> { >> struct rk_iommu *iommu; >> - struct rk_iommu_domain *rk_domain = to_rk_domain(domain); >> + struct rk_iommu_domain *rk_domain; >> unsigned long flags; >> int ret; >> /* Allow 'virtual devices' (eg drm) to detach from domain */ >> iommu = rk_iommu_from_dev(dev); >> if (!iommu) >> - return; >> + return -ENODEV; >> + >> + rk_domain = to_rk_domain(iommu->domain); >> dev_dbg(dev, "Detaching from iommu domain\n"); >> - /* iommu already detached */ >> - if (iommu->domain != domain) >> - return; >> + if (iommu->domain == identity_domain) >> + return 0; >> - iommu->domain = NULL; >> + iommu->domain = identity_domain; > > Where did identity_domain come from? Is it rk_identity_domain?
It's a parameter of the function. In the case of the call in rk_iommu_attach_device() then, yes, it's rk_identity_domain. But this function is also the "attach_dev" callback of "rk_identity_ops".
I'll admit this is cargo-culted from Jason's example:
https://lore.kernel.org/linux-iommu/ZBnef7g7GCxogPNz@ziepe.ca/
Steve
> Best regards, > baolu
| |