Re: [PATCH 1/2] mwifiex: Use non-posted PCI register writes
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: 2021-09-01 16:51:38
Also in:
linux-wireless, lkml, netdev
On 01.09.2021 17:51, Pali Rohár wrote:
On Wednesday 01 September 2021 16:01:54 Jonas Dreßler wrote:quoted
On 8/30/21 2:49 PM, Andy Shevchenko wrote:quoted
On Mon, Aug 30, 2021 at 3:38 PM Jonas Dreßler [off-list ref] wrote:quoted
On the 88W8897 card it's very important the TX ring write pointer is updated correctly to its new value before setting the TX ready interrupt, otherwise the firmware appears to crash (probably because it's trying to DMA-read from the wrong place).
This sounds somehow like the typical case where you write DMA descriptors and then ring the doorbell. This normally requires a dma_wmb(). Maybe something like that is missing here? Reading back all register writes may cause a certain performance impact, and if it can be avoided we should try to avoid it.
quoted
quoted
quoted
Since PCI uses "posted writes" when writing to a register, it's not guaranteed that a write will happen immediately. That means the pointer might be outdated when setting the TX ready interrupt, leading to firmware crashes especially when ASPM L1 and L1 substates are enabled (because of the higher link latency, the write will probably take longer). So fix those firmware crashes by always forcing non-posted writes. We do that by simply reading back the register after writing it, just as a lot of other drivers do. There are two reproducers that are fixed with this patch: 1) During rx/tx traffic and with ASPM L1 substates enabled (the enabled substates are platform dependent), the firmware crashes and eventually a command timeout appears in the logs. That crash is fixed by using a non-posted write in mwifiex_pcie_send_data(). 2) When sending lots of commands to the card, waking it up from sleep in very quick intervals, the firmware eventually crashes. That crash appears to be fixed by some other non-posted write included here.Thanks for all this work! Nevertheless, do we have any commits that may be a good candidate to be in the Fixes tag here?I don't think there's any commit we could point to, given that the bug is probably somewhere in the firmware code.Then please add Cc: stable@vger.kernel.org tag into commit message. Such bugfix is a good candidate for backporting into stable releases.quoted
quoted
quoted
Signed-off-by: Jonas Dreßler <redacted>