How to optimize SDIO WIFI performance?

Question

Hello,I am working on a project that is using wifi to communicate with the connected device and the latency is critical. We are using the wifi JODY-W263-00B from U-Blox connected by SDIO bus, and for measuring the latency we simply use ping command. The average latency of our setup is around 10 ms. As this project is the third generation of our product we have a comparison with the previous generation that was based on the imx6 chip from NXP, and with it, we were able to reach average latency of around 1.8 ms, but we were using a different wifi module (ELLA-W161). To make this comparable we have made a few measurements using different CPUs, wifi modules, and connection types. Our results are in this spreadsheet. In the name/board column is the name of the device to which was wifi module connected, the WIFI connection column shows how we connected module/evk kit, and columns under Ping [ms] are our measurements. Other columns are self-explanatory as I hope.From measurements, you can see that the average latency of wifi modules connected to the imx6 is lower than the latency of STM32MP1-based modules, regardless of the HW connection or used driver. This implies that the issue is with the STM SDIO implementation, either in HW or SW.For our tests, we used the latest ST ecosystem for ST devices and the current upstream for imx6 (kernel 6.1 but the driver supports only 5.15 but it still performs better).Can you please help me to acknowledge this issue and hopefully resolve it?p.s.: the tests were done using wipi wifi module and Windows 11.

Tomáš Juřena · Accepted Answer

Hi,with the recent changes in the 6.1 ST kernel branch that enables the SDIO interrupt, I have managed to resolve the latency issue for 8987 chipset used in the JODY-W2 module.If anyone is facing latency issues you can try to apply the attached patch to backport the new SDIO interrupt functionality to the ST kernel version 5.15, and add cap-sdio-irq property to the DTS.My thanks to the kernel team for implementing this. Do you plan to backport it to the 5.15 kernel as well?Best regards,Tomáš.p.s.: Since I cannot upload the patch as a file, I add it as text.From 82ee7ffbaa937a7a3edc8ca95aa93bff7ab7d097 Mon Sep 17 00:00:00 2001From: Tomas Jurena <tjurena@techniserv.cz>Date: Thu, 29 Jun 2023 05:03:24 +0000Subject: [PATCH] mmc: mmci: stm32: Backport changes from kernel 6.1---drivers/mmc/host/mmci.c | 67 ++++++++++++++++++++++++++++-drivers/mmc/host/mmci.h | 4 ++drivers/mmc/host/mmci_stm32_sdmmc.c | 21 +++++++++3 files changed, 90 insertions(+), 2 deletions(-)diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.cindex 3d416c4ed..05cfc28a5 100644--- a/drivers/mmc/host/mmci.c+++ b/drivers/mmc/host/mmci.c@@ -270,6 +270,7 @@ static struct variant_data variant_stm32_sdmmc = {.datactrl_any_blocksz = true,.datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,.stm32_idmabsize_mask = GENMASK(12, 5),+ .use_sdio_irq = true,.busy_timeout = true,.busy_detect = true,.busy_detect_flag = MCI_STM32_BUSYD0,@@ -296,6 +297,7 @@ static struct variant_data variant_stm32_sdmmcv2 = {.datactrl_any_blocksz = true,.datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,.stm32_idmabsize_mask = GENMASK(16, 5),+ .use_sdio_irq = true,.dma_lli = true,.busy_timeout = true,.busy_detect = true,@@ -392,6 +394,10 @@ static void mmci_write_datactrlreg(struct mmci_host *host, u32 datactrl)/* Keep busy mode in DPSM if enabled */datactrl |= host->datactrl_reg & host->variant->busy_dpsm_flag;+ /* Keep SD I/O interrupt mode enabled */+ if (host->variant->use_sdio_irq && host->mmc->caps & MMC_CAP_SDIO_IRQ)+ datactrl |= host->variant->datactrl_mask_sdio;+if (host->datactrl_reg != datactrl) {host->datactrl_reg = datactrl;writel(datactrl, host->base + MMCIDATACTRL);@@ -1644,6 +1650,11 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)mmci_data_irq(host, host->data, status);}+ if (host->variant->use_sdio_irq &&+ host->mmc->caps & MMC_CAP_SDIO_IRQ &&+ host->ops && host->ops->sdio_irq)+ host->ops->sdio_irq(host, status);+/** Busy detection has been handled by mmci_cmd_irq() above.* Clear the status bit to prevent polling in IRQ context.@@ -1729,7 +1740,8 @@ static void mmci_set_max_busy_timeout(struct mmc_host *mmc)return;if (host->variant->busy_timeout && mmc->actual_clock)- max_busy_timeout = ~0UL / (mmc->actual_clock / MSEC_PER_SEC);+ max_busy_timeout = U32_MAX / DIV_ROUND_UP(mmc->actual_clock,+ MSEC_PER_SEC);mmc->max_busy_timeout = max_busy_timeout;}@@ -1883,6 +1895,45 @@ static int mmci_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)return ret;}+static void mmci_enable_sdio_irq(struct mmc_host *mmc, int enable)+{+ struct mmci_host *host = mmc_priv(mmc);+ unsigned long flags;++ if (!host->variant->use_sdio_irq)+ return;++ if (host->ops && host->ops->enable_sdio_irq) {+ if (enable)+ /* Keep device active while SDIO IRQ is enabled */+ pm_runtime_get_sync(mmc_dev(mmc));++ spin_lock_irqsave(&host->lock, flags);+ host->ops->enable_sdio_irq(host, enable);+ spin_unlock_irqrestore(&host->lock, flags);++ if (!enable) {+ pm_runtime_mark_last_busy(mmc_dev(mmc));+ pm_runtime_put_autosuspend(mmc_dev(mmc));+ }+ }+}++static void mmci_ack_sdio_irq(struct mmc_host *mmc)+{+ struct mmci_host *host = mmc_priv(mmc);+ unsigned long flags;++ if (!host->variant->use_sdio_irq)+ return;++ if (host->ops && host->ops->enable_sdio_irq) {+ spin_lock_irqsave(&host->lock, flags);+ host->ops->enable_sdio_irq(host, 1);+ spin_unlock_irqrestore(&host->lock, flags);+ }+}+static struct mmc_host_ops mmci_ops = {.request = mmci_request,.pre_req = mmci_pre_request,@@ -1891,6 +1942,8 @@ static struct mmc_host_ops mmci_ops = {.get_ro = mmc_gpio_get_ro,.get_cd = mmci_get_cd,.start_signal_voltage_switch = mmci_sig_volt_switch,+ .enable_sdio_irq = mmci_enable_sdio_irq,+ .ack_sdio_irq = mmci_ack_sdio_irq,};static void mmci_probe_level_translator(struct mmc_host *mmc)@@ -2158,6 +2211,14 @@ static int mmci_probe(struct amba_device *dev,mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;}+ if (variant->use_sdio_irq && host->mmc->caps & MMC_CAP_SDIO_IRQ) {+ mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;++ if (variant->datactrl_mask_sdio)+ mmci_write_datactrlreg(host,+ host->variant->datactrl_mask_sdio);+ }+/* Variants with mandatory busy timeout in HW needs R1B responses. */if (variant->busy_timeout)mmc->caps |= MMC_CAP_NEED_RSP_BUSY;@@ -2254,7 +2315,9 @@ static int mmci_probe(struct amba_device *dev,pm_runtime_set_autosuspend_delay(&dev->dev, 50);pm_runtime_use_autosuspend(&dev->dev);- mmc_add_host(mmc);+ ret = mmc_add_host(mmc);+ if (ret)+ goto clk_disable;pm_runtime_put(&dev->dev);return 0;diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.hindex e1a9b96a3..a710cd686 100644--- a/drivers/mmc/host/mmci.h+++ b/drivers/mmc/host/mmci.h@@ -316,6 +316,7 @@ struct mmci_host;* @opendrain: bitmask identifying the OPENDRAIN bit inside MMCIPOWER register* @dma_lli: true if variant has dma link list feature.* @stm32_idmabsize_mask: stm32 sdmmc idma buffer size.+ * @use_sdio_irq: allow SD I/O card to interrupt the host*/struct variant_data {unsigned int clkreg;@@ -360,6 +361,7 @@ struct variant_data {u32 start_err;u32 opendrain;u8 dma_lli:1;+ u8 use_sdio_irq:1;u32 stm32_idmabsize_mask;void (*init)(struct mmci_host *host);};@@ -383,6 +385,8 @@ struct mmci_host_ops {bool (*busy_complete)(struct mmci_host *host, u32 status, u32 err_msk);void (*pre_sig_volt_switch)(struct mmci_host *host);int (*post_sig_volt_switch)(struct mmci_host *host, struct mmc_ios *ios);+ void (*enable_sdio_irq)(struct mmci_host *host, int enable);+ void (*sdio_irq)(struct mmci_host *host, u32 status);};struct mmci_host {diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.cindex 60bca78a7..400e84a93 100644--- a/drivers/mmc/host/mmci_stm32_sdmmc.c+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c@@ -559,6 +559,25 @@ static int sdmmc_post_sig_volt_switch(struct mmci_host *host,return ret;}+static void sdmmc_enable_sdio_irq(struct mmci_host *host, int enable)+{+ void __iomem *base = host->base;+ u32 mask = readl_relaxed(base + MMCIMASK0);++ if (enable)+ writel_relaxed(mask | MCI_ST_SDIOITMASK, base + MMCIMASK0);+ else+ writel_relaxed(mask & ~MCI_ST_SDIOITMASK, base + MMCIMASK0);+}++static void sdmmc_sdio_irq(struct mmci_host *host, u32 status)+{+ if (status & MCI_ST_SDIOIT) {+ sdmmc_enable_sdio_irq(host, 0);+ sdio_signal_irq(host->mmc);+ }+}+static struct mmci_host_ops sdmmc_variant_ops = {.validate_data = sdmmc_idma_validate_data,.prep_data = sdmmc_idma_prep_data,@@ -572,6 +591,8 @@ static struct mmci_host_ops sdmmc_variant_ops = {.busy_complete = sdmmc_busy_complete,.pre_sig_volt_switch = sdmmc_pre_sig_volt_vswitch,.post_sig_volt_switch = sdmmc_post_sig_volt_switch,+ .enable_sdio_irq = sdmmc_enable_sdio_irq,+ .sdio_irq = sdmmc_sdio_irq,};void sdmmc_variant_init(struct mmci_host *host)

Gregory PLANCHON · Answer

Hello @Tom�? Ju?ena​ , First of all, thanks for your investigationwe are currently working on it, we will come back to you as soon as we have more information.  Regards,Grégory

Sign up

Login with SSO

Login to the community

Login with SSO

Scanning file for viruses.

This file cannot be downloaded