From: Erick Karanja <karanja99erick@gmail.com>
To: Julia Lawall <julia.lawall@inria.fr>
Cc: gregkh@linuxfoundation.org, outreachy@lists.linux.dev,
philipp.g.hortmann@gmail.com, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] staging: rtl8723bs: Improve code readability
Date: Wed, 09 Apr 2025 14:32:36 +0300 [thread overview]
Message-ID: <4cf642ebb8d0859676230bae06a4914a57428ec3.camel@gmail.com> (raw)
In-Reply-To: <3dda7f97-4ed8-cf6d-9310-60ed6259a386@inria.fr>
On Wed, 2025-04-09 at 07:04 -0400, Julia Lawall wrote:
>
>
> On Wed, 9 Apr 2025, Erick Karanja wrote:
>
> > Make the code more readable by moving trivial
> > initializations up with the declarations instead
> > of wasting a line on that.
> >
> > Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> > ---
> > .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 84 ++++++---------
> > ----
> > 1 file changed, 25 insertions(+), 59 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > index e15ec6452fd0..2cf2c66140f1 100644
> > --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > @@ -501,8 +501,7 @@ void Hal_GetEfuseDefinition(
> > switch (type) {
> > case TYPE_EFUSE_MAX_SECTION:
> > {
> > - u8 *pMax_section;
> > - pMax_section = pOut;
> > + u8 *pMax_section = pOut;
> >
> > if (efuseType == EFUSE_WIFI)
> > *pMax_section =
> > EFUSE_MAX_SECTION_8723B;
> > @@ -513,8 +512,7 @@ void Hal_GetEfuseDefinition(
> >
> > case TYPE_EFUSE_REAL_CONTENT_LEN:
> > {
> > - u16 *pu2Tmp;
> > - pu2Tmp = pOut;
> > + u16 *pu2Tmp = pOut;
> >
> > if (efuseType == EFUSE_WIFI)
> > *pu2Tmp =
> > EFUSE_REAL_CONTENT_LEN_8723B;
> > @@ -525,8 +523,7 @@ void Hal_GetEfuseDefinition(
> >
> > case TYPE_AVAILABLE_EFUSE_BYTES_BANK:
> > {
> > - u16 *pu2Tmp;
> > - pu2Tmp = pOut;
> > + u16 *pu2Tmp = pOut;
> >
> > if (efuseType == EFUSE_WIFI)
> > *pu2Tmp =
> > (EFUSE_REAL_CONTENT_LEN_8723B-EFUSE_OOB_PROTECT_BYTES);
> > @@ -537,8 +534,7 @@ void Hal_GetEfuseDefinition(
> >
> > case TYPE_AVAILABLE_EFUSE_BYTES_TOTAL:
> > {
> > - u16 *pu2Tmp;
> > - pu2Tmp = pOut;
> > + u16 *pu2Tmp = pOut;
> >
> > if (efuseType == EFUSE_WIFI)
> > *pu2Tmp =
> > (EFUSE_REAL_CONTENT_LEN_8723B-EFUSE_OOB_PROTECT_BYTES);
> > @@ -549,8 +545,7 @@ void Hal_GetEfuseDefinition(
> >
> > case TYPE_EFUSE_MAP_LEN:
> > {
> > - u16 *pu2Tmp;
> > - pu2Tmp = pOut;
> > + u16 *pu2Tmp = pOut;
> >
> > if (efuseType == EFUSE_WIFI)
> > *pu2Tmp = EFUSE_MAX_MAP_LEN;
> > @@ -561,8 +556,7 @@ void Hal_GetEfuseDefinition(
> >
> > case TYPE_EFUSE_PROTECT_BYTES_BANK:
> > {
> > - u8 *pu1Tmp;
> > - pu1Tmp = pOut;
> > + u8 *pu1Tmp = pOut;
> >
> > if (efuseType == EFUSE_WIFI)
> > *pu1Tmp = EFUSE_OOB_PROTECT_BYTES;
> > @@ -573,8 +567,7 @@ void Hal_GetEfuseDefinition(
> >
> > case TYPE_EFUSE_CONTENT_LEN_BANK:
> > {
> > - u16 *pu2Tmp;
> > - pu2Tmp = pOut;
> > + u16 *pu2Tmp = pOut;
> >
> > if (efuseType == EFUSE_WIFI)
> > *pu2Tmp =
> > EFUSE_REAL_CONTENT_LEN_8723B;
> > @@ -585,8 +578,7 @@ void Hal_GetEfuseDefinition(
> >
> > default:
> > {
> > - u8 *pu1Tmp;
> > - pu1Tmp = pOut;
> > + u8 *pu1Tmp = pOut;
> > *pu1Tmp = 0;
> > }
> > break;
> > @@ -729,10 +721,9 @@ static void hal_ReadEFuse_WiFi(
> > }
> >
> > if (offset < EFUSE_MAX_SECTION_8723B) {
> > - u16 addr;
> > + u16 addr = offset * PGPKT_DATA_SIZE;
> > /* Get word enable value from PG header
> > */
> >
> > - addr = offset * PGPKT_DATA_SIZE;
>
> I'm not sure about this one, due to the comment.
Hello Julia, I have observed in some scenarios where the semantic patch
deletes the comments that exist between the declaration and
initialization which I think it's not okay and in this scenario where
the comments are left dangling such as this. How do I go about this
instances.
Thank you.
>
> julia
>
> > for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++)
> > {
> > /* Check word enable condition in
> > the section */
> > if (!(wden & (0x01<<i))) {
> > @@ -835,9 +826,8 @@ static void hal_ReadEFuse_BT(
> > }
> >
> > if (offset < EFUSE_BT_MAX_SECTION) {
> > - u16 addr;
> > + u16 addr = offset *
> > PGPKT_DATA_SIZE;
> >
> > - addr = offset * PGPKT_DATA_SIZE;
> > for (i = 0; i <
> > EFUSE_MAX_WORD_UNIT; i++) {
> > /* Check word enable
> > condition in the section */
> > if (!(wden & (0x01<<i))) {
> > @@ -1196,10 +1186,9 @@ void rtl8723b_InitBeaconParameters(struct
> > adapter *padapter)
> > {
> > struct hal_com_data *pHalData = GET_HAL_DATA(padapter);
> > u16 val16;
> > - u8 val8;
> > + u8 val8 = DIS_TSF_UDT;
> >
> >
> > - val8 = DIS_TSF_UDT;
> > val16 = val8 | (val8 << 8); /* port0 and port1 */
> >
> > /* Enable prot0 beacon function for PSTDMA */
> > @@ -1496,10 +1485,7 @@ s32 rtl8723b_InitLLTTable(struct adapter
> > *padapter)
> > {
> > unsigned long start, passing_time;
> > u32 val32;
> > - s32 ret;
> > -
> > -
> > - ret = _FAIL;
> > + s32 ret = _FAIL;
> >
> > val32 = rtw_read32(padapter, REG_AUTO_LLT);
> > val32 |= BIT_AUTO_INIT_LLT;
> > @@ -2273,9 +2259,8 @@ void rtl8723b_fill_fake_txdesc(
> > /* Encrypt the data frame if under security mode excepct
> > null data. Suggested by CCW. */
> > /* */
> > if (bDataFrame) {
> > - u32 EncAlg;
> > + u32 EncAlg = padapter-
> > >securitypriv.dot11PrivacyAlgrthm;
> >
> > - EncAlg = padapter-
> > >securitypriv.dot11PrivacyAlgrthm;
> > switch (EncAlg) {
> > case _NO_PRIVACY_:
> > SET_TX_DESC_SEC_TYPE_8723B(pDesc, 0x0);
> > @@ -2378,9 +2363,7 @@ static void hw_var_set_opmode(struct adapter
> > *padapter, u8 variable, u8 *val)
> > static void hw_var_set_macaddr(struct adapter *padapter, u8
> > variable, u8 *val)
> > {
> > u8 idx = 0;
> > - u32 reg_macid;
> > -
> > - reg_macid = REG_MACID;
> > + u32 reg_macid = REG_MACID;
> >
> > for (idx = 0 ; idx < 6; idx++)
> > rtw_write8(GET_PRIMARY_ADAPTER(padapter),
> > (reg_macid+idx), val[idx]);
> > @@ -2389,9 +2372,7 @@ static void hw_var_set_macaddr(struct adapter
> > *padapter, u8 variable, u8 *val)
> > static void hw_var_set_bssid(struct adapter *padapter, u8
> > variable, u8 *val)
> > {
> > u8 idx = 0;
> > - u32 reg_bssid;
> > -
> > - reg_bssid = REG_BSSID;
> > + u32 reg_bssid = REG_BSSID;
> >
> > for (idx = 0 ; idx < 6; idx++)
> > rtw_write8(padapter, (reg_bssid+idx), val[idx]);
> > @@ -2399,9 +2380,7 @@ static void hw_var_set_bssid(struct adapter
> > *padapter, u8 variable, u8 *val)
> >
> > static void hw_var_set_bcn_func(struct adapter *padapter, u8
> > variable, u8 *val)
> > {
> > - u32 bcn_ctrl_reg;
> > -
> > - bcn_ctrl_reg = REG_BCN_CTRL;
> > + u32 bcn_ctrl_reg = REG_BCN_CTRL;
> >
> > if (*(u8 *)val)
> > rtw_write8(padapter, bcn_ctrl_reg,
> > (EN_BCN_FUNCTION | EN_TXBCN_RPT));
> > @@ -2422,12 +2401,8 @@ static void hw_var_set_correct_tsf(struct
> > adapter *padapter, u8 variable, u8 *va
> > {
> > u8 val8;
> > u64 tsf;
> > - struct mlme_ext_priv *pmlmeext;
> > - struct mlme_ext_info *pmlmeinfo;
> > -
> > -
> > - pmlmeext = &padapter->mlmeextpriv;
> > - pmlmeinfo = &pmlmeext->mlmext_info;
> > + struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
> > + struct mlme_ext_info *pmlmeinfo = &pmlmeext->mlmext_info;
> >
> > tsf = pmlmeext->TSFValue-do_div(pmlmeext->TSFValue,
> > (pmlmeinfo->bcn_interval*1024))-1024; /* us */
> >
> > @@ -2543,15 +2518,12 @@ static void hw_var_set_mlme_join(struct
> > adapter *padapter, u8 variable, u8 *val)
> > u8 val8;
> > u16 val16;
> > u32 val32;
> > - u8 RetryLimit;
> > - u8 type;
> > - struct mlme_priv *pmlmepriv;
> > + u8 RetryLimit = 0x30;
> > + u8 type = *(u8 *)val;
> > + struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> > struct eeprom_priv *pEEPROM;
> >
> >
> > - RetryLimit = 0x30;
> > - type = *(u8 *)val;
> > - pmlmepriv = &padapter->mlmepriv;
> > pEEPROM = GET_EEPROM_EFUSE_PRIV(padapter);
> >
> > if (type == 0) { /* prepare to join */
> > @@ -2850,12 +2822,11 @@ void SetHwReg8723B(struct adapter
> > *padapter, u8 variable, u8 *val)
> >
> > case HW_VAR_ACK_PREAMBLE:
> > {
> > - u8 regTmp;
> > + u8 regTmp = 0;
> > u8 bShortPreamble = *val;
> >
> > /* Joseph marked out for Netgear 3500
> > TKIP channel 7 issue.(Temporarily) */
> > /* regTmp = (pHalData-
> > >nCur40MhzPrimeSC)<<5; */
> > - regTmp = 0;
> > if (bShortPreamble)
> > regTmp |= 0x80;
> > rtw_write8(padapter, REG_RRSR+2, regTmp);
> > @@ -3226,9 +3197,7 @@ void GetHwReg8723B(struct adapter *padapter,
> > u8 variable, u8 *val)
> > */
> > u8 SetHalDefVar8723B(struct adapter *padapter, enum
> > hal_def_variable variable, void *pval)
> > {
> > - u8 bResult;
> > -
> > - bResult = _SUCCESS;
> > + u8 bResult = _SUCCESS;
> >
> > switch (variable) {
> > default:
> > @@ -3244,9 +3213,7 @@ u8 SetHalDefVar8723B(struct adapter
> > *padapter, enum hal_def_variable variable, v
> > */
> > u8 GetHalDefVar8723B(struct adapter *padapter, enum
> > hal_def_variable variable, void *pval)
> > {
> > - u8 bResult;
> > -
> > - bResult = _SUCCESS;
> > + u8 bResult = _SUCCESS;
> >
> > switch (variable) {
> > case HAL_DEF_MAX_RECVBUF_SZ:
> > @@ -3281,9 +3248,8 @@ u8 GetHalDefVar8723B(struct adapter
> > *padapter, enum hal_def_variable variable, v
> > case HW_DEF_RA_INFO_DUMP:
> > {
> > u8 mac_id = *(u8 *)pval;
> > - u32 cmd;
> > + u32 cmd = 0x40000100 | mac_id;
> >
> > - cmd = 0x40000100 | mac_id;
> > rtw_write32(padapter,
> > REG_HMEBOX_DBG_2_8723B, cmd);
> > msleep(10);
> > rtw_read32(padapter, 0x2F0); // info 1
> > --
> > 2.43.0
> >
> >
> >
next prev parent reply other threads:[~2025-04-09 11:32 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-09 10:04 [PATCH v2 0/2] Improve code readability in rtl8723bs module Erick Karanja
2025-04-09 10:04 ` [PATCH v2 1/2] staging: rtl8723bs: Improve code readability Erick Karanja
2025-04-09 11:04 ` Julia Lawall
2025-04-09 11:11 ` Dan Carpenter
2025-04-09 11:22 ` Erick Karanja
2025-04-09 11:32 ` Erick Karanja [this message]
2025-04-09 15:39 ` Julia Lawall
2025-04-09 10:04 ` [PATCH v2 2/2] staging: rtl8723bs: Initializing variables at declaration Erick Karanja
2025-04-09 15:41 ` Julia Lawall
2025-04-10 5:39 ` Erick Karanja
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=4cf642ebb8d0859676230bae06a4914a57428ec3.camel@gmail.com \
--to=karanja99erick@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 \
--cc=philipp.g.hortmann@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).