[AMD Official Use Only - AMD Internal Distribution Only]

Hi Christian,

Thanks for your comments, the RREG32_SOC15() macro should return the 32bit wide 
value so it couldn't need the mask.
And I will send out v2 patch to modify the commit message and use uint32_t or 
u32 instead .

Regards,
Bob

-----Original Message-----
From: Koenig, Christian <[email protected]>
Sent: 2024年6月3日 19:11
To: Zhou, Bob <[email protected]>; [email protected]; Huang, Tim 
<[email protected]>; Zhang, Jesse(Jie) <[email protected]>
Cc: Deucher, Alexander <[email protected]>
Subject: Re: [PATCH] drm/amdgpu: replace int with unsigned int for imu_v12_0.c

Am 03.06.24 um 10:53 schrieb Zhou, Bob:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> Hi Christian,
>
> It fixes a potential Overflowed constant (INTEGER_OVERFLOW) warning reported 
> by Coverity.

You need to mention that in the commit message.

And I haven't checked the hardware docs, but it can be that this isn't the 
right solution for the warning. Instead you should probably use the
RREG32_SOC15() macro with a mask which only leaves the valid bits around.

Only when the register is really 32bit wide you need an unsigned datatype and 
if that is the case I suggest to use either uint32_t or u32 instead.

Regards,
Christian.

>
> Regards,
> Bob
>
> -----Original Message-----
> From: Koenig, Christian <[email protected]>
> Sent: 2024年6月3日 15:56
> To: Zhou, Bob <[email protected]>; [email protected];
> Huang, Tim <[email protected]>; Zhang, Jesse(Jie)
> <[email protected]>
> Cc: Deucher, Alexander <[email protected]>
> Subject: Re: [PATCH] drm/amdgpu: replace int with unsigned int for
> imu_v12_0.c
>
> Am 03.06.24 um 07:59 schrieb Bob Zhou:
>> The return value of RREG32_SOC15 is unsigned int, so modify variable to 
>> unsigned.
> And why is that an improvement?
>
> Regards,
> Christian.
>
>> Signed-off-by: Bob Zhou <[email protected]>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/imu_v12_0.c | 6 +++---
>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/imu_v12_0.c
>> b/drivers/gpu/drm/amd/amdgpu/imu_v12_0.c
>> index 0c8ef908d112..2d6f7549c2af 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/imu_v12_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/imu_v12_0.c
>> @@ -119,7 +119,7 @@ static int imu_v12_0_load_microcode(struct
>> amdgpu_device *adev)
>>
>>    static int imu_v12_0_wait_for_reset_status(struct amdgpu_device *adev)
>>    {
>> -     int i, imu_reg_val = 0;
>> +     unsigned int i, imu_reg_val = 0;
>>
>>        for (i = 0; i < adev->usec_timeout; i++) {
>>                imu_reg_val = RREG32_SOC15(GC, 0,
>> regGFX_IMU_GFX_RESET_CTRL); @@
>> -138,7 +138,7 @@ static int imu_v12_0_wait_for_reset_status(struct
>> amdgpu_device *adev)
>>
>>    static void imu_v12_0_setup(struct amdgpu_device *adev)
>>    {
>> -     int imu_reg_val;
>> +     unsigned int imu_reg_val;
>>
>>        WREG32_SOC15(GC, 0, regGFX_IMU_C2PMSG_ACCESS_CTRL0, 0xffffff);
>>        WREG32_SOC15(GC, 0, regGFX_IMU_C2PMSG_ACCESS_CTRL1, 0xffff);
>> @@
>> -157,7 +157,7 @@ static void imu_v12_0_setup(struct amdgpu_device
>> *adev)
>>
>>    static int imu_v12_0_start(struct amdgpu_device *adev)
>>    {
>> -     int imu_reg_val;
>> +     unsigned int imu_reg_val;
>>
>>        imu_reg_val = RREG32_SOC15(GC, 0, regGFX_IMU_CORE_CTRL);
>>        imu_reg_val &= 0xfffffffe;

Reply via email to