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

Linux wireless drivers development
 help / color / mirror / Atom feed

* [PATCH] mt76: fix sparse warnings: warning: dubious: x & !y
@ 2019-06-18 10:02 Lorenzo Bianconi
  2019-06-18 12:08 ` Kalle Valo
  0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Bianconi @ 2019-06-18 10:02 UTC (permalink / raw)
  To: nbd; +Cc: lorenzo.bianconi, linux-wireless

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);
 
 	return 0;
 }
@@ -745,6 +746,7 @@ mt7603_mac_write_txwi(struct mt7603_dev *dev, __le32 *txwi,
 	struct ieee80211_vif *vif = info->control.vif;
 	struct mt76_queue *q = dev->mt76.q_tx[qid].q;
 	struct mt7603_vif *mvif;
+	bool key_set = !!key;
 	int wlan_idx;
 	int hdr_len = ieee80211_get_hdrlen_from_skb(skb);
 	int tx_count = 8;
@@ -787,7 +789,7 @@ mt7603_mac_write_txwi(struct mt7603_dev *dev, __le32 *txwi,
 	      FIELD_PREP(MT_TXD1_HDR_FORMAT, MT_HDR_FORMAT_802_11) |
 	      FIELD_PREP(MT_TXD1_HDR_INFO, hdr_len / 2) |
 	      FIELD_PREP(MT_TXD1_WLAN_IDX, wlan_idx) |
-	      FIELD_PREP(MT_TXD1_PROTECTED, !!key);
+	      FIELD_PREP(MT_TXD1_PROTECTED, key_set);
 	txwi[1] = cpu_to_le32(val);
 
 	if (info->flags & IEEE80211_TX_CTL_NO_ACK)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
index 97621dbfd114..b3a35911deb1 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
@@ -110,8 +110,10 @@ int mt76x02_mac_wcid_set_key(struct mt76x02_dev *dev, u8 idx,
 
 	memset(iv_data, 0, sizeof(iv_data));
 	if (key) {
+		bool pw_key = !!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE);
+
 		mt76_rmw_field(dev, MT_WCID_ATTR(idx), MT_WCID_ATTR_PAIRWISE,
-			       !!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE));
+			       pw_key);
 
 		pn = atomic64_read(&key->tx_pn);
 
@@ -139,10 +141,11 @@ void mt76x02_mac_wcid_setup(struct mt76x02_dev *dev, u8 idx,
 			    u8 vif_idx, u8 *mac)
 {
 	struct mt76_wcid_addr addr = {};
+	bool ext_id = !!(vif_idx & 8);
 	u32 attr;
 
 	attr = FIELD_PREP(MT_WCID_ATTR_BSS_IDX, vif_idx & 7) |
-	       FIELD_PREP(MT_WCID_ATTR_BSS_IDX_EXT, !!(vif_idx & 8));
+	       FIELD_PREP(MT_WCID_ATTR_BSS_IDX_EXT, ext_id);
 
 	mt76_wr(dev, MT_WCID_ATTR(idx), attr);
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] mt76: fix sparse warnings: warning: dubious: x & !y
  2019-06-18 10:02 [PATCH] mt76: fix sparse warnings: warning: dubious: x & !y Lorenzo Bianconi
@ 2019-06-18 12:08 ` Kalle Valo
       [not found]   ` <20190618121641.GA20044@localhost.localdomain>
  0 siblings, 1 reply; 3+ messages in thread
From: Kalle Valo @ 2019-06-18 12:08 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: nbd, lorenzo.bianconi, linux-wireless

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.

FWIW I had similar warnings in ath11k, I decided to ignore those. But
anyone has suggestions how to solve it better, please do let me know.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 3+ messages in thread

[parent not found: <20190618121641.GA20044@localhost.localdomain>]

* Re: [PATCH] mt76: fix sparse warnings: warning: dubious: x & !y
       [not found]   ` <20190618121641.GA20044@localhost.localdomain>
@ 2019-06-18 12:36     ` Kalle Valo
  0 siblings, 0 replies; 3+ messages in thread
From: Kalle Valo @ 2019-06-18 12:36 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: linux-wireless

(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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-06-18 12:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-18 10:02 [PATCH] mt76: fix sparse warnings: warning: dubious: x & !y Lorenzo Bianconi
2019-06-18 12:08 ` Kalle Valo
     [not found]   ` <20190618121641.GA20044@localhost.localdomain>
2019-06-18 12:36     ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox