Re: [PATCH v4] staging: rtl8723bs: Replace custom rtw_cbuf with kfifo

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


On Thu, Aug 6, 2026 at 10:22 AM Greg KH <[email protected]> wrote:
>
> On Wed, Aug 05, 2026 at 06:33:16PM +0000, Ashmit Kumar wrote:
> > 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.
> >
> > 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?
>
> > @@ -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