Re: [PATCH 03/11] ath10k_sdio: DMA bounce buffers for read write
On 2017-12-22 21:38, Kalle Valo wrote: silexcom...@gmail.com writes: From: Alagu Sankar Some SD host controllers still need bounce buffers for SDIO data transfers. While the transfers worked fine on x86 platforms, this is found to be required for i.MX6 based systems. Changes are similar to and derived from the ath6kl sdio driver. Signed-off-by: Alagu Sankar Why is the bounce buffer needed exactly, what are the symptoms etc? To me this sounds like an ugly workaround for a SDIO controller driver bug. We faced problems with i.MX6. The authentication frame sent by the driver never reached the air. The host driver accepted the buffer, but did not send out the packet to the sdio module. No errors reported anywhere, but the buffer is not accepted due to alignment. The same driver however works fine without bounce buffer on x86 platform with stdhci drivers. To make it compliant with all host controllers, we introduced the bounce buffers, similar to what was done in ath6kl_sdio drivers.
Re: [08/11] ath10k_sdio: common read write
Hi Gary, On 2017-10-05 15:39, Gary Bisson wrote: Hi Alagu, On Sat, Sep 30, 2017 at 11:07:45PM +0530, silexcom...@gmail.com wrote: From: Alagu Sankar convert different read write functions in sdio hif to bring it under a single read-write path. This helps in having a common dma bounce buffer implementation. Also helps in address modification that is required specific to change in certain mbox addresses of sdio_write. Signed-off-by: Alagu Sankar --- drivers/net/wireless/ath/ath10k/sdio.c | 131 - 1 file changed, 64 insertions(+), 67 deletions(-) diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c index 77d4fa4..bb6fa67 100644 --- a/drivers/net/wireless/ath/ath10k/sdio.c +++ b/drivers/net/wireless/ath/ath10k/sdio.c @@ -36,6 +36,11 @@ #define ATH10K_SDIO_DMA_BUF_SIZE (32 * 1024) +static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf, + u32 len, bool incr); +static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf, +u32 len, bool incr); + As mentioned by Kalle, u32 needs to be size_t. Yes, the compiler I used is probably a step older and did not catch this. /* inlined helper functions */ /* Macro to check if DMA buffer is WORD-aligned and DMA-able. @@ -152,6 +157,7 @@ static int ath10k_sdio_config(struct ath10k *ar) struct sdio_func *func = ar_sdio->func; unsigned char byte, asyncintdelay = 2; int ret; + u32 addr; ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio configuration\n"); @@ -180,9 +186,8 @@ static int ath10k_sdio_config(struct ath10k *ar) CCCR_SDIO_DRIVER_STRENGTH_ENABLE_C | CCCR_SDIO_DRIVER_STRENGTH_ENABLE_D); - ret = ath10k_sdio_func0_cmd52_wr_byte(func->card, - CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR, - byte); + addr = CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR, + ret = ath10k_sdio_func0_cmd52_wr_byte(func->card, addr, byte); Not sure this part is needed. This is to overcome checkpatch warning. Too big a names for the function and macro not helping in there. Will have to move it as a separate patch. if (ret) { ath10k_warn(ar, "failed to enable driver strength: %d\n", ret); goto out; @@ -233,13 +238,16 @@ static int ath10k_sdio_config(struct ath10k *ar) static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val) { - struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar); - struct sdio_func *func = ar_sdio->func; + __le32 *buf; int ret; - sdio_claim_host(func); + buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) + return -ENOMEM; - sdio_writel(func, val, addr, &ret); + *buf = cpu_to_le32(val); + + ret = ath10k_sdio_write(ar, addr, &val, sizeof(val), true); Shouldn't we use buf instead of val? buf seems pretty useless otherwise. Yes, thanks for pointing this out. will be corrected in v2. Regards, Gary ___ ath10k mailing list ath...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 00/11] SDIO support for ath10k
Hi Gary, On 05-10-2017 20:42, Gary Bisson wrote: Hi Alagu, First of all, thank you for sharing your patches, this will be a very nice improvement to have SDIO QCA9377 working with ath10k. I've tried your series with Nitrogen7 [1] platform which is supported in mainline already. It uses BD-SDMAC [2] which uses the same module as the SX-SDMAC. Below are some questions/remarks I have after the testing. On Sat, Sep 30, 2017 at 11:07:37PM +0530, silexcommon at gmail.com wrote: From: Alagu Sankar This patchset, generated against master-pending branch, enables a fully functional SDIO interface driver for ath10k. Patches have been verified on QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point and P2P modes. The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1 Quick question on the firmware, is it the one from Kalle's repository?[3] If so, where does this firmware comes from? Is 00061 the firmware version? So far I've only seen up to v0.0.0.60, see qcacld-2.0 output: Host SW:4.5.20.037, FW:0.0.0.60, HW:QCA9377_REV1_1 Yes, it is from https://github.com/kvalo/ath10k-firmware/tree/master/QCA9377/hw1.0/untested. I have also used custom firmware from QCA/Silex as used in qcacld-2.0 driver without any issue. You need to use the firmware merger tool from https://github.com/erstrom/linux-ath/wiki/Firmware to combine the qwlan30.bin and otp30.bin to generate the firmware-sdio.bin. with the board data from respective SDIO card vendors. About the board-sdio.bin, is it just a copy of your bdwlan30.bin? Yes board-sdio.bin is a copy of bdwlan30.bin Receive performance matches the QCA reference driver when used with SDIO3.0 enabled platforms. iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s Nice performances. Unfortunately I don't get better than 30Mbits/s in TX and 50Mbits/s in RX: # iperf -c 192.168.1.1 Client connecting to 192.168.1.1, TCP port 5001 TCP window size: 43.8 KByte (default) [ 3] local 192.168.1.115 port 41354 connected with 192.168.1.1 port 5001 [ ID] Interval Transfer Bandwidth [ 3] 0.0-10.0 sec 34.9 MBytes 29.2 Mbits/sec # iperf -s Server listening on TCP port 5001 TCP window size: 85.3 KByte (default) [ 4] local 192.168.1.115 port 5001 connected with 192.168.1.1 port 50646 [ ID] Interval Transfer Bandwidth [ 4] 0.0-10.0 sec 63.1 MBytes 52.7 Mbits/sec Do you have any idea why? Note that qcacld-2.0 driver on that same platform (same OS) gives the performances you advertize (150Mbits/s). For some reason, if I use the imx_v6_v7_defconfig as is, performance is very poor. In fact, the firmware download itself will take about 6 seconds. This can also be seen when you up/down the wlan0 interface. For the i.MX6 SoloX board, I used the configuration of 4.1.15 as provided by the BSP from NXP/Freescale. This improved the performance quite a bit. I haven't had a chance to spend time on this to figure out the difference and reason. Another difference is that the default device tree for SoloX did not have the correct settings to support SDIO3.x. Had to modify them, but did not include the device tree patches here as it is not meant for this group. This patchset differs from the previous high latency patches, specific to SDIO. HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs the firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without this flag, the management frames are not sent out by the firmware. Possibility of management frames being sent via WMI and data frames through the reduced Tx completion needs to be probed further. Further improvements can be done on the transmit path by implementing packet bundle. Scatter Gather is another area of improvement for both Transmit and Receive, but may not work on all platforms Known issues: Surprise removal of the card, when the device is in connected state, delays sdio function remove due to delayed WMI command failures. Existing ath10k framework can not differentiate between a kernel module removae and the surprise removal of teh card. Here are some questions: - Is it normal to see a warning about board-2.bin, shouldn't it look for board-sdio.bin only? [ 14.696704] ath10k_sdio mmc1:0001:1: Direct firmware load for ath10k/QCA9377/hw1.0/board-2.bin failed with error -2 This was only noticed in the latest update. Like the different firmware versions, the driver also looks for the board-2.bin now. I have only tested with the board-sdio.bin - Did you have pre-cal and/or cal binaries for your testing? [ 14.067159] ath10k_sdio mmc1:0001:1: Direct firmware load for ath10k/pre-cal-sdio-mmc1:0001:1.bin failed with error -2 [
Re: [PATCH 00/11] SDIO support for ath10k
Hi Erik, We will work to have this support mainlined as soon as possible. Would appreciate your support in making sure that the patches do not affect the USB high latency path. On 02-10-2017 14:32, Erik Stromdahl wrote: Hi Alagu, It is great to see that we are finally about have fully working mainline support for QCA9377 SDIO chipsets! Great job! On 2017-09-30 19:37, silexcom...@gmail.com wrote: From: Alagu Sankar This patchset, generated against master-pending branch, enables a fully functional SDIO interface driver for ath10k. Patches have been verified on QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point and P2P modes. The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1 with the board data from respective SDIO card vendors. Receive performance matches the QCA reference driver when used with SDIO3.0 enabled platforms. iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s Can you share any scripts etc. (wrapping hostapd and wpa_supplicant stuff) or provide some more info about you test setup? I am not using any specific scripts for this. The standard ones from the ath10k configuration page https://wireless.wiki.kernel.org/en/users/drivers/ath10k/configuration works just fine with the NL80211 path. I made a quick socat based test on an old laptop (I don't think it has SDIO 3.0 support) and I did unfortunately not get the same figures as you did :( If it is SDIO v1.x, then the max bus speed is only 100Mbit/s. With v2.x it is 200Mbit/s and 3.x it is 832 Mbit/s. Throughput primarily depends on it. We used i.MX6 SoloX Sabre SDB platform which supports SDIO3.x and could see the maximum performance. This patchset differs from the previous high latency patches, specific to SDIO. HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs the firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without this flag, the management frames are not sent out by the firmware. Possibility of management frames being sent via WMI and data frames through the reduced Tx completion needs to be probed further. Ah, so that explains why I couldn't see any messages in the air. Further improvements can be done on the transmit path by implementing packet bundle. Scatter Gather is another area of improvement for both Transmit and Receive, but may not work on all platforms Known issues: Surprise removal of the card, when the device is in connected state, delays sdio function remove due to delayed WMI command failures. Existing ath10k framework can not differentiate between a kernel module removal and the surprise removal of teh card. Alagu Sankar (11): ath10k_sdio: sdio htt data transfer fixes ath10k_sdio: wb396 reference card fix ath10k_sdio: DMA bounce buffers for read write ath10k_sdio: reduce transmit msdu count ath10k_sdio: use clean packet headers ath10k_sdio: high latency fixes for beacon buffer ath10k_sdio: fix rssi indication ath10k_sdio: common read write ath10k_sdio: virtual scatter gather for receive ath10k_sdio: enable firmware crash dump ath10k_sdio: hif start once addition drivers/net/wireless/ath/ath10k/core.c| 35 ++- drivers/net/wireless/ath/ath10k/debug.c | 3 + drivers/net/wireless/ath/ath10k/htc.c | 4 +- drivers/net/wireless/ath/ath10k/htc.h | 1 + drivers/net/wireless/ath/ath10k/htt_rx.c | 19 +- drivers/net/wireless/ath/ath10k/htt_tx.c | 24 +- drivers/net/wireless/ath/ath10k/hw.c | 2 + drivers/net/wireless/ath/ath10k/hw.h | 1 + drivers/net/wireless/ath/ath10k/mac.c | 31 ++- drivers/net/wireless/ath/ath10k/sdio.c| 398 ++ drivers/net/wireless/ath/ath10k/sdio.h| 10 +- drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 +- 12 files changed, 403 insertions(+), 127 deletions(-)
Re: [PATCH 02/11] ath10k_sdio: wb396 reference card fix
Hi Steve, On 2017-10-02 04:17, Steve deRosier wrote: Hi Alagu, On Sat, Sep 30, 2017 at 10:37 AM, wrote: From: Alagu Sankar The QCA9377-3 WB396 sdio reference card does not get initialized due to the conflict in uart gpio pins. This fix is not required for other QCA9377 sdio cards. Signed-off-by: Alagu Sankar --- drivers/net/wireless/ath/ath10k/core.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index b4f66cd..86247c8 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -1708,8 +1708,15 @@ static int ath10k_init_uart(struct ath10k *ar) return ret; } - if (!uart_print) + if (!uart_print) { + /* Hack: override dbg TX pin to avoid side effects of default +* GPIO_6 in QCA9377 WB396 reference card +*/ + if (ar->hif.bus == ATH10K_BUS_SDIO) + ath10k_bmi_write32(ar, hi_dbg_uart_txpin, + ar->hw_params.uart_pin); If it is indeed a "hack", then I don't think the maintainer should accept this upstream. If you want it upstream you need a clean enough implementation that doesn't need to be labeled a "hack". It is a hack as per the qcacld reference driver. Your commit message states that this is only needed for a very specific card and not for other QCA9377 sdio cards. Yet, you're doing this for all ATH10K_BUS_SDIO devices. Not good. I think that it's a quirk and it's limited to a particular implementation of the device. My suggestion: if it can be automatically determined, then do so explicitly. If not, then it needs to be a DT setting or a module parameter or something like that so the platform maker can decide to do it. Having it affect all users of a SDIO QCA9377 when it doesn't apply doesn't seem like a good idea to me. - Steve Got it. The qcacld reference driver had it for all the QCA9377 sdio cards. But we found it to be a problem only for the WB396 reference card. Will have this checked again and release a v2 patch accordingly. Best Regards, Alagu Sankar
Re: [PATCH 01/11] ath10k_sdio: sdio htt data transfer fixes
Hi Arend, On 2017-10-02 13:06, Arend van Spriel wrote: On 9/30/2017 7:37 PM, silexcom...@gmail.com wrote: From: Alagu Sankar [...] Signed-off-by: Alagu Sankar Not really have a specific remark for this patch, but for the entire series. These patches are sent using an anonymous email address, apart from 'silex' being in there, which does not show up in the certificate of origin. Just wondering if this is acceptable? Regards, Arend --- drivers/net/wireless/ath/ath10k/core.c | 20 +--- drivers/net/wireless/ath/ath10k/htt_rx.c | 6 -- drivers/net/wireless/ath/ath10k/htt_tx.c | 24 +++- 3 files changed, 40 insertions(+), 10 deletions(-) Could not use git send-email from the official ID due to mail server restrictions. If this is not acceptable, I will figure out a way to overcome this. Regards, Alagu Sankar