Messages in this thread | | | Date | Thu, 30 Mar 2023 15:32:29 +0100 | Subject | Re: [PATCH v2] iommu/rockchip: Add missing set_platform_dma_ops callback | From | Steven Price <> |
| |
On 27/03/2023 16:26, Jason Gunthorpe wrote: > On Mon, Mar 27, 2023 at 03:35:04PM +0100, John Keeping wrote: >> On Fri, Mar 24, 2023 at 11:11:27AM +0000, 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 >> [snip] >>> +static struct iommu_domain rk_identity_domain = { >>> + .type = IOMMU_DOMAIN_IDENTITY, >>> + .ops = &rk_identity_ops, >>> +}; >>> + >>> +#ifdef CONFIG_ARM >> >> Is this #ifdef needed? I can't see anything ARM-specific about this >> function or .set_platform_dma_ops. > > set_platform_dma_ops is never called on ARM64.
I have to admit this was somewhat cargo-culted from the exynos-iommu change. This is only needed for Arm, but I'm not sure if there's any real harm including it for other arches.
>> Not shown in the patch are the pm_runtime hooks. Do they need to >> change like this? > > Most likely yes
Good spot - I'll send a v3 with this fixed.
Thanks,
Steve
| |