Re: [PATCH] iommu: move the adding group info to proper place

[Vasant Hegde <vasant.hegde@amd.com>]

On 1/3/2025 10:58 AM, Li RongQing wrote:
> After commit fa0828036488 ("iommu: Split iommu_group_add_device()"), the
> adding group information is left in iommu_group_alloc_device, but which
> only allocate group device, not attach the device to iommu group;
> 
> so move the print to iommu probing and adding functions
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  drivers/iommu/iommu.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 599030e..03f95f2 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -586,6 +586,9 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
>  
>  	mutex_unlock(&group->mutex);
>  
> +	trace_add_device_to_group(group->id, dev);
> +	dev_info(dev, "Adding to iommu group %d\n", group->id);
> +
>  	return 0;
>  
>  err_remove_gdev:
> @@ -594,6 +597,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
>  err_put_group:
>  	iommu_deinit_device(dev);
>  	mutex_unlock(&group->mutex);
> +	dev_err(dev, "Failed to add to iommu group %d: %d\n", group->id, ret);
>  	iommu_group_put(group);
>  
>  	return ret;
> @@ -1192,10 +1196,6 @@ static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
>  		goto err_free_name;
>  	}
>  
> -	trace_add_device_to_group(group->id, dev);
> -
> -	dev_info(dev, "Adding to iommu group %d\n", group->id);
> -
>  	return device;
>  
>  err_free_name:
> @@ -1204,7 +1204,6 @@ static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
>  	sysfs_remove_link(&dev->kobj, "iommu_group");
>  err_free_device:
>  	kfree(device);
> -	dev_err(dev, "Failed to add to iommu group %d: %d\n", group->id, ret);
>  	return ERR_PTR(ret);
>  }
>  
> @@ -1230,6 +1229,10 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)

Add error message if iommu_group_alloc_device() fails here?

-Vasant