Re: [PATCH 03/12] brcmfmac: move pno helper functions in separate source file
On Tue, 2016-11-29 at 08:08 +0100, Rafał Miłecki wrote: > On 23 November 2016 at 11:25, Arend van Spriel > wrote: > > > > Introducing new source file for pno related functionality. Moving > > existing pno functions. > > Let me ask one basic question as I'm curious: what that PNO stands > for? I couldn't find it explained in the code. It's an Android term - Preferred Network Offload(ing?) johannes
Re: [PATCH 00/12] brcmfmac: scheduled scan cleanup and chip support
On 23 November 2016 at 11:25, Arend van Spriel wrote: > This patch series contains: > * cleanup of scheduled scan code. > * fix handling schedules scan results for newer chips. > * support for bcm43341 chipset with different chip id. > * support rev6 of PCIe device interface. > > The series is intended for 4.10 and applies to the master branch > of the wireless-drivers-next repository. Tested with BCM43602 on SR400ac router, it still initializes fine, I can create multiple AP interfaces and use them.
Re: Deadlock in hacked 4.9.0-rc6+ kernel.
Hi Ben, On Mon, Nov 28, 2016 at 10:52:44AM -0800, Ben Greear wrote: > I ported forward my patch set to the 4.9 kernel, and I am seeing lockups > fairly > often. As always, could be something I added locally, but in case someone > sees > similar, then maybe I can reproduce it quicker and help track this down since > my > test config uses many virtual stations and virtual APs. [shafi] does this happens with a firmware crash (or) hardware restart ?, please let us know, we have a potential fix for the same and we will send the fix for linux wireless review soon, else please ignore. > > And, if someone knows the magic trick to make dmesg output from lockdep > not be so split up, please let me know. > > > [81871.799595] ath10k_pci :05:00.0: boot warm reset complete > [81873.983645] sta559: Failed to send nullfunc to AP 04:f0:21:0f:3c:3c after > 1000ms, disconnecting > [81873.991198] ath10k_pci :05:00.0: mac ampdu vdev_id 49 sta > 04:f0:21:0f:3c:3c tid 6 action 1 > [82195.484265] INFO: task mission-control:1506 blocked for more than 180 > seconds. > [82195.490228] Tainted: GW 4.9.0-rc6+ #9 > [82195.494367] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > this message. > [82195.500922] mission-control D0 1506 1 0x > [82195.505152] 8801f1c3a300 2629 880214de2640 > 8801f2f38000 > [82195.511330] 88021e3198d8 c90003867c80 819322eb > 0006 > [82195.518230] 8801f2f38828 03867c78 88021e3198d8 > > [82195.525121] Call Trace: > [82195.526300] [] ? __schedule+0x2fb/0xb30 > [82195.530862] [] schedule+0x38/0x90 > [82195.534552] [] schedule_preempt_disabled+0x10/0x20 > [82195.540073] [] mutex_lock_nested+0x175/0x3f0 > [82195.544723] [] ? rtnetlink_rcv+0x16/0x30 > [82195.549424] [] rtnetlink_rcv+0x16/0x30 > [82195.553555] [] netlink_unicast+0x1cd/0x2e0 > [82195.558414] [] ? netlink_unicast+0x149/0x2e0 > [82195.563062] [] netlink_sendmsg+0x2e2/0x390 > [82195.567889] [] ? __fget+0x108/0x1f0 > [82195.571755] [] sock_sendmsg+0x33/0x40 > [82195.575791] [] SYSC_sendto+0x101/0x190 > [82195.580258] [] ? security_file_permission+0x96/0xb0 > [82195.585521] [] ? rw_verify_area+0x49/0xb0 > [82195.589909] [] ? trace_hardirqs_on_caller+0x129/0x1b0 > [82195.595364] [] ? trace_hardirqs_on_thunk+0x1a/0x1c > [82195.600538] [] SyS_sendto+0x9/0x10 > [82195.604319] [] entry_SYSCALL_64_fastpath+0x23/0xc6 > [82195.609502] > Showing all locks held in the system: > [82195.613134] 2 locks held by khungtaskd/37: > [82195.615958] #0: > [82195.616408] ( > [82195.616802] rcu_read_lock > [82195.617946] ){..} > [82195.618983] , at: > [82195.619522] [] watchdog+0x9c/0x600 > [82195.623219] #1: > [82195.623666] ( > [82195.624060] tasklist_lock > [82195.625208] ){.+.+..} > [82195.626213] , at: > [82195.626748] [] debug_show_all_locks+0x3d/0x1a0 > [82195.631513] 2 locks held by agetty/1149: > [82195.634160] #0: > [82195.634610] ( > [82195.635001] &tty->ldisc_sem > [82195.636320] ){.+} > [82195.637323] , at: > [82195.637859] [] ldsem_down_read+0x2d/0x40 > [82195.642077] #1: > [82195.642528] ( > [82195.642922] &ldata->atomic_read_lock > [82195.645027] ){+.+...} > [82195.646030] , at: > [82195.646569] [] n_tty_read+0xa9/0x910 > [82195.650442] 1 lock held by mission-control/1506: > [82195.653800] #0: > [82195.654249] ( > [82195.654645] rtnl_mutex > [82195.655527] ){+.+.+.} > [82195.656534] , at: > [82195.657068] [] rtnetlink_rcv+0x16/0x30 > [82195.661113] 2 locks held by bash/1691: > [82195.663586] #0: > [82195.664033] ( > [82195.664433] &tty->ldisc_sem > [82195.665750] ){.+} > [82195.666754] , at: > [82195.667294] [] ldsem_down_read+0x2d/0x40 > [82195.671509] #1: > [82195.671954] ( > [82195.672349] &ldata->atomic_read_lock > [82195.674447] ){+.+...} > [82195.675451] , at: > [82195.675987] [] n_tty_read+0xa9/0x910 > [82195.679857] 1 lock held by evolution-calen/1768: > [82195.683200] #0: > [82195.683649] ( > [82195.684040] rtnl_mutex > [82195.684925] ){+.+.+.} > [82195.685930] , at: > [82195.686467] [] rtnetlink_rcv+0x16/0x30 > [82195.690514] 2 locks held by hostapd/4100: > [82195.693249] #0: > [82195.693694] ( > [82195.694090] cb_lock > [82195.694711] ){++} > [82195.695741] , at: > [82195.696279] [] genl_rcv+0x14/0x40 > [82195.699888] #1: > [82195.700338] ( > [82195.700731] genl_mutex > [82195.701616] ){+.+.+.} > [82195.702619] , at: > [82195.703155] [] genl_rcv_msg+0xa4/0xb0 > [82195.707113] 2 locks held by hostapd/4392: > [82195.709847] #0: > [82195.710301] ( > [82195.710695] cb_lock > [82195.711317] ){++} > [82195.712324] , at: > [82195.712857] [] genl_rcv+0x14/0x40 > [82195.716467] #1: > [82195.716911] ( > [82195.717306] genl_mutex > [82195.718186] ){+.+.+.} > [82195.719193] , at: > [82195.719727] [] genl_rcv_msg+0xa4/0xb0 > [82195.723682] 3 locks held by wpa_supplicant/4574: > [82195.727028] #0: > [82195.727480] ( > [82195.727871] cb_lock >
Re: [PATCH 03/12] brcmfmac: move pno helper functions in separate source file
On 23 November 2016 at 11:25, Arend van Spriel wrote: > Introducing new source file for pno related functionality. Moving > existing pno functions. Let me ask one basic question as I'm curious: what that PNO stands for? I couldn't find it explained in the code.
Re: [PATCH] rtl8xxxu: Fix fail to reconnect to AP
Barry Day writes: > Removed the report_connect functions as the h2c commands used are > not required for a soft-mac driver and they prevent reconnecting to an > AP for a couple of the chipsets. > > Signed-off-by: Barry Day > --- > rtl8xxxu.h | 6 -- > rtl8xxxu_8192c.c | 1 - > rtl8xxxu_8192e.c | 1 - > rtl8xxxu_8723a.c | 1 - > rtl8xxxu_8723b.c | 1 - > rtl8xxxu_core.c | 36 > 6 files changed, 46 deletions(-) Hi Barry, Does removing the h2c command on the 8723bu and 8192eu make the reconnect process work properly? Do you have any documentation justifying why this isn't needed on gen1 parts? I am rather cautious of just removing them, but we may remove the call for gen2, at least until we understand better how that firmware command really works. Cheers, Jes > diff --git a/rtl8xxxu.h b/rtl8xxxu.h > index df551b2..3b1f62d 100644 > --- a/rtl8xxxu.h > +++ b/rtl8xxxu.h > @@ -1335,8 +1335,6 @@ struct rtl8xxxu_fileops { > bool ht40); > void (*update_rate_mask) (struct rtl8xxxu_priv *priv, > u32 ramask, int sgi); > - void (*report_connect) (struct rtl8xxxu_priv *priv, > - u8 macid, bool connect); > void (*fill_txdesc) (struct ieee80211_hw *hw, struct ieee80211_hdr *hdr, >struct ieee80211_tx_info *tx_info, >struct rtl8xxxu_txdesc32 *tx_desc, bool sgi, > @@ -1422,10 +1420,6 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv > *priv, > u32 ramask, int sgi); > void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv, > u32 ramask, int sgi); > -void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv, > - u8 macid, bool connect); > -void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv, > - u8 macid, bool connect); > void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv); > void rtl8xxxu_gen1_enable_rf(struct rtl8xxxu_priv *priv); > void rtl8xxxu_gen1_disable_rf(struct rtl8xxxu_priv *priv); > diff --git a/rtl8xxxu_8192c.c b/rtl8xxxu_8192c.c > index f9e2050..d5c37e0 100644 > --- a/rtl8xxxu_8192c.c > +++ b/rtl8xxxu_8192c.c > @@ -566,7 +566,6 @@ struct rtl8xxxu_fileops rtl8192cu_fops = { > .usb_quirks = rtl8xxxu_gen1_usb_quirks, > .set_tx_power = rtl8xxxu_gen1_set_tx_power, > .update_rate_mask = rtl8xxxu_update_rate_mask, > - .report_connect = rtl8xxxu_gen1_report_connect, > .fill_txdesc = rtl8xxxu_fill_txdesc_v1, > .writeN_block_size = 128, > .rx_agg_buf_size = 16000, > diff --git a/rtl8xxxu_8192e.c b/rtl8xxxu_8192e.c > index a1178c5..401aac4 100644 > --- a/rtl8xxxu_8192e.c > +++ b/rtl8xxxu_8192e.c > @@ -1648,7 +1648,6 @@ struct rtl8xxxu_fileops rtl8192eu_fops = { > .usb_quirks = rtl8xxxu_gen2_usb_quirks, > .set_tx_power = rtl8192e_set_tx_power, > .update_rate_mask = rtl8xxxu_gen2_update_rate_mask, > - .report_connect = rtl8xxxu_gen2_report_connect, > .fill_txdesc = rtl8xxxu_fill_txdesc_v2, > .writeN_block_size = 128, > .tx_desc_size = sizeof(struct rtl8xxxu_txdesc40), > diff --git a/rtl8xxxu_8723a.c b/rtl8xxxu_8723a.c > index aef3730..9c13db5 100644 > --- a/rtl8xxxu_8723a.c > +++ b/rtl8xxxu_8723a.c > @@ -383,7 +383,6 @@ struct rtl8xxxu_fileops rtl8723au_fops = { > .usb_quirks = rtl8xxxu_gen1_usb_quirks, > .set_tx_power = rtl8xxxu_gen1_set_tx_power, > .update_rate_mask = rtl8xxxu_update_rate_mask, > - .report_connect = rtl8xxxu_gen1_report_connect, > .fill_txdesc = rtl8xxxu_fill_txdesc_v1, > .writeN_block_size = 1024, > .rx_agg_buf_size = 16000, > diff --git a/rtl8xxxu_8723b.c b/rtl8xxxu_8723b.c > index 02b8ddd..30dc66e 100644 > --- a/rtl8xxxu_8723b.c > +++ b/rtl8xxxu_8723b.c > @@ -1665,7 +1665,6 @@ struct rtl8xxxu_fileops rtl8723bu_fops = { > .usb_quirks = rtl8xxxu_gen2_usb_quirks, > .set_tx_power = rtl8723b_set_tx_power, > .update_rate_mask = rtl8xxxu_gen2_update_rate_mask, > - .report_connect = rtl8xxxu_gen2_report_connect, > .fill_txdesc = rtl8xxxu_fill_txdesc_v2, > .writeN_block_size = 1024, > .tx_desc_size = sizeof(struct rtl8xxxu_txdesc40), > diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c > index a9137ab..03e88d2 100644 > --- a/rtl8xxxu_core.c > +++ b/rtl8xxxu_core.c > @@ -4352,39 +4352,6 @@ void rtl8xxxu_gen2_update_rate_mask(struct > rtl8xxxu_priv *priv, > rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.b_macid_cfg)); > } > > -void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv, > - u8 macid, bool connect) > -{ > - struct h2c_cmd h2c; > - > - memset(&h2c, 0, sizeof(struct h2c_cmd)); > - > - h2c.joinbss.cmd = H2C_JOIN_BSS_REPORT; > - > - if (connect) > - h2c.joinbss.data = H2C_JOIN_BSS_CONNECT; > - else > -
Re: [RFC] qtn: add FullMAC firmware for Quantenna QSR10G wifi device
Am 28.11.2016 um 20:01 schrieb IgorMitsyanko: > On 11/28/2016 08:33 PM, Oleksij Rempel wrote: >> Am 28.11.2016 um 18:10 schrieb Oleksij Rempel: >>> Am 28.11.2016 um 17:34 schrieb Kyle McMartin: On Tue, Nov 22, 2016 at 9:44 AM, IgorMitsyanko wrote: > Hi Ben, Kyle, > could you please share what is the position of linux-firmware > regarding > firmware binaries that include GPL components? Does it require > entire GPL > components codebase be present in linux-firmware tree, or maybe > having this > clause in license file is enough: > +Open Source Software. The Software may include components that are > licensed > +pursuant to open source software (“Open Source Components”). > Information > +regarding the Open Source Components included with the Software is > available > +upon request to osle...@quantenna.com. To the extent such Open Source > +Components are required to be licensed to you under the terms of a > separate > +license (such as an open source license) then such other terms > shall apply, > and > +nothing herein shall be deemed or interpreted to limit any rights > you may > have > +under any such applicable license. > > From technical perspective, size of the codebase used to build > Quantenna > firmware is a few hundred MBs, it seems too much to include into > linux-firmware tree. > I don't have strong feelings one way or another. I'd prefer not having several hundred MB of source that's unlikely to change included in the linux-firmware git tree. I'm also not a lawyer, so I can't help you decide what would satisfy the distribution clause of the GPLv2. We already have one GPL firmware (carl9170fw) which includes the source, but just references a seperate toolchain for downloading, so it's only approximately 1MB in size in the tree. Is your firmware source really that large, or is it just including the entire build toolchain with it? regards, --Kyle >>> We also have open BSD licensed open-ath9k-htc-firmware. Which is locate >>> out of source too. >>> https://github.com/qca/open-ath9k-htc-firmware >>> and here is location of carl firmware: >>> https://github.com/chunkeey/carl9170fw >>> >>> So, what is actual problem with Quantenna QSR10G FW? >>> I would be really interesting to take a look on it. Is it somewhere >>> available? Are there some devices to get hand on? >> After seeing specs of this device i have strong feeling that "some open >> source part" is actual linux kernel. >> >> > Oleksij, yes, that's correct, it includes entire Linux environment; the > reasoning is that it allows to hide all WiFi-related logic inside device > itself, and emulate simple Ethernet device for external system > (therefore, freeing external system resources). > > This approach was working really well for us until recently, but now > that company is expanding, we want to have more flexible and standardize > interface available for external system to manage wireless connection, > and FullMAC driver seems to be the best solution here. you mean, this driver will not use mac80211 framework provided by kernel? > For the availability of FW sources, QSR10G-based products are still > under development at this moment (not in the market yet), but many > products based on previous generation chipset QSR1000 are available. For > example, Asus has a retail design with QSR1000 chipset, and has all GPL > sourcecode available on their website (including what Quantenna has > provided): > > http://www.asus.com/Networking/RTAC87U/HelpDesk_Download/ > Quantenna provided code is in, for example, "GPL of ASUS RT-AC87U for > firmware 3.0.0.4.378.7410" archive. > It's basically the same as used for QSR10G. Will Quantenna provide documentation for at least old chipsats? Playing fair with OSS developer community has some advantages :) -- Regards, Oleksij signature.asc Description: OpenPGP digital signature
[PATCH] rtl8xxxu: Fix fail to reconnect to AP
Removed the report_connect functions as the h2c commands used are not required for a soft-mac driver and they prevent reconnecting to an AP for a couple of the chipsets. Signed-off-by: Barry Day --- rtl8xxxu.h | 6 -- rtl8xxxu_8192c.c | 1 - rtl8xxxu_8192e.c | 1 - rtl8xxxu_8723a.c | 1 - rtl8xxxu_8723b.c | 1 - rtl8xxxu_core.c | 36 6 files changed, 46 deletions(-) diff --git a/rtl8xxxu.h b/rtl8xxxu.h index df551b2..3b1f62d 100644 --- a/rtl8xxxu.h +++ b/rtl8xxxu.h @@ -1335,8 +1335,6 @@ struct rtl8xxxu_fileops { bool ht40); void (*update_rate_mask) (struct rtl8xxxu_priv *priv, u32 ramask, int sgi); - void (*report_connect) (struct rtl8xxxu_priv *priv, - u8 macid, bool connect); void (*fill_txdesc) (struct ieee80211_hw *hw, struct ieee80211_hdr *hdr, struct ieee80211_tx_info *tx_info, struct rtl8xxxu_txdesc32 *tx_desc, bool sgi, @@ -1422,10 +1420,6 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi); void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi); -void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv, - u8 macid, bool connect); -void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv, - u8 macid, bool connect); void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv); void rtl8xxxu_gen1_enable_rf(struct rtl8xxxu_priv *priv); void rtl8xxxu_gen1_disable_rf(struct rtl8xxxu_priv *priv); diff --git a/rtl8xxxu_8192c.c b/rtl8xxxu_8192c.c index f9e2050..d5c37e0 100644 --- a/rtl8xxxu_8192c.c +++ b/rtl8xxxu_8192c.c @@ -566,7 +566,6 @@ struct rtl8xxxu_fileops rtl8192cu_fops = { .usb_quirks = rtl8xxxu_gen1_usb_quirks, .set_tx_power = rtl8xxxu_gen1_set_tx_power, .update_rate_mask = rtl8xxxu_update_rate_mask, - .report_connect = rtl8xxxu_gen1_report_connect, .fill_txdesc = rtl8xxxu_fill_txdesc_v1, .writeN_block_size = 128, .rx_agg_buf_size = 16000, diff --git a/rtl8xxxu_8192e.c b/rtl8xxxu_8192e.c index a1178c5..401aac4 100644 --- a/rtl8xxxu_8192e.c +++ b/rtl8xxxu_8192e.c @@ -1648,7 +1648,6 @@ struct rtl8xxxu_fileops rtl8192eu_fops = { .usb_quirks = rtl8xxxu_gen2_usb_quirks, .set_tx_power = rtl8192e_set_tx_power, .update_rate_mask = rtl8xxxu_gen2_update_rate_mask, - .report_connect = rtl8xxxu_gen2_report_connect, .fill_txdesc = rtl8xxxu_fill_txdesc_v2, .writeN_block_size = 128, .tx_desc_size = sizeof(struct rtl8xxxu_txdesc40), diff --git a/rtl8xxxu_8723a.c b/rtl8xxxu_8723a.c index aef3730..9c13db5 100644 --- a/rtl8xxxu_8723a.c +++ b/rtl8xxxu_8723a.c @@ -383,7 +383,6 @@ struct rtl8xxxu_fileops rtl8723au_fops = { .usb_quirks = rtl8xxxu_gen1_usb_quirks, .set_tx_power = rtl8xxxu_gen1_set_tx_power, .update_rate_mask = rtl8xxxu_update_rate_mask, - .report_connect = rtl8xxxu_gen1_report_connect, .fill_txdesc = rtl8xxxu_fill_txdesc_v1, .writeN_block_size = 1024, .rx_agg_buf_size = 16000, diff --git a/rtl8xxxu_8723b.c b/rtl8xxxu_8723b.c index 02b8ddd..30dc66e 100644 --- a/rtl8xxxu_8723b.c +++ b/rtl8xxxu_8723b.c @@ -1665,7 +1665,6 @@ struct rtl8xxxu_fileops rtl8723bu_fops = { .usb_quirks = rtl8xxxu_gen2_usb_quirks, .set_tx_power = rtl8723b_set_tx_power, .update_rate_mask = rtl8xxxu_gen2_update_rate_mask, - .report_connect = rtl8xxxu_gen2_report_connect, .fill_txdesc = rtl8xxxu_fill_txdesc_v2, .writeN_block_size = 1024, .tx_desc_size = sizeof(struct rtl8xxxu_txdesc40), diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c index a9137ab..03e88d2 100644 --- a/rtl8xxxu_core.c +++ b/rtl8xxxu_core.c @@ -4352,39 +4352,6 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv, rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.b_macid_cfg)); } -void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv, - u8 macid, bool connect) -{ - struct h2c_cmd h2c; - - memset(&h2c, 0, sizeof(struct h2c_cmd)); - - h2c.joinbss.cmd = H2C_JOIN_BSS_REPORT; - - if (connect) - h2c.joinbss.data = H2C_JOIN_BSS_CONNECT; - else - h2c.joinbss.data = H2C_JOIN_BSS_DISCONNECT; - - rtl8xxxu_gen1_h2c_cmd(priv, &h2c, sizeof(h2c.joinbss)); -} - -void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv, - u8 macid, bool connect) -{ - struct h2c_cmd h2c; - - memset(&h2c, 0, sizeof(struct h2c_cmd)); - - h2c.media_status_rpt.cmd = H2C_8723B_MEDIA_STATUS_RPT; - if (connect) - h2c.media_status_rpt.parm |= BIT(0); - else -
CRDA/wireless-regdb to limit communication to certified countries
Hello, I'm in need of a way to meet various levels of certification based on the country the device is operating in. For example, let's say the device will be certified for 2.4 GHz and 5GHz in the US but in Japan it will be limited to 2.4GHz. In order to do so, I've modified wireless-regdb used by CRDA and it seems to work okay by country once associated with an AP, but I still have a few questions (mostly related to clarification of information from https://wireless.wiki.kernel.org/en/developers/regulatory/processing_rules). 1) The NO-IR flag prevents a device from initiating radiation on a channel, but supposedly it can be lifted while world roaming if an AP is broadcasting on that channel. Suppose my device is world roaming (country code 00) and all channels in country 00 are set to NO-IR. What happens if my device notices an AP beaconing with country IE of Japan on a channel that was originally NO-IR? A country IE is said to be ignored until we try to associate with it, will the device now allow initiation of radiation on the channels which the AP was beaconing on? 2) I noticed in country 00 that the power of 60GHz channels were set to 0. Is this a better way of truly disabling transmission on certain frequencies until a Country Information Element is received from an AP? The main goal is to follow our certification by country correctly, and allow no transmissions that would violate certifications. Thanks for any and all input! Brandon Please be advised that this email may contain confidential information. If you are not the intended recipient, please notify us by email by replying to the sender and delete this message. The sender disclaims that the content of this email constitutes an offer to enter into, or the acceptance of, any agreement; provided that the foregoing does not invalidate the binding effect of any digital or other electronic reproduction of a manual signature that is included in any attachment.
Re: [PATCH 0/7] rtl8xxxu: Pending patches
Barry Day writes: > On Mon, Nov 21, 2016 at 11:59:47AM -0500, Jes Sorensen wrote: >> Barry Day writes: >> > Testing is simple. Connect to an AP in the usual >> > manner..disconnect..reconnect. The 8192eu will fail to reconnect and >> > John Heenan reported the 8723bu also fails to reconnect. Even though >> > he was directly stopping and restarting wpa_supplicant, it's the same >> > thing to the driver - connect..disconnect..reconnect. >> >> Thanks for the details - I'll have a look shortly. >> >> > With using a usb_device pointer, each message starts with the usb bus >> > address. Plug it into a different port and that address could >> > change. By using a pointer to the device associated with the wiphy >> > each message will begin with the driver name. Just makes it easier for >> > the average user to report what's in the log because he can just grep >> > for "rtl8xxxu". >> >> I see - that would be problematic for me as I quite often have 3-4 of >> these things plugged in at the same time. Not knowing which port spits >> out the message would make it a lot harder to track. In fact my primary >> devel box for this (Lenovo Yoga 13) has an rtl8723au soldered on the >> motherboard, so the moment I plug in any other dongle I'll have two. >> >> The alternative would be to add a prefer to the individual messages. >> >> Cheers, >> Jes > > I should have mentioned it also places the usb address after the driver name. If you're willing to cook up a patch, I'll have a look at it - I want to see how it behaves though before deciding whether to approve it. Cheers, Jes
OOM with pktgen when transmitting on ath10k station in 4.7.10+ kernel
When I over-drive ath10k station using pktgen, system goes OOM quickly. If I catch it in time and stop pktgen, then memory is recovered over the next few seconds. I assume that there is an un-bounded queue somewhere. But, I cannot find any queues in the mac80211 tx path that apply to ath10k (as far as I can tell). The one possibility is the pending queues, but printing them out with debugfs shows them all zeros, and I have some code to bound them at 1000 pkts anyway. I didn't see anything obvious in ath10k either, but I must be missing something... pktgen transmits under the queue logic, so it will likely be ignoring any queue-stopped signals from mac80211. Are there any other places that pkts can be queued()? Thanks, Ben -- Ben Greear Candela Technologies Inc http://www.candelatech.com
Re: [RFC] qtn: add FullMAC firmware for Quantenna QSR10G wifi device
On 11/28/2016 07:34 PM, Kyle McMartin wrote: On Tue, Nov 22, 2016 at 9:44 AM, IgorMitsyanko wrote: Hi Ben, Kyle, could you please share what is the position of linux-firmware regarding firmware binaries that include GPL components? Does it require entire GPL components codebase be present in linux-firmware tree, or maybe having this clause in license file is enough: +Open Source Software. The Software may include components that are licensed +pursuant to open source software (“Open Source Components”). Information +regarding the Open Source Components included with the Software is available +upon request to osle...@quantenna.com. To the extent such Open Source +Components are required to be licensed to you under the terms of a separate +license (such as an open source license) then such other terms shall apply, and +nothing herein shall be deemed or interpreted to limit any rights you may have +under any such applicable license. From technical perspective, size of the codebase used to build Quantenna firmware is a few hundred MBs, it seems too much to include into linux-firmware tree. I don't have strong feelings one way or another. I'd prefer not having several hundred MB of source that's unlikely to change included in the linux-firmware git tree. I'm also not a lawyer, so I can't help you decide what would satisfy the distribution clause of the GPLv2. We already have one GPL firmware (carl9170fw) which includes the source, but just references a seperate toolchain for downloading, so it's only approximately 1MB in size in the tree. Is your firmware source really that large, or is it just including the entire build toolchain with it? It does include Linux environment with userspace tools, toolchain and some other components that are not actually needed, you're right. Maybe its possible to skip providing entire build environment, but simply provide GPL code and patches to 3-d party opensource components? Just state which version the patch is based on: for example, patch to apply on top of Linux 3.14 sources, but no sources themselves. In this case, it won't be possible to build firmware manually, but GPL code modifications will be available. regards, --Kyle On 11/11/2016 02:35 PM, Johannes Berg wrote: Adding linux-firmware people to Cc, since presumably they don't necessarily read linux-wireless... Johannes, from that perspective, who are the "redistributors"? Specifically, is linux-firmware git repository considered a redistributor or its just hosting files? I mean, at what moment someone else other then Quantenna will start to be legally obliged to make GPL code used in firmware available for others? Look, I don't know. I'd assume people who ship it, like any regular distro, would be (re)distributors thereof. "Normal" (non-GPL) firmware images come with a redistribution license, but that obviously can't work here. There's some info from Ben here regarding the carl9170 case: http://lkml.iu.edu/hypermail/linux/kernel/1605.3/01176.html Personally I still hope that linux-firmware itself is not legally concerned with what is the content of firmware its hosting, but looks like there already was a precedent case with carl9170 driver and we have to somehow deal with it. That's really all I wanted to bring up. I'm not involved with the linux-firmware git tree. There still may be a difference though: Quantenna is semiconductor company only, software used on actual products based on Quantenna chipsets is released by other companies. I just want to present our legal team with a clear case (and position of Linux maintainers) so that they can work with it and make decision on how to proceed. From technical perspective, as I mentioned, SDK is quite huge and include a lot of opensource components including full Linux, I don't think its reasonable to have it inside linux-firmware tree. What are the options to share it other then providing it on request basis: - git repository - store tarball somewhere on official website Clearly that wasn't deemed appropriate for carl9170, so I don't see why it'd be different here. johannes
Re: [PATCH v3 4/5] mwifiex: wait firmware dump complete during card remove process
Hi Amit, On Thu, Nov 24, 2016 at 12:14:07PM +, Amitkumar Karwar wrote: > > From: Brian Norris [mailto:briannor...@chromium.org] > > Sent: Monday, November 21, 2016 11:06 PM > > To: Amitkumar Karwar > > Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam; > > raja...@google.com; dmitry.torok...@gmail.com; Xinming Hu > > Subject: Re: [PATCH v3 4/5] mwifiex: wait firmware dump complete during > > card remove process > > > > On Wed, Nov 16, 2016 at 06:39:08PM +0530, Amitkumar Karwar wrote: > > > From: Xinming Hu > > > > > > Wait for firmware dump complete in card remove function. > > > For sdio interface, there are two diffenrent cases, card reset > > trigger > > > sdio_work and firmware dump trigger sdio_work. > > > Do code rearrangement for distinguish between these two cases. > > > > On second review of the SDIO card reset code (which I'll repeat is > > quite ugly), you seem to be making a bad distinction here. What if > > there is a firmware dump happening concurrently with your card-reset > > handling? > > You *do* want to synchronize with the firmware dump before completing the > > card reset, or else you might be freeing up internal card resources > > that are still in use. See below. > > I ran some tests and observed that if same work function is scheduled > by two threads, it won't have re-entrant calls. They will be executed > one after another. In SDIO work function, we have SDIO card reset call > after completing firmware dump. So firmware dump won't run > concurrently with card-reset as per my understanding. Ah, you're correct. It's somewhat obscure and potentially fragile, but correct AFAICT. As you noted though, you do still have a use-after-free bug, even if the concurrency isn't quite as high as I thought. > > > Signed-off-by: Xinming Hu > > > Signed-off-by: Amitkumar Karwar > > > --- ... > > > --- a/drivers/net/wireless/marvell/mwifiex/sdio.c > > > +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c > > > @@ -46,6 +46,9 @@ > > > */ > > > static u8 user_rmmod; > > > > > > +static void mwifiex_sdio_work(struct work_struct *work); static > > > +DECLARE_WORK(sdio_work, mwifiex_sdio_work); > > > + > > > static struct mwifiex_if_ops sdio_ops; static unsigned long > > > iface_work_flags; > > > > > > @@ -220,7 +223,7 @@ static int mwifiex_sdio_resume(struct device > > *dev) > > > * This function removes the interface and frees up the card > > structure. > > > */ > > > static void > > > -mwifiex_sdio_remove(struct sdio_func *func) > > > +__mwifiex_sdio_remove(struct sdio_func *func) > > > { > > > struct sdio_mmc_card *card; > > > struct mwifiex_adapter *adapter; > > > @@ -249,6 +252,13 @@ static int mwifiex_sdio_resume(struct device > > *dev) > > > mwifiex_remove_card(adapter); > > > } > > > > > > +static void > > > +mwifiex_sdio_remove(struct sdio_func *func) { > > > + cancel_work_sync(&sdio_work); > > > + __mwifiex_sdio_remove(func); > > > +} > > > + > > > /* > > > * SDIO suspend. > > > * > > > @@ -2227,7 +2237,7 @@ static void mwifiex_recreate_adapter(struct > > sdio_mmc_card *card) > > >* discovered and initializes them from scratch. > > >*/ > > > > > > - mwifiex_sdio_remove(func); > > > + __mwifiex_sdio_remove(func); > > > > ^^ So here, you're trying to avoid syncing with the card-reset work > > event, except that function will free up all your resources (including > > the static save_adapter). Thus, you're explicitly allowing a use-after- > > free error here. That seems unwise. > > Even if firmware dump is triggered after card reset is started, it > will execute after card reset is completed as discussed above. Only > problem I can see is with "save_adapter". We can put new_adapter > pointer in "save_adapter" at the end of mwifiex_recreate_adapter() to > solve the issue. Ugh, yet another band-aid? You might do better to still cancel any pending work, just don't do it synchronously. i.e., do cancel_work() after you've removed the card. It doesn't make sense to do a FW dump on the "new" adapter when it was requested for the old one. > > Instead, you should actually retain the invariant that you're doing a > > full remove/reinitialize here, which includes doing the *same* > > cancel_work_sync() here in mwifiex_recreate_adapter() as you would in > > any other remove(). > > We are executing mwifiex_recreate_adapter() as a part of sdio_work(). Calling > cancel_work_sync() here would cause deadlock. The API is supposed to waits > until sdio_work() is finished. You are correct. So using the _sync() version would be wrong. > > > > IOW, kill the __mwifiex_sdio_remove() and just call > > mwifiex_sdio_remove() as you were. > > > > That also means that you can do the same per-adapter cleanup in the > > following patch as you do for PCIe. > > Currently as sdio_work recreates "card", the work structure can't be moved > inside card structure. > Let me know your suggestions. If you address the TODO in mwifiex_create_adapter() instead, you can
Re: [PATCH] rtl8xxxu: fix tx rate debug output
Arnd Bergmann writes: > We accidentally print the rate before we know it for txdesc_v2: Hi Arnd, Thanks for the patch - Barry Day already posted a patch for this which Kalle has applied to the wireless tree. Cheers, Jes > > wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In function > 'rtl8xxxu_fill_txdesc_v2': > wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:4848:3: error: 'rate' may be used > uninitialized in this function [-Werror=maybe-uninitialized] > > txdesc_v1 got it right, so let's do it the same way here. > > Fixes: b4c3d9cfb607 ("rtl8xxxu: Pass tx_info to fill_txdesc in order to have > access to retry count") > Signed-off-by: Arnd Bergmann > --- > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > index 04141e57b8ae..a9137abc3ad9 100644 > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > @@ -4844,16 +4844,16 @@ rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, > struct ieee80211_hdr *hdr, > > tx_desc40 = (struct rtl8xxxu_txdesc40 *)tx_desc32; > > - if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX) > - dev_info(dev, "%s: TX rate: %d, pkt size %d\n", > - __func__, rate, cpu_to_le16(tx_desc40->pkt_size)); > - > if (rate_flags & IEEE80211_TX_RC_MCS && > !ieee80211_is_mgmt(hdr->frame_control)) > rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0; > else > rate = tx_rate->hw_value; > > + if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX) > + dev_info(dev, "%s: TX rate: %d, pkt size %d\n", > + __func__, rate, cpu_to_le16(tx_desc40->pkt_size)); > + > seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl)); > > tx_desc40->txdw4 = cpu_to_le32(rate);
[PATCH] rtl8xxxu: fix tx rate debug output
We accidentally print the rate before we know it for txdesc_v2: wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In function 'rtl8xxxu_fill_txdesc_v2': wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:4848:3: error: 'rate' may be used uninitialized in this function [-Werror=maybe-uninitialized] txdesc_v1 got it right, so let's do it the same way here. Fixes: b4c3d9cfb607 ("rtl8xxxu: Pass tx_info to fill_txdesc in order to have access to retry count") Signed-off-by: Arnd Bergmann --- drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c index 04141e57b8ae..a9137abc3ad9 100644 --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c @@ -4844,16 +4844,16 @@ rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr, tx_desc40 = (struct rtl8xxxu_txdesc40 *)tx_desc32; - if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX) - dev_info(dev, "%s: TX rate: %d, pkt size %d\n", -__func__, rate, cpu_to_le16(tx_desc40->pkt_size)); - if (rate_flags & IEEE80211_TX_RC_MCS && !ieee80211_is_mgmt(hdr->frame_control)) rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0; else rate = tx_rate->hw_value; + if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX) + dev_info(dev, "%s: TX rate: %d, pkt size %d\n", +__func__, rate, cpu_to_le16(tx_desc40->pkt_size)); + seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl)); tx_desc40->txdw4 = cpu_to_le32(rate); -- 2.9.0
Re: [RFC V2 4/5] nl80211: add support for gscan
On 28-11-2016 15:38, Johannes Berg wrote: > >> * the nl80211 feature flags for the device. >> + * @NL80211_SCAN_FLAGS_IE_DATA: request the device to supply IE data >> in the >> + * request. > > What does that mean? I meant "supply IE data in the result". So it informs the driver that user-space is interested in IE data, but I guess it does not need to be explicit. It is not today. The result reporting is still something I am not sure about. I would prefer to use the bss list in cfg80211 like all other scan functions, but I am not sure if that is sufficient for user-space functionality. I did not get a clear answer on that for Android. >> + * @NL80211_GSCAN_CHAN_ATTR_NO_IR: scanning should be done passive. > > why not call that passive? No-IR is something we use in regulatory code > to be more generic than "passive" (since it's also about beaconing > etc.) but here? Ok. Makes sense as we are talking just probe requests here and passive scanning is familiar term. >> + * @NL80211_GSCAN_CHAN_ATTR_MAX: highest GScan channel attribute. > > Generally, you should also document the attribute types here (and > everywhere else really) Will do. >> +NL80211_BUCKET_BAND_2GHZ= (1 << 0), > > no need for parentheses with enums :) Ok. >> +if (tb[NL80211_GSCAN_CHAN_ATTR_DWELL_TIME]) >> +chan->dwell_time = >> nla_get_u32(tb[NL80211_GSCAN_CHAN_ATTR_DWELL_TIME]); > > Maybe that should have some kind of "reasonable range" limit? Agree. > So I mostly looked at this from a pure code POV - need to compare with > our implementation, but I guess the basis is the same ... Given its origin I would guess so too. What I propose here is minimal behavior so just the logic around the buckets. I wanted to add features like BSS hotlist and ePNO stuff in separate patches. Apart from that the iwlwifi implementation has some differences in details like attribute validation and there is no base period attribute as that must be the gcd of the bucket periods. So I might drop that as well. Regards, Arend
Re: [RFC V2 1/5] nl80211: allow reporting RTT information in scan results
On 28-11-2016 15:32, Johannes Berg wrote: > >> + * @distance: distance to AP with %parent_bssid in centimeters. Zero >> + * value indicates this is undetermined. >> + * @var_distance: variance of %distance indicating accurracy. > > accuracy > > The distance between the two APs? Where are you even obtaining that > information from? I was wondering about the meaning of the term "parent_bssid". Given your remark it means something else than my guess. I actually meant the distance to the AP indicated by this BSS. Our gscan code obtains the gscan results from firmware and in that API it has RTT info. However, recent testing revealed those fields are always coming up with zero values :-( So right now I am not sure if we need this extension. Regards, Arend
Re: [RFC] qtn: add FullMAC firmware for Quantenna QSR10G wifi device
On 11/28/2016 08:33 PM, Oleksij Rempel wrote: Am 28.11.2016 um 18:10 schrieb Oleksij Rempel: Am 28.11.2016 um 17:34 schrieb Kyle McMartin: On Tue, Nov 22, 2016 at 9:44 AM, IgorMitsyanko wrote: Hi Ben, Kyle, could you please share what is the position of linux-firmware regarding firmware binaries that include GPL components? Does it require entire GPL components codebase be present in linux-firmware tree, or maybe having this clause in license file is enough: +Open Source Software. The Software may include components that are licensed +pursuant to open source software (“Open Source Components”). Information +regarding the Open Source Components included with the Software is available +upon request to osle...@quantenna.com. To the extent such Open Source +Components are required to be licensed to you under the terms of a separate +license (such as an open source license) then such other terms shall apply, and +nothing herein shall be deemed or interpreted to limit any rights you may have +under any such applicable license. From technical perspective, size of the codebase used to build Quantenna firmware is a few hundred MBs, it seems too much to include into linux-firmware tree. I don't have strong feelings one way or another. I'd prefer not having several hundred MB of source that's unlikely to change included in the linux-firmware git tree. I'm also not a lawyer, so I can't help you decide what would satisfy the distribution clause of the GPLv2. We already have one GPL firmware (carl9170fw) which includes the source, but just references a seperate toolchain for downloading, so it's only approximately 1MB in size in the tree. Is your firmware source really that large, or is it just including the entire build toolchain with it? regards, --Kyle We also have open BSD licensed open-ath9k-htc-firmware. Which is locate out of source too. https://github.com/qca/open-ath9k-htc-firmware and here is location of carl firmware: https://github.com/chunkeey/carl9170fw So, what is actual problem with Quantenna QSR10G FW? I would be really interesting to take a look on it. Is it somewhere available? Are there some devices to get hand on? After seeing specs of this device i have strong feeling that "some open source part" is actual linux kernel. Oleksij, yes, that's correct, it includes entire Linux environment; the reasoning is that it allows to hide all WiFi-related logic inside device itself, and emulate simple Ethernet device for external system (therefore, freeing external system resources). This approach was working really well for us until recently, but now that company is expanding, we want to have more flexible and standardize interface available for external system to manage wireless connection, and FullMAC driver seems to be the best solution here. For the availability of FW sources, QSR10G-based products are still under development at this moment (not in the market yet), but many products based on previous generation chipset QSR1000 are available. For example, Asus has a retail design with QSR1000 chipset, and has all GPL sourcecode available on their website (including what Quantenna has provided): http://www.asus.com/Networking/RTAC87U/HelpDesk_Download/ Quantenna provided code is in, for example, "GPL of ASUS RT-AC87U for firmware 3.0.0.4.378.7410" archive. It's basically the same as used for QSR10G.
Re: [PATCH] RFC: Universal scan proposal
On Wed, Nov 23, 2016 at 12:43 AM, Arend Van Spriel wrote: > On 22-11-2016 21:54, Dmitry Shmidt wrote: >> On Tue, Nov 22, 2016 at 12:41 PM, Arend Van Spriel >> wrote: >>> On 22-11-2016 18:29, Dmitry Shmidt wrote: Hi Luca, On Mon, Nov 21, 2016 at 11:24 PM, Luca Coelho wrote: > Hi Dmitry, > On Wed, 2016-11-16 at 22:47 +, dimitr...@google.com wrote: >> From 68a9d37a4c7e9dc7a90a6e922cdea52737a98d66 Mon Sep 17 00:00:00 2001 >> From: Dmitry Shmidt >> Date: Wed, 16 Nov 2016 14:27:26 -0800 >> Subject: [PATCH] RFC: Universal scan proposal >> >>Currently we have sched scan with possibility of various >> intervals. We would like to extend it to support also >> different types of scan. >>In case of powerful wlan CPU, all this functionality >> can be offloaded. >>In general case FW processes additional scan requests >> and puts them into queue based on start time and interval. >> Once current request is fulfilled, FW adds it (if interval != 0) >> again to the queue with proper interval. If requests are >> overlapping, new request can be combined with either one before, >> or one after, assuming that requests are not mutually exclusive. >>Combining requests is done by combining scan channels, ssids, >> bssids and types of scan result. Once combined request was fulfilled >> it will be reinserted as two (or three) different requests based on >> their type and interval. >>Each request has attribute: >> Type: connectivity / location >> Report: none / batch / immediate >>Request may have priority and can be inserted into >> the head of the queue. >>Types of scans: >> - Normal scan >> - Scheduled scan >> - Hotlist (BSSID scan) >> - Roaming >> - AutoJoin >> >> Change-Id: I9f3e4c975784f1c1c5156887144d80fc5a26bffa >> Signed-off-by: Dmitry Shmidt >> --- > > I like the initiative and I think this is definitely something that can > improve concurrent scanning instances. But IMHO the most important is > to discuss the semantics of this change, such as which scans can be > combined, who makes the decisions of combining them, how priorities are > sorted out etc. I think the types of scan are not relevant in the > nl80211 API, but the characteristics of the scans are. For instance, > "urgent scan" (for initial connection), best-effort scan for roaming... > and latency requirements, such as low-latency for location and initial > connection and high-latency for scheduled scan. Then we decided, in > the kernel, how to combine and prioritize them according to their > characteristics, instead of having to map scan types to these > characteristics. > > What do you think? 1. Combining scans. There are two scenarios in general: combine scans that can be offload and scans that can not be offload due to "weak" FW / wlan SoC. In last case this approach maybe not attractive at all - non-mobile device may not need all these different types of scan. In case of offload - it will be FW code decision - I just wanted to propose the way how to do this efficiently. >>> >>> I think Luca is looking at it as a way to deal with multiple user-space >>> entities request the device to perform a scan. Ignoring the scan types >>> on android it can be wifi-hal and wpa_supplicant. >>> 2. Priority - very good point, we need to have it. I am just not sure that we need like scale priority - maybe just flag - urgent / not urgent. Urgent one will be inserted in queue as is and conflicting request should be postponed or combined. >>> >>> What if we get another urgent one? >> >> We may restrict it only to one urgent scan - what is the use case for >> two requests? > > We don't know I guess so we can restrict to one for now. Just wondered > reading this. > 3. Scan types - I am not sure I fully understood your question, but if the idea is for kernel to decide about type of scan based on its characteristics instead of specific type request performance may cause confusion to wifi manager. However, it would definitely simplify kernel API. Still I am not sure if userspace wifi manager will "like" it. >>> >>> Actually the idea is to hide the notion of scan type entirely from the >>> API and kernel code. If you add the scan type as an attribute in the >>> API, you still need to provide additional attributes for that scan type >>> so the question is what the scan type itself provides, ie. how does it >>> affect the scan itself. >> >> Right, some scans types can be easily hidden behind scan parameters >> like scan for SSID or BSSID or maybe even Roaming with list of >> SSIDs or BSSIDs, or mix... However, scan history for example should >> be separate, right? > > Not sure what scan history means. cfg80211 currently maintains the > lates
Re: [PATCH 1/2] rsi: New firware loading method for RSI 91X devices
Prameela Rani Garnepudi writes: > RSI deprecated the old firmware loading method and introduced > new method using soft boot loader for 9113 chipsets. > Current driver only supports 9113 device model hence firmware > loading method has been changed. > > In the new method, complete RAM image and flash image are present > in the flash. Two firmwares present in the device, Boot loader firmware > and functional firmware. Boot loader firmware is fixed but functional > firmware can be changed. Before loading the functional firmware, host > issues commands to check whether existing firmware in the chip and the > firmware file content to load are same or not. If not, host issues > commands to load the RAM image and then boot loaded switches to the > functioanl firmware. > > Signed-off-by: Prameela Rani Garnepudi Forgot this: [...] > +#define FIRMWARE_RSI9113 "rsi_91x.fw" I see that you add the define here but I don't see you anywhere removing the old one. I assume you were planning just to move it. -- Kalle Valo
Re: [PATCH 1/2] rsi: New firware loading method for RSI 91X devices
Prameela Rani Garnepudi writes: > RSI deprecated the old firmware loading method and introduced > new method using soft boot loader for 9113 chipsets. > Current driver only supports 9113 device model hence firmware > loading method has been changed. > > In the new method, complete RAM image and flash image are present > in the flash. Two firmwares present in the device, Boot loader firmware > and functional firmware. Boot loader firmware is fixed but functional > firmware can be changed. Before loading the functional firmware, host > issues commands to check whether existing firmware in the chip and the > firmware file content to load are same or not. If not, host issues > commands to load the RAM image and then boot loaded switches to the > functioanl firmware. > > Signed-off-by: Prameela Rani Garnepudi A general rule is that existing firmware support should not be broken. Instead there should be a transition period for some time end then support for old firmware method is removed. But as I don't know if that upstream driver is not that widely used I guess it might be ok to break it as it won't cause that much problems to people. Typo in the title: "firware" > drivers/net/wireless/rsi/Makefile |2 +- > drivers/net/wireless/rsi/rsi_91x_hal.c | 1049 > +++ > drivers/net/wireless/rsi/rsi_91x_mgmt.c | 49 ++ > drivers/net/wireless/rsi/rsi_91x_pkt.c | 215 -- > drivers/net/wireless/rsi/rsi_91x_sdio.c | 231 +- > drivers/net/wireless/rsi/rsi_91x_sdio_ops.c | 192 + > drivers/net/wireless/rsi/rsi_91x_usb.c | 176 - > drivers/net/wireless/rsi/rsi_91x_usb_ops.c | 130 +--- > drivers/net/wireless/rsi/rsi_common.h |4 + > drivers/net/wireless/rsi/rsi_hal.h | 150 > drivers/net/wireless/rsi/rsi_main.h | 68 +- > drivers/net/wireless/rsi/rsi_mgmt.h |2 + > drivers/net/wireless/rsi/rsi_sdio.h | 18 +- > drivers/net/wireless/rsi/rsi_usb.h | 14 +- > 14 files changed, 1720 insertions(+), 580 deletions(-) > create mode 100644 drivers/net/wireless/rsi/rsi_91x_hal.c > delete mode 100644 drivers/net/wireless/rsi/rsi_91x_pkt.c > create mode 100644 drivers/net/wireless/rsi/rsi_hal.h The patch is quite big which makes review hard. If you had split this into, for example, three patches it would be a lot faster to review. You seem to be doing multiple logical changes in one path. But remember that neither the build nor runtime functionality should break between the patches. > +/* FLASH Firmware */ > +struct ta_metadata metadata_flash_content[] = { > + {"flash_content", 0x0001}, > + {"RS9113_WLAN_QSPI.rps", 0x0001}, > + {"RS9113_WLAN_BT_DUAL_MODE.rps", 0x0001}, > + {"RS9113_WLAN_ZIGBEE.rps", 0x0001}, > + {"RS9113_AP_BT_DUAL_MODE.rps", 0x0001}, > + {"RS9113_WLAN_QSPI.rps", 0x0001} > +}; Are the strings here the names of the firmware images on host filesystem? The preference is that they are lower case. Also will you be posting the firmware images to linux-firmware.git? > + > +/** > + * rsi_send_data_pkt() - This function sends the received data packet from > + *driver to device. > + * @common: Pointer to the driver private structure. > + * @skb: Pointer to the socket buffer structure. > + * > + * Return: status: 0 on success, -1 on failure. > + */ > +int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb) > +{ > + struct rsi_hw *adapter = common->priv; > + struct ieee80211_hdr *wh = NULL; > + struct ieee80211_tx_info *info; > + struct skb_info *tx_params; > + struct ieee80211_bss_conf *bss = NULL; > + int status = -EINVAL; Documentation and code don't match again. > + u8 ieee80211_hdr_size = MIN_802_11_HDR_LEN; > + u8 dword_align_bytes = 0; > + u8 header_size = 0; > + __le16 *frame_desc; > + struct xtended_desc *xtend_desc; > + u16 seq_num = 0; > + > + info = IEEE80211_SKB_CB(skb); > + bss = &info->control.vif->bss_conf; > + tx_params = (struct skb_info *)info->driver_data; > + > + if (!bss->assoc) > + goto err; > + > + dword_align_bytes = ((uintptr_t)skb->data & 0x3f); ALIGN()? > + header_size = dword_align_bytes + FRAME_DESC_SZ + > + sizeof(struct xtended_desc); > + if (header_size > skb_headroom(skb)) { > + rsi_dbg(ERR_ZONE, "%s: Not enough headroom\n", __func__); > + status = -ENOSPC; > + goto err; > + } > + > + skb_push(skb, header_size); > + frame_desc = (__le16 *)&skb->data[0]; > + xtend_desc = (struct xtended_desc *)&skb->data[FRAME_DESC_SZ]; > + memset((u8 *)frame_desc, 0, header_size); > + > + wh = (struct ieee80211_hdr *)&skb->data[header_size]; > + seq_num = (le16_to_cpu(wh->seq_ctrl) >> 4); I would hope include/linux/ieee80211.h already provides a macro for this. > + frame_desc
Deadlock in hacked 4.9.0-rc6+ kernel.
I ported forward my patch set to the 4.9 kernel, and I am seeing lockups fairly often. As always, could be something I added locally, but in case someone sees similar, then maybe I can reproduce it quicker and help track this down since my test config uses many virtual stations and virtual APs. And, if someone knows the magic trick to make dmesg output from lockdep not be so split up, please let me know. [81871.799595] ath10k_pci :05:00.0: boot warm reset complete [81873.983645] sta559: Failed to send nullfunc to AP 04:f0:21:0f:3c:3c after 1000ms, disconnecting [81873.991198] ath10k_pci :05:00.0: mac ampdu vdev_id 49 sta 04:f0:21:0f:3c:3c tid 6 action 1 [82195.484265] INFO: task mission-control:1506 blocked for more than 180 seconds. [82195.490228] Tainted: GW 4.9.0-rc6+ #9 [82195.494367] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [82195.500922] mission-control D0 1506 1 0x [82195.505152] 8801f1c3a300 2629 880214de2640 8801f2f38000 [82195.511330] 88021e3198d8 c90003867c80 819322eb 0006 [82195.518230] 8801f2f38828 03867c78 88021e3198d8 [82195.525121] Call Trace: [82195.526300] [] ? __schedule+0x2fb/0xb30 [82195.530862] [] schedule+0x38/0x90 [82195.534552] [] schedule_preempt_disabled+0x10/0x20 [82195.540073] [] mutex_lock_nested+0x175/0x3f0 [82195.544723] [] ? rtnetlink_rcv+0x16/0x30 [82195.549424] [] rtnetlink_rcv+0x16/0x30 [82195.553555] [] netlink_unicast+0x1cd/0x2e0 [82195.558414] [] ? netlink_unicast+0x149/0x2e0 [82195.563062] [] netlink_sendmsg+0x2e2/0x390 [82195.567889] [] ? __fget+0x108/0x1f0 [82195.571755] [] sock_sendmsg+0x33/0x40 [82195.575791] [] SYSC_sendto+0x101/0x190 [82195.580258] [] ? security_file_permission+0x96/0xb0 [82195.585521] [] ? rw_verify_area+0x49/0xb0 [82195.589909] [] ? trace_hardirqs_on_caller+0x129/0x1b0 [82195.595364] [] ? trace_hardirqs_on_thunk+0x1a/0x1c [82195.600538] [] SyS_sendto+0x9/0x10 [82195.604319] [] entry_SYSCALL_64_fastpath+0x23/0xc6 [82195.609502] Showing all locks held in the system: [82195.613134] 2 locks held by khungtaskd/37: [82195.615958] #0: [82195.616408] ( [82195.616802] rcu_read_lock [82195.617946] ){..} [82195.618983] , at: [82195.619522] [] watchdog+0x9c/0x600 [82195.623219] #1: [82195.623666] ( [82195.624060] tasklist_lock [82195.625208] ){.+.+..} [82195.626213] , at: [82195.626748] [] debug_show_all_locks+0x3d/0x1a0 [82195.631513] 2 locks held by agetty/1149: [82195.634160] #0: [82195.634610] ( [82195.635001] &tty->ldisc_sem [82195.636320] ){.+} [82195.637323] , at: [82195.637859] [] ldsem_down_read+0x2d/0x40 [82195.642077] #1: [82195.642528] ( [82195.642922] &ldata->atomic_read_lock [82195.645027] ){+.+...} [82195.646030] , at: [82195.646569] [] n_tty_read+0xa9/0x910 [82195.650442] 1 lock held by mission-control/1506: [82195.653800] #0: [82195.654249] ( [82195.654645] rtnl_mutex [82195.655527] ){+.+.+.} [82195.656534] , at: [82195.657068] [] rtnetlink_rcv+0x16/0x30 [82195.661113] 2 locks held by bash/1691: [82195.663586] #0: [82195.664033] ( [82195.664433] &tty->ldisc_sem [82195.665750] ){.+} [82195.666754] , at: [82195.667294] [] ldsem_down_read+0x2d/0x40 [82195.671509] #1: [82195.671954] ( [82195.672349] &ldata->atomic_read_lock [82195.674447] ){+.+...} [82195.675451] , at: [82195.675987] [] n_tty_read+0xa9/0x910 [82195.679857] 1 lock held by evolution-calen/1768: [82195.683200] #0: [82195.683649] ( [82195.684040] rtnl_mutex [82195.684925] ){+.+.+.} [82195.685930] , at: [82195.686467] [] rtnetlink_rcv+0x16/0x30 [82195.690514] 2 locks held by hostapd/4100: [82195.693249] #0: [82195.693694] ( [82195.694090] cb_lock [82195.694711] ){++} [82195.695741] , at: [82195.696279] [] genl_rcv+0x14/0x40 [82195.699888] #1: [82195.700338] ( [82195.700731] genl_mutex [82195.701616] ){+.+.+.} [82195.702619] , at: [82195.703155] [] genl_rcv_msg+0xa4/0xb0 [82195.707113] 2 locks held by hostapd/4392: [82195.709847] #0: [82195.710301] ( [82195.710695] cb_lock [82195.711317] ){++} [82195.712324] , at: [82195.712857] [] genl_rcv+0x14/0x40 [82195.716467] #1: [82195.716911] ( [82195.717306] genl_mutex [82195.718186] ){+.+.+.} [82195.719193] , at: [82195.719727] [] genl_rcv_msg+0xa4/0xb0 [82195.723682] 3 locks held by wpa_supplicant/4574: [82195.727028] #0: [82195.727480] ( [82195.727871] cb_lock [82195.728494] ){++} [82195.729501] , at: [82195.730034] [] genl_rcv+0x14/0x40 [82195.733644] #1: [82195.734089] ( [82195.734489] genl_mutex [82195.735371] ){+.+.+.} [82195.736377] , at: [82195.736910] [] genl_rcv_msg+0xa4/0xb0 [82195.740867] #2: [82195.741316] ( [82195.741712] rtnl_mutex [82195.742595] ){+.+.+.} [82195.743601] , at: [82195.744135] [] rtnl_lock+0x12/0x20 [82195.747833] 2 locks held by hostapd/4733: [82195.750566] #0: [82195.751014] ( [82195.751409] cb_lock [82195.752029] ){++} [82195.753033] , at: [82195.753570
Re: [PATCH 3/3][RFC] nl80211/mac80211: Accept multiple RSSI thresholds for CQM
On 28 November 2016 at 16:35, Johannes Berg wrote: > On Mon, 2016-11-28 at 16:29 +0100, Andrew Zaborowski wrote: >> In order to keep the hardware offload feature when working with >> hardware that can only offload the old single-thresholds method, but >> where the kernel also wants to support the new method by looking at >> all the beacons in software. IIRC I identified just one driver that >> would be in this situation: wl1271. > > IMHO it would be better to not allow that. I'd think that wl1271 is > only used in devices where power consumption will be far more > interesting than providing this kind of API. Ok. > >> This is a specific case and the semantics implemented by the wl1271 >> may be a little different from those in the rest of the drivers so >> this may be worth very little. I can change the comment to imply >> only one method should be implemented. > > We might still have to allow both to be present for mac80211 though. > >> > Seems there still should be a hysteresis? Or am I misunderstanding >> > the >> > intent here? I.e. isn't it meant to report low/medium/high later? >> >> This isn't exposed directly to users, instead it's used by the code >> in >> nl80211.c which will always reset the thresholds when either >> threshold >> is crossed. The hysteresis can then be either handled in nl80211.c >> (factored into the threshold values) or in the firmware/driver, this >> won't change the number of wakeups. > > That's only if you assume you can actually react to this fast enough > though, no? If I offload this, I'd want to also offload a hysteresis to > firmware, I'd think. I wasn't clear: nl80211 sets the thresholds so that "high" is higher than last known value and "low" is lower than last known value, also the distance is at least 2 x hysteresis. There's no purpose for reporting "middle" rssi events because we have to set a new range as soon as we receive a high or a low event. I realize I need to document better. So I don't think this can result in additional events that wouldn't occur if the firmware handled rssi hysteresis. I think this is generic enough that you can implement any monitoring logic on top of it and squeeze the same number of wakeup in all scenarios as if the driver handled it. But it shouldn't discriminate hardware that doesn't have the hysteresis parameter from offloading this. BTW I fear if you wanted to have the hysteresis parameter handled by firmware you'd end up with slightly differing semantics depending of what the firmware does the moment you change you threshold values, whether the rssi tracking is reset. Best regards
Re: [RFC] qtn: add FullMAC firmware for Quantenna QSR10G wifi device
Am 28.11.2016 um 18:10 schrieb Oleksij Rempel: > Am 28.11.2016 um 17:34 schrieb Kyle McMartin: >> On Tue, Nov 22, 2016 at 9:44 AM, IgorMitsyanko >> wrote: >>> Hi Ben, Kyle, >>> could you please share what is the position of linux-firmware regarding >>> firmware binaries that include GPL components? Does it require entire GPL >>> components codebase be present in linux-firmware tree, or maybe having this >>> clause in license file is enough: >>> +Open Source Software. The Software may include components that are licensed >>> +pursuant to open source software (“Open Source Components”). Information >>> +regarding the Open Source Components included with the Software is >>> available >>> +upon request to osle...@quantenna.com. To the extent such Open Source >>> +Components are required to be licensed to you under the terms of a separate >>> +license (such as an open source license) then such other terms shall apply, >>> and >>> +nothing herein shall be deemed or interpreted to limit any rights you may >>> have >>> +under any such applicable license. >>> >>> From technical perspective, size of the codebase used to build Quantenna >>> firmware is a few hundred MBs, it seems too much to include into >>> linux-firmware tree. >>> >> >> I don't have strong feelings one way or another. I'd prefer not having >> several hundred >> MB of source that's unlikely to change included in the linux-firmware >> git tree. I'm also not >> a lawyer, so I can't help you decide what would satisfy the >> distribution clause of the GPLv2. >> We already have one GPL firmware (carl9170fw) which includes the >> source, but just references >> a seperate toolchain for downloading, so it's only approximately 1MB >> in size in the tree. >> >> Is your firmware source really that large, or is it just including the >> entire build toolchain with it? >> >> regards, >> --Kyle > > We also have open BSD licensed open-ath9k-htc-firmware. Which is locate > out of source too. > https://github.com/qca/open-ath9k-htc-firmware > and here is location of carl firmware: > https://github.com/chunkeey/carl9170fw > > So, what is actual problem with Quantenna QSR10G FW? > I would be really interesting to take a look on it. Is it somewhere > available? Are there some devices to get hand on? After seeing specs of this device i have strong feeling that "some open source part" is actual linux kernel. -- Regards, Oleksij signature.asc Description: OpenPGP digital signature
Re: [RFC] qtn: add FullMAC firmware for Quantenna QSR10G wifi device
Am 28.11.2016 um 17:34 schrieb Kyle McMartin: > On Tue, Nov 22, 2016 at 9:44 AM, IgorMitsyanko > wrote: >> Hi Ben, Kyle, >> could you please share what is the position of linux-firmware regarding >> firmware binaries that include GPL components? Does it require entire GPL >> components codebase be present in linux-firmware tree, or maybe having this >> clause in license file is enough: >> +Open Source Software. The Software may include components that are licensed >> +pursuant to open source software (“Open Source Components”). Information >> +regarding the Open Source Components included with the Software is >> available >> +upon request to osle...@quantenna.com. To the extent such Open Source >> +Components are required to be licensed to you under the terms of a separate >> +license (such as an open source license) then such other terms shall apply, >> and >> +nothing herein shall be deemed or interpreted to limit any rights you may >> have >> +under any such applicable license. >> >> From technical perspective, size of the codebase used to build Quantenna >> firmware is a few hundred MBs, it seems too much to include into >> linux-firmware tree. >> > > I don't have strong feelings one way or another. I'd prefer not having > several hundred > MB of source that's unlikely to change included in the linux-firmware > git tree. I'm also not > a lawyer, so I can't help you decide what would satisfy the > distribution clause of the GPLv2. > We already have one GPL firmware (carl9170fw) which includes the > source, but just references > a seperate toolchain for downloading, so it's only approximately 1MB > in size in the tree. > > Is your firmware source really that large, or is it just including the > entire build toolchain with it? > > regards, > --Kyle We also have open BSD licensed open-ath9k-htc-firmware. Which is locate out of source too. https://github.com/qca/open-ath9k-htc-firmware and here is location of carl firmware: https://github.com/chunkeey/carl9170fw So, what is actual problem with Quantenna QSR10G FW? I would be really interesting to take a look on it. Is it somewhere available? Are there some devices to get hand on? -- Regards, Oleksij signature.asc Description: OpenPGP digital signature
Re: [RFC] qtn: add FullMAC firmware for Quantenna QSR10G wifi device
On Tue, Nov 22, 2016 at 9:44 AM, IgorMitsyanko wrote: > Hi Ben, Kyle, > could you please share what is the position of linux-firmware regarding > firmware binaries that include GPL components? Does it require entire GPL > components codebase be present in linux-firmware tree, or maybe having this > clause in license file is enough: > +Open Source Software. The Software may include components that are licensed > +pursuant to open source software (“Open Source Components”). Information > +regarding the Open Source Components included with the Software is > available > +upon request to osle...@quantenna.com. To the extent such Open Source > +Components are required to be licensed to you under the terms of a separate > +license (such as an open source license) then such other terms shall apply, > and > +nothing herein shall be deemed or interpreted to limit any rights you may > have > +under any such applicable license. > > From technical perspective, size of the codebase used to build Quantenna > firmware is a few hundred MBs, it seems too much to include into > linux-firmware tree. > I don't have strong feelings one way or another. I'd prefer not having several hundred MB of source that's unlikely to change included in the linux-firmware git tree. I'm also not a lawyer, so I can't help you decide what would satisfy the distribution clause of the GPLv2. We already have one GPL firmware (carl9170fw) which includes the source, but just references a seperate toolchain for downloading, so it's only approximately 1MB in size in the tree. Is your firmware source really that large, or is it just including the entire build toolchain with it? regards, --Kyle > On 11/11/2016 02:35 PM, Johannes Berg wrote: >> >> Adding linux-firmware people to Cc, since presumably they don't >> necessarily read linux-wireless... >> >>> Johannes, from that perspective, who are the "redistributors"? >>> Specifically, is linux-firmware git repository considered a >>> redistributor or its just hosting files? I mean, at what moment >>> someone else other then Quantenna will start to be legally obliged to >>> make GPL code used in firmware available for others? >> >> Look, I don't know. I'd assume people who ship it, like any regular >> distro, would be (re)distributors thereof. "Normal" (non-GPL) firmware >> images come with a redistribution license, but that obviously can't >> work here. >> >> There's some info from Ben here regarding the carl9170 case: >> http://lkml.iu.edu/hypermail/linux/kernel/1605.3/01176.html >> >>> Personally I still hope that linux-firmware itself is not legally >>> concerned with what is the content of firmware its hosting, but looks >>> like there already was a precedent case with carl9170 driver and >>> we have to somehow deal with it. >> >> That's really all I wanted to bring up. I'm not involved with the >> linux-firmware git tree. >> >>> There still may be a difference though: Quantenna is semiconductor >>> company only, software >>> used on actual products based on Quantenna chipsets is released by >>> other >>> companies. >>> I just want to present our legal team with a clear case (and position >>> of >>> Linux maintainers) so that they can >>> work with it and make decision on how to proceed. >>> >>> From technical perspective, as I mentioned, SDK is quite huge and >>> include a lot of opensource >>> components including full Linux, I don't think its reasonable to have >>> it >>> inside linux-firmware tree. >>> What are the options to share it other then providing it on request >>> basis: >>> - git repository >>> - store tarball somewhere on official website >> >> Clearly that wasn't deemed appropriate for carl9170, so I don't see why >> it'd be different here. >> >> johannes > >
Re: [PATCH] rtlwifi: Add updates for RTL8723BE and RTL8821AE
On Sun, Nov 27, 2016 at 01:28:34PM -0600, Larry Finger wrote: > The new versions will only work with new versions of the drivers. For > that reason, they are given new names and the old versions are retained. > > Signed-off-by: Larry Finger > --- > WHENCE | 4 applied, thanks Larry. regards, --kyle
Re: [PATCH] mmc: pwrseq: add support for Marvell SD8787 chip
[...] >> + >> +Example: >> + >> + wifi_pwrseq: wifi_pwrseq { >> + compatible = "mmc-pwrseq-sd8787"; >> + pwrdn-gpio = <&twl_gpio 0 GPIO_ACTIVE_LOW>; >> + reset-gpio = <&twl_gpio 1 GPIO_ACTIVE_LOW>; >> + } >> diff --git >> a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt >> b/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt >> index c421aba0a5bc..08fd65d35725 100644 >> --- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt >> +++ b/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt >> @@ -32,6 +32,9 @@ Optional properties: >>so that the wifi chip can wakeup host platform under certain >> condition. >>during system resume, the irq will be disabled to make sure >>unnecessary interrupt is not received. >> + - vmmc-supply: a phandle of a regulator, supplying VCC to the card > > This is why pwrseq is wrong. You have some properties in the card node > and some in pwrseq node. Everything should be in the card node. Put "all" in the card node, just doesn't work for MMC. Particular in cases when we have removable cards, as then it would be wrong to have a card node. The mmc pwrseq DT bindings just follows the legacy approach for MMC and that's why the pwrseq handle is at the controller node. Yes, would could have done it differently, but this is the case now, so we will have to accept that. [...] Kind regards Uffe
Re: rtl8xxxu: tx rate reported before set
Barry Day wrote: > Move the dev_info call that attempts to show the rate used before it is set. > > Signed-off-by: Barry Day > Reported-by: Stephen Rothwell > Signed-off-by: Barry Day Patch applied to wireless-drivers-next.git, thanks. c06696a95820 rtl8xxxu: tx rate reported before set -- https://patchwork.kernel.org/patch/9448225/ Documentation about submitting wireless patches and checking status from patchwork: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 3/3][RFC] nl80211/mac80211: Accept multiple RSSI thresholds for CQM
On Mon, 2016-11-28 at 16:29 +0100, Andrew Zaborowski wrote: > In order to keep the hardware offload feature when working with > hardware that can only offload the old single-thresholds method, but > where the kernel also wants to support the new method by looking at > all the beacons in software. IIRC I identified just one driver that > would be in this situation: wl1271. IMHO it would be better to not allow that. I'd think that wl1271 is only used in devices where power consumption will be far more interesting than providing this kind of API. > This is a specific case and the semantics implemented by the wl1271 > may be a little different from those in the rest of the drivers so > this may be worth very little. I can change the comment to imply > only one method should be implemented. We might still have to allow both to be present for mac80211 though. > > Seems there still should be a hysteresis? Or am I misunderstanding > > the > > intent here? I.e. isn't it meant to report low/medium/high later? > > This isn't exposed directly to users, instead it's used by the code > in > nl80211.c which will always reset the thresholds when either > threshold > is crossed. The hysteresis can then be either handled in nl80211.c > (factored into the threshold values) or in the firmware/driver, this > won't change the number of wakeups. That's only if you assume you can actually react to this fast enough though, no? If I offload this, I'd want to also offload a hysteresis to firmware, I'd think. johannes
Re: [PATCH] ath9k_htc: don't use HZ for usb msg timeouts
Hi Anthony, Am 28.11.2016 um 05:27 schrieb Anthony Romano: > The usb_*_msg() functions expect a timeout in msecs but are given HZ, > which is ticks per second. If HZ=100, firmware download often times out > when there is modest USB utilization and the device fails to initialize. > Replaces HZ in usb_*_msg timeouts with 1000 msec since HZ is one second > for timeouts in jiffies. wow.. This fix allow you use 4 adapter at same time? > Signed-off-by: Anthony Romano > --- > drivers/net/wireless/ath/ath9k/hif_usb.c | 9 + > drivers/net/wireless/ath/ath9k/hif_usb.h | 2 ++ > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c > b/drivers/net/wireless/ath/ath9k/hif_usb.c > index e1c338cb9cb5..de2d212f39ec 100644 > --- a/drivers/net/wireless/ath/ath9k/hif_usb.c > +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c > @@ -997,7 +997,8 @@ static int ath9k_hif_usb_download_fw(struct > hif_device_usb *hif_dev) > err = usb_control_msg(hif_dev->udev, > usb_sndctrlpipe(hif_dev->udev, 0), > FIRMWARE_DOWNLOAD, 0x40 | USB_DIR_OUT, > - addr >> 8, 0, buf, transfer, HZ); > + addr >> 8, 0, buf, transfer, > + USB_MSG_TIMEOUT); > if (err < 0) { > kfree(buf); > return err; > @@ -1020,7 +1021,7 @@ static int ath9k_hif_usb_download_fw(struct > hif_device_usb *hif_dev) > err = usb_control_msg(hif_dev->udev, usb_sndctrlpipe(hif_dev->udev, 0), > FIRMWARE_DOWNLOAD_COMP, > 0x40 | USB_DIR_OUT, > - firm_offset >> 8, 0, NULL, 0, HZ); > + firm_offset >> 8, 0, NULL, 0, USB_MSG_TIMEOUT); > if (err) > return -EIO; > > @@ -1249,7 +1250,7 @@ static int send_eject_command(struct usb_interface > *interface) > > dev_info(&udev->dev, "Ejecting storage device...\n"); > r = usb_bulk_msg(udev, usb_sndbulkpipe(udev, bulk_out_ep), > - cmd, 31, NULL, 2000); > + cmd, 31, NULL, 2 * USB_MSG_TIMEOUT); > kfree(cmd); > if (r) > return r; > @@ -1314,7 +1315,7 @@ static void ath9k_hif_usb_reboot(struct usb_device > *udev) > return; > > ret = usb_interrupt_msg(udev, usb_sndintpipe(udev, USB_REG_OUT_PIPE), > -buf, 4, NULL, HZ); > +buf, 4, NULL, USB_MSG_TIMEOUT); > if (ret) > dev_err(&udev->dev, "ath9k_htc: USB reboot failed\n"); > > diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h > b/drivers/net/wireless/ath/ath9k/hif_usb.h > index 7c2ef7ecd98b..7846916aa01d 100644 > --- a/drivers/net/wireless/ath/ath9k/hif_usb.h > +++ b/drivers/net/wireless/ath/ath9k/hif_usb.h > @@ -71,6 +71,8 @@ extern int htc_use_dev_fw; > #define USB_REG_IN_PIPE 3 > #define USB_REG_OUT_PIPE 4 > > +#define USB_MSG_TIMEOUT 1000 /* (ms) */ > + > #define HIF_USB_MAX_RXPIPES 2 > #define HIF_USB_MAX_TXPIPES 4 > > signature.asc Description: OpenPGP digital signature
Re: [PATCH 3/3][RFC] nl80211/mac80211: Accept multiple RSSI thresholds for CQM
Hi Johannes, On 28 November 2016 at 15:47, Johannes Berg wrote: > >> + * @set_cqm_rssi_range_config: Configure two RSSI thresholds in the >> > + * connection quality monitor. Even if the driver implements both the >> > + * single threshold and low/high thresholds mechanisms, it should assume >> + * only one is active at any time. > > Why would a driver still (be allowed to!) implement both? In order to keep the hardware offload feature when working with hardware that can only offload the old single-thresholds method, but where the kernel also wants to support the new method by looking at all the beacons in software. IIRC I identified just one driver that would be in this situation: wl1271. This is a specific case and the semantics implemented by the wl1271 may be a little different from those in the rest of the drivers so this may be worth very little. I can change the comment to imply only one method should be implemented. > >> + int (*set_cqm_rssi_range_config)(struct wiphy *wiphy, >> + struct net_device *dev, >> + s32 rssi_low, s32 rssi_high); > > Seems there still should be a hysteresis? Or am I misunderstanding the > intent here? I.e. isn't it meant to report low/medium/high later? This isn't exposed directly to users, instead it's used by the code in nl80211.c which will always reset the thresholds when either threshold is crossed. The hysteresis can then be either handled in nl80211.c (factored into the threshold values) or in the firmware/driver, this won't change the number of wakeups. It's probably easier to do it in one place and keep it simple on the drivers? > >> diff --git a/include/net/mac80211.h b/include/net/mac80211.h >> index 33026e1..7da1056 100644 >> --- a/include/net/mac80211.h >> +++ b/include/net/mac80211.h > > I'd prefer you split cfg80211 and mac80211 patches, i.e. provide the > new API first and then implement it in mac80211 separately. Yes, will do, only as this is purely an RFC I preferred to keep the conext together. > >> +void cfg80211_cqm_config_free(struct wireless_dev *wdev) >> +{ >> + if (!wdev->cqm_config) >> + return; >> + >> + kfree(wdev->cqm_config->rssi_thresholds); >> + kfree(wdev->cqm_config); >> + wdev->cqm_config = NULL; >> +} > > You can save this complexity by just making the cqm_config struct have > all the thresholds inside itself - pretty easy to allocate by just > counting them first. Ok, guess I can do that and let last_rssi_event_value get reset when the thresholds are reconfigured. Best regards
limiting times ath9k_htc devices are loaded
Hello When I plug in a ath9k_htc device, with a loaded driver, the driver attemps to attach itself to the device and load firmware etc. ( of course ). Does anyone know how I can tell the driver ( or kernel ) to do this process only once, and ignore additional insertions of ath9k_htc devices ? Bruce
Re: Break-it testing for wifi
On 11/28/2016 06:28 AM, Johannes Berg wrote: On Tue, 2016-11-22 at 08:59 -0800, Ben Greear wrote: Why would you do that? In order to test the AP implementation? Yes. And really, you should be able to do similar things on the AP to test stations, or on IBSS/Mesh devices, etc. hostapd already has some options to corrupt or drop a percentage of various management frames. Supplicant does not as far as I know. Yes, but that's a far different focus. I'm far more interested in testing *our* implementation(s), at which point I don't need to do it over the air for most cases, and can be much more efficient that way, etc. I also don't really see a point of doing anything over the air to test our implementations, apart from hitting firmware (but even then ...) OTA is of primary importance to me, but I think any solution that works for OTA would work for hwsim as well. 2) Randomly corrupt mgt frames in driver and/or mac80211 stack and/or supplicant. I think fuzzing the input path for those frames would be more useful than just corrupting things. Random corruptions, especially by code that had at least some understanding of management frames should be fast and easy to use. It would not be as good as a really clever fuzzer or hand- crafted frames, but for many users, hand-crafting attacks would be well beyond what they could ever accomplish. Define "user"? System-test engineer in random company. Like most of us, I am not working for pure charity purposes :) Currently, supplicant can (at least with some small patches that I carry), add custom information elements to probe requests and similar. But, some things are built by mac80211 (rate-sets advertised, for instance). So, I was thinking of slightly extending the API so that user-space could over-ride whatever mac80211 might normally build itself. That lets you more properly fuzz things from user space. Why bother though, at that point? If you hook up some state transitions you can probably just send the frames from userspace. For these testing scenarios you can make assumptions about the hardware or query it beforehand, so there's no need to rely on mac80211 at all? I don't want to re-implement supplicant, nor heavily modify it for this effort. I'm not sure exactly how important modifying IEs from user-space would be for my effort, maybe existing API is enough with in-kernel fuzzer I am thinking about writing. Yes, but for ease of use, and to cover frames generated by mac80211, I was thinking: echo 0.25 > /debug/.../wlan0/mgt_fuzzer The fuzzer would then corrupt 25% of the management frames. And instead of just randomly scribbling, it could also parse the frames and do some more clever (and still pseudo-random) modifications to the frames, like re-writing IEs with bad lengths, flipping bits in specific portions of the frame we feel might find problems, etc. I think if the tool became useful, then it could grow more clever over time. I'd be far more inclined to just add a tracepoint there at some spot and allow attaching BPF programs to it ... Or perhaps allow attaching BPF programs directly, if tracepoint BPF can't modify the data. Having any kind of logic here in the kernel space seems fairly useless since you'll want to change it often, etc. Well, in-kernel seems best to me. I will give it a try and see how it works. Thanks, Ben johannes -- Ben Greear Candela Technologies Inc http://www.candelatech.com
Re: cfg80211: add set/get link loss profile
> Introduce NL80211_CMD_SET_LINK_LOSS_PROFILE and > NL80211_CMD_GET_LINK_LOSS_PROFILE as it required by the user space > to configure the link loss profile. I'm not convinced. > The link loss profile represents the priority of maintaining link up > in different link quality environments. > Three types of behavior for link loss defined: > LINK_LOSS_PROFILE_RELAXED: prefer maintaining link up even in poor > link quality environment. > LINK_LOSS_PROFILE_DEFAULT: The default behavior for maintaining link > up vs link quality. > LINK_LOSS_PROFILE_AGGRESSIVE: prefer losing link up in poor link > quality environment. What you're doing here is *extremely* vague. No definitions of what this means, no description of how it's intended to be implemented, no note even on whether or not this is for full-MAC chips or not, etc. At least for mac80211, we can define signals to userspace regarding "link quality" and make decisions in wpa_s then? > --- a/drivers/net/wireless/ath/wil6210/cfg80211.c Those changes obviously don't belong into this patch anyway. johannes
Re: [PATCH 3/3][RFC] nl80211/mac80211: Accept multiple RSSI thresholds for CQM
> + * @set_cqm_rssi_range_config: Configure two RSSI thresholds in the > > + * connection quality monitor. Even if the driver implements both the > > + * single threshold and low/high thresholds mechanisms, it should assume > + * only one is active at any time. Why would a driver still (be allowed to!) implement both? > + int (*set_cqm_rssi_range_config)(struct wiphy *wiphy, > + struct net_device *dev, > + s32 rssi_low, s32 rssi_high); Seems there still should be a hysteresis? Or am I misunderstanding the intent here? I.e. isn't it meant to report low/medium/high later? > diff --git a/include/net/mac80211.h b/include/net/mac80211.h > index 33026e1..7da1056 100644 > --- a/include/net/mac80211.h > +++ b/include/net/mac80211.h I'd prefer you split cfg80211 and mac80211 patches, i.e. provide the new API first and then implement it in mac80211 separately. > +void cfg80211_cqm_config_free(struct wireless_dev *wdev) > +{ > + if (!wdev->cqm_config) > + return; > + > + kfree(wdev->cqm_config->rssi_thresholds); > + kfree(wdev->cqm_config); > + wdev->cqm_config = NULL; > +} You can save this complexity by just making the cqm_config struct have all the thresholds inside itself - pretty easy to allocate by just counting them first. johannes
Re: [PATCH 2/2] cfg80211: Add support to sched scan to report better BSSs
On Mon, 2016-11-28 at 15:20 +0100, Johannes Berg wrote: > > It seems there has already > > been taken a shot at this which may be used/extended [1]. > > > > That's a good point - it's somewhat similar. > > This is obviously a different context - offloaded BSS selection vs. > scheduled scan (for host BSS selection), but perhaps the attribute & > definitions could be reused? Yes, similar but not quite the same. The existing case is for finding BSSs that are worth waking the host up for (while disconnected), so it needs to be better than the RSSI passed (absolute number). Now this is about relative RSSI as compared to the current connection, so RELATIVE_RSSI is different than RSSI and I think the same attribute should not be used, to avoid confusion. -- Luca.
Re: [RFC V2 4/5] nl80211: add support for gscan
> * the nl80211 feature flags for the device. > + * @NL80211_SCAN_FLAGS_IE_DATA: request the device to supply IE data > in the > + * request. What does that mean? > + * @NL80211_GSCAN_CHAN_ATTR_NO_IR: scanning should be done passive. why not call that passive? No-IR is something we use in regulatory code to be more generic than "passive" (since it's also about beaconing etc.) but here? > + * @NL80211_GSCAN_CHAN_ATTR_MAX: highest GScan channel attribute. Generally, you should also document the attribute types here (and everywhere else really) > + NL80211_BUCKET_BAND_2GHZ= (1 << 0), no need for parentheses with enums :) > + if (tb[NL80211_GSCAN_CHAN_ATTR_DWELL_TIME]) > + chan->dwell_time = > nla_get_u32(tb[NL80211_GSCAN_CHAN_ATTR_DWELL_TIME]); Maybe that should have some kind of "reasonable range" limit? So I mostly looked at this from a pure code POV - need to compare with our implementation, but I guess the basis is the same ... johannes
Re: [RFC V2 1/5] nl80211: allow reporting RTT information in scan results
> + * @distance: distance to AP with %parent_bssid in centimeters. Zero > + * value indicates this is undetermined. > + * @var_distance: variance of %distance indicating accurracy. accuracy The distance between the two APs? Where are you even obtaining that information from? johannes
Re: Break-it testing for wifi
On Tue, 2016-11-22 at 08:59 -0800, Ben Greear wrote: > > Why would you do that? In order to test the AP implementation? > > Yes. And really, you should be able to do similar things on the AP > to test stations, or on IBSS/Mesh devices, etc. hostapd already has > some options to corrupt or drop a percentage of various management > frames. Supplicant does not as far as I know. Yes, but that's a far different focus. I'm far more interested in testing *our* implementation(s), at which point I don't need to do it over the air for most cases, and can be much more efficient that way, etc. I also don't really see a point of doing anything over the air to test our implementations, apart from hitting firmware (but even then ...) > > > 2) Randomly corrupt mgt frames in driver and/or mac80211 stack > > > and/or supplicant. > > > > I think fuzzing the input path for those frames would be more > > useful than just corrupting things. > > Random corruptions, especially by code that had at least some > understanding of management frames should be fast and easy to > use. It would not be as good as a really clever fuzzer or hand- > crafted frames, but for many users, hand-crafting attacks would be > well beyond what they could ever accomplish. Define "user"? > Currently, supplicant can (at least with some small patches that I > carry), add custom information elements to probe requests and > similar. But, some things are built by mac80211 (rate-sets > advertised, for instance). So, I was thinking of slightly extending > the API so that user-space could over-ride whatever mac80211 might > normally build itself. That lets you more properly fuzz things from > user space. Why bother though, at that point? If you hook up some state transitions you can probably just send the frames from userspace. For these testing scenarios you can make assumptions about the hardware or query it beforehand, so there's no need to rely on mac80211 at all? > Yes, but for ease of use, and to cover frames generated by mac80211, > I was thinking: > > echo 0.25 > /debug/.../wlan0/mgt_fuzzer > > The fuzzer would then corrupt 25% of the management frames. And > instead of just randomly scribbling, it could also parse the frames > and do some more clever (and still pseudo-random) modifications to > the frames, like re-writing IEs with bad lengths, flipping bits in > specific portions of the frame we feel might find problems, etc. > > I think if the tool became useful, then it could grow more clever > over time. I'd be far more inclined to just add a tracepoint there at some spot and allow attaching BPF programs to it ... Or perhaps allow attaching BPF programs directly, if tracepoint BPF can't modify the data. Having any kind of logic here in the kernel space seems fairly useless since you'll want to change it often, etc. johannes
Re: [PATCH 2/2] cfg80211: Add support to sched scan to report better BSSs
> It seems there has already > been taken a shot at this which may be used/extended [1]. > That's a good point - it's somewhat similar. This is obviously a different context - offloaded BSS selection vs. scheduled scan (for host BSS selection), but perhaps the attribute & definitions could be reused? johannes
Re: [PATCH 2/2] cfg80211: Add support to sched scan to report better BSSs
On Thu, 2016-11-24 at 00:07 +0200, Jouni Malinen wrote: > > + int bbr; bbr? Also, bool? johannes
Re: [PATCH] mmc: pwrseq: add support for Marvell SD8787 chip
On Thu, Nov 17, 2016 at 05:55:09PM -0800, Matt Ranostay wrote: > Allow power sequencing for the Marvell SD8787 Wifi/BT chip. > This can be abstracted to other chipsets if needed in the future. I don't like the MMC pwrseq bindings, so I won't be acking any. Look at the USB pwrseq for why. > Cc: Tony Lindgren > Cc: Ulf Hansson > Cc: Mark Rutland > Cc: Srinivas Kandagatla > Signed-off-by: Matt Ranostay > --- > .../devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt | 14 +++ > .../bindings/net/wireless/marvell-sd8xxx.txt | 4 + > drivers/mmc/core/Kconfig | 10 ++ > drivers/mmc/core/Makefile | 1 + > drivers/mmc/core/pwrseq_sd8787.c | 117 > + > 5 files changed, 146 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt > create mode 100644 drivers/mmc/core/pwrseq_sd8787.c > > diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt > b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt > new file mode 100644 > index ..1b658351629b > --- /dev/null > +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt > @@ -0,0 +1,14 @@ > +* Marvell SD8787 power sequence provider > + > +Required properties: > +- compatible: must be "mmc-pwrseq-sd8787". > +- pwndn-gpio: contains a power down GPIO specifier. powerdown-gpios > +- reset-gpio: contains a reset GPIO specifier. reset-gpios > + > +Example: > + > + wifi_pwrseq: wifi_pwrseq { > + compatible = "mmc-pwrseq-sd8787"; > + pwrdn-gpio = <&twl_gpio 0 GPIO_ACTIVE_LOW>; > + reset-gpio = <&twl_gpio 1 GPIO_ACTIVE_LOW>; > + } > diff --git > a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt > b/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt > index c421aba0a5bc..08fd65d35725 100644 > --- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt > +++ b/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt > @@ -32,6 +32,9 @@ Optional properties: >so that the wifi chip can wakeup host platform under certain > condition. >during system resume, the irq will be disabled to make sure >unnecessary interrupt is not received. > + - vmmc-supply: a phandle of a regulator, supplying VCC to the card This is why pwrseq is wrong. You have some properties in the card node and some in pwrseq node. Everything should be in the card node. > + - mmc-pwrseq: phandle to the MMC power sequence node. See "mmc-pwrseq-*" > + for documentation of MMC power sequence bindings. > > Example: > > @@ -44,6 +47,7 @@ so that firmware can wakeup host using this device side pin. > &mmc3 { > status = "okay"; > vmmc-supply = <&wlan_en_reg>; > + mmc-pwrseq = <&wifi_pwrseq>; > bus-width = <4>; > cap-power-off-card; > keep-power-in-suspend;
Re: [PATCH 1/2] nl80211: Use different attrs for BSSID and random MAC addr in scan req
> > + * @NL80211_ATTR_BSSID: The BSSID of the AP (various uses). Note > > that > > The BSSID may also be used for other things, like P2P GO, right? Also > "various uses" is probably unnecessary? Every command using this > attribute should describe it's use in their description. > > > > > > + * %NL80211_ATTR_MAC has also been used in various > > commands/events for > > + * specifying the BSSID. > > This can be a bit confusing. Maybe you can specify which commands > *used* to use NL80211_ATTR_MAC but now use NL80211_ATTR_BSSID? I'd actually avoid that, let's not make the "compatibility quirks" part of the documentation people read through normally ... In the code, that's fine, but here, I think less so. With that, perhaps just rephrase this to "The BSSID of the AP. Note that %NL80211_ATTR_MAC is also used in various commands/events for specifying the BSSID." > > - if (info->attrs[NL80211_ATTR_MAC]) > > + /* Initial implementation used NL80211_ATTR_MAC to set the > > specific > > + * BSSID to scan for. This was problematic because that > > same attribute > > + * was already used for another purpose (local random MAC > > address). The > > + * NL80211_ATTR_BSSID attribute was added to fix this. For > > backwards > > + * compatibility with older userspace components, also use > > the > > + * NL80211_ATTR_MAC value here if it can be determined to > > be used for > > + * the specific BSSID use case instead of the random MAC > > address > > + * (NL80211_ATTR_SCAN_FLAGS is used to enable random MAC > > address use). > > + */ > > You should probably add this information to the > NL80211_CMD_TRIGGER_SCAN description. It may need an update to refer to ATTR_BSSID, but again I don't think all the compatibility discussion should be there. > > > > > + if (info->attrs[NL80211_ATTR_BSSID]) > > + memcpy(request->bssid, > > + nla_data(info->attrs[NL80211_ATTR_BSSID]), > > ETH_ALEN); > > + else if (!info->attrs[NL80211_ATTR_SCAN_FLAGS] && > > You should actually check that the SCAN_FLAGS attribute either > doesn't > exist (as you already do) or, if it exists, that it doesn't have the > NL80211_SCAN_FLAG_RANDOM_ADDR flags. > Agree with that, that would make sense. johannes
Re: [PATCH 0/4] Fix -Wunused-but-set-variable in net/mac80211/
On Wed, 2016-11-23 at 20:45 -0800, Kirtika Ruchandani wrote: > This patchset is part of the effort led by Arnd Bergmann to clean up > warnings in the kernel. This and following patchsets will focus on > "-Wunused-but-set-variable" as it among the noisier ones. These were > found compiling with W=1. All four applied, thanks. johannes
wireless-drivers-next soon closing for 4.10
Linus said that he will release 4.9 either next Sunday or a week after, which means that the merge window is getting close. If there's something you absolutely need to have in 4.10 better send them NOW to not miss the window. Important bug fixes can go to 4.10 also after the merge window, but not new features. -- Kalle Valo
Re: linux-next: build warning after merge of the wireless-drivers-next tree
Kalle Valo writes: > Barry Day writes: > >> On Mon, Nov 28, 2016 at 09:25:30AM +0200, Kalle Valo wrote: >>> Stephen Rothwell writes: >>> >>> > Hi all, >>> > >>> > After merging the wireless-drivers-next tree, today's linux-next build >>> > (x86_64 allmodconfig) produced this warning: >>> > >>> > In file included from include/linux/usb/ch9.h:35:0, >>> > from include/linux/usb.h:5, >>> > from >>> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:32: >>> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In function >>> > 'rtl8xxxu_fill_txdesc_v2': >>> > include/linux/device.h:1214:36: warning: 'rate' may be used uninitialized >>> > in this function [-Wmaybe-uninitialized] >>> > #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg) >>> > ^ >>> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:4841:6: note: >>> > 'rate' was declared here >>> > u32 rate; >>> > ^ >>> > >>> > Introduced by commit >>> > >>> > b4c3d9cfb607 ("rtl8xxxu: Pass tx_info to fill_txdesc in order to have >>> > access to retry count") >>> > >>> > This is a correct diagnosis. >>> >>> Thanks for the report. Jes, can you send a patch to fix this? (Unless >>> someone else beats to it.) >>> >>> -- >>> Kalle Valo >> >> I posted a patch on the 26th that fixes this > > Thanks, I see it: > > https://patchwork.kernel.org/patch/9448225/ > > The commit log doesn't mention anything about the compilation warning, > I'll add that. Also a Fixes line is nice to have. I'm happy with this fix Acked-by: Jes Sorensen
Re: rtl8xxxu: tx rate reported before set
Kalle Valo writes: > Barry Day wrote: >> Move the dev_info call that attempts to show the rate used before it is set. >> >> Signed-off-by: Barry Day > > Jes, I would like to take this directly to fix the compiler warning. Also I'll > change the commit log to: Kalle, I'm totally fine with this, please go ahead. Barry thanks for fixing this. Cheers, Jes > rtl8xxxu: tx rate reported before set > > Move the dev_info call that attempts to show the rate used before it is set. > Fixes a compiler warning: > > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In function > 'rtl8xxxu_fill_txdesc_v2': > include/linux/device.h:1214:36: warning: 'rate' may be used > uninitialized in this function [-Wmaybe-uninitialized] > #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg) > > Fixes: b4c3d9cfb607 ("rtl8xxxu: Pass tx_info to fill_txdesc in order > to have access to retry count") > Reported-by: Stephen Rothwell > Signed-off-by: Barry Day
Re: rtl8xxxu: tx rate reported before set
Barry Day wrote: > Move the dev_info call that attempts to show the rate used before it is set. > > Signed-off-by: Barry Day Jes, I would like to take this directly to fix the compiler warning. Also I'll change the commit log to: rtl8xxxu: tx rate reported before set Move the dev_info call that attempts to show the rate used before it is set. Fixes a compiler warning: drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In function 'rtl8xxxu_fill_txdesc_v2': include/linux/device.h:1214:36: warning: 'rate' may be used uninitialized in this function [-Wmaybe-uninitialized] #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg) Fixes: b4c3d9cfb607 ("rtl8xxxu: Pass tx_info to fill_txdesc in order to have access to retry count") Reported-by: Stephen Rothwell Signed-off-by: Barry Day -- https://patchwork.kernel.org/patch/9448225/ Documentation about submitting wireless patches and checking status from patchwork: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
[PATCH 3/4] wil6210: add debugfs blobs for UCODE code and data
From: Lior David Added new areas to fw_mappings area for UCODE code and data areas. The new areas are only exposed through debugfs blobs, and mainly needed to access UCODE logs. The change does not affect crash dumps because the newly added areas overlap with the "upper" area which is already dumped. Signed-off-by: Lior David Signed-off-by: Maya Erez --- drivers/net/wireless/ath/wil6210/wil6210.h| 3 +- drivers/net/wireless/ath/wil6210/wil_crash_dump.c | 6 drivers/net/wireless/ath/wil6210/wmi.c| 39 +++ 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h index ef95db9..237e166 100644 --- a/drivers/net/wireless/ath/wil6210/wil6210.h +++ b/drivers/net/wireless/ath/wil6210/wil6210.h @@ -276,10 +276,11 @@ struct fw_map { u32 to; /* linker address - to, exclusive */ u32 host; /* PCI/Host address - BAR0 + 0x88 */ const char *name; /* for debugfs */ + bool fw; /* true if FW mapping, false if UCODE mapping */ }; /* array size should be in sync with actual definition in the wmi.c */ -extern const struct fw_map fw_mapping[8]; +extern const struct fw_map fw_mapping[10]; /** * mk_cidxtid - construct @cidxtid field diff --git a/drivers/net/wireless/ath/wil6210/wil_crash_dump.c b/drivers/net/wireless/ath/wil6210/wil_crash_dump.c index b57d280..d051eea 100644 --- a/drivers/net/wireless/ath/wil6210/wil_crash_dump.c +++ b/drivers/net/wireless/ath/wil6210/wil_crash_dump.c @@ -36,6 +36,9 @@ static int wil_fw_get_crash_dump_bounds(struct wil6210_priv *wil, for (i = 1; i < ARRAY_SIZE(fw_mapping); i++) { map = &fw_mapping[i]; + if (!map->fw) + continue; + if (map->host < host_min) host_min = map->host; @@ -73,6 +76,9 @@ int wil_fw_copy_crash_dump(struct wil6210_priv *wil, void *dest, u32 size) for (i = 0; i < ARRAY_SIZE(fw_mapping); i++) { map = &fw_mapping[i]; + if (!map->fw) + continue; + data = (void * __force)wil->csr + HOSTADDR(map->host); len = map->to - map->from; offset = map->host - host_min; diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c index d289a4d..7585003 100644 --- a/drivers/net/wireless/ath/wil6210/wmi.c +++ b/drivers/net/wireless/ath/wil6210/wmi.c @@ -84,19 +84,29 @@ * array size should be in sync with the declaration in the wil6210.h */ const struct fw_map fw_mapping[] = { - {0x00, 0x04, 0x8c, "fw_code"}, /* FW code RAM 256k */ - {0x80, 0x808000, 0x90, "fw_data"}, /* FW data RAM 32k */ - {0x84, 0x86, 0x908000, "fw_peri"}, /* periph. data RAM 128k */ - {0x88, 0x88a000, 0x88, "rgf"}, /* various RGF 40k */ - {0x88a000, 0x88b000, 0x88a000, "AGC_tbl"}, /* AGC table 4k */ - {0x88b000, 0x88c000, 0x88b000, "rgf_ext"}, /* Pcie_ext_rgf 4k */ - {0x88c000, 0x88c200, 0x88c000, "mac_rgf_ext"}, /* mac_ext_rgf 512b */ - {0x8c, 0x949000, 0x8c, "upper"}, /* upper area 548k */ - /* -* 92..93 ucode code RAM -* 93..932000 ucode data RAM -* 932000..949000 back-door debug data + /* FW code RAM 256k */ + {0x00, 0x04, 0x8c, "fw_code", true}, + /* FW data RAM 32k */ + {0x80, 0x808000, 0x90, "fw_data", true}, + /* periph data 128k */ + {0x84, 0x86, 0x908000, "fw_peri", true}, + /* various RGF 40k */ + {0x88, 0x88a000, 0x88, "rgf", true}, + /* AGC table 4k */ + {0x88a000, 0x88b000, 0x88a000, "AGC_tbl", true}, + /* Pcie_ext_rgf 4k */ + {0x88b000, 0x88c000, 0x88b000, "rgf_ext", true}, + /* mac_ext_rgf 512b */ + {0x88c000, 0x88c200, 0x88c000, "mac_rgf_ext", true}, + /* upper area 548k */ + {0x8c, 0x949000, 0x8c, "upper", true}, + /* UCODE areas - accessible by debugfs blobs but not by +* wmi_addr_remap. UCODE areas MUST be added AFTER FW areas! */ + /* ucode code RAM 128k */ + {0x00, 0x02, 0x92, "uc_code", false}, + /* ucode data RAM 16k */ + {0x80, 0x804000, 0x94, "uc_data", false}, }; struct blink_on_off_time led_blink_time[] = { @@ -108,7 +118,7 @@ struct blink_on_off_time led_blink_time[] = { u8 led_polarity = LED_POLARITY_LOW_ACTIVE; /** - * return AHB address for given firmware/ucode internal (linker) address + * return AHB address for given firmware internal (linker) address * @x - internal address * If address have no valid AHB mapping, return 0 */ @@ -117,7 +127,8 @@ static u32 wmi_addr_remap(u32 x) uint i; for (i = 0; i < ARRAY_SIZE(fw_mapping); i++) { -
[PATCH 4/4] wil6210: align to latest auto generated wmi.h
From: Lior David Align to latest version of the auto generated wmi file describing the interface with FW. Signed-off-by: Lior David Signed-off-by: Maya Erez --- drivers/net/wireless/ath/wil6210/wmi.h | 392 ++--- 1 file changed, 264 insertions(+), 128 deletions(-) diff --git a/drivers/net/wireless/ath/wil6210/wmi.h b/drivers/net/wireless/ath/wil6210/wmi.h index 2cc7775..d93a4d4 100644 --- a/drivers/net/wireless/ath/wil6210/wmi.h +++ b/drivers/net/wireless/ath/wil6210/wmi.h @@ -35,6 +35,7 @@ #define WMI_MAC_LEN(6) #define WMI_PROX_RANGE_NUM (3) #define WMI_MAX_LOSS_DMG_BEACONS (20) +#define MAX_NUM_OF_SECTORS (128) /* Mailbox interface * used for commands and events @@ -68,144 +69,149 @@ struct wmi_cmd_hdr { /* List of Commands */ enum wmi_command_id { - WMI_CONNECT_CMDID = 0x01, - WMI_DISCONNECT_CMDID= 0x03, - WMI_DISCONNECT_STA_CMDID= 0x04, - WMI_START_SCAN_CMDID= 0x07, - WMI_SET_BSS_FILTER_CMDID= 0x09, - WMI_SET_PROBED_SSID_CMDID = 0x0A, - WMI_SET_LISTEN_INT_CMDID= 0x0B, - WMI_BCON_CTRL_CMDID = 0x0F, - WMI_ADD_CIPHER_KEY_CMDID= 0x16, - WMI_DELETE_CIPHER_KEY_CMDID = 0x17, - WMI_PCP_CONF_CMDID = 0x18, - WMI_SET_APPIE_CMDID = 0x3F, - WMI_SET_WSC_STATUS_CMDID= 0x41, - WMI_PXMT_RANGE_CFG_CMDID= 0x42, - WMI_PXMT_SNR2_RANGE_CFG_CMDID = 0x43, - WMI_MEM_READ_CMDID = 0x800, - WMI_MEM_WR_CMDID= 0x801, - WMI_ECHO_CMDID = 0x803, - WMI_DEEP_ECHO_CMDID = 0x804, - WMI_CONFIG_MAC_CMDID= 0x805, - WMI_CONFIG_PHY_DEBUG_CMDID = 0x806, - WMI_ADD_DEBUG_TX_PCKT_CMDID = 0x808, - WMI_PHY_GET_STATISTICS_CMDID= 0x809, - WMI_FS_TUNE_CMDID = 0x80A, - WMI_CORR_MEASURE_CMDID = 0x80B, - WMI_READ_RSSI_CMDID = 0x80C, - WMI_TEMP_SENSE_CMDID= 0x80E, - WMI_DC_CALIB_CMDID = 0x80F, - WMI_SEND_TONE_CMDID = 0x810, - WMI_IQ_TX_CALIB_CMDID = 0x811, - WMI_IQ_RX_CALIB_CMDID = 0x812, - WMI_SET_UCODE_IDLE_CMDID= 0x813, - WMI_SET_WORK_MODE_CMDID = 0x815, - WMI_LO_LEAKAGE_CALIB_CMDID = 0x816, - WMI_MARLON_R_READ_CMDID = 0x818, - WMI_MARLON_R_WRITE_CMDID= 0x819, - WMI_MARLON_R_TXRX_SEL_CMDID = 0x81A, - MAC_IO_STATIC_PARAMS_CMDID = 0x81B, - MAC_IO_DYNAMIC_PARAMS_CMDID = 0x81C, - WMI_SILENT_RSSI_CALIB_CMDID = 0x81D, - WMI_RF_RX_TEST_CMDID= 0x81E, - WMI_CFG_RX_CHAIN_CMDID = 0x820, - WMI_VRING_CFG_CMDID = 0x821, - WMI_BCAST_VRING_CFG_CMDID = 0x822, - WMI_VRING_BA_EN_CMDID = 0x823, - WMI_VRING_BA_DIS_CMDID = 0x824, - WMI_RCP_ADDBA_RESP_CMDID= 0x825, - WMI_RCP_DELBA_CMDID = 0x826, - WMI_SET_SSID_CMDID = 0x827, - WMI_GET_SSID_CMDID = 0x828, - WMI_SET_PCP_CHANNEL_CMDID = 0x829, - WMI_GET_PCP_CHANNEL_CMDID = 0x82A, - WMI_SW_TX_REQ_CMDID = 0x82B, - WMI_READ_MAC_RXQ_CMDID = 0x830, - WMI_READ_MAC_TXQ_CMDID = 0x831, - WMI_WRITE_MAC_RXQ_CMDID = 0x832, - WMI_WRITE_MAC_TXQ_CMDID = 0x833, - WMI_WRITE_MAC_XQ_FIELD_CMDID= 0x834, - WMI_MLME_PUSH_CMDID = 0x835, - WMI_BEAMFORMING_MGMT_CMDID = 0x836, - WMI_BF_TXSS_MGMT_CMDID = 0x837, - WMI_BF_SM_MGMT_CMDID= 0x838, - WMI_BF_RXSS_MGMT_CMDID = 0x839, - WMI_BF_TRIG_CMDID = 0x83A, - WMI_LINK_MAINTAIN_CFG_WRITE_CMDID = 0x842, - WMI_LINK_MAINTAIN_CFG_READ_CMDID= 0x843, - WMI_SET_SECTORS_CMDID = 0x849, - WMI_MAINTAIN_PAUSE_CMDID= 0x850, - WMI_MAINTAIN_RESUME_CMDID = 0x851, - WMI_RS_MGMT_CMDID = 0x852, - WMI_RF_MGMT_CMDID = 0x853, - WMI_THERMAL_THROTTLING_CTRL_CMDID = 0x854, - WMI_THERMAL_THROTTLING_GET_STATUS_CMDID =
[PATCH 2/4] wil6210: validate wil_pmc_alloc parameters
num_descriptors and descriptor_size needs to be checked for: 1) not being negative values 2) no overflow occurs when these are multiplied together as done in wil_pmc_read. An overflow of two signed integers is undefined behavior. Signed-off-by: Maya Erez --- drivers/net/wireless/ath/wil6210/pmc.c | 55 ++ 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/ath/wil6210/pmc.c b/drivers/net/wireless/ath/wil6210/pmc.c index 5ca0307..b9faae0 100644 --- a/drivers/net/wireless/ath/wil6210/pmc.c +++ b/drivers/net/wireless/ath/wil6210/pmc.c @@ -54,6 +54,7 @@ void wil_pmc_alloc(struct wil6210_priv *wil, struct pmc_ctx *pmc = &wil->pmc; struct device *dev = wil_to_dev(wil); struct wmi_pmc_cmd pmc_cmd = {0}; + int last_cmd_err = -ENOMEM; mutex_lock(&pmc->lock); @@ -62,6 +63,29 @@ void wil_pmc_alloc(struct wil6210_priv *wil, wil_err(wil, "%s: ERROR pmc is already allocated\n", __func__); goto no_release_err; } + if ((num_descriptors <= 0) || (descriptor_size <= 0)) { + wil_err(wil, + "Invalid params num_descriptors(%d), descriptor_size(%d)\n", + num_descriptors, descriptor_size); + last_cmd_err = -EINVAL; + goto no_release_err; + } + + if (num_descriptors > (1 << WIL_RING_SIZE_ORDER_MAX)) { + wil_err(wil, + "num_descriptors(%d) exceeds max ring size %d\n", + num_descriptors, 1 << WIL_RING_SIZE_ORDER_MAX); + last_cmd_err = -EINVAL; + goto no_release_err; + } + + if (num_descriptors > INT_MAX / descriptor_size) { + wil_err(wil, + "Overflow in num_descriptors(%d)*descriptor_size(%d)\n", + num_descriptors, descriptor_size); + last_cmd_err = -EINVAL; + goto no_release_err; + } pmc->num_descriptors = num_descriptors; pmc->descriptor_size = descriptor_size; @@ -189,7 +213,7 @@ void wil_pmc_alloc(struct wil6210_priv *wil, pmc->descriptors = NULL; no_release_err: - pmc->last_cmd_status = -ENOMEM; + pmc->last_cmd_status = last_cmd_err; mutex_unlock(&pmc->lock); } @@ -295,7 +319,7 @@ ssize_t wil_pmc_read(struct file *filp, char __user *buf, size_t count, size_t retval = 0; unsigned long long idx; loff_t offset; - size_t pmc_size = pmc->descriptor_size * pmc->num_descriptors; + size_t pmc_size; mutex_lock(&pmc->lock); @@ -306,6 +330,8 @@ ssize_t wil_pmc_read(struct file *filp, char __user *buf, size_t count, return -EPERM; } + pmc_size = pmc->descriptor_size * pmc->num_descriptors; + wil_dbg_misc(wil, "%s: size %u, pos %lld\n", __func__, (unsigned)count, *f_pos); @@ -345,7 +371,18 @@ loff_t wil_pmc_llseek(struct file *filp, loff_t off, int whence) loff_t newpos; struct wil6210_priv *wil = filp->private_data; struct pmc_ctx *pmc = &wil->pmc; - size_t pmc_size = pmc->descriptor_size * pmc->num_descriptors; + size_t pmc_size; + + mutex_lock(&pmc->lock); + + if (!wil_is_pmc_allocated(pmc)) { + wil_err(wil, "error, pmc is not allocated!\n"); + pmc->last_cmd_status = -EPERM; + mutex_unlock(&pmc->lock); + return -EPERM; + } + + pmc_size = pmc->descriptor_size * pmc->num_descriptors; switch (whence) { case 0: /* SEEK_SET */ @@ -361,15 +398,21 @@ loff_t wil_pmc_llseek(struct file *filp, loff_t off, int whence) break; default: /* can't happen */ - return -EINVAL; + newpos = -EINVAL; + goto out; } - if (newpos < 0) - return -EINVAL; + if (newpos < 0) { + newpos = -EINVAL; + goto out; + } if (newpos > pmc_size) newpos = pmc_size; filp->f_pos = newpos; +out: + mutex_unlock(&pmc->lock); + return newpos; } -- 1.9.1
[PATCH 1/4] wil6210: delay remain on channel when scan is active
From: Lior David Currently it was possible to call remain_on_channel(ROC) while scan was active and this caused a crash in the FW. In order to fix this problem and make the behavior consistent with other drivers, queue the ROC in case a scan is active and try it again when scan is done. As part of the fix, clean up some locking issues and return error if scan is called while ROC is active. Signed-off-by: Lior David Signed-off-by: Maya Erez --- drivers/net/wireless/ath/wil6210/cfg80211.c | 45 - drivers/net/wireless/ath/wil6210/main.c | 2 + drivers/net/wireless/ath/wil6210/p2p.c | 150 +--- drivers/net/wireless/ath/wil6210/wil6210.h | 9 +- drivers/net/wireless/ath/wil6210/wmi.c | 4 + 5 files changed, 149 insertions(+), 61 deletions(-) diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c index 22078b0..6aa3ff4 100644 --- a/drivers/net/wireless/ath/wil6210/cfg80211.c +++ b/drivers/net/wireless/ath/wil6210/cfg80211.c @@ -354,14 +354,6 @@ static int wil_cfg80211_scan(struct wiphy *wiphy, wil_dbg_misc(wil, "%s(), wdev=0x%p iftype=%d\n", __func__, wdev, wdev->iftype); - mutex_lock(&wil->p2p_wdev_mutex); - if (wil->scan_request) { - wil_err(wil, "Already scanning\n"); - mutex_unlock(&wil->p2p_wdev_mutex); - return -EAGAIN; - } - mutex_unlock(&wil->p2p_wdev_mutex); - /* check we are client side */ switch (wdev->iftype) { case NL80211_IFTYPE_STATION: @@ -378,12 +370,24 @@ static int wil_cfg80211_scan(struct wiphy *wiphy, return -EBUSY; } + mutex_lock(&wil->mutex); + + mutex_lock(&wil->p2p_wdev_mutex); + if (wil->scan_request || wil->p2p.discovery_started) { + wil_err(wil, "Already scanning\n"); + mutex_unlock(&wil->p2p_wdev_mutex); + rc = -EAGAIN; + goto out; + } + mutex_unlock(&wil->p2p_wdev_mutex); + /* social scan on P2P_DEVICE is handled as p2p search */ if (wdev->iftype == NL80211_IFTYPE_P2P_DEVICE && wil_p2p_is_social_scan(request)) { if (!wil->p2p.p2p_dev_started) { wil_err(wil, "P2P search requested on stopped P2P device\n"); - return -EIO; + rc = -EIO; + goto out; } wil->scan_request = request; wil->radio_wdev = wdev; @@ -392,7 +396,7 @@ static int wil_cfg80211_scan(struct wiphy *wiphy, wil->radio_wdev = wil_to_wdev(wil); wil->scan_request = NULL; } - return rc; + goto out; } (void)wil_p2p_stop_discovery(wil); @@ -415,7 +419,7 @@ static int wil_cfg80211_scan(struct wiphy *wiphy, if (rc) { wil_err(wil, "set SSID for scan request failed: %d\n", rc); - return rc; + goto out; } wil->scan_request = request; @@ -448,7 +452,7 @@ static int wil_cfg80211_scan(struct wiphy *wiphy, rc = wmi_set_ie(wil, WMI_FRAME_PROBE_REQ, request->ie_len, request->ie); if (rc) - goto out; + goto out_restore; if (wil->discovery_mode && cmd.cmd.scan_type == WMI_ACTIVE_SCAN) { cmd.cmd.discovery_mode = 1; @@ -459,13 +463,14 @@ static int wil_cfg80211_scan(struct wiphy *wiphy, rc = wmi_send(wil, WMI_START_SCAN_CMDID, &cmd, sizeof(cmd.cmd) + cmd.cmd.num_channels * sizeof(cmd.cmd.channel_list[0])); -out: +out_restore: if (rc) { del_timer_sync(&wil->scan_timer); wil->radio_wdev = wil_to_wdev(wil); wil->scan_request = NULL; } - +out: + mutex_unlock(&wil->mutex); return rc; } @@ -988,16 +993,8 @@ static int wil_remain_on_channel(struct wiphy *wiphy, wil_dbg_misc(wil, "%s() center_freq=%d, duration=%d iftype=%d\n", __func__, chan->center_freq, duration, wdev->iftype); - rc = wil_p2p_listen(wil, duration, chan, cookie); - if (rc) - return rc; - - wil->radio_wdev = wdev; - - cfg80211_ready_on_channel(wdev, *cookie, chan, duration, - GFP_KERNEL); - - return 0; + rc = wil_p2p_listen(wil, wdev, duration, chan, cookie); + return rc; } static int wil_cancel_remain_on_channel(struct wiphy *wiphy, diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c index 70f9c07..e2e021b 100644 --- a/drivers/net/wireless/ath/wil6210/main.c +++ b/drivers/net/wireless/ath/wil6210/main.c @@ -518,6 +518,7 @@ int wil_priv_init(struct wil6210_priv *wil) INIT_WORK(&wil->wmi_event_worker, wmi_event_worker); INIT_WOR
[PATCH 0/4] wil6210 patches
Various wil6210 bug fixes. Lior David (3): wil6210: delay remain on channel when scan is active wil6210: add debugfs blobs for UCODE code and data wil6210: align to latest auto generated wmi.h Maya Erez (1): wil6210: validate wil_pmc_alloc parameters drivers/net/wireless/ath/wil6210/cfg80211.c | 45 ++- drivers/net/wireless/ath/wil6210/main.c | 2 + drivers/net/wireless/ath/wil6210/p2p.c| 150 +++-- drivers/net/wireless/ath/wil6210/pmc.c| 55 ++- drivers/net/wireless/ath/wil6210/wil6210.h| 12 +- drivers/net/wireless/ath/wil6210/wil_crash_dump.c | 6 + drivers/net/wireless/ath/wil6210/wmi.c| 43 ++- drivers/net/wireless/ath/wil6210/wmi.h| 392 +++--- 8 files changed, 495 insertions(+), 210 deletions(-) -- 1.9.1
Re: linux-next: build warning after merge of the wireless-drivers-next tree
Barry Day writes: > On Mon, Nov 28, 2016 at 09:25:30AM +0200, Kalle Valo wrote: >> Stephen Rothwell writes: >> >> > Hi all, >> > >> > After merging the wireless-drivers-next tree, today's linux-next build >> > (x86_64 allmodconfig) produced this warning: >> > >> > In file included from include/linux/usb/ch9.h:35:0, >> > from include/linux/usb.h:5, >> > from >> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:32: >> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In function >> > 'rtl8xxxu_fill_txdesc_v2': >> > include/linux/device.h:1214:36: warning: 'rate' may be used uninitialized >> > in this function [-Wmaybe-uninitialized] >> > #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg) >> > ^ >> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:4841:6: note: 'rate' >> > was declared here >> > u32 rate; >> > ^ >> > >> > Introduced by commit >> > >> > b4c3d9cfb607 ("rtl8xxxu: Pass tx_info to fill_txdesc in order to have >> > access to retry count") >> > >> > This is a correct diagnosis. >> >> Thanks for the report. Jes, can you send a patch to fix this? (Unless >> someone else beats to it.) >> >> -- >> Kalle Valo > > I posted a patch on the 26th that fixes this Thanks, I see it: https://patchwork.kernel.org/patch/9448225/ The commit log doesn't mention anything about the compilation warning, I'll add that. Also a Fixes line is nice to have. -- Kalle Valo
Re: [PATCH] bcma: add Dell Inspiron 3148
On 28 November 2016 at 08:57, Jiri Slaby wrote: > This is what is in the laptop: > 01:00.0 Network controller [0280]: Broadcom Limited BCM43142 802.11b/g/n > [14e4:4365] (rev 01) > Subsystem: Dell Device [1028:0018] > Flags: bus master, fast devsel, latency 0, IRQ 18 > Memory at b040 (64-bit, non-prefetchable) [size=32K] > Capabilities: [40] Power Management version 3 > Capabilities: [58] Vendor Specific Information: Len=78 > Capabilities: [48] MSI: Enable- Count=1/1 Maskable- 64bit+ > Capabilities: [d0] Express Endpoint, MSI 00 > Capabilities: [100] Advanced Error Reporting > Capabilities: [13c] Virtual Channel > Capabilities: [160] Device Serial Number 00-00-9a-ff-ff-f3-40-b8 > Capabilities: [16c] Power Budgeting > > With the patch, I can see: > bcma: bus0: Found chip with id 43142, rev 0x01 and package 0x08 > bcma: bus0: Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x28, class > 0x0) > bcma: bus0: Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x21, class > 0x0) > bcma: bus0: Core 2 found: PCIe (manuf 0x4BF, id 0x820, rev 0x16, class 0x0) > bcma: bus0: Core 3 found: UNKNOWN (manuf 0x43B, id 0x368, rev 0x00, class 0x0) > bcma: bus0: Bus registered > > The wifi is not currently supported by brcmsmac yet: > brcmsmac bcma1:1: mfg 4bf core 812 rev 33 class 0 irq 18 > brcmsmac: unknown device id 4365 > > So don't expect a working wifi from this patch :). > > Signed-off-by: Jiri Slaby > Cc: Rafał Miłecki > Cc: Looks good, thanks. -- Rafał
[PATCH] ath9k: Turn ath_txq_lock/unlock() into static inlines.
These are one-line functions that just call spin_lock/unlock_bh(); turn them into static inlines to avoid the function call overhead. Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/wireless/ath/ath9k/ath9k.h | 11 +-- drivers/net/wireless/ath/ath9k/xmit.c | 12 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h index 378d345..dca4aaa3 100644 --- a/drivers/net/wireless/ath/ath9k/ath9k.h +++ b/drivers/net/wireless/ath/ath9k/ath9k.h @@ -555,6 +555,15 @@ static inline void ath_chanctx_check_active(struct ath_softc *sc, #endif /* CONFIG_ATH9K_CHANNEL_CONTEXT */ +static inline void ath_txq_lock(struct ath_softc *sc, struct ath_txq *txq) +{ + spin_lock_bh(&txq->axq_lock); +} +static inline void ath_txq_unlock(struct ath_softc *sc, struct ath_txq *txq) +{ + spin_unlock_bh(&txq->axq_lock); +} + void ath_startrecv(struct ath_softc *sc); bool ath_stoprecv(struct ath_softc *sc); u32 ath_calcrxfilter(struct ath_softc *sc); @@ -562,8 +571,6 @@ int ath_rx_init(struct ath_softc *sc, int nbufs); void ath_rx_cleanup(struct ath_softc *sc); int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp); struct ath_txq *ath_txq_setup(struct ath_softc *sc, int qtype, int subtype); -void ath_txq_lock(struct ath_softc *sc, struct ath_txq *txq); -void ath_txq_unlock(struct ath_softc *sc, struct ath_txq *txq); void ath_txq_unlock_complete(struct ath_softc *sc, struct ath_txq *txq); void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq); bool ath_drain_all_txq(struct ath_softc *sc); diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index 486afa9..e5c0829 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -97,18 +97,6 @@ static void ath_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb) dev_kfree_skb(skb); } -void ath_txq_lock(struct ath_softc *sc, struct ath_txq *txq) - __acquires(&txq->axq_lock) -{ - spin_lock_bh(&txq->axq_lock); -} - -void ath_txq_unlock(struct ath_softc *sc, struct ath_txq *txq) - __releases(&txq->axq_lock) -{ - spin_unlock_bh(&txq->axq_lock); -} - void ath_txq_unlock_complete(struct ath_softc *sc, struct ath_txq *txq) __releases(&txq->axq_lock) { base-commit: 705d7aa062203226c8df73f18622664e30bd8a61 -- 2.10.2
[PATCH v3] ath9k: Introduce airtime fairness scheduling between stations
This reworks the ath9k driver to schedule transmissions to connected stations in a way that enforces airtime fairness between them. It accomplishes this by measuring the time spent transmitting to or receiving from a station at TX and RX completion, and accounting this to a per-station, per-QoS level airtime deficit. Then, an FQ-CoDel based deficit scheduler is employed at packet dequeue time, to control which station gets the next transmission opportunity. Airtime fairness can significantly improve the efficiency of the network when station rates vary. The following throughput values are from a simple three-station test scenario, where two stations operate at the highest HT20 rate, and one station at the lowest, and the scheduler is employed at the access point: Before / After Fast station 1:19.17 / 25.09 Mbps Fast station 2:19.83 / 25.21 Mbps Slow station: 2.58 /1.77 Mbps Total: 41.58 / 52.07 Mbps The benefit of airtime fairness goes up the more stations are present. In a 30-station test with one station artificially limited to 1 Mbps, we have seen aggregate throughput go from 2.14 to 17.76 Mbps. Signed-off-by: Toke Høiland-Jørgensen --- Changes since v2: - Just call spin_*lock_bh() instead of introducing ath_acq_*lock() functions. drivers/net/wireless/ath/ath9k/ath9k.h | 25 +++- drivers/net/wireless/ath/ath9k/channel.c | 14 ++- drivers/net/wireless/ath/ath9k/debug.c | 3 + drivers/net/wireless/ath/ath9k/debug.h | 13 +++ drivers/net/wireless/ath/ath9k/debug_sta.c | 54 + drivers/net/wireless/ath/ath9k/init.c | 2 + drivers/net/wireless/ath/ath9k/main.c | 6 +- drivers/net/wireless/ath/ath9k/recv.c | 65 +++ drivers/net/wireless/ath/ath9k/xmit.c | 177 + 9 files changed, 303 insertions(+), 56 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h index 378d345..79e4b71 100644 --- a/drivers/net/wireless/ath/ath9k/ath9k.h +++ b/drivers/net/wireless/ath/ath9k/ath9k.h @@ -112,6 +112,8 @@ int ath_descdma_setup(struct ath_softc *sc, struct ath_descdma *dd, #define ATH_TXFIFO_DEPTH 8 #define ATH_TX_ERROR 0x01 +#define ATH_AIRTIME_QUANTUM300 /* usec */ + /* Stop tx traffic 1ms before the GO goes away */ #define ATH_P2P_PS_STOP_TIME 1000 @@ -247,6 +249,9 @@ struct ath_atx_tid { bool has_queued; }; +void __ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid); +void ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid); + struct ath_node { struct ath_softc *sc; struct ieee80211_sta *sta; /* station struct we're part of */ @@ -258,9 +263,12 @@ struct ath_node { bool sleeping; bool no_ps_filter; + s64 airtime_deficit[IEEE80211_NUM_ACS]; + u32 airtime_rx_start; #ifdef CONFIG_ATH9K_STATION_STATISTICS struct ath_rx_rate_stats rx_rate_stats; + struct ath_airtime_stats airtime_stats; #endif u8 key_idx[4]; @@ -317,10 +325,16 @@ struct ath_rx { /* Channel Context */ /***/ +struct ath_acq { + struct list_head acq_new; + struct list_head acq_old; + spinlock_t lock; +}; + struct ath_chanctx { struct cfg80211_chan_def chandef; struct list_head vifs; - struct list_head acq[IEEE80211_NUM_ACS]; + struct ath_acq acq[IEEE80211_NUM_ACS]; int hw_queue_base; /* do not dereference, use for comparison only */ @@ -575,6 +589,8 @@ void ath_txq_schedule_all(struct ath_softc *sc); int ath_tx_init(struct ath_softc *sc, int nbufs); int ath_txq_update(struct ath_softc *sc, int qnum, struct ath9k_tx_queue_info *q); +u32 ath_pkt_duration(struct ath_softc *sc, u8 rix, int pktlen, +int width, int half_gi, bool shortPreamble); void ath_update_max_aggr_framelen(struct ath_softc *sc, int queue, int txop); void ath_assign_seq(struct ath_common *common, struct sk_buff *skb); int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb, @@ -963,6 +979,11 @@ void ath_ant_comb_scan(struct ath_softc *sc, struct ath_rx_status *rs); #define ATH9K_NUM_CHANCTX 2 /* supports 2 operating channels */ +#define AIRTIME_USE_TX BIT(0) +#define AIRTIME_USE_RX BIT(1) +#define AIRTIME_USE_NEW_QUEUES BIT(2) +#define AIRTIME_ACTIVE(flags) (!!(flags & (AIRTIME_USE_TX|AIRTIME_USE_RX))) + struct ath_softc { struct ieee80211_hw *hw; struct device *dev; @@ -1005,6 +1026,8 @@ struct ath_softc { short nbcnvifs; unsigned long ps_usecount; + u16 airtime_flags; /* AIRTIME_* */ + struct ath_rx rx; struct ath_tx tx; struct ath_beacon beacon; diff --git a/drivers/net/wireless/ath/ath9k/channel.c b/drivers/net/wireless/ath/ath9k/channel.c index 929dd70..b84539d 100644 --- a/drivers/net/wireless/ath
Re: [PATCH v2] ath9k: Introduce airtime fairness scheduling between stations
Felix Fietkau writes: > On 2016-11-27 16:58, Toke Høiland-Jørgensen wrote: >> "Valo, Kalle" writes: >> >>> (The make-wifi-fast list is annoying as it always spams me when it's on >>> CC, so dropped it.) >>> >>> Toke Høiland-Jørgensen writes: >>> This reworks the ath9k driver to schedule transmissions to connected stations in a way that enforces airtime fairness between them. It accomplishes this by measuring the time spent transmitting to or receiving from a station at TX and RX completion, and accounting this to a per-station, per-QoS level airtime deficit. Then, an FQ-CoDel based deficit scheduler is employed at packet dequeue time, to control which station gets the next transmission opportunity. Airtime fairness can significantly improve the efficiency of the network when station rates vary. The following throughput values are from a simple three-station test scenario, where two stations operate at the highest HT20 rate, and one station at the lowest, and the scheduler is employed at the access point: Before / After Fast station 1:19.17 / 25.09 Mbps Fast station 2:19.83 / 25.21 Mbps Slow station: 2.58 /1.77 Mbps Total: 41.58 / 52.07 Mbps The benefit of airtime fairness goes up the more stations are present. In a 30-station test with one station artificially limited to 1 Mbps, we have seen aggregate throughput go from 2.14 to 17.76 Mbps. Signed-off-by: Toke Høiland-Jørgensen >>> >>> [...] >>> +void ath_acq_lock(struct ath_softc *sc, struct ath_acq *acq) + __acquires(&acq->lock) +{ + spin_lock_bh(&acq->lock); +} + +void ath_acq_unlock(struct ath_softc *sc, struct ath_acq *acq) + __releases(&acq->lock) +{ + spin_unlock_bh(&acq->lock); +} >>> >>> Why these? To me it looks like they just add an extra function jump and >>> unneccessary extra layer. >> >> Well, there's already similar functions for the txq lock (ath_txq_lock() >> and ath_txq_unlock() in xmit.c), so figured I'd be consistent with >> those. And also that the __acquires and __releases macros were probably >> useful. >> >> Also, won't the compiler automatically inline them? > Not necessarily, these functions are not static. I think it would be a > good idea to turn the ath_txq_lock/unlock functions into static inline > functions as well. Right, I'll re-send with these functions fixed, and send a separate patch to fix ath_txq_lock* > Please don't blindly repeat patterns that are already there, some of > them might just not make any sense at all ;) But that would imply that kernel developers are not infallible. Surely that can't be right? ;) -Toke
Re: [PATCH v2] ath9k: Introduce airtime fairness scheduling between stations
On 2016-11-27 16:58, Toke Høiland-Jørgensen wrote: > "Valo, Kalle" writes: > >> (The make-wifi-fast list is annoying as it always spams me when it's on >> CC, so dropped it.) >> >> Toke Høiland-Jørgensen writes: >> >>> This reworks the ath9k driver to schedule transmissions to connected >>> stations in a way that enforces airtime fairness between them. It >>> accomplishes this by measuring the time spent transmitting to or >>> receiving from a station at TX and RX completion, and accounting this to >>> a per-station, per-QoS level airtime deficit. Then, an FQ-CoDel based >>> deficit scheduler is employed at packet dequeue time, to control which >>> station gets the next transmission opportunity. >>> >>> Airtime fairness can significantly improve the efficiency of the network >>> when station rates vary. The following throughput values are from a >>> simple three-station test scenario, where two stations operate at the >>> highest HT20 rate, and one station at the lowest, and the scheduler is >>> employed at the access point: >>> >>> Before / After >>> Fast station 1:19.17 / 25.09 Mbps >>> Fast station 2:19.83 / 25.21 Mbps >>> Slow station: 2.58 /1.77 Mbps >>> Total: 41.58 / 52.07 Mbps >>> >>> The benefit of airtime fairness goes up the more stations are present. >>> In a 30-station test with one station artificially limited to 1 Mbps, >>> we have seen aggregate throughput go from 2.14 to 17.76 Mbps. >>> >>> Signed-off-by: Toke Høiland-Jørgensen >> >> [...] >> >>> +void ath_acq_lock(struct ath_softc *sc, struct ath_acq *acq) >>> + __acquires(&acq->lock) >>> +{ >>> + spin_lock_bh(&acq->lock); >>> +} >>> + >>> +void ath_acq_unlock(struct ath_softc *sc, struct ath_acq *acq) >>> + __releases(&acq->lock) >>> +{ >>> + spin_unlock_bh(&acq->lock); >>> +} >> >> Why these? To me it looks like they just add an extra function jump and >> unneccessary extra layer. > > Well, there's already similar functions for the txq lock (ath_txq_lock() > and ath_txq_unlock() in xmit.c), so figured I'd be consistent with > those. And also that the __acquires and __releases macros were probably > useful. > > Also, won't the compiler automatically inline them? Not necessarily, these functions are not static. I think it would be a good idea to turn the ath_txq_lock/unlock functions into static inline functions as well. Please don't blindly repeat patterns that are already there, some of them might just not make any sense at all ;) - Felix
Re: linux-next: build warning after merge of the wireless-drivers-next tree
On Mon, Nov 28, 2016 at 09:25:30AM +0200, Kalle Valo wrote: > Stephen Rothwell writes: > > > Hi all, > > > > After merging the wireless-drivers-next tree, today's linux-next build > > (x86_64 allmodconfig) produced this warning: > > > > In file included from include/linux/usb/ch9.h:35:0, > > from include/linux/usb.h:5, > > from > > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:32: > > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In function > > 'rtl8xxxu_fill_txdesc_v2': > > include/linux/device.h:1214:36: warning: 'rate' may be used uninitialized > > in this function [-Wmaybe-uninitialized] > > #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg) > > ^ > > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:4841:6: note: 'rate' > > was declared here > > u32 rate; > > ^ > > > > Introduced by commit > > > > b4c3d9cfb607 ("rtl8xxxu: Pass tx_info to fill_txdesc in order to have > > access to retry count") > > > > This is a correct diagnosis. > > Thanks for the report. Jes, can you send a patch to fix this? (Unless > someone else beats to it.) > > -- > Kalle Valo I posted a patch on the 26th that fixes this Barry