* Re: [PATCH] ath10k: reset chip after supported check
From: Ben Greear @ 2019-03-25 20:22 UTC (permalink / raw)
To: Michał Kazior
Cc: Arend Van Spriel, Christian Lamparter, Tomislav Požega,
linux-wireless, openwrt-devel, Kalle Valo
In-Reply-To: <CABvG-CWP4y4MU1pJB8ZSpLPPYMkQ+=kQt1zRoUJW=0dMLbD5WQ@mail.gmail.com>
On 3/25/19 1:08 PM, Michał Kazior wrote:
> On Mon, 25 Mar 2019 at 16:55, Ben Greear <greearb@candelatech.com> wrote:
>> On 3/25/19 5:14 AM, Michał Kazior wrote:
>>> On Sat, 23 Mar 2019 at 08:20, Arend Van Spriel
>>> <arend.vanspriel@broadcom.com> wrote:
>>>>
>>>> * resending with corrected email address from Kalle
>>>> --------------------------------------------------------------------
>>>> + Michał
>>>
>>> Thanks!
>>>
>>>
>>>> On 3/22/2019 8:25 PM, Christian Lamparter wrote:
>>>> > On Friday, March 22, 2019 7:58:40 PM CET Tomislav Požega wrote:
>>>> >> When chip reset is done before the chip is checked if supported
>>>> >> there will be crash. Previous behaviour caused bootloops on
>>>> >> Archer C7 v1 units, this patch allows clean device boot without
>>>> >> excluding ath10k driver.
>>>
>>> Can you elaborate more a bit? What kind of crashes are you seeing?
>>> What does the bootloop look like? Do you have uart connected to
>>> diagnose?
>>>
>>> Didn't C7 v1 have the old QCA9880 hw v1 which isn't really supported
>>> by ath10k? I recall the v1 chip was really buggy and required
>>> hammering registers sometimes to get things working.
>>
>> The crash is related to the v1 chip. Is there a good way to detect
>> that this is the chip in question and only apply this work-around
>> for the problem chip?
>
> I don't know of any good way to do that.
>
> You could consider device-tree but that would be no different from
> having a module blacklist in the C7v1 build recipe, or to not build
> the module at all. That is unless you actually want to make v1 chip
> work with ath10k at which point there's more fun to be had before it
> can actually work.
I remember v1, and I have no interest in trying to make it work :)
If we could blacklist certain pci slots in the ath10k driver, I guess
that would work?
I think the goal is to not use the v1 chip, but allow users to add a
v2 NIC to the platform, so driver still needs to load.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] ath10k: reset chip after supported check
From: Michał Kazior @ 2019-03-25 20:08 UTC (permalink / raw)
To: Ben Greear
Cc: Arend Van Spriel, Christian Lamparter, Tomislav Požega,
linux-wireless, openwrt-devel, Kalle Valo
In-Reply-To: <c13da25c-9116-9388-8e40-96bac8dca087@candelatech.com>
On Mon, 25 Mar 2019 at 16:55, Ben Greear <greearb@candelatech.com> wrote:
> On 3/25/19 5:14 AM, Michał Kazior wrote:
> > On Sat, 23 Mar 2019 at 08:20, Arend Van Spriel
> > <arend.vanspriel@broadcom.com> wrote:
> >>
> >> * resending with corrected email address from Kalle
> >> --------------------------------------------------------------------
> >> + Michał
> >
> > Thanks!
> >
> >
> >> On 3/22/2019 8:25 PM, Christian Lamparter wrote:
> >> > On Friday, March 22, 2019 7:58:40 PM CET Tomislav Požega wrote:
> >> >> When chip reset is done before the chip is checked if supported
> >> >> there will be crash. Previous behaviour caused bootloops on
> >> >> Archer C7 v1 units, this patch allows clean device boot without
> >> >> excluding ath10k driver.
> >
> > Can you elaborate more a bit? What kind of crashes are you seeing?
> > What does the bootloop look like? Do you have uart connected to
> > diagnose?
> >
> > Didn't C7 v1 have the old QCA9880 hw v1 which isn't really supported
> > by ath10k? I recall the v1 chip was really buggy and required
> > hammering registers sometimes to get things working.
>
> The crash is related to the v1 chip. Is there a good way to detect
> that this is the chip in question and only apply this work-around
> for the problem chip?
I don't know of any good way to do that.
You could consider device-tree but that would be no different from
having a module blacklist in the C7v1 build recipe, or to not build
the module at all. That is unless you actually want to make v1 chip
work with ath10k at which point there's more fun to be had before it
can actually work.
Michał
^ permalink raw reply
* [PATCH v2] mt76: usb: reduce locking in mt76u_tx_tasklet
From: Lorenzo Bianconi @ 2019-03-25 19:18 UTC (permalink / raw)
To: nbd; +Cc: linux-wireless, lorenzo.bianconi, sgruszka
In-Reply-To: <cover.1553541103.git.lorenzo@kernel.org>
Similar to pci counterpart, reduce locking in mt76u_tx_tasklet since
q->head is managed just in mt76u_tx_tasklet and q->queued is updated
holding q->lock
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
Changes since v1:
- drop idx variable
Changes since RFC:
- reset done to false in mt76u_tx_tasklet instead of in mt76u_tx_queue_skb
This patch is based on https://patchwork.kernel.org/patch/10856027/
---
drivers/net/wireless/mediatek/mt76/usb.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 15aeda0582e7..fb8b22c1655a 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -624,28 +624,33 @@ static void mt76u_tx_tasklet(unsigned long data)
int i;
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+ u32 n_dequeued = 0, n_sw_dequeued = 0;
+
sq = &dev->q_tx[i];
q = sq->q;
- spin_lock_bh(&q->lock);
- while (true) {
- if (!q->entry[q->head].done || !q->queued)
+ while (q->queued > n_dequeued) {
+ if (!q->entry[q->head].done)
break;
if (q->entry[q->head].schedule) {
q->entry[q->head].schedule = false;
- sq->swq_queued--;
+ n_sw_dequeued++;
}
entry = q->entry[q->head];
+ q->entry[q->head].done = false;
q->head = (q->head + 1) % q->ndesc;
- q->queued--;
+ n_dequeued++;
- spin_unlock_bh(&q->lock);
dev->drv->tx_complete_skb(dev, i, &entry);
- spin_lock_bh(&q->lock);
}
+ spin_lock_bh(&q->lock);
+
+ sq->swq_queued -= n_sw_dequeued;
+ q->queued -= n_dequeued;
+
wake = q->stopped && q->queued < q->ndesc - 8;
if (wake)
q->stopped = false;
@@ -741,7 +746,6 @@ mt76u_tx_queue_skb(struct mt76_dev *dev, enum mt76_txq_id qid,
if (err < 0)
return err;
- q->entry[idx].done = false;
urb = q->entry[idx].urb;
err = mt76u_tx_setup_buffers(dev, skb, urb);
if (err < 0)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] net: mwifiex: fix a missing check of ssid
From: Brian Norris @ 2019-03-25 18:30 UTC (permalink / raw)
To: Kangjie Lu
Cc: pakki001, Amitkumar Karwar, Nishant Sarmukadam, Ganapathi Bhat,
Xinming Hu, Kalle Valo, David S. Miller, linux-wireless,
<netdev@vger.kernel.org>, Linux Kernel
In-Reply-To: <20190323223339.13703-1-kjlu@umn.edu>
On Sat, Mar 23, 2019 at 3:34 PM Kangjie Lu <kjlu@umn.edu> wrote:
> sme->ssid may be invalid and thus should be checked.
Under what conditions are you seeing this? I see that brcmfmac, for
one, also has a similar check, but IIUC, the connect API always
pre-checks that there's an SSID provided:
static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
{
struct cfg80211_registered_device *rdev = info->user_ptr[0];
struct net_device *dev = info->user_ptr[1];
struct cfg80211_connect_params connect;
struct wiphy *wiphy;
struct cfg80211_cached_keys *connkeys = NULL;
int err;
memset(&connect, 0, sizeof(connect));
if (!info->attrs[NL80211_ATTR_SSID] ||
!nla_len(info->attrs[NL80211_ATTR_SSID]))
return -EINVAL;
...
connect.ssid = nla_data(info->attrs[NL80211_ATTR_SSID]);
So this patch shouldn't be necessary.
Perhaps I'm missing something though.
Regards,
Brian
^ permalink raw reply
* Re: [PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support
From: Brian Norris @ 2019-03-25 17:16 UTC (permalink / raw)
To: Govind Singh; +Cc: ath10k, linux-wireless
In-Reply-To: <20190320044511.12172-2-govinds@codeaurora.org>
On Tue, Mar 19, 2019 at 9:48 PM Govind Singh <govinds@codeaurora.org> wrote:
> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
> @@ -81,6 +81,7 @@ Optional properties:
> Definition: Name of external front end module used. Some valid FEM names
> for example: "microsemi-lx5586", "sky85703-11"
> and "sky85803" etc.
> +- xo-cal-data: xo cal offset to be configured in xo trim register.
https://media.readthedocs.org/pdf/devicetree-specification/stable/devicetree-specification.pdf
"2.2.4 Properties
...
Nonstandard property names should specify a unique string prefix, such
as a stock ticker symbol, identifying the name of the company or
organization that defined the property. Examples:
fsl,channel-fifo-len ibm,ppc-interrupt-server#s
linux,network-index"
In other words, like most other custom properties in this file, you
should probably have a "qcom," prefix.
Brian
> Example (to supply PCI based wifi block details):
^ permalink raw reply
* Re: [RFC PATCH] mac80211/cfg80211: update bss channel on channel switch
From: Johannes Berg @ 2019-03-25 16:18 UTC (permalink / raw)
To: Sergey Matyukevich
Cc: linux-wireless@vger.kernel.org, Igor Mitsyanko, Arend van Spriel
In-Reply-To: <20190325154443.elzp5n6nfngtg5vk@bars>
On Mon, 2019-03-25 at 15:44 +0000, Sergey Matyukevich wrote:
> > >
> > I worry slightly about locking too, since there's no protection on the
> > wdev->current_bss pointer here.
>
> Ok, I will add WARN_ON. As for locking, this function is supposed to be
> called with wdev->mtx held, see ASSERT_WDEV_LOCK. Do you think this may
> not be enough ?
Oops, no, that's definitely sufficient. I missed it.
> I need to check mwifiex more carefully, but other cfg80211_ch_switch_notify
> users (mac80211, ath6kl, qtnfmac) seem to do that locking properly.
Well I guess they'd assert and then do something unsafe, so wouldn't
worry about it.
johannes
^ permalink raw reply
* Re: [PATCH] ath10k: reset chip after supported check
From: Ben Greear @ 2019-03-25 15:55 UTC (permalink / raw)
To: Michał Kazior, Arend Van Spriel
Cc: Christian Lamparter, Tomislav Požega, linux-wireless,
openwrt-devel, Kalle Valo
In-Reply-To: <CABvG-CUXHgJ1ZUfsZw6ajzR=4DjnRRNrPmF147mZsoWJBjp-jQ@mail.gmail.com>
On 3/25/19 5:14 AM, Michał Kazior wrote:
> On Sat, 23 Mar 2019 at 08:20, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>>
>> * resending with corrected email address from Kalle
>> --------------------------------------------------------------------
>> + Michał
>
> Thanks!
>
>
>> On 3/22/2019 8:25 PM, Christian Lamparter wrote:
>> > On Friday, March 22, 2019 7:58:40 PM CET Tomislav Požega wrote:
>> >> When chip reset is done before the chip is checked if supported
>> >> there will be crash. Previous behaviour caused bootloops on
>> >> Archer C7 v1 units, this patch allows clean device boot without
>> >> excluding ath10k driver.
>
> Can you elaborate more a bit? What kind of crashes are you seeing?
> What does the bootloop look like? Do you have uart connected to
> diagnose?
>
> Didn't C7 v1 have the old QCA9880 hw v1 which isn't really supported
> by ath10k? I recall the v1 chip was really buggy and required
> hammering registers sometimes to get things working.
The crash is related to the v1 chip. Is there a good way to detect
that this is the chip in question and only apply this work-around
for the problem chip?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [RFC PATCH] mac80211/cfg80211: update bss channel on channel switch
From: Sergey Matyukevich @ 2019-03-25 15:44 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless@vger.kernel.org, Igor Mitsyanko, Arend van Spriel
In-Reply-To: <3d76d0d033609fff4e72448a9fd7d63ad1365cb2.camel@sipsolutions.net>
> > FullMAC STAs have no way to update bss channel after CSA channel switch
> > completion. As a result, user-space tools may provide inconsistent
> > channel info. For instance, consider the following two commands:
> > $ sudo iw dev wlan0 link
> > $ sudo iw dev wlan0 info
> > The latter command gets channel info from the hardware, so most probably
> > its output will be correct. However the former command gets channel info
> > from scan cache, so its output will contain outdated channel info.
> > In fact, current bss channel info will not be updated until the
> > next [re-]connect.
> >
> > Note that mac80211 STAs have a workaround for this, but it requires
> > access to internal cfg80211 data, see ieee80211_chswitch_work:
> >
> > /* XXX: shouldn't really modify cfg80211-owned data! */
> > ifmgd->associated->channel = sdata->csa_chandef.chan;
> >
> > This patch suggests to convert mac80211 workaround to cfg80211 behavior
> > and to update current bss channel in cfg80211_ch_switch_notify.
> >
> > Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
> >
> > ---
> >
> > This issue has been observed for both qtnfmac and brcmfmac. Fix tested
> > for qtnfmac and iwlwifi, to make sure there is no regression for mac80211.
> > However this is not going to be enough to fix brcmfmac behavior as it
> > does not use cfg80211_ch_switch_notify. This issue may also affect
> > ath6kl and mwifiex, but I have no hardware to check.
>
> Yeah, I think this makes sense.
>
> > + if (wdev->iftype == NL80211_IFTYPE_STATION) {
> > + if (wdev->current_bss)
> > + wdev->current_bss->pub.channel = chandef->chan;
> > + }
>
> Maybe do
>
> if (wdev->iftype == ... &&
> !WARN_ON(!wdev->current_bss))
> wdev->current_bss->... = ...
>
> I worry slightly about locking too, since there's no protection on the
> wdev->current_bss pointer here.
Ok, I will add WARN_ON. As for locking, this function is supposed to be
called with wdev->mtx held, see ASSERT_WDEV_LOCK. Do you think this may
not be enough ?
I need to check mwifiex more carefully, but other cfg80211_ch_switch_notify
users (mac80211, ath6kl, qtnfmac) seem to do that locking properly.
Regards,
Sergey
^ permalink raw reply
* [mac80211-next:netlink-validate-build-test-2 2/6] net/sched/act_tunnel_key.c:128:8: error: implicit declaration of function 'nla_validate_deprecated'
From: kbuild test robot @ 2019-03-25 15:01 UTC (permalink / raw)
To: Johannes Berg; +Cc: kbuild-all, linux-wireless
[-- Attachment #1: Type: text/plain, Size: 2379 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git netlink-validate-build-test-2
head: 963dafe164a051f6763a6fddf7b6d7933076985c
commit: e2e82c1203c1ab2423a79f486567a49228f546ab [2/6] netlink: make validation more configurable for future strictness
config: x86_64-randconfig-l3-03250457 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
git checkout e2e82c1203c1ab2423a79f486567a49228f546ab
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
net/sched/act_tunnel_key.c: In function 'tunnel_key_copy_opts':
>> net/sched/act_tunnel_key.c:128:8: error: implicit declaration of function 'nla_validate_deprecated' [-Werror=implicit-function-declaration]
err = nla_validate_deprecated(head, len, TCA_TUNNEL_KEY_ENC_OPTS_MAX,
^
cc1: some warnings being treated as errors
vim +/nla_validate_deprecated +128 net/sched/act_tunnel_key.c
121
122 static int tunnel_key_copy_opts(const struct nlattr *nla, u8 *dst,
123 int dst_len, struct netlink_ext_ack *extack)
124 {
125 int err, rem, opt_len, len = nla_len(nla), opts_len = 0;
126 const struct nlattr *attr, *head = nla_data(nla);
127
> 128 err = nla_validate_deprecated(head, len, TCA_TUNNEL_KEY_ENC_OPTS_MAX,
129 enc_opts_policy, extack);
130 if (err)
131 return err;
132
133 nla_for_each_attr(attr, head, len, rem) {
134 switch (nla_type(attr)) {
135 case TCA_TUNNEL_KEY_ENC_OPTS_GENEVE:
136 opt_len = tunnel_key_copy_geneve_opt(attr, dst,
137 dst_len, extack);
138 if (opt_len < 0)
139 return opt_len;
140 opts_len += opt_len;
141 if (dst) {
142 dst_len -= opt_len;
143 dst += opt_len;
144 }
145 break;
146 }
147 }
148
149 if (!opts_len) {
150 NL_SET_ERR_MSG(extack, "Empty list of tunnel options");
151 return -EINVAL;
152 }
153
154 if (rem > 0) {
155 NL_SET_ERR_MSG(extack, "Trailing data after parsing tunnel key options attributes");
156 return -EINVAL;
157 }
158
159 return opts_len;
160 }
161
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33596 bytes --]
^ permalink raw reply
* Re: [PATCH v8 00/14] rtw88: mac80211 driver for Realtek 802.11ac wireless network chips
From: Brian Norris @ 2019-03-25 14:36 UTC (permalink / raw)
To: yhchuang
Cc: kvalo, johannes, linux-wireless, gregkh, sgruszka, pkshih,
tehuang, Larry.Finger
In-Reply-To: <1552450443-351-1-git-send-email-yhchuang@realtek.com>
Hi,
On Wed, Mar 13, 2019 at 12:13:49PM +0800, yhchuang@realtek.com wrote:
> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>
> This is a new mac80211 driver for Realtek 802.11ac wireless network chips.
> rtw88 now supports RTL8822BE/RTL8822CE now, with basic station mode
> functionalities. The firmware for both can be found at linux-firmware.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
> For RTL8822BE: rtw88/rtw8822b_fw.bin
> For RTL8822CE: rtw88/rtw8822c_fw.bin
> Potential problems:
> * TRX statictics misleading, as we are not reporting status correctly,
> or say, not reporting for "every" packet.
By the way, I see earlier RFC versions had similar comments/questions
from Stanislaw and Kalle that I had too. So at least Kalle didn't have a
great answer either on what to do with hardware where it is inefficient
to get ACK reports for everything :D But least you did add a little bit
of comments this time, and fixed up some of the reporting.
...
> v5
...
> - use macro instead of ugly struct layout with #ifdef __LITTLE_ENDIAN
BTW, you technically could still use a struct there if you'd like, but
removing the bitfield / __LITTLE_ENDIAN handling was nice, thank you.
> - simplify efuse logical map parsing function
Much better, thank you!
...
> - enable DMA sync to avoid pci bus timeout
...
> v8
>
> - add prefix "rtw_" to *_rates and corresponding *_sizes, otherwise
> the extern will confuse the linker there are multiple definitions
> with brcmsmac driver
I can't say I've closely reviewed every bit of this driver, but I did
tackle significant portions of it. All my comments have been addressed,
apart from one relatively insignificant piece that I re-commented on
this version. So:
Reviewed-by: Brian Norris <briannorris@chromium.org>
I also gave 8822CE some moderate testing, and while I'm sure it's not
perfect, it's at least stable, and it's in much better than when I
tested approximately v2 (as Tony has rolled in a lot of fixes since
then):
Tested-by: Brian Norris <briannorris@chromium.org>
^ permalink raw reply
* Re: [PATCH v8 03/14] rtw88: hci files
From: Brian Norris @ 2019-03-25 14:34 UTC (permalink / raw)
To: yhchuang
Cc: kvalo, johannes, linux-wireless, gregkh, sgruszka, pkshih,
tehuang, Larry.Finger
In-Reply-To: <1552450443-351-4-git-send-email-yhchuang@realtek.com>
Hi,
On Wed, Mar 13, 2019 at 12:13:52PM +0800, yhchuang@realtek.com wrote:
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> new file mode 100644
> index 0000000..cf3bffb
> --- /dev/null
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -0,0 +1,1211 @@
...
> +static u8 ac_to_hwq[] = {
> + [0] = RTW_TX_QUEUE_VO,
> + [1] = RTW_TX_QUEUE_VI,
> + [2] = RTW_TX_QUEUE_BE,
> + [3] = RTW_TX_QUEUE_BK,
> +};
I don't want to dig up and repeat the details of my comments, but I
think you missed my earlier suggestion here: you should use the
ieee80211_ac_numbers enum instead of bare 0-3 indexing.
> +static u8 rtw_hw_queue_mapping(struct sk_buff *skb)
> +{
> + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> + __le16 fc = hdr->frame_control;
> + u8 q_mapping = skb_get_queue_mapping(skb);
> + u8 queue;
> +
> + if (unlikely(ieee80211_is_beacon(fc)))
> + queue = RTW_TX_QUEUE_BCN;
> + else if (unlikely(ieee80211_is_mgmt(fc) || ieee80211_is_ctl(fc)))
> + queue = RTW_TX_QUEUE_MGMT;
> + else
> + queue = ac_to_hwq[q_mapping];
It also seems wise to be defensive about the values of 'q_mapping', so
we don't accidentally overstep the array if for some reason the
implementation details of mac80211 change some day. e.g.:
else if (WARN_ON_ONCE(q_mapping >= ARRAY_SIZE(ac_to_hwq)))
// Pick a default.
Brian
> +
> + return queue;
> +}
> +
^ permalink raw reply
* Re: [RFC v2 03/12] rtw88: hci files
From: Brian Norris @ 2019-03-25 14:33 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Tony Chuang, kvalo@codeaurora.org, Larry.Finger@lwfinger.net,
Pkshih, Andy Huang, linux-wireless@vger.kernel.org,
Grant Grundler
In-Reply-To: <20181008093435.GE1961@redhat.com>
+ Grant
I'm sorry (a little) to drudge up old revisions. But I happened to be
looking back at prior reviews, since I'm reviewing the latest, and I
realized I had this same (then-unresolved) confusion on later versions.
Maybe I should have read more before reviewing... Oh well. Comments
below:
On Mon, Oct 08, 2018 at 11:34:36AM +0200, Stanislaw Gruszka wrote:
> On Fri, Oct 05, 2018 at 12:07:06PM +0000, Tony Chuang wrote:
> > > > +static void rtw_pci_dma_check(struct rtw_dev *rtwdev,
> > > > + struct rtw_pci_rx_ring *rx_ring,
> > > > + u32 idx)
> > > > +{
> > > > + struct rtw_chip_info *chip = rtwdev->chip;
> > > > + struct rtw_pci_rx_buffer_desc *buf_desc;
> > > > + u32 desc_sz = chip->rx_buf_desc_sz;
> > > > + u16 total_pkt_size;
> > > > + int i;
> > > > +
> > > > + buf_desc = (struct rtw_pci_rx_buffer_desc *)(rx_ring->r.head +
> > > > + idx * desc_sz);
> > > > + for (i = 0; i < 20; i++) {
> > > > + total_pkt_size = le16_to_cpu(buf_desc->total_pkt_size);
> > > > + if (total_pkt_size)
> > > > + return;
> > > > + }
> > > > +
> > > > + if (i >= 20)
> > > > + rtw_warn(rtwdev, "pci bus timeout, drop packet\n");
> > > This is not right, most likely you need to use
Indeed, this was not right. Tony ended up removing it entirely later on,
based on my independent (sorry, didn't read this!) suggestion.
> > > dma_sync_single_for_cpu() .
> >
> > Not really understand how dma_sync_single_for_cpu() works.
> > Can you show me if possible?
>
> dma_sync_single_for_cpu() and dma_sync_single_for_device()
> transfer dma buffer ownership to respectivly CPU and device.
> It is well documented in:
> Documentation/DMA-API-HOWTO.txt:
> Documentation/DMA-API.txt
Yes, if we expect to need a sort of polling loop like this here, then it
would be important to flush any caches (and, as Grant noted later, it
would probably help to ensure some kind of consistent time deadline --
udelay(), or maybe just check against ktime_get()).
I'll also note here as I did on the later series: these rings are
allocated with consistent memory, so as long as the device is only
signalling this interrupt after it has completed writing to memory, then
we should be reading the latest copy and there should be no need for a
polling loop.
And Tony apparently agreed with me; the loop is gone in the latest, and
instead, we simply read once before emitting a warning.
Regards,
Brian
^ permalink raw reply
* Re: [PATCH] mt76: usb: reduce locking in mt76u_tx_tasklet
From: Stanislaw Gruszka @ 2019-03-25 14:31 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, nbd, linux-wireless
In-Reply-To: <20190325134734.GB12885@localhost.localdomain>
On Mon, Mar 25, 2019 at 02:47:35PM +0100, Lorenzo Bianconi wrote:
> > On Mon, Mar 25, 2019 at 02:00:36PM +0100, Lorenzo Bianconi wrote:
> > > > On Sun, Mar 24, 2019 at 03:51:43PM +0100, Lorenzo Bianconi wrote:
> > > > > + int idx;
> > > > > +
> > > > > sq = &dev->q_tx[i];
> > > > > q = sq->q;
> > > > >
> > > > > - spin_lock_bh(&q->lock);
> > > > > - while (true) {
> > > > > - if (!q->entry[q->head].done || !q->queued)
> > > > > + while (q->queued > n_queued) {
> > > > > + if (!q->entry[q->head].done)
> > > > > break;
> > > > If you place done = false here you will not need additional idx
> > > > variable.
> > >
> > > As Felix suggested, I would set done to false at the end of the loop, after
> > > tx_complete_skb
> > Why this is needed?
>
> logically I think it should be the last thing to do on the current skb but
Why? This is only marker that urb complete was done.
> probably moving it before tx_complete_skb will not make any difference
It will not, since code is performed in the same tasklet.
Stanislaw
^ permalink raw reply
* Re: [PATCH 2/2] rt2x00: enable experimental MFP with HW crypt
From: Stanislaw Gruszka @ 2019-03-25 14:25 UTC (permalink / raw)
To: Tom Psyborg; +Cc: linux-wireless, kvalo, daniel
In-Reply-To: <CAKR_QV+=i_Sp26zCkTC9cAHrhVLq4w0FoxLVmD-T3wFyd-ruEQ@mail.gmail.com>
On Thu, Mar 14, 2019 at 03:02:14PM +0100, Tom Psyborg wrote:
> On 13/03/2019, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>
> >
> > There should be no difference between enabling MFP with nohwcrypt and
> > without nohwcrypt,
>
> well there is. do you still want me to resend this with MFP enabled
> unconditionally for all chips?
Please share your hostapd config file with MFP enabled.
Stanislaw
^ permalink raw reply
* [mac80211-next:netlink-validate-build-test-2 2/6] net//sched/act_tunnel_key.c:128:8: error: implicit declaration of function 'nla_validate_deprecated'; did you mean 'nlmsg_validate_deprecated'?
From: kbuild test robot @ 2019-03-25 14:24 UTC (permalink / raw)
To: Johannes Berg; +Cc: kbuild-all, linux-wireless
[-- Attachment #1: Type: text/plain, Size: 2435 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git netlink-validate-build-test-2
head: 963dafe164a051f6763a6fddf7b6d7933076985c
commit: e2e82c1203c1ab2423a79f486567a49228f546ab [2/6] netlink: make validation more configurable for future strictness
config: x86_64-rhel-7.6 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout e2e82c1203c1ab2423a79f486567a49228f546ab
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
net//sched/act_tunnel_key.c: In function 'tunnel_key_copy_opts':
>> net//sched/act_tunnel_key.c:128:8: error: implicit declaration of function 'nla_validate_deprecated'; did you mean 'nlmsg_validate_deprecated'? [-Werror=implicit-function-declaration]
err = nla_validate_deprecated(head, len, TCA_TUNNEL_KEY_ENC_OPTS_MAX,
^~~~~~~~~~~~~~~~~~~~~~~
nlmsg_validate_deprecated
cc1: some warnings being treated as errors
vim +128 net//sched/act_tunnel_key.c
121
122 static int tunnel_key_copy_opts(const struct nlattr *nla, u8 *dst,
123 int dst_len, struct netlink_ext_ack *extack)
124 {
125 int err, rem, opt_len, len = nla_len(nla), opts_len = 0;
126 const struct nlattr *attr, *head = nla_data(nla);
127
> 128 err = nla_validate_deprecated(head, len, TCA_TUNNEL_KEY_ENC_OPTS_MAX,
129 enc_opts_policy, extack);
130 if (err)
131 return err;
132
133 nla_for_each_attr(attr, head, len, rem) {
134 switch (nla_type(attr)) {
135 case TCA_TUNNEL_KEY_ENC_OPTS_GENEVE:
136 opt_len = tunnel_key_copy_geneve_opt(attr, dst,
137 dst_len, extack);
138 if (opt_len < 0)
139 return opt_len;
140 opts_len += opt_len;
141 if (dst) {
142 dst_len -= opt_len;
143 dst += opt_len;
144 }
145 break;
146 }
147 }
148
149 if (!opts_len) {
150 NL_SET_ERR_MSG(extack, "Empty list of tunnel options");
151 return -EINVAL;
152 }
153
154 if (rem > 0) {
155 NL_SET_ERR_MSG(extack, "Trailing data after parsing tunnel key options attributes");
156 return -EINVAL;
157 }
158
159 return opts_len;
160 }
161
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 46954 bytes --]
^ permalink raw reply
* Re: [PATCH] wireless: carl9170: fix clang build warning
From: Christian Lamparter @ 2019-03-25 14:20 UTC (permalink / raw)
To: Arnd Bergmann, Kalle Valo
Cc: David S. Miller, clang-built-linux, Nick Desaulniers,
Nathan Chancellor, Dan Carpenter, Gustavo A. R. Silva,
linux-wireless, netdev, linux-kernel
In-Reply-To: <20190325124354.1413529-1-arnd@arndb.de>
On Monday, March 25, 2019 1:43:44 PM CET Arnd Bergmann wrote:
> clang fails to eliminate some dead code with always-taken branches
> when CONFIG_PROFILE_ANNOTATED_BRANCHES is set, leading to a false-positive
> warning:
>
> drivers/net/wireless/ath/carl9170/mac.c:522:3: error: variable 'power' is used uninitialized whenever 'if' condition is
> false [-Werror,-Wsometimes-uninitialized]
> BUG_ON(1);
> ^~~~~~~~~
>
> Change both instances of BUG_ON(1) in carl9170 to the simpler BUG()
> to avoid the warning.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Heh, I added these "default:" cases back in the day to make gcc
shut-up about "switch is missing cases" warnings. And now I
guess it's clangs time to complain. (-;
Especially the "default case" in carl9170_rx_untie_data is hilarious.
Since all the possible cases (just 4!) have been addressed and the
"default" is deadcode. I think gcc did not consider the
AR9170_RX_STATUS_MPDU mask in the
| switch (mac_status & AR9170_RX_STATUS_MPDU) {
statement. However I'll be waiting for the day until the deadcode
elimination warnings will show up.
Anyway,
Acked-by: Christian Lamparter <chunkeey@gmail.com>
Regards,
Christian
> ---
> drivers/net/wireless/ath/carl9170/mac.c | 2 +-
> drivers/net/wireless/ath/carl9170/rx.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/carl9170/mac.c b/drivers/net/wireless/ath/carl9170/mac.c
> index 7d4a72dc98db..b2eeb9fd68d2 100644
> --- a/drivers/net/wireless/ath/carl9170/mac.c
> +++ b/drivers/net/wireless/ath/carl9170/mac.c
> @@ -519,7 +519,7 @@ int carl9170_set_mac_tpc(struct ar9170 *ar, struct ieee80211_channel *channel)
> power = ar->power_5G_leg[0] & 0x3f;
> break;
> default:
> - BUG_ON(1);
> + BUG();
> }
>
> power = min_t(unsigned int, power, ar->hw->conf.power_level * 2);
> diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c
> index 8e154f6364a3..23ab8a80c18c 100644
> --- a/drivers/net/wireless/ath/carl9170/rx.c
> +++ b/drivers/net/wireless/ath/carl9170/rx.c
> @@ -795,7 +795,7 @@ static void carl9170_rx_untie_data(struct ar9170 *ar, u8 *buf, int len)
> break;
>
> default:
> - BUG_ON(1);
> + BUG();
> break;
> }
>
>
^ permalink raw reply
* Re: [PATCH] mt76: usb: reduce locking in mt76u_tx_tasklet
From: Lorenzo Bianconi @ 2019-03-25 13:47 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Lorenzo Bianconi, nbd, linux-wireless
In-Reply-To: <20190325131019.GA19204@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]
> On Mon, Mar 25, 2019 at 02:00:36PM +0100, Lorenzo Bianconi wrote:
> > > On Sun, Mar 24, 2019 at 03:51:43PM +0100, Lorenzo Bianconi wrote:
> > > > + int idx;
> > > > +
> > > > sq = &dev->q_tx[i];
> > > > q = sq->q;
> > > >
> > > > - spin_lock_bh(&q->lock);
> > > > - while (true) {
> > > > - if (!q->entry[q->head].done || !q->queued)
> > > > + while (q->queued > n_queued) {
> > > > + if (!q->entry[q->head].done)
> > > > break;
> > > If you place done = false here you will not need additional idx
> > > variable.
> >
> > As Felix suggested, I would set done to false at the end of the loop, after
> > tx_complete_skb
> Why this is needed?
logically I think it should be the last thing to do on the current skb but
probably moving it before tx_complete_skb will not make any difference
>
> > > > dev->drv->tx_complete_skb(dev, i, &entry);
> > > > - spin_lock_bh(&q->lock);
> > > > + q->entry[idx].done = false;
> > > > }
> > > >
> > > > + spin_lock_bh(&q->lock);
> > > This patch does not apply for me as there is missing
> > > mt76_txq_schedule(dev, sq);
> >
> > Sorry I forgot to mention this patch is based on
> > https://patchwork.kernel.org/patch/10856027/. Have you applied it?
> No.
>
> Stanislaw
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] mt76: usb: reduce locking in mt76u_tx_tasklet
From: Stanislaw Gruszka @ 2019-03-25 13:10 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, nbd, linux-wireless
In-Reply-To: <20190325130035.GA12885@localhost.localdomain>
On Mon, Mar 25, 2019 at 02:00:36PM +0100, Lorenzo Bianconi wrote:
> > On Sun, Mar 24, 2019 at 03:51:43PM +0100, Lorenzo Bianconi wrote:
> > > + int idx;
> > > +
> > > sq = &dev->q_tx[i];
> > > q = sq->q;
> > >
> > > - spin_lock_bh(&q->lock);
> > > - while (true) {
> > > - if (!q->entry[q->head].done || !q->queued)
> > > + while (q->queued > n_queued) {
> > > + if (!q->entry[q->head].done)
> > > break;
> > If you place done = false here you will not need additional idx
> > variable.
>
> As Felix suggested, I would set done to false at the end of the loop, after
> tx_complete_skb
Why this is needed?
> > > dev->drv->tx_complete_skb(dev, i, &entry);
> > > - spin_lock_bh(&q->lock);
> > > + q->entry[idx].done = false;
> > > }
> > >
> > > + spin_lock_bh(&q->lock);
> > This patch does not apply for me as there is missing
> > mt76_txq_schedule(dev, sq);
>
> Sorry I forgot to mention this patch is based on
> https://patchwork.kernel.org/patch/10856027/. Have you applied it?
No.
Stanislaw
^ permalink raw reply
* Re: [PATCH] mt76: usb: reduce locking in mt76u_tx_tasklet
From: Lorenzo Bianconi @ 2019-03-25 13:00 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Lorenzo Bianconi, nbd, linux-wireless
In-Reply-To: <20190325125032.GB17956@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]
> On Sun, Mar 24, 2019 at 03:51:43PM +0100, Lorenzo Bianconi wrote:
> > + int idx;
> > +
> > sq = &dev->q_tx[i];
> > q = sq->q;
> >
> > - spin_lock_bh(&q->lock);
> > - while (true) {
> > - if (!q->entry[q->head].done || !q->queued)
> > + while (q->queued > n_queued) {
> > + if (!q->entry[q->head].done)
> > break;
> If you place done = false here you will not need additional idx
> variable.
As Felix suggested, I would set done to false at the end of the loop, after
tx_complete_skb
>
> > dev->drv->tx_complete_skb(dev, i, &entry);
> > - spin_lock_bh(&q->lock);
> > + q->entry[idx].done = false;
> > }
> >
> > + spin_lock_bh(&q->lock);
> This patch does not apply for me as there is missing
> mt76_txq_schedule(dev, sq);
Sorry I forgot to mention this patch is based on
https://patchwork.kernel.org/patch/10856027/. Have you applied it?
>
> > +
> > + sq->swq_queued -= n_sw_queued;
> > + q->queued -= n_queued;
> > +
> Naming is confusing, it should rather be n_dequeued, n_sw_dequeued.
I just followed dma counterpart naming convention, but I can modify it.
Regards,
Lorenzo
>
> Stanislaw
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] mt76: usb: reduce locking in mt76u_tx_tasklet
From: Stanislaw Gruszka @ 2019-03-25 12:50 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: nbd, linux-wireless, lorenzo.bianconi
In-Reply-To: <1f489df8bc635660a1d1fb72400e5562504c0555.1553437543.git.lorenzo@kernel.org>
On Sun, Mar 24, 2019 at 03:51:43PM +0100, Lorenzo Bianconi wrote:
> + int idx;
> +
> sq = &dev->q_tx[i];
> q = sq->q;
>
> - spin_lock_bh(&q->lock);
> - while (true) {
> - if (!q->entry[q->head].done || !q->queued)
> + while (q->queued > n_queued) {
> + if (!q->entry[q->head].done)
> break;
If you place done = false here you will not need additional idx
variable.
> dev->drv->tx_complete_skb(dev, i, &entry);
> - spin_lock_bh(&q->lock);
> + q->entry[idx].done = false;
> }
>
> + spin_lock_bh(&q->lock);
This patch does not apply for me as there is missing
mt76_txq_schedule(dev, sq);
> +
> + sq->swq_queued -= n_sw_queued;
> + q->queued -= n_queued;
> +
Naming is confusing, it should rather be n_dequeued, n_sw_dequeued.
Stanislaw
^ permalink raw reply
* [PATCH] wireless: carl9170: fix clang build warning
From: Arnd Bergmann @ 2019-03-25 12:43 UTC (permalink / raw)
To: Christian Lamparter, Kalle Valo, David S. Miller
Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
Arnd Bergmann, Dan Carpenter, Gustavo A. R. Silva, linux-wireless,
netdev, linux-kernel
clang fails to eliminate some dead code with always-taken branches
when CONFIG_PROFILE_ANNOTATED_BRANCHES is set, leading to a false-positive
warning:
drivers/net/wireless/ath/carl9170/mac.c:522:3: error: variable 'power' is used uninitialized whenever 'if' condition is
false [-Werror,-Wsometimes-uninitialized]
BUG_ON(1);
^~~~~~~~~
Change both instances of BUG_ON(1) in carl9170 to the simpler BUG()
to avoid the warning.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wireless/ath/carl9170/mac.c | 2 +-
drivers/net/wireless/ath/carl9170/rx.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/carl9170/mac.c b/drivers/net/wireless/ath/carl9170/mac.c
index 7d4a72dc98db..b2eeb9fd68d2 100644
--- a/drivers/net/wireless/ath/carl9170/mac.c
+++ b/drivers/net/wireless/ath/carl9170/mac.c
@@ -519,7 +519,7 @@ int carl9170_set_mac_tpc(struct ar9170 *ar, struct ieee80211_channel *channel)
power = ar->power_5G_leg[0] & 0x3f;
break;
default:
- BUG_ON(1);
+ BUG();
}
power = min_t(unsigned int, power, ar->hw->conf.power_level * 2);
diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c
index 8e154f6364a3..23ab8a80c18c 100644
--- a/drivers/net/wireless/ath/carl9170/rx.c
+++ b/drivers/net/wireless/ath/carl9170/rx.c
@@ -795,7 +795,7 @@ static void carl9170_rx_untie_data(struct ar9170 *ar, u8 *buf, int len)
break;
default:
- BUG_ON(1);
+ BUG();
break;
}
--
2.20.0
^ permalink raw reply related
* Re: [PATCH] ath10k: reset chip after supported check
From: Michał Kazior @ 2019-03-25 12:14 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Christian Lamparter, Tomislav Požega, linux-wireless,
openwrt-devel, Kalle Valo
In-Reply-To: <edc38f32-977a-53f5-babd-605775326a20@broadcom.com>
On Sat, 23 Mar 2019 at 08:20, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
>
> * resending with corrected email address from Kalle
> --------------------------------------------------------------------
> + Michał
Thanks!
> On 3/22/2019 8:25 PM, Christian Lamparter wrote:
> > On Friday, March 22, 2019 7:58:40 PM CET Tomislav Požega wrote:
> >> When chip reset is done before the chip is checked if supported
> >> there will be crash. Previous behaviour caused bootloops on
> >> Archer C7 v1 units, this patch allows clean device boot without
> >> excluding ath10k driver.
Can you elaborate more a bit? What kind of crashes are you seeing?
What does the bootloop look like? Do you have uart connected to
diagnose?
Didn't C7 v1 have the old QCA9880 hw v1 which isn't really supported
by ath10k? I recall the v1 chip was really buggy and required
hammering registers sometimes to get things working.
[...]
> Looking at the commit subject makes me suspicious whether this is a
> proper fix.
[...]
> It seems to me the chip reset was done explicitly *before* reading the
> chipid for a reason.
>
> """
> ath10k: reset chip before reading chip_id in probe
> There are some very rare cases with some hardware
> configuration that the device doesn't init quickly
> enough in which case reading chip_id yielded 0.
> This caused driver to subsequently fail to setup
> the device.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
> """
Good catch, Arend!
> Might be the ath10k_pci_chip_reset() function needs to be modified to
> work properly for Archer C7 v1 units.
When driver boots up hardware is in an unknown state. It could've been
power cycled or it could've remained powered via pcie aux while host
cpu went through a warm reset (eg. watchdog). For what it's worth it
could still be beaconing if that is what it was doing before host cpu
reset. Therefore the device can be in a messy state and it should be
reset before talking to it or else you risk reading garbage.
I don't recall details about this particular commit but given the
timeline of commits at the time this change suggests this was related
to supporting qca61x4. In other words your change risks breaking some
qca61x4 chips.
Michał
^ permalink raw reply
* Re: [PATCH] mac80211: HE: add Spatial Reuse IE parsing support
From: John Crispin @ 2019-03-25 9:32 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <b781d983f2f5792a671a66075eaee6f3515aba6f.camel@sipsolutions.net>
On 25/03/2019 09:49, Johannes Berg wrote:
> On Mon, 2019-03-25 at 09:20 +0100, John Crispin wrote:
>> Add support to mac80211 for parsing SPR IEs as per
>> P802.11ax_D3.0 section 9.4.2.241.
> Hmm, if you're not going to use it, why parse it? Am I missing a follow-
> up patch?
>
> Also, consider checking against D4.0, Luca just sent a patch to update
> to that for all the rest of the code.
>
> johannes
>
Hi Johannes,
This patch is in preparation for the upcoming OBBS PD support inside
Kalle's ath11k-bringup tree. Forgot to mention that sorry.
I'll also check against D4.0.
John
^ permalink raw reply
* [PATCH 1/2] mac80211: minstrel_ht: add support for rates with 4 spatial streams
From: Felix Fietkau @ 2019-03-25 8:50 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes
This is needed for the upcoming driver for MT7615 4x4 802.11ac chipsets
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
net/mac80211/rc80211_minstrel_ht.c | 10 ++++++++++
net/mac80211/rc80211_minstrel_ht.h | 2 +-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index ccaf951e4e31..00a3a8ce27fe 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -157,44 +157,54 @@ const struct mcs_group minstrel_mcs_groups[] = {
MCS_GROUP(1, 0, BW_20, 5),
MCS_GROUP(2, 0, BW_20, 4),
MCS_GROUP(3, 0, BW_20, 4),
+ MCS_GROUP(4, 0, BW_20, 4),
MCS_GROUP(1, 1, BW_20, 5),
MCS_GROUP(2, 1, BW_20, 4),
MCS_GROUP(3, 1, BW_20, 4),
+ MCS_GROUP(4, 1, BW_20, 4),
MCS_GROUP(1, 0, BW_40, 4),
MCS_GROUP(2, 0, BW_40, 4),
MCS_GROUP(3, 0, BW_40, 4),
+ MCS_GROUP(4, 0, BW_40, 4),
MCS_GROUP(1, 1, BW_40, 4),
MCS_GROUP(2, 1, BW_40, 4),
MCS_GROUP(3, 1, BW_40, 4),
+ MCS_GROUP(4, 1, BW_40, 4),
CCK_GROUP(8),
VHT_GROUP(1, 0, BW_20, 5),
VHT_GROUP(2, 0, BW_20, 4),
VHT_GROUP(3, 0, BW_20, 4),
+ VHT_GROUP(4, 0, BW_20, 4),
VHT_GROUP(1, 1, BW_20, 5),
VHT_GROUP(2, 1, BW_20, 4),
VHT_GROUP(3, 1, BW_20, 4),
+ VHT_GROUP(4, 1, BW_20, 4),
VHT_GROUP(1, 0, BW_40, 4),
VHT_GROUP(2, 0, BW_40, 4),
VHT_GROUP(3, 0, BW_40, 4),
+ VHT_GROUP(4, 0, BW_40, 3),
VHT_GROUP(1, 1, BW_40, 4),
VHT_GROUP(2, 1, BW_40, 4),
VHT_GROUP(3, 1, BW_40, 4),
+ VHT_GROUP(4, 1, BW_40, 3),
VHT_GROUP(1, 0, BW_80, 4),
VHT_GROUP(2, 0, BW_80, 4),
VHT_GROUP(3, 0, BW_80, 4),
+ VHT_GROUP(4, 0, BW_80, 2),
VHT_GROUP(1, 1, BW_80, 4),
VHT_GROUP(2, 1, BW_80, 4),
VHT_GROUP(3, 1, BW_80, 4),
+ VHT_GROUP(4, 1, BW_80, 2),
};
static u8 sample_table[SAMPLE_COLUMNS][MCS_GROUP_RATES] __read_mostly;
diff --git a/net/mac80211/rc80211_minstrel_ht.h b/net/mac80211/rc80211_minstrel_ht.h
index 26b7a3244b47..f762e5ba7c2e 100644
--- a/net/mac80211/rc80211_minstrel_ht.h
+++ b/net/mac80211/rc80211_minstrel_ht.h
@@ -13,7 +13,7 @@
* The number of streams can be changed to 2 to reduce code
* size and memory footprint.
*/
-#define MINSTREL_MAX_STREAMS 3
+#define MINSTREL_MAX_STREAMS 4
#define MINSTREL_HT_STREAM_GROUPS 4 /* BW(=2) * SGI(=2) */
#define MINSTREL_VHT_STREAM_GROUPS 6 /* BW(=3) * SGI(=2) */
--
2.17.0
^ permalink raw reply related
* [PATCH 2/2] mac80211: minstrel_ht: automatically calculate rate duration shift
From: Felix Fietkau @ 2019-03-25 8:50 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes
In-Reply-To: <20190325085016.53727-1-nbd@nbd.name>
A per-group shift was added to reduce the size of the per-rate transmit
duration field to u16 without sacrificing a lot of precision
This patch changes the macros to automatically calculate the best value for
this shift based on the lowest rate within the group.
This simplifies adding more groups and slightly improves accuracy for some of
the existing groups.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
net/mac80211/rc80211_minstrel_ht.c | 134 +++++++++++++++++------------
1 file changed, 80 insertions(+), 54 deletions(-)
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 00a3a8ce27fe..8b168724c5e7 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -51,8 +51,13 @@
MINSTREL_MAX_STREAMS * _sgi + \
_streams - 1
+#define _MAX(a, b) (((a)>(b))?(a):(b))
+
+#define GROUP_SHIFT(duration) \
+ _MAX(0, 16 - __builtin_clz(duration))
+
/* MCS rate information for an MCS group */
-#define MCS_GROUP(_streams, _sgi, _ht40, _s) \
+#define __MCS_GROUP(_streams, _sgi, _ht40, _s) \
[GROUP_IDX(_streams, _sgi, _ht40)] = { \
.streams = _streams, \
.shift = _s, \
@@ -72,6 +77,13 @@
} \
}
+#define MCS_GROUP_SHIFT(_streams, _sgi, _ht40) \
+ GROUP_SHIFT(MCS_DURATION(_streams, _sgi, _ht40 ? 54 : 26))
+
+#define MCS_GROUP(_streams, _sgi, _ht40) \
+ __MCS_GROUP(_streams, _sgi, _ht40, \
+ MCS_GROUP_SHIFT(_streams, _sgi, _ht40))
+
#define VHT_GROUP_IDX(_streams, _sgi, _bw) \
(MINSTREL_VHT_GROUP_0 + \
MINSTREL_MAX_STREAMS * 2 * (_bw) + \
@@ -81,7 +93,7 @@
#define BW2VBPS(_bw, r3, r2, r1) \
(_bw == BW_80 ? r3 : _bw == BW_40 ? r2 : r1)
-#define VHT_GROUP(_streams, _sgi, _bw, _s) \
+#define __VHT_GROUP(_streams, _sgi, _bw, _s) \
[VHT_GROUP_IDX(_streams, _sgi, _bw)] = { \
.streams = _streams, \
.shift = _s, \
@@ -114,6 +126,14 @@
} \
}
+#define VHT_GROUP_SHIFT(_streams, _sgi, _bw) \
+ GROUP_SHIFT(MCS_DURATION(_streams, _sgi, \
+ BW2VBPS(_bw, 117, 54, 26)))
+
+#define VHT_GROUP(_streams, _sgi, _bw) \
+ __VHT_GROUP(_streams, _sgi, _bw, \
+ VHT_GROUP_SHIFT(_streams, _sgi, _bw))
+
#define CCK_DURATION(_bitrate, _short, _len) \
(1000 * (10 /* SIFS */ + \
(_short ? 72 + 24 : 144 + 48) + \
@@ -129,7 +149,7 @@
CCK_ACK_DURATION(55, _short) >> _s, \
CCK_ACK_DURATION(110, _short) >> _s
-#define CCK_GROUP(_s) \
+#define __CCK_GROUP(_s) \
[MINSTREL_CCK_GROUP] = { \
.streams = 1, \
.flags = 0, \
@@ -140,6 +160,12 @@
} \
}
+#define CCK_GROUP_SHIFT \
+ GROUP_SHIFT(CCK_ACK_DURATION(10, false))
+
+#define CCK_GROUP __CCK_GROUP(CCK_GROUP_SHIFT)
+
+
static bool minstrel_vht_only = true;
module_param(minstrel_vht_only, bool, 0644);
MODULE_PARM_DESC(minstrel_vht_only,
@@ -154,57 +180,57 @@ MODULE_PARM_DESC(minstrel_vht_only,
* BW -> SGI -> #streams
*/
const struct mcs_group minstrel_mcs_groups[] = {
- MCS_GROUP(1, 0, BW_20, 5),
- MCS_GROUP(2, 0, BW_20, 4),
- MCS_GROUP(3, 0, BW_20, 4),
- MCS_GROUP(4, 0, BW_20, 4),
-
- MCS_GROUP(1, 1, BW_20, 5),
- MCS_GROUP(2, 1, BW_20, 4),
- MCS_GROUP(3, 1, BW_20, 4),
- MCS_GROUP(4, 1, BW_20, 4),
-
- MCS_GROUP(1, 0, BW_40, 4),
- MCS_GROUP(2, 0, BW_40, 4),
- MCS_GROUP(3, 0, BW_40, 4),
- MCS_GROUP(4, 0, BW_40, 4),
-
- MCS_GROUP(1, 1, BW_40, 4),
- MCS_GROUP(2, 1, BW_40, 4),
- MCS_GROUP(3, 1, BW_40, 4),
- MCS_GROUP(4, 1, BW_40, 4),
-
- CCK_GROUP(8),
-
- VHT_GROUP(1, 0, BW_20, 5),
- VHT_GROUP(2, 0, BW_20, 4),
- VHT_GROUP(3, 0, BW_20, 4),
- VHT_GROUP(4, 0, BW_20, 4),
-
- VHT_GROUP(1, 1, BW_20, 5),
- VHT_GROUP(2, 1, BW_20, 4),
- VHT_GROUP(3, 1, BW_20, 4),
- VHT_GROUP(4, 1, BW_20, 4),
-
- VHT_GROUP(1, 0, BW_40, 4),
- VHT_GROUP(2, 0, BW_40, 4),
- VHT_GROUP(3, 0, BW_40, 4),
- VHT_GROUP(4, 0, BW_40, 3),
-
- VHT_GROUP(1, 1, BW_40, 4),
- VHT_GROUP(2, 1, BW_40, 4),
- VHT_GROUP(3, 1, BW_40, 4),
- VHT_GROUP(4, 1, BW_40, 3),
-
- VHT_GROUP(1, 0, BW_80, 4),
- VHT_GROUP(2, 0, BW_80, 4),
- VHT_GROUP(3, 0, BW_80, 4),
- VHT_GROUP(4, 0, BW_80, 2),
-
- VHT_GROUP(1, 1, BW_80, 4),
- VHT_GROUP(2, 1, BW_80, 4),
- VHT_GROUP(3, 1, BW_80, 4),
- VHT_GROUP(4, 1, BW_80, 2),
+ MCS_GROUP(1, 0, BW_20),
+ MCS_GROUP(2, 0, BW_20),
+ MCS_GROUP(3, 0, BW_20),
+ MCS_GROUP(4, 0, BW_20),
+
+ MCS_GROUP(1, 1, BW_20),
+ MCS_GROUP(2, 1, BW_20),
+ MCS_GROUP(3, 1, BW_20),
+ MCS_GROUP(4, 1, BW_20),
+
+ MCS_GROUP(1, 0, BW_40),
+ MCS_GROUP(2, 0, BW_40),
+ MCS_GROUP(3, 0, BW_40),
+ MCS_GROUP(4, 0, BW_40),
+
+ MCS_GROUP(1, 1, BW_40),
+ MCS_GROUP(2, 1, BW_40),
+ MCS_GROUP(3, 1, BW_40),
+ MCS_GROUP(4, 1, BW_40),
+
+ CCK_GROUP,
+
+ VHT_GROUP(1, 0, BW_20),
+ VHT_GROUP(2, 0, BW_20),
+ VHT_GROUP(3, 0, BW_20),
+ VHT_GROUP(4, 0, BW_20),
+
+ VHT_GROUP(1, 1, BW_20),
+ VHT_GROUP(2, 1, BW_20),
+ VHT_GROUP(3, 1, BW_20),
+ VHT_GROUP(4, 1, BW_20),
+
+ VHT_GROUP(1, 0, BW_40),
+ VHT_GROUP(2, 0, BW_40),
+ VHT_GROUP(3, 0, BW_40),
+ VHT_GROUP(4, 0, BW_40),
+
+ VHT_GROUP(1, 1, BW_40),
+ VHT_GROUP(2, 1, BW_40),
+ VHT_GROUP(3, 1, BW_40),
+ VHT_GROUP(4, 1, BW_40),
+
+ VHT_GROUP(1, 0, BW_80),
+ VHT_GROUP(2, 0, BW_80),
+ VHT_GROUP(3, 0, BW_80),
+ VHT_GROUP(4, 0, BW_80),
+
+ VHT_GROUP(1, 1, BW_80),
+ VHT_GROUP(2, 1, BW_80),
+ VHT_GROUP(3, 1, BW_80),
+ VHT_GROUP(4, 1, BW_80),
};
static u8 sample_table[SAMPLE_COLUMNS][MCS_GROUP_RATES] __read_mostly;
--
2.17.0
^ permalink raw reply related
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