From: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com>
To: gregkh@linuxfoundation.org, julia.lawall@inria.fr,
outreachy@lists.linux.dev
Cc: linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
david.laight.linux@gmail.com, andy@kernel.org,
dan.carpenter@linaro.org
Subject: [PATCH v5] staging: rtl8723bs: Use % 4096u instead of & 0xfff
Date: Mon, 7 Apr 2025 15:54:02 +0000 [thread overview]
Message-ID: <Z/PyMXTjCHduOhKX@HP-650> (raw)
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 <andy@kernel.org>
Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com>
---
Changes in v4:
- Corrected patch to use '%' instead of '&'
- To ensure this change does not affect the functional
behaviour, I compared the generated object files before and
after the change using the `cmp` which compares the two
object files byte by byte as shown below:
$ make drivers/staging/rtl8723bs/core/rtw_xmit.o
$ cmp rtw_xmit_before.o rtw_xmit_after.o
No differences were found in the output, confirming that the
change does not alter the compiled output.
Changes in v3:
- Added more description to the commit message.
- Removed blank line in the tag block.
- Added more patch recipients.
Changes in v2:
- Changed the commit message t a more descriptive message which
makes it clear why the patch does the change.
- changed the subject title to include `4096u` to show that an
unsigned module is used.
Changes in v1:
- Added more patch recipients.
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 026061b464f7..548fcf9968bd 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
next reply other threads:[~2025-04-07 15:54 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-07 15:54 Abraham Samuel Adekunle [this message]
2025-04-07 16:16 ` [PATCH v5] staging: rtl8723bs: Use % 4096u instead of & 0xfff Andy Shevchenko
2025-04-07 20:58 ` Samuel Abraham
2025-04-09 14:06 ` Samuel Abraham
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z/PyMXTjCHduOhKX@HP-650 \
--to=abrahamadekunle50@gmail.com \
--cc=andy@kernel.org \
--cc=dan.carpenter@linaro.org \
--cc=david.laight.linux@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=julia.lawall@inria.fr \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=outreachy@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.