Re: [PATCH] ath10k: remove TX lock from ath10k_htt_tx_inc_pending
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
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
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
+ 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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()
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
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
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"
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"
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
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
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.
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.
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
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
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
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
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