[PATCH 1/2] drm/amdgpu: cleanup amdgpu_xgmi_sysfs_add_dev_info

[Luben Tuikov]

Yeah, that's cleaner.

Reviewed-by: Luben Tuikov <luben.tuikov at amd.com>

Regards,
Luben

On 2022-01-26 06:59, Christian König wrote:
> Don't initialize variables if it isn't absolutely necessary.
>
> Use amdgpu_xgmi_sysfs_rem_dev_info to cleanup when something goes wrong.
>
> Drop the explicit warnings since the sysfs core warns about things like
> duplicate files itself.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 85 +++++++++---------------
>  1 file changed, 33 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 5929d6f528c9..68509f619ba3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -289,61 +289,10 @@ static ssize_t amdgpu_xgmi_show_error(struct device *dev,
>  static DEVICE_ATTR(xgmi_device_id, S_IRUGO, amdgpu_xgmi_show_device_id, NULL);
>  static DEVICE_ATTR(xgmi_error, S_IRUGO, amdgpu_xgmi_show_error, NULL);
>  
> -static int amdgpu_xgmi_sysfs_add_dev_info(struct amdgpu_device *adev,
> -					 struct amdgpu_hive_info *hive)
> -{
> -	int ret = 0;
> -	char node[10] = { 0 };
> -
> -	/* Create xgmi device id file */
> -	ret = device_create_file(adev->dev, &dev_attr_xgmi_device_id);
> -	if (ret) {
> -		dev_err(adev->dev, "XGMI: Failed to create device file xgmi_device_id\n");
> -		return ret;
> -	}
> -
> -	/* Create xgmi error file */
> -	ret = device_create_file(adev->dev, &dev_attr_xgmi_error);
> -	if (ret)
> -		pr_err("failed to create xgmi_error\n");
> -
> -
> -	/* Create sysfs link to hive info folder on the first device */
> -	if (hive->kobj.parent != (&adev->dev->kobj)) {
> -		ret = sysfs_create_link(&adev->dev->kobj, &hive->kobj,
> -					"xgmi_hive_info");
> -		if (ret) {
> -			dev_err(adev->dev, "XGMI: Failed to create link to hive info");
> -			goto remove_file;
> -		}
> -	}
> -
> -	sprintf(node, "node%d", atomic_read(&hive->number_devices));
> -	/* Create sysfs link form the hive folder to yourself */
> -	ret = sysfs_create_link(&hive->kobj, &adev->dev->kobj, node);
> -	if (ret) {
> -		dev_err(adev->dev, "XGMI: Failed to create link from hive info");
> -		goto remove_link;
> -	}
> -
> -	goto success;
> -
> -
> -remove_link:
> -	sysfs_remove_link(&adev->dev->kobj, adev_to_drm(adev)->unique);
> -
> -remove_file:
> -	device_remove_file(adev->dev, &dev_attr_xgmi_device_id);
> -
> -success:
> -	return ret;
> -}
> -
>  static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
>  					  struct amdgpu_hive_info *hive)
>  {
>  	char node[10];
> -	memset(node, 0, sizeof(node));
>  
>  	device_remove_file(adev->dev, &dev_attr_xgmi_device_id);
>  	device_remove_file(adev->dev, &dev_attr_xgmi_error);
> @@ -353,10 +302,42 @@ static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
>  
>  	sprintf(node, "node%d", atomic_read(&hive->number_devices));
>  	sysfs_remove_link(&hive->kobj, node);
> -
>  }
>  
> +static int amdgpu_xgmi_sysfs_add_dev_info(struct amdgpu_device *adev,
> +					 struct amdgpu_hive_info *hive)
> +{
> +	char node[10];
> +	int r;
> +
> +	r = device_create_file(adev->dev, &dev_attr_xgmi_device_id);
> +	if (r)
> +		return r;
> +
> +	r = device_create_file(adev->dev, &dev_attr_xgmi_error);
> +	if (r)
> +		goto error;
>  
> +	/* Create sysfs link to hive info folder on the first device */
> +	if (hive->kobj.parent != (&adev->dev->kobj)) {
> +		r = sysfs_create_link(&adev->dev->kobj, &hive->kobj,
> +				      "xgmi_hive_info");
> +		if (r)
> +			goto error;
> +	}
> +
> +	/* Create sysfs link form the hive folder to yourself */
> +	sprintf(node, "node%d", atomic_read(&hive->number_devices));
> +	r = sysfs_create_link(&hive->kobj, &adev->dev->kobj, node);
> +	if (r)
> +		goto error;
> +
> +	return 0;
> +
> +error:
> +	amdgpu_xgmi_sysfs_rem_dev_info(adev, hive);
> +	return r;
> +}
>  
>  struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev)
>  {

Regards,
-- 
Luben