Re: [PATCH v1 6/9] iommu/tegra: gart: Ignore devices without IOMMU phandle in DT

[Dmitry Osipenko <digetx@gmail.com>]

Hi Robin,

On 11.05.2018 14:34, Robin Murphy wrote:
> Hi Dmitry,
> 
> On 08/05/18 19:16, Dmitry Osipenko wrote:
>> GART can't handle all devices, ignore devices that aren't related to GART.
>> Device tree must explicitly assign GART IOMMU to the devices.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>   drivers/iommu/tegra-gart.c | 33 ++++++++++++++++++++++++++++++++-
>>   1 file changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
>> index 39305224c48d..5b2d27620350 100644
>> --- a/drivers/iommu/tegra-gart.c
>> +++ b/drivers/iommu/tegra-gart.c
>> @@ -366,6 +366,26 @@ static void gart_iommu_remove_device(struct device *dev)
>>       iommu_device_unlink(&gart_handle->iommu, dev);
>>   }
>>   +static int gart_iommu_check_device(struct gart_device *gart,
>> +                   struct device *dev);
>> +
>> +struct iommu_group *gart_iommu_device_group(struct device *dev)
>> +{
>> +    int err;
>> +
>> +    err = gart_iommu_check_device(gart_handle, dev);
>> +    if (err)
>> +        return ERR_PTR(err);
>> +
>> +    return generic_device_group(dev);
>> +}
>> +
>> +static int gart_iommu_of_xlate(struct device *dev,
>> +                   struct of_phandle_args *args)
>> +{
>> +    return 0;
>> +}
>> +
>>   static const struct iommu_ops gart_iommu_ops = {
>>       .capable    = gart_iommu_capable,
>>       .domain_alloc    = gart_iommu_domain_alloc,
>> @@ -374,14 +394,24 @@ static const struct iommu_ops gart_iommu_ops = {
>>       .detach_dev    = gart_iommu_detach_dev,
>>       .add_device    = gart_iommu_add_device,
>>       .remove_device    = gart_iommu_remove_device,
>> -    .device_group    = generic_device_group,
>> +    .device_group    = gart_iommu_device_group,
>>       .map        = gart_iommu_map,
>>       .map_sg        = default_iommu_map_sg,
>>       .unmap        = gart_iommu_unmap,
>>       .iova_to_phys    = gart_iommu_iova_to_phys,
>>       .pgsize_bitmap    = GART_IOMMU_PGSIZES,
>> +    .of_xlate    = gart_iommu_of_xlate,
>>   };
>>   +static int gart_iommu_check_device(struct gart_device *gart,
>> +                   struct device *dev)
>> +{
>> +    if (!dev->iommu_fwspec || dev->iommu_fwspec->ops != &gart_iommu_ops)
>> +        return -ENODEV;
> 
> Conceptually, it would be better to verify this in .add_device *before* calling
> iommu_group_get_for_dev(). That would also align with what other drivers do, and
> let you save introducing the .device_group callback until the real
> implementation in the later patch.

Indeed, thank you for the suggestion.