[bug report] drm/amdgpu : Generate XGMI topology info from driver level

[Liu, Shaoyun]

It seems  patch d492c564  revert to this  original code otherwise we 
shouldn't get the warning . I can add ret =  -EINVAL for this.

Regards

shaoyun.liu


On 2019-01-04 6:53 a.m., Dan Carpenter wrote:
> Hello Shaoyun Liu,
>
> The patch fb30fc59a245: "drm/amdgpu : Generate XGMI topology info
> from driver level" from Jun 27, 2018, leads to the following static
> checker warning:
>
> 	drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c:117 amdgpu_xgmi_add_device()
> 	warn: missing error code here? 'amdgpu_get_xgmi_hive()' failed. 'ret' = '0'
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>      88 int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
>      89 {
>      90 	struct psp_xgmi_topology_info *hive_topology;
>      91 	struct amdgpu_hive_info *hive;
>      92 	struct amdgpu_xgmi	*entry;
>      93 	struct amdgpu_device *tmp_adev = NULL;
>      94
>      95 	int count = 0, ret = -EINVAL;
>      96
>      97 	if (!adev->gmc.xgmi.supported)
>      98 		return 0;
>      99
>      100 	ret = psp_xgmi_get_node_id(&adev->psp, &adev->gmc.xgmi.node_id);
>      101 	if (ret) {
>      102 		dev_err(adev->dev,
>      103 			"XGMI: Failed to get node id\n");
>      104 		return ret;
>      105 	}
>      106
>      107 	ret = psp_xgmi_get_hive_id(&adev->psp, &adev->gmc.xgmi.hive_id);
>      108 	if (ret) {
>      109 		dev_err(adev->dev,
>      110 			"XGMI: Failed to get hive id\n");
>      111 		return ret;
>      112 	}
>      113
>      114 	mutex_lock(&xgmi_mutex);
>      115 	hive = amdgpu_get_xgmi_hive(adev);
>      116 	if (!hive)
> --> 117 		goto exit;
>
> It does look like we should set "ret = -ESOMETHING;"...  Not sure what.
>
>      118
>      119 	hive_topology = &hive->topology_info;
>      120
>      121 	list_add_tail(&adev->gmc.xgmi.head, &hive->device_list);
>      122 	list_for_each_entry(entry, &hive->device_list, head)
>      123 		hive_topology->nodes[count++].node_id = entry->node_id;
>      124 	hive->number_devices = count;
>      125
>      126 	/* Each psp need to get the latest topology */
>      127 	list_for_each_entry(tmp_adev, &hive->device_list, gmc.xgmi.head) {
>      128 		ret = psp_xgmi_get_topology_info(&tmp_adev->psp, count, hive_topology);
>      129 		if (ret) {
>      130 			dev_err(tmp_adev->dev,
>      131 				"XGMI: Get topology failure on device %llx, hive %llx, ret %d",
>      132 				tmp_adev->gmc.xgmi.node_id,
>      133 				tmp_adev->gmc.xgmi.hive_id, ret);
>      134 			/* To do : continue with some node failed or disable the whole hive */
>      135 			break;
>      136 		}
>      137 	}
>      138
>      139 	list_for_each_entry(tmp_adev, &hive->device_list, gmc.xgmi.head) {
>      140 		ret = amdgpu_xgmi_update_topology(hive, tmp_adev);
>      141 		if (ret)
>      142 			break;
>      143 	}
>      144
>      145 exit:
>      146          mutex_unlock(&xgmi_mutex);
>      147          return ret;
>      148  }
>
> regards,
> dan carpenter