Yeah, that's cleaner.
Reviewed-by: Luben Tuikov <luben.tuikov@xxxxxxx>
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@xxxxxxx>
> ---
> 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
