[PATCH v6 2/2] staging: rtl8723bs: Use % 4096u instead of & 0xfff
Abraham Samuel Adekunle <[email protected]> Mon, 7 Apr 2025 21:53:52 +0000
| Newsgroups | dev.linux.lists.outreachy,dev.linux.lists.linux-staging,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <27ff98a2d721a19cb09ea5c0d39c7d1616f8a629.1744061673.git.abrahamadekunle50@gmail.com> |
The sequence number is constrained to a range of [0, 4095], which is a total of 4096 values. The bitmask operation using `& 0xfff` is used to perform this wrap-around. While this is functionally correct, it obscures the intended semantic of a 4096-based wrap. Using a modulo operation `% 4096u` makes the wrap-around logic explicit and easier to understand. It clearly signals that the sequence number cycles through a range of 4096 values. It also makes the code robust against potential changes of the 4096 upper limit, especially when it becomes a non power of 2 value while the AND(&) works solely for power of 2 values. The use of `% 4096u` also guarantees that the modulo operation is performed with unsigned arithmetic, preventing potential issues with the signed types. Suggested-by: Andy Shevchenko <[email protected]> Suggested-by: David Laight <[email protected]> Signed-off-by: Abraham Samuel Adekunle <[email protected]> --- drivers/staging/rtl8723bs/core/rtw_xmit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c b/drivers/staging/rtl8723bs/core/rtw_xmit.c index c41de6fd897b..5cfb6d1e9dae 100644 --- a/drivers/staging/rtl8723bs/core/rtw_xmit.c +++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c @@ -943,7 +943,7 @@ s32 rtw_make_wlanhdr(struct adapter *padapter, u8 *hdr, struct pkt_attrib *pattr if (psta) { psta->sta_xmitpriv.txseq_tid[pattrib->priority]++; - psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF; + psta->sta_xmitpriv.txseq_tid[pattrib->priority] %= 4096u; pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority]; SetSeqNum(hdr, pattrib->seqnum); @@ -963,11 +963,11 @@ s32 rtw_make_wlanhdr(struct adapter *padapter, u8 *hdr, struct pkt_attrib *pattr if (SN_LESS(pattrib->seqnum, tx_seq)) { pattrib->ampdu_en = false;/* AGG BK */ } else if (SN_EQUAL(pattrib->seqnum, tx_seq)) { - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq + 1) & 0xfff; + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq + 1) % 4096u; pattrib->ampdu_en = true;/* AGG EN */ } else { - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum + 1) & 0xfff; + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum + 1) % 4096u; pattrib->ampdu_en = true;/* AGG EN */ } } -- 2.34.1