Re: [PATCH v2 08/10] iommu: Remove unused argument in is_attach_deferred

Lu Baolu
2022-02-14T00:52:12Z

Hi Robin,

On 2/9/22 9:52 PM, Robin Murphy wrote:
> On 2022-02-08 01:25, Lu Baolu wrote:
>> The is_attach_deferred iommu_ops callback is a device op. The domain
>> argument is unnecessary and never used. Remove it to make code clean.
>>
>> Suggested-by: Robin Murphy <robin.murphy@arm.com>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   include/linux/iommu.h         |  2 +-
>>   drivers/iommu/amd/amd_iommu.h |  3 +--
>>   drivers/iommu/amd/iommu.c     |  3 +--
>>   drivers/iommu/amd/iommu_v2.c  |  2 +-
>>   drivers/iommu/intel/iommu.c   |  3 +--
>>   drivers/iommu/iommu.c         | 15 ++++++---------
>>   6 files changed, 11 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index eb2684f95018..47ca7eca5d7b 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -269,7 +269,7 @@ struct iommu_ops {
>>       void (*put_resv_regions)(struct device *dev, struct list_head
>> *list);
>>       int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
>> -    bool (*is_attach_deferred)(struct iommu_domain *domain, struct
>> device *dev);
>> +    bool (*is_attach_deferred)(struct device *dev);
>>       /* Per device IOMMU features */
>>       bool (*dev_has_feat)(struct device *dev, enum iommu_dev_features
>> f);
>> diff --git a/drivers/iommu/amd/amd_iommu.h
>> b/drivers/iommu/amd/amd_iommu.h
>> index 416815a525d6..3b2f06b7aca6 100644
>> --- a/drivers/iommu/amd/amd_iommu.h
>> +++ b/drivers/iommu/amd/amd_iommu.h
>> @@ -116,8 +116,7 @@ void amd_iommu_domain_clr_pt_root(struct
>> protection_domain *domain)
>>   extern bool translation_pre_enabled(struct amd_iommu *iommu);
>> -extern bool amd_iommu_is_attach_deferred(struct iommu_domain *domain,
>> -                     struct device *dev);
>> +extern bool amd_iommu_is_attach_deferred(struct device *dev);
>>   extern int __init add_special_device(u8 type, u8 id, u16 *devid,
>>                        bool cmd_line);
>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index 461f1844ed1f..37f2fbb4b129 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -2215,8 +2215,7 @@ static void amd_iommu_get_resv_regions(struct
>> device *dev,
>>       list_add_tail(&region->list, head);
>>   }
>> -bool amd_iommu_is_attach_deferred(struct iommu_domain *domain,
>> -                  struct device *dev)
>> +bool amd_iommu_is_attach_deferred(struct device *dev)
>>   {
>>       struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
>> diff --git a/drivers/iommu/amd/iommu_v2.c b/drivers/iommu/amd/iommu_v2.c
>> index 58da08cc3d01..7c94ec05d289 100644
>> --- a/drivers/iommu/amd/iommu_v2.c
>> +++ b/drivers/iommu/amd/iommu_v2.c
>> @@ -537,7 +537,7 @@ static int ppr_notifier(struct notifier_block *nb,
>> unsigned long e, void *data)
>>       ret = NOTIFY_DONE;
>>       /* In kdump kernel pci dev is not initialized yet -> send
>> INVALID */
>> -    if (amd_iommu_is_attach_deferred(NULL, &pdev->dev)) {
>> +    if (amd_iommu_is_attach_deferred(&pdev->dev)) {
>>           amd_iommu_complete_ppr(pdev, iommu_fault->pasid,
>>                          PPR_INVALID, tag);
>>           goto out;
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index 2b5f4e57a8bb..80f1294be634 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -5052,8 +5052,7 @@ intel_iommu_dev_disable_feat(struct device *dev,
>> enum iommu_dev_features feat)
>>       }
>>   }
>> -static bool intel_iommu_is_attach_deferred(struct iommu_domain *domain,
>> -                       struct device *dev)
>> +static bool intel_iommu_is_attach_deferred(struct device *dev)
>>   {
>>       return attach_deferred(dev);
>
> Seems like there's no need to wrap this now?

I just realized that I have cleaned this up in this patch.

https://lore.kernel.org/linux-iommu/20220207064142.1092846-7-baolu.lu@linux.intel.com/

Best regards,
baolu