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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Oct 03, 2025 at 04:46:57PM +0530, Nilawar, Badal wrote:
> 
> 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.

doh! my bad.

It is still about fixing the uninitialized uval, but it should had been changed indeed.
too late now that we have many patches on top already.

> 
> 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
> > >