* Re: [PATCH v3 1/9] cfg80211: add start / stop NAN commands
From: Johannes Berg @ 2016-09-30 11:25 UTC (permalink / raw)
To: Luca Coelho
Cc: linux-wireless, ayala.beker, andrei.otcheretianski,
arend.vanspriel, Emmanuel Grumbach, Luca Coelho
In-Reply-To: <20160920143121.28247-2-luca@coelho.fi>
All applied.
johannes
^ permalink raw reply
* Re: [PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices
From: Johannes Berg @ 2016-09-30 11:33 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, make-wifi-fast, linux-wireless,
netdev
In-Reply-To: <20160923195911.4572-4-toke@toke.dk>
On Fri, 2016-09-23 at 21:59 +0200, Toke Høiland-Jørgensen wrote:
> Small devices can run out of memory from queueing too many packets.
> If VHT is not supported by the PHY, having more than 4 MBytes of
> total queue in the TXQ intermediate queues is not needed, and so we
> can safely limit the memory usage in these cases and avoid OOM.
I kinda see the logic here - we really don't need to queue as much if
we can't possibly transmit it out quickly - but it seems to me we
should also throw in some kind of limit that's relative to the amount
of memory you have on the system?
I've applied these anyway though. I just don't like your assumption (b)
much as the rationale for it.
johannes
^ permalink raw reply
* Re: [PATCH] mac80211: fix CMD_FRAME for AP_VLAN
From: Johannes Berg @ 2016-09-30 11:41 UTC (permalink / raw)
To: Michael Braun; +Cc: linux-wireless, projekt-wlan
In-Reply-To: <1474786035-15410-1-git-send-email-michael-dev@fami-braun.de>
> net/mac80211/offchannel.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
> index 55a9c5b..2afd329 100644
> --- a/net/mac80211/offchannel.c
> +++ b/net/mac80211/offchannel.c
> @@ -819,7 +819,10 @@ int ieee80211_mgmt_tx(struct wiphy *wiphy,
> struct wireless_dev *wdev,
> mgmt->u.action.category ==
> WLAN_CATEGORY_SPECTRUM_MGMT)
> break;
> rcu_read_lock();
> - sta = sta_info_get(sdata, mgmt->da);
> + if (ieee80211_vif_is_mesh(&sdata->vif))
> + sta = sta_info_get(sdata, mgmt->da);
> + else
> + sta = sta_info_get_bss(sdata, mgmt->da);
>
I don't see why you need to distinguish between mesh and non-mesh here?
get_bss() will ignore the BSS pointer if it's NULL, and that will
always be the case when the type is mesh, so ... why?
johannes
^ permalink raw reply
* Re: [PATCH 0/4] ath10k: fix mesh sync operation
From: Johannes Berg @ 2016-09-30 11:47 UTC (permalink / raw)
To: Thomas Pedersen, ath10k; +Cc: linux-wireless
In-Reply-To: <20160928235631.9197-1-twp@qca.qualcomm.com>
On Wed, 2016-09-28 at 16:56 -0700, Thomas Pedersen wrote:
> This patchset introduces a new ieee80211_op offset_tsf(), which gives
> the driver a s64 TSF offset for TSF adjustment. This is more accurate
> than programming absolute TSF since programming delay is avoided.
>
You were lucky we have patchwork, otherwise I would've missed the
mac80211 patches in this series :)
Applied both now - no reason to apply them in the order you sent them.
johannes
^ permalink raw reply
* [PATCH 1/3] cw1200: Don't leak memory if krealloc failes
From: Johannes Thumshirn @ 2016-09-30 12:11 UTC (permalink / raw)
To: Solomon Peachy, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, Johannes Thumshirn
The call to krealloc() in wsm_buf_reserve() directly assigns the newly
returned memory to buf->begin. This is all fine except when krealloc()
failes we loose the ability to free the old memory pointed to by
buf->begin. If we just create a temporary variable to assign memory to
and assign the memory to it we can mitigate the memory leak.
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
drivers/net/wireless/st/cw1200/wsm.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/st/cw1200/wsm.c b/drivers/net/wireless/st/cw1200/wsm.c
index 680d60e..12fad99 100644
--- a/drivers/net/wireless/st/cw1200/wsm.c
+++ b/drivers/net/wireless/st/cw1200/wsm.c
@@ -1807,16 +1807,18 @@ static int wsm_buf_reserve(struct wsm_buf *buf, size_t extra_size)
{
size_t pos = buf->data - buf->begin;
size_t size = pos + extra_size;
+ u8 *tmp;
size = round_up(size, FWLOAD_BLOCK_SIZE);
- buf->begin = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
- if (buf->begin) {
- buf->data = &buf->begin[pos];
- buf->end = &buf->begin[size];
- return 0;
- } else {
- buf->end = buf->data = buf->begin;
+ tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
+ if (tmp) {
+ wsm_buf_deinit(buf);
return -ENOMEM;
}
+
+ buf->begin = tmp;
+ buf->data = &buf->begin[pos];
+ buf->end = &buf->begin[size];
+ return 0;
}
--
1.8.5.6
^ permalink raw reply related
* Re: [PATCH 1/3] cw1200: Don't leak memory if krealloc failes
From: Johannes Berg @ 2016-09-30 12:29 UTC (permalink / raw)
To: Johannes Thumshirn, Solomon Peachy, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <1475237495-15030-1-git-send-email-jthumshirn@suse.de>
> + tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> + if (tmp) {
>
I think that check is inverted?
johannes
^ permalink raw reply
* Re: [PATCH 1/3] cw1200: Don't leak memory if krealloc failes
From: Johannes Thumshirn @ 2016-09-30 12:33 UTC (permalink / raw)
To: Johannes Berg
Cc: Solomon Peachy, Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <1475238579.17481.56.camel@sipsolutions.net>
On Fri, Sep 30, 2016 at 02:29:39PM +0200, Johannes Berg wrote:
>
> > + tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> > + if (tmp) {
> >
> I think that check is inverted?
>
> johannes
Fu.. you're right, of cause it's !tmp.
I'll resend.
Thanks,
Johannes
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply
* Re: [PATCH v9 2/2] mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue
From: Toke Høiland-Jørgensen @ 2016-09-30 12:39 UTC (permalink / raw)
To: Johannes Berg; +Cc: make-wifi-fast, linux-wireless
In-Reply-To: <1475231220.17481.32.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> Hi Toke,
>
> Sorry for the delay reviewing this.
>
> I think I still have a few comments/questions.
No worries. And not terribly surprised ;)
>> +static inline bool txq_has_queue(struct ieee80211_txq *txq)
>> +{
>> + struct txq_info *txqi =3D to_txq_info(txq);
>> + return !(skb_queue_empty(&txqi->frags) && !txqi->tin.backlog_packets);
>> +}
>
> Tiny nit - there should probably be a blank line between the two lines
> here, but I could just fix that when I apply if you don't resend anyway
> for some other reason.
Noted.
> [snip helper stuff that looks fine]
>
>> - if (!tx->sta->sta.txq[0])
>> - hdr->seq_ctrl =3D ieee80211_tx_next_seq(tx->sta, tid);
>> + hdr->seq_ctrl =3D ieee80211_tx_next_seq(tx->sta, tid);
>
> Just to make sure I get this right - this is because the handler is now
> run on dequeue, so the special case is no longer needed?
Yup. The same change is made in xmit_fast (but obscured by the moving of
the surrounding code into _finish()).
>> =C2=A0#define CALL_TXH(txh) \
>> @@ -1656,6 +1684,31 @@ static int invoke_tx_handlers(struct ieee80211_tx=
_data *tx)
>> =C2=A0 if (!ieee80211_hw_check(&tx->local->hw, HAS_RATE_CONTROL))
>> =C2=A0 CALL_TXH(ieee80211_tx_h_rate_ctrl);
>
> Just for reference - the code block here that's unchanged contains
> this:
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 CALL_TXH(ieee80211_tx_h_dynamic_ps);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CALL_TXH(ieee80211_tx_h_c=
heck_assoc);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CALL_TXH(ieee80211_tx_h_p=
s_buf);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CALL_TXH(ieee80211_tx_h_c=
heck_control_port_protocol);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CALL_TXH(ieee80211_tx_h_s=
elect_key);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (!ieee80211_hw_check(&=
tx->local->hw, HAS_RATE_CONTROL))
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0CALL_TXH(ieee80211_tx_h_rate_ctrl);
>
>> +static int invoke_tx_handlers_late(struct ieee80211_tx_data *tx)
>> +{
>> + struct ieee80211_tx_info *info =3D IEEE80211_SKB_CB(tx->skb);
>> + ieee80211_tx_result res =3D TX_CONTINUE;
>> +
>> =C2=A0 if (unlikely(info->flags &
>> IEEE80211_TX_INTFL_RETRANSMISSION)) {
>> =C2=A0 __skb_queue_tail(&tx->skbs, tx->skb);
>> =C2=A0 tx->skb =3D NULL;
>
> And this code here is also unchanged from the original TX handler
> invocation, so contains this:
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (unlikely(info->flags & IEEE80211_TX_INTFL=
_RETRANSMISSION)) {
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0__skb_queue_tail(&tx->skbs, tx->skb);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0tx->skb =3D NULL;
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0goto txh_done;
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
>
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CALL_TXH(ieee80211_tx_h_m=
ichael_mic_add);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CALL_TXH(ieee80211_tx_h_s=
equence);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CALL_TXH(ieee80211_tx_h_f=
ragment);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0/* handlers after fragmen=
t must be aware of tx info fragmentation! */
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CALL_TXH(ieee80211_tx_h_s=
tats);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0CALL_TXH(ieee80211_tx_h_e=
ncrypt);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (!ieee80211_hw_check(&=
tx->local->hw, HAS_RATE_CONTROL))
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0CALL_TXH(ieee80211_tx_h_calculate_duration);
>
> But now you have a problem (that you solved) that the key pointer can
> be invalidated while you have the packet queued between the two points,
> and then the tx_h_michael_mic_add and/or tx_h_encrypt would crash.
>
> You solve this by re-running tx_h_select_key() on dequeue, but it's not
> clear to me why you didn't move that to the late handlers instead?
Because I need to run it anyway for the xmit_fast path on dequeue. I
thought doing it this way simplifies the code (at the cost of the
handler getting called twice when xmit_fast is not active).
> I *think* it should commute with the rate control handler, but even
> so, wouldn't it make more sense to have rate control late? Assuming
> the packets are queued for some amount of time, having rate control
> information queued with them would get stale.
Yes, having rate control run at dequeue would be good, and that's what I
did initially. However, I found that this would lead to a deadlock
because the rate control handler would send out packets in some cases (I
forget the details but can go back and check if needed). And since the
dequeue function is called with the driver TXQ lock held, that would
lead to a deadlock when those packets made it to the driver TX path.
So I decided to just keep it this way for now; I plan to go poking into
the rate controller later anyway, so moving the handler to later could
be part of that.
> Similarly, it seems to me that checking the control port protocol later
> (or perhaps duplicating that?) would be a good idea?
But that handler only sets a few flags? Is
tx->sdata->control_port_protocol likely to change while the packet is
queued?
>> +/*
>> + * Can be called while the sta lock is held. Anything that can cause
>> packets to
>> + * be generated will cause deadlock!
>> + */
>> +static bool ieee80211_xmit_fast_finish(struct ieee80211_sub_if_data
>> *sdata,
>> + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct sta_info *sta, u8
>> pn_offs,
>> + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct ieee80211_key *key,
>> + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct sk_buff *skb)
>
> That should be a void function now, you never check the return value
> and only return true anyway.
Noted.
>> + struct ieee80211_tx_info *info;
>> + struct ieee80211_tx_data tx;
>> + ieee80211_tx_result r;
>> +
>
> nit: extra blank line
The horror ;) (thought I got rid of all those; ah well, will fix)
>
>> =C2=A0 spin_lock_bh(&fq->lock);
>> =C2=A0
>> =C2=A0 if (test_bit(IEEE80211_TXQ_STOP, &txqi->flags))
>> =C2=A0 goto out;
>> =C2=A0
>> + /* Make sure fragments stay together. */
>> + skb =3D __skb_dequeue(&txqi->frags);
>> + if (skb)
>> + goto out;
>> +
>> +begin:
>
> I guess now that you introduced that anyway, we should consider making
> the skb_linearize() failure go there. Should be a follow-up patch
> though.
Can do.
>
>> + /*
>> + =C2=A0* The key can be removed while the packet was queued, so
>> need to call
>> + =C2=A0* this here to get the current key.
>> + =C2=A0*/
>> + r =3D ieee80211_tx_h_select_key(&tx);
>> + if (r !=3D TX_CONTINUE) {
>> + ieee80211_free_txskb(&local->hw, skb);
>> + goto begin;
>> + }
>> +
>> + if (txq->sta && info->control.flags & IEEE80211_TX_CTRL_FAST_XMIT) {
>
> It's a bit unfortunate that you lose fast-xmit here completely for the
> key stuff, but I don't see a good way to avoid that, other than
> completely rejiggering all the (possibly affected) queues when keys
> change... might be very complex to do that, certainly a follow-up
> patch if it's desired.
Yeah, figured it was better to have something that's correct and then go
back and change it if the performance hit turns out to be too high.
> This check seems a bit weird though - how could fast-xmit be set
> without a TXQ station?
I think that is probably just left over from before I introduced the
control flag. Should be fine to remove it.
>> +++ b/net/mac80211/util.c
>> @@ -3393,11 +3393,18 @@ void ieee80211_txq_get_depth(struct
>> ieee80211_txq *txq,
>> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0unsigned long *byte_cnt)
>> =C2=A0{
>> =C2=A0 struct txq_info *txqi =3D to_txq_info(txq);
>> + u32 frag_cnt =3D 0, frag_bytes =3D 0;
>> + struct sk_buff *skb;
>> +
>> + skb_queue_walk(&txqi->frags, skb) {
>> + frag_cnt++;
>> + frag_bytes +=3D skb->len;
>> + }
>
> I hope this is called infrequently :)
Well, ath10k is the only user. It does get called on each wake_tx_queue,
though, so not that infrequently. My reasoning was that since the frags
queue is never going to have more than a fairly small number of packets
in it (those produced from a single split packet), counting this way is
acceptable instead of keeping a state variable up to date. Can change it
if you disagree :)
Not sure if you want a v10, or if you're satisfied with the above
comments and will just fix up the nits on merging?
-Toke
^ permalink raw reply
* [PATCH v2] cw1200: Don't leak memory if krealloc failes
From: Johannes Thumshirn @ 2016-09-30 12:39 UTC (permalink / raw)
To: Solomon Peachy, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, Johannes Thumshirn,
Johannes Berg
The call to krealloc() in wsm_buf_reserve() directly assigns the newly
returned memory to buf->begin. This is all fine except when krealloc()
failes we loose the ability to free the old memory pointed to by
buf->begin. If we just create a temporary variable to assign memory to
and assign the memory to it we can mitigate the memory leak.
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Cc: Johannes Berg <johannes@sipsolutions.net>
---
drivers/net/wireless/st/cw1200/wsm.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
Changes from v1:
* Do the correct check for a failed re-allocation (Johannes Berg)
diff --git a/drivers/net/wireless/st/cw1200/wsm.c b/drivers/net/wireless/st/cw1200/wsm.c
index 680d60e..ffb245e 100644
--- a/drivers/net/wireless/st/cw1200/wsm.c
+++ b/drivers/net/wireless/st/cw1200/wsm.c
@@ -1807,16 +1807,18 @@ static int wsm_buf_reserve(struct wsm_buf *buf, size_t extra_size)
{
size_t pos = buf->data - buf->begin;
size_t size = pos + extra_size;
+ u8 *tmp;
size = round_up(size, FWLOAD_BLOCK_SIZE);
- buf->begin = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
- if (buf->begin) {
- buf->data = &buf->begin[pos];
- buf->end = &buf->begin[size];
- return 0;
- } else {
- buf->end = buf->data = buf->begin;
+ tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
+ if (!tmp) {
+ wsm_buf_deinit(buf);
return -ENOMEM;
}
+
+ buf->begin = tmp;
+ buf->data = &buf->begin[pos];
+ buf->end = &buf->begin[size];
+ return 0;
}
--
1.8.5.6
^ permalink raw reply related
* Re: [PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices
From: Toke Høiland-Jørgensen @ 2016-09-30 12:41 UTC (permalink / raw)
To: Johannes Berg; +Cc: make-wifi-fast, linux-wireless, netdev
In-Reply-To: <1475235230.17481.43.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> On Fri, 2016-09-23 at 21:59 +0200, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>> Small devices can run out of memory from queueing too many packets.
>> If VHT is not supported by the PHY, having more than 4 MBytes of
>> total queue in the TXQ intermediate queues is not needed, and so we
>> can safely limit the memory usage in these cases and avoid OOM.
>
> I kinda see the logic here - we really don't need to queue as much if
> we can't possibly transmit it out quickly - but it seems to me we
> should also throw in some kind of limit that's relative to the amount
> of memory you have on the system?
Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently
carries a patch for that which just changes the hard-coded default to
another hard-coded default. Not sure how to get a good value to use,
though; and deciding on how large a fraction of memory to use for
packets starts smelling an awful lot like setting policy in the kernel,
doesn't it?
> I've applied these anyway though. I just don't like your assumption (b)
> much as the rationale for it.
Right, thanks. I'll come up with a better rationale next time ;)
-Toke
^ permalink raw reply
* Re: [PATCH v9 2/2] mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue
From: Johannes Berg @ 2016-09-30 12:43 UTC (permalink / raw)
To: Toke Høiland-Jørgensen; +Cc: make-wifi-fast, linux-wireless
In-Reply-To: <87y4295zpy.fsf@toke.dk>
> Because I need to run it anyway for the xmit_fast path on dequeue. I
> thought doing it this way simplifies the code (at the cost of the
> handler getting called twice when xmit_fast is not active).
Ok, that's fair.
> > I *think* it should commute with the rate control handler, but even
> > so, wouldn't it make more sense to have rate control late? Assuming
> > the packets are queued for some amount of time, having rate control
> > information queued with them would get stale.
>
> Yes, having rate control run at dequeue would be good, and that's
> what I did initially. However, I found that this would lead to a
> deadlock because the rate control handler would send out packets in
> some cases (I forget the details but can go back and check if
> needed). And since the dequeue function is called with the driver TXQ
> lock held, that would lead to a deadlock when those packets made it
> to the driver TX path.
That seems really odd, but I can see how a deadlock happens then.
> So I decided to just keep it this way for now; I plan to go poking
> into the rate controller later anyway, so moving the handler to later
> could be part of that.
Sure, that's fair.
> But that handler only sets a few flags? Is
> tx->sdata->control_port_protocol likely to change while the packet is
> queued?
Oh right, I confused things there. We check the controlled port much
earlier, but anyway that should be OK.
> > It's a bit unfortunate that you lose fast-xmit here completely for
> > the key stuff, but I don't see a good way to avoid that, other than
> > completely rejiggering all the (possibly affected) queues when keys
> > change... might be very complex to do that, certainly a follow-up
> > patch if it's desired.
>
> Yeah, figured it was better to have something that's correct and then
> go back and change it if the performance hit turns out to be too
> high.
Makes sense.
> > This check seems a bit weird though - how could fast-xmit be set
> > without a TXQ station?
>
> I think that is probably just left over from before I introduced the
> control flag. Should be fine to remove it.
Ok.
> >
> > >
> > > +++ b/net/mac80211/util.c
> > > @@ -3393,11 +3393,18 @@ void ieee80211_txq_get_depth(struct
> > > ieee80211_txq *txq,
> > > unsigned long *byte_cnt)
> > > {
> > > struct txq_info *txqi = to_txq_info(txq);
> > > + u32 frag_cnt = 0, frag_bytes = 0;
> > > + struct sk_buff *skb;
> > > +
> > > + skb_queue_walk(&txqi->frags, skb) {
> > > + frag_cnt++;
> > > + frag_bytes += skb->len;
> > > + }
> >
> > I hope this is called infrequently :)
>
> Well, ath10k is the only user. It does get called on each
> wake_tx_queue, though, so not that infrequently. My reasoning was
> that since the frags queue is never going to have more than a fairly
> small number of packets in it (those produced from a single split
> packet), counting this way is acceptable instead of keeping a state
> variable up to date. Can change it if you disagree :)
No, I guess you're right, it can't be a long queue.
> Not sure if you want a v10, or if you're satisfied with the above
> comments and will just fix up the nits on merging?
>
I'll fix it up. Thanks!
johannes
^ permalink raw reply
* Re: [PATCH v9 2/2] mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue
From: Toke Høiland-Jørgensen @ 2016-09-30 12:45 UTC (permalink / raw)
To: Johannes Berg; +Cc: make-wifi-fast, linux-wireless
In-Reply-To: <1475239412.17481.59.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
>> Not sure if you want a v10, or if you're satisfied with the above
>> comments and will just fix up the nits on merging?
>>
>
> I'll fix it up. Thanks!
Cool, thanks :)
-Toke
^ permalink raw reply
* Re: [PATCH v9 2/2] mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue
From: Johannes Berg @ 2016-09-30 12:49 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, make-wifi-fast, linux-wireless
In-Reply-To: <20160922170420.5193-3-toke@toke.dk>
Applied, with the nits fixed as discussed.
Come to think of it, if somebody is bored ;-) perhaps a hwsim option to
use TXQs (should be optional I guess) would be nice so we can exercise
this code with the wpa_supplicant hwsim tests. That would have caught
the TKIP issues etc. pretty early on too, I think.
johannes
^ permalink raw reply
* Re: [PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices
From: Johannes Berg @ 2016-09-30 12:51 UTC (permalink / raw)
To: Toke Høiland-Jørgensen; +Cc: make-wifi-fast, linux-wireless, netdev
In-Reply-To: <87twcx5zll.fsf@toke.dk>
> > I kinda see the logic here - we really don't need to queue as much
> > if we can't possibly transmit it out quickly - but it seems to me
> > we should also throw in some kind of limit that's relative to the
> > amount of memory you have on the system?
>
> Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently
> carries a patch for that which just changes the hard-coded default to
> another hard-coded default. Not sure how to get a good value to use,
> though; and deciding on how large a fraction of memory to use for
> packets starts smelling an awful lot like setting policy in the
> kernel, doesn't it?
Yeah, I agree it does seem awkward.
Perhaps we should instead pick a low limit and let users change it more
easily (i.e. not debugfs)? I don't know a good answer to this either.
johannes
^ permalink raw reply
* Re: [PATCH 1/3] cw1200: Don't leak memory if krealloc failes
From: Sergei Shtylyov @ 2016-09-30 12:56 UTC (permalink / raw)
To: Johannes Thumshirn, Solomon Peachy, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <1475237495-15030-1-git-send-email-jthumshirn@suse.de>
Hello.
On 9/30/2016 3:11 PM, Johannes Thumshirn wrote:
> The call to krealloc() in wsm_buf_reserve() directly assigns the newly
> returned memory to buf->begin. This is all fine except when krealloc()
> failes we loose the ability to free the old memory pointed to by
Fails.
> buf->begin. If we just create a temporary variable to assign memory to
> and assign the memory to it we can mitigate the memory leak.
>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
> drivers/net/wireless/st/cw1200/wsm.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/st/cw1200/wsm.c b/drivers/net/wireless/st/cw1200/wsm.c
> index 680d60e..12fad99 100644
> --- a/drivers/net/wireless/st/cw1200/wsm.c
> +++ b/drivers/net/wireless/st/cw1200/wsm.c
> @@ -1807,16 +1807,18 @@ static int wsm_buf_reserve(struct wsm_buf *buf, size_t extra_size)
> {
> size_t pos = buf->data - buf->begin;
> size_t size = pos + extra_size;
> + u8 *tmp;
>
> size = round_up(size, FWLOAD_BLOCK_SIZE);
>
> - buf->begin = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> - if (buf->begin) {
> - buf->data = &buf->begin[pos];
> - buf->end = &buf->begin[size];
> - return 0;
> - } else {
> - buf->end = buf->data = buf->begin;
> + tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> + if (tmp) {
!tmp, you mean?
> + wsm_buf_deinit(buf);
> return -ENOMEM;
> }
> +
> + buf->begin = tmp;
> + buf->data = &buf->begin[pos];
> + buf->end = &buf->begin[size];
> + return 0;
> }
MBR, Sergei
^ permalink raw reply
* Re: [PATCH] ath10k: Fix spinlock use in coverage class hack
From: Valo, Kalle @ 2016-09-30 12:58 UTC (permalink / raw)
To: Benjamin Berg
Cc: Simon Wunderlich, Thiagarajan, Vasanthakumar,
linux-wireless@vger.kernel.org, Sebastian Gottschall,
ath10k @ lists . infradead . org, michal.kazior@tieto.com,
Mathias Kretschmer
In-Reply-To: <20160914163231.20863-1-benjamin@sipsolutions.net>
Benjamin Berg <benjamin@sipsolutions.net> writes:
> ath10k_hw_qca988x_set_coverage_class needs to hold both conf_mutex and
> the data_lock spin lock for parts of the function. However, data_lock
> is only needed while storing the coverage_class to store the value that
> the card is configured to.
>
> Fix the locking issue by only holding data_lock for the required duration=
.
>
> Signed-off-by: Benjamin Berg <benjamin@sipsolutions.net>
Thanks, I also folded this with the patch in the pending branch.
> And yes, I fully agree with your points of it being rather fragile. But a=
s
> you said, it should be entirely safe if not used.
That's good.
> Obviously a firmware implementation would be preferential.
That's a shame as this feature is quite often requested. But if the
firmware ever starts supporting the featrue we can then remove this hack
from ath10k.
> This locking issue was pretty unnecessary. Lets see if any more issues sh=
ow
> up in a closer review.
I can't see the locking problem anymore so it seems to be fixed. I'll
fix some minor things and send v2. I'll also CC linux-wireless.
--=20
Kalle Valo=
^ permalink raw reply
* Re: [PATCH 1/3] cw1200: Don't leak memory if krealloc failes
From: Johannes Thumshirn @ 2016-09-30 13:00 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Solomon Peachy, Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <25a38254-f0e3-1f4e-de46-688d9fd1b736@cogentembedded.com>
On Fri, Sep 30, 2016 at 03:56:45PM +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 9/30/2016 3:11 PM, Johannes Thumshirn wrote:
>
> > The call to krealloc() in wsm_buf_reserve() directly assigns the newly
> > returned memory to buf->begin. This is all fine except when krealloc()
> > failes we loose the ability to free the old memory pointed to by
>
> Fails.
>
> > buf->begin. If we just create a temporary variable to assign memory to
> > and assign the memory to it we can mitigate the memory leak.
> >
> > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> > ---
> > drivers/net/wireless/st/cw1200/wsm.c | 16 +++++++++-------
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/wireless/st/cw1200/wsm.c b/drivers/net/wireless/st/cw1200/wsm.c
> > index 680d60e..12fad99 100644
> > --- a/drivers/net/wireless/st/cw1200/wsm.c
> > +++ b/drivers/net/wireless/st/cw1200/wsm.c
> > @@ -1807,16 +1807,18 @@ static int wsm_buf_reserve(struct wsm_buf *buf, size_t extra_size)
> > {
> > size_t pos = buf->data - buf->begin;
> > size_t size = pos + extra_size;
> > + u8 *tmp;
> >
> > size = round_up(size, FWLOAD_BLOCK_SIZE);
> >
> > - buf->begin = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> > - if (buf->begin) {
> > - buf->data = &buf->begin[pos];
> > - buf->end = &buf->begin[size];
> > - return 0;
> > - } else {
> > - buf->end = buf->data = buf->begin;
> > + tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> > + if (tmp) {
>
> !tmp, you mean?
Yes, I've already sent out a v2.
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply
* Re: [PATCH 2/2] Revert "staging: wilc1000: Replace kthread with workqueue for host interface"
From: Greg KH @ 2016-09-30 13:22 UTC (permalink / raw)
To: Aditya Shankar
Cc: ganesh.krishna, linux-wireless, devel, linux-kernel,
Nicolas.Ferre
In-Reply-To: <20160930101318.GA24973@aditya-ubuntu.local>
On Fri, Sep 30, 2016 at 03:43:18PM +0530, Aditya Shankar wrote:
> This reverts commit 2518ac59eb27 ("staging: wilc1000: Replace kthread
> with workqueue for host interface")
>
> This commit breaks wilc1000 driver init. A crash was seen
> everytime the wlan interface was brought up and wilc device
> open was attempted. This change is being reverted until we
> figure out the problem in this change. The driver is
> usable now with this change reverted.
>
> Signed-off-by: Aditya Shankar <Aditya.Shankar@microchip.com>
>
> Conflicts:
> drivers/staging/wilc1000/host_interface.c
What is this line doing here?
And shouldn't we add a cc: stable tag as well? Or at the least, put a
"fixes:" tag to let people know exactly what commit it is fixing (the
id that it is reverting.)
thanks,
greg k-h
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2016-09-29
From: Aaron Conole @ 2016-09-30 13:30 UTC (permalink / raw)
To: David Miller, Pablo Neira Ayuso
Cc: kvalo, linux-wireless, netdev, linux-kernel
In-Reply-To: <20160930.013245.1474369794552343595.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Kalle Valo <kvalo@codeaurora.org>
> Date: Thu, 29 Sep 2016 19:57:28 +0300
>
...
>> Or actually I had one problem. While doing a test merge I noticed that
>> net-next fails to compile for me, but I don't think this is anything
>> wireless related:
>>
>> CC net/netfilter/core.o
>> net/netfilter/core.c: In function 'nf_set_hooks_head':
>> net/netfilter/core.c:96:149: error: 'struct net_device' has no
>> member named 'nf_hooks_ingress'
>
> Yes, I am aware of this build issue and will tackle it myself if someone
> doesn't beat me to it.
Sorry, I introduced this. I posted a series targetted at nf-next to
solve this, but it could be merged to net-next instead, if that makes
sense.
The patches are here:
https://patchwork.ozlabs.org/patch/676287/
https://patchwork.ozlabs.org/patch/676288/
Again, sorry for this.
^ permalink raw reply
* [PATCH v2] ath10k: allow setting coverage class
From: Kalle Valo @ 2016-09-30 13:38 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Benjamin Berg, Simon Wunderlich,
Mathias Kretschmer
From: Benjamin Berg <benjamin@sipsolutions.net>
Unfortunately ath10k does not generally allow modifying the coverage class
with the stock firmware and Qualcomm has so far refused to implement this
feature so that it can be properly supported in ath10k. If we however know
the registers that need to be modified for proper operation with a higher
coverage class, then we can do these modifications from the driver.
This is a hack and might cause subtle problems but as it's not enabled by
default (only when user space changes the coverage class explicitly) it should
not cause new problems for existing setups. But still this should be considered
as an experimental feature and used with caution.
This patch implements the support for first generation cards (QCA9880, QCA9887
and so on) which are based on a core that is similar to ath9k. The registers
are modified in place and need to be re-written every time the firmware sets
them. To achieve this the register status is verified after certain WMI events
from the firmware.
The coverage class may not be modified temporarily right after the card
re-initializes the registers. This is for example the case during scanning.
Thanks to Sebastian Gottschall <s.gottschall@dd-wrt.com> for initially
working on a userspace support for this. This patch wouldn't have been
possible without this documentation.
Signed-off-by: Benjamin Berg <benjamin@sipsolutions.net>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fit.fraunhofer.de>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
v2:
* cc linux-wireless
* fold "ath10k: Fix spinlock use in coverage class hack" into this one (Benjamin)
* emphasise more on the commit log that this is a hack
* remove unnecessary use of unlikely(), this is not in hotpath
* workaround ifdef CONFIG_ATH10K_DEBUGFS with adding
ath10k_debug_get_fw_dbglog_[mask|level]()
drivers/net/wireless/ath/ath10k/core.c | 11 +++
drivers/net/wireless/ath/ath10k/core.h | 13 +++
drivers/net/wireless/ath/ath10k/debug.h | 22 +++++
drivers/net/wireless/ath/ath10k/hw.c | 142 +++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/hw.h | 28 +++++-
drivers/net/wireless/ath/ath10k/mac.c | 19 +++++
drivers/net/wireless/ath/ath10k/wmi.c | 48 +++++++++++
7 files changed, 282 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 867a8403b15b..7005e2a98726 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1562,6 +1562,15 @@ static void ath10k_core_restart(struct work_struct *work)
mutex_unlock(&ar->conf_mutex);
}
+static void ath10k_core_set_coverage_class_work(struct work_struct *work)
+{
+ struct ath10k *ar = container_of(work, struct ath10k,
+ set_coverage_class_work);
+
+ if (ar->hw_params.hw_ops->set_coverage_class)
+ ar->hw_params.hw_ops->set_coverage_class(ar, -1);
+}
+
static int ath10k_core_init_firmware_features(struct ath10k *ar)
{
struct ath10k_fw_file *fw_file = &ar->normal_mode_fw.fw_file;
@@ -2344,6 +2353,8 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
INIT_WORK(&ar->register_work, ath10k_core_register_work);
INIT_WORK(&ar->restart_work, ath10k_core_restart);
+ INIT_WORK(&ar->set_coverage_class_work,
+ ath10k_core_set_coverage_class_work);
init_dummy_netdev(&ar->napi_dev);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index c091306bca33..6e5aa2d09699 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -912,6 +912,19 @@ struct ath10k {
struct net_device napi_dev;
struct napi_struct napi;
+ struct work_struct set_coverage_class_work;
+ /* protected by conf_mutex */
+ struct {
+ /* writing also protected by data_lock */
+ s16 coverage_class;
+
+ u32 reg_phyclk;
+ u32 reg_slottime_conf;
+ u32 reg_slottime_orig;
+ u32 reg_ack_cts_timeout_conf;
+ u32 reg_ack_cts_timeout_orig;
+ } fw_coverage;
+
/* must be last */
u8 drv_priv[0] __aligned(sizeof(void *));
};
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index c458fa96a6d4..335512b11ca2 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -94,7 +94,19 @@ int ath10k_debug_get_et_sset_count(struct ieee80211_hw *hw,
void ath10k_debug_get_et_stats(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ethtool_stats *stats, u64 *data);
+
+static inline u64 ath10k_debug_get_fw_dbglog_mask(struct ath10k *ar)
+{
+ return ar->debug.fw_dbglog_mask;
+}
+
+static inline u32 ath10k_debug_get_fw_dbglog_level(struct ath10k *ar)
+{
+ return ar->debug.fw_dbglog_level;
+}
+
#else
+
static inline int ath10k_debug_start(struct ath10k *ar)
{
return 0;
@@ -144,6 +156,16 @@ ath10k_debug_get_new_fw_crash_data(struct ath10k *ar)
return NULL;
}
+static inline u64 ath10k_debug_get_fw_dbglog_mask(struct ath10k *ar)
+{
+ return 0;
+}
+
+static inline u32 ath10k_debug_get_fw_dbglog_level(struct ath10k *ar)
+{
+ return 0;
+}
+
#define ATH10K_DFS_STAT_INC(ar, c) do { } while (0)
#define ath10k_debug_get_et_strings NULL
diff --git a/drivers/net/wireless/ath/ath10k/hw.c b/drivers/net/wireless/ath/ath10k/hw.c
index 675e75d66db2..33fb26833cd0 100644
--- a/drivers/net/wireless/ath/ath10k/hw.c
+++ b/drivers/net/wireless/ath/ath10k/hw.c
@@ -17,11 +17,14 @@
#include <linux/types.h>
#include "core.h"
#include "hw.h"
+#include "hif.h"
+#include "wmi-ops.h"
const struct ath10k_hw_regs qca988x_regs = {
.rtc_soc_base_address = 0x00004000,
.rtc_wmac_base_address = 0x00005000,
.soc_core_base_address = 0x00009000,
+ .wlan_mac_base_address = 0x00020000,
.ce_wrapper_base_address = 0x00057000,
.ce0_base_address = 0x00057400,
.ce1_base_address = 0x00057800,
@@ -48,6 +51,7 @@ const struct ath10k_hw_regs qca6174_regs = {
.rtc_soc_base_address = 0x00000800,
.rtc_wmac_base_address = 0x00001000,
.soc_core_base_address = 0x0003a000,
+ .wlan_mac_base_address = 0x00020000,
.ce_wrapper_base_address = 0x00034000,
.ce0_base_address = 0x00034400,
.ce1_base_address = 0x00034800,
@@ -74,6 +78,7 @@ const struct ath10k_hw_regs qca99x0_regs = {
.rtc_soc_base_address = 0x00080000,
.rtc_wmac_base_address = 0x00000000,
.soc_core_base_address = 0x00082000,
+ .wlan_mac_base_address = 0x00030000,
.ce_wrapper_base_address = 0x0004d000,
.ce0_base_address = 0x0004a000,
.ce1_base_address = 0x0004a400,
@@ -109,6 +114,7 @@ const struct ath10k_hw_regs qca99x0_regs = {
const struct ath10k_hw_regs qca4019_regs = {
.rtc_soc_base_address = 0x00080000,
.soc_core_base_address = 0x00082000,
+ .wlan_mac_base_address = 0x00030000,
.ce_wrapper_base_address = 0x0004d000,
.ce0_base_address = 0x0004a000,
.ce1_base_address = 0x0004a400,
@@ -220,7 +226,143 @@ void ath10k_hw_fill_survey_time(struct ath10k *ar, struct survey_info *survey,
survey->time_busy = CCNT_TO_MSEC(ar, rcc);
}
+/* The firmware does not support setting the coverage class. Instead this
+ * function monitors and modifies the corresponding MAC registers.
+ */
+static void ath10k_hw_qca988x_set_coverage_class(struct ath10k *ar,
+ s16 value)
+{
+ u32 slottime_reg;
+ u32 slottime;
+ u32 timeout_reg;
+ u32 ack_timeout;
+ u32 cts_timeout;
+ u32 phyclk_reg;
+ u32 phyclk;
+ u64 fw_dbglog_mask;
+ u32 fw_dbglog_level;
+
+ mutex_lock(&ar->conf_mutex);
+
+ /* Only modify registers if the core is started. */
+ if ((ar->state != ATH10K_STATE_ON) &&
+ (ar->state != ATH10K_STATE_RESTARTED))
+ goto unlock;
+
+ /* Retrieve the current values of the two registers that need to be
+ * adjusted.
+ */
+ slottime_reg = ath10k_hif_read32(ar, WLAN_MAC_BASE_ADDRESS +
+ WAVE1_PCU_GBL_IFS_SLOT);
+ timeout_reg = ath10k_hif_read32(ar, WLAN_MAC_BASE_ADDRESS +
+ WAVE1_PCU_ACK_CTS_TIMEOUT);
+ phyclk_reg = ath10k_hif_read32(ar, WLAN_MAC_BASE_ADDRESS +
+ WAVE1_PHYCLK);
+ phyclk = MS(phyclk_reg, WAVE1_PHYCLK_USEC) + 1;
+
+ if (value < 0)
+ value = ar->fw_coverage.coverage_class;
+
+ /* Break out if the coverage class and registers have the expected
+ * value.
+ */
+ if (value == ar->fw_coverage.coverage_class &&
+ slottime_reg == ar->fw_coverage.reg_slottime_conf &&
+ timeout_reg == ar->fw_coverage.reg_ack_cts_timeout_conf &&
+ phyclk_reg == ar->fw_coverage.reg_phyclk)
+ goto unlock;
+
+ /* Store new initial register values from the firmware. */
+ if (slottime_reg != ar->fw_coverage.reg_slottime_conf)
+ ar->fw_coverage.reg_slottime_orig = slottime_reg;
+ if (timeout_reg != ar->fw_coverage.reg_ack_cts_timeout_conf)
+ ar->fw_coverage.reg_ack_cts_timeout_orig = timeout_reg;
+ ar->fw_coverage.reg_phyclk = phyclk_reg;
+
+ /* Calculat new value based on the (original) firmware calculation. */
+ slottime_reg = ar->fw_coverage.reg_slottime_orig;
+ timeout_reg = ar->fw_coverage.reg_ack_cts_timeout_orig;
+
+ /* Do some sanity checks on the slottime register. */
+ if (slottime_reg % phyclk) {
+ ath10k_warn(ar,
+ "failed to set coverage class: expected integer microsecond value in register\n");
+
+ goto store_regs;
+ }
+
+ slottime = MS(slottime_reg, WAVE1_PCU_GBL_IFS_SLOT);
+ slottime = slottime / phyclk;
+ if (slottime != 9 && slottime != 20) {
+ ath10k_warn(ar,
+ "failed to set coverage class: expected slot time of 9 or 20us in HW register. It is %uus.\n",
+ slottime);
+
+ goto store_regs;
+ }
+
+ /* Recalculate the register values by adding the additional propagation
+ * delay (3us per coverage class).
+ */
+
+ slottime = MS(slottime_reg, WAVE1_PCU_GBL_IFS_SLOT);
+ slottime += value * 3 * phyclk;
+ slottime = min_t(u32, slottime, WAVE1_PCU_GBL_IFS_SLOT_MAX);
+ slottime = SM(slottime, WAVE1_PCU_GBL_IFS_SLOT);
+ slottime_reg = (slottime_reg & ~WAVE1_PCU_GBL_IFS_SLOT_MASK) | slottime;
+
+ /* Update ack timeout (lower halfword). */
+ ack_timeout = MS(timeout_reg, WAVE1_PCU_ACK_CTS_TIMEOUT_ACK);
+ ack_timeout += 3 * value * phyclk;
+ ack_timeout = min_t(u32, ack_timeout, WAVE1_PCU_ACK_CTS_TIMEOUT_MAX);
+ ack_timeout = SM(ack_timeout, WAVE1_PCU_ACK_CTS_TIMEOUT_ACK);
+
+ /* Update cts timeout (upper halfword). */
+ cts_timeout = MS(timeout_reg, WAVE1_PCU_ACK_CTS_TIMEOUT_CTS);
+ cts_timeout += 3 * value * phyclk;
+ cts_timeout = min_t(u32, cts_timeout, WAVE1_PCU_ACK_CTS_TIMEOUT_MAX);
+ cts_timeout = SM(cts_timeout, WAVE1_PCU_ACK_CTS_TIMEOUT_CTS);
+
+ timeout_reg = ack_timeout | cts_timeout;
+
+ ath10k_hif_write32(ar,
+ WLAN_MAC_BASE_ADDRESS + WAVE1_PCU_GBL_IFS_SLOT,
+ slottime_reg);
+ ath10k_hif_write32(ar,
+ WLAN_MAC_BASE_ADDRESS + WAVE1_PCU_ACK_CTS_TIMEOUT,
+ timeout_reg);
+
+ /* Ensure we have a debug level of WARN set for the case that the
+ * coverage class is larger than 0. This is important as we need to
+ * set the registers again if the firmware does an internal reset and
+ * this way we will be notified of the event.
+ */
+ fw_dbglog_mask = ath10k_debug_get_fw_dbglog_mask(ar);
+ fw_dbglog_level = ath10k_debug_get_fw_dbglog_level(ar);
+
+ if (value > 0) {
+ if (fw_dbglog_level > ATH10K_DBGLOG_LEVEL_WARN)
+ fw_dbglog_level = ATH10K_DBGLOG_LEVEL_WARN;
+ fw_dbglog_mask = ~0;
+ }
+
+ ath10k_wmi_dbglog_cfg(ar, fw_dbglog_mask, fw_dbglog_level);
+
+store_regs:
+ /* After an error we will not retry setting the coverage class. */
+ spin_lock_bh(&ar->data_lock);
+ ar->fw_coverage.coverage_class = value;
+ spin_unlock_bh(&ar->data_lock);
+
+ ar->fw_coverage.reg_slottime_conf = slottime_reg;
+ ar->fw_coverage.reg_ack_cts_timeout_conf = timeout_reg;
+
+unlock:
+ mutex_unlock(&ar->conf_mutex);
+}
+
const struct ath10k_hw_ops qca988x_ops = {
+ .set_coverage_class = ath10k_hw_qca988x_set_coverage_class,
};
static int ath10k_qca99x0_rx_desc_get_l3_pad_bytes(struct htt_rx_desc *rxd)
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 6038b7486f1d..883547f3347c 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -230,6 +230,7 @@ struct ath10k_hw_regs {
u32 rtc_soc_base_address;
u32 rtc_wmac_base_address;
u32 soc_core_base_address;
+ u32 wlan_mac_base_address;
u32 ce_wrapper_base_address;
u32 ce0_base_address;
u32 ce1_base_address;
@@ -418,6 +419,7 @@ struct htt_rx_desc;
/* Defines needed for Rx descriptor abstraction */
struct ath10k_hw_ops {
int (*rx_desc_get_l3_pad_bytes)(struct htt_rx_desc *rxd);
+ void (*set_coverage_class)(struct ath10k *ar, s16 value);
};
extern const struct ath10k_hw_ops qca988x_ops;
@@ -614,7 +616,7 @@ ath10k_rx_desc_get_l3_pad_bytes(struct ath10k_hw_params *hw,
#define WLAN_SI_BASE_ADDRESS 0x00010000
#define WLAN_GPIO_BASE_ADDRESS 0x00014000
#define WLAN_ANALOG_INTF_BASE_ADDRESS 0x0001c000
-#define WLAN_MAC_BASE_ADDRESS 0x00020000
+#define WLAN_MAC_BASE_ADDRESS ar->regs->wlan_mac_base_address
#define EFUSE_BASE_ADDRESS 0x00030000
#define FPGA_REG_BASE_ADDRESS 0x00039000
#define WLAN_UART2_BASE_ADDRESS 0x00054c00
@@ -814,4 +816,28 @@ ath10k_rx_desc_get_l3_pad_bytes(struct ath10k_hw_params *hw,
#define RTC_STATE_V_GET(x) (((x) & RTC_STATE_V_MASK) >> RTC_STATE_V_LSB)
+/* Register definitions for first generation ath10k cards. These cards include
+ * a mac thich has a register allocation similar to ath9k and at least some
+ * registers including the ones relevant for modifying the coverage class are
+ * identical to the ath9k definitions.
+ * These registers are usually managed by the ath10k firmware. However by
+ * overriding them it is possible to support coverage class modifications.
+ */
+#define WAVE1_PCU_ACK_CTS_TIMEOUT 0x8014
+#define WAVE1_PCU_ACK_CTS_TIMEOUT_MAX 0x00003FFF
+#define WAVE1_PCU_ACK_CTS_TIMEOUT_ACK_MASK 0x00003FFF
+#define WAVE1_PCU_ACK_CTS_TIMEOUT_ACK_LSB 0
+#define WAVE1_PCU_ACK_CTS_TIMEOUT_CTS_MASK 0x3FFF0000
+#define WAVE1_PCU_ACK_CTS_TIMEOUT_CTS_LSB 16
+
+#define WAVE1_PCU_GBL_IFS_SLOT 0x1070
+#define WAVE1_PCU_GBL_IFS_SLOT_MASK 0x0000FFFF
+#define WAVE1_PCU_GBL_IFS_SLOT_MAX 0x0000FFFF
+#define WAVE1_PCU_GBL_IFS_SLOT_LSB 0
+#define WAVE1_PCU_GBL_IFS_SLOT_RESV0 0xFFFF0000
+
+#define WAVE1_PHYCLK 0x801C
+#define WAVE1_PHYCLK_USEC_MASK 0x0000007F
+#define WAVE1_PHYCLK_USEC_LSB 0
+
#endif /* _HW_H_ */
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 837384e4422b..0f3d4cf7dc00 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -5411,6 +5411,20 @@ static void ath10k_bss_info_changed(struct ieee80211_hw *hw,
mutex_unlock(&ar->conf_mutex);
}
+static void ath10k_mac_op_set_coverage_class(struct ieee80211_hw *hw, s16 value)
+{
+ struct ath10k *ar = hw->priv;
+
+ /* This function should never be called if setting the coverage class
+ * is not supported on this hardware.
+ */
+ if (!ar->hw_params.hw_ops->set_coverage_class) {
+ WARN_ON_ONCE(1);
+ return;
+ }
+ ar->hw_params.hw_ops->set_coverage_class(ar, value);
+}
+
static int ath10k_hw_scan(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_scan_request *hw_req)
@@ -7436,6 +7450,7 @@ static const struct ieee80211_ops ath10k_ops = {
.remove_interface = ath10k_remove_interface,
.configure_filter = ath10k_configure_filter,
.bss_info_changed = ath10k_bss_info_changed,
+ .set_coverage_class = ath10k_mac_op_set_coverage_class,
.hw_scan = ath10k_hw_scan,
.cancel_hw_scan = ath10k_cancel_hw_scan,
.set_key = ath10k_set_key,
@@ -8120,6 +8135,10 @@ int ath10k_mac_register(struct ath10k *ar)
goto err_dfs_detector_exit;
}
+ /* Disable set_coverage_class for chipsets that do not support it. */
+ if (!ar->hw_params.hw_ops->set_coverage_class)
+ ar->ops->set_coverage_class = NULL;
+
ret = ath_regd_init(&ar->ath_common.regulatory, ar->hw->wiphy,
ath10k_reg_notifier);
if (ret) {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index b4cfaf9aa0ac..387c4eede388 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -4931,6 +4931,23 @@ exit:
return 0;
}
+static inline void ath10k_wmi_queue_set_coverage_class_work(struct ath10k *ar)
+{
+ if (ar->hw_params.hw_ops->set_coverage_class) {
+ spin_lock_bh(&ar->data_lock);
+
+ /* This call only ensures that the modified coverage class
+ * persists in case the firmware sets the registers back to
+ * their default value. So calling it is only necessary if the
+ * coverage class has a non-zero value.
+ */
+ if (ar->fw_coverage.coverage_class)
+ queue_work(ar->workqueue, &ar->set_coverage_class_work);
+
+ spin_unlock_bh(&ar->data_lock);
+ }
+}
+
static void ath10k_wmi_op_rx(struct ath10k *ar, struct sk_buff *skb)
{
struct wmi_cmd_hdr *cmd_hdr;
@@ -4951,6 +4968,7 @@ static void ath10k_wmi_op_rx(struct ath10k *ar, struct sk_buff *skb)
return;
case WMI_SCAN_EVENTID:
ath10k_wmi_event_scan(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_CHAN_INFO_EVENTID:
ath10k_wmi_event_chan_info(ar, skb);
@@ -4960,15 +4978,18 @@ static void ath10k_wmi_op_rx(struct ath10k *ar, struct sk_buff *skb)
break;
case WMI_DEBUG_MESG_EVENTID:
ath10k_wmi_event_debug_mesg(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_UPDATE_STATS_EVENTID:
ath10k_wmi_event_update_stats(ar, skb);
break;
case WMI_VDEV_START_RESP_EVENTID:
ath10k_wmi_event_vdev_start_resp(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_VDEV_STOPPED_EVENTID:
ath10k_wmi_event_vdev_stopped(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_PEER_STA_KICKOUT_EVENTID:
ath10k_wmi_event_peer_sta_kickout(ar, skb);
@@ -4984,12 +5005,14 @@ static void ath10k_wmi_op_rx(struct ath10k *ar, struct sk_buff *skb)
break;
case WMI_ROAM_EVENTID:
ath10k_wmi_event_roam(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_PROFILE_MATCH:
ath10k_wmi_event_profile_match(ar, skb);
break;
case WMI_DEBUG_PRINT_EVENTID:
ath10k_wmi_event_debug_print(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_PDEV_QVIT_EVENTID:
ath10k_wmi_event_pdev_qvit(ar, skb);
@@ -5038,6 +5061,7 @@ static void ath10k_wmi_op_rx(struct ath10k *ar, struct sk_buff *skb)
return;
case WMI_READY_EVENTID:
ath10k_wmi_event_ready(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
default:
ath10k_warn(ar, "Unknown eventid: %d\n", id);
@@ -5081,6 +5105,7 @@ static void ath10k_wmi_10_1_op_rx(struct ath10k *ar, struct sk_buff *skb)
return;
case WMI_10X_SCAN_EVENTID:
ath10k_wmi_event_scan(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10X_CHAN_INFO_EVENTID:
ath10k_wmi_event_chan_info(ar, skb);
@@ -5090,15 +5115,18 @@ static void ath10k_wmi_10_1_op_rx(struct ath10k *ar, struct sk_buff *skb)
break;
case WMI_10X_DEBUG_MESG_EVENTID:
ath10k_wmi_event_debug_mesg(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10X_UPDATE_STATS_EVENTID:
ath10k_wmi_event_update_stats(ar, skb);
break;
case WMI_10X_VDEV_START_RESP_EVENTID:
ath10k_wmi_event_vdev_start_resp(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10X_VDEV_STOPPED_EVENTID:
ath10k_wmi_event_vdev_stopped(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10X_PEER_STA_KICKOUT_EVENTID:
ath10k_wmi_event_peer_sta_kickout(ar, skb);
@@ -5114,12 +5142,14 @@ static void ath10k_wmi_10_1_op_rx(struct ath10k *ar, struct sk_buff *skb)
break;
case WMI_10X_ROAM_EVENTID:
ath10k_wmi_event_roam(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10X_PROFILE_MATCH:
ath10k_wmi_event_profile_match(ar, skb);
break;
case WMI_10X_DEBUG_PRINT_EVENTID:
ath10k_wmi_event_debug_print(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10X_PDEV_QVIT_EVENTID:
ath10k_wmi_event_pdev_qvit(ar, skb);
@@ -5159,6 +5189,7 @@ static void ath10k_wmi_10_1_op_rx(struct ath10k *ar, struct sk_buff *skb)
return;
case WMI_10X_READY_EVENTID:
ath10k_wmi_event_ready(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10X_PDEV_UTF_EVENTID:
/* ignore utf events */
@@ -5205,6 +5236,7 @@ static void ath10k_wmi_10_2_op_rx(struct ath10k *ar, struct sk_buff *skb)
return;
case WMI_10_2_SCAN_EVENTID:
ath10k_wmi_event_scan(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_2_CHAN_INFO_EVENTID:
ath10k_wmi_event_chan_info(ar, skb);
@@ -5214,15 +5246,18 @@ static void ath10k_wmi_10_2_op_rx(struct ath10k *ar, struct sk_buff *skb)
break;
case WMI_10_2_DEBUG_MESG_EVENTID:
ath10k_wmi_event_debug_mesg(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_2_UPDATE_STATS_EVENTID:
ath10k_wmi_event_update_stats(ar, skb);
break;
case WMI_10_2_VDEV_START_RESP_EVENTID:
ath10k_wmi_event_vdev_start_resp(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_2_VDEV_STOPPED_EVENTID:
ath10k_wmi_event_vdev_stopped(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_2_PEER_STA_KICKOUT_EVENTID:
ath10k_wmi_event_peer_sta_kickout(ar, skb);
@@ -5238,12 +5273,14 @@ static void ath10k_wmi_10_2_op_rx(struct ath10k *ar, struct sk_buff *skb)
break;
case WMI_10_2_ROAM_EVENTID:
ath10k_wmi_event_roam(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_2_PROFILE_MATCH:
ath10k_wmi_event_profile_match(ar, skb);
break;
case WMI_10_2_DEBUG_PRINT_EVENTID:
ath10k_wmi_event_debug_print(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_2_PDEV_QVIT_EVENTID:
ath10k_wmi_event_pdev_qvit(ar, skb);
@@ -5274,15 +5311,18 @@ static void ath10k_wmi_10_2_op_rx(struct ath10k *ar, struct sk_buff *skb)
break;
case WMI_10_2_VDEV_STANDBY_REQ_EVENTID:
ath10k_wmi_event_vdev_standby_req(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_2_VDEV_RESUME_REQ_EVENTID:
ath10k_wmi_event_vdev_resume_req(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_2_SERVICE_READY_EVENTID:
ath10k_wmi_event_service_ready(ar, skb);
return;
case WMI_10_2_READY_EVENTID:
ath10k_wmi_event_ready(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_2_PDEV_TEMPERATURE_EVENTID:
ath10k_wmi_event_temperature(ar, skb);
@@ -5345,12 +5385,14 @@ static void ath10k_wmi_10_4_op_rx(struct ath10k *ar, struct sk_buff *skb)
break;
case WMI_10_4_DEBUG_MESG_EVENTID:
ath10k_wmi_event_debug_mesg(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_4_SERVICE_READY_EVENTID:
ath10k_wmi_event_service_ready(ar, skb);
return;
case WMI_10_4_SCAN_EVENTID:
ath10k_wmi_event_scan(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_4_CHAN_INFO_EVENTID:
ath10k_wmi_event_chan_info(ar, skb);
@@ -5360,12 +5402,14 @@ static void ath10k_wmi_10_4_op_rx(struct ath10k *ar, struct sk_buff *skb)
break;
case WMI_10_4_READY_EVENTID:
ath10k_wmi_event_ready(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_4_PEER_STA_KICKOUT_EVENTID:
ath10k_wmi_event_peer_sta_kickout(ar, skb);
break;
case WMI_10_4_ROAM_EVENTID:
ath10k_wmi_event_roam(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_4_HOST_SWBA_EVENTID:
ath10k_wmi_event_host_swba(ar, skb);
@@ -5375,12 +5419,15 @@ static void ath10k_wmi_10_4_op_rx(struct ath10k *ar, struct sk_buff *skb)
break;
case WMI_10_4_DEBUG_PRINT_EVENTID:
ath10k_wmi_event_debug_print(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_4_VDEV_START_RESP_EVENTID:
ath10k_wmi_event_vdev_start_resp(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_4_VDEV_STOPPED_EVENTID:
ath10k_wmi_event_vdev_stopped(ar, skb);
+ ath10k_wmi_queue_set_coverage_class_work(ar);
break;
case WMI_10_4_WOW_WAKEUP_HOST_EVENTID:
case WMI_10_4_PEER_RATECODE_LIST_EVENTID:
@@ -6099,6 +6146,7 @@ void ath10k_wmi_start_scan_init(struct ath10k *ar,
| WMI_SCAN_EVENT_COMPLETED
| WMI_SCAN_EVENT_BSS_CHANNEL
| WMI_SCAN_EVENT_FOREIGN_CHANNEL
+ | WMI_SCAN_EVENT_FOREIGN_CHANNEL_EXIT
| WMI_SCAN_EVENT_DEQUEUED;
arg->scan_ctrl_flags |= WMI_SCAN_CHAN_STAT_EVENT;
arg->n_bssids = 1;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v9 2/2] mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue
From: Toke Høiland-Jørgensen @ 2016-09-30 14:01 UTC (permalink / raw)
To: Johannes Berg; +Cc: make-wifi-fast, linux-wireless
In-Reply-To: <1475239765.17481.61.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> Applied, with the nits fixed as discussed.
Awesome, thanks!
> Come to think of it, if somebody is bored ;-) perhaps a hwsim option
> to use TXQs (should be optional I guess) would be nice so we can
> exercise this code with the wpa_supplicant hwsim tests. That would
> have caught the TKIP issues etc. pretty early on too, I think.
Noted. I'll look into that the next time I'm bored ;)
-Toke
^ permalink raw reply
* Re: ath10k stuck on 6Mbps and spam syslog
From: Ben Greear @ 2016-09-30 14:22 UTC (permalink / raw)
To: Matteo Grandi; +Cc: LinuxWireless Mailing List
In-Reply-To: <CAHdg3xa8g3hGhC2SdrnYNAwVtGT+kSuQGOm+WO4PO6Wa_V3p3Q@mail.gmail.com>
At least some stock ath10k firmware can send frames at CCK (.11b) encodings
on 5Ghz, which is out-of-spec, and may cause this warning. Maybe you
could check the values printed and see if that is indeed what is causing
this splat?
Thanks,
Ben
On 09/30/2016 12:23 AM, Matteo Grandi wrote:
> Dear Ben,
> Thank you for your reply.
> The fact that stock ath10k firmware does not report tx-rate explains
> why the station dump shows 6Mbps but the iperf test reach 14.6 (even
> if it is really under the expected data rate...).
> About the syslog spam, this is the code of the function that seems to
> cause the error (I put a comment at line 2621 side). (The warning
> start to spam as soon as I assign the channel "iw dev <if_name> set
> channel <ch_number>)
>
> /**
> * ieee80211_calculate_rx_timestamp - calculate timestamp in frame
> * @local: mac80211 hw info struct
> * @status: RX status
> * @mpdu_len: total MPDU length (including FCS)
> * @mpdu_offset: offset into MPDU to calculate timestamp at
> *
> * This function calculates the RX timestamp at the given MPDU offset, taking
> * into account what the RX timestamp was. An offset of 0 will just normalize
> * the timestamp to TSF at beginning of MPDU reception.
> */
> u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
> struct ieee80211_rx_status *status,
> unsigned int mpdu_len,
> unsigned int mpdu_offset)
> {
> u64 ts = status->mactime;
> struct rate_info ri;
> u16 rate;
>
> if (WARN_ON(!ieee80211_have_rx_timestamp(status))) //this is the
> line 2621 reported in the warning
> return 0;
>
> memset(&ri, 0, sizeof(ri));
>
> /* Fill cfg80211 rate info */
> if (status->flag & RX_FLAG_HT) {
> ri.mcs = status->rate_idx;
> ri.flags |= RATE_INFO_FLAGS_MCS;
> if (status->flag & RX_FLAG_40MHZ)
> ri.bw = RATE_INFO_BW_40;
> else
> ri.bw = RATE_INFO_BW_20;
> if (status->flag & RX_FLAG_SHORT_GI)
> ri.flags |= RATE_INFO_FLAGS_SHORT_GI;
> } else if (status->flag & RX_FLAG_VHT) {
> ri.flags |= RATE_INFO_FLAGS_VHT_MCS;
> ri.mcs = status->rate_idx;
> ri.nss = status->vht_nss;
> if (status->flag & RX_FLAG_40MHZ)
> ri.bw = RATE_INFO_BW_40;
> else if (status->vht_flag & RX_VHT_FLAG_80MHZ)
> ri.bw = RATE_INFO_BW_80;
> else if (status->vht_flag & RX_VHT_FLAG_160MHZ)
> ri.bw = RATE_INFO_BW_160;
> else
> ri.bw = RATE_INFO_BW_20;
> if (status->flag & RX_FLAG_SHORT_GI)
> ri.flags |= RATE_INFO_FLAGS_SHORT_GI;
> } else {
> struct ieee80211_supported_band *sband;
> int shift = 0;
> int bitrate;
>
> if (status->flag & RX_FLAG_10MHZ) {
> shift = 1;
> ri.bw = RATE_INFO_BW_10;
> } else if (status->flag & RX_FLAG_5MHZ) {
> shift = 2;
> ri.bw = RATE_INFO_BW_5;
> } else {
> ri.bw = RATE_INFO_BW_20;
> }
>
> sband = local->hw.wiphy->bands[status->band];
> bitrate = sband->bitrates[status->rate_idx].bitrate;
> ri.legacy = DIV_ROUND_UP(bitrate, (1 << shift));
> }
>
> rate = cfg80211_calculate_bitrate(&ri);
> if (WARN_ONCE(!rate,
> "Invalid bitrate: flags=0x%x, idx=%d, vht_nss=%d\n",
> status->flag, status->rate_idx, status->vht_nss))
> return 0;
>
> /* rewind from end of MPDU */
> if (status->flag & RX_FLAG_MACTIME_END)
> ts -= mpdu_len * 8 * 10 / rate;
>
> ts += mpdu_offset * 8 * 10 / rate;
> /* [SESAME] I2CAT. dbg*/
> //printk(KERN_DEBUG "calculate_rx_timestamp: ts = %lu;\t rate =
> %lu;\tmpdu_offset = %lu;\tmpdu_len = %lu\n",
> // (long unsigned int) ts, (long unsigned int) rate, (long unsigned
> int) mpdu_offset, (long unsigned int) mpdu_len );
>
> return ts;
> }
>
> Thank you so much!
>
> All the best
>
> Matteo
>
>
> 2016-09-29 16:47 GMT+02:00 Ben Greear <greearb@candelatech.com>:
>> stock ath10k firmware does not report tx-rate so the kernel always sees
>> 6Mbps.
>>
>> I don't know about the splat..maybe post the function
>> that is causing that?
>>
>> /home/matteo/linux-imx6/backports4.4.2-i2CAT/net/mac80211/util.c:2621
>>
>> Thanks,
>> Ben
>>
>>
>> On 09/29/2016 06:39 AM, Matteo Grandi wrote:
>>>
>>> Hello all,
>>>
>>> I'm struggling with a problem related on ath10k drivers:
>>> I'm using a Compex WLE600V5-27 (802.11ac) miniPCIe card for some HT
>>> tests needed for my thesis.
>>> I'm using ath10k drivers for this card, and backports-4.4.2, in
>>> particular the firmware-5.bin_10.2.4.70.54 because it seem to be the
>>> more recent.
>>> I've connected in mesh mode the WLE600V5 card with an 802.11n card
>>> (using ath9k drivers) but looking at the station dump, the tx bitrate
>>> is stuck on 6.0 Mbit/s for the ath10k. The ath9k one works well and
>>> watch -n1 iw <interfacename> station dump
>>> let me see changes of the tx rate and MCS on the ath9k during iperf
>>> tests, but the ath10k stucks on 6.0 Mbit/s.
>>>
>>> Then something misterious (for me) happen while the channel
>>> assignment: the syslog is spammed by this message:
>>>
>>> [17554.919459] ------------[ cut here ]------------
>>> [17554.919839] WARNING: CPU: 0 PID: 0 at
>>> /home/matteo/linux-imx6/backports4.4.2-i2CAT/net/mac80211/util.c:2621
>>> ieee80211_calculate_rx_timestamp+0x204/0x278 [mac80211]()
>>> [17554.919855] Modules linked in: arc4 sky2 ath10k_pci(O)
>>> ath10k_core(O) ath(O) mac80211(O) cfg80211(O) compat(O)
>>> [17554.919926] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W O
>>> 3.14.48-g408ccb9 #4
>>> [17554.919990] [<80015050>] (unwind_backtrace) from [<80011330>]
>>> (show_stack+0x10/0x14)
>>> [17554.920038] [<80011330>] (show_stack) from [<806537dc>]
>>> (dump_stack+0x80/0x90)
>>> [17554.920074] [<806537dc>] (dump_stack) from [<8002c578>]
>>> (warn_slowpath_common+0x6c/0x88)
>>> [17554.920103] [<8002c578>] (warn_slowpath_common) from [<8002c630>]
>>> (warn_slowpath_null+0x1c/0x24)
>>> [17554.920377] [<8002c630>] (warn_slowpath_null) from [<7f089c74>]
>>> (ieee80211_calculate_rx_timestamp+0x204/0x278 [mac80211])
>>> [17554.920826] [<7f089c74>] (ieee80211_calculate_rx_timestamp
>>> [mac80211]) from [<7f07d724>] (ieee80211_rx_napi+0xcc/0x8d4
>>> [mac80211])
>>> [17554.921259] [<7f07d724>] (ieee80211_rx_napi [mac80211]) from
>>> [<7f117fd0>] (ath10k_wmi_event_mgmt_rx+0x1f4/0x35c [ath10k_core])
>>> [17554.921540] [<7f117fd0>] (ath10k_wmi_event_mgmt_rx [ath10k_core])
>>> from [<7f10d844>] (ath10k_htc_rx_completion_handler+0x1cc/0x464
>>> [ath10k_core])
>>> [17554.921706] [<7f10d844>] (ath10k_htc_rx_completion_handler
>>> [ath10k_core]) from [<7f157b4c>] (ath10k_pci_process_rx_cb+0x1ac/0x1fc
>>> [ath10k_pci])
>>> [17554.921773] [<7f157b4c>] (ath10k_pci_process_rx_cb [ath10k_pci])
>>> from [<7f15b3ac>] (ath10k_ce_per_engine_service+0x5c/0x94
>>> [ath10k_pci])
>>> [17554.921835] [<7f15b3ac>] (ath10k_ce_per_engine_service
>>> [ath10k_pci]) from [<7f15b464>]
>>> (ath10k_ce_per_engine_service_any+0x80/0x88 [ath10k_pci])
>>> [17554.921892] [<7f15b464>] (ath10k_ce_per_engine_service_any
>>> [ath10k_pci]) from [<7f15a6d8>] (ath10k_pci_tasklet+0x24/0x5c
>>> [ath10k_pci])
>>> [17554.921946] [<7f15a6d8>] (ath10k_pci_tasklet [ath10k_pci]) from
>>> [<800304c8>] (tasklet_action+0x80/0x110)
>>> [17554.921979] [<800304c8>] (tasklet_action) from [<800306b8>]
>>> (__do_softirq+0x10c/0x248)
>>> [17554.922009] [<800306b8>] (__do_softirq) from [<80030a6c>]
>>> (irq_exit+0xac/0xf4)
>>> [17554.922042] [<80030a6c>] (irq_exit) from [<8000e904>]
>>> (handle_IRQ+0x44/0x90)
>>> [17554.922072] [<8000e904>] (handle_IRQ) from [<800084f8>]
>>> (gic_handle_irq+0x2c/0x5c)
>>> [17554.922105] [<800084f8>] (gic_handle_irq) from [<80011e00>]
>>> (__irq_svc+0x40/0x50)
>>> [17554.922122] Exception stack(0x80917f18 to 0x80917f60)
>>> [17554.922141] 7f00:
>>> 80917f60 000d3334
>>> [17554.922166] 7f20: 5221e106 00000ff7 4d693c44 00000ff7 a7705010
>>> 80924060 00000001 a7705014
>>> [17554.922190] 7f40: 8096243d 80916000 00000017 80917f60 a6aaaaab
>>> 80492940 60000013 ffffffff
>>> [17554.922224] [<80011e00>] (__irq_svc) from [<80492940>]
>>> (cpuidle_enter_state+0x50/0xe0)
>>> [17554.922252] [<80492940>] (cpuidle_enter_state) from [<80492ac8>]
>>> (cpuidle_idle_call+0xf8/0x148)
>>> [17554.922281] [<80492ac8>] (cpuidle_idle_call) from [<8000ec48>]
>>> (arch_cpu_idle+0x8/0x44)
>>> [17554.922322] [<8000ec48>] (arch_cpu_idle) from [<80066648>]
>>> (cpu_startup_entry+0xfc/0x140)
>>> [17554.922362] [<80066648>] (cpu_startup_entry) from [<808c5b08>]
>>> (start_kernel+0x360/0x36c)
>>> [17554.922379] ---[ end trace 87d4775146813aed ]---
>>> [17555.943454] ------------[ cut here ]------------
>>>
>>> that repeat continuously...
>>> Forcing legacy bitrates doesn't change the situation.
>>>
>>> I made some measurements using iperf, please find it in attachment.
>>>
>>> Other info:
>>>
>>> root@Yazi:~# modinfo ath10k_pci
>>> filename:
>>>
>>> /lib/modules/3.14.48-g408ccb9/kernel/drivers/net/wireless/ath/ath10k/ath10k_pci.ko
>>> firmware: ath10k/QCA9377/hw1.0/board.bin
>>> firmware: ath10k/QCA9377/hw1.0/firmware-5.bin
>>> firmware: ath10k/QCA6174/hw3.0/board-2.bin
>>> firmware: ath10k/QCA6174/hw3.0/board.bin
>>> firmware: ath10k/QCA6174/hw3.0/firmware-5.bin
>>> firmware: ath10k/QCA6174/hw3.0/firmware-4.bin
>>> firmware: ath10k/QCA6174/hw2.1/board-2.bin
>>> firmware: ath10k/QCA6174/hw2.1/board.bin
>>> firmware: ath10k/QCA6174/hw2.1/firmware-5.bin
>>> firmware: ath10k/QCA6174/hw2.1/firmware-4.bin
>>> firmware: ath10k/QCA988X/hw2.0/board-2.bin
>>> firmware: ath10k/QCA988X/hw2.0/board.bin
>>> firmware: ath10k/QCA988X/hw2.0/firmware-5.bin
>>> firmware: ath10k/QCA988X/hw2.0/firmware-4.bin
>>> firmware: ath10k/QCA988X/hw2.0/firmware-3.bin
>>> firmware: ath10k/QCA988X/hw2.0/firmware-2.bin
>>> firmware: ath10k/QCA988X/hw2.0/firmware.bin
>>> license: Dual BSD/GPL
>>> description: Driver support for Atheros QCA988X PCIe devices
>>> author: Qualcomm Atheros
>>> version: backported from Linux (v4.4.2-0-g1cb8570) using
>>> backports v4.4.2-1-0-gbec4037
>>> srcversion: EBB3D4E36DE49B7EC8057D0
>>> alias: pci:v0000168Cd00000042sv*sd*bc*sc*i*
>>> alias: pci:v0000168Cd00000040sv*sd*bc*sc*i*
>>> alias: pci:v0000168Cd0000003Esv*sd*bc*sc*i*
>>> alias: pci:v0000168Cd00000041sv*sd*bc*sc*i*
>>> alias: pci:v0000168Cd0000003Csv*sd*bc*sc*i*
>>> depends: ath10k_core,compat
>>> vermagic: 3.14.48-g408ccb9 SMP mod_unload modversions ARMv7 p2v8
>>> parm: irq_mode:0: auto, 1: legacy, 2: msi (default: 0) (uint)
>>> parm: reset_mode:0: auto, 1: warm only (default: 0) (uint)
>>>
>>> I don't know if it's only a problem of iw station dump that can't show
>>> the tx rate, but the spammed syslog honestly warn me...
>>>
>>> How shall I check what's wrong and see the HT work?
>>>
>>> Thanks a lot!
>>>
>>> Matteo
>>>
>>
>> --
>> Ben Greear <greearb@candelatech.com>
>> Candela Technologies Inc http://www.candelatech.com
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices
From: Toke Høiland-Jørgensen @ 2016-09-30 14:35 UTC (permalink / raw)
To: Johannes Berg; +Cc: make-wifi-fast, linux-wireless, netdev
In-Reply-To: <1475239915.17481.63.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
>> > I kinda see the logic here - we really don't need to queue as much
>> > if we can't possibly transmit it out quickly - but it seems to me
>> > we should also throw in some kind of limit that's relative to the
>> > amount of memory you have on the system?
>>
>> Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently
>> carries a patch for that which just changes the hard-coded default to
>> another hard-coded default. Not sure how to get a good value to use,
>> though; and deciding on how large a fraction of memory to use for
>> packets starts smelling an awful lot like setting policy in the
>> kernel, doesn't it?
>
> Yeah, I agree it does seem awkward.
>
> Perhaps we should instead pick a low limit and let users change it
> more easily (i.e. not debugfs)? I don't know a good answer to this
> either.
Hmm, I'll talk it over with some of the LEDE people who are more used to
dealing with these sorts of memory-constrained devices than I am. Will
send a patch if we come up with a good solution :)
-Toke
^ permalink raw reply
* Re: [PATCH v3 1/3] Documentation: dt: net: add mt76 wireless device binding
From: Arnd Bergmann @ 2016-09-30 14:36 UTC (permalink / raw)
To: Felix Fietkau; +Cc: Kalle Valo, linux-wireless, devicetree
In-Reply-To: <3334122f-0d79-f910-a414-5b9cafd9901f@nbd.name>
On Friday 30 September 2016, Felix Fietkau wrote:
> >> >> + pcie0 {
> >> >> + mt76@0,0 {
> >> >> + reg = <0x0000 0 0 0 0>;
> >
> > Maybe have an examplep of a real register address other than zero?
> This is a real example referring to the first device on a PCI bus.
> I copy&pasted this from a .dts file that we use in LEDE.
Ok, I see.
> >> >> + device_type = "pci";
> >> >> + mediatek,mtd-eeprom = <&factory 0x8000>;
> >> >> + mediatek,2ghz = <0>;
> >
> > It's not clear what the possible values for the 2ghz property are,
> > can you be more verbose in the description? How is <0> different
> > from no property?
> 0 means disabled, no property means unchanged (compared to EEPROM).
Maybe have a boolean property instead then to say "mediatek,2ghz-disabled" ?
If zero is the only possible value, there is no need to put a number in there.
Arnd
^ permalink raw reply
* Re: [PATCH v3 1/3] Documentation: dt: net: add mt76 wireless device binding
From: Felix Fietkau @ 2016-09-30 14:44 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Kalle Valo, linux-wireless, devicetree
In-Reply-To: <201609301636.43363.arnd@arndb.de>
On 2016-09-30 16:36, Arnd Bergmann wrote:
> On Friday 30 September 2016, Felix Fietkau wrote:
>> >> >> + pcie0 {
>> >> >> + mt76@0,0 {
>> >> >> + reg = <0x0000 0 0 0 0>;
>> >
>> > Maybe have an examplep of a real register address other than zero?
>> This is a real example referring to the first device on a PCI bus.
>> I copy&pasted this from a .dts file that we use in LEDE.
>
> Ok, I see.
>
>> >> >> + device_type = "pci";
>> >> >> + mediatek,mtd-eeprom = <&factory 0x8000>;
>> >> >> + mediatek,2ghz = <0>;
>> >
>> > It's not clear what the possible values for the 2ghz property are,
>> > can you be more verbose in the description? How is <0> different
>> > from no property?
>> 0 means disabled, no property means unchanged (compared to EEPROM).
>
> Maybe have a boolean property instead then to say "mediatek,2ghz-disabled" ?
>
> If zero is the only possible value, there is no need to put a number in there.
1 is also possible, which will force-enable the capability.
- Felix
^ 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