Re: [PATCH] drm/xe/xe_late_bind_fw: Initialize uval variable in xe_late_bind_fw_num_fans()

["Nilawar, Badal" <badal.nilawar@intel.com>]

On 02-10-2025 19:40, Rodrigo Vivi wrote:
> On Thu, Oct 02, 2025 at 06:26:48AM +0530, mallesh.koujalagi@intel.com wrote:
>> From: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
>>
>> Initialize the uval variable to 0 in xe_late_bind_fw_num_fans() to fix
>> a potential use of uninitialized variable warning and ensure predictable
>> behavior.
>>
>> The variable is passed by reference to xe_pcode_read() which should
>> populate it on success, but initializing it to 0 provides a safe
>> default value and follows kernel coding best practices.
>>
>> v2:
>> - uval = 0 which serves as both a safe default and the fallback
>>    value when the pcode read operation fails.
>>
>> v3:
>> - Handle MMIO failure (Rodrigo)
>> - The function should probably return the error and make the uval as
>>    pointer-argument, like the pcode_read.
>> - Change the caller of this function to propagate the error
>>    upwards if mmio failed.
>>
>> Fixes: 45832bf9c10f3 ("drm/xe/xe_late_bind_fw: Initialize late binding firmware")
>> Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> and pushing soon

Is it pushed? The patch is no more using uval as title suggest.

Thanks,
Badal

>
>> ---
>>   drivers/gpu/drm/xe/xe_late_bind_fw.c | 16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> index 8f5082e689dc..768442ca7da6 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -184,17 +184,13 @@ static const char *xe_late_bind_parse_status(uint32_t status)
>>   	}
>>   }
>>   
>> -static int xe_late_bind_fw_num_fans(struct xe_late_bind *late_bind)
>> +static int xe_late_bind_fw_num_fans(struct xe_late_bind *late_bind, u32 *num_fans)
>>   {
>>   	struct xe_device *xe = late_bind_to_xe(late_bind);
>>   	struct xe_tile *root_tile = xe_device_get_root_tile(xe);
>> -	u32 uval;
>>   
>> -	if (!xe_pcode_read(root_tile,
>> -			   PCODE_MBOX(FAN_SPEED_CONTROL, FSC_READ_NUM_FANS, 0), &uval, NULL))
>> -		return uval;
>> -	else
>> -		return 0;
>> +	return xe_pcode_read(root_tile,
>> +			     PCODE_MBOX(FAN_SPEED_CONTROL, FSC_READ_NUM_FANS, 0), num_fans, NULL);
>>   }
>>   
>>   void xe_late_bind_wait_for_worker_completion(struct xe_late_bind *late_bind)
>> @@ -314,7 +310,11 @@ static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
>>   	lb_fw->flags &= ~INTEL_LB_FLAG_IS_PERSISTENT;
>>   
>>   	if (lb_fw->type == INTEL_LB_TYPE_FAN_CONTROL) {
>> -		num_fans = xe_late_bind_fw_num_fans(late_bind);
>> +		ret = xe_late_bind_fw_num_fans(late_bind, &num_fans);
>> +		if (ret) {
>> +			drm_dbg(&xe->drm, "Failed to read number of fans: %d\n", ret);
>> +			return 0; /* Not a fatal error, continue without fan control */
>> +		}
>>   		drm_dbg(&xe->drm, "Number of Fans: %d\n", num_fans);
>>   		if (!num_fans)
>>   			return 0;
>> -- 
>> 2.43.0
>>