* Re: [PATCH 4/7] mmc: sdio: Drop powered-on re-init at runtime resume and HW reset
From: Ulf Hansson @ 2019-07-09 12:01 UTC (permalink / raw)
To: Doug Anderson
Cc: Linux MMC List, Adrian Hunter, Brian Norris, Shawn Lin,
Guenter Roeck, Heiko Stuebner, Kalle Valo, Arend Van Spriel,
linux-wireless
In-Reply-To: <CAD=FV=UfHd=_gqEMajABV19Mb=G-tz6VptDQa48g4kUPxo-A6g@mail.gmail.com>
On Mon, 8 Jul 2019 at 23:12, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Jul 8, 2019 at 3:54 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Thu, 4 Jul 2019 at 02:02, Doug Anderson <dianders@chromium.org> wrote:
> > >
> > > Hi,
> > >
> > > On Tue, Jun 18, 2019 at 8:35 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > >
> > > > To use the so called powered-on re-initialization of an SDIO card, the
> > > > power to the card must obviously have stayed on. If not, the initialization
> > > > will simply fail.
> > > >
> > > > In the runtime suspend case, the card is always powered off. Hence, let's
> > > > drop the support for powered-on re-initialization during runtime resume, as
> > > > it doesn't make sense.
> > > >
> > > > Moreover, during a HW reset, the point is to cut the power to the card and
> > > > then do fresh re-initialization. Therefore drop the support for powered-on
> > > > re-initialization during HW reset.
> > > >
> > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > > ---
> > > > drivers/mmc/core/sdio.c | 8 +-------
> > > > 1 file changed, 1 insertion(+), 7 deletions(-)
> > >
> > > This has been on my list of things to test for a while but I never
> > > quite got to it...
> > >
> > > ...and then, today, I spent time bisecting why the "reset"
> > > functionality of miwfiex is broken on my 4.19 kernel [1]. AKA, this
> > > is broken:
> > >
> > > cd /sys/kernel/debug/mwifiex/mlan0
> > > echo 1 > reset
> > >
> > > I finally bisected the problem and tracked it down to commit
> > > ca8971ca5753 ("mmc: dw_mmc: Prevent runtime PM suspend when SDIO IRQs
> > > are enabled"), which embarrassingly has my Tested-by on it. I guess I
> > > never tested the Marvell reset call. :-/
> > >
> > > I dug a little and found that when the Marvell code did its reset we
> > > ended up getting a call to dw_mci_enable_sdio_irq(enb=0) and never saw
> > > a dw_mci_enable_sdio_irq(enb=1) after. I tracked it down further and
> > > found that specifically it was the call to mmc_signal_sdio_irq() in
> > > mmc_sdio_power_restore() that was making the call. The call stack
> > > shown for the "enb=0" call:
> > >
> > > [<c071a290>] (dw_mci_enable_sdio_irq) from [<c070a960>]
> > > (mmc_sdio_power_restore+0x98/0xc0)
> > > [<c070a960>] (mmc_sdio_power_restore) from [<c070a9b4>]
> > > (mmc_sdio_reset+0x2c/0x30)
> > > [<c070a9b4>] (mmc_signal_sdio_irq) from [<c06ff160>] (mmc_hw_reset+0xbc/0x138)
> > > [<c06ff160>] (mmc_hw_reset) from [<bf1bbad8>]
> > > (mwifiex_sdio_work+0x5d4/0x678 [mwifiex_sdio])
> > > [<bf1bbad8>] (mwifiex_sdio_work [mwifiex_sdio]) from [<c0247cd0>]
> > > (process_one_work+0x290/0x4b4)
> > >
> > > I picked your patch here (which gets rid of the call to
> > > mmc_signal_sdio_irq()) and magically the problem went away because
> > > there is no more call to mmc_signal_sdio_irq().
> > >
> > > I personally don't have lots of history about the whole
> > > "powered_resume" code path. I checked and mmc_card_keep_power() was 0
> > > in my test case of getting called from hw_reset, so the rest of this
> > > patch doesn't affect me at all. This surprised me a little since I
> > > saw "MMC_PM_KEEP_POWER" being set in mwifiex but then I realized that
> > > it was only set for the duration of suspend and then cleared by the
> > > core. ;-)
> > >
> > > I will also say that I don't have any test case or knowledge of how
> > > SDIO runtime suspend/resume is supposed to work since on dw_mmc SDIO
> > > cards are currently not allowed to runtime suspend anyway. ;-)
> > >
> > >
> > > So I guess the result of all that long-winded reply is that for on
> > > rk3288-veyron-jerry:
> > >
> > > Fixes: ca8971ca5753 ("mmc: dw_mmc: Prevent runtime PM suspend when
> > > SDIO IRQs are enabled")
> > > Tested-by: Douglas Anderson <dianders@chromium.org>
> >
> > Thanks a lot for testing and for your detailed feedback. I have
> > amended the patch by adding your tags above.
>
> Sure! I'm going to try to do some detailed testing on the next patch
> too to confirm it's OK, but I have a few other tasks to get to first.
> ;-)
>
>
> > Moreover, we seems to need this for stable as well, but I am leaving
> > that to be managed as a separate task. We may even consider the hole
> > series for stable, but that requires more testing first.
>
> Sure, makes sense. We'll pick it to the Chrome OS 4.19 kernel
> directly but it's usually nice to get fixes like this into stable so
> everyone can benefit.
>
>
> > > One last note is that, though Marvell WiFi works after a reset after
> > > this commit, Marvell Bluetooth (on the same SDIO module) doesn't. I
> > > guess next week it'll be another bisect...
> >
> > Is the Bluetooth connected to the same SDIO interface, thus the
> > Bluetooth driver is an SDIO func driver?
>
> Yes, it's a SDIO func driver connected to the same interface. So far
> I've managed to confirm the problem on:
>
> v4.4 (plus 76ae3e26ea43 mwifiex: add debugfs file for testing reset of card)
> v4.9
> next-20190708
>
> ...so it seems like it's not a "regression", it's just never worked.
> :-P I guess I'll have to see if I can figure out what's different in
> our chromeos-3.14 kernel. Ah, I see. In 3.14 we had this solution:
>
> pr_err("Resetting card...\n");
> mmc_remove_host(reset_host);
> /* 200ms delay is based on experiment with sdhci controller */
> mdelay(200);
> reset_host->rescan_entered = 0;
> mmc_add_host(reset_host);
>
> ...I think that didn't fly upstream. ...but I can confirm that this works fine:
>
> cd /sys/bus/platform/drivers/dwmmc_rockchip
> echo ff0d0000.dwmmc > unbind
> sleep .5
> echo ff0d0000.dwmmc > bind
>
> ...so I guess this boils down to: how does the mwifiex reset code not
> behave like a full removal and re-insertion of the card? Oh, but
> maybe that's obvious. We're doing all the reset / re-init from the
> WiFi side of things (mwifiex_sdio_card_reset_work) but I don't think
> anything is communicated to the Bluetooth side of things. Presumably
> this is just totally broken for everyone? ...or am I confused?
Nope, that is most likely what is happening.
I am not sure what is the best method to deal with this. Perhaps we
should invent some callback the SDIO core code can call, for each
active SDIO func on the particular SDIO card that becomes reset.
Or is there a better way you think?
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] iwlwifi: fix warning iwl-trans.h is included more than once
From: Luca Coelho @ 2019-07-09 10:58 UTC (permalink / raw)
To: Hariprasad Kelam, Johannes Berg, Emmanuel Grumbach,
Intel Linux Wireless, Kalle Valo, David S. Miller, linux-wireless,
netdev, linux-kernel
In-Reply-To: <20190526113815.GA6328@hari-Inspiron-1545>
On Sun, 2019-05-26 at 17:08 +0530, Hariprasad Kelam wrote:
> remove duplication include of iwl-trans.h
>
> issue identified by includecheck
>
> Signed-off-by: Hariprasad Kelam <hariprasad.kelam@gmail.com>
> ---
Thanks! I have applied this (with small modifications to the commit
message) in our internal tree and it will reach the mainline following
our normal upstreaming process.
--
Cheers,
Luca.
^ permalink raw reply
* [PATCH v2 2/2] rtw88: pci: Use DMA sync instead of remapping in RX ISR
From: Jian-Hong Pan @ 2019-07-09 10:21 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo, David S . Miller, Larry Finger,
David Laight
Cc: linux-wireless, netdev, linux-kernel, linux, Daniel Drake,
Jian-Hong Pan
In-Reply-To: <20190708063252.4756-1-jian-hong@endlessm.com>
Since each skb in RX ring is reused instead of new allocation, we can
treat the DMA in a more efficient way by DMA synchronization.
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
---
drivers/net/wireless/realtek/rtw88/pci.c | 35 ++++++++++++++++++++++--
1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index e9fe3ad896c8..28ca76f71dfe 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -206,6 +206,35 @@ static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
return 0;
}
+static int rtw_pci_sync_rx_desc_cpu(struct rtw_dev *rtwdev, dma_addr_t dma)
+{
+ struct device *dev = rtwdev->dev;
+ int buf_sz = RTK_PCI_RX_BUF_SIZE;
+
+ dma_sync_single_for_cpu(dev, dma, buf_sz, PCI_DMA_FROMDEVICE);
+
+ return 0;
+}
+
+static int rtw_pci_sync_rx_desc_device(struct rtw_dev *rtwdev, dma_addr_t dma,
+ struct rtw_pci_rx_ring *rx_ring,
+ u32 idx, u32 desc_sz)
+{
+ struct device *dev = rtwdev->dev;
+ struct rtw_pci_rx_buffer_desc *buf_desc;
+ int buf_sz = RTK_PCI_RX_BUF_SIZE;
+
+ dma_sync_single_for_device(dev, dma, buf_sz, PCI_DMA_FROMDEVICE);
+
+ buf_desc = (struct rtw_pci_rx_buffer_desc *)(rx_ring->r.head +
+ idx * desc_sz);
+ memset(buf_desc, 0, sizeof(*buf_desc));
+ buf_desc->buf_size = cpu_to_le16(RTK_PCI_RX_BUF_SIZE);
+ buf_desc->dma = cpu_to_le32(dma);
+
+ return 0;
+}
+
static int rtw_pci_init_rx_ring(struct rtw_dev *rtwdev,
struct rtw_pci_rx_ring *rx_ring,
u8 desc_size, u32 len)
@@ -782,8 +811,7 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
rtw_pci_dma_check(rtwdev, ring, cur_rp);
skb = ring->buf[cur_rp];
dma = *((dma_addr_t *)skb->cb);
- pci_unmap_single(rtwpci->pdev, dma, RTK_PCI_RX_BUF_SIZE,
- PCI_DMA_FROMDEVICE);
+ rtw_pci_sync_rx_desc_cpu(rtwdev, dma);
rx_desc = skb->data;
chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
@@ -818,7 +846,8 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
next_rp:
/* new skb delivered to mac80211, re-enable original skb DMA */
- rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
+ rtw_pci_sync_rx_desc_device(rtwdev, dma, ring, cur_rp,
+ buf_desc_sz);
/* host read next element in ring */
if (++cur_rp >= ring->r.len)
--
2.22.0
^ permalink raw reply related
* [PATCH v2 1/2] rtw88: pci: Rearrange the memory usage for skb in RX ISR
From: Jian-Hong Pan @ 2019-07-09 10:20 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo, David S . Miller, Larry Finger,
David Laight
Cc: linux-wireless, netdev, linux-kernel, linux, Daniel Drake,
Jian-Hong Pan, stable
In-Reply-To: <20190708063252.4756-1-jian-hong@endlessm.com>
Testing with RTL8822BE hardware, when available memory is low, we
frequently see a kernel panic and system freeze.
First, rtw_pci_rx_isr encounters a memory allocation failure (trimmed):
rx routine starvation
WARNING: CPU: 7 PID: 9871 at drivers/net/wireless/realtek/rtw88/pci.c:822 rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
[ 2356.580313] RIP: 0010:rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
Then we see a variety of different error conditions and kernel panics,
such as this one (trimmed):
rtw_pci 0000:02:00.0: pci bus timeout, check dma status
skbuff: skb_over_panic: text:00000000091b6e66 len:415 put:415 head:00000000d2880c6f data:000000007a02b1ea tail:0x1df end:0xc0 dev:<NULL>
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:105!
invalid opcode: 0000 [#1] SMP NOPTI
RIP: 0010:skb_panic+0x43/0x45
When skb allocation fails and the "rx routine starvation" is hit, the
function returns immediately without updating the RX ring. At this
point, the RX ring may continue referencing an old skb which was already
handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
bad things happen.
This patch allocates a new, data-sized skb first in RX ISR. After
copying the data in, we pass it to the upper layers. However, if skb
allocation fails, we effectively drop the frame. In both cases, the
original, full size ring skb is reused.
In addition, to fixing the kernel crash, the RX routine should now
generally behave better under low memory conditions.
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
Cc: <stable@vger.kernel.org>
---
drivers/net/wireless/realtek/rtw88/pci.c | 49 +++++++++++-------------
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index cfe05ba7280d..e9fe3ad896c8 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -763,6 +763,7 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
u32 pkt_offset;
u32 pkt_desc_sz = chip->rx_pkt_desc_sz;
u32 buf_desc_sz = chip->rx_buf_desc_sz;
+ u32 new_len;
u8 *rx_desc;
dma_addr_t dma;
@@ -790,40 +791,34 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
pkt_stat.shift;
- if (pkt_stat.is_c2h) {
- /* keep rx_desc, halmac needs it */
- skb_put(skb, pkt_stat.pkt_len + pkt_offset);
+ /* discard current skb if the new skb cannot be allocated as a
+ * new one in rx ring later
+ */
+ new_len = pkt_stat.pkt_len + pkt_offset;
+ new = dev_alloc_skb(new_len);
+ if (WARN_ONCE(!new, "rx routine starvation\n"))
+ goto next_rp;
+
+ /* put the DMA data including rx_desc from phy to new skb */
+ skb_put_data(new, skb->data, new_len);
- /* pass offset for further operation */
- *((u32 *)skb->cb) = pkt_offset;
- skb_queue_tail(&rtwdev->c2h_queue, skb);
+ if (pkt_stat.is_c2h) {
+ /* pass rx_desc & offset for further operation */
+ *((u32 *)new->cb) = pkt_offset;
+ skb_queue_tail(&rtwdev->c2h_queue, new);
ieee80211_queue_work(rtwdev->hw, &rtwdev->c2h_work);
} else {
- /* remove rx_desc, maybe use skb_pull? */
- skb_put(skb, pkt_stat.pkt_len);
- skb_reserve(skb, pkt_offset);
-
- /* alloc a smaller skb to mac80211 */
- new = dev_alloc_skb(pkt_stat.pkt_len);
- if (!new) {
- new = skb;
- } else {
- skb_put_data(new, skb->data, skb->len);
- dev_kfree_skb_any(skb);
- }
- /* TODO: merge into rx.c */
- rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
+ /* remove rx_desc */
+ skb_pull(new, pkt_offset);
+
+ rtw_rx_stats(rtwdev, pkt_stat.vif, new);
memcpy(new->cb, &rx_status, sizeof(rx_status));
ieee80211_rx_irqsafe(rtwdev->hw, new);
}
- /* skb delivered to mac80211, alloc a new one in rx ring */
- new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
- if (WARN(!new, "rx routine starvation\n"))
- return;
-
- ring->buf[cur_rp] = new;
- rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
+next_rp:
+ /* new skb delivered to mac80211, re-enable original skb DMA */
+ rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
/* host read next element in ring */
if (++cur_rp >= ring->r.len)
--
2.22.0
^ permalink raw reply related
* Re: [PATCH 3/3] mt76: usb: use full intermediate buffer in mt76u_copy
From: Stanislaw Gruszka @ 2019-07-09 10:12 UTC (permalink / raw)
To: linux-wireless; +Cc: Felix Fietkau, Lorenzo Bianconi
In-Reply-To: <1562079961-15527-4-git-send-email-sgruszka@redhat.com>
On Tue, Jul 02, 2019 at 05:06:01PM +0200, Stanislaw Gruszka wrote:
> Instead of use several 4 bytes usb requests, use full 32 bytes buffer
> to copy data to device. With the change we use less requests and copy
> exact data size to the device regardless size is multiple of 4 or not.
And this does not work correctly on some usb hosts, request which
are not multiple of 4 do not ended being written to hardware,
what results in original problem of having last part of beacon
corrupted.
I would prefer to drop this set - and I would post 2 patches
(first patch fixed and third dropped). But since this is now in
Felix's wireless tree I guess I need to post fixes on top of this
set?
Stanislaw
^ permalink raw reply
* Re: [PATCH 1/3] mt76: usb: fix endian in mt76u_copy
From: Stanislaw Gruszka @ 2019-07-09 10:06 UTC (permalink / raw)
To: linux-wireless; +Cc: Felix Fietkau, Lorenzo Bianconi
In-Reply-To: <1562079961-15527-2-git-send-email-sgruszka@redhat.com>
On Tue, Jul 02, 2019 at 05:05:59PM +0200, Stanislaw Gruszka wrote:
> In contrast to mt76_wr() which we use to program registers,
> on mt76_wr_copy() we should not change endian of the data.
>
> Fixes: b40b15e1521f ("mt76: add usb support to mt76 layer")
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
> drivers/net/wireless/mediatek/mt76/usb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> index 87ecbe290f99..db90ec6b8775 100644
> --- a/drivers/net/wireless/mediatek/mt76/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> @@ -165,11 +165,11 @@ static void mt76u_copy(struct mt76_dev *dev, u32 offset,
>
> mutex_lock(&usb->usb_ctrl_mtx);
> for (i = 0; i < DIV_ROUND_UP(len, 4); i++) {
> - put_unaligned_le32(val[i], usb->data);
> + put_unaligned(val[i], usb->data);
This is bug as put_unaligned() use size based second argument
pointer type, correct version should looks like this:
put_unaligned(val[i], (u32 *) usb->data);
Stanislaw
^ permalink raw reply
* [PATCH v4] ath10k: Enable MSA region dump support for WCN3990
From: Govind Singh @ 2019-07-09 5:36 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Govind Singh
MSA memory region caries the hw descriptors information.
Dump MSA region in core dump as this is very helpful in debugging
hw issues.
Testing: Tested on WCN3990 HW
Tested FW: WLAN.HL.3.1-00959-QCAHLSWMTPLZ-1
Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
Changes from v3:
- Fixed error reported in v2.
Changes from v2:
- Rebased on top of 38faed150438 ath10k: perform crash dump collection in workqueue.
- Removed redundant msa permission call.
---
drivers/net/wireless/ath/ath10k/coredump.c | 21 +++++++
drivers/net/wireless/ath/ath10k/coredump.h | 1 +
drivers/net/wireless/ath/ath10k/qmi.c | 4 ++
drivers/net/wireless/ath/ath10k/snoc.c | 66 ++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/snoc.h | 1 +
5 files changed, 93 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/coredump.c b/drivers/net/wireless/ath/ath10k/coredump.c
index aa04fbf146e0..0ec690b49fb1 100644
--- a/drivers/net/wireless/ath/ath10k/coredump.c
+++ b/drivers/net/wireless/ath/ath10k/coredump.c
@@ -962,6 +962,19 @@ static const struct ath10k_mem_region qca4019_hw10_mem_regions[] = {
},
};
+static const struct ath10k_mem_region wcn399x_hw10_mem_regions[] = {
+ {
+ /* MSA region start is not fixed, hence it is assigned at runtime */
+ .type = ATH10K_MEM_REGION_TYPE_MSA,
+ .len = 0x100000,
+ .name = "DRAM",
+ .section_table = {
+ .sections = NULL,
+ .size = 0,
+ },
+ },
+};
+
static const struct ath10k_hw_mem_layout hw_mem_layouts[] = {
{
.hw_id = QCA6174_HW_1_0_VERSION,
@@ -1059,6 +1072,14 @@ static const struct ath10k_hw_mem_layout hw_mem_layouts[] = {
.size = ARRAY_SIZE(qca4019_hw10_mem_regions),
},
},
+ {
+ .hw_id = WCN3990_HW_1_0_DEV_VERSION,
+ .hw_rev = ATH10K_HW_WCN3990,
+ .region_table = {
+ .regions = wcn399x_hw10_mem_regions,
+ .size = ARRAY_SIZE(wcn399x_hw10_mem_regions),
+ },
+ },
};
static u32 ath10k_coredump_get_ramdump_size(struct ath10k *ar)
diff --git a/drivers/net/wireless/ath/ath10k/coredump.h b/drivers/net/wireless/ath/ath10k/coredump.h
index 5dac653e1649..9802e90483f4 100644
--- a/drivers/net/wireless/ath/ath10k/coredump.h
+++ b/drivers/net/wireless/ath/ath10k/coredump.h
@@ -126,6 +126,7 @@ enum ath10k_mem_region_type {
ATH10K_MEM_REGION_TYPE_IRAM2 = 5,
ATH10K_MEM_REGION_TYPE_IOSRAM = 6,
ATH10K_MEM_REGION_TYPE_IOREG = 7,
+ ATH10K_MEM_REGION_TYPE_MSA = 8,
};
/* Define a section of the region which should be copied. As not all parts
diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c
index ba8f5a8f83d1..8eb0f0f0d3a7 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -817,9 +817,13 @@ ath10k_qmi_driver_event_post(struct ath10k_qmi *qmi,
static void ath10k_qmi_event_server_exit(struct ath10k_qmi *qmi)
{
struct ath10k *ar = qmi->ar;
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
ath10k_qmi_remove_msa_permission(qmi);
ath10k_core_free_board_files(ar);
+ if (!test_bit(ATH10K_SNOC_FLAG_UNREGISTERING, &ar_snoc->flags))
+ ath10k_snoc_fw_crashed_dump(ar);
+
ath10k_snoc_fw_indication(ar, ATH10K_QMI_EVENT_FW_DOWN_IND);
ath10k_dbg(ar, ATH10K_DBG_QMI, "wifi fw qmi service disconnected\n");
}
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 0be12996beba..6c362c928e4e 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -24,6 +24,7 @@
#include <linux/regulator/consumer.h>
#include "ce.h"
+#include "coredump.h"
#include "debug.h"
#include "hif.h"
#include "htc.h"
@@ -1586,6 +1587,71 @@ static int ath10k_hw_power_off(struct ath10k *ar)
return ret;
}
+static void ath10k_msa_dump_memory(struct ath10k *ar,
+ struct ath10k_fw_crash_data *crash_data)
+{
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+ const struct ath10k_hw_mem_layout *mem_layout;
+ const struct ath10k_mem_region *current_region;
+ struct ath10k_dump_ram_data_hdr *hdr;
+ size_t buf_len;
+ u8 *buf;
+
+ if (!crash_data && !crash_data->ramdump_buf)
+ return;
+
+ mem_layout = ath10k_coredump_get_mem_layout(ar);
+ if (!mem_layout)
+ return;
+
+ current_region = &mem_layout->region_table.regions[0];
+
+ buf = crash_data->ramdump_buf;
+ buf_len = crash_data->ramdump_buf_len;
+ memset(buf, 0, buf_len);
+
+ /* Reserve space for the header. */
+ hdr = (void *)buf;
+ buf += sizeof(*hdr);
+ buf_len -= sizeof(*hdr);
+
+ hdr->region_type = cpu_to_le32(current_region->type);
+ hdr->start = cpu_to_le32((unsigned long)ar_snoc->qmi->msa_va);
+ hdr->length = cpu_to_le32(ar_snoc->qmi->msa_mem_size);
+
+ if (current_region->len < ar_snoc->qmi->msa_mem_size) {
+ memcpy(buf, ar_snoc->qmi->msa_va, current_region->len);
+ ath10k_warn(ar, "msa dump length is less than msa size %x, %x\n",
+ current_region->len, ar_snoc->qmi->msa_mem_size);
+ } else {
+ memcpy(buf, ar_snoc->qmi->msa_va, ar_snoc->qmi->msa_mem_size);
+ }
+}
+
+void ath10k_snoc_fw_crashed_dump(struct ath10k *ar)
+{
+ struct ath10k_fw_crash_data *crash_data;
+ char guid[UUID_STRING_LEN + 1];
+
+ mutex_lock(&ar->dump_mutex);
+
+ spin_lock_bh(&ar->data_lock);
+ ar->stats.fw_crash_counter++;
+ spin_unlock_bh(&ar->data_lock);
+
+ crash_data = ath10k_coredump_new(ar);
+
+ if (crash_data)
+ scnprintf(guid, sizeof(guid), "%pUl", &crash_data->guid);
+ else
+ scnprintf(guid, sizeof(guid), "n/a");
+
+ ath10k_err(ar, "firmware crashed! (guid %s)\n", guid);
+ ath10k_print_driver_info(ar);
+ ath10k_msa_dump_memory(ar, crash_data);
+ mutex_unlock(&ar->dump_mutex);
+}
+
static const struct of_device_id ath10k_snoc_dt_match[] = {
{ .compatible = "qcom,wcn3990-wifi",
.data = &drv_priv,
diff --git a/drivers/net/wireless/ath/ath10k/snoc.h b/drivers/net/wireless/ath/ath10k/snoc.h
index 25383de8f17d..6d28a6290a94 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.h
+++ b/drivers/net/wireless/ath/ath10k/snoc.h
@@ -101,5 +101,6 @@ static inline struct ath10k_snoc *ath10k_snoc_priv(struct ath10k *ar)
}
int ath10k_snoc_fw_indication(struct ath10k *ar, u64 type);
+void ath10k_snoc_fw_crashed_dump(struct ath10k *ar);
#endif /* _SNOC_H_ */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* Re: linux-next: build failure after merge of the tip tree
From: Stephen Rothwell @ 2019-07-09 3:46 UTC (permalink / raw)
To: Kalle Valo, Wireless
Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Peter Zijlstra,
Linux Next Mailing List, Linux Kernel Mailing List,
Christian Lamparter, Jason A. Donenfeld, David Miller, Networking
In-Reply-To: <20190625160432.533aa140@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 2061 bytes --]
Hi all,
On Tue, 25 Jun 2019 16:04:32 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> After merging the tip tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
>
> drivers/net/wireless/intersil/p54/txrx.c: In function 'p54_rx_data':
> drivers/net/wireless/intersil/p54/txrx.c:386:28: error: implicit declaration of function 'ktime_get_boot_ns'; did you mean 'ktime_get_raw_ns'? [-Werror=implicit-function-declaration]
> rx_status->boottime_ns = ktime_get_boot_ns();
> ^~~~~~~~~~~~~~~~~
> ktime_get_raw_ns
>
> Caused by commit
>
> c11c75ec784e ("p54: Support boottime in scan results")
>
> from the wireless-drivers-next tree interacting with commit
>
> 9285ec4c8b61 ("timekeeping: Use proper clock specifier names in functions")
>
> from the tip tree.
>
> I have added the following merge fix patch:
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 25 Jun 2019 15:55:36 +1000
> Subject: [PATCH] p54: fix up for ktime_get_boot_ns() name change
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> drivers/net/wireless/intersil/p54/txrx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intersil/p54/txrx.c b/drivers/net/wireless/intersil/p54/txrx.c
> index be6968454282..873fea59894f 100644
> --- a/drivers/net/wireless/intersil/p54/txrx.c
> +++ b/drivers/net/wireless/intersil/p54/txrx.c
> @@ -383,7 +383,7 @@ static int p54_rx_data(struct p54_common *priv, struct sk_buff *skb)
>
> fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
> if (ieee80211_is_probe_resp(fc) || ieee80211_is_beacon(fc))
> - rx_status->boottime_ns = ktime_get_boot_ns();
> + rx_status->boottime_ns = ktime_get_boottime_ns();
>
> if (unlikely(priv->hw->conf.flags & IEEE80211_CONF_PS))
> p54_pspoll_workaround(priv, skb);
This patch is now needed in the merge between the net-next tree and
Linus' tree.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: linux-next: build failure after merge of the tip tree
From: Stephen Rothwell @ 2019-07-09 0:09 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Peter Zijlstra,
Kalle Valo, Wireless
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Christian Lamparter, Jason A. Donenfeld
In-Reply-To: <20190625160432.533aa140@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]
Hi all,
On Tue, 25 Jun 2019 16:04:32 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> After merging the tip tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
>
> drivers/net/wireless/intersil/p54/txrx.c: In function 'p54_rx_data':
> drivers/net/wireless/intersil/p54/txrx.c:386:28: error: implicit declaration of function 'ktime_get_boot_ns'; did you mean 'ktime_get_raw_ns'? [-Werror=implicit-function-declaration]
> rx_status->boottime_ns = ktime_get_boot_ns();
> ^~~~~~~~~~~~~~~~~
> ktime_get_raw_ns
>
> Caused by commit
>
> c11c75ec784e ("p54: Support boottime in scan results")
>
> from the wireless-drivers-next tree interacting with commit
>
> 9285ec4c8b61 ("timekeeping: Use proper clock specifier names in functions")
>
> from the tip tree.
>
> I have added the following merge fix patch:
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 25 Jun 2019 15:55:36 +1000
> Subject: [PATCH] p54: fix up for ktime_get_boot_ns() name change
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> drivers/net/wireless/intersil/p54/txrx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intersil/p54/txrx.c b/drivers/net/wireless/intersil/p54/txrx.c
> index be6968454282..873fea59894f 100644
> --- a/drivers/net/wireless/intersil/p54/txrx.c
> +++ b/drivers/net/wireless/intersil/p54/txrx.c
> @@ -383,7 +383,7 @@ static int p54_rx_data(struct p54_common *priv, struct sk_buff *skb)
>
> fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
> if (ieee80211_is_probe_resp(fc) || ieee80211_is_beacon(fc))
> - rx_status->boottime_ns = ktime_get_boot_ns();
> + rx_status->boottime_ns = ktime_get_boottime_ns();
>
> if (unlikely(priv->hw->conf.flags & IEEE80211_CONF_PS))
> p54_pspoll_workaround(priv, skb);
> --
> 2.20.1
I am still getting this conflict (the commit ids may have changed).
Just a reminder in case you think Linus may need to know.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 4/7] mmc: sdio: Drop powered-on re-init at runtime resume and HW reset
From: Doug Anderson @ 2019-07-08 21:12 UTC (permalink / raw)
To: Ulf Hansson
Cc: Linux MMC List, Adrian Hunter, Brian Norris, Shawn Lin,
Guenter Roeck, Heiko Stuebner, Kalle Valo, Arend Van Spriel,
linux-wireless
In-Reply-To: <CAPDyKFoHzCmobCtyy-j+-4xjW0Bt1_vA5-s4vHLVN78jXJ4X-w@mail.gmail.com>
Hi,
On Mon, Jul 8, 2019 at 3:54 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 4 Jul 2019 at 02:02, Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Tue, Jun 18, 2019 at 8:35 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >
> > > To use the so called powered-on re-initialization of an SDIO card, the
> > > power to the card must obviously have stayed on. If not, the initialization
> > > will simply fail.
> > >
> > > In the runtime suspend case, the card is always powered off. Hence, let's
> > > drop the support for powered-on re-initialization during runtime resume, as
> > > it doesn't make sense.
> > >
> > > Moreover, during a HW reset, the point is to cut the power to the card and
> > > then do fresh re-initialization. Therefore drop the support for powered-on
> > > re-initialization during HW reset.
> > >
> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > ---
> > > drivers/mmc/core/sdio.c | 8 +-------
> > > 1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > This has been on my list of things to test for a while but I never
> > quite got to it...
> >
> > ...and then, today, I spent time bisecting why the "reset"
> > functionality of miwfiex is broken on my 4.19 kernel [1]. AKA, this
> > is broken:
> >
> > cd /sys/kernel/debug/mwifiex/mlan0
> > echo 1 > reset
> >
> > I finally bisected the problem and tracked it down to commit
> > ca8971ca5753 ("mmc: dw_mmc: Prevent runtime PM suspend when SDIO IRQs
> > are enabled"), which embarrassingly has my Tested-by on it. I guess I
> > never tested the Marvell reset call. :-/
> >
> > I dug a little and found that when the Marvell code did its reset we
> > ended up getting a call to dw_mci_enable_sdio_irq(enb=0) and never saw
> > a dw_mci_enable_sdio_irq(enb=1) after. I tracked it down further and
> > found that specifically it was the call to mmc_signal_sdio_irq() in
> > mmc_sdio_power_restore() that was making the call. The call stack
> > shown for the "enb=0" call:
> >
> > [<c071a290>] (dw_mci_enable_sdio_irq) from [<c070a960>]
> > (mmc_sdio_power_restore+0x98/0xc0)
> > [<c070a960>] (mmc_sdio_power_restore) from [<c070a9b4>]
> > (mmc_sdio_reset+0x2c/0x30)
> > [<c070a9b4>] (mmc_signal_sdio_irq) from [<c06ff160>] (mmc_hw_reset+0xbc/0x138)
> > [<c06ff160>] (mmc_hw_reset) from [<bf1bbad8>]
> > (mwifiex_sdio_work+0x5d4/0x678 [mwifiex_sdio])
> > [<bf1bbad8>] (mwifiex_sdio_work [mwifiex_sdio]) from [<c0247cd0>]
> > (process_one_work+0x290/0x4b4)
> >
> > I picked your patch here (which gets rid of the call to
> > mmc_signal_sdio_irq()) and magically the problem went away because
> > there is no more call to mmc_signal_sdio_irq().
> >
> > I personally don't have lots of history about the whole
> > "powered_resume" code path. I checked and mmc_card_keep_power() was 0
> > in my test case of getting called from hw_reset, so the rest of this
> > patch doesn't affect me at all. This surprised me a little since I
> > saw "MMC_PM_KEEP_POWER" being set in mwifiex but then I realized that
> > it was only set for the duration of suspend and then cleared by the
> > core. ;-)
> >
> > I will also say that I don't have any test case or knowledge of how
> > SDIO runtime suspend/resume is supposed to work since on dw_mmc SDIO
> > cards are currently not allowed to runtime suspend anyway. ;-)
> >
> >
> > So I guess the result of all that long-winded reply is that for on
> > rk3288-veyron-jerry:
> >
> > Fixes: ca8971ca5753 ("mmc: dw_mmc: Prevent runtime PM suspend when
> > SDIO IRQs are enabled")
> > Tested-by: Douglas Anderson <dianders@chromium.org>
>
> Thanks a lot for testing and for your detailed feedback. I have
> amended the patch by adding your tags above.
Sure! I'm going to try to do some detailed testing on the next patch
too to confirm it's OK, but I have a few other tasks to get to first.
;-)
> Moreover, we seems to need this for stable as well, but I am leaving
> that to be managed as a separate task. We may even consider the hole
> series for stable, but that requires more testing first.
Sure, makes sense. We'll pick it to the Chrome OS 4.19 kernel
directly but it's usually nice to get fixes like this into stable so
everyone can benefit.
> > One last note is that, though Marvell WiFi works after a reset after
> > this commit, Marvell Bluetooth (on the same SDIO module) doesn't. I
> > guess next week it'll be another bisect...
>
> Is the Bluetooth connected to the same SDIO interface, thus the
> Bluetooth driver is an SDIO func driver?
Yes, it's a SDIO func driver connected to the same interface. So far
I've managed to confirm the problem on:
v4.4 (plus 76ae3e26ea43 mwifiex: add debugfs file for testing reset of card)
v4.9
next-20190708
...so it seems like it's not a "regression", it's just never worked.
:-P I guess I'll have to see if I can figure out what's different in
our chromeos-3.14 kernel. Ah, I see. In 3.14 we had this solution:
pr_err("Resetting card...\n");
mmc_remove_host(reset_host);
/* 200ms delay is based on experiment with sdhci controller */
mdelay(200);
reset_host->rescan_entered = 0;
mmc_add_host(reset_host);
...I think that didn't fly upstream. ...but I can confirm that this works fine:
cd /sys/bus/platform/drivers/dwmmc_rockchip
echo ff0d0000.dwmmc > unbind
sleep .5
echo ff0d0000.dwmmc > bind
...so I guess this boils down to: how does the mwifiex reset code not
behave like a full removal and re-insertion of the card? Oh, but
maybe that's obvious. We're doing all the reset / re-init from the
WiFi side of things (mwifiex_sdio_card_reset_work) but I don't think
anything is communicated to the Bluetooth side of things. Presumably
this is just totally broken for everyone? ...or am I confused?
-Doug
-Doug
^ permalink raw reply
* Re: [PATCH] ath10k: work around uninitialized vht_pfr variable
From: Brian Norris @ 2019-07-08 20:34 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Kalle Valo, Miaoqing Pan, David S. Miller, Rakesh Pillai,
Balaji Pothunoori, Wen Gong, Pradeep kumar Chitrapu, Sriram R,
ath10k, linux-wireless, <netdev@vger.kernel.org>,
Linux Kernel, clang-built-linux
In-Reply-To: <20190708125050.3689133-1-arnd@arndb.de>
Hi Arnd,
On Mon, Jul 8, 2019 at 5:50 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> As clang points out, the vht_pfr is assigned to a struct member
> without being initialized in one case:
>
> drivers/net/wireless/ath/ath10k/mac.c:7528:7: error: variable 'vht_pfr' is used uninitialized whenever 'if' condition
> is false [-Werror,-Wsometimes-uninitialized]
> if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7551:20: note: uninitialized use occurs here
> arvif->vht_pfr = vht_pfr;
> ^~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7528:3: note: remove the 'if' if its condition is always true
> if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7483:12: note: initialize the variable 'vht_pfr' to silence this warning
> u8 vht_pfr;
>
> Add an explicit but probably incorrect initialization here.
> I suspect we want a better fix here, but chose this approach to
> illustrate the issue.
>
> Fixes: 8b97b055dc9d ("ath10k: fix failure to set multiple fixed rate")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/wireless/ath/ath10k/mac.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index e43a566eef77..0606416dc971 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -7541,6 +7541,8 @@ static int ath10k_mac_op_set_bitrate_mask(struct ieee80211_hw *hw,
> &vht_nss,
> true);
^^ Technically, this call to ath10k_mac_bitrate_mask_get_single_rate()
can fail to assign 'vht_pfr' as well. I can't immediately tell whether
it provably will never hit the -EINVAL case, but if we do, then you'd
have another uninitialized case.
I *believe* it shouldn't fail, since we already pre-checked the VHT
MCS lists for "exactly 1" rate. But it still seems like better code to
pre-initialize and/or add error-handling, so we don't rely on that
implicit proof.
I'm not quite sure yet what the "better" answer should be for
resolving this, but at a minimum, I think the above could be improved.
Brian
> update_bitrate_mask = false;
> + } else {
> + vht_pfr = 0;
> }
>
> mutex_lock(&ar->conf_mutex);
> --
> 2.20.0
>
^ permalink raw reply
* Re: use exact allocation for dma coherent memory
From: Christoph Hellwig @ 2019-07-08 18:43 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Christoph Hellwig, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, Daniel Vetter, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Ian Abbott, H Hartley Sweeten, devel, linux-s390,
Intel Linux Wireless, linux-rdma, netdev, intel-gfx,
linux-wireless, linux-kernel, dri-devel, linux-mm, iommu,
moderated list:ARM PORT, linux-media
In-Reply-To: <74eb9d99-6aa6-d1ad-e66d-6cc9c496b2f3@broadcom.com>
On Tue, Jul 02, 2019 at 11:48:44AM +0200, Arend Van Spriel wrote:
> You made me look ;-) Actually not touching my drivers so I'm off the hook.
> However, I was wondering if drivers could know so I decided to look into
> the DMA-API.txt documentation which currently states:
>
> """
> The flag parameter (dma_alloc_coherent() only) allows the caller to
> specify the ``GFP_`` flags (see kmalloc()) for the allocation (the
> implementation may choose to ignore flags that affect the location of
> the returned memory, like GFP_DMA).
> """
>
> I do expect you are going to change that description as well now that you
> are going to issue a warning on __GFP_COMP. Maybe include that in patch
> 15/16 where you introduce that warning.
Yes, that description needs an updated, even without this series.
I'll make sure it is more clear.
^ permalink raw reply
* Re: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
From: Larry Finger @ 2019-07-08 18:01 UTC (permalink / raw)
To: Jian-Hong Pan, Yan-Hsuan Chuang, Kalle Valo, David S . Miller
Cc: linux-wireless, netdev, linux-kernel, linux, Daniel Drake, stable
In-Reply-To: <20190708063252.4756-1-jian-hong@endlessm.com>
On 7/8/19 1:32 AM, Jian-Hong Pan wrote:
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> index cfe05ba7280d..1bfc99ae6b84 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -786,6 +786,15 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> rx_desc = skb->data;
> chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
>
> + /* discard current skb if the new skb cannot be allocated as a
> + * new one in rx ring later
> + * */
> + new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> + if (WARN(!new, "rx routine starvation\n")) {
> + new = skb;
> + goto next_rp;
This should probably be a WARN_ONCE() rather than WARN(), otherwise the logs
will be flooded once this condition triggers.
Larry
^ permalink raw reply
* [PATCH 2/2] iwlwifi: pcie: add support for qu c-step devices
From: Luca Coelho @ 2019-07-08 15:55 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, Luca Coelho, stable
In-Reply-To: <20190708155534.18241-1-luca@coelho.fi>
From: Luca Coelho <luciano.coelho@intel.com>
Add support for C-step devices. Currently we don't have a nice way of
matching the step and choosing the proper configuration, so we need to
switch the config structs one by one.
Cc: stable@vger.kernel.org
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
.../net/wireless/intel/iwlwifi/cfg/22000.c | 53 +++++++++++++++++++
.../net/wireless/intel/iwlwifi/iwl-config.h | 7 +++
drivers/net/wireless/intel/iwlwifi/iwl-csr.h | 2 +
drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 21 ++++++++
4 files changed, 83 insertions(+)
diff --git a/drivers/net/wireless/intel/iwlwifi/cfg/22000.c b/drivers/net/wireless/intel/iwlwifi/cfg/22000.c
index 93526dfaf791..1f500cddb3a7 100644
--- a/drivers/net/wireless/intel/iwlwifi/cfg/22000.c
+++ b/drivers/net/wireless/intel/iwlwifi/cfg/22000.c
@@ -80,7 +80,9 @@
#define IWL_22000_QU_B_HR_B_FW_PRE "iwlwifi-Qu-b0-hr-b0-"
#define IWL_22000_HR_B_FW_PRE "iwlwifi-QuQnj-b0-hr-b0-"
#define IWL_22000_HR_A0_FW_PRE "iwlwifi-QuQnj-a0-hr-a0-"
+#define IWL_QU_C_HR_B_FW_PRE "iwlwifi-Qu-c0-hr-b0-"
#define IWL_QU_B_JF_B_FW_PRE "iwlwifi-Qu-b0-jf-b0-"
+#define IWL_QU_C_JF_B_FW_PRE "iwlwifi-Qu-c0-jf-b0-"
#define IWL_QUZ_A_HR_B_FW_PRE "iwlwifi-QuZ-a0-hr-b0-"
#define IWL_QUZ_A_JF_B_FW_PRE "iwlwifi-QuZ-a0-jf-b0-"
#define IWL_QNJ_B_JF_B_FW_PRE "iwlwifi-QuQnj-b0-jf-b0-"
@@ -109,6 +111,8 @@
IWL_QUZ_A_HR_B_FW_PRE __stringify(api) ".ucode"
#define IWL_QUZ_A_JF_B_MODULE_FIRMWARE(api) \
IWL_QUZ_A_JF_B_FW_PRE __stringify(api) ".ucode"
+#define IWL_QU_C_HR_B_MODULE_FIRMWARE(api) \
+ IWL_QU_C_HR_B_FW_PRE __stringify(api) ".ucode"
#define IWL_QU_B_JF_B_MODULE_FIRMWARE(api) \
IWL_QU_B_JF_B_FW_PRE __stringify(api) ".ucode"
#define IWL_QNJ_B_JF_B_MODULE_FIRMWARE(api) \
@@ -256,6 +260,30 @@ const struct iwl_cfg iwl_ax201_cfg_qu_hr = {
.max_tx_agg_size = IEEE80211_MAX_AMPDU_BUF_HT,
};
+const struct iwl_cfg iwl_ax101_cfg_qu_c0_hr_b0 = {
+ .name = "Intel(R) Wi-Fi 6 AX101",
+ .fw_name_pre = IWL_QU_C_HR_B_FW_PRE,
+ IWL_DEVICE_22500,
+ /*
+ * This device doesn't support receiving BlockAck with a large bitmap
+ * so we need to restrict the size of transmitted aggregation to the
+ * HT size; mac80211 would otherwise pick the HE max (256) by default.
+ */
+ .max_tx_agg_size = IEEE80211_MAX_AMPDU_BUF_HT,
+};
+
+const struct iwl_cfg iwl_ax201_cfg_qu_c0_hr_b0 = {
+ .name = "Intel(R) Wi-Fi 6 AX201 160MHz",
+ .fw_name_pre = IWL_QU_C_HR_B_FW_PRE,
+ IWL_DEVICE_22500,
+ /*
+ * This device doesn't support receiving BlockAck with a large bitmap
+ * so we need to restrict the size of transmitted aggregation to the
+ * HT size; mac80211 would otherwise pick the HE max (256) by default.
+ */
+ .max_tx_agg_size = IEEE80211_MAX_AMPDU_BUF_HT,
+};
+
const struct iwl_cfg iwl_ax101_cfg_quz_hr = {
.name = "Intel(R) Wi-Fi 6 AX101",
.fw_name_pre = IWL_QUZ_A_HR_B_FW_PRE,
@@ -372,6 +400,30 @@ const struct iwl_cfg iwl9560_2ac_160_cfg_qu_b0_jf_b0 = {
IWL_DEVICE_22500,
};
+const struct iwl_cfg iwl9461_2ac_cfg_qu_c0_jf_b0 = {
+ .name = "Intel(R) Wireless-AC 9461",
+ .fw_name_pre = IWL_QU_C_JF_B_FW_PRE,
+ IWL_DEVICE_22500,
+};
+
+const struct iwl_cfg iwl9462_2ac_cfg_qu_c0_jf_b0 = {
+ .name = "Intel(R) Wireless-AC 9462",
+ .fw_name_pre = IWL_QU_C_JF_B_FW_PRE,
+ IWL_DEVICE_22500,
+};
+
+const struct iwl_cfg iwl9560_2ac_cfg_qu_c0_jf_b0 = {
+ .name = "Intel(R) Wireless-AC 9560",
+ .fw_name_pre = IWL_QU_C_JF_B_FW_PRE,
+ IWL_DEVICE_22500,
+};
+
+const struct iwl_cfg iwl9560_2ac_160_cfg_qu_c0_jf_b0 = {
+ .name = "Intel(R) Wireless-AC 9560 160MHz",
+ .fw_name_pre = IWL_QU_C_JF_B_FW_PRE,
+ IWL_DEVICE_22500,
+};
+
const struct iwl_cfg iwl9560_2ac_cfg_qnj_jf_b0 = {
.name = "Intel(R) Wireless-AC 9560 160MHz",
.fw_name_pre = IWL_QNJ_B_JF_B_FW_PRE,
@@ -590,6 +642,7 @@ MODULE_FIRMWARE(IWL_22000_HR_A_F0_QNJ_MODULE_FIRMWARE(IWL_22000_UCODE_API_MAX));
MODULE_FIRMWARE(IWL_22000_HR_B_F0_QNJ_MODULE_FIRMWARE(IWL_22000_UCODE_API_MAX));
MODULE_FIRMWARE(IWL_22000_HR_B_QNJ_MODULE_FIRMWARE(IWL_22000_UCODE_API_MAX));
MODULE_FIRMWARE(IWL_22000_HR_A0_QNJ_MODULE_FIRMWARE(IWL_22000_UCODE_API_MAX));
+MODULE_FIRMWARE(IWL_QU_C_HR_B_MODULE_FIRMWARE(IWL_22000_UCODE_API_MAX));
MODULE_FIRMWARE(IWL_QU_B_JF_B_MODULE_FIRMWARE(IWL_22000_UCODE_API_MAX));
MODULE_FIRMWARE(IWL_QUZ_A_HR_B_MODULE_FIRMWARE(IWL_22000_UCODE_API_MAX));
MODULE_FIRMWARE(IWL_QUZ_A_JF_B_MODULE_FIRMWARE(IWL_22000_UCODE_API_MAX));
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-config.h b/drivers/net/wireless/intel/iwlwifi/iwl-config.h
index bc267bd2c3b0..1c1bf1b281cd 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-config.h
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-config.h
@@ -565,10 +565,13 @@ extern const struct iwl_cfg iwl22000_2ac_cfg_hr;
extern const struct iwl_cfg iwl22000_2ac_cfg_hr_cdb;
extern const struct iwl_cfg iwl22000_2ac_cfg_jf;
extern const struct iwl_cfg iwl_ax101_cfg_qu_hr;
+extern const struct iwl_cfg iwl_ax101_cfg_qu_c0_hr_b0;
extern const struct iwl_cfg iwl_ax101_cfg_quz_hr;
extern const struct iwl_cfg iwl22000_2ax_cfg_hr;
extern const struct iwl_cfg iwl_ax200_cfg_cc;
extern const struct iwl_cfg iwl_ax201_cfg_qu_hr;
+extern const struct iwl_cfg iwl_ax201_cfg_qu_hr;
+extern const struct iwl_cfg iwl_ax201_cfg_qu_c0_hr_b0;
extern const struct iwl_cfg iwl_ax201_cfg_quz_hr;
extern const struct iwl_cfg iwl_ax1650i_cfg_quz_hr;
extern const struct iwl_cfg iwl_ax1650s_cfg_quz_hr;
@@ -580,6 +583,10 @@ extern const struct iwl_cfg iwl9461_2ac_cfg_qu_b0_jf_b0;
extern const struct iwl_cfg iwl9462_2ac_cfg_qu_b0_jf_b0;
extern const struct iwl_cfg iwl9560_2ac_cfg_qu_b0_jf_b0;
extern const struct iwl_cfg iwl9560_2ac_160_cfg_qu_b0_jf_b0;
+extern const struct iwl_cfg iwl9461_2ac_cfg_qu_c0_jf_b0;
+extern const struct iwl_cfg iwl9462_2ac_cfg_qu_c0_jf_b0;
+extern const struct iwl_cfg iwl9560_2ac_cfg_qu_c0_jf_b0;
+extern const struct iwl_cfg iwl9560_2ac_160_cfg_qu_c0_jf_b0;
extern const struct iwl_cfg killer1550i_2ac_cfg_qu_b0_jf_b0;
extern const struct iwl_cfg killer1550s_2ac_cfg_qu_b0_jf_b0;
extern const struct iwl_cfg iwl22000_2ax_cfg_jf;
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-csr.h b/drivers/net/wireless/intel/iwlwifi/iwl-csr.h
index 93da96a7247c..cb4c5514a556 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-csr.h
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-csr.h
@@ -328,6 +328,8 @@ enum {
#define CSR_HW_REV_TYPE_NONE (0x00001F0)
#define CSR_HW_REV_TYPE_QNJ (0x0000360)
#define CSR_HW_REV_TYPE_QNJ_B0 (0x0000364)
+#define CSR_HW_REV_TYPE_QU_B0 (0x0000334)
+#define CSR_HW_REV_TYPE_QU_C0 (0x0000338)
#define CSR_HW_REV_TYPE_QUZ (0x0000354)
#define CSR_HW_REV_TYPE_HR_CDB (0x0000340)
#define CSR_HW_REV_TYPE_SO (0x0000370)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
index fe645380bd2f..ea2a03d4bf55 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
@@ -1039,6 +1039,27 @@ static int iwl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
iwl_trans->cfg = cfg;
}
+
+ /*
+ * This is a hack to switch from Qu B0 to Qu C0. We need to
+ * do this for all cfgs that use Qu B0. All this code is in
+ * urgent need for a refactor, but for now this is the easiest
+ * thing to do to support Qu C-step.
+ */
+ if (iwl_trans->hw_rev == CSR_HW_REV_TYPE_QU_C0) {
+ if (iwl_trans->cfg == &iwl_ax101_cfg_qu_hr)
+ iwl_trans->cfg = &iwl_ax101_cfg_qu_c0_hr_b0;
+ else if (iwl_trans->cfg == &iwl_ax201_cfg_qu_hr)
+ iwl_trans->cfg = &iwl_ax201_cfg_qu_c0_hr_b0;
+ else if (iwl_trans->cfg == &iwl9461_2ac_cfg_qu_b0_jf_b0)
+ iwl_trans->cfg = &iwl9461_2ac_cfg_qu_c0_jf_b0;
+ else if (iwl_trans->cfg == &iwl9462_2ac_cfg_qu_b0_jf_b0)
+ iwl_trans->cfg = &iwl9462_2ac_cfg_qu_c0_jf_b0;
+ else if (iwl_trans->cfg == &iwl9560_2ac_cfg_qu_b0_jf_b0)
+ iwl_trans->cfg = &iwl9560_2ac_cfg_qu_c0_jf_b0;
+ else if (iwl_trans->cfg == &iwl9560_2ac_160_cfg_qu_b0_jf_b0)
+ iwl_trans->cfg = &iwl9560_2ac_160_cfg_qu_c0_jf_b0;
+ }
#endif
pci_set_drvdata(pdev, iwl_trans);
--
2.20.1
^ permalink raw reply related
* [PATCH 1/2] iwlwifi: add new cards for 9000 and 20000 series
From: Luca Coelho @ 2019-07-08 15:55 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, Ihab Zhaika, stable, Luca Coelho
In-Reply-To: <20190708155534.18241-1-luca@coelho.fi>
From: Ihab Zhaika <ihab.zhaika@intel.com>
add two new PCI ID's for 9000 and 20000 series
Cc: stable@vger.kernel.org
Signed-off-by: Ihab Zhaika <ihab.zhaika@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
index ccc83fd74649..fe645380bd2f 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
@@ -604,6 +604,7 @@ static const struct pci_device_id iwl_hw_card_ids[] = {
{IWL_PCI_DEVICE(0x2526, 0x40A4, iwl9460_2ac_cfg)},
{IWL_PCI_DEVICE(0x2526, 0x4234, iwl9560_2ac_cfg_soc)},
{IWL_PCI_DEVICE(0x2526, 0x42A4, iwl9462_2ac_cfg_soc)},
+ {IWL_PCI_DEVICE(0x2526, 0x6014, iwl9260_2ac_160_cfg)},
{IWL_PCI_DEVICE(0x2526, 0x8014, iwl9260_2ac_160_cfg)},
{IWL_PCI_DEVICE(0x2526, 0x8010, iwl9260_2ac_160_cfg)},
{IWL_PCI_DEVICE(0x2526, 0xA014, iwl9260_2ac_160_cfg)},
@@ -971,6 +972,7 @@ static const struct pci_device_id iwl_hw_card_ids[] = {
{IWL_PCI_DEVICE(0x7A70, 0x0310, iwlax211_2ax_cfg_so_gf_a0)},
{IWL_PCI_DEVICE(0x7A70, 0x0510, iwlax211_2ax_cfg_so_gf_a0)},
{IWL_PCI_DEVICE(0x7A70, 0x0A10, iwlax211_2ax_cfg_so_gf_a0)},
+ {IWL_PCI_DEVICE(0x7AF0, 0x0090, iwlax211_2ax_cfg_so_gf_a0)},
{IWL_PCI_DEVICE(0x7AF0, 0x0310, iwlax211_2ax_cfg_so_gf_a0)},
{IWL_PCI_DEVICE(0x7AF0, 0x0510, iwlax211_2ax_cfg_so_gf_a0)},
{IWL_PCI_DEVICE(0x7AF0, 0x0A10, iwlax211_2ax_cfg_so_gf_a0)},
--
2.20.1
^ permalink raw reply related
* [PATCH 0/2] iwlwifi: fixes intended for 5.3 2019-07-08
From: Luca Coelho @ 2019-07-08 15:55 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, Luca Coelho
From: Luca Coelho <luciano.coelho@intel.com>
Hi,
This is the first patchset with fixes for v5.3.
The changes are:
* A few new PCI IDs for 9000 and 22000 series;
* Support for C-step 22000 devices;
As usual, I'm pushing this to a pending branch, for kbuild bot. And
as we agreed, I'll delegate these patches to you in patchwork for you
to apply them directly. There's no hurry with this, I know we're
still in the merge window, I just wanted to send them out for people
who have these devices and are having problems. It's your call:
either send them during the merge window, if you send a new pull-req;
or take it when the merge window closes, for v5.3-rc1.
Note: all this area will probably have some conflicts when merging
with -next, because I've been sending new PCI IDs and such things for
fixes, while some other changes are being made for -next. Let me know
if you need any help merging, when time comes.
Cheers,
Luca.
Ihab Zhaika (1):
iwlwifi: add new cards for 9000 and 20000 series
Luca Coelho (1):
iwlwifi: pcie: add support for qu c-step devices
.../net/wireless/intel/iwlwifi/cfg/22000.c | 53 +++++++++++++++++++
.../net/wireless/intel/iwlwifi/iwl-config.h | 7 +++
drivers/net/wireless/intel/iwlwifi/iwl-csr.h | 2 +
drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 23 ++++++++
4 files changed, 85 insertions(+)
--
2.20.1
^ permalink raw reply
* Re: [PATCH] ath10k: work around uninitialized vht_pfr variable
From: Nathan Chancellor @ 2019-07-08 14:46 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Kalle Valo, Miaoqing Pan, David S. Miller, Rakesh Pillai,
Brian Norris, Balaji Pothunoori, Wen Gong, Pradeep kumar Chitrapu,
Sriram R, ath10k, linux-wireless, netdev, linux-kernel,
clang-built-linux
In-Reply-To: <20190708125050.3689133-1-arnd@arndb.de>
On Mon, Jul 08, 2019 at 02:50:06PM +0200, Arnd Bergmann wrote:
> As clang points out, the vht_pfr is assigned to a struct member
> without being initialized in one case:
>
> drivers/net/wireless/ath/ath10k/mac.c:7528:7: error: variable 'vht_pfr' is used uninitialized whenever 'if' condition
> is false [-Werror,-Wsometimes-uninitialized]
> if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7551:20: note: uninitialized use occurs here
> arvif->vht_pfr = vht_pfr;
> ^~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7528:3: note: remove the 'if' if its condition is always true
> if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7483:12: note: initialize the variable 'vht_pfr' to silence this warning
> u8 vht_pfr;
>
> Add an explicit but probably incorrect initialization here.
> I suspect we want a better fix here, but chose this approach to
> illustrate the issue.
Yup, I reached out to the maintainers when this issue first cropped up,
should have taken your approach though.
https://lore.kernel.org/lkml/20190702181837.GA118849@archlinux-epyc/
Initializing to zero is better than uninitialized.
>
> Fixes: 8b97b055dc9d ("ath10k: fix failure to set multiple fixed rate")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
^ permalink raw reply
* [PATCH] ath10k: work around uninitialized vht_pfr variable
From: Arnd Bergmann @ 2019-07-08 12:50 UTC (permalink / raw)
To: Kalle Valo
Cc: Arnd Bergmann, Miaoqing Pan, David S. Miller, Rakesh Pillai,
Brian Norris, Balaji Pothunoori, Wen Gong, Pradeep kumar Chitrapu,
Sriram R, ath10k, linux-wireless, netdev, linux-kernel,
clang-built-linux
As clang points out, the vht_pfr is assigned to a struct member
without being initialized in one case:
drivers/net/wireless/ath/ath10k/mac.c:7528:7: error: variable 'vht_pfr' is used uninitialized whenever 'if' condition
is false [-Werror,-Wsometimes-uninitialized]
if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/ath/ath10k/mac.c:7551:20: note: uninitialized use occurs here
arvif->vht_pfr = vht_pfr;
^~~~~~~
drivers/net/wireless/ath/ath10k/mac.c:7528:3: note: remove the 'if' if its condition is always true
if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/ath/ath10k/mac.c:7483:12: note: initialize the variable 'vht_pfr' to silence this warning
u8 vht_pfr;
Add an explicit but probably incorrect initialization here.
I suspect we want a better fix here, but chose this approach to
illustrate the issue.
Fixes: 8b97b055dc9d ("ath10k: fix failure to set multiple fixed rate")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wireless/ath/ath10k/mac.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index e43a566eef77..0606416dc971 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -7541,6 +7541,8 @@ static int ath10k_mac_op_set_bitrate_mask(struct ieee80211_hw *hw,
&vht_nss,
true);
update_bitrate_mask = false;
+ } else {
+ vht_pfr = 0;
}
mutex_lock(&ar->conf_mutex);
--
2.20.0
^ permalink raw reply related
* Re: [PATCH 4/7] mmc: sdio: Drop powered-on re-init at runtime resume and HW reset
From: Ulf Hansson @ 2019-07-08 10:53 UTC (permalink / raw)
To: Doug Anderson
Cc: Linux MMC List, Adrian Hunter, Brian Norris, Shawn Lin,
Guenter Roeck, Heiko Stuebner, Kalle Valo, Arend Van Spriel,
linux-wireless
In-Reply-To: <CAD=FV=X7P2F1k_zwHc0mbtfk55-rucTz_GoDH=PL6zWqKYcpuw@mail.gmail.com>
On Thu, 4 Jul 2019 at 02:02, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Jun 18, 2019 at 8:35 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > To use the so called powered-on re-initialization of an SDIO card, the
> > power to the card must obviously have stayed on. If not, the initialization
> > will simply fail.
> >
> > In the runtime suspend case, the card is always powered off. Hence, let's
> > drop the support for powered-on re-initialization during runtime resume, as
> > it doesn't make sense.
> >
> > Moreover, during a HW reset, the point is to cut the power to the card and
> > then do fresh re-initialization. Therefore drop the support for powered-on
> > re-initialization during HW reset.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> > drivers/mmc/core/sdio.c | 8 +-------
> > 1 file changed, 1 insertion(+), 7 deletions(-)
>
> This has been on my list of things to test for a while but I never
> quite got to it...
>
> ...and then, today, I spent time bisecting why the "reset"
> functionality of miwfiex is broken on my 4.19 kernel [1]. AKA, this
> is broken:
>
> cd /sys/kernel/debug/mwifiex/mlan0
> echo 1 > reset
>
> I finally bisected the problem and tracked it down to commit
> ca8971ca5753 ("mmc: dw_mmc: Prevent runtime PM suspend when SDIO IRQs
> are enabled"), which embarrassingly has my Tested-by on it. I guess I
> never tested the Marvell reset call. :-/
>
> I dug a little and found that when the Marvell code did its reset we
> ended up getting a call to dw_mci_enable_sdio_irq(enb=0) and never saw
> a dw_mci_enable_sdio_irq(enb=1) after. I tracked it down further and
> found that specifically it was the call to mmc_signal_sdio_irq() in
> mmc_sdio_power_restore() that was making the call. The call stack
> shown for the "enb=0" call:
>
> [<c071a290>] (dw_mci_enable_sdio_irq) from [<c070a960>]
> (mmc_sdio_power_restore+0x98/0xc0)
> [<c070a960>] (mmc_sdio_power_restore) from [<c070a9b4>]
> (mmc_sdio_reset+0x2c/0x30)
> [<c070a9b4>] (mmc_signal_sdio_irq) from [<c06ff160>] (mmc_hw_reset+0xbc/0x138)
> [<c06ff160>] (mmc_hw_reset) from [<bf1bbad8>]
> (mwifiex_sdio_work+0x5d4/0x678 [mwifiex_sdio])
> [<bf1bbad8>] (mwifiex_sdio_work [mwifiex_sdio]) from [<c0247cd0>]
> (process_one_work+0x290/0x4b4)
>
> I picked your patch here (which gets rid of the call to
> mmc_signal_sdio_irq()) and magically the problem went away because
> there is no more call to mmc_signal_sdio_irq().
>
> I personally don't have lots of history about the whole
> "powered_resume" code path. I checked and mmc_card_keep_power() was 0
> in my test case of getting called from hw_reset, so the rest of this
> patch doesn't affect me at all. This surprised me a little since I
> saw "MMC_PM_KEEP_POWER" being set in mwifiex but then I realized that
> it was only set for the duration of suspend and then cleared by the
> core. ;-)
>
> I will also say that I don't have any test case or knowledge of how
> SDIO runtime suspend/resume is supposed to work since on dw_mmc SDIO
> cards are currently not allowed to runtime suspend anyway. ;-)
>
>
> So I guess the result of all that long-winded reply is that for on
> rk3288-veyron-jerry:
>
> Fixes: ca8971ca5753 ("mmc: dw_mmc: Prevent runtime PM suspend when
> SDIO IRQs are enabled")
> Tested-by: Douglas Anderson <dianders@chromium.org>
Thanks a lot for testing and for your detailed feedback. I have
amended the patch by adding your tags above.
Moreover, we seems to need this for stable as well, but I am leaving
that to be managed as a separate task. We may even consider the hole
series for stable, but that requires more testing first.
>
>
> One last note is that, though Marvell WiFi works after a reset after
> this commit, Marvell Bluetooth (on the same SDIO module) doesn't. I
> guess next week it'll be another bisect...
Is the Bluetooth connected to the same SDIO interface, thus the
Bluetooth driver is an SDIO func driver?
>
> [1] https://crbug.com/981113
>
>
>
> -Doug
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 3/7] brcmsmac: switch source files to using SPDX license identifier
From: Greg Kroah-Hartman @ 2019-07-08 10:41 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Rafał Miłecki, Kalle Valo,
linux-wireless@vger.kernel.org, Thomas Gleixner, alan
In-Reply-To: <34aa04e4-9ddd-b6aa-721f-a20398f7740d@broadcom.com>
On Mon, Jul 08, 2019 at 12:07:43PM +0200, Arend Van Spriel wrote:
> + Alan
>
> On 5/17/2019 8:07 PM, Rafał Miłecki wrote:
> > > > > Another option could be MIT license which is in the preferred folder.
> > > > > Will have to consult our legal department about it though.
> > > > Hey, if your legal department is going to get asked this, why not just
> > > > switch it to GPLv2? That would make everything much simpler.
> > > Hah. Because I already know the answer to that.;-)
> > It's not that obvious to me, sorry. Does your legal department require
> > something more permissive than GPLv2? Is that worth asking them about
> > dual-licensing? Something like
> > GPL-2.0 OR MIT
> > ? That assures driver is compatible with Linux, no matter what's the
> > current lawyers interpretation of MIT vs. GPL 2.0. I believe Alan Cox
> > once told/suggested that dual-licensing is safer for legal reasons.
>
> Hi Alan,
>
> Rafał mentioned your name a while ago when I was struggling with the SPDX
> identifiers. The drivers sources I want to modify for this originally had a
> license text in the header that matches ISC. However,
> one of the files did not have that and it was marked in bulk to GPLv2. So
> now the question is whether I can change it to ISC like the rest or should I
> make it dual like Rafał suggested.
>
> Can you elaborate the pros and cons of dual license?
You need to talk to your lawyers about that. Please ask this of them.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 3/7] brcmsmac: switch source files to using SPDX license identifier
From: Arend Van Spriel @ 2019-07-08 10:07 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Greg Kroah-Hartman, Kalle Valo, linux-wireless@vger.kernel.org,
Thomas Gleixner, alan
In-Reply-To: <CACna6rxuSFWN8eib7FpJiVQqLwdD5GOTaAFr7msJa01rBSTLKA@mail.gmail.com>
+ Alan
On 5/17/2019 8:07 PM, Rafał Miłecki wrote:
>>>> Another option could be MIT license which is in the preferred folder.
>>>> Will have to consult our legal department about it though.
>>> Hey, if your legal department is going to get asked this, why not just
>>> switch it to GPLv2? That would make everything much simpler.
>> Hah. Because I already know the answer to that.;-)
> It's not that obvious to me, sorry. Does your legal department require
> something more permissive than GPLv2? Is that worth asking them about
> dual-licensing? Something like
> GPL-2.0 OR MIT
> ? That assures driver is compatible with Linux, no matter what's the
> current lawyers interpretation of MIT vs. GPL 2.0. I believe Alan Cox
> once told/suggested that dual-licensing is safer for legal reasons.
Hi Alan,
Rafał mentioned your name a while ago when I was struggling with the
SPDX identifiers. The drivers sources I want to modify for this
originally had a license text in the header that matches ISC. However,
one of the files did not have that and it was marked in bulk to GPLv2.
So now the question is whether I can change it to ISC like the rest or
should I make it dual like Rafał suggested.
Can you elaborate the pros and cons of dual license?
Regards,
Arend
^ permalink raw reply
* RE: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
From: David Laight @ 2019-07-08 9:18 UTC (permalink / raw)
To: 'Tony Chuang', Jian-Hong Pan
Cc: Kalle Valo, David S . Miller, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux@endlessm.com, Daniel Drake, stable@vger.kernel.org
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D1861B71@RTITMBSVM04.realtek.com.tw>
From: Tony Chuang
> Sent: 08 July 2019 10:00
> > > > @@ -803,25 +812,14 @@ static void rtw_pci_rx_isr(struct rtw_dev
> > *rtwdev,
> > > > struct rtw_pci *rtwpci,
> > > > skb_put(skb, pkt_stat.pkt_len);
> > > > skb_reserve(skb, pkt_offset);
> > > >
> > > > - /* alloc a smaller skb to mac80211 */
> > > > - new = dev_alloc_skb(pkt_stat.pkt_len);
> > > > - if (!new) {
> > > > - new = skb;
> > > > - } else {
> > > > - skb_put_data(new, skb->data,
> > skb->len);
> > > > - dev_kfree_skb_any(skb);
> > > > - }
> > >
> > > I am not sure if it's fine to deliver every huge SKB to mac80211.
> > > Because it will then be delivered to TCP/IP stack.
> > > Hence I think either it should be tested to know if the performance
> > > would be impacted or find out a more efficient way to send
> > > smaller SKB to mac80211 stack.
> >
> > I remember network stack only processes the skb with(in) pointers
> > (skb->data) and the skb->len for data part. It also checks real
> > buffer boundary (head and end) of the skb to prevent memory overflow.
> > Therefore, I think using the original skb is the most efficient way.
> >
> > If I misunderstand something, please point out.
> >
>
> It means if we still use a huge SKB (~8K) for every RX packet (~1.5K).
> There is about 6.5K not used. And even more if we ping with large packet
> size "eg. $ ping -s 65536", I am not sure if those huge SKBs will eat all of
> the SKB mem pool, and then ping fails.
>
> BTW, the original design of RTK_PCI_RX_BUF_SIZE to be (8192 + 24) is to
> receive AMSDU packet in one SKB.
> (Could probably enlarge it to RX VHT AMSDU ~11K)
If you allocate 8192+24 the memory allocated will be either 12k or 16k
and the skb truesize set appropriately.
(Probably 16k if dma memory.)
If this is fed into IP it is quite likely that a single byte of data
will end up queued on the socket in 16k of dma-able memory.
The 'truesize' stops this using all the system memory, but it isn't
good for memory usage.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* RE: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
From: Tony Chuang @ 2019-07-08 9:00 UTC (permalink / raw)
To: Jian-Hong Pan
Cc: Kalle Valo, David S . Miller, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux@endlessm.com, Daniel Drake, stable@vger.kernel.org
In-Reply-To: <CAPpJ_eebQtL0y_j98J2T7m9g77A61SVtvD8qnNN42bV0dm4MLA@mail.gmail.com>
> > > @@ -803,25 +812,14 @@ static void rtw_pci_rx_isr(struct rtw_dev
> *rtwdev,
> > > struct rtw_pci *rtwpci,
> > > skb_put(skb, pkt_stat.pkt_len);
> > > skb_reserve(skb, pkt_offset);
> > >
> > > - /* alloc a smaller skb to mac80211 */
> > > - new = dev_alloc_skb(pkt_stat.pkt_len);
> > > - if (!new) {
> > > - new = skb;
> > > - } else {
> > > - skb_put_data(new, skb->data,
> skb->len);
> > > - dev_kfree_skb_any(skb);
> > > - }
> >
> > I am not sure if it's fine to deliver every huge SKB to mac80211.
> > Because it will then be delivered to TCP/IP stack.
> > Hence I think either it should be tested to know if the performance
> > would be impacted or find out a more efficient way to send
> > smaller SKB to mac80211 stack.
>
> I remember network stack only processes the skb with(in) pointers
> (skb->data) and the skb->len for data part. It also checks real
> buffer boundary (head and end) of the skb to prevent memory overflow.
> Therefore, I think using the original skb is the most efficient way.
>
> If I misunderstand something, please point out.
>
It means if we still use a huge SKB (~8K) for every RX packet (~1.5K).
There is about 6.5K not used. And even more if we ping with large packet
size "eg. $ ping -s 65536", I am not sure if those huge SKBs will eat all of
the SKB mem pool, and then ping fails.
BTW, the original design of RTK_PCI_RX_BUF_SIZE to be (8192 + 24) is to
receive AMSDU packet in one SKB.
(Could probably enlarge it to RX VHT AMSDU ~11K)
Yan-Hsuan
^ permalink raw reply
* RE: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
From: David Laight @ 2019-07-08 8:36 UTC (permalink / raw)
To: 'Jian-Hong Pan', Yan-Hsuan Chuang, Kalle Valo,
David S . Miller
Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux@endlessm.com, Daniel Drake,
stable@vger.kernel.org
In-Reply-To: <20190708063252.4756-1-jian-hong@endlessm.com>
From: Jian-Hong Pan
> Sent: 08 July 2019 07:33
> To: Yan-Hsuan Chuang; Kalle Valo; David S . Miller
>
> Testing with RTL8822BE hardware, when available memory is low, we
> frequently see a kernel panic and system freeze.
>
> First, rtw_pci_rx_isr encounters a memory allocation failure (trimmed):
>
> rx routine starvation
> WARNING: CPU: 7 PID: 9871 at drivers/net/wireless/realtek/rtw88/pci.c:822
> rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
> [ 2356.580313] RIP: 0010:rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
>
> Then we see a variety of different error conditions and kernel panics,
> such as this one (trimmed):
>
> rtw_pci 0000:02:00.0: pci bus timeout, check dma status
> skbuff: skb_over_panic: text:00000000091b6e66 len:415 put:415 head:00000000d2880c6f
> data:000000007a02b1ea tail:0x1df end:0xc0 dev:<NULL>
> ------------[ cut here ]------------
> kernel BUG at net/core/skbuff.c:105!
> invalid opcode: 0000 [#1] SMP NOPTI
> RIP: 0010:skb_panic+0x43/0x45
>
> When skb allocation fails and the "rx routine starvation" is hit, the
> function returns immediately without updating the RX ring. At this
> point, the RX ring may continue referencing an old skb which was already
> handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
> bad things happen.
>
> This patch allocates a new skb first in RX ISR. If we don't have memory
> available, we discard the current frame, allowing the existing skb to be
> reused in the ring. Otherwise, we simplify the code flow and just hand
> over the RX-populated skb over to mac80211.
>
> In addition, to fixing the kernel crash, the RX routine should now
> generally behave better under low memory conditions.
Under low memory conditions it may be preferable to limit the amount
of memory assigned to the receive ring.
I also thought it was preferable (DM may correct me here) to do the
skb allocates from the 'bh' of the driver rather than from the hardware
interrupt.
It is also almost certainly preferable (especially on IOMMU systems)
to copy small frames into a new skb (of the right size) and then
reuse the skb (with its dma-mapped buffer) for a later frame.
Allocating a new skb before ay px processing just seems wrong...
David
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> Reviewed-by: Daniel Drake <drake@endlessm.com>
> Cc: <stable@vger.kernel.org>
> ---
> drivers/net/wireless/realtek/rtw88/pci.c | 28 +++++++++++-------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> index cfe05ba7280d..1bfc99ae6b84 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -786,6 +786,15 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> rx_desc = skb->data;
> chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
>
> + /* discard current skb if the new skb cannot be allocated as a
> + * new one in rx ring later
> + * */
> + new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> + if (WARN(!new, "rx routine starvation\n")) {
> + new = skb;
> + goto next_rp;
> + }
> +
> /* offset from rx_desc to payload */
> pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> pkt_stat.shift;
> @@ -803,25 +812,14 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> skb_put(skb, pkt_stat.pkt_len);
> skb_reserve(skb, pkt_offset);
>
> - /* alloc a smaller skb to mac80211 */
> - new = dev_alloc_skb(pkt_stat.pkt_len);
> - if (!new) {
> - new = skb;
> - } else {
> - skb_put_data(new, skb->data, skb->len);
> - dev_kfree_skb_any(skb);
> - }
> /* TODO: merge into rx.c */
> rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> - memcpy(new->cb, &rx_status, sizeof(rx_status));
> - ieee80211_rx_irqsafe(rtwdev->hw, new);
> + memcpy(skb->cb, &rx_status, sizeof(rx_status));
> + ieee80211_rx_irqsafe(rtwdev->hw, skb);
> }
>
> - /* skb delivered to mac80211, alloc a new one in rx ring */
> - new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> - if (WARN(!new, "rx routine starvation\n"))
> - return;
> -
> +next_rp:
> + /* skb delivered to mac80211, attach the new one into rx ring */
> ring->buf[cur_rp] = new;
> rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
>
> --
> 2.22.0
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
From: Jian-Hong Pan @ 2019-07-08 8:07 UTC (permalink / raw)
To: Tony Chuang
Cc: Kalle Valo, David S . Miller, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux@endlessm.com, Daniel Drake, stable@vger.kernel.org
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D1861A6D@RTITMBSVM04.realtek.com.tw>
Tony Chuang <yhchuang@realtek.com> 於 2019年7月8日 週一 下午3:23寫道：
>
> > Subject: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
>
> nit, "rtw88: pci:" would be better.
Ok.
> >
> > When skb allocation fails and the "rx routine starvation" is hit, the
> > function returns immediately without updating the RX ring. At this
> > point, the RX ring may continue referencing an old skb which was already
> > handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
> > bad things happen.
> >
> > This patch allocates a new skb first in RX ISR. If we don't have memory
> > available, we discard the current frame, allowing the existing skb to be
> > reused in the ring. Otherwise, we simplify the code flow and just hand
> > over the RX-populated skb over to mac80211.
> >
> > In addition, to fixing the kernel crash, the RX routine should now
> > generally behave better under low memory conditions.
> >
> > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
> > Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> > Reviewed-by: Daniel Drake <drake@endlessm.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> > drivers/net/wireless/realtek/rtw88/pci.c | 28 +++++++++++-------------
> > 1 file changed, 13 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
> > b/drivers/net/wireless/realtek/rtw88/pci.c
> > index cfe05ba7280d..1bfc99ae6b84 100644
> > --- a/drivers/net/wireless/realtek/rtw88/pci.c
> > +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> > @@ -786,6 +786,15 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev,
> > struct rtw_pci *rtwpci,
> > rx_desc = skb->data;
> > chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
> >
> > + /* discard current skb if the new skb cannot be allocated as a
> > + * new one in rx ring later
> > + * */
>
> nit, comment indentation.
Thanks. I will fix this.
> > + new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> > + if (WARN(!new, "rx routine starvation\n")) {
> > + new = skb;
> > + goto next_rp;
> > + }
> > +
> > /* offset from rx_desc to payload */
> > pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> > pkt_stat.shift;
> > @@ -803,25 +812,14 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev,
> > struct rtw_pci *rtwpci,
> > skb_put(skb, pkt_stat.pkt_len);
> > skb_reserve(skb, pkt_offset);
> >
> > - /* alloc a smaller skb to mac80211 */
> > - new = dev_alloc_skb(pkt_stat.pkt_len);
> > - if (!new) {
> > - new = skb;
> > - } else {
> > - skb_put_data(new, skb->data, skb->len);
> > - dev_kfree_skb_any(skb);
> > - }
>
> I am not sure if it's fine to deliver every huge SKB to mac80211.
> Because it will then be delivered to TCP/IP stack.
> Hence I think either it should be tested to know if the performance
> would be impacted or find out a more efficient way to send
> smaller SKB to mac80211 stack.
I remember network stack only processes the skb with(in) pointers
(skb->data) and the skb->len for data part. It also checks real
buffer boundary (head and end) of the skb to prevent memory overflow.
Therefore, I think using the original skb is the most efficient way.
If I misunderstand something, please point out.
> > /* TODO: merge into rx.c */
> > rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> > - memcpy(new->cb, &rx_status, sizeof(rx_status));
> > - ieee80211_rx_irqsafe(rtwdev->hw, new);
> > + memcpy(skb->cb, &rx_status, sizeof(rx_status));
> > + ieee80211_rx_irqsafe(rtwdev->hw, skb);
> > }
> >
> > - /* skb delivered to mac80211, alloc a new one in rx ring */
> > - new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> > - if (WARN(!new, "rx routine starvation\n"))
> > - return;
> > -
> > +next_rp:
> > + /* skb delivered to mac80211, attach the new one into rx ring */
> > ring->buf[cur_rp] = new;
> > rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
> >
>
> --
>
> Yan-Hsuan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox