[PATCH 0/7] staging: r8188eu: another round of xmit cleanups

[PATCH 0/7] staging: r8188eu: another round of xmit cleanups

[Martin Kaiser]

This series contains some more cleanups for the xmit code.

Amongst other things, we can reduce the data for the completion function
of the usb bulk urbs that we send out.

Martin Kaiser (7):
  staging: r8188eu: merge do_queue_select into its only caller
  staging: r8188eu: simplify rtw_alloc_xmitframe
  staging: r8188eu: remove unused frametag defines
  staging: r8188eu: xmit_buf's ff_hwaddr is not used
  staging: r8188eu: simplify xmit_buf flags
  staging: r8188eu: simplify rtw_get_ff_hwaddr
  staging: r8188eu: bagg_pkt parameter is not used

 drivers/staging/r8188eu/core/rtw_xmit.c       | 85 +++++--------------
 drivers/staging/r8188eu/hal/rtl8188eu_xmit.c  |  8 +-
 drivers/staging/r8188eu/include/rtw_xmit.h    | 17 +---
 .../staging/r8188eu/os_dep/usb_ops_linux.c    | 25 +-----
 4 files changed, 27 insertions(+), 108 deletions(-)

-- 
2.30.2

[PATCH 1/7] staging: r8188eu: merge do_queue_select into its only caller

[Martin Kaiser]

Merge do_queue_select into its only caller. It's only a simple assignment.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_xmit.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c
index 18941320e70e..4d6210d89533 100644
--- a/drivers/staging/r8188eu/core/rtw_xmit.c
+++ b/drivers/staging/r8188eu/core/rtw_xmit.c
@@ -1609,15 +1609,6 @@ u32 rtw_get_ff_hwaddr(struct xmit_frame *pxmitframe)
 	return addr;
 }
 
-static void do_queue_select(struct adapter	*padapter, struct pkt_attrib *pattrib)
-{
-	u8 qsel;
-
-	qsel = pattrib->priority;
-
-	pattrib->qsel = qsel;
-}
-
 /*
  * The main transmit(tx) entry
  *
@@ -1656,7 +1647,7 @@ s32 rtw_xmit(struct adapter *padapter, struct sk_buff **ppkt)
 
 	rtw_led_control(padapter, LED_CTL_TX);
 
-	do_queue_select(padapter, &pxmitframe->attrib);
+	pxmitframe->attrib.qsel = pxmitframe->attrib.priority;
 
 	spin_lock_bh(&pxmitpriv->lock);
 	if (xmitframe_enqueue_for_sleeping_sta(padapter, pxmitframe)) {
-- 
2.30.2

[PATCH 2/7] staging: r8188eu: simplify rtw_alloc_xmitframe

[Martin Kaiser]

Make the rtw_alloc_xmitframe function a bit simpler.

The container_of() call never returns NULL. The if (pxframe) check is
false only if pfree_xmit_queue is empty. Handle this special case
explicitly and save one level of indentation.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_xmit.c | 40 +++++++++++--------------
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c
index 4d6210d89533..6ec342b726f9 100644
--- a/drivers/staging/r8188eu/core/rtw_xmit.c
+++ b/drivers/staging/r8188eu/core/rtw_xmit.c
@@ -1256,38 +1256,32 @@ struct xmit_frame *rtw_alloc_xmitframe(struct xmit_priv *pxmitpriv)/* _queue *pf
 
 	spin_lock_bh(&pfree_xmit_queue->lock);
 
-	if (list_empty(&pfree_xmit_queue->queue)) {
-		pxframe =  NULL;
-	} else {
-		phead = get_list_head(pfree_xmit_queue);
-
-		plist = phead->next;
+	if (list_empty(&pfree_xmit_queue->queue))
+		goto out;
 
-		pxframe = container_of(plist, struct xmit_frame, list);
-
-		list_del_init(&pxframe->list);
-	}
+	phead = get_list_head(pfree_xmit_queue);
+	plist = phead->next;
+	pxframe = container_of(plist, struct xmit_frame, list);
+	list_del_init(&pxframe->list);
 
-	if (pxframe) { /* default value setting */
-		pxmitpriv->free_xmitframe_cnt--;
+	pxmitpriv->free_xmitframe_cnt--;
 
-		pxframe->buf_addr = NULL;
-		pxframe->pxmitbuf = NULL;
+	pxframe->buf_addr = NULL;
+	pxframe->pxmitbuf = NULL;
 
-		memset(&pxframe->attrib, 0, sizeof(struct pkt_attrib));
-		/* pxframe->attrib.psta = NULL; */
+	memset(&pxframe->attrib, 0, sizeof(struct pkt_attrib));
+	/* pxframe->attrib.psta = NULL; */
 
-		pxframe->frame_tag = DATA_FRAMETAG;
+	pxframe->frame_tag = DATA_FRAMETAG;
 
-		pxframe->pkt = NULL;
-		pxframe->pkt_offset = 1;/* default use pkt_offset to fill tx desc */
+	pxframe->pkt = NULL;
+	pxframe->pkt_offset = 1;/* default use pkt_offset to fill tx desc */
 
-		pxframe->agg_num = 1;
-		pxframe->ack_report = 0;
-	}
+	pxframe->agg_num = 1;
+	pxframe->ack_report = 0;
 
+out:
 	spin_unlock_bh(&pfree_xmit_queue->lock);
-
 	return pxframe;
 }
 
-- 
2.30.2

[PATCH 3/7] staging: r8188eu: remove unused frametag defines

[Martin Kaiser]

Remove some frametag defines which are not used in the r8188eu driver.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/include/rtw_xmit.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/staging/r8188eu/include/rtw_xmit.h b/drivers/staging/r8188eu/include/rtw_xmit.h
index b332c2a86dbb..0c999ae6a1f1 100644
--- a/drivers/staging/r8188eu/include/rtw_xmit.h
+++ b/drivers/staging/r8188eu/include/rtw_xmit.h
@@ -152,14 +152,7 @@ struct pkt_attrib {
 
 #define NULL_FRAMETAG		(0x0)
 #define DATA_FRAMETAG		0x01
-#define L2_FRAMETAG		0x02
 #define MGNT_FRAMETAG		0x03
-#define AMSDU_FRAMETAG	0x04
-
-#define EII_FRAMETAG		0x05
-#define IEEE8023_FRAMETAG  0x06
-
-#define MP_FRAMETAG		0x07
 
 #define TXAGG_FRAMETAG	0x08
 
-- 
2.30.2

[PATCH 4/7] staging: r8188eu: xmit_buf's ff_hwaddr is not used

[Martin Kaiser]

ff_hwaddr in struct xmit_buf is not used. Remove it.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/include/rtw_xmit.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/r8188eu/include/rtw_xmit.h b/drivers/staging/r8188eu/include/rtw_xmit.h
index 0c999ae6a1f1..9a001fbf45a0 100644
--- a/drivers/staging/r8188eu/include/rtw_xmit.h
+++ b/drivers/staging/r8188eu/include/rtw_xmit.h
@@ -193,7 +193,6 @@ struct xmit_buf {
 	u32 alloc_sz;
 	u32  len;
 	struct submit_ctx *sctx;
-	u32	ff_hwaddr;
 	struct urb *pxmit_urb;
 	int last[8];
 };
-- 
2.30.2

[PATCH 5/7] staging: r8188eu: simplify xmit_buf flags

[Martin Kaiser]

rtw_write_port stores a queue index in pxmitbuf->flags before submitting
an urb. The urb completion function reads the flags. All it needs is the
info if the high queue was used or not.

We can replace the flags with a boolean high_queue variable.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_xmit.c       |  2 +-
 drivers/staging/r8188eu/include/rtw_xmit.h    |  2 +-
 .../staging/r8188eu/os_dep/usb_ops_linux.c    | 25 ++-----------------
 3 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c
index 6ec342b726f9..df88b3e29e77 100644
--- a/drivers/staging/r8188eu/core/rtw_xmit.c
+++ b/drivers/staging/r8188eu/core/rtw_xmit.c
@@ -148,7 +148,7 @@ int _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
 				goto free_xmitbuf;
 		}
 
-		pxmitbuf->flags = XMIT_VO_QUEUE;
+		pxmitbuf->high_queue = false;
 
 		list_add_tail(&pxmitbuf->list, &pxmitpriv->free_xmitbuf_queue.queue);
 		pxmitbuf++;
diff --git a/drivers/staging/r8188eu/include/rtw_xmit.h b/drivers/staging/r8188eu/include/rtw_xmit.h
index 9a001fbf45a0..feeac85aedb0 100644
--- a/drivers/staging/r8188eu/include/rtw_xmit.h
+++ b/drivers/staging/r8188eu/include/rtw_xmit.h
@@ -189,7 +189,7 @@ struct xmit_buf {
 	u8 *pbuf;
 	void *priv_data;
 	u16 ext_tag; /*  0: Normal xmitbuf, 1: extension xmitbuf. */
-	u16 flags;
+	bool high_queue;
 	u32 alloc_sz;
 	u32  len;
 	struct submit_ctx *sctx;
diff --git a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
index 48c96f731ce1..ca09f7ed7e4d 100644
--- a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
@@ -39,7 +39,7 @@ static void usb_write_port_complete(struct urb *purb)
 	struct adapter *padapter = pxmitbuf->padapter;
 	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
 
-	if (pxmitbuf->flags == HIGH_QUEUE_INX)
+	if (pxmitbuf->high_queue)
 		rtw_chk_hi_queue_cmd(padapter);
 
 	switch (purb->status) {
@@ -83,28 +83,7 @@ u32 rtw_write_port(struct adapter *padapter, u32 addr, u32 cnt, u8 *wmem)
 	}
 
 	spin_lock_irqsave(&pxmitpriv->lock, irqL);
-
-	switch (addr) {
-	case VO_QUEUE_INX:
-		pxmitbuf->flags = VO_QUEUE_INX;
-		break;
-	case VI_QUEUE_INX:
-		pxmitbuf->flags = VI_QUEUE_INX;
-		break;
-	case BE_QUEUE_INX:
-		pxmitbuf->flags = BE_QUEUE_INX;
-		break;
-	case BK_QUEUE_INX:
-		pxmitbuf->flags = BK_QUEUE_INX;
-		break;
-	case HIGH_QUEUE_INX:
-		pxmitbuf->flags = HIGH_QUEUE_INX;
-		break;
-	default:
-		pxmitbuf->flags = MGT_QUEUE_INX;
-		break;
-	}
-
+	pxmitbuf->high_queue = (addr == HIGH_QUEUE_INX);
 	spin_unlock_irqrestore(&pxmitpriv->lock, irqL);
 
 	purb	= pxmitbuf->pxmit_urb;
-- 
2.30.2

[PATCH 6/7] staging: r8188eu: simplify rtw_get_ff_hwaddr

[Martin Kaiser]

On both occassions where rtw_get_ff_hwaddr is called, the result is used
as addr parameter for rtw_write_port. rtw_write_port only needs the info
if the high queue was used or not. Simplify rtw_get_ff_hwaddr accordingly
and remove the now unused queue defines.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_xmit.c    | 32 +---------------------
 drivers/staging/r8188eu/include/rtw_xmit.h |  7 -----
 2 files changed, 1 insertion(+), 38 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c
index df88b3e29e77..d0761ac2e695 100644
--- a/drivers/staging/r8188eu/core/rtw_xmit.c
+++ b/drivers/staging/r8188eu/core/rtw_xmit.c
@@ -1568,39 +1568,9 @@ static int rtw_br_client_tx(struct adapter *padapter, struct sk_buff **pskb)
 
 u32 rtw_get_ff_hwaddr(struct xmit_frame *pxmitframe)
 {
-	u32 addr;
 	struct pkt_attrib *pattrib = &pxmitframe->attrib;
 
-	switch (pattrib->qsel) {
-	case 0:
-	case 3:
-		addr = BE_QUEUE_INX;
-		break;
-	case 1:
-	case 2:
-		addr = BK_QUEUE_INX;
-		break;
-	case 4:
-	case 5:
-		addr = VI_QUEUE_INX;
-		break;
-	case 6:
-	case 7:
-		addr = VO_QUEUE_INX;
-		break;
-	case 0x10:
-		addr = BCN_QUEUE_INX;
-		break;
-	case 0x11:/* BC/MC in PS (HIQ) */
-		addr = HIGH_QUEUE_INX;
-		break;
-	case 0x12:
-	default:
-		addr = MGT_QUEUE_INX;
-		break;
-	}
-
-	return addr;
+	return pattrib->qsel == 0x11 ? HIGH_QUEUE_INX : 0;
 }
 
 /*
diff --git a/drivers/staging/r8188eu/include/rtw_xmit.h b/drivers/staging/r8188eu/include/rtw_xmit.h
index feeac85aedb0..e4e5af198eee 100644
--- a/drivers/staging/r8188eu/include/rtw_xmit.h
+++ b/drivers/staging/r8188eu/include/rtw_xmit.h
@@ -26,14 +26,7 @@
 #define XMIT_BE_QUEUE		(2)
 #define XMIT_BK_QUEUE		(3)
 
-#define VO_QUEUE_INX		0
-#define VI_QUEUE_INX		1
-#define BE_QUEUE_INX		2
-#define BK_QUEUE_INX		3
-#define BCN_QUEUE_INX		4
-#define MGT_QUEUE_INX		5
 #define HIGH_QUEUE_INX		6
-#define TXCMD_QUEUE_INX		7
 
 #define HW_QUEUE_ENTRY		8
 
-- 
2.30.2

[PATCH 7/7] staging: r8188eu: bagg_pkt parameter is not used

[Martin Kaiser]

The bagg_pkt parameter in function update_txdesc is not used, it can be
removed.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/hal/rtl8188eu_xmit.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c b/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
index 5aa33fc4041d..3ffab4953a5c 100644
--- a/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
+++ b/drivers/staging/r8188eu/hal/rtl8188eu_xmit.c
@@ -137,7 +137,7 @@ static void fill_txdesc_phy(struct pkt_attrib *pattrib, __le32 *pdw)
 	}
 }
 
-static s32 update_txdesc(struct xmit_frame *pxmitframe, u8 *pmem, s32 sz, u8 bagg_pkt)
+static s32 update_txdesc(struct xmit_frame *pxmitframe, u8 *pmem, s32 sz)
 {
 	uint	qsel;
 	u8 data_rate, pwr_status, offset;
@@ -319,7 +319,7 @@ static s32 rtw_dump_xframe(struct adapter *adapt, struct xmit_frame *pxmitframe)
 			sz = pattrib->last_txcmdsz;
 		}
 
-		pull = update_txdesc(pxmitframe, mem_addr, sz, false);
+		pull = update_txdesc(pxmitframe, mem_addr, sz);
 
 		if (pull) {
 			mem_addr += PACKET_OFFSET_SZ; /* pull txdesc head */
@@ -489,7 +489,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt)
 		rtw_xmit_complete(adapt, pxmitframe);
 
 		/*  (len - TXDESC_SIZE) == pxmitframe->attrib.last_txcmdsz */
-		update_txdesc(pxmitframe, pxmitframe->buf_addr, pxmitframe->attrib.last_txcmdsz, true);
+		update_txdesc(pxmitframe, pxmitframe->buf_addr, pxmitframe->attrib.last_txcmdsz);
 
 		/*  don't need xmitframe any more */
 		rtw_free_xmitframe(pxmitpriv, pxmitframe);
@@ -529,7 +529,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt)
 		pfirstframe->pkt_offset--;
 	}
 
-	update_txdesc(pfirstframe, pfirstframe->buf_addr, pfirstframe->attrib.last_txcmdsz, true);
+	update_txdesc(pfirstframe, pfirstframe->buf_addr, pfirstframe->attrib.last_txcmdsz);
 
 	/* 3 4. write xmit buffer to USB FIFO */
 	ff_hwaddr = rtw_get_ff_hwaddr(pfirstframe);
-- 
2.30.2

Re: [PATCH 0/7] staging: r8188eu: another round of xmit cleanups

[Philipp Hortmann]

On 2/7/23 20:23, Martin Kaiser wrote:
> This series contains some more cleanups for the xmit code.
> 
> Amongst other things, we can reduce the data for the completion function
> of the usb bulk urbs that we send out.
> 
> Martin Kaiser (7):
>    staging: r8188eu: merge do_queue_select into its only caller
>    staging: r8188eu: simplify rtw_alloc_xmitframe
>    staging: r8188eu: remove unused frametag defines
>    staging: r8188eu: xmit_buf's ff_hwaddr is not used
>    staging: r8188eu: simplify xmit_buf flags
>    staging: r8188eu: simplify rtw_get_ff_hwaddr
>    staging: r8188eu: bagg_pkt parameter is not used
> 
>   drivers/staging/r8188eu/core/rtw_xmit.c       | 85 +++++--------------
>   drivers/staging/r8188eu/hal/rtl8188eu_xmit.c  |  8 +-
>   drivers/staging/r8188eu/include/rtw_xmit.h    | 17 +---
>   .../staging/r8188eu/os_dep/usb_ops_linux.c    | 25 +-----
>   4 files changed, 27 insertions(+), 108 deletions(-)
> 
Tested-by: Philipp Hortmann <philipp.g.hortmann@gmail.com> # Edimax N150