Re: [PATCH] mt76: fix sparse warnings: warning: dubious: x & !y

[Kalle Valo <kvalo@codeaurora.org>]

(I think you accidentally dropped linux-wireless, adding it back)

Lorenzo Bianconi <lorenzo@kernel.org> writes:

>> Lorenzo Bianconi <lorenzo@kernel.org> writes:
>> 
>> > Fix following sparse warnings in mt7603/mac.c and mt76x02_mac.c
>> >
>> > drivers/net/wireless/mediatek/mt76/mt76x02_mac.c:113:17: warning: dubious: x & !y
>> > drivers/net/wireless/mediatek/mt76/mt76x02_mac.c:145:16: warning: dubious: x & !y
>> > drivers/net/wireless/mediatek/mt76/mt7603/mac.c:730:9: warning: dubious: x & !y
>> > drivers/net/wireless/mediatek/mt76/mt7603/mac.c:790:15: warning: dubious: x & !y
>> >
>> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>> > ---
>> >  drivers/net/wireless/mediatek/mt76/mt7603/mac.c  | 6 ++++--
>> >  drivers/net/wireless/mediatek/mt76/mt76x02_mac.c | 7 +++++--
>> >  2 files changed, 9 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/mac.c b/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> > index ab5141701997..62e0a7f4716a 100644
>> > --- a/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> > +++ b/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> > @@ -709,6 +709,7 @@ int mt7603_wtbl_set_key(struct mt7603_dev *dev, int wcid,
>> >  {
>> >  	enum mt7603_cipher_type cipher;
>> >  	u32 addr = mt7603_wtbl3_addr(wcid);
>> > +	bool key_set = !!key;
>> >  	u8 key_data[32];
>> >  	int key_len = sizeof(key_data);
>> >  
>> > @@ -727,7 +728,7 @@ int mt7603_wtbl_set_key(struct mt7603_dev *dev, int wcid,
>> >  	mt76_rmw_field(dev, addr + 2 * 4, MT_WTBL1_W2_KEY_TYPE, cipher);
>> >  	if (key)
>> >  		mt76_rmw_field(dev, addr, MT_WTBL1_W0_KEY_IDX, key->keyidx);
>> > -	mt76_rmw_field(dev, addr, MT_WTBL1_W0_RX_KEY_VALID, !!key);
>> > +	mt76_rmw_field(dev, addr, MT_WTBL1_W0_RX_KEY_VALID, key_set);
>> 
>> I'm not seeing you really _fixing_ anything here, you are just working
>> around a sparse warning by adding an extra variable. I'm having a hard
>> time to see the benefit from that, it's just an unnecessary variable.
>
> Hi Kalle,
>
> right, they are just false positive, I posted this patch since they are
> annoying and sometimes sparse spots real bugs so I think it would be better to
> remove 'noise' sources.

Sure, sparse is very useful. I run it for every ath10k and ath11k patch
and it has found a lot of valid issues.

> Anyway we can drop this patch, I did not come up with a better
> solution :)

I was lazy and I added a filter to my ath11k-check to skip this warning.
It would be nice report this to sparse developers, as we already have
two drivers suffering of the same problem.

-- 
Kalle Valo