Hi,
I'm sorry for a delay in replying, but I was busy with other stuff.
On 23.01.2023 22:00, Jason Gunthorpe wrote:
> On Mon, Jan 23, 2023 at 10:31:01AM +0100, Marek Szyprowski wrote:
>> Add set_platform_dma_ops() required for proper driver operation on ARM
>> 32bit arch after recent changes in the IOMMU framework (detach ops
>> removal).
> Thanks for looking into this!
>
> Can you explain more about how this actually solves the problem in the
> commit message? I don't get it.
Exynos DRM driver calls arm_iommu_detach_device(), then
arm_iommu_attach_device() with a difrent 'mapping', see
drivers/gpu/drm/exynos/exynos_drm_dma.c Lack of set_platform_dma_ops
leads to a warning in iommu_group_do_set_platform_dma(). The other case
of calling arm_iommu_detach_device() is after unsuccessful probe of the
platform device.
>> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
>> index 6fc58e89712f..c4955d045855 100644
>> --- a/drivers/iommu/exynos-iommu.c
>> +++ b/drivers/iommu/exynos-iommu.c
>> @@ -1346,8 +1346,10 @@ static void exynos_iommu_release_device(struct device *dev)
>> struct iommu_group *group = iommu_group_get(dev);
>>
>> if (group) {
>> +#ifndef CONFIG_ARM
>> WARN_ON(owner->domain !=
>> iommu_group_default_domain(group));
>> +#endif
>> exynos_iommu_detach_device(owner->domain, dev);
>> iommu_group_put(group);
>> }
>> @@ -1398,6 +1400,9 @@ static int exynos_iommu_of_xlate(struct device *dev,
>> static const struct iommu_ops exynos_iommu_ops = {
>> .domain_alloc = exynos_iommu_domain_alloc,
>> .device_group = generic_device_group,
>> +#ifdef CONFIG_ARM
>> + .set_platform_dma_ops = exynos_iommu_release_device,
>> +#endif
> This is ugly, if you need a set_platform_dma_ops it should not be
> called release... Release is supposed to be about putting the HW back
> to some idle state because we are unplugging the struct device.
Indeed this patch was a bit ugly, I've did it in a bit of hurry. That
time I've simply checked how it is implemented in other drivers and
found that it very similar to the release operation, so I did it in
exynos-iommu that way. I've checked again and indeed there are some
differences, so I will send a fix in a few minutes.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
