Re: [PATCH 1/1] wifi: mt76: mt7996: fix mixed FEM check of mt7996 chipset

["Bjørn Mork" <bmork@telenor.net>]

Chukun Pan <amadeus@jmu.edu.cn> writes:

> To support the following varaint of the mt7996 chipset:
> - tri-band: band0 iFEM and band1/band2 eFEM
>
> We should read band1 for the determination, using band0
> will incorrectly identify it as iFEM.
> This is also the judgment logic of the mt7992 chipset.
>
> This fixes the load error on the Quantum Fiber W1701K:
> mt7996e 0000:01:00.0: eeprom load fail, use default bin
> mt7996e 0000:01:00.0: Message 0012002d (seq 5) timeout
>
> Fixes: e8cb33ad546a ("wifi: mt76: mt7996: add support for more variants")
> Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
> ---
>  drivers/net/wireless/mediatek/mt76/mt7996/init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/init.c b/drivers/net/wireless/mediatek/mt76/mt7996/init.c
> index 5e95a36b42d1..143400541746 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/init.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/init.c
> @@ -1137,7 +1137,7 @@ static int mt7996_variant_fem_init(struct mt7996_dev *dev)
>  		adie_idx = (val & MT_PAD_GPIO_ADIE_SINGLE) ? 0 : 1;
>  		adie_comb = u32_get_bits(val, MT_PAD_GPIO_ADIE_COMB_7992);
>  	} else {
> -		adie_idx = 0;
> +		adie_idx = 1;
>  		adie_comb = u32_get_bits(val, MT_PAD_GPIO_ADIE_COMB);
>  	}

Thanks for this.  It solves half the puzzle I have.  But unfortunately
it adds new questions too.

I have two closely related devices from Zyxel, where one is supposed to
be iFEM and the other eFEM.  Looking at the eeprom data provided by
Zyxel, it seems that the eFEM device is actually mixed like your
example.  And indeed - your patch makes that device work properly.

The problem is that the patch breaks the other device variant, which
used to work before.  If we assume the eeprom data from Zyxel is
correct, then this variant is iFEM.

I added some debug printk's to mt7996_variant_fem_init() so I could see
what detection data we have.  I read both MT_ADIE_CHIP_ID(0) and
MT_ADIE_CHIP_ID(1) to be able to compare logic input before and after
your patch.

The two devices are almost identical, except for BIT(0) of
MT_ADIE_CHIP_ID(1).

iFEM device, working before, broken after this patch:

 mt7996e 0000:01:00.0: MT_PAD_GPIO=0x00008400, adie_comb=1
 mt7996e 0000:01:00.0: MT_ADIE_CHIP_ID(0)=0x79758a02
 mt7996e 0000:01:00.0: MT_ADIE_CHIP_ID(adie_idx)=0x79778a11, adie_idx=1

Mixed iFEM/eFEM device, broken before, working after this patch:

 mt7996e 0000:01:00.0: MT_PAD_GPIO=0x00008400, adie_comb=1
 mt7996e 0000:01:00.0: MT_ADIE_CHIP_ID(0)=0x79758a02
 mt7996e 0000:01:00.0: MT_ADIE_CHIP_ID(adie_idx)=0x79778a10, adie_idx=1

Looking at mt7996_eeprom_variant_valid() I understand that the
MT_EE_WIFI_PA_LNA_CONFIG is given by the two lowest bits of bytes 0x196
and 0x197 for band0 and band1/band2 respectively. 0 for iFEM and 3 for
eFEM.

And the Zyxel provided eeprom on my iFEM device has indeed 0 in the
MT_EE_WIFI_PA_LNA_CONFIG field of both bytes:

 00000190  11 23 24 24 25 24 08 a0  e0 0c 08 08 00 00 00 38

While the mixed iFEM/eFEM device has 0 for band0 and 3 for band1/band2:

 00000190  11 23 24 24 25 24 08 0b  4b 0c 22 22 00 00 80 38


Any idea how this should be fixed?  Is there still something missing in
mt7996_variant_fem_init()? Is the ADIE chip id difference significant
here?  Or is there something wrong with the devices/eeproms I have?



Bjørn