The code contains no spaces around binary operators making it
hard to read. This also doesn't adhere to Linux kernel coding
style.
Add white spaces around the binary operators to increase readability
and endure adherence to Linux kernel coding styles.
Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com>
---
drivers/staging/rtl8723bs/core/rtw_xmit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c b/drivers/staging/rtl8723bs/core/rtw_xmit.c
index 297c93d65315..c41de6fd897b 100644
--- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
@@ -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) & 0xfff;
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) & 0xfff;
pattrib->ampdu_en = true;/* AGG EN */
}
}
--
2.34.1On Tue, Apr 8, 2025 at 12:54 AM Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> wrote: > > The code contains no spaces around binary operators making it > hard to read. This also doesn't adhere to Linux kernel coding > style. > > Add white spaces around the binary operators to increase readability > and endure adherence to Linux kernel coding styles. ... > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq+1)&0xfff; > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq + 1) & 0xfff; > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff; > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum + 1) & 0xfff; You mentioned Linux coding style, which also requires lines not to be so long. These lines are. That's why a few versions ago I suggested you to change these to be two lines each. I don't know how many times to repeat this (it's third one). -- With Best Regards, Andy Shevchenko
On Tue, Apr 8, 2025 at 8:20 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Tue, Apr 8, 2025 at 12:54 AM Abraham Samuel Adekunle > <abrahamadekunle50@gmail.com> wrote: > > > > The code contains no spaces around binary operators making it > > hard to read. This also doesn't adhere to Linux kernel coding > > style. > > > > Add white spaces around the binary operators to increase readability > > and endure adherence to Linux kernel coding styles. > > ... > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq+1)&0xfff; > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq + 1) & 0xfff; > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff; > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum + 1) & 0xfff; > > You mentioned Linux coding style, which also requires lines not to be > so long. These lines are. That's why a few versions ago I suggested > you to change these to be two lines each. I don't know how many times > to repeat this (it's third one). Okay, sorry I will add a third patch for a line break before the patch for % operations since each patch should handle a single thing. Thanks.
On Tue, Apr 08, 2025 at 10:22:44AM +0100, Samuel Abraham wrote: > On Tue, Apr 8, 2025 at 8:20 AM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Tue, Apr 8, 2025 at 12:54 AM Abraham Samuel Adekunle > > <abrahamadekunle50@gmail.com> wrote: ... > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq+1)&0xfff; > > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq + 1) & 0xfff; > > > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff; > > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum + 1) & 0xfff; > > > > You mentioned Linux coding style, which also requires lines not to be > > so long. These lines are. That's why a few versions ago I suggested > > you to change these to be two lines each. I don't know how many times > > to repeat this (it's third one). > > Okay, sorry > I will add a third patch for a line break before the patch for % > operations since each patch should handle a single thing. I am not sure you need a third patch for that. It lies into category of space and indentation fix. -- With Best Regards, Andy Shevchenko
On Tue, Apr 08, 2025 at 12:35:05PM +0300, Andy Shevchenko wrote: > On Tue, Apr 08, 2025 at 10:22:44AM +0100, Samuel Abraham wrote: > > On Tue, Apr 8, 2025 at 8:20 AM Andy Shevchenko > > <andy.shevchenko@gmail.com> wrote: > > > On Tue, Apr 8, 2025 at 12:54 AM Abraham Samuel Adekunle > > > <abrahamadekunle50@gmail.com> wrote: > > ... > > > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq+1)&0xfff; > > > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq + 1) & 0xfff; > > > > > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff; > > > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum + 1) & 0xfff; > > > > > > You mentioned Linux coding style, which also requires lines not to be > > > so long. These lines are. That's why a few versions ago I suggested > > > you to change these to be two lines each. I don't know how many times > > > to repeat this (it's third one). > > > > Okay, sorry > > I will add a third patch for a line break before the patch for % > > operations since each patch should handle a single thing. > > I am not sure you need a third patch for that. It lies into category of space > and indentation fix. > Yeah. Let's not go crazy. Do the white space change as one patch. The rules are there to make reviewing easier. Splitting it up into three patches doesn't help anyone. In staging we say, "Fix one type of checkpatch warning at a time." That's because it's a simple rule to explain and it stops people from sending us huge patches that fix every checkpatch warning. But this patch is small and everything is related and it's easy to review. regards, dan carpenter
On Tue, Apr 8, 2025 at 11:36 AM Dan Carpenter <dan.carpenter@linaro.org> wrote: > > On Tue, Apr 08, 2025 at 12:35:05PM +0300, Andy Shevchenko wrote: > > On Tue, Apr 08, 2025 at 10:22:44AM +0100, Samuel Abraham wrote: > > > On Tue, Apr 8, 2025 at 8:20 AM Andy Shevchenko > > > <andy.shevchenko@gmail.com> wrote: > > > > On Tue, Apr 8, 2025 at 12:54 AM Abraham Samuel Adekunle > > > > <abrahamadekunle50@gmail.com> wrote: > > > > ... > > > > > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq+1)&0xfff; > > > > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq + 1) & 0xfff; > > > > > > > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff; > > > > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum + 1) & 0xfff; > > > > > > > > You mentioned Linux coding style, which also requires lines not to be > > > > so long. These lines are. That's why a few versions ago I suggested > > > > you to change these to be two lines each. I don't know how many times > > > > to repeat this (it's third one). > > > > > > Okay, sorry > > > I will add a third patch for a line break before the patch for % > > > operations since each patch should handle a single thing. > > > > I am not sure you need a third patch for that. It lies into category of space > > and indentation fix. > > > > Yeah. Let's not go crazy. Do the white space change as one patch. The > rules are there to make reviewing easier. Splitting it up into three > patches doesn't help anyone. Okay thank you Dan. I have collapsed the spaces and linebreaks into one patch > > In staging we say, "Fix one type of checkpatch warning at a time." > That's because it's a simple rule to explain and it stops people from > sending us huge patches that fix every checkpatch warning. But this > patch is small and everything is related and it's easy to review. > Thank you very much for the clarity. I understand now. I already asked Andy, but I would also like to seek your opinion on how I should version the next patch. I already made this current one v6. Do I send v7 with changes in the cover letter, or changes in the individual patches? Or what is the best way please Thanks Adekunle.
On Tue, Apr 08, 2025 at 12:51:03PM +0100, Samuel Abraham wrote: > On Tue, Apr 8, 2025 at 11:36 AM Dan Carpenter <dan.carpenter@linaro.org> wrote: > > > > On Tue, Apr 08, 2025 at 12:35:05PM +0300, Andy Shevchenko wrote: > > > On Tue, Apr 08, 2025 at 10:22:44AM +0100, Samuel Abraham wrote: > > > > On Tue, Apr 8, 2025 at 8:20 AM Andy Shevchenko > > > > <andy.shevchenko@gmail.com> wrote: > > > > > On Tue, Apr 8, 2025 at 12:54 AM Abraham Samuel Adekunle > > > > > <abrahamadekunle50@gmail.com> wrote: > > > > > > ... > > > > > > > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq+1)&0xfff; > > > > > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq + 1) & 0xfff; > > > > > > > > > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff; > > > > > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum + 1) & 0xfff; > > > > > > > > > > You mentioned Linux coding style, which also requires lines not to be > > > > > so long. These lines are. That's why a few versions ago I suggested > > > > > you to change these to be two lines each. I don't know how many times > > > > > to repeat this (it's third one). > > > > > > > > Okay, sorry > > > > I will add a third patch for a line break before the patch for % > > > > operations since each patch should handle a single thing. > > > > > > I am not sure you need a third patch for that. It lies into category of space > > > and indentation fix. > > > > > > > Yeah. Let's not go crazy. Do the white space change as one patch. The > > rules are there to make reviewing easier. Splitting it up into three > > patches doesn't help anyone. > > Okay thank you Dan. I have collapsed the spaces and linebreaks into one patch > > > > > In staging we say, "Fix one type of checkpatch warning at a time." > > That's because it's a simple rule to explain and it stops people from > > sending us huge patches that fix every checkpatch warning. But this > > patch is small and everything is related and it's easy to review. > > > Thank you very much for the clarity. I understand now. > > I already asked Andy, but I would also like to seek your opinion on > how I should version > the next patch. I already made this current one v6. Do I send v7 with > changes in the cover letter, Yes. > or changes in the individual patches? Both? Put yourself in Greg's shoes and do whatever is easiest for Greg. regards, dan carpenter
On Tue, Apr 8, 2025 at 1:38 PM Dan Carpenter <dan.carpenter@linaro.org> wrote: > > On Tue, Apr 08, 2025 at 12:51:03PM +0100, Samuel Abraham wrote: > > On Tue, Apr 8, 2025 at 11:36 AM Dan Carpenter <dan.carpenter@linaro.org> wrote: > > > > > > On Tue, Apr 08, 2025 at 12:35:05PM +0300, Andy Shevchenko wrote: > > > > On Tue, Apr 08, 2025 at 10:22:44AM +0100, Samuel Abraham wrote: > > > > > On Tue, Apr 8, 2025 at 8:20 AM Andy Shevchenko > > > > > <andy.shevchenko@gmail.com> wrote: > > > > > > On Tue, Apr 8, 2025 at 12:54 AM Abraham Samuel Adekunle > > > > > > <abrahamadekunle50@gmail.com> wrote: > > > > > > > > ... > > > > > > > > > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq+1)&0xfff; > > > > > > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq + 1) & 0xfff; > > > > > > > > > > > > > - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff; > > > > > > > + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum + 1) & 0xfff; > > > > > > > > > > > > You mentioned Linux coding style, which also requires lines not to be > > > > > > so long. These lines are. That's why a few versions ago I suggested > > > > > > you to change these to be two lines each. I don't know how many times > > > > > > to repeat this (it's third one). > > > > > > > > > > Okay, sorry > > > > > I will add a third patch for a line break before the patch for % > > > > > operations since each patch should handle a single thing. > > > > > > > > I am not sure you need a third patch for that. It lies into category of space > > > > and indentation fix. > > > > > > > > > > Yeah. Let's not go crazy. Do the white space change as one patch. The > > > rules are there to make reviewing easier. Splitting it up into three > > > patches doesn't help anyone. > > > > Okay thank you Dan. I have collapsed the spaces and linebreaks into one patch > > > > > > > > In staging we say, "Fix one type of checkpatch warning at a time." > > > That's because it's a simple rule to explain and it stops people from > > > sending us huge patches that fix every checkpatch warning. But this > > > patch is small and everything is related and it's easy to review. > > > > > Thank you very much for the clarity. I understand now. > > > > I already asked Andy, but I would also like to seek your opinion on > > how I should version > > the next patch. I already made this current one v6. Do I send v7 with > > changes in the cover letter, > > Yes. > > > or changes in the individual patches? > > Both? Put yourself in Greg's shoes and do whatever is easiest for > Greg. Thank you very much, Dan. Adekunle.
© 2016 - 2025 Red Hat, Inc.