Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230709195712.603200-1-martin.blumenstingl@googlemail.com>
 <b55cd3172ea7474ba1a67db2d5b39301@realtek.com> <87pm4w3rjp.fsf@mntre.com> <208ee32354b44205bb76a55c0d4bc93b@realtek.com>
In-Reply-To: <208ee32354b44205bb76a55c0d4bc93b@realtek.com>
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Wed, 26 Jul 2023 19:37:39 +0200
Message-ID: <CAFBinCDRWJGr1F0vLj_=k7TYQmGOQRWS=3n1DDQwpxr5AYtt9Q@mail.gmail.com>
Subject: Re: [PATCH] wifi: rtw88: sdio: Honor the host max_req_size in the RX path
To:     Ping-Ke Shih <pkshih@realtek.com>
Cc:     "Lukas F. Hartmann" <lukas@mntre.com>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "jernej.skrabec@gmail.com" <jernej.skrabec@gmail.com>,
        "ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
        "kvalo@kernel.org" <kvalo@kernel.org>,
        "tony0620emma@gmail.com" <tony0620emma@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hello Ping-Ke,

On Fri, Jul 14, 2023 at 2:34=E2=80=AFAM Ping-Ke Shih <pkshih@realtek.com> w=
rote:
[...]
> > Here are the errors that lead up to this:
> > http://dump.mntmn.com/rtw88-failure-1h-dmesg.txt
>
> Hi Martin,
>
> The dmesg shows that
> "rtw_8822cs mmc2:0001:1: Failed to read 1536 byte(s) from SDIO port 0x000=
000d1"
>
> Shouldn't we return an error code (with proper error handling) instead of
> just break the loop? Because 'buf' content isn't usable.
In my opinion we are properly breaking the loop:
"ret" will be non-zero so the error code is returned from
rtw_sdio_read_port() to the caller.
The (only) caller is rtw_sdio_rxfifo_recv() which sees the non-zero
return code and aborts processing.
What do you think?

> I wonder the approach of this patch is still not enough for Lukas' platfo=
rm.
On IRC Lukas wrote:
  funny, i can reproduce skb_panic when opening this page in chromium
https://embedded.avnet.com/product/msc-sm2s-ryz/
and:
  still getting spurious skb_panics, even after disabling rx aggregation.

I haven't had the time to look into this any further yet.
Unfortunately I also don't have any hardware to reproduce this problem
either, which unfortunately results in this long ping-pong.
Lukas, could you please add two more prints:
- in the rtw_warn with "Failed to read %zu byte(s) from SDIO port":
please also print the ret variable (with %d) - I'm curious what the
reported error is (it could be some CRC error which would mean ret is
-EILSEQ)
- add something like the following at the end of rtw_sdio_read_port()
(right before "return ret"):

if (!ret && count > 1000) {
    printk(KERN_INFO "rtw_sdio_read_port() with %zu bytes:", count);
    print_hex_dump(KERN_INFO, "", DUMP_PREFIX_OFFSET, 16, 1, buf, count, fa=
lse);
}

(note: I only compile-tested this)
The very last output of this (potentially spammy) output will contain
the full buffer that's causing the problem.


Best regards,
Martin