[PATCH -next] drm/amdgpu: remove set but not used variable 'ring'

[PATCH -next] drm/amdgpu: remove set but not used variable 'ring'

[YueHaibing]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/amdgpu/psp_v10_0.c: In function 'psp_v10_0_ring_stop':
drivers/gpu/drm/amd/amdgpu/psp_v10_0.c:230:19: warning:
 variable 'ring' set but not used [-Wunused-but-set-variable]

It not used since commit
4ef72453311a ("drm/amdgpu: added api for stopping psp ring (v2)")

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index 295c220..d78b430 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -240,12 +240,9 @@ static int psp_v10_0_ring_stop(struct psp_context *psp,
 			       enum psp_ring_type ring_type)
 {
 	int ret = 0;
-	struct psp_ring *ring;
 	unsigned int psp_ring_reg = 0;
 	struct amdgpu_device *adev = psp->adev;
 
-	ring = &psp->km_ring;
-
 	/* Write the ring destroy command to C2PMSG_64 */
 	psp_ring_reg = 3 << 16;
 	WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_64, psp_ring_reg);

RE: [PATCH -next] drm/amdgpu: remove set but not used variable 'ring'

[Huang, Ray]

> -----Original Message-----
> From: YueHaibing [mailto:yuehaibing@huawei.com]
> Sent: Thursday, November 15, 2018 6:52 PM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Zhou, David(ChunMing)
> <David1.Zhou@amd.com>; airlied@linux.ie; Liu, Shaoyun
> <Shaoyun.Liu@amd.com>; Francis, David <David.Francis@amd.com>; Huang,
> Ray <Ray.Huang@amd.com>; Xu, Feifei <Feifei.Xu@amd.com>; Gao, Likun
> <Likun.Gao@amd.com>
> Cc: YueHaibing <yuehaibing@huawei.com>; amd-gfx@lists.freedesktop.org;
> dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; kernel-
> janitors@vger.kernel.org
> Subject: [PATCH -next] drm/amdgpu: remove set but not used variable 'ring'
> 
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/gpu/drm/amd/amdgpu/psp_v10_0.c: In function
> 'psp_v10_0_ring_stop':
> drivers/gpu/drm/amd/amdgpu/psp_v10_0.c:230:19: warning:
>  variable 'ring' set but not used [-Wunused-but-set-variable]
> 
> It not used since commit
> 4ef72453311a ("drm/amdgpu: added api for stopping psp ring (v2)")

Thanks, please also remove the similar code in psp_v3_1. 
With that updates, patch is Reviewed-by: Huang Rui <ray.huang@amd.com>

> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> index 295c220..d78b430 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> @@ -240,12 +240,9 @@ static int psp_v10_0_ring_stop(struct psp_context
> *psp,
>  			       enum psp_ring_type ring_type)  {
>  	int ret = 0;
> -	struct psp_ring *ring;
>  	unsigned int psp_ring_reg = 0;
>  	struct amdgpu_device *adev = psp->adev;
> 
> -	ring = &psp->km_ring;
> -
>  	/* Write the ring destroy command to C2PMSG_64 */
>  	psp_ring_reg = 3 << 16;
>  	WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_64, psp_ring_reg);
> 
> 
> 
> 

Re: [PATCH -next] drm/amdgpu: remove set but not used variable 'ring'

[YueHaibing]

On 2018/11/15 19:49, Huang, Ray wrote:
>> -----Original Message-----
>> From: YueHaibing [mailto:yuehaibing@huawei.com]
>> Sent: Thursday, November 15, 2018 6:52 PM
>> To: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
>> <Christian.Koenig@amd.com>; Zhou, David(ChunMing)
>> <David1.Zhou@amd.com>; airlied@linux.ie; Liu, Shaoyun
>> <Shaoyun.Liu@amd.com>; Francis, David <David.Francis@amd.com>; Huang,
>> Ray <Ray.Huang@amd.com>; Xu, Feifei <Feifei.Xu@amd.com>; Gao, Likun
>> <Likun.Gao@amd.com>
>> Cc: YueHaibing <yuehaibing@huawei.com>; amd-gfx@lists.freedesktop.org;
>> dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; kernel-
>> janitors@vger.kernel.org
>> Subject: [PATCH -next] drm/amdgpu: remove set but not used variable 'ring'
>>
>> Fixes gcc '-Wunused-but-set-variable' warning:
>>
>> drivers/gpu/drm/amd/amdgpu/psp_v10_0.c: In function
>> 'psp_v10_0_ring_stop':
>> drivers/gpu/drm/amd/amdgpu/psp_v10_0.c:230:19: warning:
>>  variable 'ring' set but not used [-Wunused-but-set-variable]
>>
>> It not used since commit
>> 4ef72453311a ("drm/amdgpu: added api for stopping psp ring (v2)")
> 
> Thanks, please also remove the similar code in psp_v3_1. 
> With that updates, patch is Reviewed-by: Huang Rui <ray.huang@amd.com>
> 

Ok, will send v2.

>>
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
>> b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
>> index 295c220..d78b430 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
>> @@ -240,12 +240,9 @@ static int psp_v10_0_ring_stop(struct psp_context
>> *psp,
>>  			       enum psp_ring_type ring_type)  {
>>  	int ret = 0;
>> -	struct psp_ring *ring;
>>  	unsigned int psp_ring_reg = 0;
>>  	struct amdgpu_device *adev = psp->adev;
>>
>> -	ring = &psp->km_ring;
>> -
>>  	/* Write the ring destroy command to C2PMSG_64 */
>>  	psp_ring_reg = 3 << 16;
>>  	WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_64, psp_ring_reg);
>>
>>
>>
>>
> 
> 
> .
> 

[PATCH -next] drm/amdgpu: remove set but not used variables 'vm, bo'

[YueHaibing]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c: In function 'update_gpuvm_pte':
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:840:20: warning:
 variable 'bo' set but not used [-Wunused-but-set-variable]

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:838:20: warning:
 variable 'vm' set but not used [-Wunused-but-set-variable]

They're never used since introduction in a46a2cd103a8 ("drm/amdgpu: Add GPUVM
memory management functions for KFD")

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index d7b10d79f1de..63daf758cd03 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -837,13 +837,7 @@ static int update_gpuvm_pte(struct amdgpu_device *adev,
 		struct amdgpu_sync *sync)
 {
 	int ret;
-	struct amdgpu_vm *vm;
-	struct amdgpu_bo_va *bo_va;
-	struct amdgpu_bo *bo;
-
-	bo_va = entry->bo_va;
-	vm = bo_va->base.vm;
-	bo = bo_va->base.bo;
+	struct amdgpu_bo_va *bo_va = entry->bo_va;
 
 	/* Update the page tables  */
 	ret = amdgpu_vm_bo_update(adev, bo_va, false);

Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'vm, bo'

[Alex Deucher]

Applied.  thanks!

On Wed, Feb 20, 2019 at 3:05 AM YueHaibing <yuehaibing@huawei.com> wrote:
>
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c: In function 'update_gpuvm_pte':
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:840:20: warning:
>  variable 'bo' set but not used [-Wunused-but-set-variable]
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:838:20: warning:
>  variable 'vm' set but not used [-Wunused-but-set-variable]
>
> They're never used since introduction in a46a2cd103a8 ("drm/amdgpu: Add GPUVM
> memory management functions for KFD")
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index d7b10d79f1de..63daf758cd03 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -837,13 +837,7 @@ static int update_gpuvm_pte(struct amdgpu_device *adev,
>                 struct amdgpu_sync *sync)
>  {
>         int ret;
> -       struct amdgpu_vm *vm;
> -       struct amdgpu_bo_va *bo_va;
> -       struct amdgpu_bo *bo;
> -
> -       bo_va = entry->bo_va;
> -       vm = bo_va->base.vm;
> -       bo = bo_va->base.bo;
> +       struct amdgpu_bo_va *bo_va = entry->bo_va;
>
>         /* Update the page tables  */
>         ret = amdgpu_vm_bo_update(adev, bo_va, false);
>
>
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH -next] drm/amdgpu: remove set but not used variables 'ret'

[Mao Wenan]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
  int ret = 0;
      ^

It is never used since introduction in 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 0e6dba9..0bf4dd9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
 /* init amdgpu_pmu */
 int amdgpu_pmu_init(struct amdgpu_device *adev)
 {
-	int ret = 0;
-
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
 		/* init df */
-		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
+		init_pmu_by_type(adev, df_v3_6_attr_groups,
 				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
 				       DF_V3_6_MAX_COUNTERS);
 
-- 
2.7.4

Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret'

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1410 bytes --]



On Sat, 22 Jun 2019, Mao Wenan wrote:

> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>   int ret = 0;
>       ^
>
> It is never used since introduction in 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 0e6dba9..0bf4dd9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
>  /* init amdgpu_pmu */
>  int amdgpu_pmu_init(struct amdgpu_device *adev)
>  {
> -	int ret = 0;
> -
>  	switch (adev->asic_type) {
>  	case CHIP_VEGA20:
>  		/* init df */
> -		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> +		init_pmu_by_type(adev, df_v3_6_attr_groups,
>  				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>  				       DF_V3_6_MAX_COUNTERS);

Maybe it would be better to use ret?

If knowing whether the call has failed is really useless, then maybe the
return type should be void?

julia


>
> --
> 2.7.4
>
>

Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret'

[maowenan]

On 2019/6/22 14:02, Julia Lawall wrote:
> 
> 
> On Sat, 22 Jun 2019, Mao Wenan wrote:
> 
>> Fixes gcc '-Wunused-but-set-variable' warning:
>>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>>   int ret = 0;
>>       ^
>>
>> It is never used since introduction in 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> index 0e6dba9..0bf4dd9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
>>  /* init amdgpu_pmu */
>>  int amdgpu_pmu_init(struct amdgpu_device *adev)
>>  {
>> -	int ret = 0;
>> -
>>  	switch (adev->asic_type) {
>>  	case CHIP_VEGA20:
>>  		/* init df */
>> -		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
>> +		init_pmu_by_type(adev, df_v3_6_attr_groups,
>>  				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>>  				       DF_V3_6_MAX_COUNTERS);
> 
> Maybe it would be better to use ret?
> 
> If knowing whether the call has failed is really useless, then maybe the
> return type should be void?

right.

amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
which will use the return value.
amdgpu_device_init()
	r = amdgpu_pmu_init(adev);


thanks, I will send v2.
> 
> julia
> 
> 
>>
>> --
>> 2.7.4
>>
>>
> 

Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret'

[Dan Carpenter]

On Sat, Jun 22, 2019 at 11:03:14AM +0800, Mao Wenan wrote:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 0e6dba9..0bf4dd9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
>  /* init amdgpu_pmu */
>  int amdgpu_pmu_init(struct amdgpu_device *adev)
>  {
> -	int ret = 0;
> -
>  	switch (adev->asic_type) {
>  	case CHIP_VEGA20:
>  		/* init df */
> -		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> +		init_pmu_by_type(adev, df_v3_6_attr_groups,
>  				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>  				       DF_V3_6_MAX_COUNTERS);


You're resending this for other reasons, but don't forget to update the
indenting on the arguments so they still line up with the '('.

regards,
dan carpenter

[PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

[Mao Wenan]

There is one warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
  int ret = 0;
      ^
amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
which will use the return value. So it returns 'ret' for caller.
amdgpu_device_init()
	r = amdgpu_pmu_init(adev);

Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
 amdgpu_pmu_init().
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 0e6dba9..145e720 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
 	case CHIP_VEGA20:
 		/* init df */
 		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
-				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
-				       DF_V3_6_MAX_COUNTERS);
+							   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
+							   DF_V3_6_MAX_COUNTERS);
 
 		/* other pmu types go here*/
 		break;
@@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
 		return 0;
 	}
 
-	return 0;
+	return ret;
 }
 
 
-- 
2.7.4

Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1830 bytes --]



On Sat, 22 Jun 2019, Mao Wenan wrote:

> There is one warning:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>   int ret = 0;
>       ^
> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
> which will use the return value. So it returns 'ret' for caller.
> amdgpu_device_init()
> 	r = amdgpu_pmu_init(adev);
>
> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
>  amdgpu_pmu_init().
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 0e6dba9..145e720 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>  	case CHIP_VEGA20:
>  		/* init df */
>  		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> -				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> -				       DF_V3_6_MAX_COUNTERS);
> +							   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> +							   DF_V3_6_MAX_COUNTERS);
>
>  		/* other pmu types go here*/

I don't know what is the impact of the other pmu types that are planned
for the future.  Perhaps it would be better to abort the function
immediately in the case of a failure.

julia

>  		break;
> @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>  		return 0;
>  	}
>
> -	return 0;
> +	return ret;
>  }
>
>
> --
> 2.7.4
>
>

Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

[maowenan]

On 2019/6/22 21:06, Julia Lawall wrote:
> 
> 
> On Sat, 22 Jun 2019, Mao Wenan wrote:
> 
>> There is one warning:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>>   int ret = 0;
>>       ^
>> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
>> which will use the return value. So it returns 'ret' for caller.
>> amdgpu_device_init()
>> 	r = amdgpu_pmu_init(adev);
>>
>> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>>  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
>>  amdgpu_pmu_init().
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> index 0e6dba9..145e720 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>>  	case CHIP_VEGA20:
>>  		/* init df */
>>  		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
>> -				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>> -				       DF_V3_6_MAX_COUNTERS);
>> +							   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>> +							   DF_V3_6_MAX_COUNTERS);
>>
>>  		/* other pmu types go here*/
> 
> I don't know what is the impact of the other pmu types that are planned
> for the future.  Perhaps it would be better to abort the function
> immediately in the case of a failure.

I guess it would be better to use new function or new switch case clause to process different PMU types
in future.

> 
> julia
> 
>>  		break;
>> @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>>  		return 0;
>>  	}
>>
>> -	return 0;
>> +	return ret;
>>  }
>>
>>
>> --
>> 2.7.4
>>
>>
> 

Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 2459 bytes --]



On Sat, 22 Jun 2019, maowenan wrote:

>
>
> On 2019/6/22 21:06, Julia Lawall wrote:
> >
> >
> > On Sat, 22 Jun 2019, Mao Wenan wrote:
> >
> >> There is one warning:
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
> >>   int ret = 0;
> >>       ^
> >> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
> >> which will use the return value. So it returns 'ret' for caller.
> >> amdgpu_device_init()
> >> 	r = amdgpu_pmu_init(adev);
> >>
> >> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
> >>
> >> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >> ---
> >>  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
> >>  amdgpu_pmu_init().
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> >> index 0e6dba9..145e720 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> >> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
> >>  	case CHIP_VEGA20:
> >>  		/* init df */
> >>  		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> >> -				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> >> -				       DF_V3_6_MAX_COUNTERS);
> >> +							   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> >> +							   DF_V3_6_MAX_COUNTERS);
> >>
> >>  		/* other pmu types go here*/
> >
> > I don't know what is the impact of the other pmu types that are planned
> > for the future.  Perhaps it would be better to abort the function
> > immediately in the case of a failure.
>
> I guess it would be better to use new function or new switch case clause to process different PMU types
> in future.

I don't know.  But normally when an error may occur it is checked for
immediately, rather than just letting it go until the end of the function.
But maybe the developer know what is planned for the future for this
function.

julia

>
> >
> > julia
> >
> >>  		break;
> >> @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
> >>  		return 0;
> >>  	}
> >>
> >> -	return 0;
> >> +	return ret;
> >>  }
> >>
> >>
> >> --
> >> 2.7.4
> >>
> >>
> >
>
>

Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

[maowenan]

On 2019/6/22 22:00, Julia Lawall wrote:
> 
> 
> On Sat, 22 Jun 2019, maowenan wrote:
> 
>>
>>
>> On 2019/6/22 21:06, Julia Lawall wrote:
>>>
>>>
>>> On Sat, 22 Jun 2019, Mao Wenan wrote:
>>>
>>>> There is one warning:
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>>>>   int ret = 0;
>>>>       ^
>>>> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
>>>> which will use the return value. So it returns 'ret' for caller.
>>>> amdgpu_device_init()
>>>> 	r = amdgpu_pmu_init(adev);
>>>>
>>>> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
>>>>
>>>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>>>> ---
>>>>  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
>>>>  amdgpu_pmu_init().
>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>>>> index 0e6dba9..145e720 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>>>> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>>>>  	case CHIP_VEGA20:
>>>>  		/* init df */
>>>>  		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
>>>> -				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>>>> -				       DF_V3_6_MAX_COUNTERS);
>>>> +							   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>>>> +							   DF_V3_6_MAX_COUNTERS);
>>>>
>>>>  		/* other pmu types go here*/
>>>
>>> I don't know what is the impact of the other pmu types that are planned
>>> for the future.  Perhaps it would be better to abort the function
>>> immediately in the case of a failure.
>>

OK, v3 will be sent.

>> I guess it would be better to use new function or new switch case clause to process different PMU types
>> in future.
> 
> I don't know.  But normally when an error may occur it is checked for
> immediately, rather than just letting it go until the end of the function.
> But maybe the developer know what is planned for the future for this
> function.
> 
> julia
> 
>>
>>>
>>> julia
>>>
>>>>  		break;
>>>> @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>>>>  		return 0;
>>>>  	}
>>>>
>>>> -	return 0;
>>>> +	return ret;
>>>>  }
>>>>
>>>>
>>>> --
>>>> 2.7.4
>>>>
>>>>
>>>
>>
>>
> 

Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

[Joe Perches]

On Sat, 2019-06-22 at 21:05 +0800, Mao Wenan wrote:
> There is one warning:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>   int ret = 0;
[]
>  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
>  amdgpu_pmu_init().
[]
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
[]
> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>  	case CHIP_VEGA20:
>  		/* init df */
>  		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> -				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> -				       DF_V3_6_MAX_COUNTERS);
> +							   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> +							   DF_V3_6_MAX_COUNTERS);

trivia:

The indentation change seems superfluous and
appears to make the code harder to read.

You could also cc Jonathan Kim who wrote all of this.

Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

[maowenan]

On 2019/6/23 2:13, Joe Perches wrote:
> On Sat, 2019-06-22 at 21:05 +0800, Mao Wenan wrote:
>> There is one warning:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>>   int ret = 0;
> []
>>  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
>>  amdgpu_pmu_init().
> []
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> []
>> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>>  	case CHIP_VEGA20:
>>  		/* init df */
>>  		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
>> -				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>> -				       DF_V3_6_MAX_COUNTERS);
>> +							   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>> +							   DF_V3_6_MAX_COUNTERS);
> 
> trivia:
> 
> The indentation change seems superfluous and
> appears to make the code harder to read.
> 
> You could also cc Jonathan Kim who wrote all of this.
I think this is just display issue in mail format. It is correct that in vi/vim.
The arguments are line up with '(' after my change.


@@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)$
 ^Icase CHIP_VEGA20:$
 ^I^I/* init df */$
 ^I^Iret = init_pmu_by_type(adev, df_v3_6_attr_groups,$
-^I^I^I^I       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,$
-^I^I^I^I       DF_V3_6_MAX_COUNTERS);$
+^I^I^I^I^I^I^I   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,$
+^I^I^I^I^I^I^I   DF_V3_6_MAX_COUNTERS);$
 $
 ^I^I/* other pmu types go here*/$
 ^I^Ibreak;$





> 
> 
> 
> .
> 

Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

[Joe Perches]

On Mon, 2019-06-24 at 11:41 +0800, maowenan wrote:
> 
> On 2019/6/23 2:13, Joe Perches wrote:
> > On Sat, 2019-06-22 at 21:05 +0800, Mao Wenan wrote:
> > > There is one warning:
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
> > >   int ret = 0;
> > []
> > >  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
> > >  amdgpu_pmu_init().
> > []
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > []
> > > @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
> > >  	case CHIP_VEGA20:
> > >  		/* init df */
> > >  		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> > > -				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> > > -				       DF_V3_6_MAX_COUNTERS);
> > > +							   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> > > +							   DF_V3_6_MAX_COUNTERS);
> > 
> > trivia:
> > 
> > The indentation change seems superfluous and
> > appears to make the code harder to read.
> > 
> > You could also cc Jonathan Kim who wrote all of this.
> I think this is just display issue in mail format. It is correct that in vi/vim.
> The arguments are line up with '(' after my change.

Use 8 character tabs and try again please.

> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)$
>  ^Icase CHIP_VEGA20:$
>  ^I^I/* init df */$
>  ^I^Iret = init_pmu_by_type(adev, df_v3_6_attr_groups,$
> -^I^I^I^I       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,$
> -^I^I^I^I       DF_V3_6_MAX_COUNTERS);$
> +^I^I^I^I^I^I^I   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,$
> +^I^I^I^I^I^I^I   DF_V3_6_MAX_COUNTERS);$

Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret'

[Dan Carpenter]

On Sat, Jun 22, 2019 at 01:43:19PM +0300, Dan Carpenter wrote:
> On Sat, Jun 22, 2019 at 11:03:14AM +0800, Mao Wenan wrote:
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > index 0e6dba9..0bf4dd9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
> >  /* init amdgpu_pmu */
> >  int amdgpu_pmu_init(struct amdgpu_device *adev)
> >  {
> > -	int ret = 0;
> > -
> >  	switch (adev->asic_type) {
> >  	case CHIP_VEGA20:
> >  		/* init df */
> > -		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> > +		init_pmu_by_type(adev, df_v3_6_attr_groups,
> >  				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> >  				       DF_V3_6_MAX_COUNTERS);
> 
> 
> You're resending this for other reasons, but don't forget to update the
> indenting on the arguments so they still line up with the '('.
> 

Sorry, I was unclear.  If you pull the init_pmu_by_type( back 6
characters then you also need to pull the "DF" back 6 characters.

		init_pmu_by_type(adev, df_v3_6_attr_groups, "DF", "amdgpu_df",
				 PERF_TYPE_AMDGPU_DF, DF_V3_6_MAX_COUNTERS);

You can actually fit it into two lines afterwards.

regards,
dan carpenter

Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret'

[Julia Lawall]

On Sun, 23 Jun 2019, Dan Carpenter wrote:

> On Sat, Jun 22, 2019 at 01:43:19PM +0300, Dan Carpenter wrote:
> > On Sat, Jun 22, 2019 at 11:03:14AM +0800, Mao Wenan wrote:
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > > index 0e6dba9..0bf4dd9 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > > @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
> > >  /* init amdgpu_pmu */
> > >  int amdgpu_pmu_init(struct amdgpu_device *adev)
> > >  {
> > > -	int ret = 0;
> > > -
> > >  	switch (adev->asic_type) {
> > >  	case CHIP_VEGA20:
> > >  		/* init df */
> > > -		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> > > +		init_pmu_by_type(adev, df_v3_6_attr_groups,
> > >  				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> > >  				       DF_V3_6_MAX_COUNTERS);
> >
> >
> > You're resending this for other reasons, but don't forget to update the
> > indenting on the arguments so they still line up with the '('.
> >
>
> Sorry, I was unclear.  If you pull the init_pmu_by_type( back 6
> characters then you also need to pull the "DF" back 6 characters.
>
> 		init_pmu_by_type(adev, df_v3_6_attr_groups, "DF", "amdgpu_df",
> 				 PERF_TYPE_AMDGPU_DF, DF_V3_6_MAX_COUNTERS);
>
> You can actually fit it into two lines afterwards.

My suggestion was to keep the ret = instead of discarding the indication
of failure, so I think that this is not relevant.

julia

[PATCH -next v3] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init

[Mao Wenan]

There is one warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
  int ret = 0;
      ^
amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed.
amdgpu_device_init()
	r = amdgpu_pmu_init(adev);

This patch is also to update the indenting on the arguments so they line up with the '('.

Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
 amdgpu_pmu_init().
 v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). 

 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 0e6dba9..b702322 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -252,8 +252,11 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
 	case CHIP_VEGA20:
 		/* init df */
 		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
-				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
-				       DF_V3_6_MAX_COUNTERS);
+							   "DF", "amdgpu_df",
+							   PERF_TYPE_AMDGPU_DF,
+							   DF_V3_6_MAX_COUNTERS);
+		if (ret)
+			return ret;
 
 		/* other pmu types go here*/
 		break;
-- 
2.7.4

Re: [PATCH -next v3] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init

[Dan Carpenter]

On Mon, Jun 24, 2019 at 11:45:32AM +0800, Mao Wenan wrote:
> There is one warning:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>   int ret = 0;
>       ^
> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
> which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed.
> amdgpu_device_init()
> 	r = amdgpu_pmu_init(adev);
> 
> This patch is also to update the indenting on the arguments so they line up with the '('.
> 
> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
>  amdgpu_pmu_init().
>  v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). 
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 0e6dba9..b702322 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -252,8 +252,11 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>  	case CHIP_VEGA20:
>  		/* init df */
>  		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> -				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> -				       DF_V3_6_MAX_COUNTERS);
> +							   "DF", "amdgpu_df",
> +							   PERF_TYPE_AMDGPU_DF,
> +							   DF_V3_6_MAX_COUNTERS);
> +		if (ret)
> +			return ret;

No no.  Sorry, the original indenting was correct and lined up with the
'(' character in 'init_pmu_by_type(', that's the way it should be.  If
we were to remove the "ret = " then we'd have to pull the arguments back
as well.  I think this fix that Julia suggested is really the right so
leave the indenting alone.

It looks like you've right aligned the arguments.  That's not the right
way, the original was correct.

regards,
dan carpenter

Re: [PATCH -next v3] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init

[maowenan]

On 2019/6/24 16:39, Dan Carpenter wrote:
> On Mon, Jun 24, 2019 at 11:45:32AM +0800, Mao Wenan wrote:
>> There is one warning:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>>   int ret = 0;
>>       ^
>> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
>> which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed.
>> amdgpu_device_init()
>> 	r = amdgpu_pmu_init(adev);
>>
>> This patch is also to update the indenting on the arguments so they line up with the '('.
>>
>> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>>  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
>>  amdgpu_pmu_init().
>>  v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). 
>>
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> index 0e6dba9..b702322 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> @@ -252,8 +252,11 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>>  	case CHIP_VEGA20:
>>  		/* init df */
>>  		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
>> -				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>> -				       DF_V3_6_MAX_COUNTERS);
>> +							   "DF", "amdgpu_df",
>> +							   PERF_TYPE_AMDGPU_DF,
>> +							   DF_V3_6_MAX_COUNTERS);
>> +		if (ret)
>> +			return ret;
> 
> No no.  Sorry, the original indenting was correct and lined up with the
> '(' character in 'init_pmu_by_type(', that's the way it should be.  If
> we were to remove the "ret = " then we'd have to pull the arguments back
> as well.  I think this fix that Julia suggested is really the right so
> leave the indenting alone.
> 

> It looks like you've right aligned the arguments.  That's not the right
> way, the original was correct.
> 
After using 8 character for tab(thanks to Joe), the aligned here is wrong, yes, the original was correct.

so my v4 is only to change ret, don't change the indenting?

> regards,
> dan carpenter
> 
> 
> .
> 

Re: [PATCH -next v3] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init

[Dan Carpenter]

On Mon, Jun 24, 2019 at 05:29:33PM +0800, maowenan wrote:
> 
> 
> On 2019/6/24 16:39, Dan Carpenter wrote:
> > On Mon, Jun 24, 2019 at 11:45:32AM +0800, Mao Wenan wrote:
> >> There is one warning:
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
> >>   int ret = 0;
> >>       ^
> >> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
> >> which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed.
> >> amdgpu_device_init()
> >> 	r = amdgpu_pmu_init(adev);
> >>
> >> This patch is also to update the indenting on the arguments so they line up with the '('.
> >>
> >> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
> >>
> >> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >> ---
> >>  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
> >>  amdgpu_pmu_init().
> >>  v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). 
> >>
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 7 +++++--
> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> >> index 0e6dba9..b702322 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> >> @@ -252,8 +252,11 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
> >>  	case CHIP_VEGA20:
> >>  		/* init df */
> >>  		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> >> -				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> >> -				       DF_V3_6_MAX_COUNTERS);
> >> +							   "DF", "amdgpu_df",
> >> +							   PERF_TYPE_AMDGPU_DF,
> >> +							   DF_V3_6_MAX_COUNTERS);
> >> +		if (ret)
> >> +			return ret;
> > 
> > No no.  Sorry, the original indenting was correct and lined up with the
> > '(' character in 'init_pmu_by_type(', that's the way it should be.  If
> > we were to remove the "ret = " then we'd have to pull the arguments back
> > as well.  I think this fix that Julia suggested is really the right so
> > leave the indenting alone.
> > 
> 
> > It looks like you've right aligned the arguments.  That's not the right
> > way, the original was correct.
> > 
> After using 8 character for tab(thanks to Joe), the aligned here is wrong, yes, the original was correct.
> 
> so my v4 is only to change ret, don't change the indenting?
> 

Yes, please.  Sorry for my confusing email earlier.

regards,
dan carpenter

[PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init

[Mao Wenan]

There is one warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
  int ret = 0;
      ^
amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed.
amdgpu_device_init()
	r = amdgpu_pmu_init(adev);

Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in
 amdgpu_pmu_init().
 v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). 
 v3->v4: delete the indenting for init_pmu_by_type() arguments. The original indenting is correct.

 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 0e6dba9f60f0..c98cf77a37f3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -254,6 +254,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
 		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
 				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
 				       DF_V3_6_MAX_COUNTERS);
+		if (ret)
+			return ret;
 
 		/* other pmu types go here*/
 		break;
-- 
2.20.1

RE: [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init

[Kim, Jonathan]

SW1tZWRpYXRlIHJldHVybiBzaG91bGQgYmUgb2sgc2luY2UgcGVyZiByZWdpc3RyYXRpb24gaXNu
J3QgZGVwZW5kZW50IG9uIGdwdSBody4NCg0KUmV2aWV3ZWQtYnk6IEpvbmF0aGFuIEtpbSA8Sm9u
YXRoYW4uS2ltQGFtZC5jb20+DQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBN
YW8gV2VuYW4gPG1hb3dlbmFuQGh1YXdlaS5jb20+IA0KU2VudDogTW9uZGF5LCBKdW5lIDI0LCAy
MDE5IDc6MjMgQU0NClRvOiBhaXJsaWVkQGxpbnV4LmllOyBkYW5pZWxAZmZ3bGwuY2g7IERldWNo
ZXIsIEFsZXhhbmRlciA8QWxleGFuZGVyLkRldWNoZXJAYW1kLmNvbT47IEtvZW5pZywgQ2hyaXN0
aWFuIDxDaHJpc3RpYW4uS29lbmlnQGFtZC5jb20+OyBaaG91LCBEYXZpZChDaHVuTWluZykgPERh
dmlkMS5aaG91QGFtZC5jb20+OyBkYW4uY2FycGVudGVyQG9yYWNsZS5jb207IGp1bGlhLmxhd2Fs
bEBsaXA2LmZyDQpDYzoga2VybmVsLWphbml0b3JzQHZnZXIua2VybmVsLm9yZzsgYW1kLWdmeEBs
aXN0cy5mcmVlZGVza3RvcC5vcmc7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IEtpbSwg
Sm9uYXRoYW4gPEpvbmF0aGFuLktpbUBhbWQuY29tPjsgTWFvIFdlbmFuIDxtYW93ZW5hbkBodWF3
ZWkuY29tPg0KU3ViamVjdDogW1BBVENIIC1uZXh0IHY0XSBkcm0vYW1kZ3B1OiByZXR1cm4gJ3Jl
dCcgaW1tZWRpYXRlbHkgaWYgZmFpbGVkIGluIGFtZGdwdV9wbXVfaW5pdA0KDQpbQ0FVVElPTjog
RXh0ZXJuYWwgRW1haWxdDQoNClRoZXJlIGlzIG9uZSB3YXJuaW5nOg0KZHJpdmVycy9ncHUvZHJt
L2FtZC9hbWRncHUvYW1kZ3B1X3BtdS5jOiBJbiBmdW5jdGlvbiDigJhhbWRncHVfcG11X2luaXTi
gJk6DQpkcml2ZXJzL2dwdS9kcm0vYW1kL2FtZGdwdS9hbWRncHVfcG11LmM6MjQ5OjY6IHdhcm5p
bmc6IHZhcmlhYmxlIOKAmHJldOKAmSBzZXQgYnV0IG5vdCB1c2VkIFstV3VudXNlZC1idXQtc2V0
LXZhcmlhYmxlXQ0KICBpbnQgcmV0ID0gMDsNCiAgICAgIF4NCmFtZGdwdV9wbXVfaW5pdCgpIGlz
IGNhbGxlZCBieSBhbWRncHVfZGV2aWNlX2luaXQoKSBpbiBkcml2ZXJzL2dwdS9kcm0vYW1kL2Ft
ZGdwdS9hbWRncHVfZGV2aWNlLmMsDQp3aGljaCB3aWxsIHVzZSB0aGUgcmV0dXJuIHZhbHVlLiBT
byBpdCBzaG91bGQgcmV0dXJuICdyZXQnIGltbWVkaWF0ZWx5IGlmIGluaXRfcG11X2J5X3R5cGUo
KSBmYWlsZWQuDQphbWRncHVfZGV2aWNlX2luaXQoKQ0KICAgICAgICByID0gYW1kZ3B1X3BtdV9p
bml0KGFkZXYpOw0KDQpGaXhlczogOWM3Yzg1ZjdlYTFmICgiZHJtL2FtZGdwdTogYWRkIHBtdSBj
b3VudGVycyIpDQoNClNpZ25lZC1vZmYtYnk6IE1hbyBXZW5hbiA8bWFvd2VuYW5AaHVhd2VpLmNv
bT4NCi0tLQ0KIHYxLT52MjogY2hhbmdlIHRoZSBzdWJqZWN0IGZvciB0aGlzIHBhdGNoOyBjaGFu
Z2UgdGhlIGluZGVudGluZyB3aGVuIGl0IGNhbGxzIGluaXRfcG11X2J5X3R5cGU7IHVzZSB0aGUg
dmFsdWUgJ3JldCcgaW4gIGFtZGdwdV9wbXVfaW5pdCgpLg0KIHYyLT52MzogY2hhbmdlIHRoZSBz
dWJqZWN0IGZvciB0aGlzIHBhdGNoOyByZXR1cm4gJ3JldCcgaW1tZWRpYXRlbHkgaWYgZmFpbGVk
IHRvIGNhbGwgaW5pdF9wbXVfYnlfdHlwZSgpLg0KIHYzLT52NDogZGVsZXRlIHRoZSBpbmRlbnRp
bmcgZm9yIGluaXRfcG11X2J5X3R5cGUoKSBhcmd1bWVudHMuIFRoZSBvcmlnaW5hbCBpbmRlbnRp
bmcgaXMgY29ycmVjdC4NCg0KIGRyaXZlcnMvZ3B1L2RybS9hbWQvYW1kZ3B1L2FtZGdwdV9wbXUu
YyB8IDIgKysNCiAxIGZpbGUgY2hhbmdlZCwgMiBpbnNlcnRpb25zKCspDQoNCmRpZmYgLS1naXQg
YS9kcml2ZXJzL2dwdS9kcm0vYW1kL2FtZGdwdS9hbWRncHVfcG11LmMgYi9kcml2ZXJzL2dwdS9k
cm0vYW1kL2FtZGdwdS9hbWRncHVfcG11LmMNCmluZGV4IDBlNmRiYTlmNjBmMC4uYzk4Y2Y3N2Ez
N2YzIDEwMDY0NA0KLS0tIGEvZHJpdmVycy9ncHUvZHJtL2FtZC9hbWRncHUvYW1kZ3B1X3BtdS5j
DQorKysgYi9kcml2ZXJzL2dwdS9kcm0vYW1kL2FtZGdwdS9hbWRncHVfcG11LmMNCkBAIC0yNTQs
NiArMjU0LDggQEAgaW50IGFtZGdwdV9wbXVfaW5pdChzdHJ1Y3QgYW1kZ3B1X2RldmljZSAqYWRl
dikNCiAgICAgICAgICAgICAgICByZXQgPSBpbml0X3BtdV9ieV90eXBlKGFkZXYsIGRmX3YzXzZf
YXR0cl9ncm91cHMsDQogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAiREYi
LCAiYW1kZ3B1X2RmIiwgUEVSRl9UWVBFX0FNREdQVV9ERiwNCiAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgIERGX1YzXzZfTUFYX0NPVU5URVJTKTsNCisgICAgICAgICAgICAg
ICBpZiAocmV0KQ0KKyAgICAgICAgICAgICAgICAgICAgICAgcmV0dXJuIHJldDsNCg0KICAgICAg
ICAgICAgICAgIC8qIG90aGVyIHBtdSB0eXBlcyBnbyBoZXJlKi8NCiAgICAgICAgICAgICAgICBi
cmVhazsNCi0tDQoyLjIwLjENCg0K

Re: [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init

[maowenan]

On 2019/6/25 1:42, Kim, Jonathan wrote:
> Immediate return should be ok since perf registration isn't dependent on gpu hw.
> 
> Reviewed-by: Jonathan Kim <Jonathan.Kim@amd.com>

thanks for review.

> 
> -----Original Message-----
> From: Mao Wenan <maowenan@huawei.com> 
> Sent: Monday, June 24, 2019 7:23 AM
> To: airlied@linux.ie; daniel@ffwll.ch; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Zhou, David(ChunMing) <David1.Zhou@amd.com>; dan.carpenter@oracle.com; julia.lawall@lip6.fr
> Cc: kernel-janitors@vger.kernel.org; amd-gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org; Kim, Jonathan <Jonathan.Kim@amd.com>; Mao Wenan <maowenan@huawei.com>
> Subject: [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init
> 
> [CAUTION: External Email]
> 
> There is one warning:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>   int ret = 0;
>       ^
> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
> which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed.
> amdgpu_device_init()
>         r = amdgpu_pmu_init(adev);
> 
> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in  amdgpu_pmu_init().
>  v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type().
>  v3->v4: delete the indenting for init_pmu_by_type() arguments. The original indenting is correct.
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 0e6dba9f60f0..c98cf77a37f3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -254,6 +254,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>                 ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
>                                        "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>                                        DF_V3_6_MAX_COUNTERS);
> +               if (ret)
> +                       return ret;
> 
>                 /* other pmu types go here*/
>                 break;
> --
> 2.20.1
> 

Re: [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init

[maowenan]

gentle ping


On 2019/6/26 19:35, maowenan wrote:
> 
> 
> On 2019/6/25 1:42, Kim, Jonathan wrote:
>> Immediate return should be ok since perf registration isn't dependent on gpu hw.
>>
>> Reviewed-by: Jonathan Kim <Jonathan.Kim@amd.com>
> 
> thanks for review.
> 
>>
>> -----Original Message-----
>> From: Mao Wenan <maowenan@huawei.com> 
>> Sent: Monday, June 24, 2019 7:23 AM
>> To: airlied@linux.ie; daniel@ffwll.ch; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Zhou, David(ChunMing) <David1.Zhou@amd.com>; dan.carpenter@oracle.com; julia.lawall@lip6.fr
>> Cc: kernel-janitors@vger.kernel.org; amd-gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org; Kim, Jonathan <Jonathan.Kim@amd.com>; Mao Wenan <maowenan@huawei.com>
>> Subject: [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init
>>
>> [CAUTION: External Email]
>>
>> There is one warning:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>>   int ret = 0;
>>       ^
>> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c,
>> which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed.
>> amdgpu_device_init()
>>         r = amdgpu_pmu_init(adev);
>>
>> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters")
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>>  v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in  amdgpu_pmu_init().
>>  v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type().
>>  v3->v4: delete the indenting for init_pmu_by_type() arguments. The original indenting is correct.
>>
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> index 0e6dba9f60f0..c98cf77a37f3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
>> @@ -254,6 +254,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>>                 ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
>>                                        "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
>>                                        DF_V3_6_MAX_COUNTERS);
>> +               if (ret)
>> +                       return ret;
>>
>>                 /* other pmu types go here*/
>>                 break;
>> --
>> 2.20.1
>>
> 
> 
> .
> 

[PATCH -next] drm/amdgpu: remove set but not used variable 'adev'

[YueHaibing]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c: In function 'amdgpu_init_mem_type':
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:81:24: warning:
 variable 'adev' set but not used [-Wunused-but-set-variable]
  struct amdgpu_device *adev;
                        ^

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index bb95627ad2cc..8199702d3354 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -78,10 +78,6 @@
 static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 				struct ttm_mem_type_manager *man)
 {
-	struct amdgpu_device *adev;
-
-	adev = amdgpu_ttm_adev(bdev);
-
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */

Re: [PATCH -next] drm/amdgpu: remove set but not used variable 'adev'

[Alex Deucher]

On Thu, Jul 2, 2020 at 3:25 AM YueHaibing <yuehaibing@huawei.com> wrote:
>
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c: In function 'amdgpu_init_mem_type':
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:81:24: warning:
>  variable 'adev' set but not used [-Wunused-but-set-variable]
>   struct amdgpu_device *adev;
>                         ^
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

With our current -next tree, this patch is no longer applicable.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index bb95627ad2cc..8199702d3354 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -78,10 +78,6 @@
>  static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>                                 struct ttm_mem_type_manager *man)
>  {
> -       struct amdgpu_device *adev;
> -
> -       adev = amdgpu_ttm_adev(bdev);
> -
>         switch (type) {
>         case TTM_PL_SYSTEM:
>                 /* System memory */
>
>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx