| Message ID | d8a003eda05150fb21842d7755fe8081b86cf6df.1560851052.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | mt76: fix sparse warnings: warning: dubious: x & !y | expand |
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.
(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.
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);
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(-)