[PATCH 0/5] staging: r8723au: fix multiple checkpatch issues

[PATCH 0/5] staging: r8723au: fix multiple checkpatch issues

[Alison Schofield]

This patchset addresses the last 26 checkpatch WARNINGs in file:
drivers/staging/rtl8723au/core/rtw_cmd.c

I'll call your attention to the "Line over 80 chars" cases:

I chose to address 'Line over 80 chars' because all but one could be
resolved by cleaning up a single struct definition. [PATCH 1/5]

The last one appeared to be due to excessively long function names
as opposed to deep indentation/complex code requiring refactoring.
See my notes below the --- in PATCH[5/5] for further info.

alison


Alison Schofield (5):
  staging: r8723au: break parameter list at separators and align to open
    braces
  staging: r8723au: replace printk() with pr_err()
  staging: r8723au: use kernel preferred style for block comments
  staging: r8723au: move constant to right of comparison test
  staging: r8723au: add & use local variable to simplify references

 drivers/staging/rtl8723au/core/rtw_cmd.c | 154 +++++++++++++++++++------------
 1 file changed, 93 insertions(+), 61 deletions(-)

-- 
2.1.4

[PATCH 1/5] staging: r8723au: break parameter list at separators and align to open braces

[Alison Schofield]

Improve readability by aligning the struct cmd_hdl wlancmds[] to fit
within 80 chars and cleaning up the adjacent index comments.

checkpatch.pl: WARNING: line over 80 characters

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_cmd.c | 64 +++++++++++++++++++-------------
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 46aea16..c3579ac 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -22,7 +22,7 @@
 #include <rtw_sreset.h>
 
 static struct cmd_hdl wlancmds[] = {
-	GEN_DRV_CMD_HANDLER(0, NULL) /*0*/
+	GEN_DRV_CMD_HANDLER(0, NULL)					/*0*/
 	GEN_DRV_CMD_HANDLER(0, NULL)
 	GEN_DRV_CMD_HANDLER(0, NULL)
 	GEN_DRV_CMD_HANDLER(0, NULL)
@@ -32,18 +32,26 @@ static struct cmd_hdl wlancmds[] = {
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(0, NULL) /*10*/
+	GEN_MLME_EXT_HANDLER(0, NULL)					/*10*/
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), join_cmd_hdl23a) /*14*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm), disconnect_hdl23a)
-	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), createbss_hdl23a)
-	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm), setopmode_hdl23a)
-	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm), sitesurvey_cmd_hdl23a) /*18*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm), setauth_hdl23a)
-	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm), setkey_hdl23a) /*20*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm), set_stakey_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
+			     join_cmd_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm),
+			     disconnect_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
+			     createbss_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm),
+			     setopmode_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm),
+			     sitesurvey_cmd_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm),
+			     setauth_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm),		/*20*/
+			     setkey_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm),
+			     set_stakey_hdl23a)
 	GEN_MLME_EXT_HANDLER(sizeof(struct set_assocsta_parm), NULL)
 	GEN_MLME_EXT_HANDLER(sizeof(struct del_assocsta_parm), NULL)
 	GEN_MLME_EXT_HANDLER(sizeof(struct setstapwrstate_parm), NULL)
@@ -52,7 +60,7 @@ static struct cmd_hdl wlancmds[] = {
 	GEN_MLME_EXT_HANDLER(sizeof(struct setdatarate_parm), NULL)
 	GEN_MLME_EXT_HANDLER(sizeof(struct getdatarate_parm), NULL)
 	GEN_MLME_EXT_HANDLER(sizeof(struct setphyinfo_parm), NULL)
-	GEN_MLME_EXT_HANDLER(sizeof(struct getphyinfo_parm), NULL)  /*30*/
+	GEN_MLME_EXT_HANDLER(sizeof(struct getphyinfo_parm), NULL)	/*30*/
 	GEN_MLME_EXT_HANDLER(sizeof(struct setphy_parm), NULL)
 	GEN_MLME_EXT_HANDLER(sizeof(struct getphy_parm), NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
@@ -62,32 +70,36 @@ static struct cmd_hdl wlancmds[] = {
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(0, NULL)	/*40*/
+	GEN_MLME_EXT_HANDLER(0, NULL)					/*40*/
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(sizeof(struct addBaReq_parm), add_ba_hdl23a)
-	GEN_MLME_EXT_HANDLER(sizeof(struct set_ch_parm), set_ch_hdl23a) /* 46 */
+	GEN_MLME_EXT_HANDLER(sizeof(struct addBaReq_parm),
+			     add_ba_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct set_ch_parm),
+			     set_ch_hdl23a)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(0, NULL) /*50*/
+	GEN_MLME_EXT_HANDLER(0, NULL)					/*50*/
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(sizeof(struct Tx_Beacon_param), tx_beacon_hdl23a) /*55*/
-
-	GEN_MLME_EXT_HANDLER(0, mlme_evt_hdl23a) /*56*/
-	GEN_MLME_EXT_HANDLER(0, rtw_drvextra_cmd_hdl23a) /*57*/
-
-	GEN_MLME_EXT_HANDLER(0, h2c_msg_hdl23a) /*58*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct SetChannelPlan_param), set_chplan_hdl23a) /*59*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct LedBlink_param), led_blink_hdl23a) /*60*/
-
-	GEN_MLME_EXT_HANDLER(sizeof(struct SetChannelSwitch_param), set_csa_hdl23a) /*61*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct TDLSoption_param), tdls_hdl23a) /*62*/
+	GEN_MLME_EXT_HANDLER(sizeof(struct Tx_Beacon_param),
+			     tx_beacon_hdl23a)
+	GEN_MLME_EXT_HANDLER(0, mlme_evt_hdl23a)
+	GEN_MLME_EXT_HANDLER(0, rtw_drvextra_cmd_hdl23a)
+	GEN_MLME_EXT_HANDLER(0, h2c_msg_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct SetChannelPlan_param),
+			     set_chplan_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct LedBlink_param),		/*60*/
+			     led_blink_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct SetChannelSwitch_param),
+			     set_csa_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct TDLSoption_param),
+			     tdls_hdl23a)
 };
 
 struct _cmd_callback	rtw_cmd_callback[] = {
-- 
2.1.4

[PATCH 2/5] staging: r8723au: replace printk() with pr_err()

[Alison Schofield]

Replace printk() with pr_err() for uniform error reporting.
Issue found by checkpatch.

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index c3579ac..7bf0f30 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -248,7 +248,7 @@ int rtw_enqueue_cmd23a(struct cmd_priv *pcmdpriv, struct cmd_obj *cmd_obj)
 	res = queue_work(pcmdpriv->wq, &cmd_obj->work);
 
 	if (!res) {
-		printk(KERN_ERR "%s: Call to queue_work() failed\n", __func__);
+		pr_err("%s: Call to queue_work() failed\n", __func__);
 		res = _FAIL;
 	} else
 		res = _SUCCESS;
-- 
2.1.4

[PATCH 3/5] staging: r8723au: use kernel preferred style for block comments

[Alison Schofield]

Reworked comments per kernel coding style.

Resolves checkpatch:
WARNING: Block comments use * on subsequent lines

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_cmd.c | 74 ++++++++++++++++++++------------
 1 file changed, 46 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 7bf0f30..33164d3 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -176,9 +176,9 @@ struct _cmd_callback	rtw_cmd_callback[] = {
 };
 
 /*
-Caller and the rtw_cmd_thread23a can protect cmd_q by spin_lock.
-No irqsave is necessary.
-*/
+ * Caller and the rtw_cmd_thread23a can protect cmd_q by spin_lock.
+ * No irqsave is necessary.
+ */
 
 int rtw_init_cmd_priv23a(struct cmd_priv *pcmdpriv)
 {
@@ -315,8 +315,10 @@ post_process:
 				 pcmd_callback, pcmd->cmdcode);
 			rtw_free_cmd_obj23a(pcmd);
 		} else {
-			/* need consider that free cmd_obj in
-			   rtw_cmd_callback */
+			/*
+			 * need consider that free cmd_obj in
+			 * rtw_cmd_callback
+			 */
 			pcmd_callback(pcmd->padapter, pcmd);
 		}
 	} else {
@@ -518,11 +520,13 @@ int rtw_joinbss_cmd23a(struct rtw_adapter *padapter,
 	       get_wlan_bssid_ex_sz(&pnetwork->network));
 
 	psecnetwork->IELength = 0;
-	/*  Added by Albert 2009/02/18 */
-	/*  If the the driver wants to use the bssid to create the
+
+	/*
+	 *  If the the driver wants to use the bssid to create the
 	 *  connection. If not,  we have to copy the connecting AP's
 	 *  MAC address to it so that the driver just has the bssid
-	 *  information for PMKIDList searching. */
+	 *  information for PMKIDList searching.
+	 */
 
 	if (pmlmepriv->assoc_by_bssid == false)
 		ether_addr_copy(&pmlmepriv->assoc_bssid[0],
@@ -557,10 +561,13 @@ int rtw_joinbss_cmd23a(struct rtw_adapter *padapter,
 	phtpriv->ht_option = false;
 	if (pregistrypriv->ht_enable) {
 		u32 algo = padapter->securitypriv.dot11PrivacyAlgrthm;
-		/*	Added by Albert 2010/06/23 */
-		/*	For the WEP mode, we will use the bg mode to do
-			the connection to avoid some IOT issue. */
-		/*	Especially for Realtek 8192u SoftAP. */
+
+		/*
+		 * For the WEP mode, we will use the bg mode to do
+		 * the connection to avoid some IOT issue.
+		 * Especially for Realtek 8192u SoftAP.
+		 */
+
 		if (algo != WLAN_CIPHER_SUITE_WEP40 &&
 		    algo != WLAN_CIPHER_SUITE_WEP104 &&
 		    algo != WLAN_CIPHER_SUITE_TKIP) {
@@ -630,9 +637,13 @@ int rtw_disassoc_cmd23a(struct rtw_adapter *padapter, u32 deauth_timeout_ms,
 		init_h2fwcmd_w_parm_no_rsp(cmdobj, param, _DisConnect_CMD_);
 		res = rtw_enqueue_cmd23a(cmdpriv, cmdobj);
 	} else {
-		/* no need to enqueue, do the cmd hdl directly and
-		   free cmd parameter */
 		if (H2C_SUCCESS != disconnect_hdl23a(padapter, (u8 *)param))
+
+		/*
+		 * no need to enqueue, do the cmd hdl
+		 * directly and free cmd parameter
+		 */
+
 			res = _FAIL;
 		kfree(param);
 	}
@@ -867,16 +878,18 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
 	int BusyThreshold = 100;
 	struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo;
 
-	/*  */
 	/*  Determine if our traffic is busy now */
-	/*  */
 	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
 		if (rtl8723a_BT_coexist(padapter))
 			BusyThreshold = 50;
 		else if (ldi->bBusyTraffic)
 			BusyThreshold = 75;
-		/*  if we raise bBusyTraffic in last watchdog, using
-		    lower threshold. */
+
+		/*
+		 * if we raise bBusyTraffic in last watchdog,
+		 * using lower threshold.
+		 */
+
 		if (ldi->NumRxOkInPeriod > BusyThreshold ||
 		    ldi->NumTxOkInPeriod > BusyThreshold) {
 			bBusyTraffic = true;
@@ -947,9 +960,7 @@ static void dynamic_chk_wk_hdl(struct rtw_adapter *padapter, u8 *pbuf, int sz)
 
 	rtl8723a_HalDmWatchDog(padapter);
 
-	/*  */
 	/*  BT-Coexist */
-	/*  */
 	rtl8723a_BT_do_coexist(padapter);
 }
 
@@ -1234,8 +1245,8 @@ void rtw_evt_work(struct work_struct *work)
 	} else {
 		/*
 		 * Enqueue into cmd_thread for others.
-		 * ework will be turned into a c2h_evt and freed once it
-		 * has been consumed.
+		 * ework will be turned into a c2h_evt and
+		 * freed once it has been consumed.
 		 */
 		rtw_c2h_wk_cmd23a(adapter, (u8 *)&ework->u.c2h_evt);
 	}
@@ -1402,11 +1413,16 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
 		memcpy(&pwlan->network, pnetwork, pnetwork->Length);
 		/* pwlan->fixed = true; */
 
-		/* list_add_tail(&pwlan->list,
-		   &pmlmepriv->scanned_queue.queue); */
+		/*
+		 * list_add_tail(&pwlan->list,
+		 * &pmlmepriv->scanned_queue->queue);
+		 */
+
+		/*
+		 *  copy pdev_network information to
+		 *  pmlmepriv->cur_network
+		 */
 
-		/*  copy pdev_network information to
-		    pmlmepriv->cur_network */
 		memcpy(&tgt_network->network, pnetwork,
 		       get_wlan_bssid_ex_sz(pnetwork));
 
@@ -1415,8 +1431,10 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
 		clr_fwstate(pmlmepriv, _FW_UNDER_LINKING);
 
 		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
-		/*  we will set _FW_LINKED when there is one more sat to
-		    join us (rtw_stassoc_event_callback23a) */
+		/*
+		 * we will set _FW_LINKED when there is one more
+		 * sat to join us (rtw_stassoc_event_callback23a)
+		 */
 	}
 
 createbss_cmd_fail:
-- 
2.1.4

[PATCH 4/5] staging: r8723au: move constant to right of comparison test

[Alison Schofield]

Move constant to right of comparison test to improve readability.

checkpatch.pl:
	WARNING: Comparisons should place the constant on the
	right side of the test

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 33164d3..6034428 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -637,13 +637,13 @@ int rtw_disassoc_cmd23a(struct rtw_adapter *padapter, u32 deauth_timeout_ms,
 		init_h2fwcmd_w_parm_no_rsp(cmdobj, param, _DisConnect_CMD_);
 		res = rtw_enqueue_cmd23a(cmdpriv, cmdobj);
 	} else {
-		if (H2C_SUCCESS != disconnect_hdl23a(padapter, (u8 *)param))
 
 		/*
 		 * no need to enqueue, do the cmd hdl
 		 * directly and free cmd parameter
 		 */
 
+		if (disconnect_hdl23a(padapter, (u8 *)param) != H2C_SUCCESS)
 			res = _FAIL;
 		kfree(param);
 	}
-- 
2.1.4

Re: [Outreachy kernel] [PATCH 4/5] staging: r8723au: move constant to right of comparison test

[Julia Lawall]

On Mon, 19 Oct 2015, Alison Schofield wrote:

> Move constant to right of comparison test to improve readability.

Was it intentional to also move it below the comment?

julia

> 
> checkpatch.pl:
> 	WARNING: Comparisons should place the constant on the
> 	right side of the test
> 
> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> ---
>  drivers/staging/rtl8723au/core/rtw_cmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> index 33164d3..6034428 100644
> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> @@ -637,13 +637,13 @@ int rtw_disassoc_cmd23a(struct rtw_adapter *padapter, u32 deauth_timeout_ms,
>  		init_h2fwcmd_w_parm_no_rsp(cmdobj, param, _DisConnect_CMD_);
>  		res = rtw_enqueue_cmd23a(cmdpriv, cmdobj);
>  	} else {
> -		if (H2C_SUCCESS != disconnect_hdl23a(padapter, (u8 *)param))
>  
>  		/*
>  		 * no need to enqueue, do the cmd hdl
>  		 * directly and free cmd parameter
>  		 */
>  
> +		if (disconnect_hdl23a(padapter, (u8 *)param) != H2C_SUCCESS)
>  			res = _FAIL;
>  		kfree(param);
>  	}
> -- 
> 2.1.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/65c30e146c9e0fe672a78d4cc51ba07dfd9f4f6d.1445282918.git.amsfield22%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
> 

Re: [Outreachy kernel] [PATCH 3/5] staging: r8723au: use kernel preferred style for block comments

[Julia Lawall]

On Mon, 19 Oct 2015, Alison Schofield wrote:

> Reworked comments per kernel coding style.
> 
> Resolves checkpatch:
> WARNING: Block comments use * on subsequent lines
> 
> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> ---
>  drivers/staging/rtl8723au/core/rtw_cmd.c | 74 ++++++++++++++++++++------------
>  1 file changed, 46 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> index 7bf0f30..33164d3 100644
> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> @@ -176,9 +176,9 @@ struct _cmd_callback	rtw_cmd_callback[] = {
>  };
>  
>  /*
> -Caller and the rtw_cmd_thread23a can protect cmd_q by spin_lock.
> -No irqsave is necessary.
> -*/
> + * Caller and the rtw_cmd_thread23a can protect cmd_q by spin_lock.
> + * No irqsave is necessary.
> + */
>  
>  int rtw_init_cmd_priv23a(struct cmd_priv *pcmdpriv)
>  {
> @@ -315,8 +315,10 @@ post_process:
>  				 pcmd_callback, pcmd->cmdcode);
>  			rtw_free_cmd_obj23a(pcmd);
>  		} else {
> -			/* need consider that free cmd_obj in
> -			   rtw_cmd_callback */
> +			/*
> +			 * need consider that free cmd_obj in
> +			 * rtw_cmd_callback
> +			 */
>  			pcmd_callback(pcmd->padapter, pcmd);
>  		}
>  	} else {
> @@ -518,11 +520,13 @@ int rtw_joinbss_cmd23a(struct rtw_adapter *padapter,
>  	       get_wlan_bssid_ex_sz(&pnetwork->network));
>  
>  	psecnetwork->IELength = 0;
> -	/*  Added by Albert 2009/02/18 */
> -	/*  If the the driver wants to use the bssid to create the
> +
> +	/*
> +	 *  If the the driver wants to use the bssid to create the
>  	 *  connection. If not,  we have to copy the connecting AP's
>  	 *  MAC address to it so that the driver just has the bssid
> -	 *  information for PMKIDList searching. */
> +	 *  information for PMKIDList searching.
> +	 */

Maybe change the spacing here, as done in a later case.

>  
>  	if (pmlmepriv->assoc_by_bssid == false)
>  		ether_addr_copy(&pmlmepriv->assoc_bssid[0],
> @@ -557,10 +561,13 @@ int rtw_joinbss_cmd23a(struct rtw_adapter *padapter,
>  	phtpriv->ht_option = false;
>  	if (pregistrypriv->ht_enable) {
>  		u32 algo = padapter->securitypriv.dot11PrivacyAlgrthm;
> -		/*	Added by Albert 2010/06/23 */
> -		/*	For the WEP mode, we will use the bg mode to do
> -			the connection to avoid some IOT issue. */
> -		/*	Especially for Realtek 8192u SoftAP. */
> +
> +		/*
> +		 * For the WEP mode, we will use the bg mode to do
> +		 * the connection to avoid some IOT issue.
> +		 * Especially for Realtek 8192u SoftAP.
> +		 */
> +
>  		if (algo != WLAN_CIPHER_SUITE_WEP40 &&
>  		    algo != WLAN_CIPHER_SUITE_WEP104 &&
>  		    algo != WLAN_CIPHER_SUITE_TKIP) {
> @@ -630,9 +637,13 @@ int rtw_disassoc_cmd23a(struct rtw_adapter *padapter, u32 deauth_timeout_ms,
>  		init_h2fwcmd_w_parm_no_rsp(cmdobj, param, _DisConnect_CMD_);
>  		res = rtw_enqueue_cmd23a(cmdpriv, cmdobj);
>  	} else {
> -		/* no need to enqueue, do the cmd hdl directly and
> -		   free cmd parameter */
>  		if (H2C_SUCCESS != disconnect_hdl23a(padapter, (u8 *)param))
> +
> +		/*
> +		 * no need to enqueue, do the cmd hdl
> +		 * directly and free cmd parameter
> +		 */
> +
>  			res = _FAIL;
>  		kfree(param);
>  	}
> @@ -867,16 +878,18 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
>  	int BusyThreshold = 100;
>  	struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo;
>  
> -	/*  */
>  	/*  Determine if our traffic is busy now */

Spacing could change here too.

> -	/*  */
>  	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
>  		if (rtl8723a_BT_coexist(padapter))
>  			BusyThreshold = 50;
>  		else if (ldi->bBusyTraffic)
>  			BusyThreshold = 75;
> -		/*  if we raise bBusyTraffic in last watchdog, using
> -		    lower threshold. */
> +
> +		/*
> +		 * if we raise bBusyTraffic in last watchdog,
> +		 * using lower threshold.
> +		 */
> +
>  		if (ldi->NumRxOkInPeriod > BusyThreshold ||
>  		    ldi->NumTxOkInPeriod > BusyThreshold) {
>  			bBusyTraffic = true;
> @@ -947,9 +960,7 @@ static void dynamic_chk_wk_hdl(struct rtw_adapter *padapter, u8 *pbuf, int sz)
>  
>  	rtl8723a_HalDmWatchDog(padapter);
>  
> -	/*  */
>  	/*  BT-Coexist */
> -	/*  */

Etc.

julia

>  	rtl8723a_BT_do_coexist(padapter);
>  }
>  
> @@ -1234,8 +1245,8 @@ void rtw_evt_work(struct work_struct *work)
>  	} else {
>  		/*
>  		 * Enqueue into cmd_thread for others.
> -		 * ework will be turned into a c2h_evt and freed once it
> -		 * has been consumed.
> +		 * ework will be turned into a c2h_evt and
> +		 * freed once it has been consumed.
>  		 */
>  		rtw_c2h_wk_cmd23a(adapter, (u8 *)&ework->u.c2h_evt);
>  	}
> @@ -1402,11 +1413,16 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
>  		memcpy(&pwlan->network, pnetwork, pnetwork->Length);
>  		/* pwlan->fixed = true; */
>  
> -		/* list_add_tail(&pwlan->list,
> -		   &pmlmepriv->scanned_queue.queue); */
> +		/*
> +		 * list_add_tail(&pwlan->list,
> +		 * &pmlmepriv->scanned_queue->queue);
> +		 */
> +
> +		/*
> +		 *  copy pdev_network information to
> +		 *  pmlmepriv->cur_network
> +		 */
>  
> -		/*  copy pdev_network information to
> -		    pmlmepriv->cur_network */
>  		memcpy(&tgt_network->network, pnetwork,
>  		       get_wlan_bssid_ex_sz(pnetwork));
>  
> @@ -1415,8 +1431,10 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
>  		clr_fwstate(pmlmepriv, _FW_UNDER_LINKING);
>  
>  		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
> -		/*  we will set _FW_LINKED when there is one more sat to
> -		    join us (rtw_stassoc_event_callback23a) */
> +		/*
> +		 * we will set _FW_LINKED when there is one more
> +		 * sat to join us (rtw_stassoc_event_callback23a)
> +		 */
>  	}
>  
>  createbss_cmd_fail:
> -- 
> 2.1.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/ca096ddbb44df952c41fd95e6397d902d3baf767.1445282918.git.amsfield22%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
> 

Re: [Outreachy kernel] [PATCH 2/5] staging: r8723au: replace printk() with pr_err()

[Julia Lawall]

On Mon, 19 Oct 2015, Alison Schofield wrote:

> Replace printk() with pr_err() for uniform error reporting.
> Issue found by checkpatch.
> 
> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> ---
>  drivers/staging/rtl8723au/core/rtw_cmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> index c3579ac..7bf0f30 100644
> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> @@ -248,7 +248,7 @@ int rtw_enqueue_cmd23a(struct cmd_priv *pcmdpriv, struct cmd_obj *cmd_obj)
>  	res = queue_work(pcmdpriv->wq, &cmd_obj->work);
>  
>  	if (!res) {
> -		printk(KERN_ERR "%s: Call to queue_work() failed\n", __func__);
> +		pr_err("%s: Call to queue_work() failed\n", __func__);

There is no device tructure or network device structure available?

julia

>  		res = _FAIL;
>  	} else
>  		res = _SUCCESS;
> -- 
> 2.1.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/beee0bc7850917329b2b0ad3d3e0584f5cbbcdba.1445282918.git.amsfield22%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
> 

[PATCH 5/5] staging: r8723au: add & use local variable to simplify references

[Alison Schofield]

Add local variable scanned_queue to rtw_createbss_cmd23a_callback()
and use it (6x) to simplify references.

Resolves checkpatch.pl WARNING: line over 80 characters

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---

Note to reviewers:
I considered removing another local variable to compensate for the one
I added.  tgt_network is set and used once. I did not remove tgt_network
as it was beyond the scope of the checkpatch error I set out to address.

alison


 drivers/staging/rtl8723au/core/rtw_cmd.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 6034428..4d9850c 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -1365,6 +1365,7 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct wlan_bssid_ex *pnetwork = (struct wlan_bssid_ex *)pcmd->parmbuf;
 	struct wlan_network *tgt_network = &pmlmepriv->cur_network;
+	struct rtw_queue *scanned_queue = &pmlmepriv->scanned_queue;
 
 	if (pcmd->res != H2C_SUCCESS) {
 		RT_TRACE(_module_rtl871x_cmd_c_, _drv_err_,
@@ -1394,19 +1395,19 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
 		spin_unlock_bh(&pmlmepriv->lock);
 	} else {
 		pwlan = rtw_alloc_network(pmlmepriv, GFP_KERNEL);
-		spin_lock_bh(&pmlmepriv->scanned_queue.lock);
+		spin_lock_bh(&scanned_queue->lock);
 		if (!pwlan) {
-			pwlan = rtw_get_oldest_wlan_network23a(&pmlmepriv->scanned_queue);
+			pwlan = rtw_get_oldest_wlan_network23a(scanned_queue);
 			if (!pwlan) {
 				RT_TRACE(_module_rtl871x_cmd_c_, _drv_err_,
 					 "Error:  can't get pwlan in rtw23a_joinbss_event_cb\n");
-				spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
+				spin_unlock_bh(&scanned_queue->lock);
 				goto createbss_cmd_fail;
 			}
 			pwlan->last_scanned = jiffies;
 		} else {
 			list_add_tail(&pwlan->list,
-				      &pmlmepriv->scanned_queue.queue);
+				      &scanned_queue->queue);
 		}
 
 		pnetwork->Length = get_wlan_bssid_ex_sz(pnetwork);
@@ -1430,7 +1431,8 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
 
 		clr_fwstate(pmlmepriv, _FW_UNDER_LINKING);
 
-		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
+		spin_unlock_bh(&scanned_queue->lock);
+
 		/*
 		 * we will set _FW_LINKED when there is one more
 		 * sat to join us (rtw_stassoc_event_callback23a)
-- 
2.1.4

Re: [Outreachy kernel] [PATCH 4/5] staging: r8723au: move constant to right of comparison test

[Alison Schofield]

On Mon, Oct 19, 2015 at 10:06:21PM +0200, Julia Lawall wrote:
> On Mon, 19 Oct 2015, Alison Schofield wrote:
> 
> > Move constant to right of comparison test to improve readability.
> 
> Was it intentional to also move it below the comment?
> 
> julia
> 

This was not intentional and I don't see it as moved in my 'working'
file. It has raised a big red flag that the process I used to create
this patchset is wrong.  I'm going to retrace my steps, but if you have
a quick thought on where I screwed up...I'd appreciate it :)
- edited single final with all 5 types of changes.
- used git add -pi to add each hunk and commited each..one at a time
I thought the results would be 5 independent patches, but now I don't
think so!

Thanks for any pointers!

alison



> > 
> > checkpatch.pl:
> > 	WARNING: Comparisons should place the constant on the
> > 	right side of the test
> > 
> > Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> > ---
> >  drivers/staging/rtl8723au/core/rtw_cmd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > index 33164d3..6034428 100644
> > --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> > +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > @@ -637,13 +637,13 @@ int rtw_disassoc_cmd23a(struct rtw_adapter *padapter, u32 deauth_timeout_ms,
> >  		init_h2fwcmd_w_parm_no_rsp(cmdobj, param, _DisConnect_CMD_);
> >  		res = rtw_enqueue_cmd23a(cmdpriv, cmdobj);
> >  	} else {
> > -		if (H2C_SUCCESS != disconnect_hdl23a(padapter, (u8 *)param))
> >  
> >  		/*
> >  		 * no need to enqueue, do the cmd hdl
> >  		 * directly and free cmd parameter
> >  		 */
> >  
> > +		if (disconnect_hdl23a(padapter, (u8 *)param) != H2C_SUCCESS)
> >  			res = _FAIL;
> >  		kfree(param);
> >  	}
> > -- 
> > 2.1.4
> > 
> > -- 
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/65c30e146c9e0fe672a78d4cc51ba07dfd9f4f6d.1445282918.git.amsfield22%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> > 

Re: [Outreachy kernel] [PATCH 3/5] staging: r8723au: use kernel preferred style for block comments

[Alison Schofield]

On Mon, Oct 19, 2015 at 10:07:48PM +0200, Julia Lawall wrote:
> On Mon, 19 Oct 2015, Alison Schofield wrote:
> 
> > Reworked comments per kernel coding style.
> > 
> > Resolves checkpatch:
> > WARNING: Block comments use * on subsequent lines
> > 
> > Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> > ---
> >  drivers/staging/rtl8723au/core/rtw_cmd.c | 74 ++++++++++++++++++++------------
> >  1 file changed, 46 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > index 7bf0f30..33164d3 100644
> > --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> > +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > @@ -176,9 +176,9 @@ struct _cmd_callback	rtw_cmd_callback[] = {
> >  };
> >  
> >  /*
> > -Caller and the rtw_cmd_thread23a can protect cmd_q by spin_lock.
> > -No irqsave is necessary.
> > -*/
> > + * Caller and the rtw_cmd_thread23a can protect cmd_q by spin_lock.
> > + * No irqsave is necessary.
> > + */
> >  
> >  int rtw_init_cmd_priv23a(struct cmd_priv *pcmdpriv)
> >  {
> > @@ -315,8 +315,10 @@ post_process:
> >  				 pcmd_callback, pcmd->cmdcode);
> >  			rtw_free_cmd_obj23a(pcmd);
> >  		} else {
> > -			/* need consider that free cmd_obj in
> > -			   rtw_cmd_callback */
> > +			/*
> > +			 * need consider that free cmd_obj in
> > +			 * rtw_cmd_callback
> > +			 */
> >  			pcmd_callback(pcmd->padapter, pcmd);
> >  		}
> >  	} else {
> > @@ -518,11 +520,13 @@ int rtw_joinbss_cmd23a(struct rtw_adapter *padapter,
> >  	       get_wlan_bssid_ex_sz(&pnetwork->network));
> >  
> >  	psecnetwork->IELength = 0;
> > -	/*  Added by Albert 2009/02/18 */
> > -	/*  If the the driver wants to use the bssid to create the
> > +
> > +	/*
> > +	 *  If the the driver wants to use the bssid to create the
> >  	 *  connection. If not,  we have to copy the connecting AP's
> >  	 *  MAC address to it so that the driver just has the bssid
> > -	 *  information for PMKIDList searching. */
> > +	 *  information for PMKIDList searching.
> > +	 */
> 
> Maybe change the spacing here, as done in a later case.
> 
> >  
> >  	if (pmlmepriv->assoc_by_bssid == false)
> >  		ether_addr_copy(&pmlmepriv->assoc_bssid[0],
> > @@ -557,10 +561,13 @@ int rtw_joinbss_cmd23a(struct rtw_adapter *padapter,
> >  	phtpriv->ht_option = false;
> >  	if (pregistrypriv->ht_enable) {
> >  		u32 algo = padapter->securitypriv.dot11PrivacyAlgrthm;
> > -		/*	Added by Albert 2010/06/23 */
> > -		/*	For the WEP mode, we will use the bg mode to do
> > -			the connection to avoid some IOT issue. */
> > -		/*	Especially for Realtek 8192u SoftAP. */
> > +
> > +		/*
> > +		 * For the WEP mode, we will use the bg mode to do
> > +		 * the connection to avoid some IOT issue.
> > +		 * Especially for Realtek 8192u SoftAP.
> > +		 */
> > +
> >  		if (algo != WLAN_CIPHER_SUITE_WEP40 &&
> >  		    algo != WLAN_CIPHER_SUITE_WEP104 &&
> >  		    algo != WLAN_CIPHER_SUITE_TKIP) {
> > @@ -630,9 +637,13 @@ int rtw_disassoc_cmd23a(struct rtw_adapter *padapter, u32 deauth_timeout_ms,
> >  		init_h2fwcmd_w_parm_no_rsp(cmdobj, param, _DisConnect_CMD_);
> >  		res = rtw_enqueue_cmd23a(cmdpriv, cmdobj);
> >  	} else {
> > -		/* no need to enqueue, do the cmd hdl directly and
> > -		   free cmd parameter */
> >  		if (H2C_SUCCESS != disconnect_hdl23a(padapter, (u8 *)param))
> > +
> > +		/*
> > +		 * no need to enqueue, do the cmd hdl
> > +		 * directly and free cmd parameter
> > +		 */
> > +
> >  			res = _FAIL;
> >  		kfree(param);
> >  	}
> > @@ -867,16 +878,18 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
> >  	int BusyThreshold = 100;
> >  	struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo;
> >  
> > -	/*  */
> >  	/*  Determine if our traffic is busy now */
> 
> Spacing could change here too.
> 
> > -	/*  */
> >  	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
> >  		if (rtl8723a_BT_coexist(padapter))
> >  			BusyThreshold = 50;
> >  		else if (ldi->bBusyTraffic)
> >  			BusyThreshold = 75;
> > -		/*  if we raise bBusyTraffic in last watchdog, using
> > -		    lower threshold. */
> > +
> > +		/*
> > +		 * if we raise bBusyTraffic in last watchdog,
> > +		 * using lower threshold.
> > +		 */
> > +
> >  		if (ldi->NumRxOkInPeriod > BusyThreshold ||
> >  		    ldi->NumTxOkInPeriod > BusyThreshold) {
> >  			bBusyTraffic = true;
> > @@ -947,9 +960,7 @@ static void dynamic_chk_wk_hdl(struct rtw_adapter *padapter, u8 *pbuf, int sz)
> >  
> >  	rtl8723a_HalDmWatchDog(padapter);
> >  
> > -	/*  */
> >  	/*  BT-Coexist */
> > -	/*  */
> 
> Etc.
> 
> julia
>

Your comments have made me rethink how I addressed the checkpatch
errors. 

Previously, I only cleaned those that checkpatch complained about. Now,
I'm realizing while I'm cleaning up, I should clean up more throughly
and correct all the comments. (without changing wording of course.)

Similarly with the data structure.  There are 2 main structs at the top of
this file, and although only the first one triggered checkpatch
warnings, the second one could use some clean up also.

Is that the correct mindset for these type of checkpatch warnings?

alison


> >  	rtl8723a_BT_do_coexist(padapter);
> >  }
> >  
> > @@ -1234,8 +1245,8 @@ void rtw_evt_work(struct work_struct *work)
> >  	} else {
> >  		/*
> >  		 * Enqueue into cmd_thread for others.
> > -		 * ework will be turned into a c2h_evt and freed once it
> > -		 * has been consumed.
> > +		 * ework will be turned into a c2h_evt and
> > +		 * freed once it has been consumed.
> >  		 */
> >  		rtw_c2h_wk_cmd23a(adapter, (u8 *)&ework->u.c2h_evt);
> >  	}
> > @@ -1402,11 +1413,16 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
> >  		memcpy(&pwlan->network, pnetwork, pnetwork->Length);
> >  		/* pwlan->fixed = true; */
> >  
> > -		/* list_add_tail(&pwlan->list,
> > -		   &pmlmepriv->scanned_queue.queue); */
> > +		/*
> > +		 * list_add_tail(&pwlan->list,
> > +		 * &pmlmepriv->scanned_queue->queue);
> > +		 */
> > +
> > +		/*
> > +		 *  copy pdev_network information to
> > +		 *  pmlmepriv->cur_network
> > +		 */
> >  
> > -		/*  copy pdev_network information to
> > -		    pmlmepriv->cur_network */
> >  		memcpy(&tgt_network->network, pnetwork,
> >  		       get_wlan_bssid_ex_sz(pnetwork));
> >  
> > @@ -1415,8 +1431,10 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
> >  		clr_fwstate(pmlmepriv, _FW_UNDER_LINKING);
> >  
> >  		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
> > -		/*  we will set _FW_LINKED when there is one more sat to
> > -		    join us (rtw_stassoc_event_callback23a) */
> > +		/*
> > +		 * we will set _FW_LINKED when there is one more
> > +		 * sat to join us (rtw_stassoc_event_callback23a)
> > +		 */
> >  	}
> >  
> >  createbss_cmd_fail:
> > -- 
> > 2.1.4
> > 
> > -- 
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/ca096ddbb44df952c41fd95e6397d902d3baf767.1445282918.git.amsfield22%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> > 

Re: [Outreachy kernel] [PATCH 4/5] staging: r8723au: move constant to right of comparison test

[Alison Schofield]

On Mon, Oct 19, 2015 at 10:41:31PM +0200, Julia Lawall wrote:
> 
> 
> On Mon, 19 Oct 2015, Alison Schofield wrote:
> 
> > On Mon, Oct 19, 2015 at 10:06:21PM +0200, Julia Lawall wrote:
> > > On Mon, 19 Oct 2015, Alison Schofield wrote:
> > > 
> > > > Move constant to right of comparison test to improve readability.
> > > 
> > > Was it intentional to also move it below the comment?
> > > 
> > > julia
> > 
> > This was not intentional and I don't see it as moved in my 'working'
> > file. It has raised a big red flag that the process I used to create
> > this patchset is wrong.  I'm going to retrace my steps, but if you have
> > a quick thought on where I screwed up...I'd appreciate it :)
> > - edited single final with all 5 types of changes.
> > - used git add -pi to add each hunk and commited each..one at a time
> > I thought the results would be 5 independent patches, but now I don't
> > think so!
> 
> No idea, sorry.  Maybe someone else will have an idea.
> 
> julia
> 

Realized the error of my ways. (Need to put on sep branches to get
independent commits.) Will resend entire set. 

alison

alison

> > 
> > Thanks for any pointers!
> > 
> > alison
> > 
> > 
> > 
> > 
> > 
> > > 
> > > > 
> > > > checkpatch.pl:
> > > > 	WARNING: Comparisons should place the constant on the
> > > > 	right side of the test
> > > > 
> > > > Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> > > > ---
> > > >  drivers/staging/rtl8723au/core/rtw_cmd.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > > > index 33164d3..6034428 100644
> > > > --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> > > > +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > > > @@ -637,13 +637,13 @@ int rtw_disassoc_cmd23a(struct rtw_adapter *padapter, u32 deauth_timeout_ms,
> > > >  		init_h2fwcmd_w_parm_no_rsp(cmdobj, param, _DisConnect_CMD_);
> > > >  		res = rtw_enqueue_cmd23a(cmdpriv, cmdobj);
> > > >  	} else {
> > > > -		if (H2C_SUCCESS != disconnect_hdl23a(padapter, (u8 *)param))
> > > >  
> > > >  		/*
> > > >  		 * no need to enqueue, do the cmd hdl
> > > >  		 * directly and free cmd parameter
> > > >  		 */
> > > >  
> > > > +		if (disconnect_hdl23a(padapter, (u8 *)param) != H2C_SUCCESS)
> > > >  			res = _FAIL;
> > > >  		kfree(param);
> > > >  	}
> > > > -- 
> > > > 2.1.4
> > > > 
> > > > -- 
> > > > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > > > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/65c30e146c9e0fe672a78d4cc51ba07dfd9f4f6d.1445282918.git.amsfield22%40gmail.com.
> > > > For more options, visit https://groups.google.com/d/optout.
> > > > 
> > 

Re: [Outreachy kernel] [PATCH 3/5] staging: r8723au: use kernel preferred style for block comments

[Julia Lawall]

> Your comments have made me rethink how I addressed the checkpatch
> errors. 
> 
> Previously, I only cleaned those that checkpatch complained about. Now,
> I'm realizing while I'm cleaning up, I should clean up more throughly
> and correct all the comments. (without changing wording of course.)
> 
> Similarly with the data structure.  There are 2 main structs at the top of
> this file, and although only the first one triggered checkpatch
> warnings, the second one could use some clean up also.
> 
> Is that the correct mindset for these type of checkpatch warnings?

I think it depends on how complicated the other problems you are.  If you
are fixing a code formatting problem, and you see other code formatting
problems, then you could really fix them all up at once.  If there is any
doubt about the correctness of any of your changes, then you should do them
ome by one, so that they can be reverted independently, if needed.  For
example, if checkpatch tells you about spaces around parentheses in an if
test, and if it also tells you that the xyz function should be used instead
of abc, and the goal of your patch is to fix spaces around parentheses,
then you should not change abc to xyz at the same time.  But if you are
working on abc to xyz conversion, and you see a space around parentheses
problem on the same line, you could probably fix it up too, especially if
it is the only one in the file.

julia

Re: [Outreachy kernel] [PATCH 2/5] staging: r8723au: replace printk() with pr_err()

[Alison Schofield]

On Mon, Oct 19, 2015 at 10:09:46PM +0200, Julia Lawall wrote:
> On Mon, 19 Oct 2015, Alison Schofield wrote:
> 
> > Replace printk() with pr_err() for uniform error reporting.
> > Issue found by checkpatch.
> > 
> > Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> > ---
> >  drivers/staging/rtl8723au/core/rtw_cmd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > index c3579ac..7bf0f30 100644
> > --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> > +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > @@ -248,7 +248,7 @@ int rtw_enqueue_cmd23a(struct cmd_priv *pcmdpriv, struct cmd_obj *cmd_obj)
> >  	res = queue_work(pcmdpriv->wq, &cmd_obj->work);
> >  
> >  	if (!res) {
> > -		printk(KERN_ERR "%s: Call to queue_work() failed\n", __func__);
> > +		pr_err("%s: Call to queue_work() failed\n", __func__);
> 
> There is no device tructure or network device structure available?
> 
> julia
>
Yes there is a netdev. Used in v2.  Thanks!

> >  		res = _FAIL;
> >  	} else
> >  		res = _SUCCESS;
> > -- 
> > 2.1.4
> > 
> > -- 
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/beee0bc7850917329b2b0ad3d3e0584f5cbbcdba.1445282918.git.amsfield22%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> > 

[PATCH v2 0/5] staging: r8723au: fix multiple checkpatch issues

[Alison Schofield]

This patchset addresses the last 26 checkpatch WARNINGs in file:
drivers/staging/rtl8723au/core/rtw_cmd.c

I'll call your attention to the "Line over 80 chars" cases:

I chose to address 'Line over 80 chars' because all but one could be
resolved by cleaning up a single struct definition. [PATCH 1/5]

The last one appeared to be due to excessively long function names
as opposed to deep indentation/complex code requiring refactoring.
See my notes below the --- in PATCH[4/5] for further info.

alison

Changes in v2:

The patches have been reordered.  
	The comment corrections are now the last patch in the series.

[1/5] no changes
[2/5] now using netdev_err per reviewer feedback
[3/5] no change in code, but the diff changed when I moved the comment patch.
[4/5] no change in code, but same as above, diff appears a bit 'different.'
[5/5] scanned and corrected comments thru entire file, not just those
	reported by checkpatch (ie. spacing issues)




Alison Schofield (5):
  staging: r8723au: break parameter list at separators and align to open
        braces
  staging: r8723au: replace printk() with netdev_err()
  staging: r8723au: move constant to right of comparison test
  staging: r8723au: add & use local variable to simplify references
  staging: r8723au: use kernel preferred style for commenting

 drivers/staging/rtl8723au/core/rtw_cmd.c | 239 ++++++++++++++++---------------
 1 file changed, 127 insertions(+), 112 deletions(-)

-- 
2.1.4

[PATCH v2 1/5] staging: r8723au: break parameter list at separators and align to open braces

[Alison Schofield]

Improve readability by aligning the struct cmd_hdl wlancmds[] to fit
within 80 chars and cleaning up the adjacent index comments.

Addresses checkpatch.pl WARNING: line over 80 characters

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_cmd.c | 64 +++++++++++++++++++-------------
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 46aea16..c3579ac 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -22,7 +22,7 @@
 #include <rtw_sreset.h>
 
 static struct cmd_hdl wlancmds[] = {
-	GEN_DRV_CMD_HANDLER(0, NULL) /*0*/
+	GEN_DRV_CMD_HANDLER(0, NULL)					/*0*/
 	GEN_DRV_CMD_HANDLER(0, NULL)
 	GEN_DRV_CMD_HANDLER(0, NULL)
 	GEN_DRV_CMD_HANDLER(0, NULL)
@@ -32,18 +32,26 @@ static struct cmd_hdl wlancmds[] = {
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(0, NULL) /*10*/
+	GEN_MLME_EXT_HANDLER(0, NULL)					/*10*/
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), join_cmd_hdl23a) /*14*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm), disconnect_hdl23a)
-	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), createbss_hdl23a)
-	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm), setopmode_hdl23a)
-	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm), sitesurvey_cmd_hdl23a) /*18*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm), setauth_hdl23a)
-	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm), setkey_hdl23a) /*20*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm), set_stakey_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
+			     join_cmd_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm),
+			     disconnect_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
+			     createbss_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm),
+			     setopmode_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm),
+			     sitesurvey_cmd_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm),
+			     setauth_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm),		/*20*/
+			     setkey_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm),
+			     set_stakey_hdl23a)
 	GEN_MLME_EXT_HANDLER(sizeof(struct set_assocsta_parm), NULL)
 	GEN_MLME_EXT_HANDLER(sizeof(struct del_assocsta_parm), NULL)
 	GEN_MLME_EXT_HANDLER(sizeof(struct setstapwrstate_parm), NULL)
@@ -52,7 +60,7 @@ static struct cmd_hdl wlancmds[] = {
 	GEN_MLME_EXT_HANDLER(sizeof(struct setdatarate_parm), NULL)
 	GEN_MLME_EXT_HANDLER(sizeof(struct getdatarate_parm), NULL)
 	GEN_MLME_EXT_HANDLER(sizeof(struct setphyinfo_parm), NULL)
-	GEN_MLME_EXT_HANDLER(sizeof(struct getphyinfo_parm), NULL)  /*30*/
+	GEN_MLME_EXT_HANDLER(sizeof(struct getphyinfo_parm), NULL)	/*30*/
 	GEN_MLME_EXT_HANDLER(sizeof(struct setphy_parm), NULL)
 	GEN_MLME_EXT_HANDLER(sizeof(struct getphy_parm), NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
@@ -62,32 +70,36 @@ static struct cmd_hdl wlancmds[] = {
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(0, NULL)	/*40*/
+	GEN_MLME_EXT_HANDLER(0, NULL)					/*40*/
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(sizeof(struct addBaReq_parm), add_ba_hdl23a)
-	GEN_MLME_EXT_HANDLER(sizeof(struct set_ch_parm), set_ch_hdl23a) /* 46 */
+	GEN_MLME_EXT_HANDLER(sizeof(struct addBaReq_parm),
+			     add_ba_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct set_ch_parm),
+			     set_ch_hdl23a)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(0, NULL) /*50*/
+	GEN_MLME_EXT_HANDLER(0, NULL)					/*50*/
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
 	GEN_MLME_EXT_HANDLER(0, NULL)
-	GEN_MLME_EXT_HANDLER(sizeof(struct Tx_Beacon_param), tx_beacon_hdl23a) /*55*/
-
-	GEN_MLME_EXT_HANDLER(0, mlme_evt_hdl23a) /*56*/
-	GEN_MLME_EXT_HANDLER(0, rtw_drvextra_cmd_hdl23a) /*57*/
-
-	GEN_MLME_EXT_HANDLER(0, h2c_msg_hdl23a) /*58*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct SetChannelPlan_param), set_chplan_hdl23a) /*59*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct LedBlink_param), led_blink_hdl23a) /*60*/
-
-	GEN_MLME_EXT_HANDLER(sizeof(struct SetChannelSwitch_param), set_csa_hdl23a) /*61*/
-	GEN_MLME_EXT_HANDLER(sizeof(struct TDLSoption_param), tdls_hdl23a) /*62*/
+	GEN_MLME_EXT_HANDLER(sizeof(struct Tx_Beacon_param),
+			     tx_beacon_hdl23a)
+	GEN_MLME_EXT_HANDLER(0, mlme_evt_hdl23a)
+	GEN_MLME_EXT_HANDLER(0, rtw_drvextra_cmd_hdl23a)
+	GEN_MLME_EXT_HANDLER(0, h2c_msg_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct SetChannelPlan_param),
+			     set_chplan_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct LedBlink_param),		/*60*/
+			     led_blink_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct SetChannelSwitch_param),
+			     set_csa_hdl23a)
+	GEN_MLME_EXT_HANDLER(sizeof(struct TDLSoption_param),
+			     tdls_hdl23a)
 };
 
 struct _cmd_callback	rtw_cmd_callback[] = {
-- 
2.1.4

[PATCH v2 2/5] staging: r8723au: replace printk() with netdev_err()

[Alison Schofield]

Replace printk() with netdev_err() for uniform error reporting.
Issue found by checkpatch.

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_cmd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index c3579ac..389ad03 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -248,7 +248,8 @@ int rtw_enqueue_cmd23a(struct cmd_priv *pcmdpriv, struct cmd_obj *cmd_obj)
 	res = queue_work(pcmdpriv->wq, &cmd_obj->work);
 
 	if (!res) {
-		printk(KERN_ERR "%s: Call to queue_work() failed\n", __func__);
+		netdev_err(pcmdpriv->padapter->pnetdev,
+			   "%s: Call to queue_work() failed\n", __func__);
 		res = _FAIL;
 	} else
 		res = _SUCCESS;
-- 
2.1.4

[PATCH v2 3/5] staging: r8723au: move constant to right of comparison test

[Alison Schofield]

Move constant to right of comparison test to improve readability.

Addresses checkpatch.pl:
	WARNING: Comparisons should place the constant on the
	right side of the test

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 389ad03..334cc5b 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -633,7 +633,7 @@ int rtw_disassoc_cmd23a(struct rtw_adapter *padapter, u32 deauth_timeout_ms,
 	} else {
 		/* no need to enqueue, do the cmd hdl directly and
 		   free cmd parameter */
-		if (H2C_SUCCESS != disconnect_hdl23a(padapter, (u8 *)param))
+		if (disconnect_hdl23a(padapter, (u8 *)param) != H2C_SUCCESS)
 			res = _FAIL;
 		kfree(param);
 	}
-- 
2.1.4

[PATCH v2 4/5] staging: r8723au: add & use local variable to simplify references

[Alison Schofield]

Add local variable scanned_queue to rtw_createbss_cmd23a_callback()
and use it (5x) to simplify references.

Addresses checkpatch.pl WARNING: line over 80 characters

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---

Note to reviewers:
I considered removing another local variable to compensate for the one
I added.  tgt_network is set and used once. I did not remove tgt_network
as it was beyond the scope of the checkpatch error I set out to address.

a

 drivers/staging/rtl8723au/core/rtw_cmd.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 334cc5b..6330933 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -1355,6 +1355,7 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct wlan_bssid_ex *pnetwork = (struct wlan_bssid_ex *)pcmd->parmbuf;
 	struct wlan_network *tgt_network = &pmlmepriv->cur_network;
+	struct rtw_queue *scanned_queue = &pmlmepriv->scanned_queue;
 
 	if (pcmd->res != H2C_SUCCESS) {
 		RT_TRACE(_module_rtl871x_cmd_c_, _drv_err_,
@@ -1384,19 +1385,19 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
 		spin_unlock_bh(&pmlmepriv->lock);
 	} else {
 		pwlan = rtw_alloc_network(pmlmepriv, GFP_KERNEL);
-		spin_lock_bh(&pmlmepriv->scanned_queue.lock);
+		spin_lock_bh(&scanned_queue->lock);
 		if (!pwlan) {
-			pwlan = rtw_get_oldest_wlan_network23a(&pmlmepriv->scanned_queue);
+			pwlan = rtw_get_oldest_wlan_network23a(scanned_queue);
 			if (!pwlan) {
 				RT_TRACE(_module_rtl871x_cmd_c_, _drv_err_,
 					 "Error:  can't get pwlan in rtw23a_joinbss_event_cb\n");
-				spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
+				spin_unlock_bh(&scanned_queue->lock);
 				goto createbss_cmd_fail;
 			}
 			pwlan->last_scanned = jiffies;
 		} else {
 			list_add_tail(&pwlan->list,
-				      &pmlmepriv->scanned_queue.queue);
+				      &scanned_queue->queue);
 		}
 
 		pnetwork->Length = get_wlan_bssid_ex_sz(pnetwork);
@@ -1415,9 +1416,9 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
 
 		clr_fwstate(pmlmepriv, _FW_UNDER_LINKING);
 
-		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
 		/*  we will set _FW_LINKED when there is one more sat to
 		    join us (rtw_stassoc_event_callback23a) */
+		spin_unlock_bh(&scanned_queue->lock);
 	}
 
 createbss_cmd_fail:
-- 
2.1.4

[PATCH v2 5/5] staging: r8723au: use kernel preferred style for commenting

[Alison Schofield]

Reworked comments per kernel coding style to improve readability.

Addresses checkpatch.pl:
	WARNING: Block comments use * on subsequent lines

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_cmd.c | 159 ++++++++++++++++---------------
 1 file changed, 80 insertions(+), 79 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 6330933..4db4dfe 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -102,83 +102,76 @@ static struct cmd_hdl wlancmds[] = {
 			     tdls_hdl23a)
 };
 
-struct _cmd_callback	rtw_cmd_callback[] = {
-	{GEN_CMD_CODE(_Read_MACREG), NULL}, /*0*/
+struct _cmd_callback rtw_cmd_callback[] = {
+	{GEN_CMD_CODE(_Read_MACREG), NULL},				/*0*/
 	{GEN_CMD_CODE(_Write_MACREG), NULL},
 	{GEN_CMD_CODE(_Read_BBREG), &rtw_getbbrfreg_cmdrsp_callback23a},
 	{GEN_CMD_CODE(_Write_BBREG), NULL},
 	{GEN_CMD_CODE(_Read_RFREG), &rtw_getbbrfreg_cmdrsp_callback23a},
-	{GEN_CMD_CODE(_Write_RFREG), NULL}, /*5*/
+	{GEN_CMD_CODE(_Write_RFREG), NULL},
 	{GEN_CMD_CODE(_Read_EEPROM), NULL},
 	{GEN_CMD_CODE(_Write_EEPROM), NULL},
 	{GEN_CMD_CODE(_Read_EFUSE), NULL},
 	{GEN_CMD_CODE(_Write_EFUSE), NULL},
-
-	{GEN_CMD_CODE(_Read_CAM),	NULL},	/*10*/
-	{GEN_CMD_CODE(_Write_CAM),	 NULL},
+	{GEN_CMD_CODE(_Read_CAM), NULL},				/*10*/
+	{GEN_CMD_CODE(_Write_CAM), NULL},
 	{GEN_CMD_CODE(_setBCNITV), NULL},
 	{GEN_CMD_CODE(_setMBIDCFG), NULL},
-	{GEN_CMD_CODE(_JoinBss), &rtw_joinbss_cmd23a_callback},  /*14*/
-	{GEN_CMD_CODE(_DisConnect), &rtw_disassoc_cmd23a_callback}, /*15*/
+	{GEN_CMD_CODE(_JoinBss), &rtw_joinbss_cmd23a_callback},
+	{GEN_CMD_CODE(_DisConnect), &rtw_disassoc_cmd23a_callback},
 	{GEN_CMD_CODE(_CreateBss), &rtw_createbss_cmd23a_callback},
 	{GEN_CMD_CODE(_SetOpMode), NULL},
-	{GEN_CMD_CODE(_SiteSurvey), &rtw_survey_cmd_callback23a}, /*18*/
+	{GEN_CMD_CODE(_SiteSurvey), &rtw_survey_cmd_callback23a},
 	{GEN_CMD_CODE(_SetAuth), NULL},
-
-	{GEN_CMD_CODE(_SetKey), NULL},	/*20*/
+	{GEN_CMD_CODE(_SetKey), NULL},					/*20*/
 	{GEN_CMD_CODE(_SetStaKey), &rtw_setstaKey_cmdrsp_callback23a},
 	{GEN_CMD_CODE(_SetAssocSta), &rtw_setassocsta_cmdrsp_callback23a},
 	{GEN_CMD_CODE(_DelAssocSta), NULL},
 	{GEN_CMD_CODE(_SetStaPwrState), NULL},
-	{GEN_CMD_CODE(_SetBasicRate), NULL}, /*25*/
+	{GEN_CMD_CODE(_SetBasicRate), NULL},
 	{GEN_CMD_CODE(_GetBasicRate), NULL},
 	{GEN_CMD_CODE(_SetDataRate), NULL},
 	{GEN_CMD_CODE(_GetDataRate), NULL},
 	{GEN_CMD_CODE(_SetPhyInfo), NULL},
-
-	{GEN_CMD_CODE(_GetPhyInfo), NULL}, /*30*/
+	{GEN_CMD_CODE(_GetPhyInfo), NULL},				/*30*/
 	{GEN_CMD_CODE(_SetPhy), NULL},
 	{GEN_CMD_CODE(_GetPhy), NULL},
 	{GEN_CMD_CODE(_readRssi), NULL},
 	{GEN_CMD_CODE(_readGain), NULL},
-	{GEN_CMD_CODE(_SetAtim), NULL}, /*35*/
+	{GEN_CMD_CODE(_SetAtim), NULL},
 	{GEN_CMD_CODE(_SetPwrMode), NULL},
 	{GEN_CMD_CODE(_JoinbssRpt), NULL},
 	{GEN_CMD_CODE(_SetRaTable), NULL},
 	{GEN_CMD_CODE(_GetRaTable), NULL},
-
-	{GEN_CMD_CODE(_GetCCXReport), NULL}, /*40*/
-	{GEN_CMD_CODE(_GetDTMReport),	NULL},
+	{GEN_CMD_CODE(_GetCCXReport), NULL},				/*40*/
+	{GEN_CMD_CODE(_GetDTMReport), NULL},
 	{GEN_CMD_CODE(_GetTXRateStatistics), NULL},
 	{GEN_CMD_CODE(_SetUsbSuspend), NULL},
 	{GEN_CMD_CODE(_SetH2cLbk), NULL},
-	{GEN_CMD_CODE(_AddBAReq), NULL}, /*45*/
-	{GEN_CMD_CODE(_SetChannel), NULL},		/*46*/
+	{GEN_CMD_CODE(_AddBAReq), NULL},
+	{GEN_CMD_CODE(_SetChannel), NULL},
 	{GEN_CMD_CODE(_SetTxPower), NULL},
 	{GEN_CMD_CODE(_SwitchAntenna), NULL},
 	{GEN_CMD_CODE(_SetCrystalCap), NULL},
-	{GEN_CMD_CODE(_SetSingleCarrierTx), NULL},	/*50*/
-
-	{GEN_CMD_CODE(_SetSingleToneTx), NULL}, /*51*/
+	{GEN_CMD_CODE(_SetSingleCarrierTx), NULL},			/*50*/
+	{GEN_CMD_CODE(_SetSingleToneTx), NULL},
 	{GEN_CMD_CODE(_SetCarrierSuppressionTx), NULL},
 	{GEN_CMD_CODE(_SetContinuousTx), NULL},
-	{GEN_CMD_CODE(_SwitchBandwidth), NULL},		/*54*/
-	{GEN_CMD_CODE(_TX_Beacon), NULL},/*55*/
-
-	{GEN_CMD_CODE(_Set_MLME_EVT), NULL},/*56*/
-	{GEN_CMD_CODE(_Set_Drv_Extra), NULL},/*57*/
-	{GEN_CMD_CODE(_Set_H2C_MSG), NULL},/*58*/
-	{GEN_CMD_CODE(_SetChannelPlan), NULL},/*59*/
-	{GEN_CMD_CODE(_LedBlink), NULL},/*60*/
-
-	{GEN_CMD_CODE(_SetChannelSwitch), NULL},/*61*/
-	{GEN_CMD_CODE(_TDLS), NULL},/*62*/
+	{GEN_CMD_CODE(_SwitchBandwidth), NULL},
+	{GEN_CMD_CODE(_TX_Beacon), NULL},
+	{GEN_CMD_CODE(_Set_MLME_EVT), NULL},
+	{GEN_CMD_CODE(_Set_Drv_Extra), NULL},
+	{GEN_CMD_CODE(_Set_H2C_MSG), NULL},
+	{GEN_CMD_CODE(_SetChannelPlan), NULL},
+	{GEN_CMD_CODE(_LedBlink), NULL},				/*60*/
+	{GEN_CMD_CODE(_SetChannelSwitch), NULL},
+	{GEN_CMD_CODE(_TDLS), NULL},
 };
 
 /*
-Caller and the rtw_cmd_thread23a can protect cmd_q by spin_lock.
-No irqsave is necessary.
-*/
+ * Caller and the rtw_cmd_thread23a can protect cmd_q by spin_lock.
+ * No irqsave is necessary.
+ */
 
 int rtw_init_cmd_priv23a(struct cmd_priv *pcmdpriv)
 {
@@ -316,8 +309,10 @@ post_process:
 				 pcmd_callback, pcmd->cmdcode);
 			rtw_free_cmd_obj23a(pcmd);
 		} else {
-			/* need consider that free cmd_obj in
-			   rtw_cmd_callback */
+			/*
+			 * need to consider that free
+			 * cmd_obj in rtw_cmd_callback
+			 */
 			pcmd_callback(pcmd->padapter, pcmd);
 		}
 	} else {
@@ -398,7 +393,7 @@ int rtw_sitesurvey_cmd23a(struct rtw_adapter *padapter,
 		mod_timer(&pmlmepriv->scan_to_timer, jiffies +
 			  msecs_to_jiffies(SCANNING_TIMEOUT));
 
-		pmlmepriv->scan_interval = SCAN_INTERVAL;/*  30*2 sec = 60sec */
+		pmlmepriv->scan_interval = SCAN_INTERVAL; /* 30*2 sec = 60sec */
 	} else
 		_clr_fwstate_(pmlmepriv, _FW_UNDER_SURVEY);
 
@@ -519,12 +514,12 @@ int rtw_joinbss_cmd23a(struct rtw_adapter *padapter,
 	       get_wlan_bssid_ex_sz(&pnetwork->network));
 
 	psecnetwork->IELength = 0;
-	/*  Added by Albert 2009/02/18 */
-	/*  If the the driver wants to use the bssid to create the
+	/*
+	 *  If the the driver wants to use the bssid to create the
 	 *  connection. If not,  we have to copy the connecting AP's
 	 *  MAC address to it so that the driver just has the bssid
-	 *  information for PMKIDList searching. */
-
+	 *  information for PMKIDList searching.
+	 */
 	if (pmlmepriv->assoc_by_bssid == false)
 		ether_addr_copy(&pmlmepriv->assoc_bssid[0],
 				&pnetwork->network.MacAddress[0]);
@@ -558,10 +553,11 @@ int rtw_joinbss_cmd23a(struct rtw_adapter *padapter,
 	phtpriv->ht_option = false;
 	if (pregistrypriv->ht_enable) {
 		u32 algo = padapter->securitypriv.dot11PrivacyAlgrthm;
-		/*	Added by Albert 2010/06/23 */
-		/*	For the WEP mode, we will use the bg mode to do
-			the connection to avoid some IOT issue. */
-		/*	Especially for Realtek 8192u SoftAP. */
+		/*
+		 * For the WEP mode, we will use the bg mode to
+		 * do the connection to avoid some IOT issue.
+		 * Especially for Realtek 8192u SoftAP.
+		 */
 		if (algo != WLAN_CIPHER_SUITE_WEP40 &&
 		    algo != WLAN_CIPHER_SUITE_WEP104 &&
 		    algo != WLAN_CIPHER_SUITE_TKIP) {
@@ -590,7 +586,7 @@ int rtw_joinbss_cmd23a(struct rtw_adapter *padapter,
 	/* get cmdsz before endian conversion */
 	pcmd->cmdsz = get_wlan_bssid_ex_sz(psecnetwork);
 
-	pcmd->cmdcode = _JoinBss_CMD_;/* GEN_CMD_CODE(_JoinBss) */
+	pcmd->cmdcode = _JoinBss_CMD_; /* GEN_CMD_CODE(_JoinBss) */
 	pcmd->parmbuf = (unsigned char *)psecnetwork;
 	pcmd->rsp = NULL;
 	pcmd->rspsz = 0;
@@ -631,8 +627,10 @@ int rtw_disassoc_cmd23a(struct rtw_adapter *padapter, u32 deauth_timeout_ms,
 		init_h2fwcmd_w_parm_no_rsp(cmdobj, param, _DisConnect_CMD_);
 		res = rtw_enqueue_cmd23a(cmdpriv, cmdobj);
 	} else {
-		/* no need to enqueue, do the cmd hdl directly and
-		   free cmd parameter */
+		/*
+		 * no need to enqueue, do the cmd hdl
+		 * directly and free cmd parameter
+		 */
 		if (disconnect_hdl23a(padapter, (u8 *)param) != H2C_SUCCESS)
 			res = _FAIL;
 		kfree(param);
@@ -726,7 +724,7 @@ int rtw_setstakey_cmd23a(struct rtw_adapter *padapter, u8 *psta, u8 unicast_key)
 		       &psecuritypriv->dot118021XGrpKey[idx].skey, 16);
 	}
 
-	/* jeff: set this because at least sw key is ready */
+	/* set this because at least sw key is ready */
 	padapter->securitypriv.busetkipkey = 1;
 
 	res = rtw_enqueue_cmd23a(pcmdpriv, ph2c);
@@ -868,16 +866,16 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
 	int BusyThreshold = 100;
 	struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo;
 
-	/*  */
-	/*  Determine if our traffic is busy now */
-	/*  */
+	/* Determine if our traffic is busy now */
 	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
 		if (rtl8723a_BT_coexist(padapter))
 			BusyThreshold = 50;
 		else if (ldi->bBusyTraffic)
 			BusyThreshold = 75;
-		/*  if we raise bBusyTraffic in last watchdog, using
-		    lower threshold. */
+		/*
+		 * if we raise bBusyTraffic in last
+		 * watchdog, using lower threshold
+		 */
 		if (ldi->NumRxOkInPeriod > BusyThreshold ||
 		    ldi->NumTxOkInPeriod > BusyThreshold) {
 			bBusyTraffic = true;
@@ -888,7 +886,7 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
 				bTxBusyTraffic = true;
 		}
 
-		/*  Higher Tx/Rx data. */
+		/* Higher Tx/Rx data */
 		if (ldi->NumRxOkInPeriod > 4000 ||
 		    ldi->NumTxOkInPeriod > 4000) {
 			bHigherBusyTraffic = true;
@@ -901,7 +899,7 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
 
 		if (!rtl8723a_BT_coexist(padapter) ||
 		    !rtl8723a_BT_using_antenna_1(padapter)) {
-		/*  check traffic for  powersaving. */
+		/* check traffic for powersaving */
 			if (((ldi->NumRxUnicastOkInPeriod +
 			      ldi->NumTxOkInPeriod) > 8) ||
 			    ldi->NumRxUnicastOkInPeriod > 2)
@@ -909,7 +907,7 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
 			else
 				bEnterPS = true;
 
-			/*  LeisurePS only work in infra mode. */
+			/* LeisurePS only work in infra mode */
 			if (bEnterPS)
 				LPS_Enter23a(padapter);
 			else
@@ -948,9 +946,7 @@ static void dynamic_chk_wk_hdl(struct rtw_adapter *padapter, u8 *pbuf, int sz)
 
 	rtl8723a_HalDmWatchDog(padapter);
 
-	/*  */
-	/*  BT-Coexist */
-	/*  */
+	/* BT-Coexist */
 	rtl8723a_BT_do_coexist(padapter);
 }
 
@@ -976,14 +972,14 @@ static void lps_ctrl_wk_hdl(struct rtw_adapter *padapter, u8 lps_ctrl_type)
 		LPS_Leave23a(padapter);
 		break;
 	case LPS_CTRL_CONNECT:
-		mstatus = 1;/* connect */
-		/*  Reset LPS Setting */
+		mstatus = 1; /* connect */
+		/* Reset LPS Setting */
 		padapter->pwrctrlpriv.LpsIdleCount = 0;
 		rtl8723a_set_FwJoinBssReport_cmd(padapter, 1);
 		rtl8723a_BT_mediastatus_notify(padapter, mstatus);
 		break;
 	case LPS_CTRL_DISCONNECT:
-		mstatus = 0;/* disconnect */
+		mstatus = 0; /* disconnect */
 		rtl8723a_BT_mediastatus_notify(padapter, mstatus);
 		if (!rtl8723a_BT_using_antenna_1(padapter))
 			LPS_Leave23a(padapter);
@@ -1190,7 +1186,7 @@ static int c2h_evt_hdl(struct rtw_adapter *adapter, struct c2h_evt_hdr *c2h_evt)
 	u8 buf[16];
 
 	if (!c2h_evt) {
-		/* No c2h event in cmd_obj, read c2h event before handling*/
+		/* No c2h event in cmd_obj, read c2h event before handling */
 		if (c2h_evt_read23a(adapter, buf) == _SUCCESS) {
 			c2h_evt = (struct c2h_evt_hdr *)buf;
 
@@ -1235,8 +1231,8 @@ void rtw_evt_work(struct work_struct *work)
 	} else {
 		/*
 		 * Enqueue into cmd_thread for others.
-		 * ework will be turned into a c2h_evt and freed once it
-		 * has been consumed.
+		 * ework will be turned into a c2h_evt
+		 * and freed once it has been consumed.
 		 */
 		rtw_c2h_wk_cmd23a(adapter, (u8 *)&ework->u.c2h_evt);
 	}
@@ -1323,7 +1319,7 @@ void rtw_disassoc_cmd23a_callback(struct rtw_adapter *padapter,
 		return;
 	}
 
-	/*  free cmd */
+	/* free cmd */
 	rtw_free_cmd_obj23a(pcmd);
 }
 
@@ -1402,23 +1398,28 @@ void rtw_createbss_cmd23a_callback(struct rtw_adapter *padapter,
 
 		pnetwork->Length = get_wlan_bssid_ex_sz(pnetwork);
 		memcpy(&pwlan->network, pnetwork, pnetwork->Length);
-		/* pwlan->fixed = true; */
-
-		/* list_add_tail(&pwlan->list,
-		   &pmlmepriv->scanned_queue.queue); */
+		/*
+		 * pwlan->fixed = true;
+		 * list_add_tail(&pwlan->list,
+		 *		 &pmlmepriv->scanned_queue.queue);
+		 */
 
-		/*  copy pdev_network information to
-		    pmlmepriv->cur_network */
+		/*
+		 *  copy pdev_network information
+		 *  to pmlmepriv->cur_network
+		 */
 		memcpy(&tgt_network->network, pnetwork,
 		       get_wlan_bssid_ex_sz(pnetwork));
 
-		/*  reset DSConfig */
+		/* reset DSConfig */
 
 		clr_fwstate(pmlmepriv, _FW_UNDER_LINKING);
 
-		/*  we will set _FW_LINKED when there is one more sat to
-		    join us (rtw_stassoc_event_callback23a) */
 		spin_unlock_bh(&scanned_queue->lock);
+		/*
+		 * we will set _FW_LINKED when there is one more
+		 * sat to join us (rtw_stassoc_event_callback23a)
+		 */
 	}
 
 createbss_cmd_fail:
-- 
2.1.4

Re: [Outreachy kernel] [PATCH v2 1/5] staging: r8723au: break parameter list at separators and align to open braces

[Greg KH]

On Tue, Oct 20, 2015 at 11:01:58PM -0700, Alison Schofield wrote:
> Improve readability by aligning the struct cmd_hdl wlancmds[] to fit
> within 80 chars and cleaning up the adjacent index comments.
> 
> Addresses checkpatch.pl WARNING: line over 80 characters
> 
> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> ---
>  drivers/staging/rtl8723au/core/rtw_cmd.c | 64 +++++++++++++++++++-------------
>  1 file changed, 38 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> index 46aea16..c3579ac 100644
> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> @@ -22,7 +22,7 @@
>  #include <rtw_sreset.h>
>  
>  static struct cmd_hdl wlancmds[] = {
> -	GEN_DRV_CMD_HANDLER(0, NULL) /*0*/
> +	GEN_DRV_CMD_HANDLER(0, NULL)					/*0*/
>  	GEN_DRV_CMD_HANDLER(0, NULL)
>  	GEN_DRV_CMD_HANDLER(0, NULL)
>  	GEN_DRV_CMD_HANDLER(0, NULL)
> @@ -32,18 +32,26 @@ static struct cmd_hdl wlancmds[] = {
>  	GEN_MLME_EXT_HANDLER(0, NULL)
>  	GEN_MLME_EXT_HANDLER(0, NULL)
>  	GEN_MLME_EXT_HANDLER(0, NULL)
> -	GEN_MLME_EXT_HANDLER(0, NULL) /*10*/
> +	GEN_MLME_EXT_HANDLER(0, NULL)					/*10*/
>  	GEN_MLME_EXT_HANDLER(0, NULL)
>  	GEN_MLME_EXT_HANDLER(0, NULL)
>  	GEN_MLME_EXT_HANDLER(0, NULL)
> -	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), join_cmd_hdl23a) /*14*/
> -	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm), disconnect_hdl23a)
> -	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), createbss_hdl23a)
> -	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm), setopmode_hdl23a)
> -	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm), sitesurvey_cmd_hdl23a) /*18*/
> -	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm), setauth_hdl23a)
> -	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm), setkey_hdl23a) /*20*/
> -	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm), set_stakey_hdl23a)
> +	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
> +			     join_cmd_hdl23a)
> +	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm),
> +			     disconnect_hdl23a)
> +	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
> +			     createbss_hdl23a)
> +	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm),
> +			     setopmode_hdl23a)
> +	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm),
> +			     sitesurvey_cmd_hdl23a)
> +	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm),
> +			     setauth_hdl23a)
> +	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm),		/*20*/
> +			     setkey_hdl23a)
> +	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm),
> +			     set_stakey_hdl23a)


This seems harder to read to me than before, how about you?

Re: [Outreachy kernel] [PATCH v2 5/5] staging: r8723au: use kernel preferred style for commenting

[Greg KH]

On Tue, Oct 20, 2015 at 11:10:41PM -0700, Alison Schofield wrote:
> Reworked comments per kernel coding style to improve readability.
> 
> Addresses checkpatch.pl:
> 	WARNING: Block comments use * on subsequent lines
> 
> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> ---
>  drivers/staging/rtl8723au/core/rtw_cmd.c | 159 ++++++++++++++++---------------
>  1 file changed, 80 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> index 6330933..4db4dfe 100644
> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> @@ -102,83 +102,76 @@ static struct cmd_hdl wlancmds[] = {
>  			     tdls_hdl23a)
>  };
>  
> -struct _cmd_callback	rtw_cmd_callback[] = {
> -	{GEN_CMD_CODE(_Read_MACREG), NULL}, /*0*/
> +struct _cmd_callback rtw_cmd_callback[] = {
> +	{GEN_CMD_CODE(_Read_MACREG), NULL},				/*0*/
>  	{GEN_CMD_CODE(_Write_MACREG), NULL},
>  	{GEN_CMD_CODE(_Read_BBREG), &rtw_getbbrfreg_cmdrsp_callback23a},
>  	{GEN_CMD_CODE(_Write_BBREG), NULL},
>  	{GEN_CMD_CODE(_Read_RFREG), &rtw_getbbrfreg_cmdrsp_callback23a},
> -	{GEN_CMD_CODE(_Write_RFREG), NULL}, /*5*/
> +	{GEN_CMD_CODE(_Write_RFREG), NULL},
>  	{GEN_CMD_CODE(_Read_EEPROM), NULL},
>  	{GEN_CMD_CODE(_Write_EEPROM), NULL},
>  	{GEN_CMD_CODE(_Read_EFUSE), NULL},
>  	{GEN_CMD_CODE(_Write_EFUSE), NULL},
> -
> -	{GEN_CMD_CODE(_Read_CAM),	NULL},	/*10*/
> -	{GEN_CMD_CODE(_Write_CAM),	 NULL},
> +	{GEN_CMD_CODE(_Read_CAM), NULL},				/*10*/
> +	{GEN_CMD_CODE(_Write_CAM), NULL},
>  	{GEN_CMD_CODE(_setBCNITV), NULL},
>  	{GEN_CMD_CODE(_setMBIDCFG), NULL},
> -	{GEN_CMD_CODE(_JoinBss), &rtw_joinbss_cmd23a_callback},  /*14*/
> -	{GEN_CMD_CODE(_DisConnect), &rtw_disassoc_cmd23a_callback}, /*15*/
> +	{GEN_CMD_CODE(_JoinBss), &rtw_joinbss_cmd23a_callback},
> +	{GEN_CMD_CODE(_DisConnect), &rtw_disassoc_cmd23a_callback},

Why delete the comments?

Re: [Outreachy kernel] [PATCH v2 1/5] staging: r8723au: break parameter list at separators and align to open braces

[Alison Schofield]

On Sat, Oct 24, 2015 at 07:40:29PM -0700, Greg KH wrote:
> On Tue, Oct 20, 2015 at 11:01:58PM -0700, Alison Schofield wrote:
> > Improve readability by aligning the struct cmd_hdl wlancmds[] to fit
> > within 80 chars and cleaning up the adjacent index comments.
> > 
> > Addresses checkpatch.pl WARNING: line over 80 characters
> > 
> > Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> > ---
> >  drivers/staging/rtl8723au/core/rtw_cmd.c | 64 +++++++++++++++++++-------------
> >  1 file changed, 38 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > index 46aea16..c3579ac 100644
> > --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> > +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > @@ -22,7 +22,7 @@
> >  #include <rtw_sreset.h>
> >  
> >  static struct cmd_hdl wlancmds[] = {
> > -	GEN_DRV_CMD_HANDLER(0, NULL) /*0*/
> > +	GEN_DRV_CMD_HANDLER(0, NULL)					/*0*/
> >  	GEN_DRV_CMD_HANDLER(0, NULL)
> >  	GEN_DRV_CMD_HANDLER(0, NULL)
> >  	GEN_DRV_CMD_HANDLER(0, NULL)
> > @@ -32,18 +32,26 @@ static struct cmd_hdl wlancmds[] = {
> >  	GEN_MLME_EXT_HANDLER(0, NULL)
> >  	GEN_MLME_EXT_HANDLER(0, NULL)
> >  	GEN_MLME_EXT_HANDLER(0, NULL)
> > -	GEN_MLME_EXT_HANDLER(0, NULL) /*10*/
> > +	GEN_MLME_EXT_HANDLER(0, NULL)					/*10*/
> >  	GEN_MLME_EXT_HANDLER(0, NULL)
> >  	GEN_MLME_EXT_HANDLER(0, NULL)
> >  	GEN_MLME_EXT_HANDLER(0, NULL)
> > -	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), join_cmd_hdl23a) /*14*/
> > -	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm), disconnect_hdl23a)
> > -	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), createbss_hdl23a)
> > -	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm), setopmode_hdl23a)
> > -	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm), sitesurvey_cmd_hdl23a) /*18*/
> > -	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm), setauth_hdl23a)
> > -	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm), setkey_hdl23a) /*20*/
> > -	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm), set_stakey_hdl23a)
> > +	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
> > +			     join_cmd_hdl23a)
> > +	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm),
> > +			     disconnect_hdl23a)
> > +	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
> > +			     createbss_hdl23a)
> > +	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm),
> > +			     setopmode_hdl23a)
> > +	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm),
> > +			     sitesurvey_cmd_hdl23a)
> > +	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm),
> > +			     setauth_hdl23a)
> > +	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm),		/*20*/
> > +			     setkey_hdl23a)
> > +	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm),
> > +			     set_stakey_hdl23a)
> 
> 
> This seems harder to read to me than before, how about you?
>
It does look worse in the diff, but when I'm scanning the entire 62 item
list, keeping the items to the left and the index comments to the right
seemed to make it easier to scan and find the item I want. (A maintainer
probably wouldn't be reading this list top to bottom.)

Is it appropriate for me to ask reviewers (you) to apply the patch to
review it?  It seems there is not always enough context in the diff.

alison

Re: [Outreachy kernel] [PATCH v2 5/5] staging: r8723au: use kernel preferred style for commenting

[Alison Schofield]

On Sat, Oct 24, 2015 at 07:41:21PM -0700, Greg KH wrote:
> On Tue, Oct 20, 2015 at 11:10:41PM -0700, Alison Schofield wrote:
> > Reworked comments per kernel coding style to improve readability.
> > 
> > Addresses checkpatch.pl:
> > 	WARNING: Block comments use * on subsequent lines
> > 
> > Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> > ---
> >  drivers/staging/rtl8723au/core/rtw_cmd.c | 159 ++++++++++++++++---------------
> >  1 file changed, 80 insertions(+), 79 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > index 6330933..4db4dfe 100644
> > --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> > +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > @@ -102,83 +102,76 @@ static struct cmd_hdl wlancmds[] = {
> >  			     tdls_hdl23a)
> >  };
> >  
> > -struct _cmd_callback	rtw_cmd_callback[] = {
> > -	{GEN_CMD_CODE(_Read_MACREG), NULL}, /*0*/
> > +struct _cmd_callback rtw_cmd_callback[] = {
> > +	{GEN_CMD_CODE(_Read_MACREG), NULL},				/*0*/
> >  	{GEN_CMD_CODE(_Write_MACREG), NULL},
> >  	{GEN_CMD_CODE(_Read_BBREG), &rtw_getbbrfreg_cmdrsp_callback23a},
> >  	{GEN_CMD_CODE(_Write_BBREG), NULL},
> >  	{GEN_CMD_CODE(_Read_RFREG), &rtw_getbbrfreg_cmdrsp_callback23a},
> > -	{GEN_CMD_CODE(_Write_RFREG), NULL}, /*5*/
> > +	{GEN_CMD_CODE(_Write_RFREG), NULL},
> >  	{GEN_CMD_CODE(_Read_EEPROM), NULL},
> >  	{GEN_CMD_CODE(_Write_EEPROM), NULL},
> >  	{GEN_CMD_CODE(_Read_EFUSE), NULL},
> >  	{GEN_CMD_CODE(_Write_EFUSE), NULL},
> > -
> > -	{GEN_CMD_CODE(_Read_CAM),	NULL},	/*10*/
> > -	{GEN_CMD_CODE(_Write_CAM),	 NULL},
> > +	{GEN_CMD_CODE(_Read_CAM), NULL},				/*10*/
> > +	{GEN_CMD_CODE(_Write_CAM), NULL},
> >  	{GEN_CMD_CODE(_setBCNITV), NULL},
> >  	{GEN_CMD_CODE(_setMBIDCFG), NULL},
> > -	{GEN_CMD_CODE(_JoinBss), &rtw_joinbss_cmd23a_callback},  /*14*/
> > -	{GEN_CMD_CODE(_DisConnect), &rtw_disassoc_cmd23a_callback}, /*15*/
> > +	{GEN_CMD_CODE(_JoinBss), &rtw_joinbss_cmd23a_callback},
> > +	{GEN_CMD_CODE(_DisConnect), &rtw_disassoc_cmd23a_callback},
> 
> Why delete the comments?
>
The comments note the place in the list. I didn't delete them entirely,
but replaced them with a marker every 10 items.  This was the second of
these two lists and I tried to give them the same look n feel.  

alison

Re: [Outreachy kernel] [PATCH v2 1/5] staging: r8723au: break parameter list at separators and align to open braces

[Greg KH]

On Sun, Oct 25, 2015 at 08:07:44PM -0700, Alison Schofield wrote:
> On Sat, Oct 24, 2015 at 07:40:29PM -0700, Greg KH wrote:
> > On Tue, Oct 20, 2015 at 11:01:58PM -0700, Alison Schofield wrote:
> > > Improve readability by aligning the struct cmd_hdl wlancmds[] to fit
> > > within 80 chars and cleaning up the adjacent index comments.
> > > 
> > > Addresses checkpatch.pl WARNING: line over 80 characters
> > > 
> > > Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> > > ---
> > >  drivers/staging/rtl8723au/core/rtw_cmd.c | 64 +++++++++++++++++++-------------
> > >  1 file changed, 38 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > > index 46aea16..c3579ac 100644
> > > --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> > > +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > > @@ -22,7 +22,7 @@
> > >  #include <rtw_sreset.h>
> > >  
> > >  static struct cmd_hdl wlancmds[] = {
> > > -	GEN_DRV_CMD_HANDLER(0, NULL) /*0*/
> > > +	GEN_DRV_CMD_HANDLER(0, NULL)					/*0*/
> > >  	GEN_DRV_CMD_HANDLER(0, NULL)
> > >  	GEN_DRV_CMD_HANDLER(0, NULL)
> > >  	GEN_DRV_CMD_HANDLER(0, NULL)
> > > @@ -32,18 +32,26 @@ static struct cmd_hdl wlancmds[] = {
> > >  	GEN_MLME_EXT_HANDLER(0, NULL)
> > >  	GEN_MLME_EXT_HANDLER(0, NULL)
> > >  	GEN_MLME_EXT_HANDLER(0, NULL)
> > > -	GEN_MLME_EXT_HANDLER(0, NULL) /*10*/
> > > +	GEN_MLME_EXT_HANDLER(0, NULL)					/*10*/
> > >  	GEN_MLME_EXT_HANDLER(0, NULL)
> > >  	GEN_MLME_EXT_HANDLER(0, NULL)
> > >  	GEN_MLME_EXT_HANDLER(0, NULL)
> > > -	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), join_cmd_hdl23a) /*14*/
> > > -	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm), disconnect_hdl23a)
> > > -	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), createbss_hdl23a)
> > > -	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm), setopmode_hdl23a)
> > > -	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm), sitesurvey_cmd_hdl23a) /*18*/
> > > -	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm), setauth_hdl23a)
> > > -	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm), setkey_hdl23a) /*20*/
> > > -	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm), set_stakey_hdl23a)
> > > +	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
> > > +			     join_cmd_hdl23a)
> > > +	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm),
> > > +			     disconnect_hdl23a)
> > > +	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
> > > +			     createbss_hdl23a)
> > > +	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm),
> > > +			     setopmode_hdl23a)
> > > +	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm),
> > > +			     sitesurvey_cmd_hdl23a)
> > > +	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm),
> > > +			     setauth_hdl23a)
> > > +	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm),		/*20*/
> > > +			     setkey_hdl23a)
> > > +	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm),
> > > +			     set_stakey_hdl23a)
> > 
> > 
> > This seems harder to read to me than before, how about you?
> >
> It does look worse in the diff, but when I'm scanning the entire 62 item
> list, keeping the items to the left and the index comments to the right
> seemed to make it easier to scan and find the item I want. (A maintainer
> probably wouldn't be reading this list top to bottom.)
> 
> Is it appropriate for me to ask reviewers (you) to apply the patch to
> review it?  It seems there is not always enough context in the diff.

There's enough context, we can read it :)

I think one-line-per-entry here is just fine, no need to change it at
all, sorry.

thanks,

greg k-h

Re: [Outreachy kernel] [PATCH v2 5/5] staging: r8723au: use kernel preferred style for commenting

[Greg KH]

On Sun, Oct 25, 2015 at 08:12:44PM -0700, Alison Schofield wrote:
> On Sat, Oct 24, 2015 at 07:41:21PM -0700, Greg KH wrote:
> > On Tue, Oct 20, 2015 at 11:10:41PM -0700, Alison Schofield wrote:
> > > Reworked comments per kernel coding style to improve readability.
> > > 
> > > Addresses checkpatch.pl:
> > > 	WARNING: Block comments use * on subsequent lines
> > > 
> > > Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> > > ---
> > >  drivers/staging/rtl8723au/core/rtw_cmd.c | 159 ++++++++++++++++---------------
> > >  1 file changed, 80 insertions(+), 79 deletions(-)
> > > 
> > > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > > index 6330933..4db4dfe 100644
> > > --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> > > +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> > > @@ -102,83 +102,76 @@ static struct cmd_hdl wlancmds[] = {
> > >  			     tdls_hdl23a)
> > >  };
> > >  
> > > -struct _cmd_callback	rtw_cmd_callback[] = {
> > > -	{GEN_CMD_CODE(_Read_MACREG), NULL}, /*0*/
> > > +struct _cmd_callback rtw_cmd_callback[] = {
> > > +	{GEN_CMD_CODE(_Read_MACREG), NULL},				/*0*/
> > >  	{GEN_CMD_CODE(_Write_MACREG), NULL},
> > >  	{GEN_CMD_CODE(_Read_BBREG), &rtw_getbbrfreg_cmdrsp_callback23a},
> > >  	{GEN_CMD_CODE(_Write_BBREG), NULL},
> > >  	{GEN_CMD_CODE(_Read_RFREG), &rtw_getbbrfreg_cmdrsp_callback23a},
> > > -	{GEN_CMD_CODE(_Write_RFREG), NULL}, /*5*/
> > > +	{GEN_CMD_CODE(_Write_RFREG), NULL},
> > >  	{GEN_CMD_CODE(_Read_EEPROM), NULL},
> > >  	{GEN_CMD_CODE(_Write_EEPROM), NULL},
> > >  	{GEN_CMD_CODE(_Read_EFUSE), NULL},
> > >  	{GEN_CMD_CODE(_Write_EFUSE), NULL},
> > > -
> > > -	{GEN_CMD_CODE(_Read_CAM),	NULL},	/*10*/
> > > -	{GEN_CMD_CODE(_Write_CAM),	 NULL},
> > > +	{GEN_CMD_CODE(_Read_CAM), NULL},				/*10*/
> > > +	{GEN_CMD_CODE(_Write_CAM), NULL},
> > >  	{GEN_CMD_CODE(_setBCNITV), NULL},
> > >  	{GEN_CMD_CODE(_setMBIDCFG), NULL},
> > > -	{GEN_CMD_CODE(_JoinBss), &rtw_joinbss_cmd23a_callback},  /*14*/
> > > -	{GEN_CMD_CODE(_DisConnect), &rtw_disassoc_cmd23a_callback}, /*15*/
> > > +	{GEN_CMD_CODE(_JoinBss), &rtw_joinbss_cmd23a_callback},
> > > +	{GEN_CMD_CODE(_DisConnect), &rtw_disassoc_cmd23a_callback},
> > 
> > Why delete the comments?
> >
> The comments note the place in the list. I didn't delete them entirely,
> but replaced them with a marker every 10 items.  This was the second of
> these two lists and I tried to give them the same look n feel.  

But maybe items 14 and 15 were "special" and wanted to be found easier,
you don't know :)

Just leave them there please.

thanks,

greg k-h

Re: [Outreachy kernel] [PATCH v2 1/5] staging: r8723au: break parameter list at separators and align to open braces

[Jes Sorensen]

On 10/27/15 01:44, Greg KH wrote:
> On Sun, Oct 25, 2015 at 08:07:44PM -0700, Alison Schofield wrote:
>> On Sat, Oct 24, 2015 at 07:40:29PM -0700, Greg KH wrote:
>>> On Tue, Oct 20, 2015 at 11:01:58PM -0700, Alison Schofield wrote:
>>>> Improve readability by aligning the struct cmd_hdl wlancmds[] to fit
>>>> within 80 chars and cleaning up the adjacent index comments.
>>>>
>>>> Addresses checkpatch.pl WARNING: line over 80 characters
>>>>
>>>> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
>>>> ---
>>>>  drivers/staging/rtl8723au/core/rtw_cmd.c | 64 +++++++++++++++++++-------------
>>>>  1 file changed, 38 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
>>>> index 46aea16..c3579ac 100644
>>>> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
>>>> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
>>>> @@ -22,7 +22,7 @@
>>>>  #include <rtw_sreset.h>
>>>>  
>>>>  static struct cmd_hdl wlancmds[] = {
>>>> -	GEN_DRV_CMD_HANDLER(0, NULL) /*0*/
>>>> +	GEN_DRV_CMD_HANDLER(0, NULL)					/*0*/
>>>>  	GEN_DRV_CMD_HANDLER(0, NULL)
>>>>  	GEN_DRV_CMD_HANDLER(0, NULL)
>>>>  	GEN_DRV_CMD_HANDLER(0, NULL)
>>>> @@ -32,18 +32,26 @@ static struct cmd_hdl wlancmds[] = {
>>>>  	GEN_MLME_EXT_HANDLER(0, NULL)
>>>>  	GEN_MLME_EXT_HANDLER(0, NULL)
>>>>  	GEN_MLME_EXT_HANDLER(0, NULL)
>>>> -	GEN_MLME_EXT_HANDLER(0, NULL) /*10*/
>>>> +	GEN_MLME_EXT_HANDLER(0, NULL)					/*10*/
>>>>  	GEN_MLME_EXT_HANDLER(0, NULL)
>>>>  	GEN_MLME_EXT_HANDLER(0, NULL)
>>>>  	GEN_MLME_EXT_HANDLER(0, NULL)
>>>> -	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), join_cmd_hdl23a) /*14*/
>>>> -	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm), disconnect_hdl23a)
>>>> -	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex), createbss_hdl23a)
>>>> -	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm), setopmode_hdl23a)
>>>> -	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm), sitesurvey_cmd_hdl23a) /*18*/
>>>> -	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm), setauth_hdl23a)
>>>> -	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm), setkey_hdl23a) /*20*/
>>>> -	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm), set_stakey_hdl23a)
>>>> +	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
>>>> +			     join_cmd_hdl23a)
>>>> +	GEN_MLME_EXT_HANDLER(sizeof(struct disconnect_parm),
>>>> +			     disconnect_hdl23a)
>>>> +	GEN_MLME_EXT_HANDLER(sizeof(struct wlan_bssid_ex),
>>>> +			     createbss_hdl23a)
>>>> +	GEN_MLME_EXT_HANDLER(sizeof(struct setopmode_parm),
>>>> +			     setopmode_hdl23a)
>>>> +	GEN_MLME_EXT_HANDLER(sizeof(struct sitesurvey_parm),
>>>> +			     sitesurvey_cmd_hdl23a)
>>>> +	GEN_MLME_EXT_HANDLER(sizeof(struct setauth_parm),
>>>> +			     setauth_hdl23a)
>>>> +	GEN_MLME_EXT_HANDLER(sizeof(struct setkey_parm),		/*20*/
>>>> +			     setkey_hdl23a)
>>>> +	GEN_MLME_EXT_HANDLER(sizeof(struct set_stakey_parm),
>>>> +			     set_stakey_hdl23a)
>>>
>>>
>>> This seems harder to read to me than before, how about you?
>>>
>> It does look worse in the diff, but when I'm scanning the entire 62 item
>> list, keeping the items to the left and the index comments to the right
>> seemed to make it easier to scan and find the item I want. (A maintainer
>> probably wouldn't be reading this list top to bottom.)
>>
>> Is it appropriate for me to ask reviewers (you) to apply the patch to
>> review it?  It seems there is not always enough context in the diff.
> 
> There's enough context, we can read it :)
> 
> I think one-line-per-entry here is just fine, no need to change it at
> all, sorry.

I agree, definitely no reason to do this.

In addition, please remember to CC the driver maintainer when you post
patches for a driver.

Jes