Re: [PATCH] ath10k: remove TX lock from ath10k_htt_tx_inc_pending

2019-09-17 Thread Kalle Valo
Erik Stromdahl  wrote:

> This commit removes the call to ath10k_mac_tx_lock() from
> ath10k_htt_tx_inc_pending() in case the high water mark is reached.
> 
> ath10k_mac_tx_lock() calls ieee80211_stop_queues() in order to stop
> mac80211 from pushing more TX data to the driver (this is the TX lock).
> 
> If a driver is trying to fetch an skb from a queue while the queue is
> stopped, ieee80211_tx_dequeue() will return NULL.
> 
> So, in ath10k_mac_tx_push_txq(), there is a risk that the call to
> ath10k_htt_tx_inc_pending() results in a stop of the mac80211 TX queues
> just before the skb is fetched.
> 
> This will cause ieee80211_tx_dequeue() to return NULL and
> ath10k_mac_tx_push_txq() to exit prematurely and return -ENOENT.
> Before the function returns ath10k_htt_tx_dec_pending() will be called.
> This call will re-enable the TX queues through ath10k_mac_tx_unlock().
> When ath10k_mac_tx_push_txq() has returned, the TX queue will be
> returned back to mac80211 with ieee80211_return_txq() without the skb
> being properly consumed.
> 
> Since the TX queues were re-enabled in the error exit path of
> ath10k_mac_tx_push_txq(), mac80211 can continue pushing data to the
> driver. If the hardware does not consume the data, the above mentioned
> case will be repeated over and over.
> 
> A case when the hardware is not able to transmit the data from the host
> is when a STA has been dis-associated from an AP and has not yet been
> able to re-associate. In this case there will be no TX_COMPL_INDs from
> the hardware, resulting in the TX counter not be decremented.
> 
> This phenomenon has been observed in both a real and a test setup.
> 
> In order to fix this, the actual TX locking (the call to
> ath10k_mac_tx_lock()) was removed from ath10k_htt_tx_inc_pending().
> Instead, ath10k_mac_tx_lock() is called separately after the skb has
> been fetched (after the call to ieee80211_tx_dequeue()). At this point
> it is OK to stop the queues.
> 
> Signed-off-by: Erik Stromdahl 

What hardware and firmware versions did you test this? Please always add that
to the commit log.

As Erik mostly works on SDIO I assume PCI is not that well tested. Has anyone
else tried this?

-- 
https://patchwork.kernel.org/patch/2997/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element

2019-09-17 Thread Kalle Valo
zhong jiang  wrote:

> With the help of Coccinelle, ARRAY_SIZE can be replaced in 
> ath10k_snoc_wlan_enable.
> 
> Signed-off-by: zhong jiang 

I already got an identical patch so dropping this one.

error: patch failed: drivers/net/wireless/ath/ath10k/snoc.c:976
error: drivers/net/wireless/ath/ath10k/snoc.c: patch does not apply
stg import: Diff does not apply cleanly

-- 
https://patchwork.kernel.org/patch/11129531/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element

2019-09-17 Thread Kalle Valo
zhong jiang  wrote:

> With the help of Coccinelle, ARRAY_SIZE can be replaced in 
> ath10k_snoc_wlan_enable.
> 
> Signed-off-by: zhong jiang 

I already got an identical patch so dropping this one.

error: patch failed: drivers/net/wireless/ath/ath10k/snoc.c:976
error: drivers/net/wireless/ath/ath10k/snoc.c: patch does not apply
stg import: Diff does not apply cleanly

-- 
https://patchwork.kernel.org/patch/11129531/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



Re: [PATCH v2] ath10k: fix max antenna gain unit

2019-09-17 Thread Kalle Valo
+ Govind, Wen

Sven Eckelmann  writes:

> From: Sven Eckelmann 
>
> Most of the txpower for the ath10k firmware is stored as twicepower (0.5 dB
> steps). This isn't the case for max_antenna_gain - which is still expected
> by the firmware as dB.
>
> The firmware is converting it from dB to the internal (twicepower)
> representation when it calculates the limits of a channel. This can be seen
> in tpc_stats when configuring "12" as max_antenna_gain. Instead of the
> expected 12 (6 dB), the tpc_stats shows 24 (12 dB).
>
> Tested on QCA9888 and IPQ4019 with firmware 10.4-3.5.3-00057.
>
> Fixes: 02256930d9b8 ("ath10k: use proper tx power unit")
> Signed-off-by: Sven Eckelmann 

[...]

> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -1008,7 +1008,7 @@ static int ath10k_monitor_vdev_start(struct ath10k *ar, 
> int vdev_id)
>   arg.channel.min_power = 0;
>   arg.channel.max_power = channel->max_power * 2;
>   arg.channel.max_reg_power = channel->max_reg_power * 2;
> - arg.channel.max_antenna_gain = channel->max_antenna_gain * 2;
> + arg.channel.max_antenna_gain = channel->max_antenna_gain;
>  
>   reinit_completion(&ar->vdev_setup_done);
>  
> @@ -1450,7 +1450,7 @@ static int ath10k_vdev_start_restart(struct ath10k_vif 
> *arvif,
>   arg.channel.min_power = 0;
>   arg.channel.max_power = chandef->chan->max_power * 2;
>   arg.channel.max_reg_power = chandef->chan->max_reg_power * 2;
> - arg.channel.max_antenna_gain = chandef->chan->max_antenna_gain * 2;
> + arg.channel.max_antenna_gain = chandef->chan->max_antenna_gain;
>  
>   if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
>   arg.ssid = arvif->u.ap.ssid;
> @@ -3105,7 +3105,7 @@ static int ath10k_update_channel_list(struct ath10k *ar)
>   ch->min_power = 0;
>   ch->max_power = channel->max_power * 2;
>   ch->max_reg_power = channel->max_reg_power * 2;
> - ch->max_antenna_gain = channel->max_antenna_gain * 2;
> + ch->max_antenna_gain = channel->max_antenna_gain;
>   ch->reg_class_id = 0; /* FIXME */

What about firmwares using WMI TLV interface, for example QCA6174 or
WCN3990? Does this break max antenna gain on those devices? Govind, Wen?

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


ath10k-firmware: QCA4019 hw1.0: Add EnGenius EMD1 specific BDFs

2019-09-17 Thread frank.shen
Hi,

The support for this device was added a while ago to OpenWrt. This AP requires 
two special BDFs to get the Wi-Fi PHYs working (correctly). The bmi-board-id='s 
would clash with one of the the IPQ401X AP-DK boards because QCA doesn't 
provide special IDs for customers and seems to use the AP-DK board-ids in the 
wifi firmware to change its behavior.

Now to the questions from the wiki page [1]:

l   description for what hardware this is:
-   it is a IPQ4018 based board
-   one QCA40xx radio is used as 2.4GHz radio
-   one QCA40xx radio is used as 5GHz radio

l   origin of the board file (did you create it yourself or where you 
downloaded)
-   taken from stock firmware image

l   ids to be used with the board file (ATH10K_BD_IE_BOARD_NAME in ath10k)
-QCA4019 hw1.0

md5sum of each new board file to add
+ bus=ahb,bmi-chip-id=0,bmi-board-id=16,variant=EnGenius-EMD1'
md5sum: da14e0efd0b408685a0e5aa83227d460
+ bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=EnGenius-EMD1'
md5sum: 685aa42c29ac39248ca75b748d8f9e18


Sincerely,
Frank Shen
Engineer
Research and Development
Senao Networks, Inc.
Tel: +886-3-3289289 ext. 2697
Fax: +886-3-3961112

bus=ahb,bmi-chip-id=0,bmi-board-id=16,variant=EnGenius-EMD1.bin
Description: bus=ahb,bmi-chip-id=0,bmi-board-id=16,variant=EnGenius-EMD1.bin


bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=EnGenius-EMD1.bin
Description: bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=EnGenius-EMD1.bin
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element

2019-09-17 Thread zhong jiang
On 2019/9/17 15:16, Kalle Valo wrote:
> zhong jiang  wrote:
>
>> With the help of Coccinelle, ARRAY_SIZE can be replaced in 
>> ath10k_snoc_wlan_enable.
>>
>> Signed-off-by: zhong jiang 
> I already got an identical patch so dropping this one.
>
> error: patch failed: drivers/net/wireless/ath/ath10k/snoc.c:976
> error: drivers/net/wireless/ath/ath10k/snoc.c: patch does not apply
> stg import: Diff does not apply cleanly
>
Thank for your remainder.

Sincerely,
zhong jiang



Re: [PATCH] ath10k: Check if station exists before forwarding tx airtime report

2019-09-17 Thread Kalle Valo
Hauke Mehrtens  wrote:

> It looks like the FW on QCA9984 already reports the tx airtimes before
> the station is added to the peer entry. The peer entry is created in
> ath10k_peer_map_event() just with the vdev_id and the ethaddr, but
> not with a station entry, this is added later in ath10k_peer_create() in
> callbacks from mac80211.
> 
> When there is no sta added to the peer entry, this function fails
> because it calls ieee80211_sta_register_airtime() with NULL.
> 
> This was reported in OpenWrt some time ago:
> https://bugs.openwrt.org/index.php?do=details&task_id=2414
> 
> This commit should fix this crash:
> [   75.991714] Unable to handle kernel paging request at virtual address 
> f9e8
> [   75.991756] pgd = c0204000
> [   75.997955] [f9e8] *pgd=5fdfd861, *pte=, *ppte=
> [   76.000537] Internal error: Oops: 37 [#1] SMP ARM
> [   76.006686] Modules linked in: pppoe ppp_async ath10k_pci ath10k_core ath 
> pptp pppox ppp_mppe ppp_generic mac80211 iptable_nat ipt_REJECT 
> ipt_MASQUERADE cfg80211 xt_time xt_tcpudp xt_tcpmss xt_statistic xt_state 
> xt_recent xt_nat xt_multiport xt_mark xt_mac xt_limit xt_length xt_hl 
> xt_helper xt_esp xt_ecn xt_dscp xt_conntrack xt_connmark xt_connlimit 
> xt_connbytes xt_comment xt_TCPMSS xt_REDIRECT xt_LOG xt_HL xt_FLOWOFFLOAD 
> xt_DSCP xt_CT xt_CLASSIFY usbserial slhc nf_reject_ipv4 nf_nat_redirect 
> nf_nat_masquerade_ipv4 nf_conntrack_ipv4 nf_nat_ipv4 nf_log_ipv4 
> nf_flow_table_hw nf_flow_table nf_defrag_ipv4 nf_conntrack_rtcache 
> nf_conntrack_netlink iptable_raw iptable_mangle iptable_filter ipt_ah ipt_ECN 
> ip_tables crc_ccitt compat chaoskey fuse sch_cake sch_tbf sch_ingress sch_htb 
> sch_hfsc em_u32 cls_u32
> [   76.059974]  cls_tcindex cls_route cls_matchall cls_fw cls_flow cls_basic 
> act_skbedit act_mirred ledtrig_usbport xt_set ip_set_list_set 
> ip_set_hash_netportnet ip_set_hash_netport ip_set_hash_netnet 
> ip_set_hash_netiface ip_set_hash_net ip_set_hash_mac ip_set_hash_ipportnet 
> ip_set_hash_ipportip ip_set_hash_ipport ip_set_hash_ipmark ip_set_hash_ip 
> ip_set_bitmap_port ip_set_bitmap_ipmac ip_set_bitmap_ip ip_set nfnetlink 
> ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6t_NPT 
> ip6t_MASQUERADE nf_nat_masquerade_ipv6 nf_nat nf_conntrack nf_log_ipv6 
> nf_log_common ip6table_mangle ip6table_filter ip6_tables ip6t_REJECT x_tables 
> nf_reject_ipv6 msdos ip_gre gre ifb sit tunnel4 ip_tunnel tun vfat fat 
> hfsplus cifs nls_utf8 nls_iso8859_15 nls_iso8859_1 nls_cp850 nls_cp437 
> nls_cp1250 sha1_generic md5 md4
> [   76.130634]  usb_storage leds_gpio xhci_plat_hcd xhci_pci xhci_hcd dwc3 
> dwc3_of_simple ohci_platform ohci_hcd phy_qcom_dwc3 ahci ehci_platform sd_mod 
> ahci_platform libahci_platform libahci libata scsi_mod ehci_hcd 
> gpio_button_hotplug ext4 mbcache jbd2 exfat crc32c_generic
> [   76.154772] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.132 #0
> [   76.177001] Hardware name: Generic DT based system
> [   76.182990] task: c0b06d80 task.stack: c0b0
> [   76.187832] PC is at ieee80211_sta_register_airtime+0x24/0x148 [mac80211]
> [   76.192211] LR is at ath10k_htt_t2h_msg_handler+0x678/0x10f4 [ath10k_core]
> [   76.199052] pc : []lr : []psr: a113
> [   76.205820] sp : c0b01d54  ip : 0002  fp : bf869c0c
> [   76.211981] r10: 003c  r9 : dbdca138  r8 : 00060002
> [   76.217192] r7 :   r6 : dabe1150  r5 :   r4 : dbdc95c0
> [   76.222401] r3 :   r2 : 00060002  r1 :   r0 : 
> [   76.229003] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment 
> none
> [   76.235509] Control: 10c5787d  Table: 5c94006a  DAC: 0051
> [   76.242716] Process swapper/0 (pid: 0, stack limit = 0xc0b00210)
> [   76.248446] Stack: (0xc0b01d54 to 0xc0b02000)
> [   76.254532] 1d40:  dbdc95c0 
>  dabe1150
> [   76.258808] 1d60: 0001 dabe1150 dbdca138 003c bf869c0c bf83e8b0 
> 0002 c0314b10
> [   76.266969] 1d80: dbdc9c70 0001 0001 dabe114c 0001  
> dbdcd724 bf88f3d8
> [   76.275126] 1da0: c0310d28 db393c00 dbdc95c0  c0b01dd0 c07fb4c4 
> dbdcd724 0001
> [   76.283286] 1dc0: 0022 bf88b09c db393c00 0022 c0b01dd0 c0b01dd0 
>  dbdcc5c0
> [   76.291445] 1de0: bf88f04c dbdcd654 dbdcd71c dbdc95c0 0014 dbdcd724 
> dbdcc5c0 0005
> [   76.299605] 1e00: 0004b400 bf85c360  bf87101c c0b01e24 0006 
>  dbdc95c0
> [   76.307764] 1e20: 0001 0040 012c c0b01e80 1cf51000 bf85c448 
> dbdcd440 dbdc95c0
> [   76.315925] 1e40: dbdca440 a880 0040 bf88cb68 dbdcd440 0001 
> 0040 a880
> [   76.324084] 1e60: c0b02d00 c06d72e0 dd990080 c0a3f080 c0b255dc c0b047e4 
> c090afac c090e80c
> [   76.332244] 1e80: c0b01e80 c0b01e80 c0b01e88 c0b01e88 dd4cc200  
> 0003 c0b0208c
> [   76.340405] 1ea0: c0b02080 4003 e000 0100 c0b02080 c03015c8 
>  0001
> [   76.348564] 1ec0: dd40800

Re: [PATCH] ath10k: Check if station exists before forwarding tx airtime report

2019-09-17 Thread Kalle Valo
Hauke Mehrtens  wrote:

> It looks like the FW on QCA9984 already reports the tx airtimes before
> the station is added to the peer entry. The peer entry is created in
> ath10k_peer_map_event() just with the vdev_id and the ethaddr, but
> not with a station entry, this is added later in ath10k_peer_create() in
> callbacks from mac80211.
> 
> When there is no sta added to the peer entry, this function fails
> because it calls ieee80211_sta_register_airtime() with NULL.
> 
> This was reported in OpenWrt some time ago:
> https://bugs.openwrt.org/index.php?do=details&task_id=2414
> 
> This commit should fix this crash:
> [   75.991714] Unable to handle kernel paging request at virtual address 
> f9e8
> [   75.991756] pgd = c0204000
> [   75.997955] [f9e8] *pgd=5fdfd861, *pte=, *ppte=
> [   76.000537] Internal error: Oops: 37 [#1] SMP ARM
> [   76.006686] Modules linked in: pppoe ppp_async ath10k_pci ath10k_core ath 
> pptp pppox ppp_mppe ppp_generic mac80211 iptable_nat ipt_REJECT 
> ipt_MASQUERADE cfg80211 xt_time xt_tcpudp xt_tcpmss xt_statistic xt_state 
> xt_recent xt_nat xt_multiport xt_mark xt_mac xt_limit xt_length xt_hl 
> xt_helper xt_esp xt_ecn xt_dscp xt_conntrack xt_connmark xt_connlimit 
> xt_connbytes xt_comment xt_TCPMSS xt_REDIRECT xt_LOG xt_HL xt_FLOWOFFLOAD 
> xt_DSCP xt_CT xt_CLASSIFY usbserial slhc nf_reject_ipv4 nf_nat_redirect 
> nf_nat_masquerade_ipv4 nf_conntrack_ipv4 nf_nat_ipv4 nf_log_ipv4 
> nf_flow_table_hw nf_flow_table nf_defrag_ipv4 nf_conntrack_rtcache 
> nf_conntrack_netlink iptable_raw iptable_mangle iptable_filter ipt_ah ipt_ECN 
> ip_tables crc_ccitt compat chaoskey fuse sch_cake sch_tbf sch_ingress sch_htb 
> sch_hfsc em_u32 cls_u32
> [   76.059974]  cls_tcindex cls_route cls_matchall cls_fw cls_flow cls_basic 
> act_skbedit act_mirred ledtrig_usbport xt_set ip_set_list_set 
> ip_set_hash_netportnet ip_set_hash_netport ip_set_hash_netnet 
> ip_set_hash_netiface ip_set_hash_net ip_set_hash_mac ip_set_hash_ipportnet 
> ip_set_hash_ipportip ip_set_hash_ipport ip_set_hash_ipmark ip_set_hash_ip 
> ip_set_bitmap_port ip_set_bitmap_ipmac ip_set_bitmap_ip ip_set nfnetlink 
> ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6t_NPT 
> ip6t_MASQUERADE nf_nat_masquerade_ipv6 nf_nat nf_conntrack nf_log_ipv6 
> nf_log_common ip6table_mangle ip6table_filter ip6_tables ip6t_REJECT x_tables 
> nf_reject_ipv6 msdos ip_gre gre ifb sit tunnel4 ip_tunnel tun vfat fat 
> hfsplus cifs nls_utf8 nls_iso8859_15 nls_iso8859_1 nls_cp850 nls_cp437 
> nls_cp1250 sha1_generic md5 md4
> [   76.130634]  usb_storage leds_gpio xhci_plat_hcd xhci_pci xhci_hcd dwc3 
> dwc3_of_simple ohci_platform ohci_hcd phy_qcom_dwc3 ahci ehci_platform sd_mod 
> ahci_platform libahci_platform libahci libata scsi_mod ehci_hcd 
> gpio_button_hotplug ext4 mbcache jbd2 exfat crc32c_generic
> [   76.154772] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.132 #0
> [   76.177001] Hardware name: Generic DT based system
> [   76.182990] task: c0b06d80 task.stack: c0b0
> [   76.187832] PC is at ieee80211_sta_register_airtime+0x24/0x148 [mac80211]
> [   76.192211] LR is at ath10k_htt_t2h_msg_handler+0x678/0x10f4 [ath10k_core]
> [   76.199052] pc : []lr : []psr: a113
> [   76.205820] sp : c0b01d54  ip : 0002  fp : bf869c0c
> [   76.211981] r10: 003c  r9 : dbdca138  r8 : 00060002
> [   76.217192] r7 :   r6 : dabe1150  r5 :   r4 : dbdc95c0
> [   76.222401] r3 :   r2 : 00060002  r1 :   r0 : 
> [   76.229003] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment 
> none
> [   76.235509] Control: 10c5787d  Table: 5c94006a  DAC: 0051
> [   76.242716] Process swapper/0 (pid: 0, stack limit = 0xc0b00210)
> [   76.248446] Stack: (0xc0b01d54 to 0xc0b02000)
> [   76.254532] 1d40:  dbdc95c0 
>  dabe1150
> [   76.258808] 1d60: 0001 dabe1150 dbdca138 003c bf869c0c bf83e8b0 
> 0002 c0314b10
> [   76.266969] 1d80: dbdc9c70 0001 0001 dabe114c 0001  
> dbdcd724 bf88f3d8
> [   76.275126] 1da0: c0310d28 db393c00 dbdc95c0  c0b01dd0 c07fb4c4 
> dbdcd724 0001
> [   76.283286] 1dc0: 0022 bf88b09c db393c00 0022 c0b01dd0 c0b01dd0 
>  dbdcc5c0
> [   76.291445] 1de0: bf88f04c dbdcd654 dbdcd71c dbdc95c0 0014 dbdcd724 
> dbdcc5c0 0005
> [   76.299605] 1e00: 0004b400 bf85c360  bf87101c c0b01e24 0006 
>  dbdc95c0
> [   76.307764] 1e20: 0001 0040 012c c0b01e80 1cf51000 bf85c448 
> dbdcd440 dbdc95c0
> [   76.315925] 1e40: dbdca440 a880 0040 bf88cb68 dbdcd440 0001 
> 0040 a880
> [   76.324084] 1e60: c0b02d00 c06d72e0 dd990080 c0a3f080 c0b255dc c0b047e4 
> c090afac c090e80c
> [   76.332244] 1e80: c0b01e80 c0b01e80 c0b01e88 c0b01e88 dd4cc200  
> 0003 c0b0208c
> [   76.340405] 1ea0: c0b02080 4003 e000 0100 c0b02080 c03015c8 
>  0001
> [   76.348564] 1ec0: dd40800

Re: [PATCH] ath10k: Fix HOST capability QMI incompatibility

2019-09-17 Thread Kalle Valo
Bjorn Andersson  wrote:

> The introduction of 768ec4c012ac ("ath10k: update HOST capability QMI
> message") served the purpose of supporting the new and extended HOST
> capability QMI message.
> 
> But while the new message adds a slew of optional members it changes the
> data type of the "daemon_support" member, which means that older
> versions of the firmware will fail to decode the incoming request
> message.
> 
> There is no way to detect this breakage from Linux and there's no way to
> recover from sending the wrong message (i.e. we can't just try one
> format and then fallback to the other), so a quirk is introduced in
> DeviceTree to indicate to the driver that the firmware requires the 8bit
> version of this message.
> 
> Cc: sta...@vger.kernel.org
> Fixes: 768ec4c012ac ("ath10k: update HOST capability qmi message")
> Signed-off-by: Bjorn Andersson 
> Acked-by: Rob Herring 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

7165ef890a4c ath10k: Fix HOST capability QMI incompatibility

-- 
https://patchwork.kernel.org/patch/11058005/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



Re: [PATCH] ath10k: Fix HOST capability QMI incompatibility

2019-09-17 Thread Kalle Valo
Bjorn Andersson  wrote:

> The introduction of 768ec4c012ac ("ath10k: update HOST capability QMI
> message") served the purpose of supporting the new and extended HOST
> capability QMI message.
> 
> But while the new message adds a slew of optional members it changes the
> data type of the "daemon_support" member, which means that older
> versions of the firmware will fail to decode the incoming request
> message.
> 
> There is no way to detect this breakage from Linux and there's no way to
> recover from sending the wrong message (i.e. we can't just try one
> format and then fallback to the other), so a quirk is introduced in
> DeviceTree to indicate to the driver that the firmware requires the 8bit
> version of this message.
> 
> Cc: sta...@vger.kernel.org
> Fixes: 768ec4c012ac ("ath10k: update HOST capability qmi message")
> Signed-off-by: Bjorn Andersson 
> Acked-by: Rob Herring 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

7165ef890a4c ath10k: Fix HOST capability QMI incompatibility

-- 
https://patchwork.kernel.org/patch/11058005/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH 1/3] ath10k: snoc: skip regulator operations

2019-09-17 Thread Kalle Valo
Bjorn Andersson  wrote:

> The regulator operations is trying to set a voltage to a fixed value, by
> giving some wiggle room. But some board designs specifies regulator
> voltages outside this limited range. One such example is the Lenovo Yoga
> C630, with vdd-3.3-ch0 in particular specified at 3.1V.
> 
> But consumers with fixed voltage requirements should just rely on the
> board configuration to provide the power at the required level, so this
> code should be removed.
> 
> Signed-off-by: Bjorn Andersson 
> Signed-off-by: Kalle Valo 

3 patches applied to ath-next branch of ath.git, thanks.

b003e7f1974e ath10k: snoc: skip regulator operations
c56c7f24d7f8 ath10k: Use standard regulator bulk API in snoc
f93bcf0ce6a1 ath10k: Use standard bulk clock API in snoc

-- 
https://patchwork.kernel.org/patch/11059507/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



Re: [PATCH 1/3] ath10k: snoc: skip regulator operations

2019-09-17 Thread Kalle Valo
Bjorn Andersson  wrote:

> The regulator operations is trying to set a voltage to a fixed value, by
> giving some wiggle room. But some board designs specifies regulator
> voltages outside this limited range. One such example is the Lenovo Yoga
> C630, with vdd-3.3-ch0 in particular specified at 3.1V.
> 
> But consumers with fixed voltage requirements should just rely on the
> board configuration to provide the power at the required level, so this
> code should be removed.
> 
> Signed-off-by: Bjorn Andersson 
> Signed-off-by: Kalle Valo 

3 patches applied to ath-next branch of ath.git, thanks.

b003e7f1974e ath10k: snoc: skip regulator operations
c56c7f24d7f8 ath10k: Use standard regulator bulk API in snoc
f93bcf0ce6a1 ath10k: Use standard bulk clock API in snoc

-- 
https://patchwork.kernel.org/patch/11059507/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v3] ath10k: avoid leaving .bss_info_changed prematurely

2019-09-17 Thread Kalle Valo
Sven Eckelmann  wrote:

> ath10k_bss_info_changed() handles various events from the upper layers. It
> parses the changed bitfield and then configures the driver/firmware
> accordingly. Each detected event is handled in a separate scope which is
> independent of each other - but in the same function.
> 
> The commit f279294e9ee2 ("ath10k: add support for configuring management
> packet rate") changed this behavior by returning from this function
> prematurely when some precondition was not fulfilled. All new event
> handlers added after the BSS_CHANGED_BASIC_RATES event handler would then
> also be skipped.
> 
> Signed-off-by: Sven Eckelmann 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

0227ff3656f2 ath10k: avoid leaving .bss_info_changed prematurely

-- 
https://patchwork.kernel.org/patch/11029683/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v3] ath10k: avoid leaving .bss_info_changed prematurely

2019-09-17 Thread Kalle Valo
Sven Eckelmann  wrote:

> ath10k_bss_info_changed() handles various events from the upper layers. It
> parses the changed bitfield and then configures the driver/firmware
> accordingly. Each detected event is handled in a separate scope which is
> independent of each other - but in the same function.
> 
> The commit f279294e9ee2 ("ath10k: add support for configuring management
> packet rate") changed this behavior by returning from this function
> prematurely when some precondition was not fulfilled. All new event
> handlers added after the BSS_CHANGED_BASIC_RATES event handler would then
> also be skipped.
> 
> Signed-off-by: Sven Eckelmann 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

0227ff3656f2 ath10k: avoid leaving .bss_info_changed prematurely

-- 
https://patchwork.kernel.org/patch/11029683/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



Re: [PATCH] ath10k: Use ARRAY_SIZE

2019-09-17 Thread Kalle Valo
Vasyl Gomonovych  wrote:

> fix coccinelle warning, use ARRAY_SIZE
> 
> Signed-off-by: Vasyl Gomonovych 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

7921ae091907 ath10k: Use ARRAY_SIZE

-- 
https://patchwork.kernel.org/patch/11049553/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



Re: [PATCH] ath10k: Use ARRAY_SIZE

2019-09-17 Thread Kalle Valo
Vasyl Gomonovych  wrote:

> fix coccinelle warning, use ARRAY_SIZE
> 
> Signed-off-by: Vasyl Gomonovych 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

7921ae091907 ath10k: Use ARRAY_SIZE

-- 
https://patchwork.kernel.org/patch/11049553/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: revalidate the msa region coming from firmware

2019-09-17 Thread Kalle Valo
Govind Singh  wrote:

> driver sends QMI_WLFW_MSA_INFO_REQ_V01 QMI request to firmware
> and in response expects range of addresses and size to be mapped.
> Add condition to check whether addresses in response falls
> under valid range otherwise return failure.
> 
> Testing: Tested on WCN3990 HW
> Tested FW: WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
> 
> Signed-off-by: Govind Singh 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

c41305993ff5 ath10k: revalidate the msa region coming from firmware

-- 
https://patchwork.kernel.org/patch/11067547/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: revalidate the msa region coming from firmware

2019-09-17 Thread Kalle Valo
Govind Singh  wrote:

> driver sends QMI_WLFW_MSA_INFO_REQ_V01 QMI request to firmware
> and in response expects range of addresses and size to be mapped.
> Add condition to check whether addresses in response falls
> under valid range otherwise return failure.
> 
> Testing: Tested on WCN3990 HW
> Tested FW: WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
> 
> Signed-off-by: Govind Singh 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

c41305993ff5 ath10k: revalidate the msa region coming from firmware

-- 
https://patchwork.kernel.org/patch/11067547/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



Re: [PATCH] ath10k: add cleanup in ath10k_sta_state()

2019-09-17 Thread Kalle Valo
Wenwen Wang  wrote:

> If 'sta->tdls' is false, no cleanup is executed, leading to memory/resource
> leaks, e.g., 'arsta->tx_stats'. To fix this issue, perform cleanup before
> go to the 'exit' label.
> 
> Signed-off-by: Wenwen Wang 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

334f5b61a6f2 ath10k: add cleanup in ath10k_sta_state()

-- 
https://patchwork.kernel.org/patch/11096481/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



Re: [PATCH] ath10k: add cleanup in ath10k_sta_state()

2019-09-17 Thread Kalle Valo
Wenwen Wang  wrote:

> If 'sta->tdls' is false, no cleanup is executed, leading to memory/resource
> leaks, e.g., 'arsta->tx_stats'. To fix this issue, perform cleanup before
> go to the 'exit' label.
> 
> Signed-off-by: Wenwen Wang 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

334f5b61a6f2 ath10k: add cleanup in ath10k_sta_state()

-- 
https://patchwork.kernel.org/patch/11096481/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: fix latency issue for QCA988x

2019-09-17 Thread Kalle Valo
Miaoqing Pan  wrote:

> Bad latency is found on QCA988x, the issue was introduced by
> commit 4504f0e5b571 ("ath10k: sdio: workaround firmware UART
> pin configuration bug"). If uart_pin_workaround is false, this
> change will set uart pin even if uart_print is false.
> 
> Tested HW: QCA9880
> Tested FW: 10.2.4-1.0-00037
> 
> Fixes: 4504f0e5b571 ("ath10k: sdio: workaround firmware UART pin 
> configuration bug")
> Signed-off-by: Miaoqing Pan 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

1340cc631bd0 ath10k: fix latency issue for QCA988x

-- 
https://patchwork.kernel.org/patch/11122193/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: fix latency issue for QCA988x

2019-09-17 Thread Kalle Valo
Miaoqing Pan  wrote:

> Bad latency is found on QCA988x, the issue was introduced by
> commit 4504f0e5b571 ("ath10k: sdio: workaround firmware UART
> pin configuration bug"). If uart_pin_workaround is false, this
> change will set uart pin even if uart_print is false.
> 
> Tested HW: QCA9880
> Tested FW: 10.2.4-1.0-00037
> 
> Fixes: 4504f0e5b571 ("ath10k: sdio: workaround firmware UART pin 
> configuration bug")
> Signed-off-by: Miaoqing Pan 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

1340cc631bd0 ath10k: fix latency issue for QCA988x

-- 
https://patchwork.kernel.org/patch/11122193/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



Re: [PATCH] ath10k: fix spelling mistake "eanble" -> "enable"

2019-09-17 Thread Kalle Valo
Colin King  wrote:

> There is a spelling mistake in a ath10k_warn warning message. Fix it.
> 
> Signed-off-by: Colin Ian King 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

09764659003d ath10k: fix spelling mistake "eanble" -> "enable"

-- 
https://patchwork.kernel.org/patch/11144035/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: fix spelling mistake "eanble" -> "enable"

2019-09-17 Thread Kalle Valo
Colin King  wrote:

> There is a spelling mistake in a ath10k_warn warning message. Fix it.
> 
> Signed-off-by: Colin Ian King 
> Signed-off-by: Kalle Valo 

Patch applied to ath-next branch of ath.git, thanks.

09764659003d ath10k: fix spelling mistake "eanble" -> "enable"

-- 
https://patchwork.kernel.org/patch/11144035/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



Re: [PATCH 1/4] mac80211: Switch to a virtual time-based airtime scheduler

2019-09-17 Thread Toke Høiland-Jørgensen
Yibo Zhao  writes:

> From: Toke Høiland-Jørgensen 
>
> This switches the airtime scheduler in mac80211 to use a virtual time-based
> scheduler instead of the round-robin scheduler used before. This has a
> couple of advantages:

Thank you for keeping at this! I'll take a look at the series in detail
tomorrow.

While you're testing things related to this, I've also prototyped a port
of the "airtime queue limit" feature from chromeos into mainline. It's
currently in my tree here:
https://git.kernel.org/pub/scm/linux/kernel/git/toke/linux.git/log/?h=mac80211-aql-01

If you have time to test it at some point, that would be awesome. I'm
planning to submit it as an RFC, but it needs a bit more work first.
Also, it's completely untested, but it does compile :)

-Toke



Re: [PATCH v3] ath10k: support NET_DETECT WoWLAN feature

2019-09-17 Thread Brian Norris
Since Wen has once again suggested I use this patch in other forums,
I'll ping here to note:

On Wed, Nov 14, 2018 at 2:59 PM Brian Norris  wrote:
> You've introduced a regression in 4.20-rc1:

This regression still survives in the latest tree. Is it fair to just
submit a revert?

Brian


Re: [PATCH] cfg80211: Add cumulative channel survey dump support.

2019-09-17 Thread Sven Eckelmann
On Thursday, 31 May 2018 11:06:59 CEST vnara...@codeaurora.org wrote:
> I will sent next version of patch with updated commit log.

Can you please point me to the second version?

Btw. I've just checked the minimal changes in ath10k to get this working. It 
seems we need SURVEY_INFO_NON_ACC_DATA in ath10k's ath10k_get_survey + memset 
of ar->survey[idx].

But right now the total time looks (especially) wrong to me. At least it is 
rather unlikely that I can have around 30 second active time delta in
roughly 1 real world second.  Maybe a bug with the READ_CLEAR handling in
firmware 10.2.4-1.0-00043 or maybe all firmware version? More logs about
that at the end.

@Ben: Was this also what you've experience in the past with the 10.2 firmware
bss_chan_info counter bugs or am I just misusing the functionality of the
firmware?


And yes, I am aware that the minimal change in ath10k is not enough - but it 
was good enough for a simple test. I think ath10k_wmi_event_pdev_bss_chan_info 
+ ath10k_hw_fill_survey_time also have to be modified to accumulate .time, 
.time_busy, .time_rx, and .time_tx.  But it makes me wonder whether there is 
now any benefit from the cfg80211 patch (for ath10k). You can basically get 
the same functionality from

--- a/drivers/net/wireless/ath/ath10k/hw.c
+++ b/drivers/net/wireless/ath/ath10k/hw.c
@@ -216,8 +217,9 @@ void ath10k_hw_fill_survey_time(struct ath10k *ar, struct 
survey_info *survey,
cc -= cc_prev - cc_fix;
rcc -= rcc_prev - rcc_fix;
 
-   survey->time = CCNT_TO_MSEC(ar, cc);
-   survey->time_busy = CCNT_TO_MSEC(ar, rcc);
+   survey->time += CCNT_TO_MSEC(ar, cc);
+   if (survey->filled & SURVEY_INFO_TIME_BUSY)
+   survey->time_busy += CCNT_TO_MSEC(ar, rcc);
 }
 
 const struct ath10k_hw_ops qca988x_ops = {
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -4905,6 +4905,13 @@ static int ath10k_wmi_event_pdev_bss_chan_info(struct 
ath10k *ar,
   "wmi event pdev bss chan info:\n freq: %d noise: %d cycle: 
busy %llu total %llu tx %llu rx %llu rx_bss %llu\n",
   freq, noise_floor, busy, total, tx, rx, rx_bss);
 
+   /* everything zero means invalid data -> drop it to avoid ruining the
+* noisefloor
+*/
+   if (noise_floor == 0 && busy == 0 && total == 0 && tx == 0 && rx == 0 &&
+   rx_bss == 0)
+   return -EPROTO;
+
spin_lock_bh(&ar->data_lock);
idx = freq_to_idx(ar, freq);
if (idx >= ARRAY_SIZE(ar->survey)) {
@@ -4916,10 +4923,10 @@ static int ath10k_wmi_event_pdev_bss_chan_info(struct 
ath10k *ar,
survey = &ar->survey[idx];
 
survey->noise = noise_floor;
-   survey->time  = div_u64(total, cc_freq_hz);
-   survey->time_busy = div_u64(busy, cc_freq_hz);
-   survey->time_rx   = div_u64(rx_bss, cc_freq_hz);
-   survey->time_tx   = div_u64(tx, cc_freq_hz);
+   survey->time  += div_u64(total, cc_freq_hz);
+   survey->time_busy += div_u64(busy, cc_freq_hz);
+   survey->time_rx   += div_u64(rx_bss, cc_freq_hz);
+   survey->time_tx   += div_u64(tx, cc_freq_hz);
survey->filled   |= (SURVEY_INFO_NOISE_DBM |
 SURVEY_INFO_TIME |
 SURVEY_INFO_TIME_BUSY |


Both version get to the same (incorrect) timing results :)

Here is the time data I get reported from the firmware (entries
with 0 in each field were removed)

[  162.875008] wmi event pdev bss chan info:  freq: 5180 noise: -102 cycle: 
busy 83895506 total 3801904834 tx 267520 rx 71507709 rx_bss 0
[  163.917612] wmi event pdev bss chan info:  freq: 5180 noise: -102 cycle: 
busy 85745287 total 3889908918 tx 214016 rx 73140278 rx_bss 0
[  164.960733] wmi event pdev bss chan info:  freq: 5180 noise: -102 cycle: 
busy 87612741 total 3977912729 tx 221408 rx 74750020 rx_bss 22880
[  166.004348] wmi event pdev bss chan info:  freq: 5180 noise: -102 cycle: 
busy 89489393 total 4065916182 tx 214016 rx 76383244 rx_bss 0
[  167.048330] wmi event pdev bss chan info:  freq: 5180 noise: -102 cycle: 
busy 91292949 total 4153919205 tx 214016 rx 77942266 rx_bss 0
[  168.091828] wmi event pdev bss chan info:  freq: 5180 noise: -102 cycle: 
busy 93118897 total 4241923018 tx 187264 rx 79551036 rx_bss 0
[  169.134349] wmi event pdev bss chan info:  freq: 5180 noise: -102 cycle: 
busy 47834553 total 2182443012 tx 160512 rx 40879686 rx_bss 0
[  170.177511] wmi event pdev bss chan info:  freq: 5180 noise: -102 cycle: 
busy 49751910 total 2270447314 tx 167904 rx 42539598 rx_bss 22880
[  171.221156] wmi event pdev bss chan info:  freq: 5180 noise: -102 cycle: 
busy 51639827 total 2358451435 tx 160512 rx 44184146 rx_bss 0
[  172.262426] wmi event pdev bss chan info:  freq: 5180 noise: -102 cycle: 
busy 53420475 total 2446455772 tx 133760 rx 45746866 rx_bss 0
[  173.309067] wmi event pdev bss chan info:  freq: 5180 noise: 

Re: [PATCH] cfg80211: Add cumulative channel survey dump support.

2019-09-17 Thread Ben Greear

On 9/17/19 10:27 AM, Sven Eckelmann wrote:

On Thursday, 31 May 2018 11:06:59 CEST vnara...@codeaurora.org wrote:

I will sent next version of patch with updated commit log.


Can you please point me to the second version?

Btw. I've just checked the minimal changes in ath10k to get this working. It
seems we need SURVEY_INFO_NON_ACC_DATA in ath10k's ath10k_get_survey + memset
of ar->survey[idx].

But right now the total time looks (especially) wrong to me. At least it is
rather unlikely that I can have around 30 second active time delta in
roughly 1 real world second.  Maybe a bug with the READ_CLEAR handling in
firmware 10.2.4-1.0-00043 or maybe all firmware version? More logs about
that at the end.

@Ben: Was this also what you've experience in the past with the 10.2 firmware
bss_chan_info counter bugs or am I just misusing the functionality of the
firmware?


Last I recall, the upstream code had several bugs.  Maybe some QCA
firmware person can let you know if they fixed the upstream firmware.

If you want to test against ath10k-ct driver/firmware, and if you still see 
bogus values, then
I can debug and fix it.

Thanks,
Ben

--
Ben Greear 
Candela Technologies Inc  http://www.candelatech.com



Re: [PATCH 2/4] mac80211: defer txqs removal from rbtree

2019-09-17 Thread Toke Høiland-Jørgensen
Yibo Zhao  writes:

> In a loop txqs dequeue scenario, if the first txq in the rbtree gets
> removed from rbtree immediately in the ieee80211_return_txq(), the
> loop will break soon in the ieee80211_next_txq() due to schedule_pos
> not leading to the second txq in the rbtree. Thus, defering the
> removal right before the end of this schedule round.
>
> Co-developed-by: Yibo Zhao 
> Signed-off-by: Yibo Zhao 
> Signed-off-by: Toke Høiland-Jørgensen 

I didn't write this patch, so please don't use my sign-off. I'll add
ack or review tags as appropriate in reply; but a few comments first:

> ---
>  include/net/mac80211.h | 16 ++--
>  net/mac80211/ieee80211_i.h |  3 +++
>  net/mac80211/main.c|  6 +
>  net/mac80211/tx.c  | 63 
> +++---
>  4 files changed, 83 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index ac2ed8e..ba5a345 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -925,6 +925,8 @@ struct ieee80211_tx_rate {
>  
>  #define IEEE80211_MAX_TX_RETRY   31
>  
> +#define IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS 100
> +
>  static inline void ieee80211_rate_set_vht(struct ieee80211_tx_rate *rate,
> u8 mcs, u8 nss)
>  {
> @@ -6232,7 +6234,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct 
> ieee80211_hw *hw,
>   * @ac: AC number to return packets from.
>   *
>   * Should only be called between calls to ieee80211_txq_schedule_start()
> - * and ieee80211_txq_schedule_end().
> + * and ieee80211_txq_schedule_end(). If the txq is empty, it will be added
> + * to a remove list and get removed later.
>   * Returns the next txq if successful, %NULL if no queue is eligible. If a 
> txq
>   * is returned, it should be returned with ieee80211_return_txq() after the
>   * driver has finished scheduling it.
> @@ -6268,7 +6271,8 @@ void ieee80211_txq_schedule_start(struct ieee80211_hw 
> *hw, u8 ac)
>   * @hw: pointer as obtained from ieee80211_alloc_hw()
>   * @ac: AC number to acquire locks for
>   *
> - * Release locks previously acquired by ieee80211_txq_schedule_end().
> + * Release locks previously acquired by ieee80211_txq_schedule_end(). Check
> + * and remove the empty txq from rb-tree.
>   */
>  void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac)
>   __releases(txq_lock);
> @@ -6287,6 +6291,14 @@ void ieee80211_schedule_txq(struct ieee80211_hw *hw, 
> struct ieee80211_txq *txq)
>   __acquires(txq_lock) __releases(txq_lock);
>  
>  /**
> + * ieee80211_txqs_check - Check txqs waiting for removal
> + *
> + * @tmr: pointer as obtained from local
> + *
> + */
> +void ieee80211_txqs_check(struct timer_list *tmr);
> +
> +/**
>   * ieee80211_txq_may_transmit - check whether TXQ is allowed to transmit
>   *
>   * This function is used to check whether given txq is allowed to transmit by
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index a4556f9..49aa143e 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -847,6 +847,7 @@ struct txq_info {
>   struct codel_stats cstats;
>   struct sk_buff_head frags;
>   struct rb_node schedule_order;
> + struct list_head candidate;
>   unsigned long flags;
>  
>   /* keep last! */
> @@ -1145,6 +1146,8 @@ struct ieee80211_local {
>   u64 airtime_v_t[IEEE80211_NUM_ACS];
>   u64 airtime_weight_sum[IEEE80211_NUM_ACS];
>  
> + struct list_head remove_list[IEEE80211_NUM_ACS];
> + struct timer_list remove_timer;
>   u16 airtime_flags;
>  
>   const struct ieee80211_ops *ops;
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index e9ffa8e..78fe24a 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -667,10 +667,15 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t 
> priv_data_len,
>  
>   for (i = 0; i < IEEE80211_NUM_ACS; i++) {
>   local->active_txqs[i] = RB_ROOT_CACHED;
> + INIT_LIST_HEAD(&local->remove_list[i]);
>   spin_lock_init(&local->active_txq_lock[i]);
>   }
>   local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX;
>  
> + timer_setup(&local->remove_timer, ieee80211_txqs_check, 0);
> + mod_timer(&local->remove_timer,
> +   jiffies + 
> msecs_to_jiffies(IEEE80211_AIRTIME_TXQ_RM_CHK_INTV_IN_MS));
> +
>   INIT_LIST_HEAD(&local->chanctx_list);
>   mutex_init(&local->chanctx_mtx);
>  
> @@ -1305,6 +1310,7 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
>   tasklet_kill(&local->tx_pending_tasklet);
>   tasklet_kill(&local->tasklet);
>  
> + del_timer_sync(&local->remove_timer);
>  #ifdef CONFIG_INET
>   unregister_inetaddr_notifier(&local->ifa_notifier);
>  #endif
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index d00baaa..42ca010 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1450,6 +1450,7 @@ void ieee80211_txq_init(

Re: [PATCH 3/4] mac80211: fix low throughput in push pull mode

2019-09-17 Thread Toke Høiland-Jørgensen
Yibo Zhao  writes:

> On 2019-09-16 23:27, Johannes Berg wrote:
>> Without really looking at the code -
>> 
>>> If station is ineligible for transmission in 
>>> ieee80211_txq_may_transmit(),
>>> no packet will be delivered to FW. During the tests in push-pull mode 
>>> with
>>> many clients, after several seconds, not a single station is an 
>>> eligible
>>> candidate for transmission since global time is smaller than all the
>>> station's virtual airtime. As a consequence, the Tx has been blocked 
>>> and
>>> throughput is quite low.
>> 
>> You should rewrite this to be, erm, a bit more understandable in
>> mac80211 context. I assume you're speaking (mostly?) about ath10k, but 
>> I
>> have very little context there. "push pull mode"? "firmware"? These
>> things are not something mac80211 knows about.
> Hi Johannes,
>
> Thanks for your kindly reminder. Will rewrite the commit log.
>
>> 
>>> Co-developed-by: Yibo Zhao 
>> 
>> That also seems wrong, should be Toke I guess, unless you intended for 
>> a
>> From: Toke to be present?
> Do you mean it should be something like:
>
> Co-developed-by: Toke Høiland-Jørgensen 
> Signed-off-by: Yibo Zhao 
> Signed-off-by: Toke Høiland-Jørgensen 
>
> Am I understanding right?

I think the right thing here, as with the previous patch, is to just
drop my sign-off; you're writing this patch, and I'll add ack/reviews as
appropriate. And in that case, well, no need to have co-developed-by
yourself when your name is on the patch as author :)

-Toke



Re: [PATCH 4/4] mac80211: Sync airtime weight sum with per AC synced sta airtime weight together

2019-09-17 Thread Toke Høiland-Jørgensen
Yibo Zhao  writes:

> Global airtime weight sum is updated only when txq is added/removed
> from rbtree. If upper layer configures sta weight during high load,
> airtime weight sum will not be updated since txq is most likely on the
> tree. It could a little late for upper layer to reconfigure sta weight
> when txq is already in the rbtree. And thus, incorrect airtime weight sum
> will lead to incorrect global virtual time calculation as well as global
> airtime weight sum overflow of airtime weight sum during txq removed.
>
> Hence, need to update airtime weight sum upon receiving event for
> configuring sta weight once sta's txq is on the rbtree.
>
> Besides, if airtime weight sum of ACs and sta weight is synced under the
> same per AC lock protection, there can be a very short window causing
> incorrct airtime weight sum calculation as below:
>
> active_txq_lock_VO  .
> VO weight sum is syncd.
> sta airtime weight sum is synced  .
> active_txq_unlock_VO  .
> . .
> active_txq_lock_VI.
> VI weight sum is syncd.
> sta airtime weight sumactive_txq_lock_BE
> active_txq_unlock_VIRemove txq and thus sum
> .   is calculated with synced
> .   sta airtime weight
> . active_txq_unlock_BE
>
> So introduce a per ac synced station airtime weight synced with per
> AC synced weight sum together. And the per-AC station airtime weight
> is used to calculate weight sum.
>
> Signed-off-by: Yibo Zhao 
> ---
>  net/mac80211/cfg.c  | 27 +--
>  net/mac80211/sta_info.c |  6 --
>  net/mac80211/sta_info.h |  3 +++
>  net/mac80211/tx.c   |  4 ++--
>  4 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index d65aa01..4b420bb 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1284,7 +1284,8 @@ static int sta_apply_parameters(struct ieee80211_local 
> *local,
>   int ret = 0;
>   struct ieee80211_supported_band *sband;
>   struct ieee80211_sub_if_data *sdata = sta->sdata;
> - u32 mask, set;
> + u32 mask, set, tid, ac;
> + struct txq_info *txqi;
>  
>   sband = ieee80211_get_sband(sdata);
>   if (!sband)
> @@ -1452,8 +1453,30 @@ static int sta_apply_parameters(struct ieee80211_local 
> *local,
>   if (ieee80211_vif_is_mesh(&sdata->vif))
>   sta_apply_mesh_params(local, sta, params);
>  
> - if (params->airtime_weight)
> + if (params->airtime_weight &&
> + params->airtime_weight != sta->airtime_weight) {
> + for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
> + spin_lock_bh(&local->active_txq_lock[ac]);
> + for (tid = 0; tid < IEEE80211_NUM_TIDS + 1; tid++) {
> + if (!sta->sta.txq[tid] ||
> + ac != ieee80211_ac_from_tid(tid))
> + continue;
> +
> + sta->airtime_weight_synced[ac] =
> + params->airtime_weight;
> +
> + txqi = to_txq_info(sta->sta.txq[tid]);
> + if (RB_EMPTY_NODE(&txqi->schedule_order))
> + continue;
> +
> + local->airtime_weight_sum[ac] = 
> local->airtime_weight_sum[ac] +
> + 
> params->airtime_weight -
> + 
> sta->airtime_weight;
> + }
> + spin_unlock_bh(&local->active_txq_lock[ac]);
> + }
>   sta->airtime_weight = params->airtime_weight;

With this, airtime_weight is basically only used to return to and from
userspace, right? I.e., after the above loop has run, it will match the
contents of airtime_weight_synced; so why not just turn airtime_weight
into  a per-ac array? You could just use airtime_weight[0] as the value
to return to userspace...

-Toke



Re: [PATCH 1/4] mac80211: Switch to a virtual time-based airtime scheduler

2019-09-17 Thread Toke Høiland-Jørgensen
Yibo Zhao  writes:

> From: Toke Høiland-Jørgensen 
>
> This switches the airtime scheduler in mac80211 to use a virtual time-based
> scheduler instead of the round-robin scheduler used before. This has a
> couple of advantages:
>
> - No need to sync up the round-robin scheduler in firmware/hardware with
>   the round-robin airtime scheduler.
>
> - If several stations are eligible for transmission we can schedule both of
>   them; no need to hard-block the scheduling rotation until the head of the
>   queue has used up its quantum.
>
> - The check of whether a station is eligible for transmission becomes
>   simpler (in ieee80211_txq_may_transmit()).
>
> The drawback is that scheduling becomes slightly more expensive, as we need
> to maintain an rbtree of TXQs sorted by virtual time. This means that
> ieee80211_register_airtime() becomes O(logN) in the number of currently
> scheduled TXQs. However, hopefully this number rarely grows too big (it's
> only TXQs currently backlogged, not all associated stations), so it
> shouldn't be too big of an issue.
>
> Signed-off-by: Toke Høiland-Jørgensen 

I'll note that this patch still has the two issues that Felix pointed
out when I posted the RFC version. Namely:

- The use of divisions in the fast path. I guess I need to go write some
  reciprocal-calculation code, since that is also an issue with the AQL
  patches I linked to before.

- The fact that we don't count the airtime usage of multicast traffic,
  which with this series means that the vif TXQ will get priority over
  the others. I think we agreed to fix this by just adding an airtime
  v_t to the vif as well and use that for scheduling the TXQ. Does
  ath10k report airtime usage for multicast as well, or only for
  stations?


-Toke