RE: [PATCH v8 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS

[Ping-Ke Shih]

Luka Gejak <luka.gejak@xxxxxxxxx> wrote:
> Hi Ping-Ke,
> 
> On August 28, 2026 11:23:26 AM GMT+02:00, Ping-Ke Shih <pkshih@xxxxxxxxxxx> wrote:
> 
> > Since you have two branches for RTL8723BS and others, let's implement
> > them by individual functions. Here can just dispatch, like
> 
> Will do in v9.
> 
> Two details I would rather ask about than decide on my own, since both
> touch the other SDIO parts rather than this chip.
> 
> The unaligned SKB warning uses __func__, and today that resolves to
> rtw_sdio_write_port on every chip. Once the generic path moves into its
> own function the string becomes that function's name for 8703b, 8723d,
> 8821c, 8822b and 8822c. Nothing reads it, but it is still a visible
> change to parts this series is not about. The options I see are to let
> it change and say so in the change log, to drop __func__ and word the
> message without a function name, or to keep the check in the dispatcher
> so the string stays as it is.
> 
> I would take the first, since the name then simply follows whichever
> function ran. The third is the only one that leaves the other parts
> completely alone, but it would move the warning ahead of
> rtw_sdio_check_free_txpg() on the generic path, so a page shortage would
> start logging a line it does not log today. That seems worse than the
> string changing. Say if you would rather have it either of the other
> ways.

I think __func__ is only to address which path is running, so I don't
have bias. Take the one you like. 

However, if you have simple pseudo code as examples, it'd be easier
to taste.

I feel the third is also not bad:

int rtw_sdio_write_port()
{
    if (unaligned(skb->data))
        rtw_warn(rtwdev, "....");

    if (rtw_is_8723bs())
        rtw_sdio_write_port_8723bs()
    else
        rtw_sdio_write_port_generic()
}

> 
> > rtw_sdio_write_port_8723bs()
> 
> The chip specific helpers already use the _8723bs_ prefix, which is the
> convention you asked for in v4, so I plan to keep
> rtw_sdio_8723bs_write_port() and add rtw_sdio_write_port_common()
> next to it. Happy to rename them to match your sketch if you would
> rather they read that way.

At this moment, I prefer having similar naming for this series of
functions, so I propose rtw_sdio_write_port_* as prefix. 

By the way, using 'generic' seems more suitable than 'common' I proposed.