* Re: [RFC PATCH v2 3/3] ath11k: register HE mesh capabilities
From: Bob Copeland @ 2019-06-12 16:34 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: ath11k, linux-wireless
In-Reply-To: <1919330.hVZVHELXip@bentobox>
On Tue, Jun 11, 2019 at 09:52:20PM +0200, Sven Eckelmann wrote:
> On Tuesday, 11 June 2019 20:02:47 CEST Sven Eckelmann wrote:
> [...]
> > ---
> > This doesn't work currently as expected. No HE rates are used between
> > the two HE mesh peers:
> [...]
>
> There seems to be also an ordering problem. ath11k_peer_assoc_h_he is only
> called before ieee80211_he_cap_ie_to_sta_he_cap is called. So ath11k_bss_assoc
> will not have the information whether the remote has HE support or not.
>
> Looks like I have adjust mesh_sta_info_init to get this somehow to
> ath11k_peer_assoc_h_he. Maybe through ath11k_sta_rc_update but this is not
> called by mesh_sta_info_init at the moment. Just because
> rate_control_rate_init is called and not rate_control_rate_update.
>
> The easiest method seems to adjust the check at the end of mesh_sta_info_init
> to
>
> if (!test_sta_flag(sta, WLAN_STA_RATE_CONTROL) &&
> !ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL)) {
> rate_control_rate_init(sta);
> } else {
> rate_control_rate_update(local, sband, sta, changed);
> }
Maybe we should just do this?
diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 76f303fda3ed..6f8bde840bb9 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -42,7 +42,7 @@ void rate_control_rate_init(struct sta_info *sta)
ieee80211_sta_set_rx_nss(sta);
if (!ref)
- return;
+ goto out;
rcu_read_lock();
@@ -59,6 +59,7 @@ void rate_control_rate_init(struct sta_info *sta)
priv_sta);
spin_unlock_bh(&sta->rate_ctrl_lock);
rcu_read_unlock();
+out:
set_sta_flag(sta, WLAN_STA_RATE_CONTROL);
}
That was my intent, anyway -- that NSS always got set before
rate_control_rate_update() even if using HW rate control.
> if (!test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
> rate_control_rate_init(sta);
>
> /* inform drivers about changes */
> rate_control_rate_update(local, sband, sta, changed);
>
> Both will at least cause a call to ath11k_peer_assoc_prepare +
> ath11k_wmi_send_peer_assoc_cmd but unfortunately the ath11k firmware hangs
> afterwards.
I think this would be OK.
--
Bob Copeland %% https://bobcopeland.com/
^ permalink raw reply related
* Re: [EXT] INFO: trying to register non-static key in del_timer_sync (2)
From: Andrey Konovalov @ 2019-06-12 16:13 UTC (permalink / raw)
To: Ganapathi Bhat
Cc: Dmitry Vyukov, syzbot, amitkarwar@gmail.com, davem@davemloft.net,
huxinming820@gmail.com, kvalo@codeaurora.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
nishants@marvell.com, syzkaller-bugs@googlegroups.com
In-Reply-To: <MN2PR18MB263710E8F1F8FFA06B2EDB3CA0EC0@MN2PR18MB2637.namprd18.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 348 bytes --]
On Wed, Jun 12, 2019 at 6:03 PM Ganapathi Bhat <gbhat@marvell.com> wrote:
>
> Hi Dmitry,
>
> We have a patch to fix this: https://patchwork.kernel.org/patch/10990275/
Hi Ganapathi,
Great, thanks for working on this!
We can ask syzbot to test the fix:
#syz test: https://github.com/google/kasan.git usb-fuzzer
Thanks!
>
> Regards,
> Ganapathi
[-- Attachment #2: mwifiex-avoid-deleting-uninitialized-timer-during-USB-cleanup.diff --]
[-- Type: text/x-patch, Size: 2079 bytes --]
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index c2365ee..939f1e9 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -1348,6 +1348,8 @@ static void mwifiex_usb_cleanup_tx_aggr(struct mwifiex_adapter *adapter)
for (idx = 0; idx < MWIFIEX_TX_DATA_PORT; idx++) {
port = &card->port[idx];
+ if (!port->tx_data_ep)
+ continue;
if (adapter->bus_aggr.enable)
while ((skb_tmp =
skb_dequeue(&port->tx_aggr.aggr_list)))
@@ -1365,8 +1367,6 @@ static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
mwifiex_usb_free(card);
- mwifiex_usb_cleanup_tx_aggr(adapter);
-
card->adapter = NULL;
}
@@ -1510,7 +1510,7 @@ static int mwifiex_prog_fw_w_helper(struct mwifiex_adapter *adapter,
static int mwifiex_usb_dnld_fw(struct mwifiex_adapter *adapter,
struct mwifiex_fw_image *fw)
{
- int ret;
+ int ret = 0;
struct usb_card_rec *card = (struct usb_card_rec *)adapter->card;
if (card->usb_boot_state == USB8XXX_FW_DNLD) {
@@ -1523,10 +1523,6 @@ static int mwifiex_usb_dnld_fw(struct mwifiex_adapter *adapter,
return -1;
}
- ret = mwifiex_usb_rx_init(adapter);
- if (!ret)
- ret = mwifiex_usb_tx_init(adapter);
-
return ret;
}
@@ -1584,7 +1580,29 @@ static void mwifiex_usb_submit_rem_rx_urbs(struct mwifiex_adapter *adapter)
return 0;
}
+static int mwifiex_init_usb(struct mwifiex_adapter *adapter)
+{
+ struct usb_card_rec *card = (struct usb_card_rec *)adapter->card;
+ int ret = 0;
+
+ if (card->usb_boot_state == USB8XXX_FW_DNLD)
+ return 0;
+
+ ret = mwifiex_usb_rx_init(adapter);
+ if (!ret)
+ ret = mwifiex_usb_tx_init(adapter);
+
+ return ret;
+}
+
+static void mwifiex_cleanup_usb(struct mwifiex_adapter *adapter)
+{
+ mwifiex_usb_cleanup_tx_aggr(adapter);
+}
+
static struct mwifiex_if_ops usb_ops = {
+ .init_if = mwifiex_init_usb,
+ .cleanup_if = mwifiex_cleanup_usb,
.register_dev = mwifiex_register_dev,
.unregister_dev = mwifiex_unregister_dev,
.wakeup = mwifiex_pm_wakeup_card,
^ permalink raw reply related
* RE: [EXT] INFO: trying to register non-static key in del_timer_sync (2)
From: Ganapathi Bhat @ 2019-06-12 16:01 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: syzbot, amitkarwar@gmail.com, andreyknvl@google.com,
davem@davemloft.net, huxinming820@gmail.com, kvalo@codeaurora.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
nishants@marvell.com, syzkaller-bugs@googlegroups.com
In-Reply-To: <MN2PR18MB26372D98386D79736A7947EEA0140@MN2PR18MB2637.namprd18.prod.outlook.com>
Hi Dmitry,
We have a patch to fix this: https://patchwork.kernel.org/patch/10990275/
Regards,
Ganapathi
^ permalink raw reply
* [PATCH] mwifiex: avoid deleting uninitialized timer during USB cleanup
From: Ganapathi Bhat @ 2019-06-12 15:54 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Zhiyuan Yang, James Cao, Rakesh Parmar, Brian Norris,
Dmitry Vyukov, Ganapathi Bhat
Driver calls del_timer_sync(hold_timer), in unregister_dev(), but
there exists is a case when the timer is yet to be initialized. A
restructure of init and cleanup is needed to synchronize timer
creation and delee. Make use of init_if() / cleanup_if() handlers
to get this done.
Reported-by: syzbot+373e6719b49912399d21@syzkaller.appspotmail.com
Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
---
drivers/net/wireless/marvell/mwifiex/usb.c | 32 +++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index c2365ee..939f1e9 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -1348,6 +1348,8 @@ static void mwifiex_usb_cleanup_tx_aggr(struct mwifiex_adapter *adapter)
for (idx = 0; idx < MWIFIEX_TX_DATA_PORT; idx++) {
port = &card->port[idx];
+ if (!port->tx_data_ep)
+ continue;
if (adapter->bus_aggr.enable)
while ((skb_tmp =
skb_dequeue(&port->tx_aggr.aggr_list)))
@@ -1365,8 +1367,6 @@ static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
mwifiex_usb_free(card);
- mwifiex_usb_cleanup_tx_aggr(adapter);
-
card->adapter = NULL;
}
@@ -1510,7 +1510,7 @@ static int mwifiex_prog_fw_w_helper(struct mwifiex_adapter *adapter,
static int mwifiex_usb_dnld_fw(struct mwifiex_adapter *adapter,
struct mwifiex_fw_image *fw)
{
- int ret;
+ int ret = 0;
struct usb_card_rec *card = (struct usb_card_rec *)adapter->card;
if (card->usb_boot_state == USB8XXX_FW_DNLD) {
@@ -1523,10 +1523,6 @@ static int mwifiex_usb_dnld_fw(struct mwifiex_adapter *adapter,
return -1;
}
- ret = mwifiex_usb_rx_init(adapter);
- if (!ret)
- ret = mwifiex_usb_tx_init(adapter);
-
return ret;
}
@@ -1584,7 +1580,29 @@ static void mwifiex_usb_submit_rem_rx_urbs(struct mwifiex_adapter *adapter)
return 0;
}
+static int mwifiex_init_usb(struct mwifiex_adapter *adapter)
+{
+ struct usb_card_rec *card = (struct usb_card_rec *)adapter->card;
+ int ret = 0;
+
+ if (card->usb_boot_state == USB8XXX_FW_DNLD)
+ return 0;
+
+ ret = mwifiex_usb_rx_init(adapter);
+ if (!ret)
+ ret = mwifiex_usb_tx_init(adapter);
+
+ return ret;
+}
+
+static void mwifiex_cleanup_usb(struct mwifiex_adapter *adapter)
+{
+ mwifiex_usb_cleanup_tx_aggr(adapter);
+}
+
static struct mwifiex_if_ops usb_ops = {
+ .init_if = mwifiex_init_usb,
+ .cleanup_if = mwifiex_cleanup_usb,
.register_dev = mwifiex_register_dev,
.unregister_dev = mwifiex_unregister_dev,
.wakeup = mwifiex_pm_wakeup_card,
--
1.9.1
^ permalink raw reply related
* RE: [PATCH 09/11] mwifiex: update set_mac_address logic
From: Ganapathi Bhat @ 2019-06-12 15:18 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org
Cc: Cathy Luo, Zhiyuan Yang, James Cao, Rakesh Parmar,
Sharvari Harisangam
In-Reply-To: <1560352331-16898-1-git-send-email-gbhat@marvell.com>
Hi Kalle,
This change is correct one, but I missed changing the .patch file name before sending. Let me know if this needs to be resend.
Regards,
Ganapathi
^ permalink raw reply
* [PATCH 09/11] mwifiex: update set_mac_address logic
From: Ganapathi Bhat @ 2019-06-12 15:12 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Zhiyuan Yang, James Cao, Rakesh Parmar,
Sharvari Harisangam, Ganapathi Bhat
From: Sharvari Harisangam <sharvari@marvell.com>
In set_mac_address, driver check for interfaces with same bss_type
For first STA entry, this would return 3 interfaces since all priv's have
bss_type as 0 due to kzalloc. Thus mac address gets changed for STA
unexpected. This patch adds check for first STA and avoids mac address
change. This patch also adds mac_address change for p2p based on bss_num
type.
Signed-off-by: Sharvari Harisangam <sharvari@marvell.com>
Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
---
drivers/net/wireless/marvell/mwifiex/main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 5ed2d9b..4eabd94 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -965,10 +965,10 @@ int mwifiex_set_mac_address(struct mwifiex_private *priv,
mac_addr = old_mac_addr;
- if (priv->bss_type == MWIFIEX_BSS_TYPE_P2P)
+ if (priv->bss_type == MWIFIEX_BSS_TYPE_P2P) {
mac_addr |= BIT_ULL(MWIFIEX_MAC_LOCAL_ADMIN_BIT);
-
- if (mwifiex_get_intf_num(priv->adapter, priv->bss_type) > 1) {
+ mac_addr += priv->bss_num;
+ } else if (priv->adapter->priv[0] != priv) {
/* Set mac address based on bss_type/bss_num */
mac_addr ^= BIT_ULL(priv->bss_type + 8);
mac_addr += priv->bss_num;
--
1.9.1
^ permalink raw reply related
* RE: [PATCH] mwifiex: update set_mac_address logic
From: Ganapathi Bhat @ 2019-06-12 15:07 UTC (permalink / raw)
To: Ganapathi Bhat, linux-wireless@vger.kernel.org
In-Reply-To: <1560351524-16779-1-git-send-email-gbhat@marvell.com>
Hi Kalle,
Kindly ignore this change. I will resend this change with original author name;
I had an issue with my 'git send-email'; and was trying to fix it. Now it is working.
Regards,
Ganapathi
^ permalink raw reply
* Re: [PATCH] wlcore/wl18xx: Add invert-irq OF property for physically inverted IRQ
From: Eugeniu Rosca @ 2019-06-12 15:06 UTC (permalink / raw)
To: Marc Zyngier
Cc: Geert Uytterhoeven, Tony Lindgren, Kalle Valo, Eyal Reizer,
Simon Horman, David S. Miller, Greg Kroah-Hartman, Randy Dunlap,
Ulf Hansson, John Stultz, linux-wireless, netdev,
Linux Kernel Mailing List, Spyridon Papageorgiou, Joshua Frkuska,
George G . Davis, Andrey Gusakov, Linux-Renesas, Eugeniu Rosca,
Eugeniu Rosca, Thomas Gleixner, Jason Cooper, Linus Walleij,
Harish Jenny K N
In-Reply-To: <86d0jjglax.wl-marc.zyngier@arm.com>
Hi Marc,
Thanks for your comment.
On Wed, Jun 12, 2019 at 11:17:10AM +0100, Marc Zyngier wrote:
> Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
> > On Tue, Jun 11, 2019 at 10:00:41AM +0100, Marc Zyngier wrote:
[..]
> > > We already have plenty of that in the tree, the canonical example
> > > probably being drivers/irqchip/irq-mtk-sysirq.c. It should be pretty
> > > easy to turn this driver into something more generic.
> >
> > I don't think drivers/irqchip/irq-mtk-sysirq.c can serve the
> > use-case/purpose of this patch. The MTK driver seems to be dealing with
> > the polarity inversion of on-SoC interrupts which are routed to GiC,
> > whereas in this patch we are talking about an off-chip interrupt
> > wired to R-Car GPIO controller.
>
> And how different is that? The location of the interrupt source is
> pretty irrelevant here.
The main difference which I sense is that a driver like irq-mtk-sysirq
mostly (if not exclusively) deals with internal kernel implementation
detail (tuned via DT) whilst adding an inverter for GPIO IRQs raises
a whole bunch of new questions (e.g. how to arbitrate between
kernel-space and user-space IRQ polarity configuration?).
> The point is that there is already a general
> scheme to deal with these "signal altering widgets", and that we
> should try to reuse at least the concept, if not the code.
Since Harish Jenny K N might be working on a new driver doing GPIO IRQ
inversion, I have CC-ed him as well to avoid any overlapping work.
>
> > It looks to me that the nice DTS sketch shared by Linus Walleij in [5]
> > might come closer to the concept proposed by Geert? FWIW, the
> > infrastructure/implementation to make this possible is still not
> > ready.
>
> Which looks like what I'm suggesting.
Then we are on the same page. Thanks.
>
> M.
>
> --
> Jazz is not dead, it just smells funny.
--
Best Regards,
Eugeniu.
^ permalink raw reply
* [PATCH] mwifiex: update set_mac_address logic
From: Ganapathi Bhat @ 2019-06-12 14:58 UTC (permalink / raw)
To: linux-wireless; +Cc: Ganapathi Bhat
In set_mac_address, driver check for interfaces with same bss_type
For first STA entry, this would return 3 interfaces since all priv's have
bss_type as 0 due to kzalloc. Thus mac address gets changed for STA
unexpected. This patch adds check for first STA and avoids mac address
change. This patch also adds mac_address change for p2p based on bss_num
type.
Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
---
drivers/net/wireless/marvell/mwifiex/main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index f6da8ed..4c14b48 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -960,10 +960,10 @@ int mwifiex_set_mac_address(struct mwifiex_private *priv,
mac_addr = old_mac_addr;
- if (priv->bss_type == MWIFIEX_BSS_TYPE_P2P)
+ if (priv->bss_type == MWIFIEX_BSS_TYPE_P2P) {
mac_addr |= BIT_ULL(MWIFIEX_MAC_LOCAL_ADMIN_BIT);
-
- if (mwifiex_get_intf_num(priv->adapter, priv->bss_type) > 1) {
+ mac_addr += priv->bss_num;
+ } else if (priv->adapter->priv[0] != priv) {
/* Set mac address based on bss_type/bss_num */
mac_addr ^= BIT_ULL(priv->bss_type + 8);
mac_addr += priv->bss_num;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2 2/2] mt76: usb: do not always copy the first part of received frames
From: Lorenzo Bianconi @ 2019-06-12 14:44 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Lorenzo Bianconi, nbd, kvalo, linux-wireless
In-Reply-To: <20190612142128.GA20760@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1955 bytes --]
On Jun 12, Stanislaw Gruszka wrote:
> On Wed, Jun 12, 2019 at 02:59:05PM +0200, Stanislaw Gruszka wrote:
> > > > If max RX AMSDU size is 3839B I do not see reason why we allocate
> > > > MT_SG_MAX_SIZE=8 of MT_RX_BUF_SIZE=2k buffers for sg_en case.
> > > > I thought the reason is that max AMSDU size is 16kB so it fit into
> > > > 8 sg buffers of 2k.
> > > >
> > > > In other words, for me, looks like either
> > > > - we can not handle AMSDU for non sg case because we do not
> > > > allocate big enough buffer
> > >
> > > I think AMSDU is mandatory and we currently support it even for non-sg case
> > > (since max rx AMSDU is 3839B)
> > >
> > > > or
> > > > - we can just use one PAGE_SIZE buffer for rx and remove sg
> > > > buffers for rx completely
> > >
> > > using sg buffers we can support bigger rx AMSDU size in the future without using
> > > huge buffers (e.g. we can try to use IEEE80211_MAX_MPDU_LEN_HT_7935 with
> > > mt76x2u)
> >
> > I think it would be simpler just to allocate 2 pages for 7935B .
>
> And if we could determine that there is no true need to use sg for rx,
> I think best approach here would be revert f8f527b16db5 in v5.2 to fix
> regression and remove rx sg in -next. That would make code simpler,
> allocate 4k instead 16k per packet, allow to use build_skb (4096 - 3839
> give enough space for shared info) and not use usb hcd bounce buffer.
I do not think we should drop sg support since:
- it allow us to rx huge amsdu frames (e.g. IEEE80211_MAX_MPDU_LEN_VHT_11454)
using multiple one page buffer. I think there will be new usb devices where we can
increase amsdu size (we can increase it even on mt76x2u usb 3.0 devices)
- without SG we can't use build_skb() without copying a given size of the packet since the
space needed for skb_shared_info is 320B on a x86_64 device
- the fix for f8f527b16db5 has been already tested
Regards,
Lorenzo
>
> Stanislaw
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v2 3/4] mt76: mt76x02: fix tx status reporting issues
From: Felix Fietkau @ 2019-06-12 14:38 UTC (permalink / raw)
To: linux-wireless
In-Reply-To: <20190607164355.51876-3-nbd@nbd.name>
When the hardware falls back to lower rates for a transmit attempt, only the
first status report will show the number of retries correctly. The frames
that follow will report the correct final rate, but number of retries set to 0.
This can cause the rate control module to vastly underestimate the number of
retransmissions per rate.
To fix this, we need to keep track of the initial requested tx rate per packet
and pass it to the status information.
For frames with tx status requested, this is simple: use the rate configured
in info->control.rates[0] as reference.
For no-skb tx status information, we have to encode the requested tx rate in
the packet id (and make it possible to distinguish it from real packet ids).
To do that, reduce the packet id field size by one bit, and use that bit to
indicate packet id vs rate.
This change also improves reporting by filling the status rate array with
rates from first rate to final rate, taking the same steps as the hardware
fallback table. This matters in corner cases like MCS8 on HT, where the
fallback target is MCS0, not MCS7.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
v2: fix reporting of non-probing frames with tx status requested
drivers/net/wireless/mediatek/mt76/mt76.h | 11 +-
.../net/wireless/mediatek/mt76/mt76x02_mac.c | 102 +++++++++++++++---
.../net/wireless/mediatek/mt76/mt76x02_txrx.c | 7 +-
.../wireless/mediatek/mt76/mt76x02_usb_core.c | 7 +-
4 files changed, 107 insertions(+), 20 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index fc4169c83e76..907bec9d5e4c 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -258,10 +258,11 @@ struct mt76_rx_tid {
#define MT_TX_CB_TXS_DONE BIT(1)
#define MT_TX_CB_TXS_FAILED BIT(2)
-#define MT_PACKET_ID_MASK GENMASK(7, 0)
+#define MT_PACKET_ID_MASK GENMASK(6, 0)
#define MT_PACKET_ID_NO_ACK 0
#define MT_PACKET_ID_NO_SKB 1
#define MT_PACKET_ID_FIRST 2
+#define MT_PACKET_ID_HAS_RATE BIT(7)
#define MT_TX_STATUS_SKB_TIMEOUT HZ
@@ -689,6 +690,14 @@ static inline void mt76_insert_hdr_pad(struct sk_buff *skb)
skb->data[len + 1] = 0;
}
+static inline bool mt76_is_skb_pktid(u8 pktid)
+{
+ if (pktid & MT_PACKET_ID_HAS_RATE)
+ return false;
+
+ return pktid >= MT_PACKET_ID_FIRST;
+}
+
void mt76_rx(struct mt76_dev *dev, enum mt76_rxq_id q, struct sk_buff *skb);
void mt76_tx(struct mt76_dev *dev, struct ieee80211_sta *sta,
struct mt76_wcid *wcid, struct sk_buff *skb);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
index ee4a86971be7..82bafb5ac326 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
@@ -420,30 +420,92 @@ void mt76x02_mac_write_txwi(struct mt76x02_dev *dev, struct mt76x02_txwi *txwi,
EXPORT_SYMBOL_GPL(mt76x02_mac_write_txwi);
static void
-mt76x02_mac_fill_tx_status(struct mt76x02_dev *dev,
+mt76x02_tx_rate_fallback(struct ieee80211_tx_rate *rates, int idx, int phy)
+{
+ u8 mcs, nss;
+
+ if (!idx)
+ return;
+
+ rates += idx - 1;
+ rates[1] = rates[0];
+ switch (phy) {
+ case MT_PHY_TYPE_VHT:
+ mcs = ieee80211_rate_get_vht_mcs(rates);
+ nss = ieee80211_rate_get_vht_nss(rates);
+
+ if (mcs == 0)
+ nss = max_t(int, nss - 1, 1);
+ else
+ mcs--;
+
+ ieee80211_rate_set_vht(rates + 1, mcs, nss);
+ break;
+ case MT_PHY_TYPE_HT_GF:
+ case MT_PHY_TYPE_HT:
+ /* MCS 8 falls back to MCS 0 */
+ if (rates[0].idx == 8) {
+ rates[1].idx = 0;
+ break;
+ }
+ /* fall through */
+ default:
+ rates[1].idx = max_t(int, rates[0].idx - 1, 0);
+ break;
+ }
+}
+
+static void
+mt76x02_mac_fill_tx_status(struct mt76x02_dev *dev, struct mt76x02_sta *msta,
struct ieee80211_tx_info *info,
struct mt76x02_tx_status *st, int n_frames)
{
struct ieee80211_tx_rate *rate = info->status.rates;
- int cur_idx, last_rate;
+ struct ieee80211_tx_rate last_rate;
+ u16 first_rate;
+ int retry = st->retry;
+ int phy;
int i;
if (!n_frames)
return;
- last_rate = min_t(int, st->retry, IEEE80211_TX_MAX_RATES - 1);
- mt76x02_mac_process_tx_rate(&rate[last_rate], st->rate,
+ phy = FIELD_GET(MT_RXWI_RATE_PHY, st->rate);
+
+ if (st->pktid & MT_PACKET_ID_HAS_RATE) {
+ first_rate = st->rate & ~MT_RXWI_RATE_INDEX;
+ first_rate |= st->pktid & MT_RXWI_RATE_INDEX;
+
+ mt76x02_mac_process_tx_rate(&rate[0], first_rate,
+ dev->mt76.chandef.chan->band);
+ } else if (rate[0].idx < 0) {
+ if (!msta)
+ return;
+
+ mt76x02_mac_process_tx_rate(&rate[0], msta->wcid.tx_info,
+ dev->mt76.chandef.chan->band);
+ }
+
+ mt76x02_mac_process_tx_rate(&last_rate, st->rate,
dev->mt76.chandef.chan->band);
- if (last_rate < IEEE80211_TX_MAX_RATES - 1)
- rate[last_rate + 1].idx = -1;
-
- cur_idx = rate[last_rate].idx + last_rate;
- for (i = 0; i <= last_rate; i++) {
- rate[i].flags = rate[last_rate].flags;
- rate[i].idx = max_t(int, 0, cur_idx - i);
- rate[i].count = 1;
+
+ for (i = 0; i < ARRAY_SIZE(info->status.rates); i++) {
+ retry--;
+ if (i + 1 == ARRAY_SIZE(info->status.rates)) {
+ info->status.rates[i] = last_rate;
+ info->status.rates[i].count = max_t(int, retry, 1);
+ break;
+ }
+
+ mt76x02_tx_rate_fallback(info->status.rates, i, phy);
+ if (info->status.rates[i].idx == last_rate.idx)
+ break;
+ }
+
+ if (i + 1 < ARRAY_SIZE(info->status.rates)) {
+ info->status.rates[i + 1].idx = -1;
+ info->status.rates[i + 1].count = 0;
}
- rate[last_rate].count = st->retry + 1 - last_rate;
info->status.ampdu_len = n_frames;
info->status.ampdu_ack_len = st->success ? n_frames : 0;
@@ -489,13 +551,19 @@ void mt76x02_send_tx_status(struct mt76x02_dev *dev,
mt76_tx_status_lock(mdev, &list);
if (wcid) {
- if (stat->pktid >= MT_PACKET_ID_FIRST)
+ if (mt76_is_skb_pktid(stat->pktid))
status.skb = mt76_tx_status_skb_get(mdev, wcid,
stat->pktid, &list);
if (status.skb)
status.info = IEEE80211_SKB_CB(status.skb);
}
+ if (!status.skb && !(stat->pktid & MT_PACKET_ID_HAS_RATE)) {
+ mt76_tx_status_unlock(mdev, &list);
+ rcu_read_unlock();
+ return;
+ }
+
if (msta && stat->aggr && !status.skb) {
u32 stat_val, stat_cache;
@@ -512,14 +580,14 @@ void mt76x02_send_tx_status(struct mt76x02_dev *dev,
return;
}
- mt76x02_mac_fill_tx_status(dev, status.info, &msta->status,
- msta->n_frames);
+ mt76x02_mac_fill_tx_status(dev, msta, status.info,
+ &msta->status, msta->n_frames);
msta->status = *stat;
msta->n_frames = 1;
*update = 0;
} else {
- mt76x02_mac_fill_tx_status(dev, status.info, stat, 1);
+ mt76x02_mac_fill_tx_status(dev, msta, status.info, stat, 1);
*update = 1;
}
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c b/drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c
index cf7abd9b7d2e..95c73049a68b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c
@@ -164,9 +164,14 @@ int mt76x02_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
mt76x02_mac_write_txwi(dev, txwi, tx_info->skb, wcid, sta, len);
pid = mt76_tx_status_skb_add(mdev, wcid, tx_info->skb);
+
+ /* encode packet rate for no-skb packet id to fix up status reporting */
+ if (pid == MT_PACKET_ID_NO_SKB)
+ pid = MT_PACKET_ID_HAS_RATE | (txwi->rate & MT_RXWI_RATE_INDEX);
+
txwi->pktid = pid;
- if (pid >= MT_PACKET_ID_FIRST)
+ if (mt76_is_skb_pktid(pid))
qsel = MT_QSEL_MGMT;
tx_info->info = FIELD_PREP(MT_TXD_INFO_QSEL, qsel) |
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
index 6b89f7eab26c..2436f14ca24a 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
@@ -89,9 +89,14 @@ int mt76x02u_tx_prepare_skb(struct mt76_dev *mdev, void *data,
skb_push(tx_info->skb, sizeof(*txwi));
pid = mt76_tx_status_skb_add(mdev, wcid, tx_info->skb);
+
+ /* encode packet rate for no-skb packet id to fix up status reporting */
+ if (pid == MT_PACKET_ID_NO_SKB)
+ pid = MT_PACKET_ID_HAS_RATE | (txwi->rate & MT_RXWI_RATE_INDEX);
+
txwi->pktid = pid;
- if (pid >= MT_PACKET_ID_FIRST || ep == MT_EP_OUT_HCCA)
+ if (mt76_is_skb_pktid(pid) || ep == MT_EP_OUT_HCCA)
qsel = MT_QSEL_MGMT;
else
qsel = MT_QSEL_EDCA;
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] mt76: usb: do not always copy the first part of received frames
From: Lorenzo Bianconi @ 2019-06-12 14:27 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Lorenzo Bianconi, nbd, kvalo, linux-wireless
In-Reply-To: <20190612125905.GB2600@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 663 bytes --]
> On Wed, Jun 12, 2019 at 02:28:48PM +0200, Lorenzo Bianconi wrote:
[...]
> >
> > using sg buffers we can support bigger rx AMSDU size in the future without using
> > huge buffers (e.g. we can try to use IEEE80211_MAX_MPDU_LEN_HT_7935 with
> > mt76x2u)
>
> I think it would be simpler just to allocate 2 pages for 7935B .
>
We should avoid increasing buffer size to more than PAGE_SIZE for
performance reasons. As suggested by Felix what about of setting buf_size to
PAGE_SIZE for both sg and non-sg cases and for sg setting usb data size to
SKB_WITH_OVERHEAD(q->buf_size) & (usb_endpoint_maxp() - 1);
Regards,
Lorenzo
> Stanislaw
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH 3/5] iwlwifi: dvm: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-06-12 14:26 UTC (permalink / raw)
To: Johannes Berg
Cc: Greg Kroah-Hartman, Emmanuel Grumbach, Luca Coelho,
Intel Linux Wireless, Kalle Valo, David S. Miller, linux-wireless,
netdev
In-Reply-To: <20190612142658.12792-1-gregkh@linuxfoundation.org>
When calling debugfs functions, there is no need to ever check the
return value. This driver was saving the debugfs file away to be
removed at a later time. However, the 80211 core would delete the whole
directory that the debugfs files are created in, after it asks the
driver to do the deletion, so just rely on the 80211 core to do all of
the cleanup for us, making us not need to keep a pointer to the dentries
around at all.
This cleans up the structure of the driver data a bit and makes the code
a tiny bit smaller.
Cc: Johannes Berg <johannes.berg@intel.com>
Cc: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Cc: Luca Coelho <luciano.coelho@intel.com>
Cc: Intel Linux Wireless <linuxwifi@intel.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/wireless/intel/iwlwifi/dvm/rs.c | 29 ++++++---------------
drivers/net/wireless/intel/iwlwifi/dvm/rs.h | 4 ---
2 files changed, 8 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/rs.c b/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
index ef4b9de256f7..91e571c99f81 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
@@ -3271,28 +3271,16 @@ static void rs_add_debugfs(void *priv, void *priv_sta,
struct dentry *dir)
{
struct iwl_lq_sta *lq_sta = priv_sta;
- lq_sta->rs_sta_dbgfs_scale_table_file =
- debugfs_create_file("rate_scale_table", 0600, dir,
- lq_sta, &rs_sta_dbgfs_scale_table_ops);
- lq_sta->rs_sta_dbgfs_stats_table_file =
- debugfs_create_file("rate_stats_table", 0400, dir,
- lq_sta, &rs_sta_dbgfs_stats_table_ops);
- lq_sta->rs_sta_dbgfs_rate_scale_data_file =
- debugfs_create_file("rate_scale_data", 0400, dir,
- lq_sta, &rs_sta_dbgfs_rate_scale_data_ops);
- lq_sta->rs_sta_dbgfs_tx_agg_tid_en_file =
- debugfs_create_u8("tx_agg_tid_enable", 0600, dir,
- &lq_sta->tx_agg_tid_en);
-}
+ debugfs_create_file("rate_scale_table", 0600, dir, lq_sta,
+ &rs_sta_dbgfs_scale_table_ops);
+ debugfs_create_file("rate_stats_table", 0400, dir, lq_sta,
+ &rs_sta_dbgfs_stats_table_ops);
+ debugfs_create_file("rate_scale_data", 0400, dir, lq_sta,
+ &rs_sta_dbgfs_rate_scale_data_ops);
+ debugfs_create_u8("tx_agg_tid_enable", 0600, dir,
+ &lq_sta->tx_agg_tid_en);
-static void rs_remove_debugfs(void *priv, void *priv_sta)
-{
- struct iwl_lq_sta *lq_sta = priv_sta;
- debugfs_remove(lq_sta->rs_sta_dbgfs_scale_table_file);
- debugfs_remove(lq_sta->rs_sta_dbgfs_stats_table_file);
- debugfs_remove(lq_sta->rs_sta_dbgfs_rate_scale_data_file);
- debugfs_remove(lq_sta->rs_sta_dbgfs_tx_agg_tid_en_file);
}
#endif
@@ -3318,7 +3306,6 @@ static const struct rate_control_ops rs_ops = {
.free_sta = rs_free_sta,
#ifdef CONFIG_MAC80211_DEBUGFS
.add_sta_debugfs = rs_add_debugfs,
- .remove_sta_debugfs = rs_remove_debugfs,
#endif
};
diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/rs.h b/drivers/net/wireless/intel/iwlwifi/dvm/rs.h
index b2df3a8cc464..92836a815234 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/rs.h
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/rs.h
@@ -367,10 +367,6 @@ struct iwl_lq_sta {
struct iwl_traffic_load load[IWL_MAX_TID_COUNT];
u8 tx_agg_tid_en;
#ifdef CONFIG_MAC80211_DEBUGFS
- struct dentry *rs_sta_dbgfs_scale_table_file;
- struct dentry *rs_sta_dbgfs_stats_table_file;
- struct dentry *rs_sta_dbgfs_rate_scale_data_file;
- struct dentry *rs_sta_dbgfs_tx_agg_tid_en_file;
u32 dbg_fixed_rate;
#endif
struct iwl_priv *drv;
--
2.22.0
^ permalink raw reply related
* [PATCH 5/5] mac80211: remove unused and unneeded remove_sta_debugfs callback
From: Greg Kroah-Hartman @ 2019-06-12 14:26 UTC (permalink / raw)
To: Johannes Berg
Cc: Greg Kroah-Hartman, Johannes Berg, David S. Miller,
linux-wireless, netdev
In-Reply-To: <20190612142658.12792-1-gregkh@linuxfoundation.org>
The remove_sta_debugfs callback in struct rate_control_ops is no longer
used by any driver, as there is no need for it (the debugfs directory is
already removed recursivly by the mac80211 core.) Because no one needs
it, just remove it to keep anyone else from accidentally using it in the
future.
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
include/net/mac80211.h | 1 -
net/mac80211/rate.h | 9 ---------
net/mac80211/sta_info.c | 1 -
3 files changed, 11 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 72080d9d617e..f42c61422fdf 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5939,7 +5939,6 @@ struct rate_control_ops {
void (*add_sta_debugfs)(void *priv, void *priv_sta,
struct dentry *dir);
- void (*remove_sta_debugfs)(void *priv, void *priv_sta);
u32 (*get_expected_throughput)(void *priv_sta);
};
diff --git a/net/mac80211/rate.h b/net/mac80211/rate.h
index d59198191a79..a94ce3804962 100644
--- a/net/mac80211/rate.h
+++ b/net/mac80211/rate.h
@@ -63,15 +63,6 @@ static inline void rate_control_add_sta_debugfs(struct sta_info *sta)
#endif
}
-static inline void rate_control_remove_sta_debugfs(struct sta_info *sta)
-{
-#ifdef CONFIG_MAC80211_DEBUGFS
- struct rate_control_ref *ref = sta->rate_ctrl;
- if (ref && ref->ops->remove_sta_debugfs)
- ref->ops->remove_sta_debugfs(ref->priv, sta->rate_ctrl_priv);
-#endif
-}
-
void ieee80211_check_rate_mask(struct ieee80211_sub_if_data *sdata);
/* Get a reference to the rate control algorithm. If `name' is NULL, get the
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index a4932ee3595c..d2933b9f8197 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1027,7 +1027,6 @@ static void __sta_info_destroy_part2(struct sta_info *sta)
cfg80211_del_sta_sinfo(sdata->dev, sta->sta.addr, sinfo, GFP_KERNEL);
kfree(sinfo);
- rate_control_remove_sta_debugfs(sta);
ieee80211_sta_debugfs_remove(sta);
cleanup_single_sta(sta);
--
2.22.0
^ permalink raw reply related
* [PATCH 4/5] iwlwifi: mvm: remove unused .remove_sta_debugfs callback
From: Greg Kroah-Hartman @ 2019-06-12 14:26 UTC (permalink / raw)
To: Johannes Berg
Cc: Greg Kroah-Hartman, Emmanuel Grumbach, Luca Coelho,
Intel Linux Wireless, Kalle Valo, David S. Miller, Sara Sharon,
Erel Geron, Avraham Stern, linux-wireless, netdev
In-Reply-To: <20190612142658.12792-1-gregkh@linuxfoundation.org>
The .remove_sta_debugfs callback was not doing anything in this driver,
so remove it as it is not needed.
Cc: Johannes Berg <johannes.berg@intel.com>
Cc: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Cc: Luca Coelho <luciano.coelho@intel.com>
Cc: Intel Linux Wireless <linuxwifi@intel.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Sara Sharon <sara.sharon@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Erel Geron <erelx.geron@intel.com>
Cc: Avraham Stern <avraham.stern@intel.com>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/wireless/intel/iwlwifi/mvm/rs.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
index c182821ab22b..15c1f825b749 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
@@ -4108,10 +4108,6 @@ static void rs_drv_add_sta_debugfs(void *mvm, void *priv_sta,
MVM_DEBUGFS_ADD_FILE_RS(ss_force, dir, 0600);
}
-
-void rs_remove_sta_debugfs(void *mvm, void *mvm_sta)
-{
-}
#endif
/*
@@ -4139,7 +4135,6 @@ static const struct rate_control_ops rs_mvm_ops_drv = {
.rate_update = rs_drv_rate_update,
#ifdef CONFIG_MAC80211_DEBUGFS
.add_sta_debugfs = rs_drv_add_sta_debugfs,
- .remove_sta_debugfs = rs_remove_sta_debugfs,
#endif
.capa = RATE_CTRL_CAPA_VHT_EXT_NSS_BW,
};
--
2.22.0
^ permalink raw reply related
* [PATCH 2/5] iwlegacy: 4965: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-06-12 14:26 UTC (permalink / raw)
To: Johannes Berg
Cc: Greg Kroah-Hartman, Stanislaw Gruszka, Kalle Valo,
David S. Miller, linux-wireless, netdev
In-Reply-To: <20190612142658.12792-1-gregkh@linuxfoundation.org>
When calling debugfs functions, there is no need to ever check the
return value. This driver was saving the debugfs file away to be
removed at a later time. However, the 80211 core would delete the whole
directory that the debugfs files are created in, after it asks the
driver to do the deletion, so just rely on the 80211 core to do all of
the cleanup for us, making us not need to keep a pointer to the dentries
around at all.
This cleans up the structure of the driver data a bit and makes the code
a tiny bit smaller.
Cc: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/wireless/intel/iwlegacy/4965-rs.c | 31 +++++--------------
drivers/net/wireless/intel/iwlegacy/common.h | 4 ---
2 files changed, 8 insertions(+), 27 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlegacy/4965-rs.c b/drivers/net/wireless/intel/iwlegacy/4965-rs.c
index 54ff83829afb..f640709d9862 100644
--- a/drivers/net/wireless/intel/iwlegacy/4965-rs.c
+++ b/drivers/net/wireless/intel/iwlegacy/4965-rs.c
@@ -2767,29 +2767,15 @@ static void
il4965_rs_add_debugfs(void *il, void *il_sta, struct dentry *dir)
{
struct il_lq_sta *lq_sta = il_sta;
- lq_sta->rs_sta_dbgfs_scale_table_file =
- debugfs_create_file("rate_scale_table", 0600, dir,
- lq_sta, &rs_sta_dbgfs_scale_table_ops);
- lq_sta->rs_sta_dbgfs_stats_table_file =
- debugfs_create_file("rate_stats_table", 0400, dir, lq_sta,
- &rs_sta_dbgfs_stats_table_ops);
- lq_sta->rs_sta_dbgfs_rate_scale_data_file =
- debugfs_create_file("rate_scale_data", 0400, dir, lq_sta,
- &rs_sta_dbgfs_rate_scale_data_ops);
- lq_sta->rs_sta_dbgfs_tx_agg_tid_en_file =
- debugfs_create_u8("tx_agg_tid_enable", 0600, dir,
- &lq_sta->tx_agg_tid_en);
-}
-
-static void
-il4965_rs_remove_debugfs(void *il, void *il_sta)
-{
- struct il_lq_sta *lq_sta = il_sta;
- debugfs_remove(lq_sta->rs_sta_dbgfs_scale_table_file);
- debugfs_remove(lq_sta->rs_sta_dbgfs_stats_table_file);
- debugfs_remove(lq_sta->rs_sta_dbgfs_rate_scale_data_file);
- debugfs_remove(lq_sta->rs_sta_dbgfs_tx_agg_tid_en_file);
+ debugfs_create_file("rate_scale_table", 0600, dir, lq_sta,
+ &rs_sta_dbgfs_scale_table_ops);
+ debugfs_create_file("rate_stats_table", 0400, dir, lq_sta,
+ &rs_sta_dbgfs_stats_table_ops);
+ debugfs_create_file("rate_scale_data", 0400, dir, lq_sta,
+ &rs_sta_dbgfs_rate_scale_data_ops);
+ debugfs_create_u8("tx_agg_tid_enable", 0600, dir,
+ &lq_sta->tx_agg_tid_en);
}
#endif
@@ -2816,7 +2802,6 @@ static const struct rate_control_ops rs_4965_ops = {
.free_sta = il4965_rs_free_sta,
#ifdef CONFIG_MAC80211_DEBUGFS
.add_sta_debugfs = il4965_rs_add_debugfs,
- .remove_sta_debugfs = il4965_rs_remove_debugfs,
#endif
};
diff --git a/drivers/net/wireless/intel/iwlegacy/common.h b/drivers/net/wireless/intel/iwlegacy/common.h
index 986646af8dfd..3c6f41763dde 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.h
+++ b/drivers/net/wireless/intel/iwlegacy/common.h
@@ -2822,10 +2822,6 @@ struct il_lq_sta {
struct il_traffic_load load[TID_MAX_LOAD_COUNT];
u8 tx_agg_tid_en;
#ifdef CONFIG_MAC80211_DEBUGFS
- struct dentry *rs_sta_dbgfs_scale_table_file;
- struct dentry *rs_sta_dbgfs_stats_table_file;
- struct dentry *rs_sta_dbgfs_rate_scale_data_file;
- struct dentry *rs_sta_dbgfs_tx_agg_tid_en_file;
u32 dbg_fixed_rate;
#endif
struct il_priv *drv;
--
2.22.0
^ permalink raw reply related
* [PATCH 1/5] iwlegacy: 3945: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-06-12 14:26 UTC (permalink / raw)
To: Johannes Berg
Cc: Greg Kroah-Hartman, Stanislaw Gruszka, Kalle Valo,
David S. Miller, linux-wireless, netdev
When calling debugfs functions, there is no need to ever check the
return value. This driver was saving the debugfs file away to be
removed at a later time. However, the 80211 core would delete the whole
directory that the debugfs files are created in, after it asks the
driver to do the deletion, so just rely on the 80211 core to do all of
the cleanup for us, making us not need to keep a pointer to the dentries
around at all.
This cleans up the structure of the driver data a bit and makes the code
a tiny bit smaller.
Cc: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/wireless/intel/iwlegacy/3945-rs.c | 14 ++------------
drivers/net/wireless/intel/iwlegacy/3945.h | 3 ---
2 files changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlegacy/3945-rs.c b/drivers/net/wireless/intel/iwlegacy/3945-rs.c
index a697edd46e7f..1d810cae6091 100644
--- a/drivers/net/wireless/intel/iwlegacy/3945-rs.c
+++ b/drivers/net/wireless/intel/iwlegacy/3945-rs.c
@@ -861,17 +861,8 @@ il3945_add_debugfs(void *il, void *il_sta, struct dentry *dir)
{
struct il3945_rs_sta *lq_sta = il_sta;
- lq_sta->rs_sta_dbgfs_stats_table_file =
- debugfs_create_file("rate_stats_table", 0600, dir, lq_sta,
- &rs_sta_dbgfs_stats_table_ops);
-
-}
-
-static void
-il3945_remove_debugfs(void *il, void *il_sta)
-{
- struct il3945_rs_sta *lq_sta = il_sta;
- debugfs_remove(lq_sta->rs_sta_dbgfs_stats_table_file);
+ debugfs_create_file("rate_stats_table", 0600, dir, lq_sta,
+ &rs_sta_dbgfs_stats_table_ops);
}
#endif
@@ -898,7 +889,6 @@ static const struct rate_control_ops rs_ops = {
.free_sta = il3945_rs_free_sta,
#ifdef CONFIG_MAC80211_DEBUGFS
.add_sta_debugfs = il3945_add_debugfs,
- .remove_sta_debugfs = il3945_remove_debugfs,
#endif
};
diff --git a/drivers/net/wireless/intel/iwlegacy/3945.h b/drivers/net/wireless/intel/iwlegacy/3945.h
index 00030d43a194..1aeb4b238fcf 100644
--- a/drivers/net/wireless/intel/iwlegacy/3945.h
+++ b/drivers/net/wireless/intel/iwlegacy/3945.h
@@ -87,9 +87,6 @@ struct il3945_rs_sta {
u8 start_rate;
struct timer_list rate_scale_flush;
struct il3945_rate_scale_data win[RATE_COUNT_3945];
-#ifdef CONFIG_MAC80211_DEBUGFS
- struct dentry *rs_sta_dbgfs_stats_table_file;
-#endif
/* used to be in sta_info */
int last_txrate_idx;
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] mt76: usb: do not always copy the first part of received frames
From: Stanislaw Gruszka @ 2019-06-12 14:21 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, nbd, kvalo, linux-wireless
In-Reply-To: <20190612125905.GB2600@redhat.com>
On Wed, Jun 12, 2019 at 02:59:05PM +0200, Stanislaw Gruszka wrote:
> > > If max RX AMSDU size is 3839B I do not see reason why we allocate
> > > MT_SG_MAX_SIZE=8 of MT_RX_BUF_SIZE=2k buffers for sg_en case.
> > > I thought the reason is that max AMSDU size is 16kB so it fit into
> > > 8 sg buffers of 2k.
> > >
> > > In other words, for me, looks like either
> > > - we can not handle AMSDU for non sg case because we do not
> > > allocate big enough buffer
> >
> > I think AMSDU is mandatory and we currently support it even for non-sg case
> > (since max rx AMSDU is 3839B)
> >
> > > or
> > > - we can just use one PAGE_SIZE buffer for rx and remove sg
> > > buffers for rx completely
> >
> > using sg buffers we can support bigger rx AMSDU size in the future without using
> > huge buffers (e.g. we can try to use IEEE80211_MAX_MPDU_LEN_HT_7935 with
> > mt76x2u)
>
> I think it would be simpler just to allocate 2 pages for 7935B .
And if we could determine that there is no true need to use sg for rx,
I think best approach here would be revert f8f527b16db5 in v5.2 to fix
regression and remove rx sg in -next. That would make code simpler,
allocate 4k instead 16k per packet, allow to use build_skb (4096 - 3839
give enough space for shared info) and not use usb hcd bounce buffer.
Stanislaw
^ permalink raw reply
* Re: [PATCH v3 3/5] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Arend Van Spriel @ 2019-06-12 13:58 UTC (permalink / raw)
To: Ulf Hansson
Cc: Doug Anderson, Hunter, Adrian, Kalle Valo, brcm80211-dev-list.pdl,
linux-rockchip, Double Lo, briannorris, linux-wireless,
Naveen Gupta, Madhan Mohan R, mka, Wright Feng, Chi-Hsien Lin,
netdev, brcm80211-dev-list, Franky Lin, linux-kernel,
Hante Meuleman, YueHaibing, David S. Miller
In-Reply-To: <CAPDyKFpM0+FfvoMo8Z_hxM9rzSjeQZHCsA2SPa8WP+SRDhhsPA@mail.gmail.com>
On 6/12/2019 1:48 PM, Ulf Hansson wrote:
> On Wed, 12 Jun 2019 at 13:11, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>>
>> On 6/12/2019 12:10 PM, Ulf Hansson wrote:
>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c:
>>>> mmc_set_data_timeout(md, func->card);
>>>> mmc_wait_for_req(func->card->host, mr);
>>> These are not okay, none of these things calls should really be done
>>> from an SDIO func driver.
>>>
>>> It tells me that the func driver is a doing workaround for something
>>> that should be managed in a common way.
>>
>> We are using some low-level functions passing chain of skbuff to the
>> device using CMD53 with scatterlist. If I recall correctly Marvell made
>> an attempt to have a similar function for it in the mmc stack. Not sure
>> if that ever made it in. If so I can rework our driver using that API.
>> If not, I can make a new attempt.
>
> I recall there were some patches, but not sure why we didn't merge them.
>
> Anyway, if you want to move this forward, that would be awesome!
Let's scope it before moving forward. Our use-case is to transfer a
chain of skbuff's. I am pretty sure that is not something we want to
deal with in mmc stack api. So I suppose passing a scatterlist is more
sensible, right? Maybe on sdio layer of the stack we could consider
dealing with skbuff's for network func drivers?
Let me see if I can find those Marvell patches. Might be a good start.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH v3 2/5] mmc: core: API for temporarily disabling auto-retuning due to errors
From: Ulf Hansson @ 2019-06-12 13:25 UTC (permalink / raw)
To: Douglas Anderson
Cc: Kalle Valo, Adrian Hunter, Arend van Spriel,
brcm80211-dev-list.pdl, open list:ARM/Rockchip SoC..., Double Lo,
Brian Norris, linux-wireless, Naveen Gupta, Madhan Mohan R,
Matthias Kaehlcke, Wright Feng, Chi-Hsien Lin, netdev,
brcm80211-dev-list, Jiong Wu, Ritesh Harjani,
linux-mmc@vger.kernel.org, Linux Kernel Mailing List, Shawn Lin,
Wolfram Sang, Avri Altman
In-Reply-To: <20190607223716.119277-3-dianders@chromium.org>
On Sat, 8 Jun 2019 at 00:37, Douglas Anderson <dianders@chromium.org> wrote:
>
> Normally when the MMC core sees an "-EILSEQ" error returned by a host
> controller then it will trigger a retuning of the card. This is
> generally a good idea.
>
> However, if a command is expected to sometimes cause transfer errors
> then these transfer errors shouldn't cause a re-tuning. This
> re-tuning will be a needless waste of time. One example case where a
> transfer is expected to cause errors is when transitioning between
> idle (sometimes referred to as "sleep" in Broadcom code) and active
> state on certain Broadcom WiFi cards. Specifically if the card was
> already transitioning between states when the command was sent it
> could cause an error on the SDIO bus.
>
> Let's add an API that the SDIO card drivers can call that will
> temporarily disable the auto-tuning functionality. Then we can add a
> call to this in the Broadcom WiFi driver and any other driver that
> might have similar needs.
>
> NOTE: this makes the assumption that the card is already tuned well
> enough that it's OK to disable the auto-retuning during one of these
> error-prone situations. Presumably the driver code performing the
> error-prone transfer knows how to recover / retry from errors. ...and
> after we can get back to a state where transfers are no longer
> error-prone then we can enable the auto-retuning again. If we truly
> find ourselves in a case where the card needs to be retuned sometimes
> to handle one of these error-prone transfers then we can always try a
> few transfers first without auto-retuning and then re-try with
> auto-retuning if the first few fail.
>
> Without this change on rk3288-veyron-minnie I periodically see this in
> the logs of a machine just sitting there idle:
> dwmmc_rockchip ff0d0000.dwmmc: Successfully tuned phase to XYZ
>
> Fixes: bd11e8bd03ca ("mmc: core: Flag re-tuning is needed on CRC errors")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Note that are are a whole boatload of different ways that we could
> provide an API for the Broadcom WiFi SDIO driver. This patch
> illustrates one way but if maintainers feel strongly that this is too
> ugly and have a better idea then I can give it a shot too. From a
> purist point of view I kinda felt that the "expect errors" really
> belonged as part of the mmc_request structure, but getting it into
> there meant changing a whole pile of core SD/MMC APIs. Simply adding
> it to the host seemed to match the current style better and was a less
> intrusive change.
>
> Changes in v3:
> - Took out the spinlock since I believe this is all in one context.
This needs to be clarified, preferable also in a function header.
If I understand correctly, the SDIO func driver needs the host to be
claimed when it calls mmc_expect_errors_begin(). More importantly, it
also needs to be keep it claimed until after it had called
mmc_expect_errors_end(). Correct?
>
> Changes in v2:
> - Updated commit message to clarify based on discussion of v1.
>
> drivers/mmc/core/core.c | 19 +++++++++++++++++--
> include/linux/mmc/core.h | 2 ++
> include/linux/mmc/host.h | 1 +
> 3 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 6db36dc870b5..bc109ec49406 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -144,8 +144,9 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq)
> int err = cmd->error;
>
> /* Flag re-tuning needed on CRC errors */
> - if ((cmd->opcode != MMC_SEND_TUNING_BLOCK &&
> - cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200) &&
> + if (cmd->opcode != MMC_SEND_TUNING_BLOCK &&
> + cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200 &&
> + !host->expect_errors &&
> (err == -EILSEQ || (mrq->sbc && mrq->sbc->error == -EILSEQ) ||
> (mrq->data && mrq->data->error == -EILSEQ) ||
> (mrq->stop && mrq->stop->error == -EILSEQ)))
> @@ -2163,6 +2164,20 @@ int mmc_sw_reset(struct mmc_host *host)
> }
> EXPORT_SYMBOL(mmc_sw_reset);
>
> +void mmc_expect_errors_begin(struct mmc_host *host)
> +{
> + WARN_ON(host->expect_errors);
Please remove the WARN_ON. If you believe there is a need for
reference counting, then please add that instead (but likely not in
the phase?).
> + host->expect_errors = true;
> +}
> +EXPORT_SYMBOL_GPL(mmc_expect_errors_begin);
> +
> +void mmc_expect_errors_end(struct mmc_host *host)
> +{
> + WARN_ON(!host->expect_errors);
Ditto.
> + host->expect_errors = false;
> +}
> +EXPORT_SYMBOL_GPL(mmc_expect_errors_end);
These new APIs seems to be useful solely for SDIO. Even if it turns
out later that they can be made generic, I suggest to start with a
SDIO func API instead.
However, using a new host variable (->expect_errors) is fine by me.
> +
> static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
> {
> host->f_init = freq;
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index 134a6483347a..02a13abf0cda 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -178,6 +178,8 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd,
>
> int mmc_hw_reset(struct mmc_host *host);
> int mmc_sw_reset(struct mmc_host *host);
> +void mmc_expect_errors_begin(struct mmc_host *host);
> +void mmc_expect_errors_end(struct mmc_host *host);
The API prevents a new re-tune to be "scheduled" in case requests are
failing with -EILSEQ.
To better reflect that, may I suggest to rename this to
sdio_retune_crc_disable() and sdio_retune_crc_enable(). Or something
along those lines.
> void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card);
>
> #endif /* LINUX_MMC_CORE_H */
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 43d0f0c496f6..8d553fb8c834 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -398,6 +398,7 @@ struct mmc_host {
> unsigned int retune_now:1; /* do re-tuning at next req */
> unsigned int retune_paused:1; /* re-tuning is temporarily disabled */
> unsigned int use_blk_mq:1; /* use blk-mq */
> + unsigned int expect_errors:1; /* don't trigger retune upon errors */
>
> int rescan_disable; /* disable card detection */
> int rescan_entered; /* used with nonremovable devices */
> --
> 2.22.0.rc2.383.gf4fbbf30c2-goog
>
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v2 2/2] mt76: usb: do not always copy the first part of received frames
From: Stanislaw Gruszka @ 2019-06-12 12:59 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, nbd, kvalo, linux-wireless
In-Reply-To: <20190612122845.GH8107@localhost.localdomain>
On Wed, Jun 12, 2019 at 02:28:48PM +0200, Lorenzo Bianconi wrote:
> [...]
> >
> > My sense of humor declined quite drastically lastly ;-/
> >
> > > > > but we can be more cautious since probably copying
> > > > > the first 128B will not make any difference
> > > >
> > > > Not sure if I understand what you mean.
> > >
> > > Please correct me if I am wrong but I think max amsdu rx size is 3839B for
> > > mt76. For the sg_en case this frame will span over multiple sg buffers since
> > > sg buffer size is 2048B (2 sg buffers). Moreover if we do not take into account
> > > skb_shared_info when configuring the sg buffer size we will need to always copy
> > > the first 128B of the first buffer since received len will be set to 2048 and
> > > the following if condition will always fail:
> > >
> > > if (SKB_WITH_OVERHEAD(buf_size) >= MT_DMA_HDR_LEN + len) {
> > > }
> >
> > Ok, that I understand.
> >
> > > > > > And skb_shered_info is needed only in first buffer IIUC.
> > > > > >
> > > > > > Also this patch seems to make first patch unnecessary except for
> > > > > > non sg_en case (in which I think rx AMSDU is broken anyway),
> > > > > > so I would prefer just to apply first patch.
> > > > >
> > > > > I do not think rx AMSDU is broken for non sg_en case since the max rx value
> > > > > allowed should be 3839 IIRC and we alloc one page in this case
> > > >
> > > > If that's the case we should be fine, but then I do not understand
> > > > why we allocate 8*2k buffers for sg_en case, isn't that AP can
> > > > sent AMSDU frame 16k big?
> > >
> > > Sorry I did not get what you mean here, could you please explain?
> >
> > If max RX AMSDU size is 3839B I do not see reason why we allocate
> > MT_SG_MAX_SIZE=8 of MT_RX_BUF_SIZE=2k buffers for sg_en case.
> > I thought the reason is that max AMSDU size is 16kB so it fit into
> > 8 sg buffers of 2k.
> >
> > In other words, for me, looks like either
> > - we can not handle AMSDU for non sg case because we do not
> > allocate big enough buffer
>
> I think AMSDU is mandatory and we currently support it even for non-sg case
> (since max rx AMSDU is 3839B)
>
> > or
> > - we can just use one PAGE_SIZE buffer for rx and remove sg
> > buffers for rx completely
>
> using sg buffers we can support bigger rx AMSDU size in the future without using
> huge buffers (e.g. we can try to use IEEE80211_MAX_MPDU_LEN_HT_7935 with
> mt76x2u)
I think it would be simpler just to allocate 2 pages for 7935B .
Stanislaw
^ permalink raw reply
* Re: [PATCH v2 2/2] mt76: usb: do not always copy the first part of received frames
From: Lorenzo Bianconi @ 2019-06-12 12:28 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Lorenzo Bianconi, nbd, kvalo, linux-wireless
In-Reply-To: <20190612115120.GA3496@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2358 bytes --]
[...]
>
> My sense of humor declined quite drastically lastly ;-/
>
> > > > but we can be more cautious since probably copying
> > > > the first 128B will not make any difference
> > >
> > > Not sure if I understand what you mean.
> >
> > Please correct me if I am wrong but I think max amsdu rx size is 3839B for
> > mt76. For the sg_en case this frame will span over multiple sg buffers since
> > sg buffer size is 2048B (2 sg buffers). Moreover if we do not take into account
> > skb_shared_info when configuring the sg buffer size we will need to always copy
> > the first 128B of the first buffer since received len will be set to 2048 and
> > the following if condition will always fail:
> >
> > if (SKB_WITH_OVERHEAD(buf_size) >= MT_DMA_HDR_LEN + len) {
> > }
>
> Ok, that I understand.
>
> > > > > And skb_shered_info is needed only in first buffer IIUC.
> > > > >
> > > > > Also this patch seems to make first patch unnecessary except for
> > > > > non sg_en case (in which I think rx AMSDU is broken anyway),
> > > > > so I would prefer just to apply first patch.
> > > >
> > > > I do not think rx AMSDU is broken for non sg_en case since the max rx value
> > > > allowed should be 3839 IIRC and we alloc one page in this case
> > >
> > > If that's the case we should be fine, but then I do not understand
> > > why we allocate 8*2k buffers for sg_en case, isn't that AP can
> > > sent AMSDU frame 16k big?
> >
> > Sorry I did not get what you mean here, could you please explain?
>
> If max RX AMSDU size is 3839B I do not see reason why we allocate
> MT_SG_MAX_SIZE=8 of MT_RX_BUF_SIZE=2k buffers for sg_en case.
> I thought the reason is that max AMSDU size is 16kB so it fit into
> 8 sg buffers of 2k.
>
> In other words, for me, looks like either
> - we can not handle AMSDU for non sg case because we do not
> allocate big enough buffer
I think AMSDU is mandatory and we currently support it even for non-sg case
(since max rx AMSDU is 3839B)
> or
> - we can just use one PAGE_SIZE buffer for rx and remove sg
> buffers for rx completely
using sg buffers we can support bigger rx AMSDU size in the future without using
huge buffers (e.g. we can try to use IEEE80211_MAX_MPDU_LEN_HT_7935 with
mt76x2u)
Regards,
Lorenzo
>
> Do I miss something ?
>
> Stanislaw
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] mt76: usb: do not always copy the first part of received frames
From: Stanislaw Gruszka @ 2019-06-12 11:51 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, nbd, kvalo, linux-wireless
In-Reply-To: <20190612104921.GF8107@localhost.localdomain>
On Wed, Jun 12, 2019 at 12:49:22PM +0200, Lorenzo Bianconi wrote:
> > On Wed, Jun 12, 2019 at 11:53:03AM +0200, Lorenzo Bianconi wrote:
> > > > On Fri, May 31, 2019 at 11:38:23AM +0200, Lorenzo Bianconi wrote:
> > >
> > > [...]
> > >
> > > > > }
> > > > >
> > > > > urb->num_sgs = max_t(int, i, urb->num_sgs);
> > > > > - urb->transfer_buffer_length = urb->num_sgs * q->buf_size,
> > > > > + urb->transfer_buffer_length = urb->num_sgs * data_size;
> > > > > sg_init_marker(urb->sg, urb->num_sgs);
> > > > >
> > > > > return i ? : -ENOMEM;
> > > > > @@ -611,8 +611,12 @@ static int mt76u_alloc_rx(struct mt76_dev *dev)
> > > > > if (!q->entry)
> > > > > return -ENOMEM;
> > > > >
> > > > > - q->buf_size = dev->usb.sg_en ? MT_RX_BUF_SIZE : PAGE_SIZE;
> > > > > + if (dev->usb.sg_en)
> > > > > + q->buf_size = MT_BUF_WITH_OVERHEAD(MT_RX_BUF_SIZE);
> > > >
> > > > I strongly recommend to not doing this. While this should work
> > > > in theory creating buffer with size of 2k + some bytes might
> > > > trigger various bugs in dma mapping or other low level code.
> > >
> > > even in practice actually :)
> >
> > I wouldn't be sure about this. It's not common to have buffers of
> > such size and crossing pages boundaries. It really can trigger
> > nasty bugs on various IOMMU drivers.
>
> I was just joking, I mean that it worked in the tests I carried out, but I
> agree it can trigger some issues in buggy IOMMU drivers
My sense of humor declined quite drastically lastly ;-/
> > > but we can be more cautious since probably copying
> > > the first 128B will not make any difference
> >
> > Not sure if I understand what you mean.
>
> Please correct me if I am wrong but I think max amsdu rx size is 3839B for
> mt76. For the sg_en case this frame will span over multiple sg buffers since
> sg buffer size is 2048B (2 sg buffers). Moreover if we do not take into account
> skb_shared_info when configuring the sg buffer size we will need to always copy
> the first 128B of the first buffer since received len will be set to 2048 and
> the following if condition will always fail:
>
> if (SKB_WITH_OVERHEAD(buf_size) >= MT_DMA_HDR_LEN + len) {
> }
Ok, that I understand.
> > > > And skb_shered_info is needed only in first buffer IIUC.
> > > >
> > > > Also this patch seems to make first patch unnecessary except for
> > > > non sg_en case (in which I think rx AMSDU is broken anyway),
> > > > so I would prefer just to apply first patch.
> > >
> > > I do not think rx AMSDU is broken for non sg_en case since the max rx value
> > > allowed should be 3839 IIRC and we alloc one page in this case
> >
> > If that's the case we should be fine, but then I do not understand
> > why we allocate 8*2k buffers for sg_en case, isn't that AP can
> > sent AMSDU frame 16k big?
>
> Sorry I did not get what you mean here, could you please explain?
If max RX AMSDU size is 3839B I do not see reason why we allocate
MT_SG_MAX_SIZE=8 of MT_RX_BUF_SIZE=2k buffers for sg_en case.
I thought the reason is that max AMSDU size is 16kB so it fit into
8 sg buffers of 2k.
In other words, for me, looks like either
- we can not handle AMSDU for non sg case because we do not
allocate big enough buffer
or
- we can just use one PAGE_SIZE buffer for rx and remove sg
buffers for rx completely
Do I miss something ?
Stanislaw
^ permalink raw reply
* Re: [PATCH v3 3/5] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Ulf Hansson @ 2019-06-12 11:48 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Doug Anderson, Hunter, Adrian, Kalle Valo,
brcm80211-dev-list.pdl@broadcom.com,
linux-rockchip@lists.infradead.org, Double Lo,
briannorris@chromium.org, linux-wireless@vger.kernel.org,
Naveen Gupta, Madhan Mohan R, mka@chromium.org, Wright Feng,
Chi-Hsien Lin, netdev@vger.kernel.org,
brcm80211-dev-list@cypress.com, Franky Lin,
linux-kernel@vger.kernel.org, Hante Meuleman, YueHaibing,
David S. Miller
In-Reply-To: <c7c6d3f4-ebb1-8964-0616-973fae1ab47d@broadcom.com>
On Wed, 12 Jun 2019 at 13:11, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
>
> On 6/12/2019 12:10 PM, Ulf Hansson wrote:
> >> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c:
> >> mmc_set_data_timeout(md, func->card);
> >> mmc_wait_for_req(func->card->host, mr);
> > These are not okay, none of these things calls should really be done
> > from an SDIO func driver.
> >
> > It tells me that the func driver is a doing workaround for something
> > that should be managed in a common way.
>
> We are using some low-level functions passing chain of skbuff to the
> device using CMD53 with scatterlist. If I recall correctly Marvell made
> an attempt to have a similar function for it in the mmc stack. Not sure
> if that ever made it in. If so I can rework our driver using that API.
> If not, I can make a new attempt.
I recall there were some patches, but not sure why we didn't merge them.
Anyway, if you want to move this forward, that would be awesome!
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v2 1/2] mt76: usb: fix rx A-MSDU support
From: Lorenzo Bianconi @ 2019-06-12 11:17 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Lorenzo Bianconi, nbd, kvalo, linux-wireless
In-Reply-To: <20190612105801.GA2600@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]
> On Wed, Jun 12, 2019 at 12:21:34PM +0200, Lorenzo Bianconi wrote:
> > On Jun 12, Stanislaw Gruszka wrote:
> > > On Wed, Jun 12, 2019 at 11:45:21AM +0200, Lorenzo Bianconi wrote:
> > > > > > +mt76u_build_rx_skb(u8 *data, int len, int buf_size,
> > > > > > + int *nsgs)
> > > > > > +{
> > > > > > + int data_len = min(len, MT_SKB_HEAD_LEN);
> > >
> > > Oh, and this looks unneeded as well as for len < MT_SKB_HEAD_LEN=128
> > > we will go through fast path.
> >
> > I guess if we remove data_len = min(len, MT_SKB_HEAD_LEN) and even *nsgs = 0 at
> > the end we are making some assumptions on the value of MT_SKB_HEAD_LEN and
> > buf_size. In the patch I just avoided them but maybe we can just assume that
> > MT_SKB_HEAD_LEN and buf_size will not changed in the future. What do you
> > think?
>
> Yes, sure. Other drivers just use 128 value directly and don't even
> create a macro for that. And if somebody will decide to change
> buf_size it will not be small value.
Ok, I will simplify it in v2
>
> > > > > mt7601u and iwlmvm just copy hdrlen + 8 and put the rest
> > > > > of the buffer in fragment, which supose to be more efficient,
> > > > > see comment in iwl_mvm_pass_packet_to_mac80211().
> > > >
> > > > Right here we copy 128B instead of 32 but I think it is good to have L3 and L4
> > > > header in the linear area of the skb since otherwise the stack will need to
> > > > align them
> > >
> > > Not sure if understand, I think aliment of L3 & L4 headers will be
> > > the same, assuming ieee80211 header is aligned the same in fragment
> > > buffer and in linear area. But if you think this is better to copy those
> > > to linear area I'm ok with that.
> >
> > Sorry I have not been so clear. I mean in the stack before accessing a given
> > header we will run pskb_may_pull() that can end up copying the skb if there is
> > not enough space in the skb->head
>
> Ok, so L3 and L4 headers should be in linear area of skb and if not
> network stack will copy them from fragment. But I wonder why other
> drivers just copy ieee80211_hdr and SNAP ? Isn't that if we copy
> 128B then is possible that part of the payload will be in linear
> area and part in fragment, whereas is expected that payload
> will not be broken into two parts?
I think the payload can be split in multiple skb fragments, the constraint is
just related to header parsing
Regards,
Lorenzo
>
> Stanislaw
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ 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