Hello!
This is implementation of SDHCI CQE support for sdhci-of-dwcmshc driver.
For enabling CQE support just set 'supports-cqe' in your DevTree file
for appropriate mmc node.
Also, while implementing CQE support for the driver, I faced with a problem
which I will describe below.
According to the IP block documentation CQE works only with "AMDA-2 only"
mode which is activated only with v4 mode enabled. I see in dwcmshc_probe()
function that v4 mode gets enabled only for 'sdhci_dwcmshc_bf3_pdata' platform data.
So my question is: is it correct to enable v4 mode for all platform data
if 'SDHCI_CAN_64BIT_V4' bit is set in hw?
Because I`m afraid that enabling v4 mode for some platforms could break
them down. On the other hand, if host controller says that it can do v4
(caps & SDHCI_CAN_64BIT_V4), lets do v4 or disable it manualy by some
quirk. Anyway - RFC.
v2:
- Added dwcmshc specific cqe_disable hook to prevent losing
in-flight cmd when an ioctl is issued and cqe_disable is called;
- Added processing 128Mb boundary for the host memory data buffer size
and the data buffer. For implementing this processing an extra
callback is added to the struct 'sdhci_ops'.
- Fixed typo.
v3:
- Fix warning reported by kernel test robot:
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Thanks in advance, Sergey.
Sergey Khimich (2):
mmc: cqhci: Add cqhci_set_tran_desc() callback
Implement SDHCI CQE support for DesignWare SDHCI.
drivers/mmc/host/Kconfig | 1 +
drivers/mmc/host/cqhci-core.c | 5 +
drivers/mmc/host/cqhci.h | 4 +
drivers/mmc/host/sdhci-of-dwcmshc.c | 233 +++++++++++++++++++++++++++-
4 files changed, 241 insertions(+), 2 deletions(-)
--
2.30.2
From: Sergey Khimich <[email protected]>
There are could be specific limitations for some mmc
controllers for preparing and setting cqhci transfer
descriptors. So add callback to allow implement
driver specific function.
Signed-off-by: Sergey Khimich <[email protected]>
---
drivers/mmc/host/cqhci-core.c | 5 +++++
drivers/mmc/host/cqhci.h | 4 ++++
2 files changed, 9 insertions(+)
diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
index b3d7d6d8d654..22be6b0b32ca 100644
--- a/drivers/mmc/host/cqhci-core.c
+++ b/drivers/mmc/host/cqhci-core.c
@@ -516,6 +516,11 @@ static int cqhci_prep_tran_desc(struct mmc_request *mrq,
desc = get_trans_desc(cq_host, tag);
+ if (cq_host->ops->prep_tran_desc) {
+ cq_host->ops->prep_tran_desc(data, cq_host, desc, sg_count);
+ return 0;
+ }
+
for_each_sg(data->sg, sg, sg_count, i) {
addr = sg_dma_address(sg);
len = sg_dma_len(sg);
diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index 1a12e40a02e6..868822292ce6 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -217,6 +217,7 @@ struct cqhci_host_ops;
struct mmc_host;
struct mmc_request;
struct cqhci_slot;
+struct mmc_data;
struct cqhci_host {
const struct cqhci_host_ops *ops;
@@ -293,6 +294,9 @@ struct cqhci_host_ops {
int (*program_key)(struct cqhci_host *cq_host,
const union cqhci_crypto_cfg_entry *cfg, int slot);
#endif
+ void (*prep_tran_desc)(struct mmc_data *data, struct cqhci_host *cq_host,
+ u8 *desc, int sg_count);
+
};
static inline void cqhci_writel(struct cqhci_host *host, u32 val, int reg)
--
2.30.2
From: Sergey Khimich <[email protected]>
For enabling CQE support just set 'supports-cqe' in your DevTree file
for appropriate mmc node.
Signed-off-by: Sergey Khimich <[email protected]>
---
drivers/mmc/host/Kconfig | 1 +
drivers/mmc/host/sdhci-of-dwcmshc.c | 233 +++++++++++++++++++++++++++-
2 files changed, 232 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 554e67103c1a..f3380b014ca9 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -233,6 +233,7 @@ config MMC_SDHCI_OF_DWCMSHC
depends on MMC_SDHCI_PLTFM
depends on OF
depends on COMMON_CLK
+ select MMC_CQHCI
help
This selects Synopsys DesignWare Cores Mobile Storage Controller
support.
diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 3a3bae6948a8..7d43ae011811 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -20,6 +20,7 @@
#include <linux/sizes.h>
#include "sdhci-pltfm.h"
+#include "cqhci.h"
#define SDHCI_DWCMSHC_ARG2_STUFF GENMASK(31, 16)
@@ -36,6 +37,9 @@
#define DWCMSHC_ENHANCED_STROBE BIT(8)
#define DWCMSHC_EMMC_ATCTRL 0x40
+/* DWC IP vendor area 2 pointer */
+#define DWCMSHC_P_VENDOR_AREA2 0xea
+
/* Rockchip specific Registers */
#define DWCMSHC_EMMC_DLL_CTRL 0x800
#define DWCMSHC_EMMC_DLL_RXCLK 0x804
@@ -75,6 +79,10 @@
#define BOUNDARY_OK(addr, len) \
((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
+#define DWCMSHC_SDHCI_CQE_TRNS_MODE (SDHCI_TRNS_MULTI | \
+ SDHCI_TRNS_BLK_CNT_EN | \
+ SDHCI_TRNS_DMA)
+
enum dwcmshc_rk_type {
DWCMSHC_RK3568,
DWCMSHC_RK3588,
@@ -90,7 +98,8 @@ struct rk35xx_priv {
struct dwcmshc_priv {
struct clk *bus_clk;
- int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA reg */
+ int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA1 reg */
+ int vendor_specific_area2; /* P_VENDOR_SPECIFIC_AREA2 reg */
void *priv; /* pointer to SoC private stuff */
};
@@ -210,6 +219,147 @@ static void dwcmshc_hs400_enhanced_strobe(struct mmc_host *mmc,
sdhci_writel(host, vendor, reg);
}
+static u32 dwcmshc_cqe_irq_handler(struct sdhci_host *host, u32 intmask)
+{
+ int cmd_error = 0;
+ int data_error = 0;
+
+ if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
+ return intmask;
+
+ cqhci_irq(host->mmc, intmask, cmd_error, data_error);
+
+ return 0;
+}
+
+static void dwcmshc_sdhci_cqe_enable(struct mmc_host *mmc)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ u32 pstate;
+ u8 ctrl;
+ int count = 10;
+
+ /*
+ * CQE gets stuck if it sees Buffer Read Enable bit set, which can be
+ * the case after tuning, so ensure the buffer is drained.
+ */
+ pstate = sdhci_readl(host, SDHCI_PRESENT_STATE);
+ while (pstate & SDHCI_DATA_AVAILABLE) {
+ sdhci_readl(host, SDHCI_BUFFER);
+ pstate = sdhci_readl(host, SDHCI_PRESENT_STATE);
+ if (count-- == 0) {
+ dev_warn(mmc_dev(host->mmc),
+ "CQE may get stuck because the Buffer Read Enable bit is set\n");
+ break;
+ }
+ mdelay(1);
+ }
+
+ sdhci_writew(host, DWCMSHC_SDHCI_CQE_TRNS_MODE, SDHCI_TRANSFER_MODE);
+
+ sdhci_cqe_enable(mmc);
+
+ /*
+ * The "DesignWare Cores Mobile Storage Host Controller
+ * DWC_mshc / DWC_mshc_lite Databook" says:
+ * when Host Version 4 Enable" is 1 in Host Control 2 register,
+ * SDHCI_CTRL_ADMA32 bit means ADMA2 is selected.
+ * Selection of 32-bit/64-bit System Addressing:
+ * either 32-bit or 64-bit system addressing is selected by
+ * 64-bit Addressing bit in Host Control 2 register.
+ *
+ * On the other hand the "DesignWare Cores Mobile Storage Host
+ * Controller DWC_mshc / DWC_mshc_lite User Guide" says, that we have to
+ * set DMA_SEL to ADMA2 _only_ mode in the Host Control 2 register.
+ */
+ ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+ ctrl &= ~SDHCI_CTRL_DMA_MASK;
+ ctrl |= SDHCI_CTRL_ADMA32;
+ sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+}
+
+static void dwcmshc_sdhci_cqe_disable(struct mmc_host *mmc, bool recovery)
+{
+ /*
+ * If an ioctl was issued, cqe_disable will be called.
+ * For CQE of sdhci-of-dwcmshc, the previous in-flight cmd will be lost quietly.
+ * So wait for mmc idle state.
+ */
+ mmc->cqe_ops->cqe_wait_for_idle(mmc);
+
+ return sdhci_cqe_disable(mmc, recovery);
+}
+
+static void dwcmshc_cqhci_set_tran_desc(u8 *desc, dma_addr_t addr, int len, bool end,
+ bool dma64)
+{
+ __le32 *attr = (__le32 __force *)desc;
+
+ *attr = (CQHCI_VALID(1) |
+ CQHCI_END(end ? 1 : 0) |
+ CQHCI_INT(0) |
+ CQHCI_ACT(0x4) |
+ CQHCI_DAT_LENGTH(len));
+
+ if (dma64) {
+ __le64 *dataddr = (__le64 __force *)(desc + 4);
+
+ dataddr[0] = cpu_to_le64(addr);
+ } else {
+ __le32 *dataddr = (__le32 __force *)(desc + 4);
+
+ dataddr[0] = cpu_to_le32(addr);
+ }
+}
+
+static void dwcmshc_cqhci_prep_tran_desc(struct mmc_data *data,
+ struct cqhci_host *cq_host,
+ u8 *desc, int sg_count)
+{
+ int i, len, tmplen, offset;
+ bool end = false;
+ bool dma64 = cq_host->dma64;
+ dma_addr_t addr;
+ struct scatterlist *sg;
+
+ for_each_sg(data->sg, sg, sg_count, i) {
+ addr = sg_dma_address(sg);
+ len = sg_dma_len(sg);
+
+ /*
+ * According to the "DesignWare Cores Mobile Storage Host Controller
+ * DWC_mshc / DWC_mshc_lite Databook" the host memory data buffer size
+ * and start address must not exceed 128 Mb. If it exceeds,
+ * the data buffer must be split using two descritors.
+ */
+
+ if (likely(BOUNDARY_OK(addr, len))) {
+ if ((i + 1) == sg_count)
+ end = true;
+ dwcmshc_cqhci_set_tran_desc(desc, addr, len, end, dma64);
+ desc += cq_host->trans_desc_len;
+ } else {
+ offset = addr & (SZ_128M - 1);
+ tmplen = SZ_128M - offset;
+ dwcmshc_cqhci_set_tran_desc(desc, addr, tmplen, end, dma64);
+ desc += cq_host->trans_desc_len;
+
+ if ((i + 1) == sg_count)
+ end = true;
+
+ addr += tmplen;
+ len -= tmplen;
+ dwcmshc_cqhci_set_tran_desc(desc, addr, len, end, dma64);
+ desc += cq_host->trans_desc_len;
+ }
+ }
+}
+
+static void dwcmshc_cqhci_dumpregs(struct mmc_host *mmc)
+{
+ sdhci_dumpregs(mmc_priv(mmc));
+}
+
static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -345,6 +495,7 @@ static const struct sdhci_ops sdhci_dwcmshc_ops = {
.get_max_clock = dwcmshc_get_max_clock,
.reset = sdhci_reset,
.adma_write_desc = dwcmshc_adma_write_desc,
+ .irq = dwcmshc_cqe_irq_handler,
};
static const struct sdhci_ops sdhci_dwcmshc_rk35xx_ops = {
@@ -379,6 +530,70 @@ static const struct sdhci_pltfm_data sdhci_dwcmshc_rk35xx_pdata = {
SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
};
+static const struct cqhci_host_ops dwcmshc_cqhci_ops = {
+ .enable = dwcmshc_sdhci_cqe_enable,
+ .disable = dwcmshc_sdhci_cqe_disable,
+ .dumpregs = dwcmshc_cqhci_dumpregs,
+ .prep_tran_desc = dwcmshc_cqhci_prep_tran_desc,
+};
+
+static void dwcmshc_cqhci_init(struct sdhci_host *host, struct platform_device *pdev)
+{
+ struct cqhci_host *cq_host;
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
+ bool dma64 = false;
+ u16 clk;
+ int err;
+
+ host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD;
+ cq_host = devm_kzalloc(&pdev->dev, sizeof(*cq_host), GFP_KERNEL);
+ if (!cq_host) {
+ dev_err(mmc_dev(host->mmc), "Unable to setup CQE: not enough memory\n");
+ return;
+ }
+
+ /*
+ * For dwcmshc host controller we have to enable internal clock
+ * before access to some registers from Vendor Specific Aria 2.
+ */
+ clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+ clk |= SDHCI_CLOCK_INT_EN;
+ sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+ clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+ if (!(clk & SDHCI_CLOCK_INT_EN)) {
+ dev_err(mmc_dev(host->mmc), "Unable to setup CQE: internal clock enable error\n");
+ goto free_cq_host;
+ }
+
+ cq_host->mmio = host->ioaddr + priv->vendor_specific_area2;
+ cq_host->ops = &dwcmshc_cqhci_ops;
+
+ /* Enable using of 128-bit task descriptors */
+ dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
+ if (dma64) {
+ dev_dbg(mmc_dev(host->mmc), "128-bit task descriptors\n");
+ cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
+ }
+ err = cqhci_init(cq_host, host->mmc, dma64);
+ if (err) {
+ dev_err(mmc_dev(host->mmc), "Unable to setup CQE: error %d\n", err);
+ goto int_clok_disable;
+ }
+
+ dev_dbg(mmc_dev(host->mmc), "CQE init done\n");
+
+ return;
+
+int_clok_disable:
+ clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+ clk &= ~SDHCI_CLOCK_INT_EN;
+ sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+
+free_cq_host:
+ devm_kfree(&pdev->dev, cq_host);
+}
+
static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
{
int err;
@@ -471,7 +686,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
struct rk35xx_priv *rk_priv = NULL;
const struct sdhci_pltfm_data *pltfm_data;
int err;
- u32 extra;
+ u32 extra, caps;
pltfm_data = device_get_match_data(&pdev->dev);
if (!pltfm_data) {
@@ -519,6 +734,8 @@ static int dwcmshc_probe(struct platform_device *pdev)
priv->vendor_specific_area1 =
sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK;
+ priv->vendor_specific_area2 =
+ sdhci_readw(host, DWCMSHC_P_VENDOR_AREA2);
host->mmc_host_ops.request = dwcmshc_request;
host->mmc_host_ops.hs400_enhanced_strobe = dwcmshc_hs400_enhanced_strobe;
@@ -547,6 +764,10 @@ static int dwcmshc_probe(struct platform_device *pdev)
sdhci_enable_v4_mode(host);
#endif
+ caps = sdhci_readl(host, SDHCI_CAPABILITIES);
+ if (caps & SDHCI_CAN_64BIT_V4)
+ sdhci_enable_v4_mode(host);
+
host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
pm_runtime_get_noresume(dev);
@@ -557,6 +778,14 @@ static int dwcmshc_probe(struct platform_device *pdev)
if (err)
goto err_rpm;
+ /* Setup Command Queue Engine if enabled */
+ if (device_property_read_bool(&pdev->dev, "supports-cqe")) {
+ if (caps & SDHCI_CAN_64BIT_V4)
+ dwcmshc_cqhci_init(host, pdev);
+ else
+ dev_warn(dev, "Cannot enable CQE without V4 mode support\n");
+ }
+
if (rk_priv)
dwcmshc_rk35xx_postinit(host, priv);
--
2.30.2
On 2/10/23 14:33, Sergey Khimich wrote:
> From: Sergey Khimich <[email protected]>
>
> For enabling CQE support just set 'supports-cqe' in your DevTree file
> for appropriate mmc node.
>
> Signed-off-by: Sergey Khimich <[email protected]>
> ---
> drivers/mmc/host/Kconfig | 1 +
> drivers/mmc/host/sdhci-of-dwcmshc.c | 233 +++++++++++++++++++++++++++-
> 2 files changed, 232 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 554e67103c1a..f3380b014ca9 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -233,6 +233,7 @@ config MMC_SDHCI_OF_DWCMSHC
> depends on MMC_SDHCI_PLTFM
> depends on OF
> depends on COMMON_CLK
> + select MMC_CQHCI
> help
> This selects Synopsys DesignWare Cores Mobile Storage Controller
> support.
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 3a3bae6948a8..7d43ae011811 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -20,6 +20,7 @@
> #include <linux/sizes.h>
>
> #include "sdhci-pltfm.h"
> +#include "cqhci.h"
>
> #define SDHCI_DWCMSHC_ARG2_STUFF GENMASK(31, 16)
>
> @@ -36,6 +37,9 @@
> #define DWCMSHC_ENHANCED_STROBE BIT(8)
> #define DWCMSHC_EMMC_ATCTRL 0x40
>
> +/* DWC IP vendor area 2 pointer */
> +#define DWCMSHC_P_VENDOR_AREA2 0xea
> +
> /* Rockchip specific Registers */
> #define DWCMSHC_EMMC_DLL_CTRL 0x800
> #define DWCMSHC_EMMC_DLL_RXCLK 0x804
> @@ -75,6 +79,10 @@
> #define BOUNDARY_OK(addr, len) \
> ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
>
> +#define DWCMSHC_SDHCI_CQE_TRNS_MODE (SDHCI_TRNS_MULTI | \
> + SDHCI_TRNS_BLK_CNT_EN | \
> + SDHCI_TRNS_DMA)
> +
> enum dwcmshc_rk_type {
> DWCMSHC_RK3568,
> DWCMSHC_RK3588,
> @@ -90,7 +98,8 @@ struct rk35xx_priv {
>
> struct dwcmshc_priv {
> struct clk *bus_clk;
> - int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA reg */
> + int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA1 reg */
> + int vendor_specific_area2; /* P_VENDOR_SPECIFIC_AREA2 reg */
> void *priv; /* pointer to SoC private stuff */
> };
>
> @@ -210,6 +219,147 @@ static void dwcmshc_hs400_enhanced_strobe(struct mmc_host *mmc,
> sdhci_writel(host, vendor, reg);
> }
>
> +static u32 dwcmshc_cqe_irq_handler(struct sdhci_host *host, u32 intmask)
> +{
> + int cmd_error = 0;
> + int data_error = 0;
> +
> + if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
> + return intmask;
> +
> + cqhci_irq(host->mmc, intmask, cmd_error, data_error);
> +
> + return 0;
> +}
> +
> +static void dwcmshc_sdhci_cqe_enable(struct mmc_host *mmc)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + u32 pstate;
> + u8 ctrl;
> + int count = 10;
> +
> + /*
> + * CQE gets stuck if it sees Buffer Read Enable bit set, which can be
> + * the case after tuning, so ensure the buffer is drained.
> + */
> + pstate = sdhci_readl(host, SDHCI_PRESENT_STATE);
> + while (pstate & SDHCI_DATA_AVAILABLE) {
> + sdhci_readl(host, SDHCI_BUFFER);
> + pstate = sdhci_readl(host, SDHCI_PRESENT_STATE);
> + if (count-- == 0) {
> + dev_warn(mmc_dev(host->mmc),
> + "CQE may get stuck because the Buffer Read Enable bit is set\n");
> + break;
> + }
> + mdelay(1);
> + }
An alternative, which might be easier, is to do a
data reset which may also help allow the device to
subsequently enter low power states.
Refer commit f8870ae6e2d6be75b1accc2db981169fdfbea7ab
and commit 7b7d57fd1b773d25d8358c6017592b4928bf76ce
> +
> + sdhci_writew(host, DWCMSHC_SDHCI_CQE_TRNS_MODE, SDHCI_TRANSFER_MODE);
> +
> + sdhci_cqe_enable(mmc);
> +
> + /*
> + * The "DesignWare Cores Mobile Storage Host Controller
> + * DWC_mshc / DWC_mshc_lite Databook" says:
> + * when Host Version 4 Enable" is 1 in Host Control 2 register,
> + * SDHCI_CTRL_ADMA32 bit means ADMA2 is selected.
> + * Selection of 32-bit/64-bit System Addressing:
> + * either 32-bit or 64-bit system addressing is selected by
> + * 64-bit Addressing bit in Host Control 2 register.
> + *
> + * On the other hand the "DesignWare Cores Mobile Storage Host
> + * Controller DWC_mshc / DWC_mshc_lite User Guide" says, that we have to
> + * set DMA_SEL to ADMA2 _only_ mode in the Host Control 2 register.
> + */
> + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> + ctrl &= ~SDHCI_CTRL_DMA_MASK;
> + ctrl |= SDHCI_CTRL_ADMA32;
> + sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> +}
> +
> +static void dwcmshc_sdhci_cqe_disable(struct mmc_host *mmc, bool recovery)
> +{
> + /*
> + * If an ioctl was issued, cqe_disable will be called.
> + * For CQE of sdhci-of-dwcmshc, the previous in-flight cmd will be lost quietly.
> + * So wait for mmc idle state.
This sounds like it should be fixed in the mmc block driver.
Can you provide an example of when this happens?
> + */
> + mmc->cqe_ops->cqe_wait_for_idle(mmc);
> +
> + return sdhci_cqe_disable(mmc, recovery);
> +}
> +
> +static void dwcmshc_cqhci_set_tran_desc(u8 *desc, dma_addr_t addr, int len, bool end,
> + bool dma64)
> +{
> + __le32 *attr = (__le32 __force *)desc;
> +
> + *attr = (CQHCI_VALID(1) |
> + CQHCI_END(end ? 1 : 0) |
> + CQHCI_INT(0) |
> + CQHCI_ACT(0x4) |
> + CQHCI_DAT_LENGTH(len));
> +
> + if (dma64) {
> + __le64 *dataddr = (__le64 __force *)(desc + 4);
> +
> + dataddr[0] = cpu_to_le64(addr);
> + } else {
> + __le32 *dataddr = (__le32 __force *)(desc + 4);
> +
> + dataddr[0] = cpu_to_le32(addr);
> + }
> +}
This is the same as cqhci_set_tran_desc(). Might as well export that
instead.
> +
> +static void dwcmshc_cqhci_prep_tran_desc(struct mmc_data *data,
> + struct cqhci_host *cq_host,
> + u8 *desc, int sg_count)
> +{
> + int i, len, tmplen, offset;
> + bool end = false;
> + bool dma64 = cq_host->dma64;
> + dma_addr_t addr;
> + struct scatterlist *sg;
> +
> + for_each_sg(data->sg, sg, sg_count, i) {
> + addr = sg_dma_address(sg);
> + len = sg_dma_len(sg);
> +
> + /*
> + * According to the "DesignWare Cores Mobile Storage Host Controller
> + * DWC_mshc / DWC_mshc_lite Databook" the host memory data buffer size
> + * and start address must not exceed 128 Mb. If it exceeds,
> + * the data buffer must be split using two descritors.
> + */
> +
> + if (likely(BOUNDARY_OK(addr, len))) {
> + if ((i + 1) == sg_count)
> + end = true;
> + dwcmshc_cqhci_set_tran_desc(desc, addr, len, end, dma64);
> + desc += cq_host->trans_desc_len;
> + } else {
> + offset = addr & (SZ_128M - 1);
> + tmplen = SZ_128M - offset;
> + dwcmshc_cqhci_set_tran_desc(desc, addr, tmplen, end, dma64);
> + desc += cq_host->trans_desc_len;
> +
> + if ((i + 1) == sg_count)
> + end = true;
> +
> + addr += tmplen;
> + len -= tmplen;
> + dwcmshc_cqhci_set_tran_desc(desc, addr, len, end, dma64);
> + desc += cq_host->trans_desc_len;
> + }
> + }
> +}
Could this be done more like dwcmshc_adma_write_desc()
> +
> +static void dwcmshc_cqhci_dumpregs(struct mmc_host *mmc)
> +{
> + sdhci_dumpregs(mmc_priv(mmc));
> +}
> +
> static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -345,6 +495,7 @@ static const struct sdhci_ops sdhci_dwcmshc_ops = {
> .get_max_clock = dwcmshc_get_max_clock,
> .reset = sdhci_reset,
> .adma_write_desc = dwcmshc_adma_write_desc,
> + .irq = dwcmshc_cqe_irq_handler,
> };
>
> static const struct sdhci_ops sdhci_dwcmshc_rk35xx_ops = {
> @@ -379,6 +530,70 @@ static const struct sdhci_pltfm_data sdhci_dwcmshc_rk35xx_pdata = {
> SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
> };
>
> +static const struct cqhci_host_ops dwcmshc_cqhci_ops = {
> + .enable = dwcmshc_sdhci_cqe_enable,
> + .disable = dwcmshc_sdhci_cqe_disable,
> + .dumpregs = dwcmshc_cqhci_dumpregs,
> + .prep_tran_desc = dwcmshc_cqhci_prep_tran_desc,
> +};
> +
> +static void dwcmshc_cqhci_init(struct sdhci_host *host, struct platform_device *pdev)
> +{
> + struct cqhci_host *cq_host;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + bool dma64 = false;
> + u16 clk;
> + int err;
> +
> + host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD;
> + cq_host = devm_kzalloc(&pdev->dev, sizeof(*cq_host), GFP_KERNEL);
> + if (!cq_host) {
> + dev_err(mmc_dev(host->mmc), "Unable to setup CQE: not enough memory\n");
> + return;
> + }
> +
> + /*
> + * For dwcmshc host controller we have to enable internal clock
> + * before access to some registers from Vendor Specific Aria 2.
> + */
> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> + clk |= SDHCI_CLOCK_INT_EN;
> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> + if (!(clk & SDHCI_CLOCK_INT_EN)) {
> + dev_err(mmc_dev(host->mmc), "Unable to setup CQE: internal clock enable error\n");
> + goto free_cq_host;
> + }
> +
> + cq_host->mmio = host->ioaddr + priv->vendor_specific_area2;
> + cq_host->ops = &dwcmshc_cqhci_ops;
> +
> + /* Enable using of 128-bit task descriptors */
> + dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
> + if (dma64) {
> + dev_dbg(mmc_dev(host->mmc), "128-bit task descriptors\n");
> + cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
> + }
> + err = cqhci_init(cq_host, host->mmc, dma64);
> + if (err) {
> + dev_err(mmc_dev(host->mmc), "Unable to setup CQE: error %d\n", err);
> + goto int_clok_disable;
> + }
> +
> + dev_dbg(mmc_dev(host->mmc), "CQE init done\n");
> +
> + return;
> +
> +int_clok_disable:
'clok' is an odd abbreviation of 'clock'. Perhaps 'clk' or just 'clock'
> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> + clk &= ~SDHCI_CLOCK_INT_EN;
> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +
> +free_cq_host:
> + devm_kfree(&pdev->dev, cq_host);
> +}
> +
> static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
> {
> int err;
> @@ -471,7 +686,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
> struct rk35xx_priv *rk_priv = NULL;
> const struct sdhci_pltfm_data *pltfm_data;
> int err;
> - u32 extra;
> + u32 extra, caps;
>
> pltfm_data = device_get_match_data(&pdev->dev);
> if (!pltfm_data) {
> @@ -519,6 +734,8 @@ static int dwcmshc_probe(struct platform_device *pdev)
>
> priv->vendor_specific_area1 =
> sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK;
> + priv->vendor_specific_area2 =
> + sdhci_readw(host, DWCMSHC_P_VENDOR_AREA2);
>
> host->mmc_host_ops.request = dwcmshc_request;
> host->mmc_host_ops.hs400_enhanced_strobe = dwcmshc_hs400_enhanced_strobe;
> @@ -547,6 +764,10 @@ static int dwcmshc_probe(struct platform_device *pdev)
> sdhci_enable_v4_mode(host);
> #endif
>
> + caps = sdhci_readl(host, SDHCI_CAPABILITIES);
> + if (caps & SDHCI_CAN_64BIT_V4)
> + sdhci_enable_v4_mode(host);
> +
> host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
>
> pm_runtime_get_noresume(dev);
> @@ -557,6 +778,14 @@ static int dwcmshc_probe(struct platform_device *pdev)
> if (err)
> goto err_rpm;
>
> + /* Setup Command Queue Engine if enabled */
> + if (device_property_read_bool(&pdev->dev, "supports-cqe")) {
> + if (caps & SDHCI_CAN_64BIT_V4)
> + dwcmshc_cqhci_init(host, pdev);
> + else
> + dev_warn(dev, "Cannot enable CQE without V4 mode support\n");
> + }
> +
> if (rk_priv)
> dwcmshc_rk35xx_postinit(host, priv);
>
Hello Adrian!
Thanks for review and comments! There are few questions I`d like to clarify:
On 09.10.2023 18:39, Adrian Hunter wrote:
> On 2/10/23 14:33, Sergey Khimich wrote:
>> From: Sergey Khimich <[email protected]>
>>
>> For enabling CQE support just set 'supports-cqe' in your DevTree file
>> for appropriate mmc node.
>>
>> Signed-off-by: Sergey Khimich <[email protected]>
>> ---
>> drivers/mmc/host/Kconfig | 1 +
>> drivers/mmc/host/sdhci-of-dwcmshc.c | 233 +++++++++++++++++++++++++++-
>> 2 files changed, 232 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 554e67103c1a..f3380b014ca9 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -233,6 +233,7 @@ config MMC_SDHCI_OF_DWCMSHC
>> depends on MMC_SDHCI_PLTFM
>> depends on OF
>> depends on COMMON_CLK
>> + select MMC_CQHCI
>> help
>> This selects Synopsys DesignWare Cores Mobile Storage Controller
>> support.
>> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
>> index 3a3bae6948a8..7d43ae011811 100644
>> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
>> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
>> @@ -20,6 +20,7 @@
>> #include <linux/sizes.h>
>>
>> #include "sdhci-pltfm.h"
>> +#include "cqhci.h"
>>
>> #define SDHCI_DWCMSHC_ARG2_STUFF GENMASK(31, 16)
>>
>> @@ -36,6 +37,9 @@
>> #define DWCMSHC_ENHANCED_STROBE BIT(8)
>> #define DWCMSHC_EMMC_ATCTRL 0x40
>>
>> +/* DWC IP vendor area 2 pointer */
>> +#define DWCMSHC_P_VENDOR_AREA2 0xea
>> +
>> /* Rockchip specific Registers */
>> #define DWCMSHC_EMMC_DLL_CTRL 0x800
>> #define DWCMSHC_EMMC_DLL_RXCLK 0x804
>> @@ -75,6 +79,10 @@
>> #define BOUNDARY_OK(addr, len) \
>> ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
>>
>> +#define DWCMSHC_SDHCI_CQE_TRNS_MODE (SDHCI_TRNS_MULTI | \
>> + SDHCI_TRNS_BLK_CNT_EN | \
>> + SDHCI_TRNS_DMA)
>> +
>> enum dwcmshc_rk_type {
>> DWCMSHC_RK3568,
>> DWCMSHC_RK3588,
>> @@ -90,7 +98,8 @@ struct rk35xx_priv {
>>
>> struct dwcmshc_priv {
>> struct clk *bus_clk;
>> - int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA reg */
>> + int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA1 reg */
>> + int vendor_specific_area2; /* P_VENDOR_SPECIFIC_AREA2 reg */
>> void *priv; /* pointer to SoC private stuff */
>> };
>>
>> @@ -210,6 +219,147 @@ static void dwcmshc_hs400_enhanced_strobe(struct mmc_host *mmc,
>> sdhci_writel(host, vendor, reg);
>> }
>>
>> +static u32 dwcmshc_cqe_irq_handler(struct sdhci_host *host, u32 intmask)
>> +{
>> + int cmd_error = 0;
>> + int data_error = 0;
>> +
>> + if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
>> + return intmask;
>> +
>> + cqhci_irq(host->mmc, intmask, cmd_error, data_error);
>> +
>> + return 0;
>> +}
>> +
>> +static void dwcmshc_sdhci_cqe_enable(struct mmc_host *mmc)
>> +{
>> + struct sdhci_host *host = mmc_priv(mmc);
>> + u32 pstate;
>> + u8 ctrl;
>> + int count = 10;
>> +
>> + /*
>> + * CQE gets stuck if it sees Buffer Read Enable bit set, which can be
>> + * the case after tuning, so ensure the buffer is drained.
>> + */
>> + pstate = sdhci_readl(host, SDHCI_PRESENT_STATE);
>> + while (pstate & SDHCI_DATA_AVAILABLE) {
>> + sdhci_readl(host, SDHCI_BUFFER);
>> + pstate = sdhci_readl(host, SDHCI_PRESENT_STATE);
>> + if (count-- == 0) {
>> + dev_warn(mmc_dev(host->mmc),
>> + "CQE may get stuck because the Buffer Read Enable bit is set\n");
>> + break;
>> + }
>> + mdelay(1);
>> + }
> An alternative, which might be easier, is to do a
> data reset which may also help allow the device to
> subsequently enter low power states.
> Refer commit f8870ae6e2d6be75b1accc2db981169fdfbea7ab
> and commit 7b7d57fd1b773d25d8358c6017592b4928bf76ce
Thanks, I'll fix it in the next version of the patch.
>
>> +
>> + sdhci_writew(host, DWCMSHC_SDHCI_CQE_TRNS_MODE, SDHCI_TRANSFER_MODE);
>> +
>> + sdhci_cqe_enable(mmc);
>> +
>> + /*
>> + * The "DesignWare Cores Mobile Storage Host Controller
>> + * DWC_mshc / DWC_mshc_lite Databook" says:
>> + * when Host Version 4 Enable" is 1 in Host Control 2 register,
>> + * SDHCI_CTRL_ADMA32 bit means ADMA2 is selected.
>> + * Selection of 32-bit/64-bit System Addressing:
>> + * either 32-bit or 64-bit system addressing is selected by
>> + * 64-bit Addressing bit in Host Control 2 register.
>> + *
>> + * On the other hand the "DesignWare Cores Mobile Storage Host
>> + * Controller DWC_mshc / DWC_mshc_lite User Guide" says, that we have to
>> + * set DMA_SEL to ADMA2 _only_ mode in the Host Control 2 register.
>> + */
>> + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
>> + ctrl &= ~SDHCI_CTRL_DMA_MASK;
>> + ctrl |= SDHCI_CTRL_ADMA32;
>> + sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>> +}
>> +
>> +static void dwcmshc_sdhci_cqe_disable(struct mmc_host *mmc, bool recovery)
>> +{
>> + /*
>> + * If an ioctl was issued, cqe_disable will be called.
>> + * For CQE of sdhci-of-dwcmshc, the previous in-flight cmd will be lost quietly.
>> + * So wait for mmc idle state.
> This sounds like it should be fixed in the mmc block driver.
> Can you provide an example of when this happens?
Unfortunately I can't provide an example.
But this part of the patch is updated by me on the advice of Shawn Lin
after his reviewing V1 of the patch.
Please find his message here:
https://patchwork.kernel.org/project/linux-mmc/patch/[email protected]/
Just in case here I also quote the part of his comment that refers to
this part of the patch:
"And another issue was found when Rockchip added CQE support for
sdhci-of-dwcmshc internally, is that if a ioctl was issued, cqe_disable
will be called. For CQE of sdhci-of-dwcmshc, the previous in-flight cmd
will be lost quietly. So a mmc->cqe_ops->cqe_wait_for_idle(mmc) should
be added before sdhci_cqe_disable(), so you need a dwcmshc specified
cqe_disable hook in sdhci-of-dwcmshc."
>
>> + */
>> + mmc->cqe_ops->cqe_wait_for_idle(mmc);
>> +
>> + return sdhci_cqe_disable(mmc, recovery);
>> +}
>> +
>> +static void dwcmshc_cqhci_set_tran_desc(u8 *desc, dma_addr_t addr, int len, bool end,
>> + bool dma64)
>> +{
>> + __le32 *attr = (__le32 __force *)desc;
>> +
>> + *attr = (CQHCI_VALID(1) |
>> + CQHCI_END(end ? 1 : 0) |
>> + CQHCI_INT(0) |
>> + CQHCI_ACT(0x4) |
>> + CQHCI_DAT_LENGTH(len));
>> +
>> + if (dma64) {
>> + __le64 *dataddr = (__le64 __force *)(desc + 4);
>> +
>> + dataddr[0] = cpu_to_le64(addr);
>> + } else {
>> + __le32 *dataddr = (__le32 __force *)(desc + 4);
>> +
>> + dataddr[0] = cpu_to_le32(addr);
>> + }
>> +}
> This is the same as cqhci_set_tran_desc(). Might as well export that
> instead.
Thanks, I'll fix it in the next version of the patch.
>> +
>> +static void dwcmshc_cqhci_prep_tran_desc(struct mmc_data *data,
>> + struct cqhci_host *cq_host,
>> + u8 *desc, int sg_count)
>> +{
>> + int i, len, tmplen, offset;
>> + bool end = false;
>> + bool dma64 = cq_host->dma64;
>> + dma_addr_t addr;
>> + struct scatterlist *sg;
>> +
>> + for_each_sg(data->sg, sg, sg_count, i) {
>> + addr = sg_dma_address(sg);
>> + len = sg_dma_len(sg);
>> +
>> + /*
>> + * According to the "DesignWare Cores Mobile Storage Host Controller
>> + * DWC_mshc / DWC_mshc_lite Databook" the host memory data buffer size
>> + * and start address must not exceed 128 Mb. If it exceeds,
>> + * the data buffer must be split using two descritors.
>> + */
>> +
>> + if (likely(BOUNDARY_OK(addr, len))) {
>> + if ((i + 1) == sg_count)
>> + end = true;
>> + dwcmshc_cqhci_set_tran_desc(desc, addr, len, end, dma64);
>> + desc += cq_host->trans_desc_len;
>> + } else {
>> + offset = addr & (SZ_128M - 1);
>> + tmplen = SZ_128M - offset;
>> + dwcmshc_cqhci_set_tran_desc(desc, addr, tmplen, end, dma64);
>> + desc += cq_host->trans_desc_len;
>> +
>> + if ((i + 1) == sg_count)
>> + end = true;
>> +
>> + addr += tmplen;
>> + len -= tmplen;
>> + dwcmshc_cqhci_set_tran_desc(desc, addr, len, end, dma64);
>> + desc += cq_host->trans_desc_len;
>> + }
>> + }
>> +}
> Could this be done more like dwcmshc_adma_write_desc()
Actually I'm not sure what do you mean. I reused checking boundary
construction with
"BOUNDARY_OK" macro and used the same variable names. I would appreciate
it if you could clarify
what does mean "more like dwcmshc_adma_write_desc()"
>
>> +
>> +static void dwcmshc_cqhci_dumpregs(struct mmc_host *mmc)
>> +{
>> + sdhci_dumpregs(mmc_priv(mmc));
>> +}
>> +
>> static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock)
>> {
>> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> @@ -345,6 +495,7 @@ static const struct sdhci_ops sdhci_dwcmshc_ops = {
>> .get_max_clock = dwcmshc_get_max_clock,
>> .reset = sdhci_reset,
>> .adma_write_desc = dwcmshc_adma_write_desc,
>> + .irq = dwcmshc_cqe_irq_handler,
>> };
>>
>> static const struct sdhci_ops sdhci_dwcmshc_rk35xx_ops = {
>> @@ -379,6 +530,70 @@ static const struct sdhci_pltfm_data sdhci_dwcmshc_rk35xx_pdata = {
>> SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
>> };
>>
>> +static const struct cqhci_host_ops dwcmshc_cqhci_ops = {
>> + .enable = dwcmshc_sdhci_cqe_enable,
>> + .disable = dwcmshc_sdhci_cqe_disable,
>> + .dumpregs = dwcmshc_cqhci_dumpregs,
>> + .prep_tran_desc = dwcmshc_cqhci_prep_tran_desc,
>> +};
>> +
>> +static void dwcmshc_cqhci_init(struct sdhci_host *host, struct platform_device *pdev)
>> +{
>> + struct cqhci_host *cq_host;
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + bool dma64 = false;
>> + u16 clk;
>> + int err;
>> +
>> + host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD;
>> + cq_host = devm_kzalloc(&pdev->dev, sizeof(*cq_host), GFP_KERNEL);
>> + if (!cq_host) {
>> + dev_err(mmc_dev(host->mmc), "Unable to setup CQE: not enough memory\n");
>> + return;
>> + }
>> +
>> + /*
>> + * For dwcmshc host controller we have to enable internal clock
>> + * before access to some registers from Vendor Specific Aria 2.
>> + */
>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>> + clk |= SDHCI_CLOCK_INT_EN;
>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>> + if (!(clk & SDHCI_CLOCK_INT_EN)) {
>> + dev_err(mmc_dev(host->mmc), "Unable to setup CQE: internal clock enable error\n");
>> + goto free_cq_host;
>> + }
>> +
>> + cq_host->mmio = host->ioaddr + priv->vendor_specific_area2;
>> + cq_host->ops = &dwcmshc_cqhci_ops;
>> +
>> + /* Enable using of 128-bit task descriptors */
>> + dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
>> + if (dma64) {
>> + dev_dbg(mmc_dev(host->mmc), "128-bit task descriptors\n");
>> + cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
>> + }
>> + err = cqhci_init(cq_host, host->mmc, dma64);
>> + if (err) {
>> + dev_err(mmc_dev(host->mmc), "Unable to setup CQE: error %d\n", err);
>> + goto int_clok_disable;
>> + }
>> +
>> + dev_dbg(mmc_dev(host->mmc), "CQE init done\n");
>> +
>> + return;
>> +
>> +int_clok_disable:
> 'clok' is an odd abbreviation of 'clock'. Perhaps 'clk' or just 'clock'
Thanks, I'll fix it in the next version of the patch.
>
>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>> + clk &= ~SDHCI_CLOCK_INT_EN;
>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>> +
>> +free_cq_host:
>> + devm_kfree(&pdev->dev, cq_host);
>> +}
>> +
>> static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
>> {
>> int err;
>> @@ -471,7 +686,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>> struct rk35xx_priv *rk_priv = NULL;
>> const struct sdhci_pltfm_data *pltfm_data;
>> int err;
>> - u32 extra;
>> + u32 extra, caps;
>>
>> pltfm_data = device_get_match_data(&pdev->dev);
>> if (!pltfm_data) {
>> @@ -519,6 +734,8 @@ static int dwcmshc_probe(struct platform_device *pdev)
>>
>> priv->vendor_specific_area1 =
>> sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK;
>> + priv->vendor_specific_area2 =
>> + sdhci_readw(host, DWCMSHC_P_VENDOR_AREA2);
>>
>> host->mmc_host_ops.request = dwcmshc_request;
>> host->mmc_host_ops.hs400_enhanced_strobe = dwcmshc_hs400_enhanced_strobe;
>> @@ -547,6 +764,10 @@ static int dwcmshc_probe(struct platform_device *pdev)
>> sdhci_enable_v4_mode(host);
>> #endif
>>
>> + caps = sdhci_readl(host, SDHCI_CAPABILITIES);
>> + if (caps & SDHCI_CAN_64BIT_V4)
>> + sdhci_enable_v4_mode(host);
>> +
>> host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
>>
>> pm_runtime_get_noresume(dev);
>> @@ -557,6 +778,14 @@ static int dwcmshc_probe(struct platform_device *pdev)
>> if (err)
>> goto err_rpm;
>>
>> + /* Setup Command Queue Engine if enabled */
>> + if (device_property_read_bool(&pdev->dev, "supports-cqe")) {
>> + if (caps & SDHCI_CAN_64BIT_V4)
>> + dwcmshc_cqhci_init(host, pdev);
>> + else
>> + dev_warn(dev, "Cannot enable CQE without V4 mode support\n");
>> + }
>> +
>> if (rk_priv)
>> dwcmshc_rk35xx_postinit(host, priv);
>>
On 16/10/23 14:42, Sergey Khimich wrote:
> Hello Adrian!
>
> Thanks for review and comments! There are few questions I`d like to clarify:
>
> On 09.10.2023 18:39, Adrian Hunter wrote:
>> On 2/10/23 14:33, Sergey Khimich wrote:
>>> From: Sergey Khimich <[email protected]>
>>>
>>> For enabling CQE support just set 'supports-cqe' in your DevTree file
>>> for appropriate mmc node.
>>>
>>> Signed-off-by: Sergey Khimich <[email protected]>
>>> ---
>>> drivers/mmc/host/Kconfig | 1 +
>>> drivers/mmc/host/sdhci-of-dwcmshc.c | 233 +++++++++++++++++++++++++++-
>>> 2 files changed, 232 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>> index 554e67103c1a..f3380b014ca9 100644
>>> --- a/drivers/mmc/host/Kconfig
>>> +++ b/drivers/mmc/host/Kconfig
>>> @@ -233,6 +233,7 @@ config MMC_SDHCI_OF_DWCMSHC
>>> depends on MMC_SDHCI_PLTFM
>>> depends on OF
>>> depends on COMMON_CLK
>>> + select MMC_CQHCI
>>> help
>>> This selects Synopsys DesignWare Cores Mobile Storage Controller
>>> support.
>>> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
>>> index 3a3bae6948a8..7d43ae011811 100644
>>> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
>>> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
>>> @@ -20,6 +20,7 @@
>>> #include <linux/sizes.h>
>>> #include "sdhci-pltfm.h"
>>> +#include "cqhci.h"
>>> #define SDHCI_DWCMSHC_ARG2_STUFF GENMASK(31, 16)
>>> @@ -36,6 +37,9 @@
>>> #define DWCMSHC_ENHANCED_STROBE BIT(8)
>>> #define DWCMSHC_EMMC_ATCTRL 0x40
>>> +/* DWC IP vendor area 2 pointer */
>>> +#define DWCMSHC_P_VENDOR_AREA2 0xea
>>> +
>>> /* Rockchip specific Registers */
>>> #define DWCMSHC_EMMC_DLL_CTRL 0x800
>>> #define DWCMSHC_EMMC_DLL_RXCLK 0x804
>>> @@ -75,6 +79,10 @@
>>> #define BOUNDARY_OK(addr, len) \
>>> ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
>>> +#define DWCMSHC_SDHCI_CQE_TRNS_MODE (SDHCI_TRNS_MULTI | \
>>> + SDHCI_TRNS_BLK_CNT_EN | \
>>> + SDHCI_TRNS_DMA)
>>> +
>>> enum dwcmshc_rk_type {
>>> DWCMSHC_RK3568,
>>> DWCMSHC_RK3588,
>>> @@ -90,7 +98,8 @@ struct rk35xx_priv {
>>> struct dwcmshc_priv {
>>> struct clk *bus_clk;
>>> - int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA reg */
>>> + int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA1 reg */
>>> + int vendor_specific_area2; /* P_VENDOR_SPECIFIC_AREA2 reg */
>>> void *priv; /* pointer to SoC private stuff */
>>> };
>>> @@ -210,6 +219,147 @@ static void dwcmshc_hs400_enhanced_strobe(struct mmc_host *mmc,
>>> sdhci_writel(host, vendor, reg);
>>> }
>>> +static u32 dwcmshc_cqe_irq_handler(struct sdhci_host *host, u32 intmask)
>>> +{
>>> + int cmd_error = 0;
>>> + int data_error = 0;
>>> +
>>> + if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
>>> + return intmask;
>>> +
>>> + cqhci_irq(host->mmc, intmask, cmd_error, data_error);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void dwcmshc_sdhci_cqe_enable(struct mmc_host *mmc)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + u32 pstate;
>>> + u8 ctrl;
>>> + int count = 10;
>>> +
>>> + /*
>>> + * CQE gets stuck if it sees Buffer Read Enable bit set, which can be
>>> + * the case after tuning, so ensure the buffer is drained.
>>> + */
>>> + pstate = sdhci_readl(host, SDHCI_PRESENT_STATE);
>>> + while (pstate & SDHCI_DATA_AVAILABLE) {
>>> + sdhci_readl(host, SDHCI_BUFFER);
>>> + pstate = sdhci_readl(host, SDHCI_PRESENT_STATE);
>>> + if (count-- == 0) {
>>> + dev_warn(mmc_dev(host->mmc),
>>> + "CQE may get stuck because the Buffer Read Enable bit is set\n");
>>> + break;
>>> + }
>>> + mdelay(1);
>>> + }
>> An alternative, which might be easier, is to do a
>> data reset which may also help allow the device to
>> subsequently enter low power states.
>> Refer commit f8870ae6e2d6be75b1accc2db981169fdfbea7ab
>> and commit 7b7d57fd1b773d25d8358c6017592b4928bf76ce
>
> Thanks, I'll fix it in the next version of the patch.
>
>>
>>> +
>>> + sdhci_writew(host, DWCMSHC_SDHCI_CQE_TRNS_MODE, SDHCI_TRANSFER_MODE);
>>> +
>>> + sdhci_cqe_enable(mmc);
>>> +
>>> + /*
>>> + * The "DesignWare Cores Mobile Storage Host Controller
>>> + * DWC_mshc / DWC_mshc_lite Databook" says:
>>> + * when Host Version 4 Enable" is 1 in Host Control 2 register,
>>> + * SDHCI_CTRL_ADMA32 bit means ADMA2 is selected.
>>> + * Selection of 32-bit/64-bit System Addressing:
>>> + * either 32-bit or 64-bit system addressing is selected by
>>> + * 64-bit Addressing bit in Host Control 2 register.
>>> + *
>>> + * On the other hand the "DesignWare Cores Mobile Storage Host
>>> + * Controller DWC_mshc / DWC_mshc_lite User Guide" says, that we have to
>>> + * set DMA_SEL to ADMA2 _only_ mode in the Host Control 2 register.
>>> + */
>>> + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
>>> + ctrl &= ~SDHCI_CTRL_DMA_MASK;
>>> + ctrl |= SDHCI_CTRL_ADMA32;
>>> + sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>> +}
>>> +
>>> +static void dwcmshc_sdhci_cqe_disable(struct mmc_host *mmc, bool recovery)
>>> +{
>>> + /*
>>> + * If an ioctl was issued, cqe_disable will be called.
>>> + * For CQE of sdhci-of-dwcmshc, the previous in-flight cmd will be lost quietly.
>>> + * So wait for mmc idle state.
>> This sounds like it should be fixed in the mmc block driver.
>> Can you provide an example of when this happens?
> Unfortunately I can't provide an example.
> But this part of the patch is updated by me on the advice of Shawn Lin after his reviewing V1 of the patch.
> Please find his message here:
> https://patchwork.kernel.org/project/linux-mmc/patch/[email protected]/
>
> Just in case here I also quote the part of his comment that refers to this part of the patch:
> "And another issue was found when Rockchip added CQE support for
> sdhci-of-dwcmshc internally, is that if a ioctl was issued, cqe_disable
> will be called. For CQE of sdhci-of-dwcmshc, the previous in-flight cmd
> will be lost quietly. So a mmc->cqe_ops->cqe_wait_for_idle(mmc) should
> be added before sdhci_cqe_disable(), so you need a dwcmshc specified
> cqe_disable hook in sdhci-of-dwcmshc."
mmc block driver already does a wait for idle before ioctl commands,
refer mmc_blk_mq_issue_rq() case MMC_ISSUE_SYNC.
Without more informaton we cannot assume the upstream kernel has
a problem with this.
>
>>
>>> + */
>>> + mmc->cqe_ops->cqe_wait_for_idle(mmc);
>>> +
>>> + return sdhci_cqe_disable(mmc, recovery);
>>> +}
>>> +
>>> +static void dwcmshc_cqhci_set_tran_desc(u8 *desc, dma_addr_t addr, int len, bool end,
>>> + bool dma64)
>>> +{
>>> + __le32 *attr = (__le32 __force *)desc;
>>> +
>>> + *attr = (CQHCI_VALID(1) |
>>> + CQHCI_END(end ? 1 : 0) |
>>> + CQHCI_INT(0) |
>>> + CQHCI_ACT(0x4) |
>>> + CQHCI_DAT_LENGTH(len));
>>> +
>>> + if (dma64) {
>>> + __le64 *dataddr = (__le64 __force *)(desc + 4);
>>> +
>>> + dataddr[0] = cpu_to_le64(addr);
>>> + } else {
>>> + __le32 *dataddr = (__le32 __force *)(desc + 4);
>>> +
>>> + dataddr[0] = cpu_to_le32(addr);
>>> + }
>>> +}
>> This is the same as cqhci_set_tran_desc(). Might as well export that
>> instead.
> Thanks, I'll fix it in the next version of the patch.
>>> +
>>> +static void dwcmshc_cqhci_prep_tran_desc(struct mmc_data *data,
>>> + struct cqhci_host *cq_host,
>>> + u8 *desc, int sg_count)
>>> +{
>>> + int i, len, tmplen, offset;
>>> + bool end = false;
>>> + bool dma64 = cq_host->dma64;
>>> + dma_addr_t addr;
>>> + struct scatterlist *sg;
>>> +
>>> + for_each_sg(data->sg, sg, sg_count, i) {
>>> + addr = sg_dma_address(sg);
>>> + len = sg_dma_len(sg);
>>> +
>>> + /*
>>> + * According to the "DesignWare Cores Mobile Storage Host Controller
>>> + * DWC_mshc / DWC_mshc_lite Databook" the host memory data buffer size
>>> + * and start address must not exceed 128 Mb. If it exceeds,
>>> + * the data buffer must be split using two descritors.
>>> + */
>>> +
>>> + if (likely(BOUNDARY_OK(addr, len))) {
>>> + if ((i + 1) == sg_count)
>>> + end = true;
>>> + dwcmshc_cqhci_set_tran_desc(desc, addr, len, end, dma64);
>>> + desc += cq_host->trans_desc_len;
>>> + } else {
>>> + offset = addr & (SZ_128M - 1);
>>> + tmplen = SZ_128M - offset;
>>> + dwcmshc_cqhci_set_tran_desc(desc, addr, tmplen, end, dma64);
>>> + desc += cq_host->trans_desc_len;
>>> +
>>> + if ((i + 1) == sg_count)
>>> + end = true;
>>> +
>>> + addr += tmplen;
>>> + len -= tmplen;
>>> + dwcmshc_cqhci_set_tran_desc(desc, addr, len, end, dma64);
>>> + desc += cq_host->trans_desc_len;
>>> + }
>>> + }
>>> +}
>> Could this be done more like dwcmshc_adma_write_desc()
> Actually I'm not sure what do you mean. I reused checking boundary construction with
> "BOUNDARY_OK" macro and used the same variable names. I would appreciate it if you could clarify
> what does mean "more like dwcmshc_adma_write_desc()"
Provide a hook for cqhci_set_tran_desc() instead of cqhci_prep_tran_desc()
You'll need to check the details, but something like:
diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
index b3d7d6d8d654..98e7e9d3030d 100644
--- a/drivers/mmc/host/cqhci-core.c
+++ b/drivers/mmc/host/cqhci-core.c
@@ -522,7 +522,10 @@ static int cqhci_prep_tran_desc(struct mmc_request *mrq,
if ((i+1) == sg_count)
end = true;
- cqhci_set_tran_desc(desc, addr, len, end, dma64);
+ if (cq_host->ops->set_tran_desc)
+ cq_host->ops->set_tran_desc(&desc, addr, len, end, dma64);
+ else
+ cqhci_set_tran_desc(desc, addr, len, end, dma64);
desc += cq_host->trans_desc_len;
}
And:
static void dwcmshc_set_tran_desc(u8 **desc, dma_addr_t addr, int len, bool end, bool dma64)
{
int tmplen, offset;
if (likely(!len || BOUNDARY_OK(addr, len))) {
cqhci_set_tran_desc(*desc, addr, len, end, dma64);
return;
}
offset = addr & (SZ_128M - 1);
tmplen = SZ_128M - offset;
cqhci_set_tran_desc(*desc, addr, tmplen, false, dma64);
addr += tmplen;
len -= tmplen;
*desc += cq_host->trans_desc_len;
cqhci_set_tran_desc(*desc, addr, len, end, dma64);
}
>>
>>> +
>>> +static void dwcmshc_cqhci_dumpregs(struct mmc_host *mmc)
>>> +{
>>> + sdhci_dumpregs(mmc_priv(mmc));
>>> +}
>>> +
>>> static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock)
>>> {
>>> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> @@ -345,6 +495,7 @@ static const struct sdhci_ops sdhci_dwcmshc_ops = {
>>> .get_max_clock = dwcmshc_get_max_clock,
>>> .reset = sdhci_reset,
>>> .adma_write_desc = dwcmshc_adma_write_desc,
>>> + .irq = dwcmshc_cqe_irq_handler,
>>> };
>>> static const struct sdhci_ops sdhci_dwcmshc_rk35xx_ops = {
>>> @@ -379,6 +530,70 @@ static const struct sdhci_pltfm_data sdhci_dwcmshc_rk35xx_pdata = {
>>> SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
>>> };
>>> +static const struct cqhci_host_ops dwcmshc_cqhci_ops = {
>>> + .enable = dwcmshc_sdhci_cqe_enable,
>>> + .disable = dwcmshc_sdhci_cqe_disable,
>>> + .dumpregs = dwcmshc_cqhci_dumpregs,
>>> + .prep_tran_desc = dwcmshc_cqhci_prep_tran_desc,
>>> +};
>>> +
>>> +static void dwcmshc_cqhci_init(struct sdhci_host *host, struct platform_device *pdev)
>>> +{
>>> + struct cqhci_host *cq_host;
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> + bool dma64 = false;
>>> + u16 clk;
>>> + int err;
>>> +
>>> + host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD;
>>> + cq_host = devm_kzalloc(&pdev->dev, sizeof(*cq_host), GFP_KERNEL);
>>> + if (!cq_host) {
>>> + dev_err(mmc_dev(host->mmc), "Unable to setup CQE: not enough memory\n");
>>> + return;
>>> + }
>>> +
>>> + /*
>>> + * For dwcmshc host controller we have to enable internal clock
>>> + * before access to some registers from Vendor Specific Aria 2.
>>> + */
>>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>>> + clk |= SDHCI_CLOCK_INT_EN;
>>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>>> + if (!(clk & SDHCI_CLOCK_INT_EN)) {
>>> + dev_err(mmc_dev(host->mmc), "Unable to setup CQE: internal clock enable error\n");
>>> + goto free_cq_host;
>>> + }
>>> +
>>> + cq_host->mmio = host->ioaddr + priv->vendor_specific_area2;
>>> + cq_host->ops = &dwcmshc_cqhci_ops;
>>> +
>>> + /* Enable using of 128-bit task descriptors */
>>> + dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
>>> + if (dma64) {
>>> + dev_dbg(mmc_dev(host->mmc), "128-bit task descriptors\n");
>>> + cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
>>> + }
>>> + err = cqhci_init(cq_host, host->mmc, dma64);
>>> + if (err) {
>>> + dev_err(mmc_dev(host->mmc), "Unable to setup CQE: error %d\n", err);
>>> + goto int_clok_disable;
>>> + }
>>> +
>>> + dev_dbg(mmc_dev(host->mmc), "CQE init done\n");
>>> +
>>> + return;
>>> +
>>> +int_clok_disable:
>> 'clok' is an odd abbreviation of 'clock'. Perhaps 'clk' or just 'clock'
> Thanks, I'll fix it in the next version of the patch.
>>
>>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>>> + clk &= ~SDHCI_CLOCK_INT_EN;
>>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>> +
>>> +free_cq_host:
>>> + devm_kfree(&pdev->dev, cq_host);
>>> +}
>>> +
>>> static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
>>> {
>>> int err;
>>> @@ -471,7 +686,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>>> struct rk35xx_priv *rk_priv = NULL;
>>> const struct sdhci_pltfm_data *pltfm_data;
>>> int err;
>>> - u32 extra;
>>> + u32 extra, caps;
>>> pltfm_data = device_get_match_data(&pdev->dev);
>>> if (!pltfm_data) {
>>> @@ -519,6 +734,8 @@ static int dwcmshc_probe(struct platform_device *pdev)
>>> priv->vendor_specific_area1 =
>>> sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK;
>>> + priv->vendor_specific_area2 =
>>> + sdhci_readw(host, DWCMSHC_P_VENDOR_AREA2);
>>> host->mmc_host_ops.request = dwcmshc_request;
>>> host->mmc_host_ops.hs400_enhanced_strobe = dwcmshc_hs400_enhanced_strobe;
>>> @@ -547,6 +764,10 @@ static int dwcmshc_probe(struct platform_device *pdev)
>>> sdhci_enable_v4_mode(host);
>>> #endif
>>> + caps = sdhci_readl(host, SDHCI_CAPABILITIES);
>>> + if (caps & SDHCI_CAN_64BIT_V4)
>>> + sdhci_enable_v4_mode(host);
>>> +
>>> host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
>>> pm_runtime_get_noresume(dev);
>>> @@ -557,6 +778,14 @@ static int dwcmshc_probe(struct platform_device *pdev)
>>> if (err)
>>> goto err_rpm;
>>> + /* Setup Command Queue Engine if enabled */
>>> + if (device_property_read_bool(&pdev->dev, "supports-cqe")) {
>>> + if (caps & SDHCI_CAN_64BIT_V4)
>>> + dwcmshc_cqhci_init(host, pdev);
>>> + else
>>> + dev_warn(dev, "Cannot enable CQE without V4 mode support\n");
>>> + }
>>> +
>>> if (rk_priv)
>>> dwcmshc_rk35xx_postinit(host, priv);
>>>