RE: [PATCH v7 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS
Luka Gejak <[email protected]>
| Newsgroups | org.kernel.vger.linux-wireless,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On August 25, 2026 9:50:05 AM GMT+02:00, Ping-Ke Shih <[email protected]> wrote: >Luka Gejak <[email protected]> wrote: >> On August 25, 2026 9:25:13 AM GMT+02:00, Ping-Ke Shih <[email protected]> wrote: >> >Luka Gejak <[email protected]> wrote: >> >> Hi Ping-Ke, >> >> >> >> On August 25, 2026 8:12:58 AM GMT+02:00, Ping-Ke Shih <[email protected]> wrote: >> >> > guard(mutex)(&rtwsdio->tx_credit_lock); >> >> >> >> The lock is only taken for this chip, and guard() is unconditional, so I >> >> split the locked region into its own function instead of branching around >> >> the lock: >> >> >> >> if (!rtw_is_8723bs(rtwdev)) { >> >> txsize = sdio_align_size(rtwsdio->sdio_func, skb->len); >> >> >> >> ret = rtw_sdio_check_free_txpg(rtwdev, queue, txsize); >> >> if (ret) >> >> return ret; >> >> >> >> return rtw_sdio_write_to_port(rtwdev, skb, queue, txaddr, >> >> txsize); >> >> } >> >> ... >> >> guard(mutex)(&rtwsdio->tx_credit_lock); >> >> >> >> return rtw_sdio_8723bs_write_port(rtwdev, skb, queue, txaddr, txsize, >> >> write_size); >> > >> >I meant >> > >> > if (rtw_is_8723bs(rtwdev)) >> > guard(mutex)(&rtwsdio->tx_credit_lock); >> > >> >Doesn't it work? >> > >> > >> >> I tried it, and no. guard() expands to a declaration, and a declaration >> cannot be the body of an if, so it does not build: >> >> sdio.c: In function 'rtw_sdio_write_port': >> include/linux/cleanup.h:302:9: error: expected expression before >> 'class_mutex_t' >> 302 | class_##_name##_t var __cleanup(...) = >> >> Adding braces builds cleanly, which is the more dangerous version, because >> the cleanup then runs at the closing brace and the mutex is released again >> before any of the code it is meant to protect runs. >> >> That is why I moved the locked part into its own function: the guard needs >> a scope that is exactly the critical section, and an if body is either too >> small or not allowed. > >Okay. Got it. > > Do you mind answering this question, it seems like you missed it: The unaligned SKB warning now sits in both callers rather than once inside rtw_sdio_write_to_port(), because __func__ would otherwise report rtw_sdio_write_to_port for every other SDIO chip where it reports rtw_sdio_write_port today. Keeping that message identical costs three duplicated lines. Putting it once in rtw_sdio_write_to_port() is the tidier code and arguably the more accurate message, at the price of changing a log line on chips this series is not about. I went with the duplicate to leave the other parts alone, but I have no strong feeling either way, so say which you prefer.