drop some unnecessary wrappers

PATCH Superseded Vihas Makwana 4 participants 22 messages Apr 11, 2022 – Apr 22, 2022 Staging

Share & tools

Vihas Makwana

author

2022-04-11 10:21 UTC #0 lore raw

v2 cover drop some unnecessary wrappers

Drop some unnecessary wrappers and update all the references accordingly.
Tested on Comfast CF-WU810N RTL8188EUS wireless adapter.

v1 -> v2:
Drop the wrapper functions with underscores prefixed.

Vihas Makwana (7):
staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv
staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv
staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network
staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd

 drivers/staging/r8188eu/core/rtw_cmd.c     | 145 +++++++----------
 drivers/staging/r8188eu/core/rtw_mlme.c    | 179 +++++++++------------
 drivers/staging/r8188eu/include/rtw_mlme.h |   4 +-
 3 files changed, 135 insertions(+), 193 deletions(-)

Signature · 1 line

2.30.2

Vihas Makwana

author

2022-04-11 10:21 UTC #1 lore raw

PATCH v2 1/7 staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv

Drop unnecessary wrapper _rtw_free_cmd_priv and move its logic to rtw_free_cmd_priv.

Signed-off-by Vihas Makwana <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_cmd.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

drivers/staging/r8188eu/core/rtw_cmd.c +4 -9

index f036a980e..d53217900 100644
@@ -87,14 +87,6 @@ void rtw_free_evt_priv(struct	evt_priv *pevtpriv)
 	}
 }
 
-static void _rtw_free_cmd_priv(struct cmd_priv *pcmdpriv)
-{
-	if (pcmdpriv) {
-		kfree(pcmdpriv->cmd_allocated_buf);
-		kfree(pcmdpriv->rsp_allocated_buf);
-	}
-}
-
 /* Calling Context:
  *
  * rtw_enqueue_cmd can only be called between kernel thread,

@@ -159,7 +151,10 @@ u32 rtw_init_evt_priv(struct evt_priv *pevtpriv)
 
 void rtw_free_cmd_priv(struct	cmd_priv *pcmdpriv)
 {
-	_rtw_free_cmd_priv(pcmdpriv);
+	if (pcmdpriv) {
+		kfree(pcmdpriv->cmd_allocated_buf);
+		kfree(pcmdpriv->rsp_allocated_buf);
+	}
 }
 
 static int rtw_cmd_filter(struct cmd_priv *pcmdpriv, struct cmd_obj *cmd_obj)

Signature · 1 line

2.30.2

Vihas Makwana

author

2022-04-11 10:22 UTC #2 lore raw

PATCH v2 2/7 staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv

In reply to Vihas Makwana

Drop unnecessary wrapper _rtw_init_cmd_priv and move its logic to rtw_init_cmd_priv.

Signed-off-by Vihas Makwana <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_cmd.c | 77 ++++++++++++--------------
 1 file changed, 34 insertions(+), 43 deletions(-)

drivers/staging/r8188eu/core/rtw_cmd.c +34 ~~-43~~

index d53217900..3d1fdc852 100644
@@ -15,47 +15,6 @@
  * No irqsave is necessary.
  */
 
-static int _rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
-{
-	int res = _SUCCESS;
-
-	init_completion(&pcmdpriv->enqueue_cmd);
-	/* sema_init(&(pcmdpriv->cmd_done_sema), 0); */
-	init_completion(&pcmdpriv->start_cmd_thread);
-	init_completion(&pcmdpriv->stop_cmd_thread);
-
-	rtw_init_queue(&pcmdpriv->cmd_queue);
-
-	/* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
-
-	pcmdpriv->cmd_seq = 1;
-
-	pcmdpriv->cmd_allocated_buf = kzalloc(MAX_CMDSZ + CMDBUFF_ALIGN_SZ,
-					      GFP_KERNEL);
-
-	if (!pcmdpriv->cmd_allocated_buf) {
-		res = _FAIL;
-		goto exit;
-	}
-
-	pcmdpriv->cmd_buf = pcmdpriv->cmd_allocated_buf  +  CMDBUFF_ALIGN_SZ - ((size_t)(pcmdpriv->cmd_allocated_buf) & (CMDBUFF_ALIGN_SZ - 1));
-
-	pcmdpriv->rsp_allocated_buf = kzalloc(MAX_RSPSZ + 4, GFP_KERNEL);
-
-	if (!pcmdpriv->rsp_allocated_buf) {
-		res = _FAIL;
-		goto exit;
-	}
-
-	pcmdpriv->rsp_buf = pcmdpriv->rsp_allocated_buf  +  4 - ((size_t)(pcmdpriv->rsp_allocated_buf) & 3);
-
-	pcmdpriv->cmd_done_cnt = 0;
-	pcmdpriv->rsp_cnt = 0;
-exit:
-
-	return res;
-}
-
 static void c2h_wk_callback(struct work_struct *work);
 
 static int _rtw_init_evt_priv(struct evt_priv *pevtpriv)

@@ -133,9 +92,41 @@ static struct cmd_obj *_rtw_dequeue_cmd(struct __queue *queue)
 
 u32	rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
 {
-	u32	res;
+	u32 res = _SUCCESS;
+
+	init_completion(&pcmdpriv->enqueue_cmd);
+	/* sema_init(&(pcmdpriv->cmd_done_sema), 0); */
+	init_completion(&pcmdpriv->start_cmd_thread);
+	init_completion(&pcmdpriv->stop_cmd_thread);
+
+	rtw_init_queue(&pcmdpriv->cmd_queue);
+
+	/* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
+
+	pcmdpriv->cmd_seq = 1;
+
+	pcmdpriv->cmd_allocated_buf = kzalloc(MAX_CMDSZ + CMDBUFF_ALIGN_SZ,
+					      GFP_KERNEL);
+
+	if (!pcmdpriv->cmd_allocated_buf) {
+		res = _FAIL;
+		goto exit;
+	}
+
+	pcmdpriv->cmd_buf = pcmdpriv->cmd_allocated_buf  +  CMDBUFF_ALIGN_SZ - ((size_t)(pcmdpriv->cmd_allocated_buf) & (CMDBUFF_ALIGN_SZ - 1));
+
+	pcmdpriv->rsp_allocated_buf = kzalloc(MAX_RSPSZ + 4, GFP_KERNEL);
+
+	if (!pcmdpriv->rsp_allocated_buf) {
+		res = _FAIL;
+		goto exit;
+	}
 
-	res = _rtw_init_cmd_priv(pcmdpriv);
+	pcmdpriv->rsp_buf = pcmdpriv->rsp_allocated_buf  +  4 - ((size_t)(pcmdpriv->rsp_allocated_buf) & 3);
+
+	pcmdpriv->cmd_done_cnt = 0;
+	pcmdpriv->rsp_cnt = 0;
+exit:
 
 	return res;
 }

Signature · 1 line

2.30.2

Vihas Makwana

author

2022-04-11 10:22 UTC #3 lore raw

PATCH v2 3/7 staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv

In reply to Vihas Makwana

Drop unnecessary wrapper _rtw_init_evt_priv and move its logic to rtw_init_evt_priv.

Signed-off-by Vihas Makwana <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_cmd.c | 27 +++++++++-----------------
 1 file changed, 9 insertions(+), 18 deletions(-)

drivers/staging/r8188eu/core/rtw_cmd.c +9 ~~-18~~

index 3d1fdc852..3781706f7 100644
@@ -17,22 +17,6 @@
 
 static void c2h_wk_callback(struct work_struct *work);
 
-static int _rtw_init_evt_priv(struct evt_priv *pevtpriv)
-{
-	int res = _SUCCESS;
-
-	/* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
-	atomic_set(&pevtpriv->event_seq, 0);
-
-	INIT_WORK(&pevtpriv->c2h_wk, c2h_wk_callback);
-	pevtpriv->c2h_wk_alive = false;
-	pevtpriv->c2h_queue = rtw_cbuf_alloc(C2H_QUEUE_MAX_LEN + 1);
-	if (!pevtpriv->c2h_queue)
-		res = _FAIL;
-
-	return res;
-}
-
 void rtw_free_evt_priv(struct	evt_priv *pevtpriv)
 {
 	cancel_work_sync(&pevtpriv->c2h_wk);

@@ -133,9 +117,16 @@ u32	rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
 
 u32 rtw_init_evt_priv(struct evt_priv *pevtpriv)
 {
-	int	res;
+	u32 res = _SUCCESS;
+
+	/* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
+	atomic_set(&pevtpriv->event_seq, 0);
 
-	res = _rtw_init_evt_priv(pevtpriv);
+	INIT_WORK(&pevtpriv->c2h_wk, c2h_wk_callback);
+	pevtpriv->c2h_wk_alive = false;
+	pevtpriv->c2h_queue = rtw_cbuf_alloc(C2H_QUEUE_MAX_LEN + 1);
+	if (!pevtpriv->c2h_queue)
+		res = _FAIL;
 
 	return res;
 }

Signature · 1 line

2.30.2

Vihas Makwana

author

2022-04-11 10:22 UTC #4 lore raw

PATCH v2 4/7 staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv

In reply to Vihas Makwana

Drop unnecessary wrapper _rtw_init_mlme_priv and move its logic to _rtw_init_mlme_priv.

Signed-off-by Vihas Makwana <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c | 103 +++++++++++-------------
 1 file changed, 47 insertions(+), 56 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +47 ~~-56~~

index c90f36dee..6b8c80fe9 100644
@@ -30,60 +30,6 @@ u8 rtw_to_roaming(struct adapter *adapter)
 	return adapter->mlmepriv.to_roaming;
 }
 
-static int _rtw_init_mlme_priv(struct adapter *padapter)
-{
-	int	i;
-	u8	*pbuf;
-	struct wlan_network	*pnetwork;
-	struct mlme_priv		*pmlmepriv = &padapter->mlmepriv;
-	int	res = _SUCCESS;
-
-	/*  We don't need to memset padapter->XXX to zero, because adapter is allocated by vzalloc(). */
-
-	pmlmepriv->nic_hdl = (u8 *)padapter;
-
-	pmlmepriv->pscanned = NULL;
-	pmlmepriv->fw_state = 0;
-	pmlmepriv->cur_network.network.InfrastructureMode = Ndis802_11AutoUnknown;
-	pmlmepriv->scan_mode = SCAN_ACTIVE;/*  1: active, 0: pasive. Maybe someday we should rename this varable to "active_mode" (Jeff) */
-
-	spin_lock_init(&pmlmepriv->lock);
-	rtw_init_queue(&pmlmepriv->free_bss_pool);
-	rtw_init_queue(&pmlmepriv->scanned_queue);
-
-	set_scanned_network_val(pmlmepriv, 0);
-
-	memset(&pmlmepriv->assoc_ssid, 0, sizeof(struct ndis_802_11_ssid));
-
-	pbuf = vzalloc(MAX_BSS_CNT * (sizeof(struct wlan_network)));
-
-	if (!pbuf) {
-		res = _FAIL;
-		goto exit;
-	}
-	pmlmepriv->free_bss_buf = pbuf;
-
-	pnetwork = (struct wlan_network *)pbuf;
-
-	for (i = 0; i < MAX_BSS_CNT; i++) {
-		INIT_LIST_HEAD(&pnetwork->list);
-
-		list_add_tail(&pnetwork->list, &pmlmepriv->free_bss_pool.queue);
-
-		pnetwork++;
-	}
-
-	/* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
-
-	rtw_clear_scan_deny(padapter);
-
-	rtw_init_mlme_timer(padapter);
-
-exit:
-
-	return res;
-}
-
 static void rtw_free_mlme_ie_data(u8 **ppie, u32 *plen)
 {
 	kfree(*ppie);

@@ -290,9 +236,54 @@ u8 *rtw_get_beacon_interval_from_ie(u8 *ie)
 
 int rtw_init_mlme_priv(struct adapter *padapter)/* struct	mlme_priv *pmlmepriv) */
 {
-	int	res;
+	int	i;
+	u8	*pbuf;
+	struct wlan_network	*pnetwork;
+	struct mlme_priv		*pmlmepriv = &padapter->mlmepriv;
+	int	res = _SUCCESS;
+
+	/*  We don't need to memset padapter->XXX to zero, because adapter is allocated by vzalloc(). */
+
+	pmlmepriv->nic_hdl = (u8 *)padapter;
+
+	pmlmepriv->pscanned = NULL;
+	pmlmepriv->fw_state = 0;
+	pmlmepriv->cur_network.network.InfrastructureMode = Ndis802_11AutoUnknown;
+	pmlmepriv->scan_mode = SCAN_ACTIVE;/*  1: active, 0: pasive. Maybe someday we should rename this varable to "active_mode" (Jeff) */
+
+	spin_lock_init(&pmlmepriv->lock);
+	rtw_init_queue(&pmlmepriv->free_bss_pool);
+	rtw_init_queue(&pmlmepriv->scanned_queue);
+
+	set_scanned_network_val(pmlmepriv, 0);
+
+	memset(&pmlmepriv->assoc_ssid, 0, sizeof(struct ndis_802_11_ssid));
 
-	res = _rtw_init_mlme_priv(padapter);/*  (pmlmepriv); */
+	pbuf = vzalloc(MAX_BSS_CNT * (sizeof(struct wlan_network)));
+
+	if (!pbuf) {
+		res = _FAIL;
+		goto exit;
+	}
+	pmlmepriv->free_bss_buf = pbuf;
+
+	pnetwork = (struct wlan_network *)pbuf;
+
+	for (i = 0; i < MAX_BSS_CNT; i++) {
+		INIT_LIST_HEAD(&pnetwork->list);
+
+		list_add_tail(&pnetwork->list, &pmlmepriv->free_bss_pool.queue);
+
+		pnetwork++;
+	}
+
+	/* allocate DMA-able/Non-Page memory for cmd_buf and rsp_buf */
+
+	rtw_clear_scan_deny(padapter);
+
+	rtw_init_mlme_timer(padapter);
+
+exit:
 
 	return res;
 }

Signature · 1 line

2.30.2

Vihas Makwana

author

2022-04-11 10:22 UTC #5 lore raw

PATCH v2 5/7 staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv

In reply to Vihas Makwana

Drop unnecessary wrapper _rtw_free_mlme_priv and move its logic to rtw_free_mlme_priv.

Signed-off-by Vihas Makwana <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c    | 17 +++++------------
 drivers/staging/r8188eu/include/rtw_mlme.h |  2 --
 2 files changed, 5 insertions(+), 14 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +5 ~~-12~~

index 6b8c80fe9..5584e1b84 100644
@@ -53,17 +53,6 @@ void rtw_free_mlme_priv_ie_data(struct mlme_priv *pmlmepriv)
 	rtw_free_mlme_ie_data(&pmlmepriv->p2p_assoc_req_ie, &pmlmepriv->p2p_assoc_req_ie_len);
 }
 
-void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
-{
-
-	rtw_free_mlme_priv_ie_data(pmlmepriv);
-
-	if (pmlmepriv) {
-		vfree(pmlmepriv->free_bss_buf);
-	}
-
-}
-
 struct	wlan_network *_rtw_alloc_network(struct	mlme_priv *pmlmepriv)/* _queue *free_queue) */
 {
 	struct	wlan_network	*pnetwork;

@@ -290,7 +279,11 @@ int rtw_init_mlme_priv(struct adapter *padapter)/* struct	mlme_priv *pmlmepriv)
 
 void rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
 {
-	_rtw_free_mlme_priv(pmlmepriv);
+	rtw_free_mlme_priv_ie_data(pmlmepriv);
+
+	if (pmlmepriv) {
+		vfree(pmlmepriv->free_bss_buf);
+	}
 }
 
 static struct wlan_network *rtw_alloc_network(struct mlme_priv *pmlmepriv)

drivers/staging/r8188eu/include/rtw_mlme.h +0 -2

index 1dc1fbf04..fc30793f4 100644
@@ -560,8 +560,6 @@ void rtw_scan_timeout_handler(struct adapter *adapter);
 
 void rtw_free_mlme_priv_ie_data(struct mlme_priv *pmlmepriv);
 
-void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv);
-
  struct wlan_network *_rtw_alloc_network(struct mlme_priv *pmlmepriv);
 
 void _rtw_free_network(struct mlme_priv *pmlmepriv,

Signature · 1 line

2.30.2

Dan Carpenter

2022-04-11 10:33 UTC #6 lore raw

Re: [PATCH v2 5/7] staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv

Quoted 15 lines · Vihas Makwana

drivers/staging/r8188eu/core/rtw_mlme.c +0 ~~-11~~

-void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
-{
-
-	rtw_free_mlme_priv_ie_data(pmlmepriv);
-
-	if (pmlmepriv) {
-		vfree(pmlmepriv->free_bss_buf);
-	}
-
-}
-
 struct	wlan_network *_rtw_alloc_network(struct	mlme_priv *pmlmepriv)/* _queue *free_queue) */
 {
 	struct	wlan_network	*pnetwork;

drivers/staging/r8188eu/core/rtw_mlme.c int rtw_init_mlme_priv(struct adapter *padapter)/* struct mlme_priv *pmlmepriv) · line 284

Quoted 7 lines

drivers/staging/r8188eu/core/rtw_mlme.c +3 -1

@@ -290,7 +279,11 @@ int rtw_init_mlme_priv(struct adapter *padapter)/* struct	mlme_priv *pmlmepriv)
 
 void rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
 {
-	_rtw_free_mlme_priv(pmlmepriv);
+	rtw_free_mlme_priv_ie_data(pmlmepriv);
+
+	if (pmlmepriv) {

Not related to your patch, but this check makes no sense. The rtw_free_mlme_priv_ie_data() function dereference pmlmepriv unconditionally so it would have already crashed.

drivers/staging/r8188eu/core/rtw_mlme.c int rtw_init_mlme_priv(struct adapter *padapter)/* struct mlme_priv *pmlmepriv) · line 287

Quoted 3 lines

drivers/staging/r8188eu/core/rtw_mlme.c +2 -0
+		vfree(pmlmepriv->free_bss_buf);
+	}
 }

regards,
dan carpenter

Vihas Makwana

author

2022-04-11 10:22 UTC #7 lore raw

PATCH v2 6/7 staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network

In reply to Vihas Makwana

Drop unnecessary wrapper _rtw_alloc_network and move its logic to rtw_alloc_network.

Signed-off-by Vihas Makwana <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_cmd.c     |  2 +-
 drivers/staging/r8188eu/core/rtw_mlme.c    | 59 +++++++++-------------
 drivers/staging/r8188eu/include/rtw_mlme.h |  2 +-
 3 files changed, 27 insertions(+), 36 deletions(-)

drivers/staging/r8188eu/core/rtw_cmd.c +1 -1

index 3781706f7..959b69be5 100644
@@ -1457,7 +1457,7 @@ void rtw_createbss_cmd_callback(struct adapter *padapter, struct cmd_obj *pcmd)
 		rtw_indicate_connect(padapter);
 	} else {
 
-		pwlan = _rtw_alloc_network(pmlmepriv);
+		pwlan = rtw_alloc_network(pmlmepriv);
 		spin_lock_bh(&pmlmepriv->scanned_queue.lock);
 		if (!pwlan) {
 			pwlan = rtw_get_oldest_wlan_network(&pmlmepriv->scanned_queue);

drivers/staging/r8188eu/core/rtw_mlme.c +25 ~~-34~~

74 lines not on this page · raw message

drivers/staging/r8188eu/include/rtw_mlme.h +1 -1

index fc30793f4..95c23bf11 100644
@@ -560,7 +560,7 @@ void rtw_scan_timeout_handler(struct adapter *adapter);
 
 void rtw_free_mlme_priv_ie_data(struct mlme_priv *pmlmepriv);
 
- struct wlan_network *_rtw_alloc_network(struct mlme_priv *pmlmepriv);
+struct wlan_network *rtw_alloc_network(struct mlme_priv *pmlmepriv);
 
 void _rtw_free_network(struct mlme_priv *pmlmepriv,
 		       struct wlan_network *pnetwork, u8 isfreeall);

Signature · 1 line

2.30.2

Vihas Makwana

author

2022-04-11 10:22 UTC #8 lore raw

PATCH v2 7/7 staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd

In reply to Vihas Makwana

Drop unnecessary wrapper _rtw_dequeue_cmd and move its logic to rtw_dequeue_cmd.

Signed-off-by Vihas Makwana <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_cmd.c | 34 ++++++++++----------------
 1 file changed, 13 insertions(+), 21 deletions(-)

drivers/staging/r8188eu/core/rtw_cmd.c +13 ~~-21~~

index 959b69be5..06523d919 100644
@@ -56,24 +56,6 @@ static int _rtw_enqueue_cmd(struct __queue *queue, struct cmd_obj *obj)
 	return _SUCCESS;
 }
 
-static struct cmd_obj *_rtw_dequeue_cmd(struct __queue *queue)
-{
-	struct cmd_obj *obj;
-	unsigned long flags;
-
-	spin_lock_irqsave(&queue->lock, flags);
-	if (list_empty(&queue->queue)) {
-		obj = NULL;
-	} else {
-		obj = container_of((&queue->queue)->next, struct cmd_obj, list);
-		list_del_init(&obj->list);
-	}
-
-	spin_unlock_irqrestore(&queue->lock, flags);
-
-	return obj;
-}
-
 u32	rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
 {
 	u32 res = _SUCCESS;

@@ -180,11 +162,21 @@ u32 rtw_enqueue_cmd(struct cmd_priv *pcmdpriv, struct cmd_obj *cmd_obj)
 
 struct	cmd_obj	*rtw_dequeue_cmd(struct cmd_priv *pcmdpriv)
 {
-	struct cmd_obj *cmd_obj;
+	struct cmd_obj *obj;
+	struct __queue *queue = &pcmdpriv->cmd_queue;
+	unsigned long flags;
+
+	spin_lock_irqsave(&queue->lock, flags);
+	if (list_empty(&queue->queue)) {
+		obj = NULL;
+	} else {
+		obj = container_of((&queue->queue)->next, struct cmd_obj, list);
+		list_del_init(&obj->list);
+	}
 
-	cmd_obj = _rtw_dequeue_cmd(&pcmdpriv->cmd_queue);
+	spin_unlock_irqrestore(&queue->lock, flags);
 
-	return cmd_obj;
+	return obj;
 }
 
 void rtw_free_cmd_obj(struct cmd_obj *pcmd)

Signature · 1 line

2.30.2

Dan Carpenter

Reviewed-by on the v2 series

2022-04-11 14:10 UTC #9 lore raw

In reply to Vihas Makwana

Quoted 7 lines · Vihas Makwana

Drop some unnecessary wrappers and update all the references accordingly.
Tested on Comfast CF-WU810N RTL8188EUS wireless adapter.
v1 -> v2:
Drop the wrapper functions with underscores prefixed.

Quoted 9 lines

Vihas Makwana (7):
staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv
staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv
staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network
staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd

Thanks!

Reviewed-by Dan Carpenter <[email protected]>

regards,
dan carpenter

Fabio M. De Francesco

2022-04-11 20:34 UTC #10 lore raw

In reply to Vihas Makwana

Quoted 16 lines · On lunedì 11 aprile 2022 12:21:29 CEST Vihas Makwana wrote:

Drop some unnecessary wrappers and update all the references accordingly.
Tested on Comfast CF-WU810N RTL8188EUS wireless adapter.
v1 -> v2:
Drop the wrapper functions with underscores prefixed.
Vihas Makwana (7):
staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv
staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv
staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network
staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd

Quoted 8 lines

 drivers/staging/r8188eu/core/rtw_cmd.c     | 145 +++++++----------
 drivers/staging/r8188eu/core/rtw_mlme.c    | 179 +++++++++------------
 drivers/staging/r8188eu/include/rtw_mlme.h |   4 +-
 3 files changed, 135 insertions(+), 193 deletions(-)

Signature · 1 line

2.30.2

Formally, you are removing the wrapped functions (or helpers, if you prefer) by moving their code into the wrappers. To say that you are removing the wrappers is not correct.

However, I'm sure this kind of formal mistake won't prevent your patches to be accepted. I wanted to point this out to you in case you decide to submit similar patches in the future and perhaps want to describe your changes a little more accurately.

Thanks,

Fabio M. De Francesco

Dan Carpenter

2022-04-12 05:07 UTC #11 lore raw

Quoted 22 lines · Fabio M. De Francesco

Quoted 21 lines · On lunedì 11 aprile 2022 12:21:29 CEST Vihas Makwana wrote:
Drop some unnecessary wrappers and update all the references accordingly.
Tested on Comfast CF-WU810N RTL8188EUS wireless adapter.
v1 -> v2:
Drop the wrapper functions with underscores prefixed.
Vihas Makwana (7):
staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv
staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv
staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network
staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd
 drivers/staging/r8188eu/core/rtw_cmd.c     | 145 +++++++----------
 drivers/staging/r8188eu/core/rtw_mlme.c    | 179 +++++++++------------
 drivers/staging/r8188eu/include/rtw_mlme.h |   4 +-
 3 files changed, 135 insertions(+), 193 deletions(-)

Quoted 6 lines

Quoted 3 lines
Signature · 1 line
2.30.2
Formally, you are removing the wrapped functions (or helpers, if you prefer) by moving their code into the wrappers. To say that you are removing the wrappers is not correct.

I once had someone make me re-write a commit message four time just as a kind of bullying and then at the end he was like, "You said NULL dereference instead of NULL pointer dereference so I had to re-write the commit message and I added some comment to the kernel git log explaining how you suck." So these days I have made it a rule that if you're going to complain about commit messages then you have to write your own for people to cut and paste. Otherwise people are like, "You're too stupid to read my mind. LOL. Do it again."

But in this case the commit message is fine. The key things with a commit message are:

1) What's the motivation
2) What's the effect for the user
3) Are all the surprising aspects are explained. Do I have enough information to review it quickly.

Removing wrappers is the motivation. No need to explain that further. No effects for the user.
There were no surprising bits.

It's fine.

regards,
dan carpenter

Fabio M. De Francesco

2022-04-12 09:53 UTC #12 lore raw

Quoted 30 lines · On martedì 12 aprile 2022 07:06:30 CEST Dan Carpenter wrote:

Quoted 22 lines · Fabio M. De Francesco
Quoted 21 lines · On lunedì 11 aprile 2022 12:21:29 CEST Vihas Makwana wrote:
Drop some unnecessary wrappers and update all the references accordingly.
Tested on Comfast CF-WU810N RTL8188EUS wireless adapter.
v1 -> v2:
Drop the wrapper functions with underscores prefixed.
Vihas Makwana (7):
staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv
staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv
staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv
staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network
staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd
 drivers/staging/r8188eu/core/rtw_cmd.c     | 145 +++++++----------
 drivers/staging/r8188eu/core/rtw_mlme.c    | 179 +++++++++------------
 drivers/staging/r8188eu/include/rtw_mlme.h |   4 +-
 3 files changed, 135 insertions(+), 193 deletions(-)
Quoted 6 lines
Quoted 3 lines
Signature · 1 line
2.30.2
Formally, you are removing the wrapped functions (or helpers, if you prefer) by moving their code into the wrappers. To say that you are removing the wrappers is not correct.

Quoted 9 lines

I once had someone make me re-write a commit message four time just as a kind of bullying and then at the end he was like, "You said NULL dereference instead of NULL pointer dereference so I had to re-write the commit message and I added some comment to the kernel git log explaining how you suck."
So these days I have made it a rule that if you're going
to complain about commit messages then you have to write your own for people to cut and paste. Otherwise people are like, "You're too stupid to read my mind. LOL. Do it again."

I'm assuming you don't believe that I attempted some kind of bullying similar to what you had to face when you sent the above-mentioned patch.

Nevertheless, I was expecting some sort of reaction from you but I think that you are misunderstanding what my intentions were.

I didn't suggest a re-write of the commit messages. I just pointed out that those messages are formally inaccurate but that these kinds of small formal inaccuracies won't ever prevent Vihas' patches from being accepted as they are.

It's only that, IMO, if people start to take care of being formally correct in drivers/staging, even when they do simple changes, they train their minds to improve their abilities to communicate in the future when they eventually decide to submit much more complex patches in other subsystems.

Quoted 8 lines

But in this case the commit message is fine. The key things with a commit message are:
1) What's the motivation
2) What's the effect for the user
3) Are all the surprising aspects are explained. Do I have enough information to review it quickly.

Quoted 5 lines

Removing wrappers is the motivation. No need to explain that further. No effects for the user.
There were no surprising bits.
It's fine.

I agree with you: "it's fine". I respect your opinion and your pragmatism:
after all, here, everything is pretty clear, so why bother?

However, writing accurate and formally correct commit messages takes the same amount of time of writing inaccurate messages. So, why not do better next time?

Again, I don't suggest to do so now. Not for these patches, because it would only be a loss of precious time.

Thanks,

Fabio M. De Francesco

Quoted 5 lines

regards,
dan carpenter

Dan Carpenter

2022-04-12 15:16 UTC #13 lore raw

Quoted 1 line · Fabio M. De Francesco

I didn't suggest a re-write of the commit messages.

Yes. That's the problem. If you want to complain about a commit message then you *should* suggest how you would re-write it. Otherwise how are we supposed to read your mind?

Quoted 2 lines

I just pointed out that those messages are formally inaccurate but that these kinds of small formal inaccuracies

I'm so puzzled but what you mean here. There were no "small formal inaccuracies". The patch removed unnecessary wrappers exactly as described. Anyway, I haven't seen your proposed commit message so I can't say.

regards,
dan carpenter

Fabio M. De Francesco

2022-04-12 16:08 UTC #14 lore raw

Quoted 6 lines · On marted? 12 aprile 2022 17:15:29 CEST Dan Carpenter wrote:

Quoted 1 line · Fabio M. De Francesco
I didn't suggest a re-write of the commit messages.
Yes. That's the problem. If you want to complain about a commit message then you *should* suggest how you would re-write it. Otherwise how are we supposed to read your mind?

Ah, sorry. I had missed that you were asking for a re-write (literally).

Here it is a sort of diff on Vihas' patch:

- [PATCH v2 1/7] staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv
+ [PATCH v2 1/7] staging: r8188eu: drop unnecessary helper _rtw_free_cmd_priv()

- Drop unnecessary wrapper _rtw_free_cmd_priv and move its logic to
- rtw_free_cmd_priv.
+ Drop unnecessary helper (wrapped function) _rtw_free_cmd_priv() and
+ move its code to the wrapper rtw_free_cmd_priv().

Quoted 6 lines

Quoted 2 lines
I just pointed out that those messages are formally inaccurate but that these kinds of small formal inaccuracies
I'm so puzzled but what you mean here. There were no "small formal inaccuracies". The patch removed unnecessary wrappers exactly as described.

The patch removed unnecessary helpers.

Quoted 5 lines

Anyway, I haven't seen your proposed commit message so I can't say.
regards,
dan carpenter

Thanks,

Fabio M. De Francesco

Dan Carpenter

2022-04-13 05:43 UTC #15 lore raw

Quoted 12 lines · Fabio M. De Francesco

Quoted 6 lines · On marted? 12 aprile 2022 17:15:29 CEST Dan Carpenter wrote:
Quoted 1 line · Fabio M. De Francesco
I didn't suggest a re-write of the commit messages.
Yes. That's the problem. If you want to complain about a commit message then you *should* suggest how you would re-write it. Otherwise how are we supposed to read your mind?
Ah, sorry. I had missed that you were asking for a re-write (literally).
Here it is a sort of diff on Vihas' patch:

Quoted 8 lines

- [PATCH v2 1/7] staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv
+ [PATCH v2 1/7] staging: r8188eu: drop unnecessary helper _rtw_free_cmd_priv()
- Drop unnecessary wrapper _rtw_free_cmd_priv and move its logic to
- rtw_free_cmd_priv.
+ Drop unnecessary helper (wrapped function) _rtw_free_cmd_priv() and
+ move its code to the wrapper rtw_free_cmd_priv().

I kind of feel like the original is better. Why would we want to remove helper functions? Helper functions are ok. What we don't like are pointless wrapper functions. The whole motivation for the patch is to remove wrapper functions and that's what it does.

regards,
dan carpenter

Vihas Makwana

author

2022-04-21 18:56 UTC #16 lore raw

Did this patch series get lost?
just pinging :)

Quoted 31 lines · Dan Carpenter

Quoted 12 lines · Fabio M. De Francesco
Quoted 6 lines · On marted? 12 aprile 2022 17:15:29 CEST Dan Carpenter wrote:
Quoted 1 line · Fabio M. De Francesco
I didn't suggest a re-write of the commit messages.
Yes. That's the problem. If you want to complain about a commit message then you *should* suggest how you would re-write it. Otherwise how are we supposed to read your mind?
Ah, sorry. I had missed that you were asking for a re-write (literally).
Here it is a sort of diff on Vihas' patch:
Quoted 8 lines
- [PATCH v2 1/7] staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv
+ [PATCH v2 1/7] staging: r8188eu: drop unnecessary helper _rtw_free_cmd_priv()
- Drop unnecessary wrapper _rtw_free_cmd_priv and move its logic to
- rtw_free_cmd_priv.
+ Drop unnecessary helper (wrapped function) _rtw_free_cmd_priv() and
+ move its code to the wrapper rtw_free_cmd_priv().
I kind of feel like the original is better. Why would we want to remove helper functions? Helper functions are ok. What we don't like are pointless wrapper functions. The whole motivation for the patch is to remove wrapper functions and that's what it does.
regards,
dan carpenter

Signature · 2 lines

Thanks,
Vihas

Dan Carpenter

2022-04-22 05:54 UTC #17 lore raw

Quoted 3 lines · Vihas Makwana

Did this patch series get lost?
just pinging :)

Wait for at least two weeks (not counting merge windows) before asking for feedback.

regards,
dan carpenter

Greg Kroah-Hartman

maintainer

2022-04-22 06:10 UTC #18 lore raw

In reply to Vihas Makwana

Quoted 1 line · Vihas Makwana

Did this patch series get lost?

What series? (hint, do not top post).

My queue of staging patches was empty as of a few days ago (it's filled up since then), so if they were not applied, please resend.

thanks,

greg k-h

Fabio M. De Francesco

2022-04-22 09:21 UTC #19 lore raw

Quoted 7 lines · On venerdì 22 aprile 2022 08:10:23 CEST Greg Kroah-Hartman wrote:

Quoted 1 line · Vihas Makwana
Did this patch series get lost?
What series? (hint, do not top post).
My queue of staging patches was empty as of a few days ago (it's filled up since then), so if they were not applied, please resend.

Are you also talking about re-sending patches for media/atomisp? I submitted three patches for that driver between April 8-12. They have already been reviewed and tested by Ira Weiny and Hans de Goede.

However, if I remember correctly, they should take a different path and it should be up to Mauro C. Chehab to apply them. Do you want them to be sent again?

Thanks,

Fabio M. De Francesco

Dan Carpenter

2022-04-22 09:29 UTC #20 lore raw

Quoted 10 lines · Fabio M. De Francesco

Quoted 7 lines · On venerdì 22 aprile 2022 08:10:23 CEST Greg Kroah-Hartman wrote:
Quoted 1 line · Vihas Makwana
Did this patch series get lost?
What series? (hint, do not top post).
My queue of staging patches was empty as of a few days ago (it's filled up since then), so if they were not applied, please resend.
Are you also talking about re-sending patches for media/atomisp?

Heh. No. No one was discussing your patches or anything to do with media. These are for staging: r8188eu.

Vihas, please put the subsystem prefix in the PATCH 0/7 email next time.

regards,
dan carpenter

Fabio M. De Francesco

2022-04-22 09:52 UTC #21 lore raw

Quoted 8 lines · On venerdì 22 aprile 2022 11:29:15 CEST Dan Carpenter wrote:

Quoted 7 lines · Fabio M. De Francesco
Quoted 6 lines · On venerdì 22 aprile 2022 08:10:23 CEST Greg Kroah-Hartman wrote:
Quoted 1 line · Vihas Makwana
Did this patch series get lost?
What series? (hint, do not top post).
My queue of staging patches was empty as of a few days ago (it's

filled

Quoted 6 lines

Quoted 3 lines
Quoted 1 line
up since then), so if they were not applied, please resend.
Are you also talking about re-sending patches for media/atomisp?
Heh. No. No one was discussing your patches or anything to do with media. These are for staging: r8188eu.

Dan, got it now :)

I was confused by Greg's words talking about "My queue of staging..." . Now you confirm that media/atomisp has never been in his queue.

Thanks,

Fabio M. De Francesco

Quoted 6 lines

Vihas, please put the subsystem prefix in the PATCH 0/7 email next time.
regards,
dan carpenter