RE: [RFC PATCH v1 12/19] rtw88: sdio: Add HCI implementation for SDIO based chipsets

[Ping-Ke Shih]

> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
> Sent: Wednesday, December 28, 2022 8:00 PM
> To: Ping-Ke Shih <pkshih@xxxxxxxxxxx>
> Cc: linux-wireless@xxxxxxxxxxxxxxx; Yan-Hsuan Chuang <tony0620emma@xxxxxxxxx>; Kalle Valo
> <kvalo@xxxxxxxxxx>; Ulf Hansson <ulf.hansson@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; Chris Morgan <macroalpha82@xxxxxxxxx>; Nitin Gupta
> <nitin.gupta981@xxxxxxxxx>; Neo Jou <neojou@xxxxxxxxx>; Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> Subject: Re: [RFC PATCH v1 12/19] rtw88: sdio: Add HCI implementation for SDIO based chipsets
> 
> Hi Ping-Ke,
> 
> as always: thank you so much for taking time to go through this!
> 
> On Wed, Dec 28, 2022 at 10:39 AM Ping-Ke Shih <pkshih@xxxxxxxxxxx> wrote:

[...]

> > > +static void rtw_sdio_rx_isr(struct rtw_dev *rtwdev)
> > > +{
> > > +     u32 rx_len;
> > > +
> > > +     while (true) {
> >
> > add a limit to prevent infinite loop.
> Do you have any recommendations on how many packets to pull in one go?
> My thinking is: pulling to little data at once can hurt performance

This is to prevent unexpected things happen, and normally we receive/send
all packets at once like while(true) code. So, maybe we can have rough limit,
like 512 that would be enough for most cases. To have accurate number, you
can do experiments with the highest performance to see the loop count in
real usage, and 5 or 10 times count as limit.

However, when I reviewed rtw88 USB patches, I found we can't always break
the loop, because it could only one chance to free skb(s). So, you should
make sure we really can break the loop logically, and then decide which
limit is suitable for us.

> 
> [...]
> >
> > > +
> > > +static void rtw_sdio_process_tx_queue(struct rtw_dev *rtwdev,
> > > +                                   enum rtw_tx_queue_type queue)
> > > +{
> > > +     struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
> > > +     struct sk_buff *skb;
> > > +     int ret;
> > > +
> > > +     while (true) {
> >
> > Can we have a limit?
> Similar to the question above: do you have any recommendations on how
> many packets (per queue) to send in one go?
> 

Ping-Ke