Re: [PATCH mt76 5/6] wifi: mt76: mt7996: fix the temporary buffer for calibration-free data

["Shayne Chen (陳軒丞)" <Shayne.Chen@mediatek.com>]

On Thu, 2026-03-12 at 11:49 +0100, Lorenzo Bianconi wrote:
> > From: StanleyYP Wang <StanleyYP.Wang@mediatek.com>
> > 
> > Move the declaration of buf[] outside the for loop.
> > 
> > Fixes: 224c7c2be578 ("wifi: mt76: mt7996: apply calibration-free
> > data from OTP")
> > Signed-off-by: StanleyYP Wang <StanleyYP.Wang@mediatek.com>
> > Signed-off-by: Shayne Chen <shayne.chen@mediatek.com>
> > ---
> >  drivers/net/wireless/mediatek/mt76/mt7996/mcu.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > index 2a9c59d15894..4c733f51a03e 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > @@ -4104,6 +4104,7 @@ mt7996_mcu_get_cal_free_data(struct
> > mt7996_dev *dev)
> >  	}
> >  
> >  	for (band = 0; band < __MT_MAX_BAND; band++) {
> > +		u8 buf[MT7996_EEPROM_BLOCK_SIZE];
> 
> why are you moving buf here? It is only used in the inner block.
> 
> >  		const struct cal_free_data *cal;
> >  		u16 prev_block_idx = -1;
> >  		u16 adie_base;
> > @@ -4126,13 +4127,13 @@ mt7996_mcu_get_cal_free_data(struct
> > mt7996_dev *dev)
> >  			u16 eep_offset = cal[i].eep_offs;
> >  			u16 block_idx = adie_offset /
> > MT7996_EEPROM_BLOCK_SIZE;
> >  			u16 offset = adie_offset %
> > MT7996_EEPROM_BLOCK_SIZE;
> > -			u8 buf[MT7996_EEPROM_BLOCK_SIZE];
> 
> I think we should always define and initialize buf array here,
> otherwise we
> could use it with uninitialized values. Something like:
> 
> 			u8 buf[MT7996_EEPROM_BLOCK_SIZE] = {};
> 
Hi Lorenzo,

The buf array is updated only if the next offset is in a different
block. For example, offsets 0x4c and 0x4d have the same block index, so
0x4d will use the same buf data read by 0x4c, instead of reading it via
MCU command again.

So we need to preserve the values in the buf array, otherwise, it will
cause undefined behavior.

Thanks,
Shayne

> Regards,
> Lorenzo
> 
> >  
> >  			if (is_mt7996(&dev->mt76) && band ==
> > MT_BAND1 &&
> >  			    dev->var.type == MT7996_VAR_TYPE_444)
> >  				eep_offset -= MT_EE_7977BN_OFFSET;
> >  
> >  			if (prev_block_idx != block_idx) {
> > +				memset(buf, 0, sizeof(buf));
> >  				ret = mt7996_mcu_get_eeprom(dev,
> > adie_offset, buf,
> >  							   
> > MT7996_EEPROM_BLOCK_SIZE,
> >  							   
> > EEPROM_MODE_EFUSE);
> > -- 
> > 2.51.0
> >