staging: rtl8723bs: Replace custom rtw_cbuf with kfifo

PATCH In review Ashmit Kumar 2 participants 3 messages Aug 5 – Aug 6

Subsystems (2)

Media (V4L/DVB) Staging

lore

Share & tools

lore

Ashmit Kumar

author

2026-08-05 18:33 UTC #0 lore raw

PATCH v4 1/1 staging: rtl8723bs: Replace custom rtw_cbuf with kfifo

The rtl8723bs driver implemented its own custom circular buffer (rtw_cbuf) for c2h event handling. The kernel already provides a standard, lockless circular buffer implementation in <linux/kfifo.h>.

This patch replaces the custom rtw_cbuf struct and its associated functions with the standard kfifo API (kfifo_alloc, kfifo_put, kfifo_get, kfifo_is_empty, kfifo_free), simplifying the driver code and relying on the robust kernel infrastructure. Furthermore, the allocation size is simplified to C2H_QUEUE_MAX_LEN, dropping the vestigial + 1 that the original naive ring buffer required to disambiguate full from empty.

Signed-off-by Ashmit Kumar <[email protected]>

---
Changes in v4:
- Abandoned ternary operator style fixes in favor of completely replacing the custom rtw_cbuf ring buffer with the standard Linux kfifo API, as suggested by Greg Kroah-Hartman.

Changes in v3:
- Formatted as a standalone patch (removed 1/3 series numbering).

Changes in v2:
- Fixed patch formatting and versioning issues.

Changes in v1:
- Initial submission addressing ternary operator style issues in rtw_cbuf.

 drivers/staging/rtl8723bs/core/rtw_cmd.c      | 16 ++--
 drivers/staging/rtl8723bs/hal/sdio_ops.c      |  2 +-
 .../staging/rtl8723bs/include/osdep_service.h | 14 ----
 drivers/staging/rtl8723bs/include/rtw_cmd.h   |  3 +-
 .../staging/rtl8723bs/os_dep/osdep_service.c  | 77 -------------------
 5 files changed, 11 insertions(+), 101 deletions(-)

drivers/staging/rtl8723bs/core/rtw_cmd.c +8 -8

index ce3dfa1fee26..424aa180eecc 100644
@@ -202,8 +202,7 @@ int rtw_init_evt_priv(struct evt_priv *pevtpriv)
 
 	_init_workitem(&pevtpriv->c2h_wk, c2h_wk_callback, NULL);
 	pevtpriv->c2h_wk_alive = false;
-	pevtpriv->c2h_queue = rtw_cbuf_alloc(C2H_QUEUE_MAX_LEN + 1);
-	if (!pevtpriv->c2h_queue)
+	if (kfifo_alloc(&pevtpriv->c2h_queue, C2H_QUEUE_MAX_LEN, GFP_KERNEL))
 		return -ENOMEM;
 
 	return 0;

@@ -211,17 +210,17 @@ int rtw_init_evt_priv(struct evt_priv *pevtpriv)
 
 void _rtw_free_evt_priv(struct	evt_priv *pevtpriv)
 {
+	void *c2h;
+
 	_cancel_workitem_sync(&pevtpriv->c2h_wk);
 	while (pevtpriv->c2h_wk_alive)
 		fsleep(10 * USEC_PER_MSEC);
 
-	while (!rtw_cbuf_empty(pevtpriv->c2h_queue)) {
-		void *c2h = rtw_cbuf_pop(pevtpriv->c2h_queue);
-
+	while (kfifo_get(&pevtpriv->c2h_queue, &c2h)) {
 		if (c2h && c2h != (void *)pevtpriv)
 			kfree(c2h);
 	}
-	kfree(pevtpriv->c2h_queue);
+	kfifo_free(&pevtpriv->c2h_queue);
 }
 
 void _rtw_free_cmd_priv(struct	cmd_priv *pcmdpriv)

@@ -1695,12 +1694,13 @@ static void c2h_wk_callback(struct work_struct *work)
 	struct evt_priv *evtpriv = container_of(work, struct evt_priv, c2h_wk);
 	struct adapter *adapter = container_of(evtpriv, struct adapter, evtpriv);
 	u8 *c2h_evt;
+	void *c2h_ptr;
 	c2h_id_filter ccx_id_filter = rtw_hal_c2h_id_filter_ccx(adapter);
 
 	evtpriv->c2h_wk_alive = true;
 
-	while (!rtw_cbuf_empty(evtpriv->c2h_queue)) {
-		c2h_evt = (u8 *)rtw_cbuf_pop(evtpriv->c2h_queue);
+	while (kfifo_get(&evtpriv->c2h_queue, &c2h_ptr)) {
+		c2h_evt = (u8 *)c2h_ptr;
 		if (c2h_evt) {
 			/* This C2H event is read, clear it */
 			c2h_evt_clear(adapter);

drivers/staging/rtl8723bs/hal/sdio_ops.c +1 -1

index da2d9088ab5a..e39faed1f195 100644
@@ -763,7 +763,7 @@ void sd_int_dpc(struct adapter *adapter)
 			}
 		} else {
 			/* Error handling for malloc fail */
-			rtw_cbuf_push(adapter->evtpriv.c2h_queue, NULL);
+			kfifo_put(&adapter->evtpriv.c2h_queue, NULL);
 			_set_workitem(&adapter->evtpriv.c2h_wk);
 		}
 	}

drivers/staging/rtl8723bs/include/osdep_service.h +0 ~~-14~~

index 2f5011a8210c..6feeb1351faa 100644
@@ -36,18 +36,4 @@ extern void rtw_free_netdev(struct net_device *netdev);
 void rtw_buf_free(u8 **buf, u32 *buf_len);
 void rtw_buf_update(u8 **buf, u32 *buf_len, u8 *src, u32 src_len);
 
-struct rtw_cbuf {
-	u32 write;
-	u32 read;
-	u32 size;
-	void *bufs[];
-};
-
-bool rtw_cbuf_full(struct rtw_cbuf *cbuf);
-bool rtw_cbuf_empty(struct rtw_cbuf *cbuf);
-bool rtw_cbuf_push(struct rtw_cbuf *cbuf, void *buf);
-void *rtw_cbuf_pop(struct rtw_cbuf *cbuf);
-struct rtw_cbuf *rtw_cbuf_alloc(u32 size);
-
-
 #endif

drivers/staging/rtl8723bs/include/rtw_cmd.h +2 -1

index 2e791da7e815..62e6b9011625 100644
@@ -8,6 +8,7 @@
 #define __RTW_CMD_H_
 
 #include <linux/completion.h>
+#include <linux/kfifo.h>
 
 #define C2H_MEM_SZ (16*1024)

@@ -58,7 +59,7 @@
 	struct	evt_priv {
 		struct work_struct c2h_wk;
 		bool c2h_wk_alive;
-		struct rtw_cbuf *c2h_queue;
+		DECLARE_KFIFO_PTR(c2h_queue, void *);
 		#define C2H_QUEUE_MAX_LEN 10
 
 		atomic_t event_seq;

drivers/staging/rtl8723bs/os_dep/osdep_service.c +0 ~~-77~~

80 lines not on this page · raw message

Signature · 1 line

2.51.0.windows.1

Greg KH

maintainer

2026-08-06 04:52 UTC #1 lore raw

Quoted 11 lines · Ashmit Kumar

The rtl8723bs driver implemented its own custom circular buffer (rtw_cbuf) for c2h event handling. The kernel already provides a standard, lockless circular buffer implementation in <linux/kfifo.h>.
This patch replaces the custom rtw_cbuf struct and its associated functions with the standard kfifo API (kfifo_alloc, kfifo_put, kfifo_get, kfifo_is_empty, kfifo_free), simplifying the driver code and relying on the robust kernel infrastructure. Furthermore, the allocation size is simplified to C2H_QUEUE_MAX_LEN, dropping the vestigial + 1 that the original naive ring buffer required to disambiguate full from empty.

Quoted 7 lines

Suggested-by Greg Kroah-Hartman <[email protected]>
Signed-off-by Ashmit Kumar <[email protected]>
---
Changes in v4:
- Abandoned ternary operator style fixes in favor of completely replacing the custom rtw_cbuf ring buffer with the standard Linux kfifo API, as suggested by Greg Kroah-Hartman.

Great, how was this tested?

drivers/staging/rtl8723bs/include/rtw_cmd.h line 62

Quoted 6 lines

drivers/staging/rtl8723bs/include/rtw_cmd.h +1 -1

@@ -58,7 +59,7 @@
 	struct	evt_priv {
 		struct work_struct c2h_wk;
 		bool c2h_wk_alive;
-		struct rtw_cbuf *c2h_queue;
+		DECLARE_KFIFO_PTR(c2h_queue, void *);

Why did you loose the type of the pointer? Was a LLM used to create this change?

thanks,

greg k-h

Ashmit Kumar

author

2026-08-06 10:55 UTC #2 lore raw

Hi Greg,

Regarding the pointer type, no type information was actually lost. The original struct rtw_cbuf was implemented with an untyped void *bufs[] array, and I used void * in DECLARE_KFIFO_PTR to preserve that exact behavior 1:1.

The technical reason the original code used void * is that the consumer (c2h_wk_callback) expects the queue to multiplex three different untyped pointer states -

A valid u8 * payload buffer.
A special sentinel pointer (void *)evtpriv used to trigger c2h_evt_clear. A NULL pointer.

I traced every push into c2h_queue across the codebase. Today, there is exactly one reachable push site (hal/sdio_ops.c:766), which exclusively pushes NULL to signal that the interrupt handler failed to allocate memory. However, because the consumer logic still explicitly branches on all three pointer states, preserving void * was the safest way to swap the ring buffer without rewriting the underlying event state machine.

And to answer your second question, no LLM was used to create this change. I analyzed the driver's event queue and wrote the migration manually.

If you prefer, I can submit a v5 that changes the kfifo type to u8 * and explicitly casts the sentinel, or I can submit a follow-up patch that rips out the unreachable consumer branches entirely.

Thanks,
Ashmit Kumar

Quoted 36 lines · Greg KH

Quoted 11 lines · Ashmit Kumar
The rtl8723bs driver implemented its own custom circular buffer (rtw_cbuf) for c2h event handling. The kernel already provides a standard, lockless circular buffer implementation in <linux/kfifo.h>.
This patch replaces the custom rtw_cbuf struct and its associated functions with the standard kfifo API (kfifo_alloc, kfifo_put, kfifo_get, kfifo_is_empty, kfifo_free), simplifying the driver code and relying on the robust kernel infrastructure. Furthermore, the allocation size is simplified to C2H_QUEUE_MAX_LEN, dropping the vestigial + 1 that the original naive ring buffer required to disambiguate full from empty.
Quoted 7 lines
Suggested-by Greg Kroah-Hartman <[email protected]>
Signed-off-by Ashmit Kumar <[email protected]>
---
Changes in v4:
- Abandoned ternary operator style fixes in favor of completely replacing the custom rtw_cbuf ring buffer with the standard Linux kfifo API, as suggested by Greg Kroah-Hartman.
Great, how was this tested?
drivers/staging/rtl8723bs/include/rtw_cmd.h line 62
Quoted 6 lines
drivers/staging/rtl8723bs/include/rtw_cmd.h +1 -1
@@ -58,7 +59,7 @@
      struct  evt_priv {
              struct work_struct c2h_wk;
              bool c2h_wk_alive;
-             struct rtw_cbuf *c2h_queue;
+             DECLARE_KFIFO_PTR(c2h_queue, void *);
Why did you loose the type of the pointer? Was a LLM used to create this change?
thanks,
greg k-h