mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Luka Gejak" <luka.gejak@linux.dev>
To: "Ping-Ke Shih" <pkshih@realtek.com>,
	"Luka Gejak" <luka.gejak@linux.dev>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Michael Straube" <straube.linux@gmail.com>,
	"Bitterblue Smith" <rtl8821cerfe2@gmail.com>,
	"Peter Robinson" <pbrobinson@gmail.com>,
	"Hans de Goede" <johannes.goede@oss.qualcomm.com>
Subject: Re: [PATCH v9 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS
Date: Wed, 09 Sep 2026 10:04:07 +0200	[thread overview]
Message-ID: <DLAMOBQS6117.2OQCR1L2707BO@linux.dev> (raw)
In-Reply-To: <6a59823f69d140b1a6c44d475f5b93bd@realtek.com>

On Wed Sep 9, 2026 at 7:36 AM CEST, Ping-Ke Shih wrote:
>
> Luka Gejak <luka.gejak@linux.dev> wrote:
>>
[...]
>> On Sun Sep 6, 2026 at 4:22 AM CEST, Ping-Ke Shih wrote:
>>>
[...]
> Will/should you add this in v10 or v11 before your patch 4/6?
>
> As your v10 didn't get ran by NIPA because of [1], you can send v10 with
> RESEND or just send v11 (including the additional one).
>
> (To answer the ordering point question below.) I prefer to have the patch
> in advance, for two reasons. One is to backport easier, and another is to
> clear to know it is existing problem.
>
> [1] https://lore.kernel.org/linux-wireless/fe0d8a3c2951c051b321902582eaf95f230a754d.camel@sipsolutions.net/T/#u
>

I have decided to send v11, with the fix as patch 4, ahead of the accounting
patch. Your two reasons are better than mine, so I have dropped the plan to 
send it afterwards.

It is not gated on anything, so it does change behaviour for the other
five SDIO parts: the padding they send is zeroed now, and a transmit can
fail with -ENOMEM if the skb has to be reallocated. That is called out in
the Scope section rather than buried in the changelog.

One consequence worth flagging. The back-pressure patch stopped retrying
-ENOMEM in v10, because with no padding in the series nothing could
return it. It is reachable again now, so the retry is back. Without it a
stopped access category has nothing left to kick it. That patch is
therefore code identical to what you acked in v9; only its comments
differ, so I have carried your Acked-by.

[...]

> In v10, the beginning of rtw_sdio_write_port_8723bs() and
> rtw_sdio_write_port_generic become the similar. And
> I think ' txsize = round_up(skb->len, 4);' is not necessary to
> rtw_sdio_write_port_8723bs() because it is 512-byte aligned.
>
> Therefore, you can do the same thing in rtw_sdio_write_port() and then
> call 
>     rtw_sdio_write_port_8723bs()
> or
>     rtw_sdio_write_port_generic()
> by condition. 
>

Done. rtw_sdio_write_port() works out the transfer address and the
aligned size and passes both down, and the padding stays there too, so it
runs once for both paths and outside the credit mutex.

I kept the separate length in the RTL8723BS path, though, and I think it
is needed. It is not the transfer size, it feeds the page accounting:
rtw_sdio_check_free_txpg() and DIV_ROUND_UP(txsize, page_size), with a
128 byte page. The chip charges pages by the frame length rather than by
the padded transfer, which is what the vendor driver does, and the two
differ just above a block boundary:

  frame   pages by length   pages by padded size
   513          5                   8
  1025          9                  12

So using the aligned size there would charge up to three pages per frame
that the chip never took. There is a comment on the line now saying why.

[...]

> Will this be needed by the additional patch
>
>    "wifi: rtw88: sdio: zero the padding added to a TX transfer"?
>

Yes, with pad_size declared at the top, computed unconditionally, and tested
with if (pad_size > 0).

[...]

> I will check this by v11, because I'd like you can have v11 with
>
>   1. "wifi: rtw88: sdio: zero the padding added to a TX transfer"
>   2. move the same beginning of rtw_sdio_write_port_8723bs()
>      and rtw_sdio_write_port_generic() to rtw_sdio_write_port()
>
> Is it okay to you?

Both are in v11, so yes.

>
> Ping-Ke

Best regards,
Luka Gejak

  reply	other threads:[~2026-09-09  8:04 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31  6:11 [PATCH v9 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS luka.gejak
2026-08-31  6:11 ` [PATCH v9 1/6] wifi: rtw88: add the RTL8723B chip type and SDIO helper luka.gejak
2026-08-31  6:11 ` [PATCH v9 2/6] wifi: rtw88: rx: mark zero length packets on RTL8723BS luka.gejak
2026-08-31  6:11 ` [PATCH v9 3/6] wifi: rtw88: tx: extend the TX report purge timeout to RTL8723BS luka.gejak
2026-08-31  6:11 ` [PATCH v9 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS luka.gejak
2026-09-06  2:22   ` Ping-Ke Shih
2026-09-08 12:56     ` Luka Gejak
2026-09-09  5:36       ` Ping-Ke Shih
2026-09-09  8:04         ` Luka Gejak [this message]
2026-08-31  6:11 ` [PATCH v9 5/6] wifi: rtw88: sdio: set up RX aggregation and interrupts " luka.gejak
2026-08-31  6:11 ` [PATCH v9 6/6] wifi: rtw88: sdio: add TX back-pressure and retry on page starvation luka.gejak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DLAMOBQS6117.2OQCR1L2707BO@linux.dev \
    --to=luka.gejak@linux.dev \
    --cc=johannes.goede@oss.qualcomm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=pbrobinson@gmail.com \
    --cc=pkshih@realtek.com \
    --cc=rtl8821cerfe2@gmail.com \
    --cc=straube.linux@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®