[PATCH] ath10k: fix tlv 5ghz channel missing issue
From: Zhi Chen The 5ghz channel parameters of TLV target wasn't passed to host, it caused host can only use lower channels from 36 to 64. Signed-off-by: Zhi Chen --- drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c index 25efbb5..b8985ce 100644 --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c @@ -1010,6 +1010,8 @@ static int ath10k_wmi_tlv_op_pull_svc_rdy_ev(struct ath10k *ar, arg->phy_capab = ev->phy_capability; arg->num_rf_chains = ev->num_rf_chains; arg->eeprom_rd = reg->eeprom_rd; + arg->low_5ghz_chan = reg->low_5ghz_chan; + arg->high_5ghz_chan = reg->high_5ghz_chan; arg->num_mem_reqs = ev->num_mem_reqs; arg->service_map = svc_bmap; arg->service_map_len = ath10k_wmi_tlv_len(svc_bmap); -- 2.1.4
[PATCH] ath10k: fixed scan crash
From: Zhi Chen Length of WMI scan message was not calculated correctly. The allocated buffer was smaller than what we expected. So WMI message corrupted skb_info, which is at the end of skb->data. This fix takes TLV header into account even if the element is zero-length. Crash log: [49.629986] Unhandled kernel unaligned access[#1]: [49.634932] CPU: 0 PID: 1176 Comm: logd Not tainted 4.4.60 #180 [49.641040] task: 83051460 ti: 8329c000 task.ti: 8329c000 [49.646608] $ 0 : 0001 80984a80 [49.652038] $ 4 : 45259e89 8046d484 8046df30 8024ba70 [49.657468] $ 8 : 804cc4c0 0001 20306320 [49.662898] $12 : 33322037 000110f2 31203930 [49.668327] $16 : 82792b40 80984a80 0001 804207fc [49.673757] $20 : 012c 0040 8047 [49.679186] $24 : 8024af7c [49.684617] $28 : 8329c000 8329db88 0001 802c58d0 [49.690046] Hi: [49.693022] Lo: 453c [49.696013] epc : 800efae4 put_page+0x0/0x58 [49.700615] ra: 802c58d0 skb_release_data+0x148/0x1d4 [49.706184] Status: 1000fc03 KERNEL EXL IE [49.710531] Cause : 00800010 (ExcCode 04) [49.714669] BadVA : 45259e89 [49.717644] PrId : 00019374 (MIPS 24Kc) Signed-off-by: Zhi Chen --- drivers/net/wireless/ath/ath10k/wmi-tlv.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c index ae77a00..25efbb5 100644 --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c @@ -1515,10 +1515,10 @@ ath10k_wmi_tlv_op_gen_start_scan(struct ath10k *ar, bssid_len = arg->n_bssids * sizeof(struct wmi_mac_addr); ie_len = roundup(arg->ie_len, 4); len = (sizeof(*tlv) + sizeof(*cmd)) + - (arg->n_channels ? sizeof(*tlv) + chan_len : 0) + - (arg->n_ssids ? sizeof(*tlv) + ssid_len : 0) + - (arg->n_bssids ? sizeof(*tlv) + bssid_len : 0) + - (arg->ie_len ? sizeof(*tlv) + ie_len : 0); + sizeof(*tlv) + chan_len + + sizeof(*tlv) + ssid_len + + sizeof(*tlv) + bssid_len + + sizeof(*tlv) + ie_len; skb = ath10k_wmi_alloc_skb(ar, len); if (!skb) -- 2.1.4
Re: [PATCH] ath9k: introduce endian_check module parameter
On 14-03-18 15:34, Kalle Valo wrote: Bas Vermeulen writes: --- a/drivers/net/wireless/ath/ath9k/init.c +++ b/drivers/net/wireless/ath/ath9k/init.c @@ -67,6 +67,9 @@ static int ath9k_ps_enable; module_param_named(ps_enable, ath9k_ps_enable, int, 0444); MODULE_PARM_DESC(ps_enable, "Enable WLAN PowerSave"); +static int ath9k_endian_check; +module_param_named(endian_check, ath9k_endian_check, int, 0444); +MODULE_PARM_DESC(endian_check, "Check EEPROM for endianness compatibility"); #ifdef CONFIG_ATH9K_CHANNEL_CONTEXT int ath9k_use_chanctx; @@ -587,7 +590,8 @@ static int ath9k_of_init(struct ath_softc *sc) ether_addr_copy(common->macaddr, mac); ah->ah_flags &= ~AH_USE_EEPROM; - ah->ah_flags |= AH_NO_EEP_SWAP; + if (!ath9k_endian_check) + ah->ah_flags |= AH_NO_EEP_SWAP; A bit annoying to have a module parameter, isn't there any automatic way to detect/try this? But on the other hand I guess this isn't a common problem as nobody has reported this before? There is an automatic way to detect this, but that is disabled by the AH_NO_EEP_SWAP flag. Ah, I didn't check the code at all. The platform initialisation does not set this flag if the endian_check member of pdata is set to true, but there is no way to not set this when using a device tree. I used a module parameter instead of a device tree variable because I don't know of a way to modify the device tree my PowerMac boots with. Ok, makes sense. A module parameter is not an ideal solution I guess it's ok in this case. Kalle: Are there any changes you want me to make in order to get this accepted? I didn't see anything for me to resolve, but I may have missed something. I can submit a patch to not set the AH_NO_EEP_SWAP flag by default if you prefer, as that would fix my problem as well. I am just not sure that doesn't break things for some platform/device I don't have. I'm not really sure what to do. Basically this is a choise between bad for user experience (the module parameter) or risk of regressions (disable AH_NO_EEP_SWAP by default). As ath9k is used in very exotic hardware I'm starting to lean towards the module parameter approach (your patch) but would like to know what others think, especially Felix (CCed). Full patch here: https://patchwork.kernel.org/patch/10241731/ Just another ping. As I understood things, all OpenWRT dts' use qca,no-eeprom, and perhaps we could move ~AH_USE_EEPROM and |= AH_NO_EEP_SWAP in that if block. This would solve my problem, as I need to have AH_NO_EEP_SWAP removed from flags for my card (which is a little endian eeprom/card used on a big endian machine). Would you like me to prepare a patch for this? Is there anyone who can test this on the various OpenWRT boards/soc and or other configurations? I don't want to break things for other people. Bas Vermeulen -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.
Re: [ANN] mt76x0 usb driver
On Mon, Apr 09, 2018 at 04:53:44PM +0200, Lorenzo Bianconi wrote: > On Apr 09, Stanislaw Gruszka wrote: > > On Mon, Apr 09, 2018 at 04:26:42PM +0200, Lorenzo Bianconi wrote: > > > > I would like to integrate the driver to kernel via mt76 driver, i.e. > > > > add USB hooks and mt76x0 mac/phy code to mt76. This will open > > > > possibility to develop support for mt76x2 USB devices as well as mt76x0 > > > > PCIe devices in mt76. > > > > > > > > > > I have already started supporting mt76x2 USB devices in mt76 since > > > register map > > > is pretty similar to PCIe devices: > > > https://github.com/LorenzoBianconi/wireless-drivers-next/tree/mt76x2u > > > I added some usb utility routines so I think we can integrate mt76x0 in > > > mt76 as > > > well > > > > Great, I'll start to integrate mt76x0 on top of your tree. > > Cool :) actually this branch is in under development but what is really > changing day by day is mt7612u part not usb one so I guess we can use it as > common ground for mt76x2u and mt76x0. > Maybe even the mcu code is in common with mt76x2, isn't it? There are some new bits in mcu/mcu_and.c in Mediatek MT7612U driver, compared to MT7610U driver, but most of the code there looks pretty much the same :-) Regards Stanislaw
Re: [ANN] mt76x0 usb driver
Hi On Mon, Apr 09, 2018 at 08:45:20PM +0200, Hans Ulli Kroll wrote: > here are my changes for working 5Ghz band and more USB ID's form my side I applied the changes and pushed to github repo. Thanks Stanislaw
Re: [PATCH v13] ath10k: add LED and GPIO controlling support for various chipsets
Am 09.04.2018 um 17:49 schrieb Kalle Valo: Sebastian Gottschall writes: you removed the call to leds_start from certain locations but you seem to have ignored the comment i wrote above the function call. IIRC I moved the comment to ath10k_leds_start(). there is a reason why i reinitialize the gpio output state in these locations. the firmware for 9984 and 99xx resets the gpio registers at certain points. this will lead to a non working led code. What are the certain points exactly? I tried to be careful that from firmware's point of view the functionality is the same, even if I moved the call to a different location. Did you test the patch? Is it broken now? i reviewed the firmware code as well, but havent found the reason. can be core/chipset specific and not firmware software related. i just can say it doesnt happen on 988x, just newer chipsets are affected The naming refers to phases of ath10k initialisation which are: create() - destroy() register() - unregister() start() - stop() So the naming doesn't mean that every ath10k_foo_start() has to start something, it just describes the phase of driver initialisation it's called in. yes, but its not a initialisation too. its just a gpio pin reset. the initialisation is the trigger code itself from my point of view Is this really so frequently called that we need to think about CPU time? How often are we expecting the LED state to change? But I'm not really following you, from firmware point of view the functionality should be the same as with your patch. a typical tpt trigger as used in openwrt for instance may trigger it several times per second. i mean it might be really just a micro effect, but i just save cpu time whereever i can since i'm focused in embedded development so if you want to follow this up. remove ath10k_leds_start and insert ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0, WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE); in ath10k_leds_set_brightness_blocking Calling ath10k_wmi_gpio_config() every time sounds like quite odd to me and your patch didn't do that either. Are you sure this is really needed? yes. i tested this. you must set the gpio to output before setting it to any value, in case the output state was resetted back to input or any other default value. (which it does on 9984 at certain events) i tested this on a netgear r7800 which is ipq8064 based with 2 9984 chipsets. but i cannot reproduce this on mips routers with 988x chipsets. but as you say. its odd to call it every time. so i just call it with the reset method when neccessary. in our case, when interface operation starts. -- Mit freundlichen Grüssen / Regards Sebastian Gottschall / CTO NewMedia-NET GmbH - DD-WRT Firmensitz: Stubenwaldallee 21a, 64625 Bensheim Registergericht: Amtsgericht Darmstadt, HRB 25473 Geschäftsführer: Peter Steinhäuser, Christian Scheele http://www.dd-wrt.com email: s.gottsch...@dd-wrt.com Tel.: +496251-582650 / Fax: +496251-5826565
Re: iwlwifi-7265-ucode-25.30.14.0.tgz. in Centos 7
On Sun, Apr 08, 2018 at 08:58:35AM +0200, Dusan Bruncko wrote: > My problem is following: This firmware was created for kernel 4.2+, > but I am using > Centos 7 with kernels a la 3.10. Therefore I see during booting and Wireless stack and drivers are regularly updated in Centos kernel, (for example latest release has wireless drivers from 4.14) and should match firmware provided in the distribution. Regards Stanislaw
[PATCH 1/2] net: wireless: zydas: Replace mdelay with msleep in zd1201_probe
zd1201_probe() is never called in atomic context. zd1201_probe() is only set as ".probe" in struct usb_driver. Despite never getting called from atomic context, zd1201_probe() calls mdelay() to busily wait. This is not necessary and can be replaced with msleep() to avoid busy waiting. This is found by a static analysis tool named DCNS written by myself. And I also manually check it. Signed-off-by: Jia-Ju Bai --- drivers/net/wireless/zydas/zd1201.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/zydas/zd1201.c b/drivers/net/wireless/zydas/zd1201.c index 581e857..12774e9 100644 --- a/drivers/net/wireless/zydas/zd1201.c +++ b/drivers/net/wireless/zydas/zd1201.c @@ -1767,7 +1767,7 @@ static int zd1201_probe(struct usb_interface *interface, goto err_zd; } - mdelay(100); + msleep(100); err = zd1201_drvr_start(zd); if (err) goto err_zd; -- 1.9.1
[PATCH 2/2] net: wireless: zydas: Replace GFP_ATOMIC with GFP_KERNEL in zd1201_fw_upload
zd1201_probe() is never called in atomic context. zd1201_fw_upload() is only called by zd1201_probe(), which is only set as ".probe" in struct usb_driver. Despite never getting called from atomic context, zd1201_fw_upload() calls kmalloc() with GFP_ATOMIC, which does not sleep for allocation. GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL, which can sleep and improve the possibility of sucessful allocation. This is found by a static analysis tool named DCNS written by myself. And I also manually check it. Signed-off-by: Jia-Ju Bai --- drivers/net/wireless/zydas/zd1201.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/zydas/zd1201.c b/drivers/net/wireless/zydas/zd1201.c index 581e857..cba2bed 100644 --- a/drivers/net/wireless/zydas/zd1201.c +++ b/drivers/net/wireless/zydas/zd1201.c @@ -74,7 +74,7 @@ static int zd1201_fw_upload(struct usb_device *dev, int apfw) data = fw_entry->data; len = fw_entry->size; - buf = kmalloc(1024, GFP_ATOMIC); + buf = kmalloc(1024, GFP_KERNEL); if (!buf) { err = -ENOMEM; goto exit; -- 1.9.1
[PATCH] net: wireless: cisco: airo: Replace mdelay with usleep_range in flashgchar
flashgchar() is never called in atomic context. flashgchar() is only called by flashcard(). flashcard() calls copy_from_user() and kmalloc(GFP_KERNEL), which indicates this function is not called in atomic context. Despite never getting called from atomic context, flashgchar() calls mdelay() to busily wait. This is not necessary and can be replaced with usleep_range() to avoid busy waiting. This is found by a static analysis tool named DCNS written by myself. And I also manually check it. Signed-off-by: Jia-Ju Bai --- drivers/net/wireless/cisco/airo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c index 54201c0..45747b7 100644 --- a/drivers/net/wireless/cisco/airo.c +++ b/drivers/net/wireless/cisco/airo.c @@ -8106,7 +8106,7 @@ static int flashgchar(struct airo_info *ai,int matchbyte,int dwelltime){ if(dwelltime && !(0x8000 & rchar)){ dwelltime -= 10; - mdelay(10); + usleep_range(1, 11000); continue; } rbyte = 0xff & rchar; -- 1.9.1
[PATCH] net: wireless: b43legacy: Replace GFP_ATOMIC with GFP_KERNEL in dma_tx_fragment
dma_tx_fragment() is never called in atomic context. dma_tx_fragment() is only called by b43legacy_dma_tx(), which is only called by b43legacy_tx_work(). b43legacy_tx_work() is only set a parameter of INIT_WORK() in b43legacy_wireless_init(). Despite never getting called from atomic context, dma_tx_fragment() calls alloc_skb() with GFP_ATOMIC, which does not sleep for allocation. GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL, which can sleep and improve the possibility of sucessful allocation. This is found by a static analysis tool named DCNS written by myself. And I also manually check it. Signed-off-by: Jia-Ju Bai --- drivers/net/wireless/broadcom/b43legacy/dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c b/drivers/net/wireless/broadcom/b43legacy/dma.c index cfa617d..2f0c64c 100644 --- a/drivers/net/wireless/broadcom/b43legacy/dma.c +++ b/drivers/net/wireless/broadcom/b43legacy/dma.c @@ -1064,7 +1064,7 @@ static int dma_tx_fragment(struct b43legacy_dmaring *ring, meta->dmaaddr = map_descbuffer(ring, skb->data, skb->len, 1); /* create a bounce buffer in zone_dma on mapping failure. */ if (b43legacy_dma_mapping_error(ring, meta->dmaaddr, skb->len, 1)) { - bounce_skb = alloc_skb(skb->len, GFP_ATOMIC | GFP_DMA); + bounce_skb = alloc_skb(skb->len, GFP_KERNEL | GFP_DMA); if (!bounce_skb) { ring->current_slot = old_top_slot; ring->used_slots = old_used_slots; -- 1.9.1
[PATCH 0/4] Support for STA idle mode power save(IMPS)
From: Rakesh Pillai Enable STA idle mode power save(IMPS) for WCN3990 TLV target. In-order to support STA idle mode ps direct access to CE registers are protected via 2 mechanism. As target can power collapse based on idle inactivity and any target register access during that state can lead to un-clocked access. 2) Caching SRRI/DRRI in DDR. WN3990 has a shadow block in HW which is always on domain and allows HOST/APPS access irrespective of power state of the target(Common subsystem(includes CE block) might be down due to idle/inactivity). Any operation on the shadow registers are directly reflected back in the actual CE registers( SRWI/DRWI) once common subsystem gets up. The shadow registers configuration is supplied via QMI message. WC3990 has 24 shadow registers and mapping of shadow register(0-23) to CE registers(0-12) is as following. --- Shadow Register | CE |src/dst write index --- 0| 0| src 3| 3| src 4| 4| src 5| 5| src 7| 7| src 13 | 1| dst 14 | 2| dst 19 | 7| dst 20 | 8| dst 21 | 9 | dst 22 | 10 | dst 23 | 11| dst 1) Caching SRWI/SRWI in HW shadow block. Since shadow block allows only WRITE access, for read access(SRRI/DRRI) driver allocates region in DDR(12CE*half WORD) which is being configured in CE UPD control register. CE SRRI/DRRI are restored and replayed on the configured region(DDR) on each update. HOST/APPS reads SRRI/DRRI from the DDR to make the access independent of target power state. Govind Singh (2): ath10k: Enable SRRI/DRRI support on ddr for WCN3990 ath10k: Enable sta idle power save Rakesh Pillai (2): ath10k: Add hw params for shadow register support ath10k: Add support for shadow register for WNC3990 drivers/net/wireless/ath/ath10k/ce.c | 245 +++-- drivers/net/wireless/ath/ath10k/ce.h | 14 ++ drivers/net/wireless/ath/ath10k/core.c | 26 drivers/net/wireless/ath/ath10k/hw.c | 9 +- drivers/net/wireless/ath/ath10k/hw.h | 17 ++- drivers/net/wireless/ath/ath10k/mac.c | 7 + drivers/net/wireless/ath/ath10k/snoc.c | 3 + 7 files changed, 310 insertions(+), 11 deletions(-) -- 2.14.1
[PATCH 1/4] ath10k: Add hw params for shadow register support
From: Rakesh Pillai wcn3990 supports shadow register for ce write. Add a hw param for shadow register support. Signed-off-by: Rakesh Pillai --- drivers/net/wireless/ath/ath10k/core.c | 13 + drivers/net/wireless/ath/ath10k/hw.h | 4 2 files changed, 17 insertions(+) diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index 610bb32ec9f4..7e451b76c8a4 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -120,6 +120,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = QCA9887_HW_1_0_VERSION, @@ -150,6 +151,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = QCA6174_HW_2_1_VERSION, @@ -179,6 +181,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = QCA6174_HW_2_1_VERSION, @@ -208,6 +211,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = QCA6174_HW_3_0_VERSION, @@ -237,6 +241,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = QCA6174_HW_3_2_VERSION, @@ -269,6 +274,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = QCA99X0_HW_2_0_DEV_VERSION, @@ -304,6 +310,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = QCA9984_HW_1_0_DEV_VERSION, @@ -344,6 +351,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = QCA9888_HW_2_0_DEV_VERSION, @@ -383,6 +391,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = QCA9377_HW_1_0_DEV_VERSION, @@ -412,6 +421,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = QCA9377_HW_1_1_DEV_VERSION, @@ -443,6 +453,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = QCA4019_HW_1_0_DEV_VERSION, @@ -479,6 +490,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = false, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL, .per_ce_irq = false, + .shadow_reg_support = false, }, { .id = WCN3990_HW_1_0_DEV_VERSION, @@ -500,6 +512,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .target_64bit = true, .rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL_DUAL_MAC, .per_ce_irq = true, + .shadow_reg_support = true, }, }; diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h index 57dad208ef16..cf18adda9f20 100644
[PATCH 3/4] ath10k: Enable SRRI/DRRI support on ddr for WCN3990
From: Govind Singh SRRI/DRRI are not mapped in the HW Shadow block and can lead to un-clocked access if common subsystem in the target is powered down due to idle mode. To mitigate this problem SRRI/DRRI can be read from DDR instead of doing an actual hardware read. Host allocates non cached memory on ddr and configures the physical address of this memory to the CE hardware. The hardware updates the RRI on this particular location. Read SRRI/DRRI from DDR location instead of direct target read. Enable retention restore on ddr using hw params to enable in specific targets. Signed-off-by: Govind Singh Signed-off-by: Rakesh Pillai --- drivers/net/wireless/ath/ath10k/ce.c | 102 +++-- drivers/net/wireless/ath/ath10k/ce.h | 10 drivers/net/wireless/ath/ath10k/core.c | 13 + drivers/net/wireless/ath/ath10k/hw.c | 9 ++- drivers/net/wireless/ath/ath10k/hw.h | 13 - drivers/net/wireless/ath/ath10k/snoc.c | 3 + 6 files changed, 141 insertions(+), 9 deletions(-) diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c index 5053dd92bf01..b21fbedd0cad 100644 --- a/drivers/net/wireless/ath/ath10k/ce.c +++ b/drivers/net/wireless/ath/ath10k/ce.c @@ -185,11 +185,30 @@ static inline u32 ath10k_ce_src_ring_write_index_get(struct ath10k *ar, ar->hw_ce_regs->sr_wr_index_addr); } +static inline u32 ath10k_ce_src_ring_read_index_from_ddr(struct ath10k *ar, +u32 ce_id) +{ + struct ath10k_ce *ce = ath10k_ce_priv(ar); + + return ce->vaddr_rri[ce_id] & CE_DDR_RRI_MASK; +} + static inline u32 ath10k_ce_src_ring_read_index_get(struct ath10k *ar, u32 ce_ctrl_addr) { - return ath10k_ce_read32(ar, ce_ctrl_addr + - ar->hw_ce_regs->current_srri_addr); + struct ath10k_ce *ce = ath10k_ce_priv(ar); + u32 ce_id = COPY_ENGINE_ID(ce_ctrl_addr); + struct ath10k_ce_pipe *ce_state = &ce->ce_states[ce_id]; + u32 index; + + if (ar->hw_params.rri_on_ddr && + (ce_state->attr_flags & CE_ATTR_DIS_INTR)) + index = ath10k_ce_src_ring_read_index_from_ddr(ar, ce_id); + else + index = ath10k_ce_read32(ar, ce_ctrl_addr + +ar->hw_ce_regs->current_srri_addr); + + return index; } static inline void @@ -266,11 +285,31 @@ static inline void ath10k_ce_dest_ring_byte_swap_set(struct ath10k *ar, ath10k_set_ring_byte(n, ctrl_regs->dst_ring)); } +static inline + u32 ath10k_ce_dest_ring_read_index_from_ddr(struct ath10k *ar, u32 ce_id) +{ + struct ath10k_ce *ce = ath10k_ce_priv(ar); + + return (ce->vaddr_rri[ce_id] >> CE_DDR_DRRI_SHIFT) & + CE_DDR_RRI_MASK; +} + static inline u32 ath10k_ce_dest_ring_read_index_get(struct ath10k *ar, u32 ce_ctrl_addr) { - return ath10k_ce_read32(ar, ce_ctrl_addr + - ar->hw_ce_regs->current_drri_addr); + struct ath10k_ce *ce = ath10k_ce_priv(ar); + u32 ce_id = COPY_ENGINE_ID(ce_ctrl_addr); + struct ath10k_ce_pipe *ce_state = &ce->ce_states[ce_id]; + u32 index; + + if (ar->hw_params.rri_on_ddr && + (ce_state->attr_flags & CE_ATTR_DIS_INTR)) + index = ath10k_ce_dest_ring_read_index_from_ddr(ar, ce_id); + else + index = ath10k_ce_read32(ar, ce_ctrl_addr + +ar->hw_ce_regs->current_drri_addr); + + return index; } static inline void ath10k_ce_dest_ring_base_addr_set(struct ath10k *ar, @@ -486,7 +525,7 @@ static int _ath10k_ce_send_nolock_64(struct ath10k_ce_pipe *ce_state, struct ath10k_ce_ring *src_ring = ce_state->src_ring; struct ce_desc_64 *desc, sdesc; unsigned int nentries_mask = src_ring->nentries_mask; - unsigned int sw_index = src_ring->sw_index; + unsigned int sw_index; unsigned int write_index = src_ring->write_index; u32 ctrl_addr = ce_state->ctrl_addr; __le32 *addr; @@ -500,6 +539,11 @@ static int _ath10k_ce_send_nolock_64(struct ath10k_ce_pipe *ce_state, ath10k_warn(ar, "%s: send more we can (nbytes: %d, max: %d)\n", __func__, nbytes, ce_state->src_sz_max); + if (ar->hw_params.rri_on_ddr) + sw_index = ath10k_ce_src_ring_read_index_from_ddr(ar, ce_state->id); + else + sw_index = src_ring->sw_index; + if (unlikely(CE_RING_DELTA(nentries_mask, write_index, sw_index - 1) <= 0)) { ret = -ENOSR; @@ -1016,7 +1060,10 @@ int ath10k_ce_completed_send_next_nolock(struct ath10k_ce_pipe *ce_state, src_ring->hw_index = read_index; }
[PATCH 4/4] ath10k: Enable sta idle power save
From: Govind Singh Enable sta power save in fw for the targets that supports idle power save. The idle ps enable command will be ignored by the firmware which does not support this feature. Signed-off-by: Govind Singh Signed-off-by: Rakesh Pillai --- drivers/net/wireless/ath/ath10k/mac.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index 7e02ca02b28e..1d9222af1bb2 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -4679,6 +4679,13 @@ static int ath10k_start(struct ieee80211_hw *hw) } } + param = ar->wmi.pdev_param->idle_ps_config; + ret = ath10k_wmi_pdev_set_param(ar, param, 1); + if (ret && ret != -EOPNOTSUPP) { + ath10k_warn(ar, "failed to enable idle_ps_config: %d\n", ret); + goto err_core_stop; + } + __ath10k_set_antenna(ar, ar->cfg_tx_chainmask, ar->cfg_rx_chainmask); /* -- 2.14.1
[PATCH 2/4] ath10k: Add support for shadow register for WNC3990
From: Rakesh Pillai WCN3990 needs shadow register write operation support for copy engine for regular operation in powersave mode. Add support for copy engine shadow register write in datapath tx for WCN3990 Signed-off-by: Rakesh Pillai --- drivers/net/wireless/ath/ath10k/ce.c | 143 ++- drivers/net/wireless/ath/ath10k/ce.h | 4 + 2 files changed, 145 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c index e7e7b342e5b8..5053dd92bf01 100644 --- a/drivers/net/wireless/ath/ath10k/ce.c +++ b/drivers/net/wireless/ath/ath10k/ce.c @@ -1,6 +1,7 @@ /* * Copyright (c) 2005-2011 Atheros Communications Inc. * Copyright (c) 2011-2017 Qualcomm Atheros, Inc. + * Copyright (c) 2018 The Linux Foundation. All rights reserved. * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -58,6 +59,74 @@ * the buffer is sent/received. */ +static inline u32 shadow_sr_wr_ind_addr(struct ath10k *ar, + struct ath10k_ce_pipe *ce_state) +{ + u32 ce_id = ce_state->id; + u32 addr = 0; + + switch (ce_id) { + case 0: + addr = 0x00032000; + break; + case 3: + addr = 0x0003200C; + break; + case 4: + addr = 0x00032010; + break; + case 5: + addr = 0x00032014; + break; + case 7: + addr = 0x0003201C; + break; + default: + ath10k_warn(ar, "invalid CE id: %d", ce_id); + break; + } + return addr; +} + +static inline u32 shadow_dst_wr_ind_addr(struct ath10k *ar, +struct ath10k_ce_pipe *ce_state) +{ + u32 ce_id = ce_state->id; + u32 addr = 0; + + switch (ce_id) { + case 1: + addr = 0x00032034; + break; + case 2: + addr = 0x00032038; + break; + case 5: + addr = 0x00032044; + break; + case 7: + addr = 0x0003204C; + break; + case 8: + addr = 0x00032050; + break; + case 9: + addr = 0x00032054; + break; + case 10: + addr = 0x00032058; + break; + case 11: + addr = 0x0003205C; + break; + default: + ath10k_warn(ar, "invalid CE id: %d", ce_id); + break; + } + + return addr; +} + static inline unsigned int ath10k_set_ring_byte(unsigned int offset, struct ath10k_hw_ce_regs_addr_map *addr_map) @@ -123,6 +192,22 @@ static inline u32 ath10k_ce_src_ring_read_index_get(struct ath10k *ar, ar->hw_ce_regs->current_srri_addr); } +static inline void +ath10k_ce_shadow_src_ring_write_index_set(struct ath10k *ar, + struct ath10k_ce_pipe *ce_state, + unsigned int value) +{ + ath10k_ce_write32(ar, shadow_sr_wr_ind_addr(ar, ce_state), value); +} + +static inline void +ath10k_ce_shadow_dest_ring_write_index_set(struct ath10k *ar, + struct ath10k_ce_pipe *ce_state, + unsigned int value) +{ + ath10k_ce_write32(ar, shadow_dst_wr_ind_addr(ar, ce_state), value); +} + static inline void ath10k_ce_src_ring_base_addr_set(struct ath10k *ar, u32 ce_ctrl_addr, unsigned int addr) @@ -376,8 +461,14 @@ static int _ath10k_ce_send_nolock(struct ath10k_ce_pipe *ce_state, write_index = CE_RING_IDX_INCR(nentries_mask, write_index); /* WORKAROUND */ - if (!(flags & CE_SEND_FLAG_GATHER)) - ath10k_ce_src_ring_write_index_set(ar, ctrl_addr, write_index); + if (!(flags & CE_SEND_FLAG_GATHER)) { + if (ar->hw_params.shadow_reg_support) + ath10k_ce_shadow_src_ring_write_index_set(ar, ce_state, + write_index); + else + ath10k_ce_src_ring_write_index_set(ar, ctrl_addr, + write_index); + } src_ring->write_index = write_index; exit: @@ -1251,6 +1342,22 @@ static int ath10k_ce_init_dest_ring(struct ath10k *ar, return 0; } +static int ath10k_ce_alloc_shadow_base(struct ath10k *ar, + struct ath10k_ce_ring *src_ring, + u32 nentries) +{ + src_ring->shadow_base_unaligned = kcalloc(nentries,
Re: [v2,13/13] dt: bindings: add bindings for wcn3990 wifi block
Govind Singh wrote: > Add device tree binding documentation details for wcn3990 > wifi block present in Qualcomm SDM845/APQ8098 SoC into > "qcom,ath10k.txt". > > Signed-off-by: Govind Singh > Signed-off-by: Kalle Valo Govind, you forgot to CC devicetree list. I was wondering why there's no ack from device tree maintainers and realised only now that they were not CCed. Please resubmit this patch unmodified (no need to resubmit others) and CC also devicetree list. Hopefully we get an ack from them and I can finally apply this patchset. -- https://patchwork.kernel.org/patch/10302683/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: ath10k: avoid possible string overflow
Arnd Bergmann wrote: > The way that 'strncat' is used here raised a warning in gcc-8: > > drivers/net/wireless/ath/ath10k/wmi.c: In function > 'ath10k_wmi_tpc_stats_final_disp_tables': > drivers/net/wireless/ath/ath10k/wmi.c:4649:4: error: 'strncat' output > truncated before terminating nul copying as many bytes from a string as its > length [-Werror=stringop-truncation] > > Effectively, this is simply a strcat() but the use of strncat() suggests > some form of overflow check. Regardless of whether this might actually > overflow, using strlcat() instead of strncat() avoids the warning and > makes the code more robust. > > Fixes: bc64d05220f3 ("ath10k: debugfs support to get final TPC stats for 10.4 > variants") > Signed-off-by: Arnd Bergmann > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. 6707ba0105a2 ath10k: avoid possible string overflow -- https://patchwork.kernel.org/patch/10314201/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [next] wil6210: fix potential null dereference of ndev before null check
Colin Ian King wrote: > The pointer ndev is being dereferenced before it is being null checked, > hence there is a potential null pointer deference. Fix this by only > dereferencing ndev after it has been null checked > > Detected by CoverityScan, CID#1467010 ("Dereference before null check") > > Fixes: e00243fab84b ("wil6210: infrastructure for multiple virtual > interfaces") > Signed-off-by: Colin Ian King > Reviewed-by: Maya Erez > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. db5a4d5e1073 wil6210: fix potential null dereference of ndev before null check -- https://patchwork.kernel.org/patch/10313705/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: ath10k: sdio: fix memory leak for probe allocations
Marcus Folkesson wrote: > These allocations are not freed upon release. > When on it; go for managed resources instead. > > Signed-off-by: Marcus Folkesson > [kvalo: fix two checkpatch warnings] > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. ec2c64e20257 ath10k: sdio: fix memory leak for probe allocations -- https://patchwork.kernel.org/patch/10311597/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: ath10k: fix spelling mistake: "tiggers" -> "triggers"
Colin Ian King wrote: > Trivial fix to spelling mistake in message text > > Signed-off-by: Colin Ian King > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. 6ce36faeac35 ath10k: fix spelling mistake: "tiggers" -> "triggers" -- https://patchwork.kernel.org/patch/10315725/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: ath9k: dfs: remove accidental use of stack VLA
"Gustavo A. R. Silva" wrote: > In preparation to enabling -Wvla, remove accidental use of stack VLA. > > This avoids an accidental stack VLA (since the compiler thinks > the value of FFT_NUM_SAMPLES can change, even when marked > "const"). This just replaces it with a #define. > > Also, fixed as part of the directive to remove all VLAs from > the kernel: https://lkml.org/lkml/2018/3/7/621 > > Signed-off-by: Gustavo A. R. Silva > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. 9c27489a3454 ath9k: dfs: remove accidental use of stack VLA -- https://patchwork.kernel.org/patch/10318271/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [v2 PATCH] mwifiex: parse key management suite from RSN IE
On Tue, 2018-04-10 at 02:19 +, Xinming Hu wrote: > Hi Kalle, > > > -Original Message- > > From: Kalle Valo [mailto:kv...@codeaurora.org] > > Sent: 2018年4月9日 18:51 > > To: Xinming Hu > > Cc: Linux Wireless ; Brian Norris > > ; Dmitry Torokhov ; > > raja...@google.com; Zhiyuan Yang ; Tim Song > > ; Cathy Luo ; James Cao > > ; Ganapathi Bhat ; Ellie > > Reeves > > > > Subject: Re: [v2 PATCH] mwifiex: parse key management suite from > > RSN IE > > > > Xinming Hu writes: > > > > > Association failed when using wpa_supplicant with configuration > > > key_mgmt=WPA-PSK-SHA256, and it is noticed that wpa_supplicant > > > set > > > WLAN_AKM_SUITE_PSK_SHA256 in RSN IE, but miss it in start_ap > > > parameters. > > > > > > This patch parse key management suite from RSN IE, in case it is > > > missed. > > > > > > Signed-off-by: Xinming Hu > > > > Shouldn't you fix wpa_supplicant instead of adding a workaround to > > the driver? > > > > Yes, I am preparing a wpa_supplicant fix on this. > Even so, driver fix is still need to compatible with old > wpa_supplicant stable release Typically the kernel does not add hacks to work around userspace software bugs. The userspace software needs to be fixed and upgraded. Dan
Re: [1/3] wcn36xx: check for DMA mapping errors in wcn36xx_dxe_tx_frame()
Daniel Mack wrote: > Bail out if the mapping fails. Even though this hasn't occured during > tests, this unlikely case should still be handled. > > Signed-off-by: Daniel Mack > Acked-by: Ramon Fried > Signed-off-by: Kalle Valo 3 patches applied to ath-next branch of ath.git, thanks. 7cae35199bee wcn36xx: check for DMA mapping errors in wcn36xx_dxe_tx_frame() 271f1e65ff38 wcn36xx: don't keep reference to skb if transmission failed 2edfcf2b303c wcn36xx: don't delete invalid bss indices -- https://patchwork.kernel.org/patch/10321545/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: wcn36xx: Add missing fall through comment in smd.c
Loic Poulain wrote: > This prevents GCC warning. > > Reported-by: Dan Carpenter > Signed-off-by: Loic Poulain > Acked-by: Bjorn Andersson > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. 1391cce7daf6 wcn36xx: Add missing fall through comment in smd.c -- https://patchwork.kernel.org/patch/10324469/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: wcn36xx: Remove useless skb spinlock
Loic Poulain wrote: > Each DXE control block is associated to a specific channel. > The channel lock is always taken before accessing a control block. > There is no need to have an extra (useless) spinlock for the control > block skb. > > Signed-off-by: Loic Poulain > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. 6062546d9b7f wcn36xx: Remove useless skb spinlock -- https://patchwork.kernel.org/patch/10325829/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
[PATCH] staging: wilc1000: Remove unnecessary braces {} around single statement block
Remove unnecessary braces {} around an 'if' statement block with a single statement. Issue found by checkpatch. Signed-off-by: Eyal Ilsar --- This is part of my take on the Eudyptula challenge drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index 205304c..325afe1 100644 --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -284,9 +284,8 @@ static void remove_network_from_shadow(struct timer_list *unused) } } - if (last_scanned_cnt != 0) { + if (last_scanned_cnt != 0) mod_timer(&hAgingTimer, jiffies + msecs_to_jiffies(AGING_TIME)); - } } static void clear_duringIP(struct timer_list *unused) -- 2.7.4
Re: wcn36xx: allocate skbs with GFP_KERNEL during init
Daniel Mack wrote: > GFP_ATOMIC should only be used when the allocation is done from atomic > context. Introduce a new flag to wcn36xx_dxe_fill_skb() and use GFP_KERNEL > when pre-allocating buffers during init. > > This doesn't fix an issue that was observed in the wild, but it reduces > the chance of failed allocations under memory pressure. > > Signed-off-by: Daniel Mack > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. 5151a673da43 wcn36xx: allocate skbs with GFP_KERNEL during init -- https://patchwork.kernel.org/patch/10291685/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
[v2 0/7] rsi driver improvements
From: Siva Rebbagondla This patch series brings in some improvements in power save, Tx data path etc. Changes in v2: Below patches are split and created smaller incremental patches[Kalle] [08/10] rsi: device disconnect changes [09/10] rsi: tx improvements Amitkumar Karwar (7): rsi: disable fw watchdog timer during reset rsi: device bootup parameter configuration rsi: use appropriate interface for power save configuration rsi: increase max supported aggregation subframes rsi: parse TID from data frame correctly rsi: enable power save by default for coex rsi: advertise 5GHz support based on device capability drivers/net/wireless/rsi/rsi_91x_core.c | 4 +++- drivers/net/wireless/rsi/rsi_91x_mac80211.c | 29 - drivers/net/wireless/rsi/rsi_91x_usb.c | 7 +++ drivers/net/wireless/rsi/rsi_boot_params.h | 3 ++- drivers/net/wireless/rsi/rsi_hal.h | 1 + drivers/net/wireless/rsi/rsi_mgmt.h | 3 +++ drivers/net/wireless/rsi/rsi_usb.h | 1 + 7 files changed, 37 insertions(+), 11 deletions(-) -- 2.7.4
[v2 2/7] rsi: device bootup parameter configuration
From: Amitkumar Karwar Some device configuration flags need to be enabled while sending 'bootup params' internal frame to firmware. This patch takes care of it. Signed-off-by: Amitkumar Karwar Signed-off-by: Siva Rebbagondla --- drivers/net/wireless/rsi/rsi_boot_params.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/rsi/rsi_boot_params.h b/drivers/net/wireless/rsi/rsi_boot_params.h index 238ee96..ad903b22 100644 --- a/drivers/net/wireless/rsi/rsi_boot_params.h +++ b/drivers/net/wireless/rsi/rsi_boot_params.h @@ -46,7 +46,8 @@ (((TA_PLL_M_VAL_20 + 1) * 40) / \ ((TA_PLL_N_VAL_20 + 1) * (TA_PLL_P_VAL_20 + 1))) #define VALID_20 \ - (WIFI_PLL960_CONFIGS | WIFI_AFEPLL_CONFIGS | WIFI_SWITCH_CLK_CONFIGS) + (WIFI_TAPLL_CONFIGS | WIFI_PLL960_CONFIGS | WIFI_AFEPLL_CONFIGS | \ +WIFI_SWITCH_CLK_CONFIGS | BOOTUP_MODE_INFO | CRYSTAL_GOOD_TIME) #define UMAC_CLK_40BW \ (((TA_PLL_M_VAL_40 + 1) * 40) / \ ((TA_PLL_N_VAL_40 + 1) * (TA_PLL_P_VAL_40 + 1))) -- 2.7.4
[v2 3/7] rsi: use appropriate interface for power save configuration
From: Amitkumar Karwar Power save request should be sent on station interface. Virtual interface which is connected should be preferred. This patch resolves device not entering power save problem in certain situations Signed-off-by: Amitkumar Karwar Signed-off-by: Siva Rebbagondla --- drivers/net/wireless/rsi/rsi_91x_mac80211.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/rsi/rsi_91x_mac80211.c b/drivers/net/wireless/rsi/rsi_91x_mac80211.c index 5edc3a9..77aa3bb 100644 --- a/drivers/net/wireless/rsi/rsi_91x_mac80211.c +++ b/drivers/net/wireless/rsi/rsi_91x_mac80211.c @@ -614,7 +614,7 @@ static int rsi_mac80211_config(struct ieee80211_hw *hw, /* Power save parameters */ if (changed & IEEE80211_CONF_CHANGE_PS) { - struct ieee80211_vif *vif; + struct ieee80211_vif *vif, *sta_vif = NULL; unsigned long flags; int i, set_ps = 1; @@ -628,13 +628,17 @@ static int rsi_mac80211_config(struct ieee80211_hw *hw, set_ps = 0; break; } + if ((vif->type == NL80211_IFTYPE_STATION || +vif->type == NL80211_IFTYPE_P2P_CLIENT) && + (!sta_vif || vif->bss_conf.assoc)) + sta_vif = vif; } - if (set_ps) { + if (set_ps && sta_vif) { spin_lock_irqsave(&adapter->ps_lock, flags); if (conf->flags & IEEE80211_CONF_PS) - rsi_enable_ps(adapter, vif); + rsi_enable_ps(adapter, sta_vif); else - rsi_disable_ps(adapter, vif); + rsi_disable_ps(adapter, sta_vif); spin_unlock_irqrestore(&adapter->ps_lock, flags); } } -- 2.7.4
[v2 1/7] rsi: disable fw watchdog timer during reset
From: Amitkumar Karwar Firmware's watchdog timer should be disabled as a part of reset sequence. This change fixes a firmware hang issue observed during stress tests. Signed-off-by: Amitkumar Karwar Signed-off-by: Siva Rebbagondla --- drivers/net/wireless/rsi/rsi_91x_usb.c | 7 +++ drivers/net/wireless/rsi/rsi_hal.h | 1 + drivers/net/wireless/rsi/rsi_usb.h | 1 + 3 files changed, 9 insertions(+) diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c index 7b8bae3..b065438 100644 --- a/drivers/net/wireless/rsi/rsi_91x_usb.c +++ b/drivers/net/wireless/rsi/rsi_91x_usb.c @@ -687,6 +687,13 @@ static int rsi_reset_card(struct rsi_hw *adapter) */ msleep(100); + if (rsi_usb_master_reg_write(adapter, SWBL_REGOUT, +RSI_FW_WDT_DISABLE_REQ, +RSI_COMMON_REG_SIZE) < 0) { + rsi_dbg(ERR_ZONE, "Disabling firmware watchdog timer failed\n"); + goto fail; + } + ret = usb_ulp_read_write(adapter, RSI_WATCH_DOG_TIMER_1, RSI_ULP_WRITE_2, 32); if (ret < 0) diff --git a/drivers/net/wireless/rsi/rsi_hal.h b/drivers/net/wireless/rsi/rsi_hal.h index d6c2baa..327638c 100644 --- a/drivers/net/wireless/rsi/rsi_hal.h +++ b/drivers/net/wireless/rsi/rsi_hal.h @@ -115,6 +115,7 @@ #define FW_FLASH_OFFSET0x820 #define LMAC_VER_OFFSET(FW_FLASH_OFFSET + 0x200) #define MAX_DWORD_ALIGN_BYTES 64 +#define RSI_COMMON_REG_SIZE2 struct bl_header { __le32 flags; diff --git a/drivers/net/wireless/rsi/rsi_usb.h b/drivers/net/wireless/rsi/rsi_usb.h index a88d592..b6fe79f 100644 --- a/drivers/net/wireless/rsi/rsi_usb.h +++ b/drivers/net/wireless/rsi/rsi_usb.h @@ -26,6 +26,7 @@ #define RSI_USB_READY_MAGIC_NUM 0xab #define FW_STATUS_REG0x41050012 #define RSI_TA_HOLD_REG 0x22000844 +#define RSI_FW_WDT_DISABLE_REQ 0x69 #define USB_VENDOR_REGISTER_READ 0x15 #define USB_VENDOR_REGISTER_WRITE0x16 -- 2.7.4
[v2 5/7] rsi: parse TID from data frame correctly
From: Amitkumar Karwar Currently TID is extracted by checking at specific offset in data frame. This approach doesn't work for some of the frames. This patch uses mac80211 API and do it correctly Signed-off-by: Amitkumar Karwar Signed-off-by: Siva Rebbagondla --- drivers/net/wireless/rsi/rsi_91x_core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/rsi/rsi_91x_core.c b/drivers/net/wireless/rsi/rsi_91x_core.c index 3ca468b9..1f1b972 100644 --- a/drivers/net/wireless/rsi/rsi_91x_core.c +++ b/drivers/net/wireless/rsi/rsi_91x_core.c @@ -432,7 +432,9 @@ void rsi_core_xmit(struct rsi_common *common, struct sk_buff *skb) } } else { if (ieee80211_is_data_qos(wh->frame_control)) { - tid = (skb->data[24] & IEEE80211_QOS_TID); + u8 *qos = ieee80211_get_qos_ctl(wh); + + tid = *qos & IEEE80211_QOS_CTL_TID_MASK; skb->priority = TID_TO_WME_AC(tid); } else { tid = IEEE80211_NONQOS_TID; -- 2.7.4
[v2 6/7] rsi: enable power save by default for coex
From: Amitkumar Karwar Power save is by default enabled for WLAN and BT coex mode. Signed-off-by: Amitkumar Karwar Signed-off-by: Siva Rebbagondla --- drivers/net/wireless/rsi/rsi_91x_mac80211.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wireless/rsi/rsi_91x_mac80211.c b/drivers/net/wireless/rsi/rsi_91x_mac80211.c index 2a77743..bb497bd 100644 --- a/drivers/net/wireless/rsi/rsi_91x_mac80211.c +++ b/drivers/net/wireless/rsi/rsi_91x_mac80211.c @@ -2008,6 +2008,9 @@ int rsi_mac80211_attach(struct rsi_common *common) wiphy->iface_combinations = rsi_iface_combinations; wiphy->n_iface_combinations = ARRAY_SIZE(rsi_iface_combinations); + if (common->coex_mode > 1) + wiphy->flags |= WIPHY_FLAG_PS_ON_BY_DEFAULT; + status = ieee80211_register_hw(hw); if (status) return status; -- 2.7.4
[v2 4/7] rsi: increase max supported aggregation subframes
From: Amitkumar Karwar Maximum number of supported aggregation subframes has been increased to 8. This is the optimal number for the driver. Signed-off-by: Amitkumar Karwar Signed-off-by: Siva Rebbagondla --- drivers/net/wireless/rsi/rsi_91x_mac80211.c | 3 ++- drivers/net/wireless/rsi/rsi_mgmt.h | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/rsi/rsi_91x_mac80211.c b/drivers/net/wireless/rsi/rsi_91x_mac80211.c index 77aa3bb..2a77743 100644 --- a/drivers/net/wireless/rsi/rsi_91x_mac80211.c +++ b/drivers/net/wireless/rsi/rsi_91x_mac80211.c @@ -1955,7 +1955,8 @@ int rsi_mac80211_attach(struct rsi_common *common) hw->uapsd_queues = RSI_IEEE80211_UAPSD_QUEUES; hw->uapsd_max_sp_len = IEEE80211_WMM_IE_STA_QOSINFO_SP_ALL; - hw->max_tx_aggregation_subframes = 6; + hw->max_tx_aggregation_subframes = RSI_MAX_TX_AGGR_FRMS; + hw->max_rx_aggregation_subframes = RSI_MAX_RX_AGGR_FRMS; rsi_register_rates_channels(adapter, NL80211_BAND_2GHZ); rsi_register_rates_channels(adapter, NL80211_BAND_5GHZ); hw->rate_control_algorithm = "AARF"; diff --git a/drivers/net/wireless/rsi/rsi_mgmt.h b/drivers/net/wireless/rsi/rsi_mgmt.h index 5f946f3..1462093 100644 --- a/drivers/net/wireless/rsi/rsi_mgmt.h +++ b/drivers/net/wireless/rsi/rsi_mgmt.h @@ -225,6 +225,9 @@ #define RSI_WOW_DISCONNECT BIT(5) #endif +#define RSI_MAX_TX_AGGR_FRMS 8 +#define RSI_MAX_RX_AGGR_FRMS 8 + enum opmode { RSI_OPMODE_UNSUPPORTED = -1, RSI_OPMODE_AP = 0, -- 2.7.4
[v2 7/7] rsi: advertise 5GHz support based on device capability
From: Amitkumar Karwar Currently 5GHz gets advertised even for the device which supports only 2.4Ghz band. This patch fixes the issue Signed-off-by: Amitkumar Karwar Signed-off-by: Siva Rebbagondla --- drivers/net/wireless/rsi/rsi_91x_mac80211.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/rsi/rsi_91x_mac80211.c b/drivers/net/wireless/rsi/rsi_91x_mac80211.c index bb497bd..766d874 100644 --- a/drivers/net/wireless/rsi/rsi_91x_mac80211.c +++ b/drivers/net/wireless/rsi/rsi_91x_mac80211.c @@ -1957,8 +1957,6 @@ int rsi_mac80211_attach(struct rsi_common *common) hw->max_tx_aggregation_subframes = RSI_MAX_TX_AGGR_FRMS; hw->max_rx_aggregation_subframes = RSI_MAX_RX_AGGR_FRMS; - rsi_register_rates_channels(adapter, NL80211_BAND_2GHZ); - rsi_register_rates_channels(adapter, NL80211_BAND_5GHZ); hw->rate_control_algorithm = "AARF"; SET_IEEE80211_PERM_ADDR(hw, common->mac_addr); @@ -1979,10 +1977,15 @@ int rsi_mac80211_attach(struct rsi_common *common) wiphy->available_antennas_rx = 1; wiphy->available_antennas_tx = 1; + + rsi_register_rates_channels(adapter, NL80211_BAND_2GHZ); wiphy->bands[NL80211_BAND_2GHZ] = &adapter->sbands[NL80211_BAND_2GHZ]; - wiphy->bands[NL80211_BAND_5GHZ] = - &adapter->sbands[NL80211_BAND_5GHZ]; + if (common->num_supp_bands > 1) { + rsi_register_rates_channels(adapter, NL80211_BAND_5GHZ); + wiphy->bands[NL80211_BAND_5GHZ] = + &adapter->sbands[NL80211_BAND_5GHZ]; + } /* AP Parameters */ wiphy->max_ap_assoc_sta = rsi_max_ap_stas[common->oper_mode - 1]; -- 2.7.4
[PATCH v2 13/13] dt: bindings: add bindings for wcn3990 wifi block
Add device tree binding documentation details for wcn3990 wifi block present in Qualcomm SDM845/APQ8098 SoC into "qcom,ath10k.txt". Signed-off-by: Govind Singh --- .../bindings/net/wireless/qcom,ath10k.txt | 31 ++ 1 file changed, 31 insertions(+) diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt index 3d2a031..34e4f98 100644 --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt @@ -4,6 +4,7 @@ Required properties: - compatible: Should be one of the following: * "qcom,ath10k" * "qcom,ipq4019-wifi" + * "qcom,wcn3990-wifi" PCI based devices uses compatible string "qcom,ath10k" and takes calibration data along with board specific data via "qcom,ath10k-calibration-data". @@ -18,8 +19,12 @@ In general, entry "qcom,ath10k-pre-calibration-data" and "qcom,ath10k-calibration-data" conflict with each other and only one can be provided per device. +SNOC based devices (i.e. wcn3990) uses compatible string "qcom,wcn3990-wifi". + Optional properties: - reg: Address and length of the register set for the device. +- reg-names: Must include the list of following reg names, +"membase" - resets: Must contain an entry for each entry in reset-names. See ../reset/reseti.txt for details. - reset-names: Must include the list of following reset names, @@ -49,6 +54,8 @@ Optional properties: hw versions. - qcom,ath10k-pre-calibration-data : pre calibration data as an array, the length can vary between hw versions. +- -supply: handle to the regulator device tree node + optional "supply-name" is "vdd-0.8-cx-mx". Example (to supply the calibration data alone): @@ -119,3 +126,27 @@ wifi0: wifi@a00 { qcom,msi_base = <0x40>; qcom,ath10k-pre-calibration-data = [ 01 02 03 ... ]; }; + +Example (to supply wcn3990 SoC wifi block details): + +qcom,wifi@1800 { + compatible = "qcom,wcn3990-wifi"; + reg = <0x1880 0x80>; + reg-names = "membase"; + clocks = <&clock_gcc clk_aggre2_noc_clk>; + clock-names = "smmu_aggre2_noc_clk" + interrupts = + <0 130 0 /* CE0 */ >, + <0 131 0 /* CE1 */ >, + <0 132 0 /* CE2 */ >, + <0 133 0 /* CE3 */ >, + <0 134 0 /* CE4 */ >, + <0 135 0 /* CE5 */ >, + <0 136 0 /* CE6 */ >, + <0 137 0 /* CE7 */ >, + <0 138 0 /* CE8 */ >, + <0 139 0 /* CE9 */ >, + <0 140 0 /* CE10 */ >, + <0 141 0 /* CE11 */ >; + vdd-0.8-cx-mx-supply = <&pm8998_l5>; +}; -- 1.9.1
Re: [PATCH] net: wireless: b43legacy: Replace GFP_ATOMIC with GFP_KERNEL in dma_tx_fragment
On Tue, 10 Apr 2018 21:54:19 +0800 Jia-Ju Bai wrote: > dma_tx_fragment() is never called in atomic context. > > dma_tx_fragment() is only called by b43legacy_dma_tx(), which is > only called by b43legacy_tx_work(). > b43legacy_tx_work() is only set a parameter of INIT_WORK() in > b43legacy_wireless_init(). > > Despite never getting called from atomic context, > dma_tx_fragment() calls alloc_skb() with GFP_ATOMIC, > which does not sleep for allocation. > GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL, > which can sleep and improve the possibility of sucessful allocation. > > This is found by a static analysis tool named DCNS written by myself. > And I also manually check it. > > Signed-off-by: Jia-Ju Bai > --- > drivers/net/wireless/broadcom/b43legacy/dma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c > b/drivers/net/wireless/broadcom/b43legacy/dma.c > index cfa617d..2f0c64c 100644 > --- a/drivers/net/wireless/broadcom/b43legacy/dma.c > +++ b/drivers/net/wireless/broadcom/b43legacy/dma.c > @@ -1064,7 +1064,7 @@ static int dma_tx_fragment(struct b43legacy_dmaring > *ring, > meta->dmaaddr = map_descbuffer(ring, skb->data, skb->len, 1); > /* create a bounce buffer in zone_dma on mapping failure. */ > if (b43legacy_dma_mapping_error(ring, meta->dmaaddr, skb->len, 1)) { > - bounce_skb = alloc_skb(skb->len, GFP_ATOMIC | GFP_DMA); > + bounce_skb = alloc_skb(skb->len, GFP_KERNEL | GFP_DMA); > if (!bounce_skb) { > ring->current_slot = old_top_slot; > ring->used_slots = old_used_slots; Ack. I think the GFP_ATOMIC came from the days where we did DMA operations under spinlock instead of mutex. The same thing can be done in b43. Also setup_rx_descbuffer(ring, desc, meta, GFP_ATOMIC) could be GFP_KERNEL in dma_rx(). This function is called from IRQ thread context. -- Michael pgpHWZAi1Njhu.pgp Description: OpenPGP digital signature
Re: [PATCH v2] dt: bindings: add bindings for wcn3990 wifi block
Thanks Rob for the review comments. I have addressed the review comments in [PATCH v2 13/13] dt: bindings: add bindings for wcn3990 wifi block. diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt index 3d2a031..e34d8f8 100644 --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt @@ -4,6 +4,7 @@ Required properties: - compatible: Should be one of the following: * "qcom,ath10k" * "qcom,ipq4019-wifi" + * "qcom,wcn3990-wifi" PCI based devices uses compatible string "qcom,ath10k" and takes calibration data along with board specific data via "qcom,ath10k-calibration-data". @@ -18,8 +19,11 @@ In general, entry "qcom,ath10k-pre-calibration-data" and "qcom,ath10k-calibration-data" conflict with each other and only one can be provided per device. +SNOC based devices (i.e. wcn3990) uses compatible string "qcom,wcn3990-wifi". + Optional properties: - reg: Address and length of the register set for the device. +- reg-names: Names of the memory regions defined in reg entry. You must define what the names are and how many (so reg needs updating too if there is more than 1). Though with only 1 -names, it is kind of pointless. Updated the names. - resets: Must contain an entry for each entry in reset-names. See ../reset/reseti.txt for details. - reset-names: Must include the list of following reset names, @@ -49,6 +53,12 @@ Optional properties: hw versions. - qcom,ath10k-pre-calibration-data : pre calibration data as an array, the length can vary between hw versions. +- qcom,-supply: handle to the regulator device tree node + optional "supply-name" is "vdd-0.8-cx-mx". You can drop the qcom prefix. Just name this "vdd-0.8-cx-mx-supply". Updated the same. +- qcom,-config: Specifies voltage levels for supply. Should be + specified in pairs (min, max), units uV. There can + be optional load in uA and Regulator settle delay in + uS. We have standard regulator properties for this I think. Removed the supply config. Example (to supply the calibration data alone): @@ -119,3 +129,28 @@ wifi0: wifi@a00 { qcom,msi_base = <0x40>; qcom,ath10k-pre-calibration-data = [ 01 02 03 ... ]; }; + +Example (to supply wcn3990 SoC wifi block details): + +qcom,msm_ath10k@1800 { wifi@... Updated the same. + compatible = "qcom,wcn3990-wifi"; + reg = <0x1880 0x80>; + reg-names = "membase"; + clocks = <&clock_gcc clk_aggre2_noc_clk>; + clock-names = "smmu_aggre2_noc_clk" + interrupts = + <0 130 0 /* CE0 */ >, + <0 131 0 /* CE1 */ >, + <0 132 0 /* CE2 */ >, + <0 133 0 /* CE3 */ >, + <0 134 0 /* CE4 */ >, + <0 135 0 /* CE5 */ >, + <0 136 0 /* CE6 */ >, + <0 137 0 /* CE7 */ >, + <0 138 0 /* CE8 */ >, + <0 139 0 /* CE9 */ >, + <0 140 0 /* CE10 */ >, + <0 141 0 /* CE11 */ >; + qcom,vdd-0.8-cx-mx-supply = <&pm8998_l5>; + qcom,vdd-0.8-cx-mx-config = <80 80 2400 1000>; +}; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] wcn36xx: use READ_ONCE() to access desc->ctrl
When accessing shared memory to check for the stat of submitted descriptors, make sure to use READ_ONCE(). This will guarantee the compiler treats these memory locations as volatile and doesn't apply any caching. While this doesn't fix any particular problem I ran into, it's best practice to do it this way. Note that this patch also removes the superflous extra condition check in the do-while loop in reap_tx_dxes(), as the loop will break instantly anyway in that case. Signed-off-by: Daniel Mack --- drivers/net/wireless/ath/wcn36xx/dxe.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c index 26e6c42f886a..c52f1c21ece9 100644 --- a/drivers/net/wireless/ath/wcn36xx/dxe.c +++ b/drivers/net/wireless/ath/wcn36xx/dxe.c @@ -363,7 +363,7 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch) spin_lock_irqsave(&ch->lock, flags); ctl = ch->tail_blk_ctl; do { - if (ctl->desc->ctrl & WCN36xx_DXE_CTRL_VLD) + if (READ_ONCE(ctl->desc->ctrl) & WCN36xx_DXE_CTRL_VLD) break; if (ctl->skb) { dma_unmap_single(wcn->dev, ctl->desc->src_addr_l, @@ -382,8 +382,7 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch) ctl->skb = NULL; } ctl = ctl->next; - } while (ctl != ch->head_blk_ctl && - !(ctl->desc->ctrl & WCN36xx_DXE_CTRL_VLD)); + } while (ctl != ch->head_blk_ctl); ch->tail_blk_ctl = ctl; spin_unlock_irqrestore(&ch->lock, flags); @@ -525,7 +524,7 @@ static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn, int_mask = WCN36XX_DXE_INT_CH3_MASK; } - while (!(dxe->ctrl & WCN36xx_DXE_CTRL_VLD)) { + while (!(READ_ONCE(dxe->ctrl) & WCN36xx_DXE_CTRL_VLD)) { skb = ctl->skb; dma_addr = dxe->dst_addr_l; ret = wcn36xx_dxe_fill_skb(wcn->dev, ctl, GFP_ATOMIC); -- 2.14.3
[PATCH 1/1 RFC] wcn36xx: fix buffer commit logic on TX path
When wcn36xx_dxe_tx_frame() is entered while the device is still processing the queue asyncronously, we are racing against the firmware code with updates to the buffer descriptors. Presumably, the firmware scans the ring buffer that holds the descriptors and scans for a valid control descriptor, and then assumes that the next descriptor contains the payload. If, however, the control descriptor is marked valid, but the payload descriptor isn't, the packet is not sent out. Another issue with the current code is that is lacks memory barriers before descriptors are marked valid. This is important because the CPU may reorder writes to memory, even if it is allocated as coherent DMA area, and hence the device may see incompletely written data. To fix this, the code in wcn36xx_dxe_tx_frame() was restructured a bit so that the payload descriptor is made valid before the control descriptor. Memory barriers are added to ensure coherency of shared memory areas. Signed-off-by: Daniel Mack --- drivers/net/wireless/ath/wcn36xx/dxe.c | 75 +- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c index 26e6c42f886a..cb93545a42ce 100644 --- a/drivers/net/wireless/ath/wcn36xx/dxe.c +++ b/drivers/net/wireless/ath/wcn36xx/dxe.c @@ -638,8 +638,8 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn, struct sk_buff *skb, bool is_low) { - struct wcn36xx_dxe_ctl *ctl = NULL; - struct wcn36xx_dxe_desc *desc = NULL; + struct wcn36xx_dxe_desc *desc_bd, *desc_skb; + struct wcn36xx_dxe_ctl *ctl_bd, *ctl_skb; struct wcn36xx_dxe_ch *ch = NULL; unsigned long flags; int ret; @@ -647,74 +647,75 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn, ch = is_low ? &wcn->dxe_tx_l_ch : &wcn->dxe_tx_h_ch; spin_lock_irqsave(&ch->lock, flags); - ctl = ch->head_blk_ctl; + ctl_bd = ch->head_blk_ctl; + ctl_skb = ctl_bd->next; /* * If skb is not null that means that we reached the tail of the ring * hence ring is full. Stop queues to let mac80211 back off until ring * has an empty slot again. */ - if (NULL != ctl->next->skb) { + if (NULL != ctl_skb->skb) { ieee80211_stop_queues(wcn->hw); wcn->queues_stopped = true; spin_unlock_irqrestore(&ch->lock, flags); return -EBUSY; } - ctl->skb = NULL; - desc = ctl->desc; + if (unlikely(ctl_skb->bd_cpu_addr)) { + wcn36xx_err("bd_cpu_addr cannot be NULL for skb DXE\n"); + ret = -EINVAL; + goto unlock; + } + + desc_bd = ctl_bd->desc; + desc_skb = ctl_skb->desc; + + ctl_bd->skb = NULL; /* write buffer descriptor */ - memcpy(ctl->bd_cpu_addr, bd, sizeof(*bd)); + memcpy(ctl_bd->bd_cpu_addr, bd, sizeof(*bd)); /* Set source address of the BD we send */ - desc->src_addr_l = ctl->bd_phy_addr; - - desc->dst_addr_l = ch->dxe_wq; - desc->fr_len = sizeof(struct wcn36xx_tx_bd); - desc->ctrl = ch->ctrl_bd; + desc_bd->src_addr_l = ctl_bd->bd_phy_addr; + desc_bd->dst_addr_l = ch->dxe_wq; + desc_bd->fr_len = sizeof(struct wcn36xx_tx_bd); wcn36xx_dbg(WCN36XX_DBG_DXE, "DXE TX\n"); wcn36xx_dbg_dump(WCN36XX_DBG_DXE_DUMP, "DESC1 >>> ", -(char *)desc, sizeof(*desc)); +(char *)desc_bd, sizeof(*desc_bd)); wcn36xx_dbg_dump(WCN36XX_DBG_DXE_DUMP, -"BD >>> ", (char *)ctl->bd_cpu_addr, +"BD >>> ", (char *)ctl_bd->bd_cpu_addr, sizeof(struct wcn36xx_tx_bd)); - /* Set source address of the SKB we send */ - ctl = ctl->next; - desc = ctl->desc; - if (ctl->bd_cpu_addr) { - wcn36xx_err("bd_cpu_addr cannot be NULL for skb DXE\n"); - ret = -EINVAL; - goto unlock; - } - - desc->src_addr_l = dma_map_single(wcn->dev, - skb->data, - skb->len, - DMA_TO_DEVICE); - if (dma_mapping_error(wcn->dev, desc->src_addr_l)) { + desc_skb->src_addr_l = dma_map_single(wcn->dev, + skb->data, + skb->len, + DMA_TO_DEVICE); + if (dma_mapping_error(wcn->dev, desc_skb->src_addr_l)) { dev_err(wcn->dev, "unable to DMA map src_addr_l\n"); ret = -ENOMEM; goto unlock; } - ctl->skb = skb; - desc->dst_addr_l = ch->dxe_wq; - desc->fr_len = ctl->skb->len; - - /* set dxe descripto
[PATCH 0/1] RFC: memory coherency and data races on TX path
I found something that I believe might be an issue, and I have an idea on how to correct this, but unfortunately, this doesn't solve the issues I occasionally see with this driver. I'd still like to share it, because I might be totally mistaken in my understanding. With no firmware code or documentation at hand, it's not exactly clear which assumption the firmware makes, but obviously, the driver and the firmware share memory to exchange descriptors that either contain control information or payload. The driver puts control descriptors and payload descriptors in a ring buffer in an interleaved fashion. When the driver wants to send an skb, it looks for a currently unused control descriptor, and then fills it, together with its directly chained payload descriptor. The descriptors are both marked valid and then the firmware is instructed to start processing the ringbuffer. In case the firmware is idle when wcn36xx_dxe_tx_frame() is entered, this is all fine. However, when sending many packets in a short time frame, there are cases when the firmware is still processing the ring buffer (iow, ch->reg_ctrl & WCN36xx_DXE_CH_CTRL_EN != 0), and in this case, writes to the shared memory area depict a data race. The local spinlock of course doesn't help to prevent that. OTOH, it should be completely fine to modify the descriptors while firmware is still reading them, as the firmware should only pay attention to such that are marked valid. There's a problem with the latter presumption however which looks like this in the driver code: desc->fr_len = ctl->skb->len; /* set dxe descriptor to VALID */ desc->ctrl = ch->ctrl_skb; The CPU may very well reorder the two writes, even though the memory is allocated as coherent DMA. When that happens, the firmware may see a wrong length for a valid descriptor. A simple memory write barrier would suffice to solve this, but then again, there are two descriptors involved. My attempt to fix that restructures the code a bit and makes the payload descriptor valid first and then the control descriptor, both strongly ordered through memory fences. This however assumes that the firmware will ignore valid payload descriptors unless they have a valid control descriptor preceding them, but that's really just guessing. Does that make sense? As I said, I can't really say this improves anything, sadly, so I might be mistaken entirely. But I'll leave this here for further discussion. Ideally, somebody with access to the firmware sources could give an assessment whether this is an issue at all or not. Thanks, Daniel Daniel Mack (1): wcn36xx: fix buffer commit logic on TX path drivers/net/wireless/ath/wcn36xx/dxe.c | 75 +- 1 file changed, 38 insertions(+), 37 deletions(-) -- 2.14.3
Re: [PATCH 0/1] RFC: memory coherency and data races on TX path
On 10 April 2018 at 20:42, Daniel Mack wrote: > I found something that I believe might be an issue, and I have an > idea on how to correct this, but unfortunately, this doesn't solve > the issues I occasionally see with this driver. I'd still like to > share it, because I might be totally mistaken in my understanding. > Thanks for sharing you idea. Are you aware of the recent patch I sent that Loic Poulain wrote that fixes a race issue in access to wcn36xx_dxe_tx_frame() ? Kalle just recently applied it to ath-next, I don't think it's available yet upstream. The patch was fixing something similar, perhaps it will solve the issue you're experiencing. > With no firmware code or documentation at hand, it's not exactly clear > which assumption the firmware makes, but obviously, the driver and the > firmware share memory to exchange descriptors that either contain > control information or payload. The driver puts control descriptors > and payload descriptors in a ring buffer in an interleaved fashion. > > When the driver wants to send an skb, it looks for a currently unused > control descriptor, and then fills it, together with its directly > chained payload descriptor. The descriptors are both marked valid and > then the firmware is instructed to start processing the ringbuffer. > In case the firmware is idle when wcn36xx_dxe_tx_frame() is entered, > this is all fine. However, when sending many packets in a short time > frame, there are cases when the firmware is still processing the ring > buffer (iow, ch->reg_ctrl & WCN36xx_DXE_CH_CTRL_EN != 0), and in this > case, writes to the shared memory area depict a data race. The local > spinlock of course doesn't help to prevent that. OTOH, it should be > completely fine to modify the descriptors while firmware is still > reading them, as the firmware should only pay attention to such that > are marked valid. > > There's a problem with the latter presumption however which looks like > this in the driver code: > > desc->fr_len = ctl->skb->len; > /* set dxe descriptor to VALID */ > desc->ctrl = ch->ctrl_skb; > The firmware won't start reading the descriptors until it receives an interrupt from driver. which in turn happen later in the function using: wcn36xx_dxe_write_register() so I don't think reordering in this case make any problem. > The CPU may very well reorder the two writes, even though the memory is > allocated as coherent DMA. When that happens, the firmware may see a > wrong length for a valid descriptor. A simple memory write barrier would > suffice to solve this, but then again, there are two descriptors > involved. > > My attempt to fix that restructures the code a bit and makes the > payload descriptor valid first and then the control descriptor, both > strongly ordered through memory fences. This however assumes that the > firmware will ignore valid payload descriptors unless they have a > valid control descriptor preceding them, but that's really just > guessing. > > Does that make sense? As I said, I can't really say this improves > anything, sadly, so I might be mistaken entirely. But I'll leave this > here for further discussion. Ideally, somebody with access to the > firmware sources could give an assessment whether this is an issue at > all or not. When I'm not sure regarding some implementation I consult with the downstream pronto driver. https://github.com/sultanqasim/qcom_wlan_prima Did you look there if they actually placed wmb() ? > > > Thanks, > Daniel > > > Daniel Mack (1): > wcn36xx: fix buffer commit logic on TX path > > drivers/net/wireless/ath/wcn36xx/dxe.c | 75 > +- > 1 file changed, 38 insertions(+), 37 deletions(-) > > -- > 2.14.3 >
Re: [PATCH] wcn36xx: use READ_ONCE() to access desc->ctrl
On 10 April 2018 at 20:35, Daniel Mack wrote: > When accessing shared memory to check for the stat of submitted > descriptors, make sure to use READ_ONCE(). This will guarantee the > compiler treats these memory locations as volatile and doesn't apply > any caching. The structure that is tested is not shared memory. It's accessed only by the apps processor. In this case, caching does make sense. I'm not sure regarding READ_ONCE in that context I'll have to do some reading regarding that. But I don't think this is right here. Thanks, Ramon > > While this doesn't fix any particular problem I ran into, it's best > practice to do it this way. > > Note that this patch also removes the superflous extra condition check > in the do-while loop in reap_tx_dxes(), as the loop will break > instantly anyway in that case. > > Signed-off-by: Daniel Mack > --- > drivers/net/wireless/ath/wcn36xx/dxe.c | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c > b/drivers/net/wireless/ath/wcn36xx/dxe.c > index 26e6c42f886a..c52f1c21ece9 100644 > --- a/drivers/net/wireless/ath/wcn36xx/dxe.c > +++ b/drivers/net/wireless/ath/wcn36xx/dxe.c > @@ -363,7 +363,7 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct > wcn36xx_dxe_ch *ch) > spin_lock_irqsave(&ch->lock, flags); > ctl = ch->tail_blk_ctl; > do { > - if (ctl->desc->ctrl & WCN36xx_DXE_CTRL_VLD) > + if (READ_ONCE(ctl->desc->ctrl) & WCN36xx_DXE_CTRL_VLD) > break; > if (ctl->skb) { > dma_unmap_single(wcn->dev, ctl->desc->src_addr_l, > @@ -382,8 +382,7 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct > wcn36xx_dxe_ch *ch) > ctl->skb = NULL; > } > ctl = ctl->next; > - } while (ctl != ch->head_blk_ctl && > - !(ctl->desc->ctrl & WCN36xx_DXE_CTRL_VLD)); > + } while (ctl != ch->head_blk_ctl); > > ch->tail_blk_ctl = ctl; > spin_unlock_irqrestore(&ch->lock, flags); > @@ -525,7 +524,7 @@ static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn, > int_mask = WCN36XX_DXE_INT_CH3_MASK; > } > > - while (!(dxe->ctrl & WCN36xx_DXE_CTRL_VLD)) { > + while (!(READ_ONCE(dxe->ctrl) & WCN36xx_DXE_CTRL_VLD)) { > skb = ctl->skb; > dma_addr = dxe->dst_addr_l; > ret = wcn36xx_dxe_fill_skb(wcn->dev, ctl, GFP_ATOMIC); > -- > 2.14.3 >
Re: [PATCH v3 1/2] dma-mapping: move dma configuration to bus infrastructure
On Fri, Mar 30, 2018 at 01:24:44PM +0530, Nipun Gupta wrote: > It is bus specific aspect to map a given device on the bus and > relevant firmware description of its DMA configuration. > So, this change introduces 'dma_configure' as bus callback > giving flexibility to busses for implementing its own dma > configuration function. > > The change eases the addition of new busses w.r.t. adding the dma > configuration functionality. > > This patch also updates the PCI, Platform, ACPI and host1x bus to > use new introduced callbacks. s/dma/DMA/ consistently above. > Suggested-by: Christoph Hellwig > Signed-off-by: Nipun Gupta > Reviewed-by: Greg Kroah-Hartman Acked-by: Bjorn Helgaas # PCI parts I assume you'll merge this via some non-PCI tree. Let me know if you need anything else from me. > --- > - The patches are based on the comments on: >https://patchwork.kernel.org/patch/10259087/ > > Changes in v2: > - Do not have dma_deconfigure callback > - Have '/dma_common_configure/' API to provide a common DMA > configuration which can be used by busses if it suits them. > - Platform and ACPI bus to use '/dma_common_configure/' in > '/dma_configure/' callback. > - Updated commit message > - Updated pci_dma_configure API with changes suggested by Robin > > Changes in v3 > - Move dma_common_configure() within platform_dma_configure() and > reuse platofrm_dma_configure() for AMBA bus too > - Declare 'attr' in pci_dma_configure() inside the else statement > where it is used. > > drivers/amba/bus.c | 4 > drivers/base/dma-mapping.c | 31 --- > drivers/base/platform.c | 17 + > drivers/gpu/host1x/bus.c| 8 > drivers/pci/pci-driver.c| 32 > include/linux/device.h | 4 > include/linux/platform_device.h | 2 ++ > 7 files changed, 71 insertions(+), 27 deletions(-) > > diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c > index 594c228..867dc2b 100644 > --- a/drivers/amba/bus.c > +++ b/drivers/amba/bus.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > > #include > > @@ -188,12 +189,15 @@ static int amba_pm_runtime_resume(struct device *dev) > /* > * Primecells are part of the Advanced Microcontroller Bus Architecture, > * so we call the bus "amba". > + * DMA configuration for platform and AMBA bus is same. So here we reuse > + * platform's DMA config routine. > */ > struct bus_type amba_bustype = { > .name = "amba", > .dev_groups = amba_dev_groups, > .match = amba_match, > .uevent = amba_uevent, > + .dma_configure = platform_dma_configure, > .pm = &amba_pm, > .force_dma = true, > }; > diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c > index 3b11835..fdc1502 100644 > --- a/drivers/base/dma-mapping.c > +++ b/drivers/base/dma-mapping.c > @@ -331,36 +331,13 @@ void dma_common_free_remap(void *cpu_addr, size_t size, > unsigned long vm_flags) > #endif > > /* > - * Common configuration to enable DMA API use for a device > + * enables DMA API use for a device > */ > -#include > - > int dma_configure(struct device *dev) > { > - struct device *bridge = NULL, *dma_dev = dev; > - enum dev_dma_attr attr; > - int ret = 0; > - > - if (dev_is_pci(dev)) { > - bridge = pci_get_host_bridge_device(to_pci_dev(dev)); > - dma_dev = bridge; > - if (IS_ENABLED(CONFIG_OF) && dma_dev->parent && > - dma_dev->parent->of_node) > - dma_dev = dma_dev->parent; > - } > - > - if (dma_dev->of_node) { > - ret = of_dma_configure(dev, dma_dev->of_node); > - } else if (has_acpi_companion(dma_dev)) { > - attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode)); > - if (attr != DEV_DMA_NOT_SUPPORTED) > - ret = acpi_dma_configure(dev, attr); > - } > - > - if (bridge) > - pci_put_host_bridge_device(bridge); > - > - return ret; > + if (dev->bus->dma_configure) > + return dev->bus->dma_configure(dev); > + return 0; > } > > void dma_deconfigure(struct device *dev) > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > index f1bf7b3..72fdbf6 100644 > --- a/drivers/base/platform.c > +++ b/drivers/base/platform.c > @@ -1130,6 +1130,22 @@ int platform_pm_restore(struct device *dev) > > #endif /* CONFIG_HIBERNATE_CALLBACKS */ > > +int platform_dma_configure(struct device *dev) > +{ > + enum dev_dma_attr attr; > + int ret = 0; > + > + if (dev->of_node) { > + ret = of_dma_configure(dev, dev->of_node); > + } else if (has_acpi_companion(dev)) { > + attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode)); > + if (attr != DEV_DMA_NOT_SUPPORTED) > +
Re: [PATCH v3 2/2] drivers: remove force dma flag from buses
On Fri, Mar 30, 2018 at 01:24:45PM +0530, Nipun Gupta wrote: > With each bus implementing its own DMA configuration callback, > there is no need for bus to explicitly have force_dma in its > global structure. This patch modifies of_dma_configure API to > accept an input parameter which specifies if implicit DMA > configuration is required even when it is not described by the > firmware. > > Signed-off-by: Nipun Gupta Acked-by: Bjorn Helgaas # PCI parts > --- > Changes in v2: > - This is a new change suggested by Robin and Christoph > and is added to the series. > > Changes in v3: > - Rebase and changes corresponding to the changes in patch 1/2 > > drivers/amba/bus.c| 1 - > drivers/base/platform.c | 3 +-- > drivers/bcma/main.c | 2 +- > drivers/dma/qcom/hidma_mgmt.c | 2 +- > drivers/gpu/host1x/bus.c | 5 ++--- > drivers/of/device.c | 6 -- > drivers/of/of_reserved_mem.c | 2 +- > drivers/pci/pci-driver.c | 3 +-- > include/linux/device.h| 4 > include/linux/of_device.h | 8 ++-- > 10 files changed, 17 insertions(+), 19 deletions(-) > > diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c > index 867dc2b..abe73c4 100644 > --- a/drivers/amba/bus.c > +++ b/drivers/amba/bus.c > @@ -199,7 +199,6 @@ struct bus_type amba_bustype = { > .uevent = amba_uevent, > .dma_configure = platform_dma_configure, > .pm = &amba_pm, > - .force_dma = true, > }; > > static int __init amba_init(void) > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > index 72fdbf6..cfbc569 100644 > --- a/drivers/base/platform.c > +++ b/drivers/base/platform.c > @@ -1136,7 +1136,7 @@ int platform_dma_configure(struct device *dev) > int ret = 0; > > if (dev->of_node) { > - ret = of_dma_configure(dev, dev->of_node); > + ret = of_dma_configure(dev, dev->of_node, true); > } else if (has_acpi_companion(dev)) { > attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode)); > if (attr != DEV_DMA_NOT_SUPPORTED) > @@ -1159,7 +1159,6 @@ struct bus_type platform_bus_type = { > .uevent = platform_uevent, > .dma_configure = platform_dma_configure, > .pm = &platform_dev_pm_ops, > - .force_dma = true, > }; > EXPORT_SYMBOL_GPL(platform_bus_type); > > diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c > index e6986c7..fc1f4ac 100644 > --- a/drivers/bcma/main.c > +++ b/drivers/bcma/main.c > @@ -207,7 +207,7 @@ static void bcma_of_fill_device(struct device *parent, > > core->irq = bcma_of_get_irq(parent, core, 0); > > - of_dma_configure(&core->dev, node); > + of_dma_configure(&core->dev, node, false); > } > > unsigned int bcma_core_irq(struct bcma_device *core, int num) > diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c > index 000c7019..d64edeb 100644 > --- a/drivers/dma/qcom/hidma_mgmt.c > +++ b/drivers/dma/qcom/hidma_mgmt.c > @@ -398,7 +398,7 @@ static int __init hidma_mgmt_of_populate_channels(struct > device_node *np) > } > of_node_get(child); > new_pdev->dev.of_node = child; > - of_dma_configure(&new_pdev->dev, child); > + of_dma_configure(&new_pdev->dev, child, true); > /* >* It is assumed that calling of_msi_configure is safe on >* platforms with or without MSI support. > diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c > index a9ec99d..b39c1e9 100644 > --- a/drivers/gpu/host1x/bus.c > +++ b/drivers/gpu/host1x/bus.c > @@ -317,7 +317,7 @@ static int host1x_device_match(struct device *dev, struct > device_driver *drv) > static int host1x_dma_configure(struct device *dev) > { > if (dev->of_node) > - return of_dma_configure(dev, dev->of_node); > + return of_dma_configure(dev, dev->of_node, true); > return 0; > } > > @@ -335,7 +335,6 @@ struct bus_type host1x_bus_type = { > .match = host1x_device_match, > .dma_configure = host1x_dma_configure, > .pm = &host1x_device_pm_ops, > - .force_dma = true, > }; > > static void __host1x_device_del(struct host1x_device *device) > @@ -424,7 +423,7 @@ static int host1x_device_add(struct host1x *host1x, > device->dev.bus = &host1x_bus_type; > device->dev.parent = host1x->dev; > > - of_dma_configure(&device->dev, host1x->dev->of_node); > + of_dma_configure(&device->dev, host1x->dev->of_node, true); > > err = host1x_device_parse_dt(device, driver); > if (err < 0) { > diff --git a/drivers/of/device.c b/drivers/of/device.c > index 064c818..33d8551 100644 > --- a/drivers/of/device.c > +++ b/drivers/of/device.c > @@ -76,6 +76,8 @@ int of_device_add(struct platform_device *ofdev) > * of_dma_configure - Setup DMA configuration > * @dev: Device to apply DMA configuration
second wifi card enforce CN reg dom
hi. I am trying to capture on 2 channels at the same time with 2 cards. One card is TP-Link TL-W722N v1 using ath9k_htc and the second one is an Alfa AWUS051NH v2 using rt2800usb. I have tried this, first, on raspberry pi 0 W using archlinux-arm and reproduced the issue on a netbook using archlinux x64 too using latest kernel and drivers. (seems to happen on ubuntu 17.10 on dell laptop too) So when the Alfa card is used alone using the default reg dom FR, one can change to 112 channel for example (using iw dev wlan1 set channel 112) But once the tp-link is plugged in, reg dom seems to become CN and one can't change the alfa card to 112 channel. iw reg get output change from global country FR: DFS-ETSI (2402 - 2482 @ 40), (N/A, 20), (N/A) (5170 - 5250 @ 80), (N/A, 20), (N/A), AUTO-BW (5250 - 5330 @ 80), (N/A, 20), (0 ms), DFS, AUTO-BW (5490 - 5710 @ 160), (N/A, 27), (0 ms), DFS (57000 - 66000 @ 2160), (N/A, 40), (N/A) to global country 98: DFS-UNSET (2402 - 2482 @ 40), (N/A, 20), (N/A) (5170 - 5250 @ 80), (N/A, 20), (N/A), AUTO-BW (5250 - 5330 @ 80), (N/A, 20), (0 ms), DFS, AUTO-BW (57240 - 59400 @ 2160), (N/A, 28), (N/A) (59400 - 63720 @ 2160), (N/A, 40), (N/A) (63720 - 65880 @ 2160), (N/A, 28), (N/A) phy#2 country CN: DFS-FCC (2402 - 2482 @ 40), (N/A, 20), (N/A) (5170 - 5250 @ 80), (N/A, 23), (N/A), AUTO-BW (5250 - 5330 @ 80), (N/A, 23), (0 ms), DFS, AUTO-BW (5735 - 5835 @ 80), (N/A, 30), (N/A) (57240 - 59400 @ 2160), (N/A, 28), (N/A) (59400 - 63720 @ 2160), (N/A, 44), (N/A) (63720 - 65880 @ 2160), (N/A, 28), (N/A) and all the channels above 100 are marked as disabled in iw list output (after the plug not before) for the alfa card It is as if the TL-WN722N has CN reg dom hard-coded and that switches it globally to CN too ??? Is this a bug in ath9k_htc ? a bug with the TL-WN722N card ??
Re: [PATCH 0/1] RFC: memory coherency and data races on TX path
Hi Ramon, On Tuesday, April 10, 2018 08:11 PM, Ramon Fried wrote: > On 10 April 2018 at 20:42, Daniel Mack wrote: >> I found something that I believe might be an issue, and I have an >> idea on how to correct this, but unfortunately, this doesn't solve >> the issues I occasionally see with this driver. I'd still like to >> share it, because I might be totally mistaken in my understanding. > > Thanks for sharing you idea. Are you aware of the recent patch I sent > that Loic Poulain > wrote that fixes a race issue in access to wcn36xx_dxe_tx_frame() ? > Kalle just recently applied it to ath-next, I don't think it's > available yet upstream. Yes, my patch builds upon yours. > The patch was fixing something similar, perhaps it will solve the > issue you're experiencing. I'm not even sure what kind of effect I'm hunting, so it's hard to tell. Your patch definitely addresses a data race too though. >> There's a problem with the latter presumption however which looks like >> this in the driver code: >> >> desc->fr_len = ctl->skb->len; >> /* set dxe descriptor to VALID */ >> desc->ctrl = ch->ctrl_skb; > > The firmware won't start reading the descriptors until it receives an > interrupt from driver. > which in turn happen later in the function using: wcn36xx_dxe_write_register() > so I don't think reordering in this case make any problem. I understand, but as I said, there's definitely the problem that the channel is already running when wcn36xx_dxe_tx_frame() is entered. Try adding this at the beginning of the the function and then run iperf: int reg; wcn36xx_dxe_read_register(wcn, ch->reg_ctrl, ®); WARN_ON(reg & WCN36xx_DXE_CH_CTRL_EN); I fail to see how the firmware would determine which descriptors to look at without any type of synchronization mechanism. >> Does that make sense? As I said, I can't really say this improves >> anything, sadly, so I might be mistaken entirely. But I'll leave this >> here for further discussion. Ideally, somebody with access to the >> firmware sources could give an assessment whether this is an issue at >> all or not. > > When I'm not sure regarding some implementation I consult with the > downstream pronto driver. > https://github.com/sultanqasim/qcom_wlan_prima > > Did you look there if they actually placed wmb() ? Yes, I've read some of that as well. They use wmb() before writing to and rmb() after reading firmware registers. The equivalent upstream is wcn36xx_dxe_[read,write}_register(), and they use writel() and readl() which have the same barriers on arm64. So that's fine. What's interesting though is that the downstream drivers sets the VLD bit of the first descriptor of the chain *after* they set all the others. There are also some confusing comments about that (/* First frame not set VAL bit, why ??? */). Can you make sense of that? Thanks, Daniel
[PATCH 1/2] rsi: fix nommu_map_sg overflow kernel panic
From: Siva Rebbagondla Following overflow kernel panic is observed on some platforms while loading the driver. It is fixed if dynamically allocated memory is passed to SDIO instead of static one [ 927.513963] nommu_map_sg: overflow 17d54064ba7c+20 of device mask [ 927.517712] Modules linked in: rsi_sdio(+) cmac bnep arc4 rsi_91x mac80211 cfg80211 btrsi rfcomm bluetooth ecdh_generic snd_soc_sst_bytcr_rt5660 [ 927.517861] CPU: 0 PID: 1624 Comm: insmod Tainted: G W 4.15.0-1000 #1 [ 927.517870] RIP: 0010:sdhci_send_command+0x5f0/0xa90 [sdhci] [ 927.517873] RSP: :ac3fc064b6d8 EFLAGS: 00010086 [ 927.517895] Call Trace: [ 927.517908] ? __schedule+0x3cd/0x890 [ 927.517915] ? mod_timer+0x17b/0x3c0 [ 927.517922] sdhci_request+0x7c/0xf0 [sdhci] [ 927.517928] __mmc_start_request+0x5a/0x170 [ 927.517932] mmc_start_request+0x74/0x90 [ 927.517936] mmc_wait_for_req+0x87/0xe0 [ 927.517940] mmc_io_rw_extended+0x2fd/0x330 [ 927.517946] ? mmc_wait_data_done+0x30/0x30 [ 927.517951] sdio_io_rw_ext_helper+0x160/0x210 [ 927.517956] sdio_writesb+0x1d/0x20 [ 927.517966] rsi_sdio_write_register_multiple+0x68/0x110 [rsi_sdio] [ 927.517976] rsi_hal_device_init+0x357/0x910 [rsi_91x] [ 927.517983] ? rsi_hal_device_init+0x357/0x910 [rsi_91x] [ 927.517990] rsi_probe+0x2c6/0x450 [rsi_sdio] [ 927.517995] sdio_bus_probe+0xfc/0x110 [ 927.518000] driver_probe_device+0x2b3/0x490 [ 927.518005] __driver_attach+0xdf/0xf0 [ 927.518008] ? driver_probe_device+0x490/0x490 [ 927.518014] bus_for_each_dev+0x6c/0xc0 [ 927.518018] driver_attach+0x1e/0x20 [ 927.518021] bus_add_driver+0x1f4/0x270 [ 927.518028] ? rsi_sdio_ack_intr+0x50/0x50 [rsi_sdio] [ 927.518031] driver_register+0x60/0xe0 [ 927.518038] ? rsi_sdio_ack_intr+0x50/0x50 [rsi_sdio] [ 927.518041] sdio_register_driver+0x20/0x30 [ 927.518047] rsi_module_init+0x16/0x40 [rsi_sdio] Signed-off-by: Siva Rebbagondla Signed-off-by: Amitkumar Karwar --- drivers/net/wireless/rsi/rsi_91x_hal.c | 35 - drivers/net/wireless/rsi/rsi_91x_sdio.c | 21 +--- drivers/net/wireless/rsi/rsi_sdio.h | 2 +- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/drivers/net/wireless/rsi/rsi_91x_hal.c b/drivers/net/wireless/rsi/rsi_91x_hal.c index b7c5403..0761e61 100644 --- a/drivers/net/wireless/rsi/rsi_91x_hal.c +++ b/drivers/net/wireless/rsi/rsi_91x_hal.c @@ -635,28 +635,32 @@ static int bl_write_header(struct rsi_hw *adapter, u8 *flash_content, u32 content_size) { struct rsi_host_intf_ops *hif_ops = adapter->host_intf_ops; - struct bl_header bl_hdr; + struct bl_header *bl_hdr; u32 write_addr, write_len; int status; - bl_hdr.flags = 0; - bl_hdr.image_no = cpu_to_le32(adapter->priv->coex_mode); - bl_hdr.check_sum = cpu_to_le32( - *(u32 *)&flash_content[CHECK_SUM_OFFSET]); - bl_hdr.flash_start_address = cpu_to_le32( - *(u32 *)&flash_content[ADDR_OFFSET]); - bl_hdr.flash_len = cpu_to_le32(*(u32 *)&flash_content[LEN_OFFSET]); + bl_hdr = kzalloc(sizeof(*bl_hdr), GFP_KERNEL); + if (!bl_hdr) + return -ENOMEM; + + bl_hdr->flags = 0; + bl_hdr->image_no = cpu_to_le32(adapter->priv->coex_mode); + bl_hdr->check_sum = + cpu_to_le32(*(u32 *)&flash_content[CHECK_SUM_OFFSET]); + bl_hdr->flash_start_address = + cpu_to_le32(*(u32 *)&flash_content[ADDR_OFFSET]); + bl_hdr->flash_len = cpu_to_le32(*(u32 *)&flash_content[LEN_OFFSET]); write_len = sizeof(struct bl_header); if (adapter->rsi_host_intf == RSI_HOST_INTF_USB) { write_addr = PING_BUFFER_ADDRESS; status = hif_ops->write_reg_multiple(adapter, write_addr, -(u8 *)&bl_hdr, write_len); +(u8 *)bl_hdr, write_len); if (status < 0) { rsi_dbg(ERR_ZONE, "%s: Failed to load Version/CRC structure\n", __func__); - return status; + goto fail; } } else { write_addr = PING_BUFFER_ADDRESS >> 16; @@ -665,20 +669,23 @@ static int bl_write_header(struct rsi_hw *adapter, u8 *flash_content, rsi_dbg(ERR_ZONE, "%s: Unable to set ms word to common reg\n", __func__); - return status; + goto fail; } write_addr = RSI_SD_REQUEST_MASTER | (PING_BUFFER_ADDRESS & 0x); status = hif_ops->write_reg_multiple(adapter, write_addr, -
[PATCH 2/2] rsi: Fix 'invalid vdd' warning in mmc
From: Siva Rebbagondla While performing cleanup, driver is messing with card->ocr value by not masking rocr against ocr_avail. Below panic is observed with some of the SDIO host controllers due to this. Issue is resolved by reverting incorrect modifications to vdd. [ 927.423821] mmc1: Invalid vdd 0x1f [ 927.423925] Modules linked in: rsi_sdio(+) cmac bnep arc4 rsi_91x mac80211 cfg80211 btrsi rfcomm bluetooth ecdh_generic [ 927.424073] CPU: 0 PID: 1624 Comm: insmod Tainted: G W 4.15.0-1000-caracalla #1 [ 927.424075] Hardware name: Dell Inc. Edge Gateway3003/ , BIOS 01.00.06 01/22/2018 [ 927.424082] RIP: 0010:sdhci_set_power_noreg+0xdd/0x190[sdhci] [ 927.424085] RSP: 0018:ac3fc064b930 EFLAGS: 00010282 [ 927.424107] Call Trace: [ 927.424118] sdhci_set_power+0x5a/0x60 [sdhci] [ 927.424125] sdhci_set_ios+0x360/0x3b0 [sdhci] [ 927.424133] mmc_set_initial_state+0x92/0x120 [ 927.424137] mmc_power_up.part.34+0x33/0x1d0 [ 927.424141] mmc_power_up+0x17/0x20 [ 927.424147] mmc_sdio_runtime_resume+0x2d/0x50 [ 927.424151] mmc_runtime_resume+0x17/0x20 [ 927.424156] __rpm_callback+0xc4/0x200 [ 927.424161] ? idr_alloc_cyclic+0x57/0xd0 [ 927.424165] ? mmc_runtime_suspend+0x20/0x20 [ 927.424169] rpm_callback+0x24/0x80 [ 927.424172] ? mmc_runtime_suspend+0x20/0x20 [ 927.424176] rpm_resume+0x4b3/0x6c0 [ 927.424181] __pm_runtime_resume+0x4e/0x80 [ 927.424188] driver_probe_device+0x41/0x490 [ 927.424192] __driver_attach+0xdf/0xf0 [ 927.424196] ? driver_probe_device+0x490/0x490 [ 927.424201] bus_for_each_dev+0x6c/0xc0 [ 927.424205] driver_attach+0x1e/0x20 [ 927.424209] bus_add_driver+0x1f4/0x270 [ 927.424217] ? rsi_sdio_ack_intr+0x50/0x50 [rsi_sdio] [ 927.424221] driver_register+0x60/0xe0 [ 927.424227] ? rsi_sdio_ack_intr+0x50/0x50 [rsi_sdio] [ 927.424231] sdio_register_driver+0x20/0x30 [ 927.424237] rsi_module_init+0x16/0x40 [rsi_sdio] Signed-off-by: Siva Rebbagondla Signed-off-by: Amitkumar Karwar --- drivers/net/wireless/rsi/rsi_91x_sdio.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c index f7f2820..416981d 100644 --- a/drivers/net/wireless/rsi/rsi_91x_sdio.c +++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c @@ -170,7 +170,6 @@ static void rsi_reset_card(struct sdio_func *pfunction) int err; struct mmc_card *card = pfunction->card; struct mmc_host *host = card->host; - s32 bit = (fls(host->ocr_avail) - 1); u8 cmd52_resp; u32 clock, resp, i; u16 rca; @@ -190,7 +189,6 @@ static void rsi_reset_card(struct sdio_func *pfunction) msleep(20); /* Initialize the SDIO card */ - host->ios.vdd = bit; host->ios.chip_select = MMC_CS_DONTCARE; host->ios.bus_mode = MMC_BUSMODE_OPENDRAIN; host->ios.power_mode = MMC_POWER_UP; -- 2.7.4
Re: [PATCH] wcn36xx: use READ_ONCE() to access desc->ctrl
On Tuesday, April 10, 2018 08:17 PM, Ramon Fried wrote: > On 10 April 2018 at 20:35, Daniel Mack wrote: >> When accessing shared memory to check for the stat of submitted >> descriptors, make sure to use READ_ONCE(). This will guarantee the >> compiler treats these memory locations as volatile and doesn't apply >> any caching. > > The structure that is tested is not shared memory. It's accessed only > by the apps processor. Hmm? ctl->desc is of type struct wcn36xx_dxe_desc, which is packed and shared with the firmware. The WCN36xx_DXE_CTRL_VLD bit in ctrl bitfield is set by the firmware when a frame is valid, and before asserting the RX interrupt. So the host CPU must treat it as volatile and expect it to change. Am I reading this wrong? Thanks, Daniel