2023-04-30 17:00:53

by Larry Finger

[permalink] [raw]
Subject: Driver for rtw8723ds

Jernej,

Is there a reason that the driver for RTW8723DS was not included along with the
other SDIO versions in rtw88?

I have a user of one of my GitHub repos that uses this device.

Thanks,

Larry


2023-05-02 19:34:55

by Jernej Škrabec

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

Hi Larry,

/cc Martin

Dne nedelja, 30. april 2023 ob 18:51:15 CEST je Larry Finger napisal(a):
> Jernej,
>
> Is there a reason that the driver for RTW8723DS was not included along with
> the other SDIO versions in rtw88?

We have no HW to test.

>
> I have a user of one of my GitHub repos that uses this device.

Can you ask that user to test following commits?
https://github.com/xdarklight/linux/commit/
3866a7a3702f7f24557f2c065b7d4088f7027466
https://github.com/xdarklight/linux/commit/
66fd078556a6bf246337270b2e91d73c079fce2d

Patches are trivial, but some testing needs to be done to confirm the driver
actually works as intended.

Best regards,
Jernej

>
> Thanks,
>
> Larry




2023-05-04 19:43:32

by Larry Finger

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

On 5/2/23 14:27, Jernej Škrabec wrote:
> Hi Larry,
>
> /cc Martin
>
> Dne nedelja, 30. april 2023 ob 18:51:15 CEST je Larry Finger napisal(a):
>> Jernej,
>>
>> Is there a reason that the driver for RTW8723DS was not included along with
>> the other SDIO versions in rtw88?
>
> We have no HW to test.
>
>>
>> I have a user of one of my GitHub repos that uses this device.
>
> Can you ask that user to test following commits?
> https://github.com/xdarklight/linux/commit/
> 3866a7a3702f7f24557f2c065b7d4088f7027466
> https://github.com/xdarklight/linux/commit/
> 66fd078556a6bf246337270b2e91d73c079fce2d
>
> Patches are trivial, but some testing needs to be done to confirm the driver
> actually works as intended.

Jernej,

The user needs the rtw8723ds driver - the SDIO equivalent of rtw8723du.c that is
used by the USB device. The riscv changes may be needed, but we are not quite
that far yet.

Larry


2023-05-04 19:52:03

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

Hi Larry,

On Thu, May 4, 2023 at 9:25 PM Larry Finger <[email protected]> wrote:
[...]
> > Can you ask that user to test following commits?
> > https://github.com/xdarklight/linux/commit/
> > 3866a7a3702f7f24557f2c065b7d4088f7027466
> > https://github.com/xdarklight/linux/commit/
> > 66fd078556a6bf246337270b2e91d73c079fce2d
> >
> > Patches are trivial, but some testing needs to be done to confirm the driver
> > actually works as intended.
>
> Jernej,
>
> The user needs the rtw8723ds driver - the SDIO equivalent of rtw8723du.c that is
> used by the USB device. The riscv changes may be needed, but we are not quite
> that far yet.
Strange, can you please elaborate what you are seeing in terms of riscv?
The two commits that Jernej shared are for an update to rtw8723d.c [0]
and the addition of a rtw8723ds driver [1]. None of these mentions
riscv (at least that's what my tired eyes are seeing today).


Best regards,
Martin


[0] https://github.com/xdarklight/linux/commit/3866a7a3702f7f24557f2c065b7d4088f7027466
[1] https://github.com/xdarklight/linux/commit/66fd078556a6bf246337270b2e91d73c079fce2d

2023-05-04 20:47:42

by Jernej Škrabec

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

Dne četrtek, 04. maj 2023 ob 21:45:31 CEST je Martin Blumenstingl napisal(a):
> Hi Larry,
>
> On Thu, May 4, 2023 at 9:25 PM Larry Finger <[email protected]>
> wrote: [...]
>
> > > Can you ask that user to test following commits?
> > > https://github.com/xdarklight/linux/commit/
> > > 3866a7a3702f7f24557f2c065b7d4088f7027466
> > > https://github.com/xdarklight/linux/commit/
> > > 66fd078556a6bf246337270b2e91d73c079fce2d
> > >
> > > Patches are trivial, but some testing needs to be done to confirm the
> > > driver actually works as intended.
> >
> > Jernej,
> >
> > The user needs the rtw8723ds driver - the SDIO equivalent of rtw8723du.c
> > that is used by the USB device. The riscv changes may be needed, but we
> > are not quite that far yet.
>
> Strange, can you please elaborate what you are seeing in terms of riscv?
> The two commits that Jernej shared are for an update to rtw8723d.c [0]
> and the addition of a rtw8723ds driver [1]. None of these mentions
> riscv (at least that's what my tired eyes are seeing today).
>
>
> Best regards,
> Martin
>
>
> [0]
> https://github.com/xdarklight/linux/commit/3866a7a3702f7f24557f2c065b7d4088
> f7027466 [1]
> https://github.com/xdarklight/linux/commit/66fd078556a6bf246337270b2e91d73c
> 079fce2d

Problem is that my e-mail client split commit links in two parts. If you
combine hashes with rest it opens correct commits. Currently, it opens head of
your master branch, which is by chance risc-v merge commit.

Best regards,
Jernej


2023-05-04 21:07:35

by Larry Finger

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

On 5/4/23 15:13, Jernej Škrabec wrote:
> Dne četrtek, 04. maj 2023 ob 21:45:31 CEST je Martin Blumenstingl napisal(a):
>> Hi Larry,
>>
>> On Thu, May 4, 2023 at 9:25 PM Larry Finger <[email protected]>
>> wrote: [...]
>>
>>>> Can you ask that user to test following commits?
>>>> https://github.com/xdarklight/linux/commit/
>>>> 3866a7a3702f7f24557f2c065b7d4088f7027466
>>>> https://github.com/xdarklight/linux/commit/
>>>> 66fd078556a6bf246337270b2e91d73c079fce2d
>>>>
>>>> Patches are trivial, but some testing needs to be done to confirm the
>>>> driver actually works as intended.
>>>
>>> Jernej,
>>>
>>> The user needs the rtw8723ds driver - the SDIO equivalent of rtw8723du.c
>>> that is used by the USB device. The riscv changes may be needed, but we
>>> are not quite that far yet.
>>
>> Strange, can you please elaborate what you are seeing in terms of riscv?
>> The two commits that Jernej shared are for an update to rtw8723d.c [0]
>> and the addition of a rtw8723ds driver [1]. None of these mentions
>> riscv (at least that's what my tired eyes are seeing today).
>>
>>
>> Best regards,
>> Martin
>>
>>
>> [0]
>> https://github.com/xdarklight/linux/commit/3866a7a3702f7f24557f2c065b7d4088
>> f7027466 [1]
>> https://github.com/xdarklight/linux/commit/66fd078556a6bf246337270b2e91d73c
>> 079fce2d
>
> Problem is that my e-mail client split commit links in two parts. If you
> combine hashes with rest it opens correct commits. Currently, it opens head of
> your master branch, which is by chance risc-v merge commit.

After I compared the links that Martin sent with yours, I figured out the problem.

I downloaded the patches and applied them to my rtw88 repo. The user that wanted
rtw8723ds support has been notified. We will see how it goes.

Thanks,

Larry


2023-05-08 18:59:43

by Larry Finger

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

Martin and Jernej,

It took a bit of working through some problems, but the user now has the
rtw8723ds loading and starting. The first problem other than the mechanics of
building and installing the driver on his SoC was that there is a second SDIO
vendor ID/device ID for his device, namely 0x024c:0xd724. As I am carrying a
local copy of sdio_ids.h, that was relatively easy to fix.

At the moment, when the device starts, the log shows the following:
[ 3.640866] rtw_8723ds mmc1:0001:1: Firmware version 48.0.0, H2C version 0
[ 3.645302] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
[ 3.654556] sunxi-mmc 4021000.mmc: map DMA failed
[ 3.659268] rtw_8723ds mmc1:0001:1: sdio read16 failed (0x10040): -22
[ 3.665710] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
[ 3.672499] sunxi-mmc 4021000.mmc: map DMA failed
[ 3.677208] rtw_8723ds mmc1:0001:1: sdio write16 failed (0x10040): -22
[ 3.683739] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
[ 3.690528] sunxi-mmc 4021000.mmc: map DMA failed
[ 3.695236] rtw_8723ds mmc1:0001:1: sdio write16 failed (0x10100): -22
[ 3.873173] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
[ 3.879978] sunxi-mmc 4021000.mmc: map DMA failed
[ 3.884690] rtw_8723ds mmc1:0001:1: sdio read16 failed (0x10002): -22
[ 3.891141] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
[ 3.897931] sunxi-mmc 4021000.mmc: map DMA failed
[ 3.902640] rtw_8723ds mmc1:0001:1: sdio write16 failed (0x10002): -22
[ 3.909172] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
[ 3.915962] sunxi-mmc 4021000.mmc: map DMA failed
[ 3.920672] rtw_8723ds mmc1:0001:1: sdio read16 failed (0x10008): -22
[ 3.927116] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
[ 3.933906] sunxi-mmc 4021000.mmc: map DMA failed
[ 3.938616] rtw_8723ds mmc1:0001:1: sdio write16 failed (0x10008): -22
[ ok ]

This problem is beyond my knowledge of SDIO.

Thanks for any help you can give.

Larry

2023-05-08 20:29:10

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

Hi Larry,

On Mon, May 8, 2023 at 8:55 PM Larry Finger <[email protected]> wrote:
>
> Martin and Jernej,
>
> It took a bit of working through some problems, but the user now has the
> rtw8723ds loading and starting. The first problem other than the mechanics of
> building and installing the driver on his SoC was that there is a second SDIO
> vendor ID/device ID for his device, namely 0x024c:0xd724. As I am carrying a
> local copy of sdio_ids.h, that was relatively easy to fix.
Thanks for this hint - I guess we need to find out how to name that
0xd724 device and add it's ID to sdio_ids.h when I upstream the
patches then.
But let's look at the other problem first.

> At the moment, when the device starts, the log shows the following:
> [ 3.640866] rtw_8723ds mmc1:0001:1: Firmware version 48.0.0, H2C version 0
> [ 3.645302] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
> [ 3.654556] sunxi-mmc 4021000.mmc: map DMA failed
> [ 3.659268] rtw_8723ds mmc1:0001:1: sdio read16 failed (0x10040): -22
> [ 3.665710] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
> [ 3.672499] sunxi-mmc 4021000.mmc: map DMA failed
> [ 3.677208] rtw_8723ds mmc1:0001:1: sdio write16 failed (0x10040): -22
> [ 3.683739] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
> [ 3.690528] sunxi-mmc 4021000.mmc: map DMA failed
> [ 3.695236] rtw_8723ds mmc1:0001:1: sdio write16 failed (0x10100): -22
> [ 3.873173] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
> [ 3.879978] sunxi-mmc 4021000.mmc: map DMA failed
> [ 3.884690] rtw_8723ds mmc1:0001:1: sdio read16 failed (0x10002): -22
> [ 3.891141] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
> [ 3.897931] sunxi-mmc 4021000.mmc: map DMA failed
> [ 3.902640] rtw_8723ds mmc1:0001:1: sdio write16 failed (0x10002): -22
> [ 3.909172] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
> [ 3.915962] sunxi-mmc 4021000.mmc: map DMA failed
> [ 3.920672] rtw_8723ds mmc1:0001:1: sdio read16 failed (0x10008): -22
> [ 3.927116] sunxi-mmc 4021000.mmc: unaligned scatterlist: os b00 length 2
> [ 3.933906] sunxi-mmc 4021000.mmc: map DMA failed
> [ 3.938616] rtw_8723ds mmc1:0001:1: sdio write16 failed (0x10008): -22
This looks like an issue with the Allwinner SDIO controller. We can
try to work around this with the attached patch.
Please note that I've only compile-tested that patch.
In theory it can hurt 16-bit register access (read/write) performance
a bit (since we now require two MMC commands instead of one). Whether
this can be measured in the real world is unknown to me. Let's see if
it fixes the observed issue first.


Best regards,
Martin


Attachments:
rtw88-byte-wise-word-io.diff (870.00 B)

2023-05-09 22:17:34

by Larry Finger

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

On 5/8/23 15:16, Martin Blumenstingl wrote:
> This looks like an issue with the Allwinner SDIO controller. We can
> try to work around this with the attached patch.
> Please note that I've only compile-tested that patch.
> In theory it can hurt 16-bit register access (read/write) performance
> a bit (since we now require two MMC commands instead of one). Whether
> this can be measured in the real world is unknown to me. Let's see if
> it fixes the observed issue first.
>

Martin,

I added that patch to the driver. The user reports that he was able to do a ping
and an nslookup before it crashed with the following in the log:

[ 8.700626] skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
head:ffff000003b3c000 data:ffff000003b3c040 tail:0xd4d end:0x2c0 dev:<NULL>
[ 8.714254] ------------[ cut here ]------------
[ 8.718867] kernel BUG at net/core/skbuff.c:200!
[ 8.723481] Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
[ 8.730261] Modules linked in: rtw_8723ds rtw_8723d rtw_sdio rtw_core
mac80211 libarc4 cfg80211 rfkill ipv6
[ 8.740024] CPU: 1 PID: 222 Comm: ksdioirqd/mmc1 Not tainted 6.4.0-rc1-dirty #1
[ 8.747327] Hardware name: MangoPi MQ Quad (DT)
[ 8.751852] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 8.758808] pc : skb_panic+0x58/0x5c
[ 8.762391] lr : skb_panic+0x58/0x5c
[ 8.765964] sp : ffff80000aa3bc20
[ 8.769274] x29: ffff80000aa3bc30 x28: ffff000003e16dd8 x27: 0000000000000055
[ 8.776409] x26: 000000000000e001 x25: ffff000003e94200 x24: ffff80000119f408
[ 8.783543] x23: 0000000000000018 x22: ffff000003e12060 x21: ffff80000aa3bd28
[ 8.790677] x20: 000000000000003a x19: ffff000003e94200 x18: 0000000000000000
[ 8.797811] x17: 010006000cc00100 x16: ffff80000a22ffc0 x15: 0000000000000030
[ 8.804945] x14: ffff80000a241b20 x13: 0000000000000339 x12: 0000000000000113
[ 8.812079] x11: 7265766f5f626b73 x10: ffff80000a299b20 x9 : 00000000fffff000
[ 8.819212] x8 : ffff80000a241b20 x7 : ffff80000a299b20 x6 : 0000000000000000
[ 8.826346] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000
[ 8.833479] x2 : 0000000000000000 x1 : ffff000003d6d880 x0 : 000000000000008b
[ 8.840614] Call trace:
[ 8.843057] skb_panic+0x58/0x5c
[ 8.846286] skb_find_text+0x0/0xc4
[ 8.849773] rtw_sdio_rx_skb+0x50/0xc8 [rtw_sdio]
[ 8.854488] rtw_sdio_rxfifo_recv+0x1a0/0x24c [rtw_sdio]
[ 8.859803] rtw_sdio_handle_interrupt+0xf0/0x124 [rtw_sdio]
[ 8.865465] process_sdio_pending_irqs+0x5c/0x1c4
[ 8.870169] sdio_irq_thread+0x84/0x178
[ 8.874003] kthread+0x118/0x11c
[ 8.877231] ret_from_fork+0x10/0x20
[ 8.880814] Code: f90007e9 b940b108 f90003e8 97cb9ac2 (d4210000)
[ 8.886903] ---[ end trace 0000000000000000 ]---
[ 8.891515] note: ksdioirqd/mmc1[222] exited with irqs disabled
[ 8.897478] note: ksdioirqd/mmc1[222] exited with preempt_count 1
[ 8.903732] ------------[ cut here ]------------
[ 8.908348] WARNING: CPU: 1 PID: 0 at kernel/context_tracking.c:128
ct_kernel_exit.constprop.0+0x98/0xa0
[ 8.917828] Modules linked in: rtw_8723ds rtw_8723d rtw_sdio rtw_core
mac80211 libarc4 cfg80211 rfkill ipv6
[ 8.927590] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G D
6.4.0-rc1-dirty #1
[ 8.935758] Hardware name: MangoPi MQ Quad (DT)
[ 8.940282] pstate: 200003c5 (nzCv DAIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 8.947237] pc : ct_kernel_exit.constprop.0+0x98/0xa0
[ 8.952285] lr : ct_idle_enter+0x10/0x1c
[ 8.956206] sp : ffff80000a76bdd0
[ 8.959516] x29: ffff80000a76bdd0 x28: 0000000000000000 x27: 0000000000000000
[ 8.966650] x26: 0000000000000000 x25: ffff000002d68000 x24: 0000000000000000
[ 8.973784] x23: 0000000000000000 x22: ffff80000a229b48 x21: ffff800009b8daf8
[ 8.980917] x20: ffff80000a229a40 x19: ffff00003fd95a20 x18: 0000000000000000
[ 8.988051] x17: 3830303030303030 x16: 3030303030303020 x15: 0000000000000001
[ 8.995185] x14: 00000000000000cb x13: 0000000000000000 x12: 0000000000000001
[ 9.002317] x11: 0000000000000000 x10: 0000000000000a60 x9 : ffff80000a76bd30
[ 9.009452] x8 : ffff000002d68ac0 x7 : 0000000000000000 x6 : 00000000106f30a4
[ 9.016584] x5 : 00ffffffffffffff x4 : 4000000000000002 x3 : ffff80000a76bdd0
[ 9.023717] x2 : 4000000000000000 x1 : ffff800009b8ba20 x0 : ffff800009b8ba20
[ 9.030851] Call trace:
[ 9.033295] ct_kernel_exit.constprop.0+0x98/0xa0
[ 9.037998] ct_idle_enter+0x10/0x1c
[ 9.041572] default_idle_call+0x1c/0x3c
[ 9.045494] do_idle+0x214/0x270
[ 9.048724] cpu_startup_entry+0x24/0x2c
[ 9.052646] secondary_start_kernel+0x130/0x154
[ 9.057179] __secondary_switched+0xb8/0xbc
[ 9.061363] ---[ end trace 0000000000000000 ]---
[ 14.048126] platform leds: deferred probe pending

Somehow skb->tail was greater than skb->end. Unfortunately I do not have access
to gdb to tell you what line corresponds to rtw_sdio_rx_skb+0x50 on the MangoPi
MQ Quad.

Larry


2023-05-10 21:25:44

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

On Wed, May 10, 2023 at 12:02 AM Larry Finger <[email protected]> wrote:
[...]
> I added that patch to the driver. The user reports that he was able to do a ping
> and an nslookup before it crashed with the following in the log:
That's some positive news alongside the crash log: it seems that a
part of the driver works! :-)

> [ 8.700626] skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
> head:ffff000003b3c000 data:ffff000003b3c040 tail:0xd4d end:0x2c0 dev:<NULL>
[...]
> Somehow skb->tail was greater than skb->end. Unfortunately I do not have access
> to gdb to tell you what line corresponds to rtw_sdio_rx_skb+0x50 on the MangoPi
> MQ Quad.
I need to have a closer look at the pkg_offset and struct
rtw_rx_pkt_stat which we receive.
Recently my own MangoPI MQ-Quad arrived but I did not have the time to
set it up yet. I'll try to do so during the weekend so I can debug
this on my own.

Please ping me next week in case I haven't provided any update until then.


Best regards,
Martin

2023-05-10 21:49:47

by Larry Finger

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

On 5/10/23 16:07, Martin Blumenstingl wrote:
> On Wed, May 10, 2023 at 12:02 AM Larry Finger <[email protected]> wrote:
> [...]
>> I added that patch to the driver. The user reports that he was able to do a ping
>> and an nslookup before it crashed with the following in the log:
> That's some positive news alongside the crash log: it seems that a
> part of the driver works! :-)
>
>> [ 8.700626] skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
>> head:ffff000003b3c000 data:ffff000003b3c040 tail:0xd4d end:0x2c0 dev:<NULL>
> [...]
>> Somehow skb->tail was greater than skb->end. Unfortunately I do not have access
>> to gdb to tell you what line corresponds to rtw_sdio_rx_skb+0x50 on the MangoPi
>> MQ Quad.
> I need to have a closer look at the pkg_offset and struct
> rtw_rx_pkt_stat which we receive.
> Recently my own MangoPI MQ-Quad arrived but I did not have the time to
> set it up yet. I'll try to do so during the weekend so I can debug
> this on my own.
>
> Please ping me next week in case I haven't provided any update until then.

I have some test prints in to check for skb overrun. My initial indication is
that the problem was in the c2h branch of rtw_sdio_rx_skb(), but my next run
should verify that. My changes will do a pr_warn_once() when the problem
happens, and then drop the skb.

My contact reported that he had one run of 3 minutes before the problem
happened, which is good news for most of the driver.

Larry



2023-05-13 10:32:20

by Jernej Škrabec

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

Larry,

Dne sreda, 10. maj 2023 ob 23:47:24 CEST je Larry Finger napisal(a):
> On 5/10/23 16:07, Martin Blumenstingl wrote:
> > On Wed, May 10, 2023 at 12:02 AM Larry Finger <[email protected]> wrote:
> > [...]
> >> I added that patch to the driver. The user reports that he was able to do a ping
> >> and an nslookup before it crashed with the following in the log:
> > That's some positive news alongside the crash log: it seems that a
> > part of the driver works! :-)
> >
> >> [ 8.700626] skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
> >> head:ffff000003b3c000 data:ffff000003b3c040 tail:0xd4d end:0x2c0 dev:<NULL>
> > [...]
> >> Somehow skb->tail was greater than skb->end. Unfortunately I do not have access
> >> to gdb to tell you what line corresponds to rtw_sdio_rx_skb+0x50 on the MangoPi
> >> MQ Quad.
> > I need to have a closer look at the pkg_offset and struct
> > rtw_rx_pkt_stat which we receive.
> > Recently my own MangoPI MQ-Quad arrived but I did not have the time to
> > set it up yet. I'll try to do so during the weekend so I can debug
> > this on my own.
> >
> > Please ping me next week in case I haven't provided any update until then.
>
> I have some test prints in to check for skb overrun. My initial indication is
> that the problem was in the c2h branch of rtw_sdio_rx_skb(), but my next run
> should verify that. My changes will do a pr_warn_once() when the problem
> happens, and then drop the skb.
>
> My contact reported that he had one run of 3 minutes before the problem
> happened, which is good news for most of the driver.

I may have discovered something interesting. rtl8723ds vendor driver has
following checks in RX data parsing code:
https://github.com/lwfinger/rtl8723ds/blob/master/hal/rtl8723d/sdio/rtl8723ds_recv.c#L83-L99

Those checks are absent in rtl8822bs vendor driver, which was my original
development platform for SDIO. This may indicate some kind of bug in FW
and/or HW.

I think that at least second check, which checks for exactly the case your
client experience, can be easily added and tested.

Best regards,
Jernej




2023-05-13 20:18:03

by Larry Finger

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

On 5/13/23 05:23, Jernej Škrabec wrote:
> Larry,
>
> Dne sreda, 10. maj 2023 ob 23:47:24 CEST je Larry Finger napisal(a):
>> On 5/10/23 16:07, Martin Blumenstingl wrote:
>>> On Wed, May 10, 2023 at 12:02 AM Larry Finger <[email protected]> wrote:
>>> [...]
>>>> I added that patch to the driver. The user reports that he was able to do a ping
>>>> and an nslookup before it crashed with the following in the log:
>>> That's some positive news alongside the crash log: it seems that a
>>> part of the driver works! :-)
>>>
>>>> [ 8.700626] skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
>>>> head:ffff000003b3c000 data:ffff000003b3c040 tail:0xd4d end:0x2c0 dev:<NULL>
>>> [...]
>>>> Somehow skb->tail was greater than skb->end. Unfortunately I do not have access
>>>> to gdb to tell you what line corresponds to rtw_sdio_rx_skb+0x50 on the MangoPi
>>>> MQ Quad.
>>> I need to have a closer look at the pkg_offset and struct
>>> rtw_rx_pkt_stat which we receive.
>>> Recently my own MangoPI MQ-Quad arrived but I did not have the time to
>>> set it up yet. I'll try to do so during the weekend so I can debug
>>> this on my own.
>>>
>>> Please ping me next week in case I haven't provided any update until then.
>>
>> I have some test prints in to check for skb overrun. My initial indication is
>> that the problem was in the c2h branch of rtw_sdio_rx_skb(), but my next run
>> should verify that. My changes will do a pr_warn_once() when the problem
>> happens, and then drop the skb.
>>
>> My contact reported that he had one run of 3 minutes before the problem
>> happened, which is good news for most of the driver.
>
> I may have discovered something interesting. rtl8723ds vendor driver has
> following checks in RX data parsing code:
> https://github.com/lwfinger/rtl8723ds/blob/master/hal/rtl8723d/sdio/rtl8723ds_recv.c#L83-L99
>
> Those checks are absent in rtl8822bs vendor driver, which was my original
> development platform for SDIO. This may indicate some kind of bug in FW
> and/or HW.
>
> I think that at least second check, which checks for exactly the case your
> client experience, can be easily added and tested.

Thanks for this update. I added the following to the start of rtw_sdio_rx_skb():
/* fix Hardware RX data error, drop whole recv_buffer */
if (!(rtwdev->hal.rcr & BIT_ACRC32) && pkt_stat->crc_err) {
kfree_skb(skb);
return;
}
I think that duplicates the code in the vendor driver.

I have not heard from my user as to whether it helps. My communications with him
are at https://github.com/lwfinger/rtl8723ds/issues/37.

Larry



2023-05-13 20:18:32

by Jernej Škrabec

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

Dne sobota, 13. maj 2023 ob 21:55:51 CEST je Larry Finger napisal(a):
> On 5/13/23 05:23, Jernej Škrabec wrote:
> > Larry,
> >
> > Dne sreda, 10. maj 2023 ob 23:47:24 CEST je Larry Finger napisal(a):
> >> On 5/10/23 16:07, Martin Blumenstingl wrote:
> >>> On Wed, May 10, 2023 at 12:02 AM Larry Finger <[email protected]> wrote:
> >>> [...]
> >>>> I added that patch to the driver. The user reports that he was able to do a ping
> >>>> and an nslookup before it crashed with the following in the log:
> >>> That's some positive news alongside the crash log: it seems that a
> >>> part of the driver works! :-)
> >>>
> >>>> [ 8.700626] skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
> >>>> head:ffff000003b3c000 data:ffff000003b3c040 tail:0xd4d end:0x2c0 dev:<NULL>
> >>> [...]
> >>>> Somehow skb->tail was greater than skb->end. Unfortunately I do not have access
> >>>> to gdb to tell you what line corresponds to rtw_sdio_rx_skb+0x50 on the MangoPi
> >>>> MQ Quad.
> >>> I need to have a closer look at the pkg_offset and struct
> >>> rtw_rx_pkt_stat which we receive.
> >>> Recently my own MangoPI MQ-Quad arrived but I did not have the time to
> >>> set it up yet. I'll try to do so during the weekend so I can debug
> >>> this on my own.
> >>>
> >>> Please ping me next week in case I haven't provided any update until then.
> >>
> >> I have some test prints in to check for skb overrun. My initial indication is
> >> that the problem was in the c2h branch of rtw_sdio_rx_skb(), but my next run
> >> should verify that. My changes will do a pr_warn_once() when the problem
> >> happens, and then drop the skb.
> >>
> >> My contact reported that he had one run of 3 minutes before the problem
> >> happened, which is good news for most of the driver.
> >
> > I may have discovered something interesting. rtl8723ds vendor driver has
> > following checks in RX data parsing code:
> > https://github.com/lwfinger/rtl8723ds/blob/master/hal/rtl8723d/sdio/rtl8723ds_recv.c#L83-L99
> >
> > Those checks are absent in rtl8822bs vendor driver, which was my original
> > development platform for SDIO. This may indicate some kind of bug in FW
> > and/or HW.
> >
> > I think that at least second check, which checks for exactly the case your
> > client experience, can be easily added and tested.
>
> Thanks for this update. I added the following to the start of rtw_sdio_rx_skb():
> /* fix Hardware RX data error, drop whole recv_buffer */
> if (!(rtwdev->hal.rcr & BIT_ACRC32) && pkt_stat->crc_err) {
> kfree_skb(skb);
> return;
> }
> I think that duplicates the code in the vendor driver.
>
> I have not heard from my user as to whether it helps. My communications with him
> are at https://github.com/lwfinger/rtl8723ds/issues/37.

I had second part in mind (see attachment), but this is IMO only sanity check
and it will mask the issue. At this point I'm not sure if this is something that
can happen occasionally or is there additional bug in rtw88 code. I'll check
rtl8723ds c2h code in greater detail.

In any case, I would argue that all 3 patches in this thread are valid and
should be submitted upstream.

Best regards,
Jernej


Attachments:
sdio-size-check.patch (845.00 B)

2023-05-13 21:38:08

by Larry Finger

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

On 5/13/23 15:13, Jernej Škrabec wrote:
> Dne sobota, 13. maj 2023 ob 21:55:51 CEST je Larry Finger napisal(a):
>> On 5/13/23 05:23, Jernej Škrabec wrote:
>>> Larry,
>>>
>>> Dne sreda, 10. maj 2023 ob 23:47:24 CEST je Larry Finger napisal(a):
>>>> On 5/10/23 16:07, Martin Blumenstingl wrote:
>>>>> On Wed, May 10, 2023 at 12:02 AM Larry Finger <[email protected]> wrote:
>>>>> [...]
>>>>>> I added that patch to the driver. The user reports that he was able to do a ping
>>>>>> and an nslookup before it crashed with the following in the log:
>>>>> That's some positive news alongside the crash log: it seems that a
>>>>> part of the driver works! :-)
>>>>>
>>>>>> [ 8.700626] skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
>>>>>> head:ffff000003b3c000 data:ffff000003b3c040 tail:0xd4d end:0x2c0 dev:<NULL>
>>>>> [...]
>>>>>> Somehow skb->tail was greater than skb->end. Unfortunately I do not have access
>>>>>> to gdb to tell you what line corresponds to rtw_sdio_rx_skb+0x50 on the MangoPi
>>>>>> MQ Quad.
>>>>> I need to have a closer look at the pkg_offset and struct
>>>>> rtw_rx_pkt_stat which we receive.
>>>>> Recently my own MangoPI MQ-Quad arrived but I did not have the time to
>>>>> set it up yet. I'll try to do so during the weekend so I can debug
>>>>> this on my own.
>>>>>
>>>>> Please ping me next week in case I haven't provided any update until then.
>>>>
>>>> I have some test prints in to check for skb overrun. My initial indication is
>>>> that the problem was in the c2h branch of rtw_sdio_rx_skb(), but my next run
>>>> should verify that. My changes will do a pr_warn_once() when the problem
>>>> happens, and then drop the skb.
>>>>
>>>> My contact reported that he had one run of 3 minutes before the problem
>>>> happened, which is good news for most of the driver.
>>>
>>> I may have discovered something interesting. rtl8723ds vendor driver has
>>> following checks in RX data parsing code:
>>> https://github.com/lwfinger/rtl8723ds/blob/master/hal/rtl8723d/sdio/rtl8723ds_recv.c#L83-L99
>>>
>>> Those checks are absent in rtl8822bs vendor driver, which was my original
>>> development platform for SDIO. This may indicate some kind of bug in FW
>>> and/or HW.
>>>
>>> I think that at least second check, which checks for exactly the case your
>>> client experience, can be easily added and tested.
>>
>> Thanks for this update. I added the following to the start of rtw_sdio_rx_skb():
>> /* fix Hardware RX data error, drop whole recv_buffer */
>> if (!(rtwdev->hal.rcr & BIT_ACRC32) && pkt_stat->crc_err) {
>> kfree_skb(skb);
>> return;
>> }
>> I think that duplicates the code in the vendor driver.
>>
>> I have not heard from my user as to whether it helps. My communications with him
>> are at https://github.com/lwfinger/rtl8723ds/issues/37.
>
> I had second part in mind (see attachment), but this is IMO only sanity check
> and it will mask the issue. At this point I'm not sure if this is something that
> can happen occasionally or is there additional bug in rtw88 code. I'll check
> rtl8723ds c2h code in greater detail.
>
> In any case, I would argue that all 3 patches in this thread are valid and
> should be submitted upstream.

That patch looks good. I ill apply it to my rtw88 repo.

Larry



2023-05-15 16:45:14

by Jernej Škrabec

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

Dne sobota, 13. maj 2023 ob 23:21:47 CEST je Larry Finger napisal(a):
> On 5/13/23 15:13, Jernej Škrabec wrote:
> > Dne sobota, 13. maj 2023 ob 21:55:51 CEST je Larry Finger napisal(a):
> >> On 5/13/23 05:23, Jernej Škrabec wrote:
> >>> Larry,
> >>>
> >>> Dne sreda, 10. maj 2023 ob 23:47:24 CEST je Larry Finger napisal(a):
> >>>> On 5/10/23 16:07, Martin Blumenstingl wrote:
> >>>>> On Wed, May 10, 2023 at 12:02 AM Larry Finger <[email protected]> wrote:
> >>>>> [...]
> >>>>>> I added that patch to the driver. The user reports that he was able to do a ping
> >>>>>> and an nslookup before it crashed with the following in the log:
> >>>>> That's some positive news alongside the crash log: it seems that a
> >>>>> part of the driver works! :-)
> >>>>>
> >>>>>> [ 8.700626] skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
> >>>>>> head:ffff000003b3c000 data:ffff000003b3c040 tail:0xd4d end:0x2c0 dev:<NULL>
> >>>>> [...]
> >>>>>> Somehow skb->tail was greater than skb->end. Unfortunately I do not have access
> >>>>>> to gdb to tell you what line corresponds to rtw_sdio_rx_skb+0x50 on the MangoPi
> >>>>>> MQ Quad.
> >>>>> I need to have a closer look at the pkg_offset and struct
> >>>>> rtw_rx_pkt_stat which we receive.
> >>>>> Recently my own MangoPI MQ-Quad arrived but I did not have the time to
> >>>>> set it up yet. I'll try to do so during the weekend so I can debug
> >>>>> this on my own.
> >>>>>
> >>>>> Please ping me next week in case I haven't provided any update until then.
> >>>>
> >>>> I have some test prints in to check for skb overrun. My initial indication is
> >>>> that the problem was in the c2h branch of rtw_sdio_rx_skb(), but my next run
> >>>> should verify that. My changes will do a pr_warn_once() when the problem
> >>>> happens, and then drop the skb.
> >>>>
> >>>> My contact reported that he had one run of 3 minutes before the problem
> >>>> happened, which is good news for most of the driver.
> >>>
> >>> I may have discovered something interesting. rtl8723ds vendor driver has
> >>> following checks in RX data parsing code:
> >>> https://github.com/lwfinger/rtl8723ds/blob/master/hal/rtl8723d/sdio/rtl8723ds_recv.c#L83-L99
> >>>
> >>> Those checks are absent in rtl8822bs vendor driver, which was my original
> >>> development platform for SDIO. This may indicate some kind of bug in FW
> >>> and/or HW.
> >>>
> >>> I think that at least second check, which checks for exactly the case your
> >>> client experience, can be easily added and tested.
> >>
> >> Thanks for this update. I added the following to the start of rtw_sdio_rx_skb():
> >> /* fix Hardware RX data error, drop whole recv_buffer */
> >> if (!(rtwdev->hal.rcr & BIT_ACRC32) && pkt_stat->crc_err) {
> >> kfree_skb(skb);
> >> return;
> >> }
> >> I think that duplicates the code in the vendor driver.
> >>
> >> I have not heard from my user as to whether it helps. My communications with him
> >> are at https://github.com/lwfinger/rtl8723ds/issues/37.
> >
> > I had second part in mind (see attachment), but this is IMO only sanity check
> > and it will mask the issue. At this point I'm not sure if this is something that
> > can happen occasionally or is there additional bug in rtw88 code. I'll check
> > rtl8723ds c2h code in greater detail.
> >
> > In any case, I would argue that all 3 patches in this thread are valid and
> > should be submitted upstream.
>
> That patch looks good. I ill apply it to my rtw88 repo.

I see that issue is still there. Next idea would be to check if RX aggregation
is the problem. Just commenting out call to rtw_sdio_enable_rx_aggregation()
is enough to disable it.

Best regards,
Jernej




2023-05-15 20:32:43

by Larry Finger

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

On 5/15/23 11:43, Jernej Škrabec wrote:
> Dne sobota, 13. maj 2023 ob 23:21:47 CEST je Larry Finger napisal(a):
>> On 5/13/23 15:13, Jernej Škrabec wrote:
>>> Dne sobota, 13. maj 2023 ob 21:55:51 CEST je Larry Finger napisal(a):
>>>> On 5/13/23 05:23, Jernej Škrabec wrote:
>>>>> Larry,
>>>>>
>>>>> Dne sreda, 10. maj 2023 ob 23:47:24 CEST je Larry Finger napisal(a):
>>>>>> On 5/10/23 16:07, Martin Blumenstingl wrote:
>>>>>>> On Wed, May 10, 2023 at 12:02 AM Larry Finger <[email protected]> wrote:
>>>>>>> [...]
>>>>>>>> I added that patch to the driver. The user reports that he was able to do a ping
>>>>>>>> and an nslookup before it crashed with the following in the log:
>>>>>>> That's some positive news alongside the crash log: it seems that a
>>>>>>> part of the driver works! :-)
>>>>>>>
>>>>>>>> [ 8.700626] skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
>>>>>>>> head:ffff000003b3c000 data:ffff000003b3c040 tail:0xd4d end:0x2c0 dev:<NULL>
>>>>>>> [...]
>>>>>>>> Somehow skb->tail was greater than skb->end. Unfortunately I do not have access
>>>>>>>> to gdb to tell you what line corresponds to rtw_sdio_rx_skb+0x50 on the MangoPi
>>>>>>>> MQ Quad.
>>>>>>> I need to have a closer look at the pkg_offset and struct
>>>>>>> rtw_rx_pkt_stat which we receive.
>>>>>>> Recently my own MangoPI MQ-Quad arrived but I did not have the time to
>>>>>>> set it up yet. I'll try to do so during the weekend so I can debug
>>>>>>> this on my own.
>>>>>>>
>>>>>>> Please ping me next week in case I haven't provided any update until then.
>>>>>>
>>>>>> I have some test prints in to check for skb overrun. My initial indication is
>>>>>> that the problem was in the c2h branch of rtw_sdio_rx_skb(), but my next run
>>>>>> should verify that. My changes will do a pr_warn_once() when the problem
>>>>>> happens, and then drop the skb.
>>>>>>
>>>>>> My contact reported that he had one run of 3 minutes before the problem
>>>>>> happened, which is good news for most of the driver.
>>>>>
>>>>> I may have discovered something interesting. rtl8723ds vendor driver has
>>>>> following checks in RX data parsing code:
>>>>> https://github.com/lwfinger/rtl8723ds/blob/master/hal/rtl8723d/sdio/rtl8723ds_recv.c#L83-L99
>>>>>
>>>>> Those checks are absent in rtl8822bs vendor driver, which was my original
>>>>> development platform for SDIO. This may indicate some kind of bug in FW
>>>>> and/or HW.
>>>>>
>>>>> I think that at least second check, which checks for exactly the case your
>>>>> client experience, can be easily added and tested.
>>>>
>>>> Thanks for this update. I added the following to the start of rtw_sdio_rx_skb():
>>>> /* fix Hardware RX data error, drop whole recv_buffer */
>>>> if (!(rtwdev->hal.rcr & BIT_ACRC32) && pkt_stat->crc_err) {
>>>> kfree_skb(skb);
>>>> return;
>>>> }
>>>> I think that duplicates the code in the vendor driver.
>>>>
>>>> I have not heard from my user as to whether it helps. My communications with him
>>>> are at https://github.com/lwfinger/rtl8723ds/issues/37.
>>>
>>> I had second part in mind (see attachment), but this is IMO only sanity check
>>> and it will mask the issue. At this point I'm not sure if this is something that
>>> can happen occasionally or is there additional bug in rtw88 code. I'll check
>>> rtl8723ds c2h code in greater detail.
>>>
>>> In any case, I would argue that all 3 patches in this thread are valid and
>>> should be submitted upstream.
>>
>> That patch looks good. I ill apply it to my rtw88 repo.
>
> I see that issue is still there. Next idea would be to check if RX aggregation
> is the problem. Just commenting out call to rtw_sdio_enable_rx_aggregation()
> is enough to disable it.

Jernej,

With aggregation disabled, we still get "Invalid RX packet size!" messages. I am
changing the statement to log (curr_pkt_len + pkt_desc_sz) > rx_len. I will let
you know when the OP responds.

Larry



2023-05-15 20:49:36

by Jernej Škrabec

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

Dne ponedeljek, 15. maj 2023 ob 22:23:39 CEST je Larry Finger napisal(a):
> On 5/15/23 11:43, Jernej Škrabec wrote:
> > Dne sobota, 13. maj 2023 ob 23:21:47 CEST je Larry Finger napisal(a):
> >> On 5/13/23 15:13, Jernej Škrabec wrote:
> >>> Dne sobota, 13. maj 2023 ob 21:55:51 CEST je Larry Finger napisal(a):
> >>>> On 5/13/23 05:23, Jernej Škrabec wrote:
> >>>>> Larry,
> >>>>>
> >>>>> Dne sreda, 10. maj 2023 ob 23:47:24 CEST je Larry Finger napisal(a):
> >>>>>> On 5/10/23 16:07, Martin Blumenstingl wrote:
> >>>>>>> On Wed, May 10, 2023 at 12:02 AM Larry Finger <[email protected]> wrote:
> >>>>>>> [...]
> >>>>>>>> I added that patch to the driver. The user reports that he was able to do a ping
> >>>>>>>> and an nslookup before it crashed with the following in the log:
> >>>>>>> That's some positive news alongside the crash log: it seems that a
> >>>>>>> part of the driver works! :-)
> >>>>>>>
> >>>>>>>> [ 8.700626] skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
> >>>>>>>> head:ffff000003b3c000 data:ffff000003b3c040 tail:0xd4d end:0x2c0 dev:<NULL>
> >>>>>>> [...]
> >>>>>>>> Somehow skb->tail was greater than skb->end. Unfortunately I do not have access
> >>>>>>>> to gdb to tell you what line corresponds to rtw_sdio_rx_skb+0x50 on the MangoPi
> >>>>>>>> MQ Quad.
> >>>>>>> I need to have a closer look at the pkg_offset and struct
> >>>>>>> rtw_rx_pkt_stat which we receive.
> >>>>>>> Recently my own MangoPI MQ-Quad arrived but I did not have the time to
> >>>>>>> set it up yet. I'll try to do so during the weekend so I can debug
> >>>>>>> this on my own.
> >>>>>>>
> >>>>>>> Please ping me next week in case I haven't provided any update until then.
> >>>>>>
> >>>>>> I have some test prints in to check for skb overrun. My initial indication is
> >>>>>> that the problem was in the c2h branch of rtw_sdio_rx_skb(), but my next run
> >>>>>> should verify that. My changes will do a pr_warn_once() when the problem
> >>>>>> happens, and then drop the skb.
> >>>>>>
> >>>>>> My contact reported that he had one run of 3 minutes before the problem
> >>>>>> happened, which is good news for most of the driver.
> >>>>>
> >>>>> I may have discovered something interesting. rtl8723ds vendor driver has
> >>>>> following checks in RX data parsing code:
> >>>>> https://github.com/lwfinger/rtl8723ds/blob/master/hal/rtl8723d/sdio/rtl8723ds_recv.c#L83-L99
> >>>>>
> >>>>> Those checks are absent in rtl8822bs vendor driver, which was my original
> >>>>> development platform for SDIO. This may indicate some kind of bug in FW
> >>>>> and/or HW.
> >>>>>
> >>>>> I think that at least second check, which checks for exactly the case your
> >>>>> client experience, can be easily added and tested.
> >>>>
> >>>> Thanks for this update. I added the following to the start of rtw_sdio_rx_skb():
> >>>> /* fix Hardware RX data error, drop whole recv_buffer */
> >>>> if (!(rtwdev->hal.rcr & BIT_ACRC32) && pkt_stat->crc_err) {
> >>>> kfree_skb(skb);
> >>>> return;
> >>>> }
> >>>> I think that duplicates the code in the vendor driver.
> >>>>
> >>>> I have not heard from my user as to whether it helps. My communications with him
> >>>> are at https://github.com/lwfinger/rtl8723ds/issues/37.
> >>>
> >>> I had second part in mind (see attachment), but this is IMO only sanity check
> >>> and it will mask the issue. At this point I'm not sure if this is something that
> >>> can happen occasionally or is there additional bug in rtw88 code. I'll check
> >>> rtl8723ds c2h code in greater detail.
> >>>
> >>> In any case, I would argue that all 3 patches in this thread are valid and
> >>> should be submitted upstream.
> >>
> >> That patch looks good. I ill apply it to my rtw88 repo.
> >
> > I see that issue is still there. Next idea would be to check if RX aggregation
> > is the problem. Just commenting out call to rtw_sdio_enable_rx_aggregation()
> > is enough to disable it.
>
> Jernej,
>
> With aggregation disabled, we still get "Invalid RX packet size!" messages. I am
> changing the statement to log (curr_pkt_len + pkt_desc_sz) > rx_len. I will let
> you know when the OP responds.

Yeah, I saw. I just find another possible reason, which fits nicely in current
situation. Vendor driver parses drv_info_sz and shift fields only if packet
is normal, e.g. not c2h type. However, rtw88 always parses those fields. It's
possible that they have some value which should be ignored on 8723ds. I
appended another patch to test.

If this doesn't work, I'm out of ideas.

Best regards,
Jernej


Attachments:
sdio-size.patch (1.12 kB)

2023-05-15 21:17:32

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

Hi Jernej,

On Mon, May 15, 2023 at 10:37 PM Jernej Škrabec
<[email protected]> wrote:
[...]
> > With aggregation disabled, we still get "Invalid RX packet size!" messages. I am
> > changing the statement to log (curr_pkt_len + pkt_desc_sz) > rx_len. I will let
> > you know when the OP responds.
>
> Yeah, I saw. I just find another possible reason, which fits nicely in current
> situation. Vendor driver parses drv_info_sz and shift fields only if packet
> is normal, e.g. not c2h type. However, rtw88 always parses those fields. It's
> possible that they have some value which should be ignored on 8723ds. I
> appended another patch to test.
I tried that patch and it didn't work for me (I can't get the card to
assoc to my AP with that patch).
Additionally I tried a simplified version (attached) and it didn't work.

I'm out of time for today though so I cannot continue testing.


Best regards,
Martin


Attachments:
sdio-check-is-c2h.diff (709.00 B)

2023-05-16 10:26:37

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

On Mon, May 15, 2023 at 11:11 PM Martin Blumenstingl
<[email protected]> wrote:
>
> Hi Jernej,
>
> On Mon, May 15, 2023 at 10:37 PM Jernej Škrabec
> <[email protected]> wrote:
> [...]
> > > With aggregation disabled, we still get "Invalid RX packet size!" messages. I am
> > > changing the statement to log (curr_pkt_len + pkt_desc_sz) > rx_len. I will let
> > > you know when the OP responds.
> >
> > Yeah, I saw. I just find another possible reason, which fits nicely in current
> > situation. Vendor driver parses drv_info_sz and shift fields only if packet
> > is normal, e.g. not c2h type. However, rtw88 always parses those fields. It's
> > possible that they have some value which should be ignored on 8723ds. I
> > appended another patch to test.
> I tried that patch and it didn't work for me (I can't get the card to
> assoc to my AP with that patch).
> Additionally I tried a simplified version (attached) and it didn't work.
>
> I'm out of time for today though so I cannot continue testing.
I took time during my lunch break for some more experiments and came
up with the attached patch (the vendor driver does something similar
and I only noticed that after I observed some rtw_rx_pkt_stat with
pkt_len being zero).
It survived 30 minutes of uptime, updating my system and several
iperf3 runs (in both directions).
iperf results:
- RX: 48 Mbit/s
- TX: 33 Mbit/s

And to be clear, those results are with:
- the word IO bugfix
- the initial two patches from this series
- Larry's addition of the second RTL8723DS SDIO ID
- the attached patch


Best regards,
Martin


Attachments:
rtw88-sdio-rx-isr-check-rx-request-bit.diff (1.17 kB)

2023-05-16 18:35:52

by Larry Finger

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

On 5/16/23 05:22, Martin Blumenstingl wrote:
> On Mon, May 15, 2023 at 11:11 PM Martin Blumenstingl
> <[email protected]> wrote:
>>
>> Hi Jernej,
>>
>> On Mon, May 15, 2023 at 10:37 PM Jernej Škrabec
>> <[email protected]> wrote:
>> [...]
>>>> With aggregation disabled, we still get "Invalid RX packet size!" messages. I am
>>>> changing the statement to log (curr_pkt_len + pkt_desc_sz) > rx_len. I will let
>>>> you know when the OP responds.
>>>
>>> Yeah, I saw. I just find another possible reason, which fits nicely in current
>>> situation. Vendor driver parses drv_info_sz and shift fields only if packet
>>> is normal, e.g. not c2h type. However, rtw88 always parses those fields. It's
>>> possible that they have some value which should be ignored on 8723ds. I
>>> appended another patch to test.
>> I tried that patch and it didn't work for me (I can't get the card to
>> assoc to my AP with that patch).
>> Additionally I tried a simplified version (attached) and it didn't work.
>>
>> I'm out of time for today though so I cannot continue testing.
> I took time during my lunch break for some more experiments and came
> up with the attached patch (the vendor driver does something similar
> and I only noticed that after I observed some rtw_rx_pkt_stat with
> pkt_len being zero).
> It survived 30 minutes of uptime, updating my system and several
> iperf3 runs (in both directions).
> iperf results:
> - RX: 48 Mbit/s
> - TX: 33 Mbit/s
>
> And to be clear, those results are with:
> - the word IO bugfix
> - the initial two patches from this series
> - Larry's addition of the second RTL8723DS SDIO ID
> - the attached patch

Martin,

Please send me a copy of the version of sdio.c that works. It seems likely that
I got mine all messed up as the OP at GitHub is getting lots of warnings from
net/mac80211/rx.c:803.

I though I followed all the patches you and Jernej sent, but I must have messed
something up.

Thanks,

Larry



2023-05-16 18:51:32

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

Hi Larry,

On Tue, May 16, 2023 at 8:31 PM Larry Finger <[email protected]> wrote:
[...]
> >> I'm out of time for today though so I cannot continue testing.
> > I took time during my lunch break for some more experiments and came
> > up with the attached patch (the vendor driver does something similar
> > and I only noticed that after I observed some rtw_rx_pkt_stat with
> > pkt_len being zero).
> > It survived 30 minutes of uptime, updating my system and several
> > iperf3 runs (in both directions).
> > iperf results:
> > - RX: 48 Mbit/s
> > - TX: 33 Mbit/s
> >
> > And to be clear, those results are with:
> > - the word IO bugfix
> > - the initial two patches from this series
> > - Larry's addition of the second RTL8723DS SDIO ID
> > - the attached patch
>
> Martin,
>
> Please send me a copy of the version of sdio.c that works. It seems likely that
> I got mine all messed up as the OP at GitHub is getting lots of warnings from
> net/mac80211/rx.c:803.
Please take the file from [0] - this is how I successfully tested it.


Best regards,
Martin


[0] https://github.com/xdarklight/linux/commits/mangopi-mq-quad-20230516

2023-05-16 23:56:51

by Larry Finger

[permalink] [raw]
Subject: Re: Driver for rtw8723ds

On 5/16/23 13:48, Martin Blumenstingl wrote:
> Hi Larry,
>
> On Tue, May 16, 2023 at 8:31 PM Larry Finger <[email protected]> wrote:
> [...]
>>>> I'm out of time for today though so I cannot continue testing.
>>> I took time during my lunch break for some more experiments and came
>>> up with the attached patch (the vendor driver does something similar
>>> and I only noticed that after I observed some rtw_rx_pkt_stat with
>>> pkt_len being zero).
>>> It survived 30 minutes of uptime, updating my system and several
>>> iperf3 runs (in both directions).
>>> iperf results:
>>> - RX: 48 Mbit/s
>>> - TX: 33 Mbit/s
>>>
>>> And to be clear, those results are with:
>>> - the word IO bugfix
>>> - the initial two patches from this series
>>> - Larry's addition of the second RTL8723DS SDIO ID
>>> - the attached patch
>>
>> Martin,
>>
>> Please send me a copy of the version of sdio.c that works. It seems likely that
>> I got mine all messed up as the OP at GitHub is getting lots of warnings from
>> net/mac80211/rx.c:803.
> Please take the file from [0] - this is how I successfully tested it.

Thanks. I got it and replaced the one in rtw88. Speedtest shows him getting a
little over 40 Mbps.

Thanks for the help from both of you,

Larry