2015-08-20 08:44:13

by Shawn Lin

[permalink] [raw]
Subject: [RFC PATCH v6 0/9] Add external dma support for Synopsys MSHC


Synopsys DesignWare mobile storage host controller supports three
types of transfer mode: pio, internal dma and external dma. However,
dw_mmc can only supports pio and internal dma now. Thus some platforms
using dw-mshc integrated with generic dma can't work in dma mode. So we
submit this patch to achieve it.

And the config option, CONFIG_MMC_DW_IDMAC, was added by Will Newton
(commit:f95f3850) for the first version of dw_mmc and never be touched since
then. At that time dt-bindings hadn't been introduced into dw_mmc yet means
we should select CONFIG_MMC_DW_IDMAC to enable internal dma mode at compile
time. Nowadays, device-tree helps us to support a variety of boards with one
kernel. That's why we need to remove it and decide the transfer mode by reading
dw_mmc's HCON reg at runtime.

This RFC patch needs lots of ACKs. I know it's hard, but it does need someone
to make the running.

Patch does the following things:
- remove CONFIG_MMC_DW_IDMAC config option
- add bindings for edmac used by synopsys-dw-mshc
at runtime
- add edmac support for synopsys-dw-mshc

Patch v1~v4 is based on next of git://git.linaro.org/people/ulf.hansson/mmc
Patch v5~v6 is rebased on dw-mmc-for-ulf-v4.2 of https://github.com/jh80chung/dw-mmc.git

Test emmc throughput on my platform with edmac support and without edmac support(pio only)
iozone -L64 -S32 -azecwI -+n -r4k -r64k -r128k -s1g -i0 -i1 -i2 -f datafile -Rb out.xls > /mnt/result.txt
(light cpu loading, Direct IO, fixed line size, all pattern recycle, 1GB data in total)
___________________________________________________________
| external dma mode |
|-----------------------------------------------------------|
|blksz | Random Read | Random Write | Seq Read | Seq Write|
|-----------------------------------------------------------|
|4kB | 13953kB/s | 8602kB/s | 13672kB/s | 9785kB/s|
|-----------------------------------------------------------|
|64kB | 46058kB/s | 24794kB/s | 48058kB/s | 25418kB/s|
|-----------------------------------------------------------|
|128kB | 57026kB/s | 35117kB/s | 57375kB/s | 35183kB/s|
|-----------------------------------------------------------|
VS
___________________________________________________________
| pio mode |
|-----------------------------------------------------------|
|blksz | Random Read | Random Write | Seq Read | Seq Write|
|-----------------------------------------------------------|
|4kB | 11720kB/s | 8644kB/s | 11549kB/s | 9624kB/s|
|-----------------------------------------------------------|
|64kB | 21869kB/s | 24414kB/s | 22031kB/s | 27986kB/s|
|-----------------------------------------------------------|
|128kB | 23718kB/s | 34495kB/s | 24698kB/s | 34637kB/s|
|-----------------------------------------------------------|


Changes in v6:
- add trans_mode condition for IDMAC initialization
suggested by Heiko
- re-test my patch on rk3188 platform and update commit msg
- update performance of pio vs edmac in cover letter

Changes in v5:
- add the title of cover letter
- fix typo of comment
- add macro for reading HCON register
- add "Acked-by: Krzysztof Kozlowski <[email protected]>" for exynos_defconfig patch
- add "Acked-by: Vineet Gupta <[email protected]>" for axs10x_defconfig patch
- add "Acked-by: Govindraj Raja <[email protected]>" and
"Acked-by: Ralf Baechle <[email protected]>" for pistachio_defconfig patch
- add "Acked-by: Joachim Eastwood <[email protected]>" for lpc18xx_defconfig patch
- add "Acked-by: Wei Xu <[email protected]>" for hisi_defconfig patch
- rebase on "https://github.com/jh80chung/dw-mmc.git tags/dw-mmc-for-ulf-v4.2" for merging easily

Changes in v4:
- remove "host->trans_mode" and use "host->use_dma" to indicate
transfer mode.
- remove all bt-bindings' changes since we don't need new properities.
- check transfer mode at runtime by reading HCON reg
- spilt defconfig changes for each sub-architecture
- fix the title of cover letter
- reuse some code for reducing code size

Changes in v3:
- choose transfer mode at runtime
- remove all CONFIG_MMC_DW_IDMAC config option
- add supports-idmac property for some platforms

Changes in v2:
- Fix typo of dev_info msg
- remove unused dmach from declaration of dw_mci_dma_slave

Shawn Lin (9):
mmc: dw_mmc: Add external dma interface support
Documentation: synopsys-dw-mshc: add bindings for idmac and edmac
mips: pistachio_defconfig: remove CONFIG_MMC_DW_IDMAC
arc: axs10x_defconfig: remove CONFIG_MMC_DW_IDMAC
arm: exynos_defconfig: remove CONFIG_MMC_DW_IDMAC
arm: hisi_defconfig: remove CONFIG_MMC_DW_IDMAC
arm: lpc18xx_defconfig: remove CONFIG_MMC_DW_IDMAC
arm: multi_v7_defconfig: remove CONFIG_MMC_DW_IDMAC
arm: zx_defconfig: remove CONFIG_MMC_DW_IDMAC

.../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 25 ++
arch/arc/configs/axs101_defconfig | 1 -
arch/arc/configs/axs103_defconfig | 1 -
arch/arc/configs/axs103_smp_defconfig | 1 -
arch/arm/configs/exynos_defconfig | 1 -
arch/arm/configs/hisi_defconfig | 1 -
arch/arm/configs/lpc18xx_defconfig | 1 -
arch/arm/configs/multi_v7_defconfig | 1 -
arch/arm/configs/zx_defconfig | 1 -
arch/mips/configs/pistachio_defconfig | 1 -
drivers/mmc/host/Kconfig | 11 +-
drivers/mmc/host/dw_mmc-pltfm.c | 2 +
drivers/mmc/host/dw_mmc.c | 264 +++++++++++++++++----
drivers/mmc/host/dw_mmc.h | 5 +
include/linux/mmc/dw_mmc.h | 27 ++-
15 files changed, 267 insertions(+), 76 deletions(-)

--
2.3.7


2015-08-20 08:44:33

by Shawn Lin

[permalink] [raw]
Subject: [RFC PATCH v6 1/9] mmc: dw_mmc: Add external dma interface support

DesignWare MMC Controller can supports two types of DMA
mode: external dma and internal dma. We get a RK312x platform
integrated dw_mmc and ARM pl330 dma controller. This patch add
edmac ops to support these platforms. I've tested it on RK31xx
platform with edmac mode and RK3288 platform with idmac mode.

Signed-off-by: Shawn Lin <[email protected]>

---

Changes in v6:
- add trans_mode condition for IDMAC initialization
suggested by Heiko
- re-test my patch on rk3188 platform and update commit msg
- update performance of pio vs edmac in cover letter

Changes in v5:
- add the title of cover letter
- fix typo of comment
- add macro for reading HCON register
- add "Acked-by: Krzysztof Kozlowski <[email protected]>" for exynos_defconfig patch
- add "Acked-by: Vineet Gupta <[email protected]>" for axs10x_defconfig patch
- add "Acked-by: Govindraj Raja <[email protected]>" and
"Acked-by: Ralf Baechle <[email protected]>" for pistachio_defconfig patch
- add "Acked-by: Joachim Eastwood <[email protected]>" for lpc18xx_defconfig patch
- add "Acked-by: Wei Xu <[email protected]>" for hisi_defconfig patch
- rebase on "https://github.com/jh80chung/dw-mmc.git tags/dw-mmc-for-ulf-v4.2" for merging easily

Changes in v4:
- remove "host->trans_mode" and use "host->use_dma" to indicate
transfer mode.
- remove all bt-bindings' changes since we don't need new properities.
- check transfer mode at runtime by reading HCON reg
- spilt defconfig changes for each sub-architecture
- fix the title of cover letter
- reuse some code for reducing code size

Changes in v3:
- choose transfer mode at runtime
- remove all CONFIG_MMC_DW_IDMAC config option
- add supports-idmac property for some platforms

Changes in v2:
- Fix typo of dev_info msg
- remove unused dmach from declaration of dw_mci_dma_slave

drivers/mmc/host/Kconfig | 11 +-
drivers/mmc/host/dw_mmc-pltfm.c | 2 +
drivers/mmc/host/dw_mmc.c | 264 ++++++++++++++++++++++++++++++++--------
drivers/mmc/host/dw_mmc.h | 5 +
include/linux/mmc/dw_mmc.h | 27 +++-
5 files changed, 242 insertions(+), 67 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 6a0f9c7..a86c0eb 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -607,15 +607,7 @@ config MMC_DW
help
This selects support for the Synopsys DesignWare Mobile Storage IP
block, this provides host support for SD and MMC interfaces, in both
- PIO and external DMA modes.
-
-config MMC_DW_IDMAC
- bool "Internal DMAC interface"
- depends on MMC_DW
- help
- This selects support for the internal DMAC block within the Synopsys
- Designware Mobile Storage IP block. This disables the external DMA
- interface.
+ PIO, internal DMA mode and external DMA modes.

config MMC_DW_PLTFM
tristate "Synopsys Designware MCI Support as platform device"
@@ -644,7 +636,6 @@ config MMC_DW_K3
tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
depends on MMC_DW
select MMC_DW_PLTFM
- select MMC_DW_IDMAC
help
This selects support for Hisilicon K3 SoC specific extensions to the
Synopsys DesignWare Memory Card Interface driver. Select this option
diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
index ec6dbcd..7e1d13b 100644
--- a/drivers/mmc/host/dw_mmc-pltfm.c
+++ b/drivers/mmc/host/dw_mmc-pltfm.c
@@ -59,6 +59,8 @@ int dw_mci_pltfm_register(struct platform_device *pdev,
host->pdata = pdev->dev.platform_data;

regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ /* Get registers' physical base address */
+ host->phy_regs = (void *)(regs->start);
host->regs = devm_ioremap_resource(&pdev->dev, regs);
if (IS_ERR(host->regs))
return PTR_ERR(host->regs);
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index fcbf552..f943619 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -56,7 +56,6 @@
#define DW_MCI_FREQ_MAX 200000000 /* unit: HZ */
#define DW_MCI_FREQ_MIN 400000 /* unit: HZ */

-#ifdef CONFIG_MMC_DW_IDMAC
#define IDMAC_INT_CLR (SDMMC_IDMAC_INT_AI | SDMMC_IDMAC_INT_NI | \
SDMMC_IDMAC_INT_CES | SDMMC_IDMAC_INT_DU | \
SDMMC_IDMAC_INT_FBE | SDMMC_IDMAC_INT_RI | \
@@ -102,7 +101,6 @@ struct idmac_desc {

/* Each descriptor can transfer up to 4KB of data in chained mode */
#define DW_MCI_DESC_DATA_LENGTH 0x1000
-#endif /* CONFIG_MMC_DW_IDMAC */

static bool dw_mci_reset(struct dw_mci *host);
static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
@@ -407,7 +405,6 @@ static int dw_mci_get_dma_dir(struct mmc_data *data)
return DMA_FROM_DEVICE;
}

-#ifdef CONFIG_MMC_DW_IDMAC
static void dw_mci_dma_cleanup(struct dw_mci *host)
{
struct mmc_data *data = host->data;
@@ -445,12 +442,21 @@ static void dw_mci_idmac_stop_dma(struct dw_mci *host)
mci_writel(host, BMOD, temp);
}

-static void dw_mci_idmac_complete_dma(struct dw_mci *host)
+static void dw_mci_dmac_complete_dma(void *arg)
{
+ struct dw_mci *host = arg;
struct mmc_data *data = host->data;

dev_vdbg(host->dev, "DMA complete\n");

+ if (host->use_dma == TRANS_MODE_EDMAC)
+ if (data && (data->flags & MMC_DATA_READ))
+ /* Invalidate cache after read */
+ dma_sync_sg_for_cpu(mmc_dev(host->cur_slot->mmc),
+ data->sg,
+ data->sg_len,
+ DMA_FROM_DEVICE);
+
host->dma_ops->cleanup(host);

/*
@@ -564,7 +570,7 @@ static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
wmb(); /* drain writebuffer */
}

-static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
+static int dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
{
u32 temp;

@@ -589,6 +595,8 @@ static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)

/* Start it running */
mci_writel(host, PLDMND, 1);
+
+ return 0;
}

static int dw_mci_idmac_init(struct dw_mci *host)
@@ -669,10 +677,112 @@ static const struct dw_mci_dma_ops dw_mci_idmac_ops = {
.init = dw_mci_idmac_init,
.start = dw_mci_idmac_start_dma,
.stop = dw_mci_idmac_stop_dma,
- .complete = dw_mci_idmac_complete_dma,
+ .complete = dw_mci_dmac_complete_dma,
+ .cleanup = dw_mci_dma_cleanup,
+};
+
+static void dw_mci_edmac_stop_dma(struct dw_mci *host)
+{
+ dmaengine_terminate_all(host->dms->ch);
+}
+
+static int dw_mci_edmac_start_dma(struct dw_mci *host,
+ unsigned int sg_len)
+{
+ struct dma_slave_config cfg;
+ struct dma_async_tx_descriptor *desc = NULL;
+ struct scatterlist *sgl = host->data->sg;
+ const u32 mszs[] = {1, 4, 8, 16, 32, 64, 128, 256};
+ u32 sg_elems = host->data->sg_len;
+ u32 fifoth_val;
+ u32 fifo_offset = host->fifo_reg - host->regs;
+ int ret = 0;
+
+ /* Set external dma config: burst size, burst width */
+ cfg.dst_addr = (dma_addr_t)(host->phy_regs + fifo_offset);
+ cfg.src_addr = cfg.dst_addr;
+ cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+
+ /* Match burst msize with external dma config */
+ fifoth_val = mci_readl(host, FIFOTH);
+ cfg.dst_maxburst = mszs[(fifoth_val >> 28) & 0x7];
+ cfg.src_maxburst = cfg.dst_maxburst;
+
+ if (host->data->flags & MMC_DATA_WRITE)
+ cfg.direction = DMA_MEM_TO_DEV;
+ else /* MMC_DATA_READ */
+ cfg.direction = DMA_DEV_TO_MEM;
+
+ ret = dmaengine_slave_config(host->dms->ch, &cfg);
+ if (ret) {
+ dev_err(host->dev, "Failed to config edmac.\n");
+ return -EBUSY;
+ }
+
+ desc = dmaengine_prep_slave_sg(host->dms->ch, sgl,
+ sg_len, cfg.direction,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!desc) {
+ dev_err(host->dev, "Can't prepare slave sg.\n");
+ return -EBUSY;
+ }
+
+ /* Set dw_mci_dmac_complete_dma as callback */
+ desc->callback = dw_mci_dmac_complete_dma;
+ desc->callback_param = (void *)host;
+ dmaengine_submit(desc);
+
+ /* Flush cache before write */
+ if (host->data->flags & MMC_DATA_WRITE)
+ dma_sync_sg_for_device(mmc_dev(host->cur_slot->mmc), sgl,
+ sg_elems, DMA_TO_DEVICE);
+
+ dma_async_issue_pending(host->dms->ch);
+
+ return 0;
+}
+
+static int dw_mci_edmac_init(struct dw_mci *host)
+{
+ /* Request external dma channel */
+ host->dms = kzalloc(sizeof(struct dw_mci_dma_slave), GFP_KERNEL);
+ if (!host->dms)
+ return -ENOMEM;
+
+ host->dms->ch = dma_request_slave_channel(host->dev, "rx-tx");
+ if (!host->dms->ch) {
+ dev_err(host->dev,
+ "Failed to get external DMA channel %d\n",
+ host->dms->ch->chan_id);
+ kfree(host->dms);
+ host->dms = NULL;
+ return -ENXIO;
+ }
+
+ return 0;
+}
+
+static void dw_mci_edmac_exit(struct dw_mci *host)
+{
+ if (host->dms) {
+ if (host->dms->ch) {
+ dma_release_channel(host->dms->ch);
+ host->dms->ch = NULL;
+ }
+ kfree(host->dms);
+ host->dms = NULL;
+ }
+}
+
+static const struct dw_mci_dma_ops dw_mci_edmac_ops = {
+ .init = dw_mci_edmac_init,
+ .exit = dw_mci_edmac_exit,
+ .start = dw_mci_edmac_start_dma,
+ .stop = dw_mci_edmac_stop_dma,
+ .complete = dw_mci_dmac_complete_dma,
.cleanup = dw_mci_dma_cleanup,
};
-#endif /* CONFIG_MMC_DW_IDMAC */

static int dw_mci_pre_dma_transfer(struct dw_mci *host,
struct mmc_data *data,
@@ -752,7 +862,6 @@ static void dw_mci_post_req(struct mmc_host *mmc,

static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
{
-#ifdef CONFIG_MMC_DW_IDMAC
unsigned int blksz = data->blksz;
const u32 mszs[] = {1, 4, 8, 16, 32, 64, 128, 256};
u32 fifo_width = 1 << host->data_shift;
@@ -760,6 +869,10 @@ static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
u32 msize = 0, rx_wmark = 1, tx_wmark, tx_wmark_invers;
int idx = ARRAY_SIZE(mszs) - 1;

+ /* pio should ship this scenario */
+ if (!host->use_dma)
+ return;
+
tx_wmark = (host->fifo_depth) / 2;
tx_wmark_invers = host->fifo_depth - tx_wmark;

@@ -788,7 +901,6 @@ static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
done:
fifoth_val = SDMMC_SET_FIFOTH(msize, rx_wmark, tx_wmark);
mci_writel(host, FIFOTH, fifoth_val);
-#endif
}

static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
@@ -850,10 +962,12 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)

host->using_dma = 1;

- dev_vdbg(host->dev,
- "sd sg_cpu: %#lx sg_dma: %#lx sg_len: %d\n",
- (unsigned long)host->sg_cpu, (unsigned long)host->sg_dma,
- sg_len);
+ if (host->use_dma == TRANS_MODE_IDMAC)
+ dev_vdbg(host->dev,
+ "sd sg_cpu: %#lx sg_dma: %#lx sg_len: %d\n",
+ (unsigned long)host->sg_cpu,
+ (unsigned long)host->sg_dma,
+ sg_len);

/*
* Decide the MSIZE and RX/TX Watermark.
@@ -875,7 +989,11 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
mci_writel(host, INTMASK, temp);
spin_unlock_irqrestore(&host->irq_lock, irqflags);

- host->dma_ops->start(host, sg_len);
+ if (host->dma_ops->start(host, sg_len)) {
+ /* We can't do DMA */
+ dev_err(host->dev, "%s: failed to start DMA.\n", __func__);
+ return -ENODEV;
+ }

return 0;
}
@@ -2343,15 +2461,17 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)

}

-#ifdef CONFIG_MMC_DW_IDMAC
- /* Handle DMA interrupts */
+ if (host->use_dma != TRANS_MODE_IDMAC)
+ return IRQ_HANDLED;
+
+ /* Handle IDMA interrupts */
if (host->dma_64bit_address == 1) {
pending = mci_readl(host, IDSTS64);
if (pending & (SDMMC_IDMAC_INT_TI | SDMMC_IDMAC_INT_RI)) {
mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_TI |
SDMMC_IDMAC_INT_RI);
mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
- host->dma_ops->complete(host);
+ host->dma_ops->complete((void *)host);
}
} else {
pending = mci_readl(host, IDSTS);
@@ -2359,10 +2479,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
mci_writel(host, IDSTS, SDMMC_IDMAC_INT_TI |
SDMMC_IDMAC_INT_RI);
mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
- host->dma_ops->complete(host);
+ host->dma_ops->complete((void *)host);
}
}
-#endif

return IRQ_HANDLED;
}
@@ -2471,13 +2590,21 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
goto err_host_allocated;

/* Useful defaults if platform data is unset. */
- if (host->use_dma) {
+ if (host->use_dma == TRANS_MODE_IDMAC) {
mmc->max_segs = host->ring_size;
mmc->max_blk_size = 65536;
mmc->max_seg_size = 0x1000;
mmc->max_req_size = mmc->max_seg_size * host->ring_size;
mmc->max_blk_count = mmc->max_req_size / 512;
+ } else if (host->use_dma == TRANS_MODE_EDMAC) {
+ mmc->max_segs = 64;
+ mmc->max_blk_size = 65536;
+ mmc->max_blk_count = 65535;
+ mmc->max_req_size =
+ mmc->max_blk_size * mmc->max_blk_count;
+ mmc->max_seg_size = mmc->max_req_size;
} else {
+ /* TRANS_MODE_PIO */
mmc->max_segs = 64;
mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
mmc->max_blk_count = 512;
@@ -2517,35 +2644,66 @@ static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
static void dw_mci_init_dma(struct dw_mci *host)
{
int addr_config;
- /* Check ADDR_CONFIG bit in HCON to find IDMAC address bus width */
- addr_config = (mci_readl(host, HCON) >> 27) & 0x01;
-
- if (addr_config == 1) {
- /* host supports IDMAC in 64-bit address mode */
- host->dma_64bit_address = 1;
- dev_info(host->dev, "IDMAC supports 64-bit address mode.\n");
- if (!dma_set_mask(host->dev, DMA_BIT_MASK(64)))
- dma_set_coherent_mask(host->dev, DMA_BIT_MASK(64));
- } else {
- /* host supports IDMAC in 32-bit address mode */
- host->dma_64bit_address = 0;
- dev_info(host->dev, "IDMAC supports 32-bit address mode.\n");
- }
+ int trans_mode;
+ struct device *dev = host->dev;
+ struct device_node *np = dev->of_node;

- /* Alloc memory for sg translation */
- host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
- &host->sg_dma, GFP_KERNEL);
- if (!host->sg_cpu) {
- dev_err(host->dev, "%s: could not alloc DMA memory\n",
- __func__);
+ /* Check tansfer mode */
+ trans_mode = SDMMC_GET_TRANS_MODE(mci_readl(host, HCON));
+ if (trans_mode == 0) {
+ trans_mode = TRANS_MODE_IDMAC;
+ } else if (trans_mode == 1 || trans_mode == 2) {
+ trans_mode = TRANS_MODE_EDMAC;
+ } else {
+ trans_mode = TRANS_MODE_PIO;
goto no_dma;
}

/* Determine which DMA interface to use */
-#ifdef CONFIG_MMC_DW_IDMAC
- host->dma_ops = &dw_mci_idmac_ops;
- dev_info(host->dev, "Using internal DMA controller.\n");
-#endif
+ if (trans_mode == TRANS_MODE_IDMAC) {
+ /*
+ * Check ADDR_CONFIG bit in HCON to find
+ * IDMAC address bus width
+ */
+ addr_config = SDMMC_GET_ADDR_CONFIG(mci_readl(host, HCON));
+
+ if (addr_config == 1) {
+ /* host supports IDMAC in 64-bit address mode */
+ host->dma_64bit_address = 1;
+ dev_info(host->dev,
+ "IDMAC supports 64-bit address mode.\n");
+ if (!dma_set_mask(host->dev, DMA_BIT_MASK(64)))
+ dma_set_coherent_mask(host->dev,
+ DMA_BIT_MASK(64));
+ } else {
+ /* host supports IDMAC in 32-bit address mode */
+ host->dma_64bit_address = 0;
+ dev_info(host->dev,
+ "IDMAC supports 32-bit address mode.\n");
+ }
+
+ /* Alloc memory for sg translation */
+ host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
+ &host->sg_dma, GFP_KERNEL);
+ if (!host->sg_cpu) {
+ dev_err(host->dev,
+ "%s: could not alloc DMA memory\n",
+ __func__);
+ goto no_dma;
+ }
+
+ host->dma_ops = &dw_mci_idmac_ops;
+ dev_info(host->dev, "Using internal DMA controller.\n");
+ } else {
+ /* TRANS_MODE_EDMAC: check dma bindings again */
+ if ((of_property_count_strings(np, "dma-names") < 0) ||
+ (!of_find_property(np, "dmas", NULL))) {
+ trans_mode = TRANS_MODE_PIO;
+ goto no_dma;
+ }
+ host->dma_ops = &dw_mci_edmac_ops;
+ dev_info(host->dev, "Using external DMA controller.\n");
+ }

if (!host->dma_ops)
goto no_dma;
@@ -2562,12 +2720,12 @@ static void dw_mci_init_dma(struct dw_mci *host)
goto no_dma;
}

- host->use_dma = 1;
+ host->use_dma = trans_mode;
return;

no_dma:
dev_info(host->dev, "Using PIO mode.\n");
- host->use_dma = 0;
+ host->use_dma = trans_mode;
}

static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset)
@@ -2650,10 +2808,9 @@ static bool dw_mci_reset(struct dw_mci *host)
}
}

-#if IS_ENABLED(CONFIG_MMC_DW_IDMAC)
- /* It is also recommended that we reset and reprogram idmac */
- dw_mci_idmac_reset(host);
-#endif
+ if (host->use_dma == TRANS_MODE_IDMAC)
+ /* It is also recommended that we reset and reprogram idmac */
+ dw_mci_idmac_reset(host);

ret = true;

@@ -2890,7 +3047,7 @@ int dw_mci_probe(struct dw_mci *host)
* Get the host data width - this assumes that HCON has been set with
* the correct values.
*/
- i = (mci_readl(host, HCON) >> 7) & 0x7;
+ i = SDMMC_GET_HDATA_WIDTH(mci_readl(host, HCON));
if (!i) {
host->push_data = dw_mci_push_data16;
host->pull_data = dw_mci_pull_data16;
@@ -2972,7 +3129,7 @@ int dw_mci_probe(struct dw_mci *host)
if (host->pdata->num_slots)
host->num_slots = host->pdata->num_slots;
else
- host->num_slots = ((mci_readl(host, HCON) >> 1) & 0x1F) + 1;
+ host->num_slots = SDMMC_GET_SLOT_NUM(mci_readl(host, HCON));

/*
* Enable interrupts for command done, data over, data empty,
@@ -3067,6 +3224,9 @@ EXPORT_SYMBOL(dw_mci_remove);
*/
int dw_mci_suspend(struct dw_mci *host)
{
+ if (host->use_dma && host->dma_ops->exit)
+ host->dma_ops->exit(host);
+
return 0;
}
EXPORT_SYMBOL(dw_mci_suspend);
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 8ce4674..c453e94 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -148,6 +148,11 @@
#define SDMMC_SET_FIFOTH(m, r, t) (((m) & 0x7) << 28 | \
((r) & 0xFFF) << 16 | \
((t) & 0xFFF))
+/* HCON register defines */
+#define SDMMC_GET_SLOT_NUM(x) ((((x)>>1) & 0x1F) + 1)
+#define SDMMC_GET_HDATA_WIDTH(x) (((x)>>7) & 0x7)
+#define SDMMC_GET_TRANS_MODE(x) (((x)>>16) & 0x3)
+#define SDMMC_GET_ADDR_CONFIG(x) (((x)>>27) & 0x1)
/* Internal DMAC interrupt defines */
#define SDMMC_IDMAC_INT_AI BIT(9)
#define SDMMC_IDMAC_INT_NI BIT(8)
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index c846f42..6a2b83c 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -16,6 +16,7 @@

#include <linux/scatterlist.h>
#include <linux/mmc/core.h>
+#include <linux/dmaengine.h>

#define MAX_MCI_SLOTS 2

@@ -40,6 +41,17 @@ enum {

struct mmc_data;

+enum {
+ TRANS_MODE_PIO = 0,
+ TRANS_MODE_IDMAC,
+ TRANS_MODE_EDMAC
+};
+
+struct dw_mci_dma_slave {
+ struct dma_chan *ch;
+ enum dma_transfer_direction direction;
+};
+
/**
* struct dw_mci - MMC controller state shared between all slots
* @lock: Spinlock protecting the queue and associated data.
@@ -154,11 +166,16 @@ struct dw_mci {
dma_addr_t sg_dma;
void *sg_cpu;
const struct dw_mci_dma_ops *dma_ops;
-#ifdef CONFIG_MMC_DW_IDMAC
+ /* For idmac */
unsigned int ring_size;
-#else
+
+ /* For edmac */
+ struct dw_mci_dma_slave *dms;
+ /* Registers's physical base address */
+ void *phy_regs;
+
struct dw_mci_dma_data *dma_data;
-#endif
+
u32 cmd_status;
u32 data_status;
u32 stop_cmdr;
@@ -212,8 +229,8 @@ struct dw_mci {
struct dw_mci_dma_ops {
/* DMA Ops */
int (*init)(struct dw_mci *host);
- void (*start)(struct dw_mci *host, unsigned int sg_len);
- void (*complete)(struct dw_mci *host);
+ int (*start)(struct dw_mci *host, unsigned int sg_len);
+ void (*complete)(void *host);
void (*stop)(struct dw_mci *host);
void (*cleanup)(struct dw_mci *host);
void (*exit)(struct dw_mci *host);
--
2.3.7

2015-08-20 08:44:45

by Shawn Lin

[permalink] [raw]
Subject: [RFC PATCH v6 2/9] Documentation: synopsys-dw-mshc: add bindings for idmac and edmac

synopsys-dw-mshc supports three types of transfer mode. We add
bindings and description for how to use them at runtime.

Signed-off-by: Shawn Lin <[email protected]>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

.../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
index 346c609..8636f5a 100644
--- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
@@ -75,6 +75,12 @@ Optional properties:
* vmmc-supply: The phandle to the regulator to use for vmmc. If this is
specified we'll defer probe until we can find this regulator.

+* dmas: List of DMA specifiers with the controller specific format as described
+ in the generic DMA client binding. Refer to dma.txt for details.
+
+* dma-names: request names for generic DMA client binding. Must be "rx-tx".
+ Refer to dma.txt for details.
+
Aliases:

- All the MSHC controller nodes should be represented in the aliases node using
@@ -95,6 +101,23 @@ board specific portions as listed below.
#size-cells = <0>;
};

+[board specific internal DMA resources]
+
+ dwmmc0@12200000 {
+ clock-frequency = <400000000>;
+ clock-freq-min-max = <400000 200000000>;
+ num-slots = <1>;
+ broken-cd;
+ fifo-depth = <0x80>;
+ card-detect-delay = <200>;
+ vmmc-supply = <&buck8>;
+ bus-width = <8>;
+ cap-mmc-highspeed;
+ cap-sd-highspeed;
+ };
+
+[board specific generic DMA request binding]
+
dwmmc0@12200000 {
clock-frequency = <400000000>;
clock-freq-min-max = <400000 200000000>;
@@ -106,4 +129,6 @@ board specific portions as listed below.
bus-width = <8>;
cap-mmc-highspeed;
cap-sd-highspeed;
+ dmas = <&pdma 12>;
+ dma-names = "rx-tx";
};
--
2.3.7

2015-08-20 08:44:57

by Shawn Lin

[permalink] [raw]
Subject: [RFC PATCH v6 3/9] mips: pistachio_defconfig: remove CONFIG_MMC_DW_IDMAC

DesignWare MMC Controller's transfer mode should be decided
at runtime instead of compile-time. So we remove this config
option and read dw_mmc's register to select DMA master.

Signed-off-by: Shawn Lin <[email protected]>
Acked-by: Govindraj Raja <[email protected]>
Acked-by: Ralf Baechle <[email protected]>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

arch/mips/configs/pistachio_defconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/mips/configs/pistachio_defconfig b/arch/mips/configs/pistachio_defconfig
index 1646cce..013c62c 100644
--- a/arch/mips/configs/pistachio_defconfig
+++ b/arch/mips/configs/pistachio_defconfig
@@ -257,7 +257,6 @@ CONFIG_MMC=y
CONFIG_MMC_BLOCK_MINORS=16
CONFIG_MMC_TEST=m
CONFIG_MMC_DW=y
-CONFIG_MMC_DW_IDMAC=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=y
CONFIG_RTC_CLASS=y
--
2.3.7

2015-08-20 08:45:31

by Shawn Lin

[permalink] [raw]
Subject: [RFC PATCH v6 4/9] arc: axs10x_defconfig: remove CONFIG_MMC_DW_IDMAC

DesignWare MMC Controller's transfer mode should be decided
at runtime instead of compile-time. So we remove this config
option and read dw_mmc's register to select DMA master.

Signed-off-by: Shawn Lin <[email protected]>
Acked-by: Vineet Gupta <[email protected]>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

arch/arc/configs/axs101_defconfig | 1 -
arch/arc/configs/axs103_defconfig | 1 -
arch/arc/configs/axs103_smp_defconfig | 1 -
3 files changed, 3 deletions(-)

diff --git a/arch/arc/configs/axs101_defconfig b/arch/arc/configs/axs101_defconfig
index 562dac6..c92c0ef 100644
--- a/arch/arc/configs/axs101_defconfig
+++ b/arch/arc/configs/axs101_defconfig
@@ -89,7 +89,6 @@ CONFIG_MMC=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_PLTFM=y
CONFIG_MMC_DW=y
-CONFIG_MMC_DW_IDMAC=y
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_EXT3_FS=y
CONFIG_EXT4_FS=y
diff --git a/arch/arc/configs/axs103_defconfig b/arch/arc/configs/axs103_defconfig
index 83a6d8d..cfac24e 100644
--- a/arch/arc/configs/axs103_defconfig
+++ b/arch/arc/configs/axs103_defconfig
@@ -95,7 +95,6 @@ CONFIG_MMC=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_PLTFM=y
CONFIG_MMC_DW=y
-CONFIG_MMC_DW_IDMAC=y
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_EXT3_FS=y
CONFIG_EXT4_FS=y
diff --git a/arch/arc/configs/axs103_smp_defconfig b/arch/arc/configs/axs103_smp_defconfig
index f1e1c84..9922a11 100644
--- a/arch/arc/configs/axs103_smp_defconfig
+++ b/arch/arc/configs/axs103_smp_defconfig
@@ -96,7 +96,6 @@ CONFIG_MMC=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_PLTFM=y
CONFIG_MMC_DW=y
-CONFIG_MMC_DW_IDMAC=y
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_EXT3_FS=y
CONFIG_EXT4_FS=y
--
2.3.7

2015-08-20 08:45:43

by Shawn Lin

[permalink] [raw]
Subject: [RFC PATCH v6 5/9] arm: exynos_defconfig: remove CONFIG_MMC_DW_IDMAC

DesignWare MMC Controller's transfer mode should be decided
at runtime instead of compile-time. So we remove this config
option and read dw_mmc's register to select DMA master.

Signed-off-by: Shawn Lin <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

arch/arm/configs/exynos_defconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
index 9504e77..7e4af6e 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -161,7 +161,6 @@ CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_S3C=y
CONFIG_MMC_SDHCI_S3C_DMA=y
CONFIG_MMC_DW=y
-CONFIG_MMC_DW_IDMAC=y
CONFIG_MMC_DW_EXYNOS=y
CONFIG_RTC_CLASS=y
CONFIG_RTC_DRV_MAX77686=y
--
2.3.7

2015-08-20 08:46:03

by Shawn Lin

[permalink] [raw]
Subject: [RFC PATCH v6 6/9] arm: hisi_defconfig: remove CONFIG_MMC_DW_IDMAC

DesignWare MMC Controller's transfer mode should be decided
at runtime instead of compile-time. So we remove this config
option and read dw_mmc's register to select DMA master.

Signed-off-by: Shawn Lin <[email protected]>
Acked-by: Wei Xu <[email protected]>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

arch/arm/configs/hisi_defconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/arm/configs/hisi_defconfig b/arch/arm/configs/hisi_defconfig
index 5997dbc..b2e340b 100644
--- a/arch/arm/configs/hisi_defconfig
+++ b/arch/arm/configs/hisi_defconfig
@@ -69,7 +69,6 @@ CONFIG_NOP_USB_XCEIV=y
CONFIG_MMC=y
CONFIG_RTC_CLASS=y
CONFIG_MMC_DW=y
-CONFIG_MMC_DW_IDMAC=y
CONFIG_MMC_DW_PLTFM=y
CONFIG_RTC_DRV_PL031=y
CONFIG_DMADEVICES=y
--
2.3.7

2015-08-20 08:46:13

by Shawn Lin

[permalink] [raw]
Subject: [RFC PATCH v6 7/9] arm: lpc18xx_defconfig: remove CONFIG_MMC_DW_IDMAC

DesignWare MMC Controller's transfer mode should be decided
at runtime instead of compile-time. So we remove this config
option and read dw_mmc's register to select DMA master.

Signed-off-by: Shawn Lin <[email protected]>
Acked-by: Joachim Eastwood <[email protected]>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

arch/arm/configs/lpc18xx_defconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/arm/configs/lpc18xx_defconfig b/arch/arm/configs/lpc18xx_defconfig
index 1c47f86..b7e8cda 100644
--- a/arch/arm/configs/lpc18xx_defconfig
+++ b/arch/arm/configs/lpc18xx_defconfig
@@ -119,7 +119,6 @@ CONFIG_USB_EHCI_HCD=y
CONFIG_USB_EHCI_ROOT_HUB_TT=y
CONFIG_MMC=y
CONFIG_MMC_DW=y
-CONFIG_MMC_DW_IDMAC=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=y
CONFIG_LEDS_PCA9532=y
--
2.3.7

2015-08-20 08:46:26

by Shawn Lin

[permalink] [raw]
Subject: [RFC PATCH v6 8/9] arm: multi_v7_defconfig: remove CONFIG_MMC_DW_IDMAC

DesignWare MMC Controller's transfer mode should be decided
at runtime instead of compile-time. So we remove this config
option and read dw_mmc's register to select DMA master.

Signed-off-by: Shawn Lin <[email protected]>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

arch/arm/configs/multi_v7_defconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 5fd8df6..a3734b5 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -520,7 +520,6 @@ CONFIG_MMC_ATMELMCI=y
CONFIG_MMC_MVSDIO=y
CONFIG_MMC_SDHI=y
CONFIG_MMC_DW=y
-CONFIG_MMC_DW_IDMAC=y
CONFIG_MMC_DW_PLTFM=y
CONFIG_MMC_DW_EXYNOS=y
CONFIG_MMC_DW_ROCKCHIP=y
--
2.3.7

2015-08-20 08:46:37

by Shawn Lin

[permalink] [raw]
Subject: [RFC PATCH v6 9/9] arm: zx_defconfig: remove CONFIG_MMC_DW_IDMAC

DesignWare MMC Controller's transfer mode should be decided
at runtime instead of compile-time. So we remove this config
option and read dw_mmc's register to select DMA master.

Signed-off-by: Shawn Lin <[email protected]>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

arch/arm/configs/zx_defconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/arm/configs/zx_defconfig b/arch/arm/configs/zx_defconfig
index b200bb0..ab683fb 100644
--- a/arch/arm/configs/zx_defconfig
+++ b/arch/arm/configs/zx_defconfig
@@ -83,7 +83,6 @@ CONFIG_MMC=y
CONFIG_MMC_UNSAFE_RESUME=y
CONFIG_MMC_BLOCK_MINORS=16
CONFIG_MMC_DW=y
-CONFIG_MMC_DW_IDMAC=y
CONFIG_EXT2_FS=y
CONFIG_EXT4_FS=y
CONFIG_EXT4_FS_POSIX_ACL=y
--
2.3.7

2015-08-21 06:27:45

by Jaehoon Chung

[permalink] [raw]
Subject: Re: [RFC PATCH v6 1/9] mmc: dw_mmc: Add external dma interface support

Hi, Shawn.

Is this based on Ulf's repository?

On 08/20/2015 05:43 PM, Shawn Lin wrote:
> DesignWare MMC Controller can supports two types of DMA
> mode: external dma and internal dma. We get a RK312x platform
> integrated dw_mmc and ARM pl330 dma controller. This patch add
> edmac ops to support these platforms. I've tested it on RK31xx
> platform with edmac mode and RK3288 platform with idmac mode.
>
> Signed-off-by: Shawn Lin <[email protected]>
>
> ---
>
> Changes in v6:
> - add trans_mode condition for IDMAC initialization
> suggested by Heiko
> - re-test my patch on rk3188 platform and update commit msg
> - update performance of pio vs edmac in cover letter
>
> Changes in v5:
> - add the title of cover letter
> - fix typo of comment
> - add macro for reading HCON register
> - add "Acked-by: Krzysztof Kozlowski <[email protected]>" for exynos_defconfig patch
> - add "Acked-by: Vineet Gupta <[email protected]>" for axs10x_defconfig patch
> - add "Acked-by: Govindraj Raja <[email protected]>" and
> "Acked-by: Ralf Baechle <[email protected]>" for pistachio_defconfig patch
> - add "Acked-by: Joachim Eastwood <[email protected]>" for lpc18xx_defconfig patch
> - add "Acked-by: Wei Xu <[email protected]>" for hisi_defconfig patch
> - rebase on "https://github.com/jh80chung/dw-mmc.git tags/dw-mmc-for-ulf-v4.2" for merging easily
>
> Changes in v4:
> - remove "host->trans_mode" and use "host->use_dma" to indicate
> transfer mode.
> - remove all bt-bindings' changes since we don't need new properities.
> - check transfer mode at runtime by reading HCON reg
> - spilt defconfig changes for each sub-architecture
> - fix the title of cover letter
> - reuse some code for reducing code size
>
> Changes in v3:
> - choose transfer mode at runtime
> - remove all CONFIG_MMC_DW_IDMAC config option
> - add supports-idmac property for some platforms
>
> Changes in v2:
> - Fix typo of dev_info msg
> - remove unused dmach from declaration of dw_mci_dma_slave
>
> drivers/mmc/host/Kconfig | 11 +-
> drivers/mmc/host/dw_mmc-pltfm.c | 2 +
> drivers/mmc/host/dw_mmc.c | 264 ++++++++++++++++++++++++++++++++--------
> drivers/mmc/host/dw_mmc.h | 5 +
> include/linux/mmc/dw_mmc.h | 27 +++-
> 5 files changed, 242 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 6a0f9c7..a86c0eb 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -607,15 +607,7 @@ config MMC_DW
> help
> This selects support for the Synopsys DesignWare Mobile Storage IP
> block, this provides host support for SD and MMC interfaces, in both
> - PIO and external DMA modes.
> -
> -config MMC_DW_IDMAC
> - bool "Internal DMAC interface"
> - depends on MMC_DW
> - help
> - This selects support for the internal DMAC block within the Synopsys
> - Designware Mobile Storage IP block. This disables the external DMA
> - interface.
> + PIO, internal DMA mode and external DMA modes.
>
> config MMC_DW_PLTFM
> tristate "Synopsys Designware MCI Support as platform device"
> @@ -644,7 +636,6 @@ config MMC_DW_K3
> tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
> depends on MMC_DW
> select MMC_DW_PLTFM
> - select MMC_DW_IDMAC
> help
> This selects support for Hisilicon K3 SoC specific extensions to the
> Synopsys DesignWare Memory Card Interface driver. Select this option
> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
> index ec6dbcd..7e1d13b 100644
> --- a/drivers/mmc/host/dw_mmc-pltfm.c
> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
> @@ -59,6 +59,8 @@ int dw_mci_pltfm_register(struct platform_device *pdev,
> host->pdata = pdev->dev.platform_data;
>
> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + /* Get registers' physical base address */
> + host->phy_regs = (void *)(regs->start);
> host->regs = devm_ioremap_resource(&pdev->dev, regs);
> if (IS_ERR(host->regs))
> return PTR_ERR(host->regs);

Is this board specific code? If so, separate the patch.

> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index fcbf552..f943619 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -56,7 +56,6 @@
> #define DW_MCI_FREQ_MAX 200000000 /* unit: HZ */
> #define DW_MCI_FREQ_MIN 400000 /* unit: HZ */
>
> -#ifdef CONFIG_MMC_DW_IDMAC
> #define IDMAC_INT_CLR (SDMMC_IDMAC_INT_AI | SDMMC_IDMAC_INT_NI | \
> SDMMC_IDMAC_INT_CES | SDMMC_IDMAC_INT_DU | \
> SDMMC_IDMAC_INT_FBE | SDMMC_IDMAC_INT_RI | \
> @@ -102,7 +101,6 @@ struct idmac_desc {
>
> /* Each descriptor can transfer up to 4KB of data in chained mode */
> #define DW_MCI_DESC_DATA_LENGTH 0x1000
> -#endif /* CONFIG_MMC_DW_IDMAC */
>
> static bool dw_mci_reset(struct dw_mci *host);
> static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
> @@ -407,7 +405,6 @@ static int dw_mci_get_dma_dir(struct mmc_data *data)
> return DMA_FROM_DEVICE;
> }
>
> -#ifdef CONFIG_MMC_DW_IDMAC
> static void dw_mci_dma_cleanup(struct dw_mci *host)
> {
> struct mmc_data *data = host->data;
> @@ -445,12 +442,21 @@ static void dw_mci_idmac_stop_dma(struct dw_mci *host)
> mci_writel(host, BMOD, temp);
> }
>
> -static void dw_mci_idmac_complete_dma(struct dw_mci *host)
> +static void dw_mci_dmac_complete_dma(void *arg)
> {
> + struct dw_mci *host = arg;
> struct mmc_data *data = host->data;
>
> dev_vdbg(host->dev, "DMA complete\n");
>
> + if (host->use_dma == TRANS_MODE_EDMAC)
> + if (data && (data->flags & MMC_DATA_READ))

Combine one condition.

> + /* Invalidate cache after read */
> + dma_sync_sg_for_cpu(mmc_dev(host->cur_slot->mmc),
> + data->sg,
> + data->sg_len,
> + DMA_FROM_DEVICE);
> +
> host->dma_ops->cleanup(host);
>
> /*
> @@ -564,7 +570,7 @@ static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
> wmb(); /* drain writebuffer */
> }
>
> -static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
> +static int dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
> {
> u32 temp;
>
> @@ -589,6 +595,8 @@ static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>
> /* Start it running */
> mci_writel(host, PLDMND, 1);
> +
> + return 0;
> }
>
> static int dw_mci_idmac_init(struct dw_mci *host)
> @@ -669,10 +677,112 @@ static const struct dw_mci_dma_ops dw_mci_idmac_ops = {
> .init = dw_mci_idmac_init,
> .start = dw_mci_idmac_start_dma,
> .stop = dw_mci_idmac_stop_dma,
> - .complete = dw_mci_idmac_complete_dma,
> + .complete = dw_mci_dmac_complete_dma,
> + .cleanup = dw_mci_dma_cleanup,
> +};
> +
> +static void dw_mci_edmac_stop_dma(struct dw_mci *host)
> +{
> + dmaengine_terminate_all(host->dms->ch);
> +}
> +
> +static int dw_mci_edmac_start_dma(struct dw_mci *host,
> + unsigned int sg_len)
> +{
> + struct dma_slave_config cfg;
> + struct dma_async_tx_descriptor *desc = NULL;
> + struct scatterlist *sgl = host->data->sg;
> + const u32 mszs[] = {1, 4, 8, 16, 32, 64, 128, 256};
> + u32 sg_elems = host->data->sg_len;
> + u32 fifoth_val;
> + u32 fifo_offset = host->fifo_reg - host->regs;
> + int ret = 0;
> +
> + /* Set external dma config: burst size, burst width */
> + cfg.dst_addr = (dma_addr_t)(host->phy_regs + fifo_offset);

host->phy_regs is not assigned?

> + cfg.src_addr = cfg.dst_addr;
> + cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +
> + /* Match burst msize with external dma config */
> + fifoth_val = mci_readl(host, FIFOTH);
> + cfg.dst_maxburst = mszs[(fifoth_val >> 28) & 0x7];
> + cfg.src_maxburst = cfg.dst_maxburst;
> +
> + if (host->data->flags & MMC_DATA_WRITE)
> + cfg.direction = DMA_MEM_TO_DEV;
> + else /* MMC_DATA_READ */
> + cfg.direction = DMA_DEV_TO_MEM;
> +
> + ret = dmaengine_slave_config(host->dms->ch, &cfg);
> + if (ret) {
> + dev_err(host->dev, "Failed to config edmac.\n");
> + return -EBUSY;
> + }
> +
> + desc = dmaengine_prep_slave_sg(host->dms->ch, sgl,
> + sg_len, cfg.direction,
> + DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> + if (!desc) {
> + dev_err(host->dev, "Can't prepare slave sg.\n");
> + return -EBUSY;
> + }
> +
> + /* Set dw_mci_dmac_complete_dma as callback */
> + desc->callback = dw_mci_dmac_complete_dma;
> + desc->callback_param = (void *)host;
> + dmaengine_submit(desc);
> +
> + /* Flush cache before write */
> + if (host->data->flags & MMC_DATA_WRITE)
> + dma_sync_sg_for_device(mmc_dev(host->cur_slot->mmc), sgl,
> + sg_elems, DMA_TO_DEVICE);
> +
> + dma_async_issue_pending(host->dms->ch);
> +
> + return 0;
> +}
> +
> +static int dw_mci_edmac_init(struct dw_mci *host)
> +{
> + /* Request external dma channel */
> + host->dms = kzalloc(sizeof(struct dw_mci_dma_slave), GFP_KERNEL);
> + if (!host->dms)
> + return -ENOMEM;
> +
> + host->dms->ch = dma_request_slave_channel(host->dev, "rx-tx");
> + if (!host->dms->ch) {
> + dev_err(host->dev,
> + "Failed to get external DMA channel %d\n",
> + host->dms->ch->chan_id);
> + kfree(host->dms);
> + host->dms = NULL;
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static void dw_mci_edmac_exit(struct dw_mci *host)
> +{
> + if (host->dms) {
> + if (host->dms->ch) {
> + dma_release_channel(host->dms->ch);
> + host->dms->ch = NULL;
> + }
> + kfree(host->dms);
> + host->dms = NULL;
> + }
> +}
> +
> +static const struct dw_mci_dma_ops dw_mci_edmac_ops = {
> + .init = dw_mci_edmac_init,
> + .exit = dw_mci_edmac_exit,
> + .start = dw_mci_edmac_start_dma,
> + .stop = dw_mci_edmac_stop_dma,
> + .complete = dw_mci_dmac_complete_dma,
> .cleanup = dw_mci_dma_cleanup,
> };
> -#endif /* CONFIG_MMC_DW_IDMAC */
>
> static int dw_mci_pre_dma_transfer(struct dw_mci *host,
> struct mmc_data *data,
> @@ -752,7 +862,6 @@ static void dw_mci_post_req(struct mmc_host *mmc,
>
> static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
> {
> -#ifdef CONFIG_MMC_DW_IDMAC
> unsigned int blksz = data->blksz;
> const u32 mszs[] = {1, 4, 8, 16, 32, 64, 128, 256};
> u32 fifo_width = 1 << host->data_shift;
> @@ -760,6 +869,10 @@ static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
> u32 msize = 0, rx_wmark = 1, tx_wmark, tx_wmark_invers;
> int idx = ARRAY_SIZE(mszs) - 1;
>
> + /* pio should ship this scenario */
> + if (!host->use_dma)
> + return;
> +
> tx_wmark = (host->fifo_depth) / 2;
> tx_wmark_invers = host->fifo_depth - tx_wmark;
>
> @@ -788,7 +901,6 @@ static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
> done:
> fifoth_val = SDMMC_SET_FIFOTH(msize, rx_wmark, tx_wmark);
> mci_writel(host, FIFOTH, fifoth_val);
> -#endif
> }
>
> static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
> @@ -850,10 +962,12 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
>
> host->using_dma = 1;
>
> - dev_vdbg(host->dev,
> - "sd sg_cpu: %#lx sg_dma: %#lx sg_len: %d\n",
> - (unsigned long)host->sg_cpu, (unsigned long)host->sg_dma,
> - sg_len);
> + if (host->use_dma == TRANS_MODE_IDMAC)
> + dev_vdbg(host->dev,
> + "sd sg_cpu: %#lx sg_dma: %#lx sg_len: %d\n",
> + (unsigned long)host->sg_cpu,
> + (unsigned long)host->sg_dma,
> + sg_len);
>
> /*
> * Decide the MSIZE and RX/TX Watermark.
> @@ -875,7 +989,11 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
> mci_writel(host, INTMASK, temp);
> spin_unlock_irqrestore(&host->irq_lock, irqflags);
>
> - host->dma_ops->start(host, sg_len);
> + if (host->dma_ops->start(host, sg_len)) {
> + /* We can't do DMA */
> + dev_err(host->dev, "%s: failed to start DMA.\n", __func__);
> + return -ENODEV;
> + }
>
> return 0;
> }
> @@ -2343,15 +2461,17 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>
> }
>
> -#ifdef CONFIG_MMC_DW_IDMAC
> - /* Handle DMA interrupts */
> + if (host->use_dma != TRANS_MODE_IDMAC)
> + return IRQ_HANDLED;
> +
> + /* Handle IDMA interrupts */
> if (host->dma_64bit_address == 1) {
> pending = mci_readl(host, IDSTS64);
> if (pending & (SDMMC_IDMAC_INT_TI | SDMMC_IDMAC_INT_RI)) {
> mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_TI |
> SDMMC_IDMAC_INT_RI);
> mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
> - host->dma_ops->complete(host);
> + host->dma_ops->complete((void *)host);
> }
> } else {
> pending = mci_readl(host, IDSTS);
> @@ -2359,10 +2479,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> mci_writel(host, IDSTS, SDMMC_IDMAC_INT_TI |
> SDMMC_IDMAC_INT_RI);
> mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
> - host->dma_ops->complete(host);
> + host->dma_ops->complete((void *)host);
> }
> }
> -#endif
>
> return IRQ_HANDLED;
> }
> @@ -2471,13 +2590,21 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> goto err_host_allocated;
>
> /* Useful defaults if platform data is unset. */
> - if (host->use_dma) {
> + if (host->use_dma == TRANS_MODE_IDMAC) {
> mmc->max_segs = host->ring_size;
> mmc->max_blk_size = 65536;
> mmc->max_seg_size = 0x1000;
> mmc->max_req_size = mmc->max_seg_size * host->ring_size;
> mmc->max_blk_count = mmc->max_req_size / 512;
> + } else if (host->use_dma == TRANS_MODE_EDMAC) {
> + mmc->max_segs = 64;
> + mmc->max_blk_size = 65536;
> + mmc->max_blk_count = 65535;
> + mmc->max_req_size =
> + mmc->max_blk_size * mmc->max_blk_count;
> + mmc->max_seg_size = mmc->max_req_size;

Fix the indention

> } else {
> + /* TRANS_MODE_PIO */
> mmc->max_segs = 64;
> mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
> mmc->max_blk_count = 512;
> @@ -2517,35 +2644,66 @@ static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
> static void dw_mci_init_dma(struct dw_mci *host)
> {
> int addr_config;
> - /* Check ADDR_CONFIG bit in HCON to find IDMAC address bus width */
> - addr_config = (mci_readl(host, HCON) >> 27) & 0x01;
> -
> - if (addr_config == 1) {
> - /* host supports IDMAC in 64-bit address mode */
> - host->dma_64bit_address = 1;
> - dev_info(host->dev, "IDMAC supports 64-bit address mode.\n");
> - if (!dma_set_mask(host->dev, DMA_BIT_MASK(64)))
> - dma_set_coherent_mask(host->dev, DMA_BIT_MASK(64));
> - } else {
> - /* host supports IDMAC in 32-bit address mode */
> - host->dma_64bit_address = 0;
> - dev_info(host->dev, "IDMAC supports 32-bit address mode.\n");
> - }
> + int trans_mode;
> + struct device *dev = host->dev;
> + struct device_node *np = dev->of_node;
>
> - /* Alloc memory for sg translation */
> - host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
> - &host->sg_dma, GFP_KERNEL);
> - if (!host->sg_cpu) {
> - dev_err(host->dev, "%s: could not alloc DMA memory\n",
> - __func__);
> + /* Check tansfer mode */
> + trans_mode = SDMMC_GET_TRANS_MODE(mci_readl(host, HCON));
> + if (trans_mode == 0) {
> + trans_mode = TRANS_MODE_IDMAC;
> + } else if (trans_mode == 1 || trans_mode == 2) {
> + trans_mode = TRANS_MODE_EDMAC;
> + } else {
> + trans_mode = TRANS_MODE_PIO;

what are trans_mode "0, 1, 2"?
(00 - none) is NO-DMA interface, isn't? is it IDMAC mode?

> goto no_dma;
> }
>
> /* Determine which DMA interface to use */
> -#ifdef CONFIG_MMC_DW_IDMAC
> - host->dma_ops = &dw_mci_idmac_ops;
> - dev_info(host->dev, "Using internal DMA controller.\n");
> -#endif
> + if (trans_mode == TRANS_MODE_IDMAC) {
> + /*
> + * Check ADDR_CONFIG bit in HCON to find
> + * IDMAC address bus width
> + */
> + addr_config = SDMMC_GET_ADDR_CONFIG(mci_readl(host, HCON));
> +
> + if (addr_config == 1) {
> + /* host supports IDMAC in 64-bit address mode */
> + host->dma_64bit_address = 1;
> + dev_info(host->dev,
> + "IDMAC supports 64-bit address mode.\n");
> + if (!dma_set_mask(host->dev, DMA_BIT_MASK(64)))
> + dma_set_coherent_mask(host->dev,
> + DMA_BIT_MASK(64));
> + } else {
> + /* host supports IDMAC in 32-bit address mode */
> + host->dma_64bit_address = 0;
> + dev_info(host->dev,
> + "IDMAC supports 32-bit address mode.\n");
> + }
> +
> + /* Alloc memory for sg translation */
> + host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
> + &host->sg_dma, GFP_KERNEL);
> + if (!host->sg_cpu) {
> + dev_err(host->dev,
> + "%s: could not alloc DMA memory\n",
> + __func__);
> + goto no_dma;
> + }
> +
> + host->dma_ops = &dw_mci_idmac_ops;
> + dev_info(host->dev, "Using internal DMA controller.\n");
> + } else {
> + /* TRANS_MODE_EDMAC: check dma bindings again */
> + if ((of_property_count_strings(np, "dma-names") < 0) ||
> + (!of_find_property(np, "dmas", NULL))) {
> + trans_mode = TRANS_MODE_PIO;
> + goto no_dma;
> + }
> + host->dma_ops = &dw_mci_edmac_ops;
> + dev_info(host->dev, "Using external DMA controller.\n");
> + }
>
> if (!host->dma_ops)
> goto no_dma;
> @@ -2562,12 +2720,12 @@ static void dw_mci_init_dma(struct dw_mci *host)
> goto no_dma;
> }
>
> - host->use_dma = 1;
> + host->use_dma = trans_mode;

Also confuse, if trans_mode is assigned host->use_dma, can mode value be directly assigned to host->use_dma?

trans_mode = TRAMS_MODE_PIO;
host->use_dma = trans_mode;

-> host->use_dma = TRAMS_MODE_PIO;

Then trans_mode can be removed.

> return;
>
> no_dma:
> dev_info(host->dev, "Using PIO mode.\n");
> - host->use_dma = 0;
> + host->use_dma = trans_mode;
> }
>
> static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset)
> @@ -2650,10 +2808,9 @@ static bool dw_mci_reset(struct dw_mci *host)
> }
> }
>
> -#if IS_ENABLED(CONFIG_MMC_DW_IDMAC)
> - /* It is also recommended that we reset and reprogram idmac */
> - dw_mci_idmac_reset(host);
> -#endif
> + if (host->use_dma == TRANS_MODE_IDMAC)
> + /* It is also recommended that we reset and reprogram idmac */
> + dw_mci_idmac_reset(host);
>
> ret = true;
>
> @@ -2890,7 +3047,7 @@ int dw_mci_probe(struct dw_mci *host)
> * Get the host data width - this assumes that HCON has been set with
> * the correct values.
> */
> - i = (mci_readl(host, HCON) >> 7) & 0x7;
> + i = SDMMC_GET_HDATA_WIDTH(mci_readl(host, HCON));

This is not related with supporting external dma interface.
Separate this.

> if (!i) {
> host->push_data = dw_mci_push_data16;
> host->pull_data = dw_mci_pull_data16;
> @@ -2972,7 +3129,7 @@ int dw_mci_probe(struct dw_mci *host)
> if (host->pdata->num_slots)
> host->num_slots = host->pdata->num_slots;
> else
> - host->num_slots = ((mci_readl(host, HCON) >> 1) & 0x1F) + 1;
> + host->num_slots = SDMMC_GET_SLOT_NUM(mci_readl(host, HCON));

Ditto. (with above.)

>
> /*
> * Enable interrupts for command done, data over, data empty,
> @@ -3067,6 +3224,9 @@ EXPORT_SYMBOL(dw_mci_remove);
> */
> int dw_mci_suspend(struct dw_mci *host)
> {
> + if (host->use_dma && host->dma_ops->exit)
> + host->dma_ops->exit(host);
> +
> return 0;
> }
> EXPORT_SYMBOL(dw_mci_suspend);
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 8ce4674..c453e94 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -148,6 +148,11 @@
> #define SDMMC_SET_FIFOTH(m, r, t) (((m) & 0x7) << 28 | \
> ((r) & 0xFFF) << 16 | \
> ((t) & 0xFFF))
> +/* HCON register defines */
> +#define SDMMC_GET_SLOT_NUM(x) ((((x)>>1) & 0x1F) + 1)
> +#define SDMMC_GET_HDATA_WIDTH(x) (((x)>>7) & 0x7)
> +#define SDMMC_GET_TRANS_MODE(x) (((x)>>16) & 0x3)
> +#define SDMMC_GET_ADDR_CONFIG(x) (((x)>>27) & 0x1)
> /* Internal DMAC interrupt defines */
> #define SDMMC_IDMAC_INT_AI BIT(9)
> #define SDMMC_IDMAC_INT_NI BIT(8)
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index c846f42..6a2b83c 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -16,6 +16,7 @@
>
> #include <linux/scatterlist.h>
> #include <linux/mmc/core.h>
> +#include <linux/dmaengine.h>
>
> #define MAX_MCI_SLOTS 2
>
> @@ -40,6 +41,17 @@ enum {
>
> struct mmc_data;
>
> +enum {
> + TRANS_MODE_PIO = 0,
> + TRANS_MODE_IDMAC,
> + TRANS_MODE_EDMAC
> +};
> +
> +struct dw_mci_dma_slave {
> + struct dma_chan *ch;
> + enum dma_transfer_direction direction;
> +};
> +
> /**
> * struct dw_mci - MMC controller state shared between all slots
> * @lock: Spinlock protecting the queue and associated data.
> @@ -154,11 +166,16 @@ struct dw_mci {
> dma_addr_t sg_dma;
> void *sg_cpu;
> const struct dw_mci_dma_ops *dma_ops;
> -#ifdef CONFIG_MMC_DW_IDMAC
> + /* For idmac */
> unsigned int ring_size;
> -#else
> +
> + /* For edmac */
> + struct dw_mci_dma_slave *dms;
> + /* Registers's physical base address */
> + void *phy_regs;
> +
> struct dw_mci_dma_data *dma_data;
> -#endif
> +

On ulf's repository, this point should be conflicted.

Best Regards,
Jaehoon Chung

> u32 cmd_status;
> u32 data_status;
> u32 stop_cmdr;
> @@ -212,8 +229,8 @@ struct dw_mci {
> struct dw_mci_dma_ops {
> /* DMA Ops */
> int (*init)(struct dw_mci *host);
> - void (*start)(struct dw_mci *host, unsigned int sg_len);
> - void (*complete)(struct dw_mci *host);
> + int (*start)(struct dw_mci *host, unsigned int sg_len);
> + void (*complete)(void *host);
> void (*stop)(struct dw_mci *host);
> void (*cleanup)(struct dw_mci *host);
> void (*exit)(struct dw_mci *host);
>

2015-08-21 06:32:28

by Shawn Lin

[permalink] [raw]
Subject: Re: [RFC PATCH v6 1/9] mmc: dw_mmc: Add external dma interface support

On 2015/8/21 14:27, Jaehoon Chung wrote:
> Hi, Shawn.
>
> Is this based on Ulf's repository?


no, it's based on "https://github.com/jh80chung/dw-mmc.git
tags/dw-mmc-for-ulf-v4.2" :)

>
> On 08/20/2015 05:43 PM, Shawn Lin wrote:
>> DesignWare MMC Controller can supports two types of DMA
>> mode: external dma and internal dma. We get a RK312x platform
>> integrated dw_mmc and ARM pl330 dma controller. This patch add
>> edmac ops to support these platforms. I've tested it on RK31xx
>> platform with edmac mode and RK3288 platform with idmac mode.
>>
>> Signed-off-by: Shawn Lin <[email protected]>
>>
>> ---
>>
>> Changes in v6:
>> - add trans_mode condition for IDMAC initialization
>> suggested by Heiko
>> - re-test my patch on rk3188 platform and update commit msg
>> - update performance of pio vs edmac in cover letter
>>
>> Changes in v5:
>> - add the title of cover letter
>> - fix typo of comment
>> - add macro for reading HCON register
>> - add "Acked-by: Krzysztof Kozlowski <[email protected]>" for exynos_defconfig patch
>> - add "Acked-by: Vineet Gupta <[email protected]>" for axs10x_defconfig patch
>> - add "Acked-by: Govindraj Raja <[email protected]>" and
>> "Acked-by: Ralf Baechle <[email protected]>" for pistachio_defconfig patch
>> - add "Acked-by: Joachim Eastwood <[email protected]>" for lpc18xx_defconfig patch
>> - add "Acked-by: Wei Xu <[email protected]>" for hisi_defconfig patch
>> - rebase on "https://github.com/jh80chung/dw-mmc.git tags/dw-mmc-for-ulf-v4.2" for merging easily
>>
>> Changes in v4:
>> - remove "host->trans_mode" and use "host->use_dma" to indicate
>> transfer mode.
>> - remove all bt-bindings' changes since we don't need new properities.
>> - check transfer mode at runtime by reading HCON reg
>> - spilt defconfig changes for each sub-architecture
>> - fix the title of cover letter
>> - reuse some code for reducing code size
>>
>> Changes in v3:
>> - choose transfer mode at runtime
>> - remove all CONFIG_MMC_DW_IDMAC config option
>> - add supports-idmac property for some platforms
>>
>> Changes in v2:
>> - Fix typo of dev_info msg
>> - remove unused dmach from declaration of dw_mci_dma_slave
>>
>> drivers/mmc/host/Kconfig | 11 +-
>> drivers/mmc/host/dw_mmc-pltfm.c | 2 +
>> drivers/mmc/host/dw_mmc.c | 264 ++++++++++++++++++++++++++++++++--------
>> drivers/mmc/host/dw_mmc.h | 5 +
>> include/linux/mmc/dw_mmc.h | 27 +++-
>> 5 files changed, 242 insertions(+), 67 deletions(-)
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 6a0f9c7..a86c0eb 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -607,15 +607,7 @@ config MMC_DW
>> help
>> This selects support for the Synopsys DesignWare Mobile Storage IP
>> block, this provides host support for SD and MMC interfaces, in both
>> - PIO and external DMA modes.
>> -
>> -config MMC_DW_IDMAC
>> - bool "Internal DMAC interface"
>> - depends on MMC_DW
>> - help
>> - This selects support for the internal DMAC block within the Synopsys
>> - Designware Mobile Storage IP block. This disables the external DMA
>> - interface.
>> + PIO, internal DMA mode and external DMA modes.
>>
>> config MMC_DW_PLTFM
>> tristate "Synopsys Designware MCI Support as platform device"
>> @@ -644,7 +636,6 @@ config MMC_DW_K3
>> tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
>> depends on MMC_DW
>> select MMC_DW_PLTFM
>> - select MMC_DW_IDMAC
>> help
>> This selects support for Hisilicon K3 SoC specific extensions to the
>> Synopsys DesignWare Memory Card Interface driver. Select this option
>> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
>> index ec6dbcd..7e1d13b 100644
>> --- a/drivers/mmc/host/dw_mmc-pltfm.c
>> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
>> @@ -59,6 +59,8 @@ int dw_mci_pltfm_register(struct platform_device *pdev,
>> host->pdata = pdev->dev.platform_data;
>>
>> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + /* Get registers' physical base address */
>> + host->phy_regs = (void *)(regs->start);
>> host->regs = devm_ioremap_resource(&pdev->dev, regs);
>> if (IS_ERR(host->regs))
>> return PTR_ERR(host->regs);
>
> Is this board specific code? If so, separate the patch.
>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index fcbf552..f943619 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -56,7 +56,6 @@
>> #define DW_MCI_FREQ_MAX 200000000 /* unit: HZ */
>> #define DW_MCI_FREQ_MIN 400000 /* unit: HZ */
>>
>> -#ifdef CONFIG_MMC_DW_IDMAC
>> #define IDMAC_INT_CLR (SDMMC_IDMAC_INT_AI | SDMMC_IDMAC_INT_NI | \
>> SDMMC_IDMAC_INT_CES | SDMMC_IDMAC_INT_DU | \
>> SDMMC_IDMAC_INT_FBE | SDMMC_IDMAC_INT_RI | \
>> @@ -102,7 +101,6 @@ struct idmac_desc {
>>
>> /* Each descriptor can transfer up to 4KB of data in chained mode */
>> #define DW_MCI_DESC_DATA_LENGTH 0x1000
>> -#endif /* CONFIG_MMC_DW_IDMAC */
>>
>> static bool dw_mci_reset(struct dw_mci *host);
>> static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>> @@ -407,7 +405,6 @@ static int dw_mci_get_dma_dir(struct mmc_data *data)
>> return DMA_FROM_DEVICE;
>> }
>>
>> -#ifdef CONFIG_MMC_DW_IDMAC
>> static void dw_mci_dma_cleanup(struct dw_mci *host)
>> {
>> struct mmc_data *data = host->data;
>> @@ -445,12 +442,21 @@ static void dw_mci_idmac_stop_dma(struct dw_mci *host)
>> mci_writel(host, BMOD, temp);
>> }
>>
>> -static void dw_mci_idmac_complete_dma(struct dw_mci *host)
>> +static void dw_mci_dmac_complete_dma(void *arg)
>> {
>> + struct dw_mci *host = arg;
>> struct mmc_data *data = host->data;
>>
>> dev_vdbg(host->dev, "DMA complete\n");
>>
>> + if (host->use_dma == TRANS_MODE_EDMAC)
>> + if (data && (data->flags & MMC_DATA_READ))
>
> Combine one condition.
>
>> + /* Invalidate cache after read */
>> + dma_sync_sg_for_cpu(mmc_dev(host->cur_slot->mmc),
>> + data->sg,
>> + data->sg_len,
>> + DMA_FROM_DEVICE);
>> +
>> host->dma_ops->cleanup(host);
>>
>> /*
>> @@ -564,7 +570,7 @@ static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
>> wmb(); /* drain writebuffer */
>> }
>>
>> -static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>> +static int dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>> {
>> u32 temp;
>>
>> @@ -589,6 +595,8 @@ static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>>
>> /* Start it running */
>> mci_writel(host, PLDMND, 1);
>> +
>> + return 0;
>> }
>>
>> static int dw_mci_idmac_init(struct dw_mci *host)
>> @@ -669,10 +677,112 @@ static const struct dw_mci_dma_ops dw_mci_idmac_ops = {
>> .init = dw_mci_idmac_init,
>> .start = dw_mci_idmac_start_dma,
>> .stop = dw_mci_idmac_stop_dma,
>> - .complete = dw_mci_idmac_complete_dma,
>> + .complete = dw_mci_dmac_complete_dma,
>> + .cleanup = dw_mci_dma_cleanup,
>> +};
>> +
>> +static void dw_mci_edmac_stop_dma(struct dw_mci *host)
>> +{
>> + dmaengine_terminate_all(host->dms->ch);
>> +}
>> +
>> +static int dw_mci_edmac_start_dma(struct dw_mci *host,
>> + unsigned int sg_len)
>> +{
>> + struct dma_slave_config cfg;
>> + struct dma_async_tx_descriptor *desc = NULL;
>> + struct scatterlist *sgl = host->data->sg;
>> + const u32 mszs[] = {1, 4, 8, 16, 32, 64, 128, 256};
>> + u32 sg_elems = host->data->sg_len;
>> + u32 fifoth_val;
>> + u32 fifo_offset = host->fifo_reg - host->regs;
>> + int ret = 0;
>> +
>> + /* Set external dma config: burst size, burst width */
>> + cfg.dst_addr = (dma_addr_t)(host->phy_regs + fifo_offset);
>
> host->phy_regs is not assigned?
>
>> + cfg.src_addr = cfg.dst_addr;
>> + cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>> + cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>> +
>> + /* Match burst msize with external dma config */
>> + fifoth_val = mci_readl(host, FIFOTH);
>> + cfg.dst_maxburst = mszs[(fifoth_val >> 28) & 0x7];
>> + cfg.src_maxburst = cfg.dst_maxburst;
>> +
>> + if (host->data->flags & MMC_DATA_WRITE)
>> + cfg.direction = DMA_MEM_TO_DEV;
>> + else /* MMC_DATA_READ */
>> + cfg.direction = DMA_DEV_TO_MEM;
>> +
>> + ret = dmaengine_slave_config(host->dms->ch, &cfg);
>> + if (ret) {
>> + dev_err(host->dev, "Failed to config edmac.\n");
>> + return -EBUSY;
>> + }
>> +
>> + desc = dmaengine_prep_slave_sg(host->dms->ch, sgl,
>> + sg_len, cfg.direction,
>> + DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>> + if (!desc) {
>> + dev_err(host->dev, "Can't prepare slave sg.\n");
>> + return -EBUSY;
>> + }
>> +
>> + /* Set dw_mci_dmac_complete_dma as callback */
>> + desc->callback = dw_mci_dmac_complete_dma;
>> + desc->callback_param = (void *)host;
>> + dmaengine_submit(desc);
>> +
>> + /* Flush cache before write */
>> + if (host->data->flags & MMC_DATA_WRITE)
>> + dma_sync_sg_for_device(mmc_dev(host->cur_slot->mmc), sgl,
>> + sg_elems, DMA_TO_DEVICE);
>> +
>> + dma_async_issue_pending(host->dms->ch);
>> +
>> + return 0;
>> +}
>> +
>> +static int dw_mci_edmac_init(struct dw_mci *host)
>> +{
>> + /* Request external dma channel */
>> + host->dms = kzalloc(sizeof(struct dw_mci_dma_slave), GFP_KERNEL);
>> + if (!host->dms)
>> + return -ENOMEM;
>> +
>> + host->dms->ch = dma_request_slave_channel(host->dev, "rx-tx");
>> + if (!host->dms->ch) {
>> + dev_err(host->dev,
>> + "Failed to get external DMA channel %d\n",
>> + host->dms->ch->chan_id);
>> + kfree(host->dms);
>> + host->dms = NULL;
>> + return -ENXIO;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void dw_mci_edmac_exit(struct dw_mci *host)
>> +{
>> + if (host->dms) {
>> + if (host->dms->ch) {
>> + dma_release_channel(host->dms->ch);
>> + host->dms->ch = NULL;
>> + }
>> + kfree(host->dms);
>> + host->dms = NULL;
>> + }
>> +}
>> +
>> +static const struct dw_mci_dma_ops dw_mci_edmac_ops = {
>> + .init = dw_mci_edmac_init,
>> + .exit = dw_mci_edmac_exit,
>> + .start = dw_mci_edmac_start_dma,
>> + .stop = dw_mci_edmac_stop_dma,
>> + .complete = dw_mci_dmac_complete_dma,
>> .cleanup = dw_mci_dma_cleanup,
>> };
>> -#endif /* CONFIG_MMC_DW_IDMAC */
>>
>> static int dw_mci_pre_dma_transfer(struct dw_mci *host,
>> struct mmc_data *data,
>> @@ -752,7 +862,6 @@ static void dw_mci_post_req(struct mmc_host *mmc,
>>
>> static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
>> {
>> -#ifdef CONFIG_MMC_DW_IDMAC
>> unsigned int blksz = data->blksz;
>> const u32 mszs[] = {1, 4, 8, 16, 32, 64, 128, 256};
>> u32 fifo_width = 1 << host->data_shift;
>> @@ -760,6 +869,10 @@ static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
>> u32 msize = 0, rx_wmark = 1, tx_wmark, tx_wmark_invers;
>> int idx = ARRAY_SIZE(mszs) - 1;
>>
>> + /* pio should ship this scenario */
>> + if (!host->use_dma)
>> + return;
>> +
>> tx_wmark = (host->fifo_depth) / 2;
>> tx_wmark_invers = host->fifo_depth - tx_wmark;
>>
>> @@ -788,7 +901,6 @@ static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
>> done:
>> fifoth_val = SDMMC_SET_FIFOTH(msize, rx_wmark, tx_wmark);
>> mci_writel(host, FIFOTH, fifoth_val);
>> -#endif
>> }
>>
>> static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>> @@ -850,10 +962,12 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
>>
>> host->using_dma = 1;
>>
>> - dev_vdbg(host->dev,
>> - "sd sg_cpu: %#lx sg_dma: %#lx sg_len: %d\n",
>> - (unsigned long)host->sg_cpu, (unsigned long)host->sg_dma,
>> - sg_len);
>> + if (host->use_dma == TRANS_MODE_IDMAC)
>> + dev_vdbg(host->dev,
>> + "sd sg_cpu: %#lx sg_dma: %#lx sg_len: %d\n",
>> + (unsigned long)host->sg_cpu,
>> + (unsigned long)host->sg_dma,
>> + sg_len);
>>
>> /*
>> * Decide the MSIZE and RX/TX Watermark.
>> @@ -875,7 +989,11 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
>> mci_writel(host, INTMASK, temp);
>> spin_unlock_irqrestore(&host->irq_lock, irqflags);
>>
>> - host->dma_ops->start(host, sg_len);
>> + if (host->dma_ops->start(host, sg_len)) {
>> + /* We can't do DMA */
>> + dev_err(host->dev, "%s: failed to start DMA.\n", __func__);
>> + return -ENODEV;
>> + }
>>
>> return 0;
>> }
>> @@ -2343,15 +2461,17 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>>
>> }
>>
>> -#ifdef CONFIG_MMC_DW_IDMAC
>> - /* Handle DMA interrupts */
>> + if (host->use_dma != TRANS_MODE_IDMAC)
>> + return IRQ_HANDLED;
>> +
>> + /* Handle IDMA interrupts */
>> if (host->dma_64bit_address == 1) {
>> pending = mci_readl(host, IDSTS64);
>> if (pending & (SDMMC_IDMAC_INT_TI | SDMMC_IDMAC_INT_RI)) {
>> mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_TI |
>> SDMMC_IDMAC_INT_RI);
>> mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
>> - host->dma_ops->complete(host);
>> + host->dma_ops->complete((void *)host);
>> }
>> } else {
>> pending = mci_readl(host, IDSTS);
>> @@ -2359,10 +2479,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>> mci_writel(host, IDSTS, SDMMC_IDMAC_INT_TI |
>> SDMMC_IDMAC_INT_RI);
>> mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
>> - host->dma_ops->complete(host);
>> + host->dma_ops->complete((void *)host);
>> }
>> }
>> -#endif
>>
>> return IRQ_HANDLED;
>> }
>> @@ -2471,13 +2590,21 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>> goto err_host_allocated;
>>
>> /* Useful defaults if platform data is unset. */
>> - if (host->use_dma) {
>> + if (host->use_dma == TRANS_MODE_IDMAC) {
>> mmc->max_segs = host->ring_size;
>> mmc->max_blk_size = 65536;
>> mmc->max_seg_size = 0x1000;
>> mmc->max_req_size = mmc->max_seg_size * host->ring_size;
>> mmc->max_blk_count = mmc->max_req_size / 512;
>> + } else if (host->use_dma == TRANS_MODE_EDMAC) {
>> + mmc->max_segs = 64;
>> + mmc->max_blk_size = 65536;
>> + mmc->max_blk_count = 65535;
>> + mmc->max_req_size =
>> + mmc->max_blk_size * mmc->max_blk_count;
>> + mmc->max_seg_size = mmc->max_req_size;
>
> Fix the indention
>
>> } else {
>> + /* TRANS_MODE_PIO */
>> mmc->max_segs = 64;
>> mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
>> mmc->max_blk_count = 512;
>> @@ -2517,35 +2644,66 @@ static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
>> static void dw_mci_init_dma(struct dw_mci *host)
>> {
>> int addr_config;
>> - /* Check ADDR_CONFIG bit in HCON to find IDMAC address bus width */
>> - addr_config = (mci_readl(host, HCON) >> 27) & 0x01;
>> -
>> - if (addr_config == 1) {
>> - /* host supports IDMAC in 64-bit address mode */
>> - host->dma_64bit_address = 1;
>> - dev_info(host->dev, "IDMAC supports 64-bit address mode.\n");
>> - if (!dma_set_mask(host->dev, DMA_BIT_MASK(64)))
>> - dma_set_coherent_mask(host->dev, DMA_BIT_MASK(64));
>> - } else {
>> - /* host supports IDMAC in 32-bit address mode */
>> - host->dma_64bit_address = 0;
>> - dev_info(host->dev, "IDMAC supports 32-bit address mode.\n");
>> - }
>> + int trans_mode;
>> + struct device *dev = host->dev;
>> + struct device_node *np = dev->of_node;
>>
>> - /* Alloc memory for sg translation */
>> - host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
>> - &host->sg_dma, GFP_KERNEL);
>> - if (!host->sg_cpu) {
>> - dev_err(host->dev, "%s: could not alloc DMA memory\n",
>> - __func__);
>> + /* Check tansfer mode */
>> + trans_mode = SDMMC_GET_TRANS_MODE(mci_readl(host, HCON));
>> + if (trans_mode == 0) {
>> + trans_mode = TRANS_MODE_IDMAC;
>> + } else if (trans_mode == 1 || trans_mode == 2) {
>> + trans_mode = TRANS_MODE_EDMAC;
>> + } else {
>> + trans_mode = TRANS_MODE_PIO;
>
> what are trans_mode "0, 1, 2"?
> (00 - none) is NO-DMA interface, isn't? is it IDMAC mode?
>
>> goto no_dma;
>> }
>>
>> /* Determine which DMA interface to use */
>> -#ifdef CONFIG_MMC_DW_IDMAC
>> - host->dma_ops = &dw_mci_idmac_ops;
>> - dev_info(host->dev, "Using internal DMA controller.\n");
>> -#endif
>> + if (trans_mode == TRANS_MODE_IDMAC) {
>> + /*
>> + * Check ADDR_CONFIG bit in HCON to find
>> + * IDMAC address bus width
>> + */
>> + addr_config = SDMMC_GET_ADDR_CONFIG(mci_readl(host, HCON));
>> +
>> + if (addr_config == 1) {
>> + /* host supports IDMAC in 64-bit address mode */
>> + host->dma_64bit_address = 1;
>> + dev_info(host->dev,
>> + "IDMAC supports 64-bit address mode.\n");
>> + if (!dma_set_mask(host->dev, DMA_BIT_MASK(64)))
>> + dma_set_coherent_mask(host->dev,
>> + DMA_BIT_MASK(64));
>> + } else {
>> + /* host supports IDMAC in 32-bit address mode */
>> + host->dma_64bit_address = 0;
>> + dev_info(host->dev,
>> + "IDMAC supports 32-bit address mode.\n");
>> + }
>> +
>> + /* Alloc memory for sg translation */
>> + host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
>> + &host->sg_dma, GFP_KERNEL);
>> + if (!host->sg_cpu) {
>> + dev_err(host->dev,
>> + "%s: could not alloc DMA memory\n",
>> + __func__);
>> + goto no_dma;
>> + }
>> +
>> + host->dma_ops = &dw_mci_idmac_ops;
>> + dev_info(host->dev, "Using internal DMA controller.\n");
>> + } else {
>> + /* TRANS_MODE_EDMAC: check dma bindings again */
>> + if ((of_property_count_strings(np, "dma-names") < 0) ||
>> + (!of_find_property(np, "dmas", NULL))) {
>> + trans_mode = TRANS_MODE_PIO;
>> + goto no_dma;
>> + }
>> + host->dma_ops = &dw_mci_edmac_ops;
>> + dev_info(host->dev, "Using external DMA controller.\n");
>> + }
>>
>> if (!host->dma_ops)
>> goto no_dma;
>> @@ -2562,12 +2720,12 @@ static void dw_mci_init_dma(struct dw_mci *host)
>> goto no_dma;
>> }
>>
>> - host->use_dma = 1;
>> + host->use_dma = trans_mode;
>
> Also confuse, if trans_mode is assigned host->use_dma, can mode value be directly assigned to host->use_dma?
>
> trans_mode = TRAMS_MODE_PIO;
> host->use_dma = trans_mode;
>
> -> host->use_dma = TRAMS_MODE_PIO;
>
> Then trans_mode can be removed.
>
>> return;
>>
>> no_dma:
>> dev_info(host->dev, "Using PIO mode.\n");
>> - host->use_dma = 0;
>> + host->use_dma = trans_mode;
>> }
>>
>> static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset)
>> @@ -2650,10 +2808,9 @@ static bool dw_mci_reset(struct dw_mci *host)
>> }
>> }
>>
>> -#if IS_ENABLED(CONFIG_MMC_DW_IDMAC)
>> - /* It is also recommended that we reset and reprogram idmac */
>> - dw_mci_idmac_reset(host);
>> -#endif
>> + if (host->use_dma == TRANS_MODE_IDMAC)
>> + /* It is also recommended that we reset and reprogram idmac */
>> + dw_mci_idmac_reset(host);
>>
>> ret = true;
>>
>> @@ -2890,7 +3047,7 @@ int dw_mci_probe(struct dw_mci *host)
>> * Get the host data width - this assumes that HCON has been set with
>> * the correct values.
>> */
>> - i = (mci_readl(host, HCON) >> 7) & 0x7;
>> + i = SDMMC_GET_HDATA_WIDTH(mci_readl(host, HCON));
>
> This is not related with supporting external dma interface.
> Separate this.
>
>> if (!i) {
>> host->push_data = dw_mci_push_data16;
>> host->pull_data = dw_mci_pull_data16;
>> @@ -2972,7 +3129,7 @@ int dw_mci_probe(struct dw_mci *host)
>> if (host->pdata->num_slots)
>> host->num_slots = host->pdata->num_slots;
>> else
>> - host->num_slots = ((mci_readl(host, HCON) >> 1) & 0x1F) + 1;
>> + host->num_slots = SDMMC_GET_SLOT_NUM(mci_readl(host, HCON));
>
> Ditto. (with above.)
>
>>
>> /*
>> * Enable interrupts for command done, data over, data empty,
>> @@ -3067,6 +3224,9 @@ EXPORT_SYMBOL(dw_mci_remove);
>> */
>> int dw_mci_suspend(struct dw_mci *host)
>> {
>> + if (host->use_dma && host->dma_ops->exit)
>> + host->dma_ops->exit(host);
>> +
>> return 0;
>> }
>> EXPORT_SYMBOL(dw_mci_suspend);
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 8ce4674..c453e94 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -148,6 +148,11 @@
>> #define SDMMC_SET_FIFOTH(m, r, t) (((m) & 0x7) << 28 | \
>> ((r) & 0xFFF) << 16 | \
>> ((t) & 0xFFF))
>> +/* HCON register defines */
>> +#define SDMMC_GET_SLOT_NUM(x) ((((x)>>1) & 0x1F) + 1)
>> +#define SDMMC_GET_HDATA_WIDTH(x) (((x)>>7) & 0x7)
>> +#define SDMMC_GET_TRANS_MODE(x) (((x)>>16) & 0x3)
>> +#define SDMMC_GET_ADDR_CONFIG(x) (((x)>>27) & 0x1)
>> /* Internal DMAC interrupt defines */
>> #define SDMMC_IDMAC_INT_AI BIT(9)
>> #define SDMMC_IDMAC_INT_NI BIT(8)
>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>> index c846f42..6a2b83c 100644
>> --- a/include/linux/mmc/dw_mmc.h
>> +++ b/include/linux/mmc/dw_mmc.h
>> @@ -16,6 +16,7 @@
>>
>> #include <linux/scatterlist.h>
>> #include <linux/mmc/core.h>
>> +#include <linux/dmaengine.h>
>>
>> #define MAX_MCI_SLOTS 2
>>
>> @@ -40,6 +41,17 @@ enum {
>>
>> struct mmc_data;
>>
>> +enum {
>> + TRANS_MODE_PIO = 0,
>> + TRANS_MODE_IDMAC,
>> + TRANS_MODE_EDMAC
>> +};
>> +
>> +struct dw_mci_dma_slave {
>> + struct dma_chan *ch;
>> + enum dma_transfer_direction direction;
>> +};
>> +
>> /**
>> * struct dw_mci - MMC controller state shared between all slots
>> * @lock: Spinlock protecting the queue and associated data.
>> @@ -154,11 +166,16 @@ struct dw_mci {
>> dma_addr_t sg_dma;
>> void *sg_cpu;
>> const struct dw_mci_dma_ops *dma_ops;
>> -#ifdef CONFIG_MMC_DW_IDMAC
>> + /* For idmac */
>> unsigned int ring_size;
>> -#else
>> +
>> + /* For edmac */
>> + struct dw_mci_dma_slave *dms;
>> + /* Registers's physical base address */
>> + void *phy_regs;
>> +
>> struct dw_mci_dma_data *dma_data;
>> -#endif
>> +
>
> On ulf's repository, this point should be conflicted.
>
> Best Regards,
> Jaehoon Chung
>
>> u32 cmd_status;
>> u32 data_status;
>> u32 stop_cmdr;
>> @@ -212,8 +229,8 @@ struct dw_mci {
>> struct dw_mci_dma_ops {
>> /* DMA Ops */
>> int (*init)(struct dw_mci *host);
>> - void (*start)(struct dw_mci *host, unsigned int sg_len);
>> - void (*complete)(struct dw_mci *host);
>> + int (*start)(struct dw_mci *host, unsigned int sg_len);
>> + void (*complete)(void *host);
>> void (*stop)(struct dw_mci *host);
>> void (*cleanup)(struct dw_mci *host);
>> void (*exit)(struct dw_mci *host);
>>
>
>
>
>


--
Best Regards
Shawn Lin

2015-08-21 06:35:51

by Jaehoon Chung

[permalink] [raw]
Subject: Re: [RFC PATCH v6 1/9] mmc: dw_mmc: Add external dma interface support

On 08/21/2015 03:30 PM, Shawn Lin wrote:
> On 2015/8/21 14:27, Jaehoon Chung wrote:
>> Hi, Shawn.
>>
>> Is this based on Ulf's repository?
>
>
> no, it's based on "https://github.com/jh80chung/dw-mmc.git tags/dw-mmc-for-ulf-v4.2" :)

Oh..I will rebase to Ulf's next branch on this weekend.
Then could you rebase this patch? And i added more comments at below.. :)

Best Regards,
Jaehoon Chung

>
>>
>> On 08/20/2015 05:43 PM, Shawn Lin wrote:
>>> DesignWare MMC Controller can supports two types of DMA
>>> mode: external dma and internal dma. We get a RK312x platform
>>> integrated dw_mmc and ARM pl330 dma controller. This patch add
>>> edmac ops to support these platforms. I've tested it on RK31xx
>>> platform with edmac mode and RK3288 platform with idmac mode.
>>>
>>> Signed-off-by: Shawn Lin <[email protected]>
>>>
>>> ---
>>>
>>> Changes in v6:
>>> - add trans_mode condition for IDMAC initialization
>>> suggested by Heiko
>>> - re-test my patch on rk3188 platform and update commit msg
>>> - update performance of pio vs edmac in cover letter
>>>
>>> Changes in v5:
>>> - add the title of cover letter
>>> - fix typo of comment
>>> - add macro for reading HCON register
>>> - add "Acked-by: Krzysztof Kozlowski <[email protected]>" for exynos_defconfig patch
>>> - add "Acked-by: Vineet Gupta <[email protected]>" for axs10x_defconfig patch
>>> - add "Acked-by: Govindraj Raja <[email protected]>" and
>>> "Acked-by: Ralf Baechle <[email protected]>" for pistachio_defconfig patch
>>> - add "Acked-by: Joachim Eastwood <[email protected]>" for lpc18xx_defconfig patch
>>> - add "Acked-by: Wei Xu <[email protected]>" for hisi_defconfig patch
>>> - rebase on "https://github.com/jh80chung/dw-mmc.git tags/dw-mmc-for-ulf-v4.2" for merging easily
>>>
>>> Changes in v4:
>>> - remove "host->trans_mode" and use "host->use_dma" to indicate
>>> transfer mode.
>>> - remove all bt-bindings' changes since we don't need new properities.
>>> - check transfer mode at runtime by reading HCON reg
>>> - spilt defconfig changes for each sub-architecture
>>> - fix the title of cover letter
>>> - reuse some code for reducing code size
>>>
>>> Changes in v3:
>>> - choose transfer mode at runtime
>>> - remove all CONFIG_MMC_DW_IDMAC config option
>>> - add supports-idmac property for some platforms
>>>
>>> Changes in v2:
>>> - Fix typo of dev_info msg
>>> - remove unused dmach from declaration of dw_mci_dma_slave
>>>
>>> drivers/mmc/host/Kconfig | 11 +-
>>> drivers/mmc/host/dw_mmc-pltfm.c | 2 +
>>> drivers/mmc/host/dw_mmc.c | 264 ++++++++++++++++++++++++++++++++--------
>>> drivers/mmc/host/dw_mmc.h | 5 +
>>> include/linux/mmc/dw_mmc.h | 27 +++-
>>> 5 files changed, 242 insertions(+), 67 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>> index 6a0f9c7..a86c0eb 100644
>>> --- a/drivers/mmc/host/Kconfig
>>> +++ b/drivers/mmc/host/Kconfig
>>> @@ -607,15 +607,7 @@ config MMC_DW
>>> help
>>> This selects support for the Synopsys DesignWare Mobile Storage IP
>>> block, this provides host support for SD and MMC interfaces, in both
>>> - PIO and external DMA modes.
>>> -
>>> -config MMC_DW_IDMAC
>>> - bool "Internal DMAC interface"
>>> - depends on MMC_DW
>>> - help
>>> - This selects support for the internal DMAC block within the Synopsys
>>> - Designware Mobile Storage IP block. This disables the external DMA
>>> - interface.
>>> + PIO, internal DMA mode and external DMA modes.
>>>
>>> config MMC_DW_PLTFM
>>> tristate "Synopsys Designware MCI Support as platform device"
>>> @@ -644,7 +636,6 @@ config MMC_DW_K3
>>> tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
>>> depends on MMC_DW
>>> select MMC_DW_PLTFM
>>> - select MMC_DW_IDMAC
>>> help
>>> This selects support for Hisilicon K3 SoC specific extensions to the
>>> Synopsys DesignWare Memory Card Interface driver. Select this option
>>> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
>>> index ec6dbcd..7e1d13b 100644
>>> --- a/drivers/mmc/host/dw_mmc-pltfm.c
>>> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
>>> @@ -59,6 +59,8 @@ int dw_mci_pltfm_register(struct platform_device *pdev,
>>> host->pdata = pdev->dev.platform_data;
>>>
>>> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + /* Get registers' physical base address */
>>> + host->phy_regs = (void *)(regs->start);
>>> host->regs = devm_ioremap_resource(&pdev->dev, regs);
>>> if (IS_ERR(host->regs))
>>> return PTR_ERR(host->regs);
>>
>> Is this board specific code? If so, separate the patch.
>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index fcbf552..f943619 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -56,7 +56,6 @@
>>> #define DW_MCI_FREQ_MAX 200000000 /* unit: HZ */
>>> #define DW_MCI_FREQ_MIN 400000 /* unit: HZ */
>>>
>>> -#ifdef CONFIG_MMC_DW_IDMAC
>>> #define IDMAC_INT_CLR (SDMMC_IDMAC_INT_AI | SDMMC_IDMAC_INT_NI | \
>>> SDMMC_IDMAC_INT_CES | SDMMC_IDMAC_INT_DU | \
>>> SDMMC_IDMAC_INT_FBE | SDMMC_IDMAC_INT_RI | \
>>> @@ -102,7 +101,6 @@ struct idmac_desc {
>>>
>>> /* Each descriptor can transfer up to 4KB of data in chained mode */
>>> #define DW_MCI_DESC_DATA_LENGTH 0x1000
>>> -#endif /* CONFIG_MMC_DW_IDMAC */
>>>
>>> static bool dw_mci_reset(struct dw_mci *host);
>>> static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>>> @@ -407,7 +405,6 @@ static int dw_mci_get_dma_dir(struct mmc_data *data)
>>> return DMA_FROM_DEVICE;
>>> }
>>>
>>> -#ifdef CONFIG_MMC_DW_IDMAC
>>> static void dw_mci_dma_cleanup(struct dw_mci *host)
>>> {
>>> struct mmc_data *data = host->data;
>>> @@ -445,12 +442,21 @@ static void dw_mci_idmac_stop_dma(struct dw_mci *host)
>>> mci_writel(host, BMOD, temp);
>>> }
>>>
>>> -static void dw_mci_idmac_complete_dma(struct dw_mci *host)
>>> +static void dw_mci_dmac_complete_dma(void *arg)
>>> {
>>> + struct dw_mci *host = arg;
>>> struct mmc_data *data = host->data;
>>>
>>> dev_vdbg(host->dev, "DMA complete\n");
>>>
>>> + if (host->use_dma == TRANS_MODE_EDMAC)
>>> + if (data && (data->flags & MMC_DATA_READ))
>>
>> Combine one condition.
>>
>>> + /* Invalidate cache after read */
>>> + dma_sync_sg_for_cpu(mmc_dev(host->cur_slot->mmc),
>>> + data->sg,
>>> + data->sg_len,
>>> + DMA_FROM_DEVICE);
>>> +
>>> host->dma_ops->cleanup(host);
>>>
>>> /*
>>> @@ -564,7 +570,7 @@ static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
>>> wmb(); /* drain writebuffer */
>>> }
>>>
>>> -static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>>> +static int dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>>> {
>>> u32 temp;
>>>
>>> @@ -589,6 +595,8 @@ static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>>>
>>> /* Start it running */
>>> mci_writel(host, PLDMND, 1);
>>> +
>>> + return 0;
>>> }
>>>
>>> static int dw_mci_idmac_init(struct dw_mci *host)
>>> @@ -669,10 +677,112 @@ static const struct dw_mci_dma_ops dw_mci_idmac_ops = {
>>> .init = dw_mci_idmac_init,
>>> .start = dw_mci_idmac_start_dma,
>>> .stop = dw_mci_idmac_stop_dma,
>>> - .complete = dw_mci_idmac_complete_dma,
>>> + .complete = dw_mci_dmac_complete_dma,
>>> + .cleanup = dw_mci_dma_cleanup,
>>> +};
>>> +
>>> +static void dw_mci_edmac_stop_dma(struct dw_mci *host)
>>> +{
>>> + dmaengine_terminate_all(host->dms->ch);
>>> +}
>>> +
>>> +static int dw_mci_edmac_start_dma(struct dw_mci *host,
>>> + unsigned int sg_len)
>>> +{
>>> + struct dma_slave_config cfg;
>>> + struct dma_async_tx_descriptor *desc = NULL;
>>> + struct scatterlist *sgl = host->data->sg;
>>> + const u32 mszs[] = {1, 4, 8, 16, 32, 64, 128, 256};
>>> + u32 sg_elems = host->data->sg_len;
>>> + u32 fifoth_val;
>>> + u32 fifo_offset = host->fifo_reg - host->regs;
>>> + int ret = 0;
>>> +
>>> + /* Set external dma config: burst size, burst width */
>>> + cfg.dst_addr = (dma_addr_t)(host->phy_regs + fifo_offset);
>>
>> host->phy_regs is not assigned?
>>
>>> + cfg.src_addr = cfg.dst_addr;
>>> + cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>>> + cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>>> +
>>> + /* Match burst msize with external dma config */
>>> + fifoth_val = mci_readl(host, FIFOTH);
>>> + cfg.dst_maxburst = mszs[(fifoth_val >> 28) & 0x7];
>>> + cfg.src_maxburst = cfg.dst_maxburst;
>>> +
>>> + if (host->data->flags & MMC_DATA_WRITE)
>>> + cfg.direction = DMA_MEM_TO_DEV;
>>> + else /* MMC_DATA_READ */
>>> + cfg.direction = DMA_DEV_TO_MEM;
>>> +
>>> + ret = dmaengine_slave_config(host->dms->ch, &cfg);
>>> + if (ret) {
>>> + dev_err(host->dev, "Failed to config edmac.\n");
>>> + return -EBUSY;
>>> + }
>>> +
>>> + desc = dmaengine_prep_slave_sg(host->dms->ch, sgl,
>>> + sg_len, cfg.direction,
>>> + DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>>> + if (!desc) {
>>> + dev_err(host->dev, "Can't prepare slave sg.\n");
>>> + return -EBUSY;
>>> + }
>>> +
>>> + /* Set dw_mci_dmac_complete_dma as callback */
>>> + desc->callback = dw_mci_dmac_complete_dma;
>>> + desc->callback_param = (void *)host;
>>> + dmaengine_submit(desc);
>>> +
>>> + /* Flush cache before write */
>>> + if (host->data->flags & MMC_DATA_WRITE)
>>> + dma_sync_sg_for_device(mmc_dev(host->cur_slot->mmc), sgl,
>>> + sg_elems, DMA_TO_DEVICE);
>>> +
>>> + dma_async_issue_pending(host->dms->ch);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int dw_mci_edmac_init(struct dw_mci *host)
>>> +{
>>> + /* Request external dma channel */
>>> + host->dms = kzalloc(sizeof(struct dw_mci_dma_slave), GFP_KERNEL);
>>> + if (!host->dms)
>>> + return -ENOMEM;
>>> +
>>> + host->dms->ch = dma_request_slave_channel(host->dev, "rx-tx");
>>> + if (!host->dms->ch) {
>>> + dev_err(host->dev,
>>> + "Failed to get external DMA channel %d\n",
>>> + host->dms->ch->chan_id);
>>> + kfree(host->dms);
>>> + host->dms = NULL;
>>> + return -ENXIO;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void dw_mci_edmac_exit(struct dw_mci *host)
>>> +{
>>> + if (host->dms) {
>>> + if (host->dms->ch) {
>>> + dma_release_channel(host->dms->ch);
>>> + host->dms->ch = NULL;
>>> + }
>>> + kfree(host->dms);
>>> + host->dms = NULL;
>>> + }
>>> +}
>>> +
>>> +static const struct dw_mci_dma_ops dw_mci_edmac_ops = {
>>> + .init = dw_mci_edmac_init,
>>> + .exit = dw_mci_edmac_exit,
>>> + .start = dw_mci_edmac_start_dma,
>>> + .stop = dw_mci_edmac_stop_dma,
>>> + .complete = dw_mci_dmac_complete_dma,
>>> .cleanup = dw_mci_dma_cleanup,
>>> };
>>> -#endif /* CONFIG_MMC_DW_IDMAC */
>>>
>>> static int dw_mci_pre_dma_transfer(struct dw_mci *host,
>>> struct mmc_data *data,
>>> @@ -752,7 +862,6 @@ static void dw_mci_post_req(struct mmc_host *mmc,
>>>
>>> static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
>>> {
>>> -#ifdef CONFIG_MMC_DW_IDMAC
>>> unsigned int blksz = data->blksz;
>>> const u32 mszs[] = {1, 4, 8, 16, 32, 64, 128, 256};
>>> u32 fifo_width = 1 << host->data_shift;
>>> @@ -760,6 +869,10 @@ static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
>>> u32 msize = 0, rx_wmark = 1, tx_wmark, tx_wmark_invers;
>>> int idx = ARRAY_SIZE(mszs) - 1;
>>>
>>> + /* pio should ship this scenario */
>>> + if (!host->use_dma)
>>> + return;
>>> +
>>> tx_wmark = (host->fifo_depth) / 2;
>>> tx_wmark_invers = host->fifo_depth - tx_wmark;
>>>
>>> @@ -788,7 +901,6 @@ static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
>>> done:
>>> fifoth_val = SDMMC_SET_FIFOTH(msize, rx_wmark, tx_wmark);
>>> mci_writel(host, FIFOTH, fifoth_val);
>>> -#endif
>>> }
>>>
>>> static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>>> @@ -850,10 +962,12 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
>>>
>>> host->using_dma = 1;
>>>
>>> - dev_vdbg(host->dev,
>>> - "sd sg_cpu: %#lx sg_dma: %#lx sg_len: %d\n",
>>> - (unsigned long)host->sg_cpu, (unsigned long)host->sg_dma,
>>> - sg_len);
>>> + if (host->use_dma == TRANS_MODE_IDMAC)
>>> + dev_vdbg(host->dev,
>>> + "sd sg_cpu: %#lx sg_dma: %#lx sg_len: %d\n",
>>> + (unsigned long)host->sg_cpu,
>>> + (unsigned long)host->sg_dma,
>>> + sg_len);
>>>
>>> /*
>>> * Decide the MSIZE and RX/TX Watermark.
>>> @@ -875,7 +989,11 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
>>> mci_writel(host, INTMASK, temp);
>>> spin_unlock_irqrestore(&host->irq_lock, irqflags);
>>>
>>> - host->dma_ops->start(host, sg_len);
>>> + if (host->dma_ops->start(host, sg_len)) {
>>> + /* We can't do DMA */
>>> + dev_err(host->dev, "%s: failed to start DMA.\n", __func__);
>>> + return -ENODEV;
>>> + }
>>>
>>> return 0;
>>> }
>>> @@ -2343,15 +2461,17 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>>>
>>> }
>>>
>>> -#ifdef CONFIG_MMC_DW_IDMAC
>>> - /* Handle DMA interrupts */
>>> + if (host->use_dma != TRANS_MODE_IDMAC)
>>> + return IRQ_HANDLED;
>>> +
>>> + /* Handle IDMA interrupts */
>>> if (host->dma_64bit_address == 1) {
>>> pending = mci_readl(host, IDSTS64);
>>> if (pending & (SDMMC_IDMAC_INT_TI | SDMMC_IDMAC_INT_RI)) {
>>> mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_TI |
>>> SDMMC_IDMAC_INT_RI);
>>> mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
>>> - host->dma_ops->complete(host);
>>> + host->dma_ops->complete((void *)host);
>>> }
>>> } else {
>>> pending = mci_readl(host, IDSTS);
>>> @@ -2359,10 +2479,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>>> mci_writel(host, IDSTS, SDMMC_IDMAC_INT_TI |
>>> SDMMC_IDMAC_INT_RI);
>>> mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
>>> - host->dma_ops->complete(host);
>>> + host->dma_ops->complete((void *)host);
>>> }
>>> }
>>> -#endif
>>>
>>> return IRQ_HANDLED;
>>> }
>>> @@ -2471,13 +2590,21 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>>> goto err_host_allocated;
>>>
>>> /* Useful defaults if platform data is unset. */
>>> - if (host->use_dma) {
>>> + if (host->use_dma == TRANS_MODE_IDMAC) {
>>> mmc->max_segs = host->ring_size;
>>> mmc->max_blk_size = 65536;
>>> mmc->max_seg_size = 0x1000;
>>> mmc->max_req_size = mmc->max_seg_size * host->ring_size;
>>> mmc->max_blk_count = mmc->max_req_size / 512;
>>> + } else if (host->use_dma == TRANS_MODE_EDMAC) {
>>> + mmc->max_segs = 64;
>>> + mmc->max_blk_size = 65536;
>>> + mmc->max_blk_count = 65535;
>>> + mmc->max_req_size =
>>> + mmc->max_blk_size * mmc->max_blk_count;
>>> + mmc->max_seg_size = mmc->max_req_size;
>>
>> Fix the indention
>>
>>> } else {
>>> + /* TRANS_MODE_PIO */
>>> mmc->max_segs = 64;
>>> mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
>>> mmc->max_blk_count = 512;
>>> @@ -2517,35 +2644,66 @@ static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
>>> static void dw_mci_init_dma(struct dw_mci *host)
>>> {
>>> int addr_config;
>>> - /* Check ADDR_CONFIG bit in HCON to find IDMAC address bus width */
>>> - addr_config = (mci_readl(host, HCON) >> 27) & 0x01;
>>> -
>>> - if (addr_config == 1) {
>>> - /* host supports IDMAC in 64-bit address mode */
>>> - host->dma_64bit_address = 1;
>>> - dev_info(host->dev, "IDMAC supports 64-bit address mode.\n");
>>> - if (!dma_set_mask(host->dev, DMA_BIT_MASK(64)))
>>> - dma_set_coherent_mask(host->dev, DMA_BIT_MASK(64));
>>> - } else {
>>> - /* host supports IDMAC in 32-bit address mode */
>>> - host->dma_64bit_address = 0;
>>> - dev_info(host->dev, "IDMAC supports 32-bit address mode.\n");
>>> - }
>>> + int trans_mode;
>>> + struct device *dev = host->dev;
>>> + struct device_node *np = dev->of_node;
>>>
>>> - /* Alloc memory for sg translation */
>>> - host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
>>> - &host->sg_dma, GFP_KERNEL);
>>> - if (!host->sg_cpu) {
>>> - dev_err(host->dev, "%s: could not alloc DMA memory\n",
>>> - __func__);
>>> + /* Check tansfer mode */
>>> + trans_mode = SDMMC_GET_TRANS_MODE(mci_readl(host, HCON));
>>> + if (trans_mode == 0) {
>>> + trans_mode = TRANS_MODE_IDMAC;
>>> + } else if (trans_mode == 1 || trans_mode == 2) {
>>> + trans_mode = TRANS_MODE_EDMAC;
>>> + } else {
>>> + trans_mode = TRANS_MODE_PIO;
>>
>> what are trans_mode "0, 1, 2"?
>> (00 - none) is NO-DMA interface, isn't? is it IDMAC mode?
>>
>>> goto no_dma;
>>> }
>>>
>>> /* Determine which DMA interface to use */
>>> -#ifdef CONFIG_MMC_DW_IDMAC
>>> - host->dma_ops = &dw_mci_idmac_ops;
>>> - dev_info(host->dev, "Using internal DMA controller.\n");
>>> -#endif
>>> + if (trans_mode == TRANS_MODE_IDMAC) {
>>> + /*
>>> + * Check ADDR_CONFIG bit in HCON to find
>>> + * IDMAC address bus width
>>> + */
>>> + addr_config = SDMMC_GET_ADDR_CONFIG(mci_readl(host, HCON));
>>> +
>>> + if (addr_config == 1) {
>>> + /* host supports IDMAC in 64-bit address mode */
>>> + host->dma_64bit_address = 1;
>>> + dev_info(host->dev,
>>> + "IDMAC supports 64-bit address mode.\n");
>>> + if (!dma_set_mask(host->dev, DMA_BIT_MASK(64)))
>>> + dma_set_coherent_mask(host->dev,
>>> + DMA_BIT_MASK(64));
>>> + } else {
>>> + /* host supports IDMAC in 32-bit address mode */
>>> + host->dma_64bit_address = 0;
>>> + dev_info(host->dev,
>>> + "IDMAC supports 32-bit address mode.\n");
>>> + }
>>> +
>>> + /* Alloc memory for sg translation */
>>> + host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
>>> + &host->sg_dma, GFP_KERNEL);
>>> + if (!host->sg_cpu) {
>>> + dev_err(host->dev,
>>> + "%s: could not alloc DMA memory\n",
>>> + __func__);
>>> + goto no_dma;
>>> + }
>>> +
>>> + host->dma_ops = &dw_mci_idmac_ops;
>>> + dev_info(host->dev, "Using internal DMA controller.\n");
>>> + } else {
>>> + /* TRANS_MODE_EDMAC: check dma bindings again */
>>> + if ((of_property_count_strings(np, "dma-names") < 0) ||
>>> + (!of_find_property(np, "dmas", NULL))) {
>>> + trans_mode = TRANS_MODE_PIO;
>>> + goto no_dma;
>>> + }
>>> + host->dma_ops = &dw_mci_edmac_ops;
>>> + dev_info(host->dev, "Using external DMA controller.\n");
>>> + }
>>>
>>> if (!host->dma_ops)
>>> goto no_dma;
>>> @@ -2562,12 +2720,12 @@ static void dw_mci_init_dma(struct dw_mci *host)
>>> goto no_dma;
>>> }
>>>
>>> - host->use_dma = 1;
>>> + host->use_dma = trans_mode;
>>
>> Also confuse, if trans_mode is assigned host->use_dma, can mode value be directly assigned to host->use_dma?
>>
>> trans_mode = TRAMS_MODE_PIO;
>> host->use_dma = trans_mode;
>>
>> -> host->use_dma = TRAMS_MODE_PIO;
>>
>> Then trans_mode can be removed.
>>
>>> return;
>>>
>>> no_dma:
>>> dev_info(host->dev, "Using PIO mode.\n");
>>> - host->use_dma = 0;
>>> + host->use_dma = trans_mode;
>>> }
>>>
>>> static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset)
>>> @@ -2650,10 +2808,9 @@ static bool dw_mci_reset(struct dw_mci *host)
>>> }
>>> }
>>>
>>> -#if IS_ENABLED(CONFIG_MMC_DW_IDMAC)
>>> - /* It is also recommended that we reset and reprogram idmac */
>>> - dw_mci_idmac_reset(host);
>>> -#endif
>>> + if (host->use_dma == TRANS_MODE_IDMAC)
>>> + /* It is also recommended that we reset and reprogram idmac */
>>> + dw_mci_idmac_reset(host);
>>>
>>> ret = true;
>>>
>>> @@ -2890,7 +3047,7 @@ int dw_mci_probe(struct dw_mci *host)
>>> * Get the host data width - this assumes that HCON has been set with
>>> * the correct values.
>>> */
>>> - i = (mci_readl(host, HCON) >> 7) & 0x7;
>>> + i = SDMMC_GET_HDATA_WIDTH(mci_readl(host, HCON));
>>
>> This is not related with supporting external dma interface.
>> Separate this.
>>
>>> if (!i) {
>>> host->push_data = dw_mci_push_data16;
>>> host->pull_data = dw_mci_pull_data16;
>>> @@ -2972,7 +3129,7 @@ int dw_mci_probe(struct dw_mci *host)
>>> if (host->pdata->num_slots)
>>> host->num_slots = host->pdata->num_slots;
>>> else
>>> - host->num_slots = ((mci_readl(host, HCON) >> 1) & 0x1F) + 1;
>>> + host->num_slots = SDMMC_GET_SLOT_NUM(mci_readl(host, HCON));
>>
>> Ditto. (with above.)
>>
>>>
>>> /*
>>> * Enable interrupts for command done, data over, data empty,
>>> @@ -3067,6 +3224,9 @@ EXPORT_SYMBOL(dw_mci_remove);
>>> */
>>> int dw_mci_suspend(struct dw_mci *host)
>>> {
>>> + if (host->use_dma && host->dma_ops->exit)
>>> + host->dma_ops->exit(host);
>>> +
>>> return 0;
>>> }
>>> EXPORT_SYMBOL(dw_mci_suspend);
>>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>>> index 8ce4674..c453e94 100644
>>> --- a/drivers/mmc/host/dw_mmc.h
>>> +++ b/drivers/mmc/host/dw_mmc.h
>>> @@ -148,6 +148,11 @@
>>> #define SDMMC_SET_FIFOTH(m, r, t) (((m) & 0x7) << 28 | \
>>> ((r) & 0xFFF) << 16 | \
>>> ((t) & 0xFFF))
>>> +/* HCON register defines */
>>> +#define SDMMC_GET_SLOT_NUM(x) ((((x)>>1) & 0x1F) + 1)
>>> +#define SDMMC_GET_HDATA_WIDTH(x) (((x)>>7) & 0x7)
>>> +#define SDMMC_GET_TRANS_MODE(x) (((x)>>16) & 0x3)
>>> +#define SDMMC_GET_ADDR_CONFIG(x) (((x)>>27) & 0x1)
>>> /* Internal DMAC interrupt defines */
>>> #define SDMMC_IDMAC_INT_AI BIT(9)
>>> #define SDMMC_IDMAC_INT_NI BIT(8)
>>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>>> index c846f42..6a2b83c 100644
>>> --- a/include/linux/mmc/dw_mmc.h
>>> +++ b/include/linux/mmc/dw_mmc.h
>>> @@ -16,6 +16,7 @@
>>>
>>> #include <linux/scatterlist.h>
>>> #include <linux/mmc/core.h>
>>> +#include <linux/dmaengine.h>
>>>
>>> #define MAX_MCI_SLOTS 2
>>>
>>> @@ -40,6 +41,17 @@ enum {
>>>
>>> struct mmc_data;
>>>
>>> +enum {
>>> + TRANS_MODE_PIO = 0,
>>> + TRANS_MODE_IDMAC,
>>> + TRANS_MODE_EDMAC
>>> +};
>>> +
>>> +struct dw_mci_dma_slave {
>>> + struct dma_chan *ch;
>>> + enum dma_transfer_direction direction;
>>> +};
>>> +
>>> /**
>>> * struct dw_mci - MMC controller state shared between all slots
>>> * @lock: Spinlock protecting the queue and associated data.
>>> @@ -154,11 +166,16 @@ struct dw_mci {
>>> dma_addr_t sg_dma;
>>> void *sg_cpu;
>>> const struct dw_mci_dma_ops *dma_ops;
>>> -#ifdef CONFIG_MMC_DW_IDMAC
>>> + /* For idmac */
>>> unsigned int ring_size;
>>> -#else
>>> +
>>> + /* For edmac */
>>> + struct dw_mci_dma_slave *dms;
>>> + /* Registers's physical base address */
>>> + void *phy_regs;
>>> +
>>> struct dw_mci_dma_data *dma_data;
>>> -#endif
>>> +
>>
>> On ulf's repository, this point should be conflicted.
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> u32 cmd_status;
>>> u32 data_status;
>>> u32 stop_cmdr;
>>> @@ -212,8 +229,8 @@ struct dw_mci {
>>> struct dw_mci_dma_ops {
>>> /* DMA Ops */
>>> int (*init)(struct dw_mci *host);
>>> - void (*start)(struct dw_mci *host, unsigned int sg_len);
>>> - void (*complete)(struct dw_mci *host);
>>> + int (*start)(struct dw_mci *host, unsigned int sg_len);
>>> + void (*complete)(void *host);
>>> void (*stop)(struct dw_mci *host);
>>> void (*cleanup)(struct dw_mci *host);
>>> void (*exit)(struct dw_mci *host);
>>>
>>
>>
>>
>>
>
>

2015-08-21 07:28:11

by Shawn Lin

[permalink] [raw]
Subject: Re: [RFC PATCH v6 1/9] mmc: dw_mmc: Add external dma interface support

On 2015/8/21 14:35, Jaehoon Chung wrote:
> On 08/21/2015 03:30 PM, Shawn Lin wrote:
>> On 2015/8/21 14:27, Jaehoon Chung wrote:
>>> Hi, Shawn.
>>>
>>> Is this based on Ulf's repository?
>>
>>
>> no, it's based on "https://github.com/jh80chung/dw-mmc.git tags/dw-mmc-for-ulf-v4.2" :)
>
> Oh..I will rebase to Ulf's next branch on this weekend.
> Then could you rebase this patch? And i added more comments at below.. :)
>

Okay, I will rebase to Ulf's next.

> Best Regards,
> Jaehoon Chung
>
>>

[...]

>>>> index ec6dbcd..7e1d13b 100644
>>>> --- a/drivers/mmc/host/dw_mmc-pltfm.c
>>>> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
>>>> @@ -59,6 +59,8 @@ int dw_mci_pltfm_register(struct platform_device *pdev,
>>>> host->pdata = pdev->dev.platform_data;
>>>>
>>>> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> + /* Get registers' physical base address */
>>>> + host->phy_regs = (void *)(regs->start);
>>>> host->regs = devm_ioremap_resource(&pdev->dev, regs);
>>>> if (IS_ERR(host->regs))
>>>> return PTR_ERR(host->regs);
>>>
>>> Is this board specific code? If so, separate the patch.

It's might not board specific code.
dmaengine need dw_mmc's *physical* fifo address for data transfer, so I
get controller physical address here in order to calculate physical
fifo address.

regs is from dt-bindings, for instance:
dwmmc0@12200000 {
compatible = "snps,dw-mshc";
clocks = <&clock 351>, <&clock 132>;
clock-names = "biu", "ciu";
reg = <0x12200000 0x1000>;
interrupts = <0 75 0>;
#address-cells = <1>;
#size-cells = <0>;
};

so, host->phy_regs will be 0x12200000 .

[...]

>>>> +static void dw_mci_dmac_complete_dma(void *arg)
>>>> {
>>>> + struct dw_mci *host = arg;
>>>> struct mmc_data *data = host->data;
>>>>
>>>> dev_vdbg(host->dev, "DMA complete\n");
>>>>
>>>> + if (host->use_dma == TRANS_MODE_EDMAC)
>>>> + if (data && (data->flags & MMC_DATA_READ))
>>>
>>> Combine one condition.

okay.

[...]

>>>> + u32 fifo_offset = host->fifo_reg - host->regs;
>>>> + int ret = 0;
>>>> +
>>>> + /* Set external dma config: burst size, burst width */
>>>> + cfg.dst_addr = (dma_addr_t)(host->phy_regs + fifo_offset);
>>>
>>> host->phy_regs is not assigned?

we got it at dw_mci_pltfm_register. See comments above. :)

[...]

>>>> mmc->max_blk_count = mmc->max_req_size / 512;
>>>> + } else if (host->use_dma == TRANS_MODE_EDMAC) {
>>>> + mmc->max_segs = 64;
>>>> + mmc->max_blk_size = 65536;
>>>> + mmc->max_blk_count = 65535;
>>>> + mmc->max_req_size =
>>>> + mmc->max_blk_size * mmc->max_blk_count;
>>>> + mmc->max_seg_size = mmc->max_req_size;
>>>
>>> Fix the indention

Hmm..I check it attentively but can't find the indention . Might it's
because you apply it against Ulf's repo?

>>>

[...]

>>>>
>>>> - /* Alloc memory for sg translation */
>>>> - host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
>>>> - &host->sg_dma, GFP_KERNEL);
>>>> - if (!host->sg_cpu) {
>>>> - dev_err(host->dev, "%s: could not alloc DMA memory\n",
>>>> - __func__);
>>>> + /* Check tansfer mode */
>>>> + trans_mode = SDMMC_GET_TRANS_MODE(mci_readl(host, HCON));
>>>> + if (trans_mode == 0) {
>>>> + trans_mode = TRANS_MODE_IDMAC;
>>>> + } else if (trans_mode == 1 || trans_mode == 2) {
>>>> + trans_mode = TRANS_MODE_EDMAC;
>>>> + } else {
>>>> + trans_mode = TRANS_MODE_PIO;
>>>
>>> what are trans_mode "0, 1, 2"?
>>> (00 - none) is NO-DMA interface, isn't? is it IDMAC mode?
>>>

No, I guess the databook's ambiguous description confuse everybody.

I got double comfirm from my ASCI team as well as Synoposys
2b'00: NO-DMA interface --> It support IDMAC actually
2b'01 & 2b'02: DW_DMA & GENERIC_DMA --> Support 2 types of external dma.
2b'02: NON-DW-DMA --> only support PIO

refer to the description below:
Parameter Name: DMA_INTERFACE
Legal Values: 0-3
Default Value: 0
Description:
0- No DMA Interface
1- DesignWare DMA Interface
2- Generic DMA Interface
3- Non DW DMA Interface
In DesignWare DMA mode, request/acknowledge protocol meets DW_ahb_dmac
controller protocol. In this mode, host data bus is also used for DMA
transfers.Generic DMA-type interface has simpler request/acknowledge
handshake and has dedicated read/write data bus for DMA transfers. Non
DW DMAC interface uses dw_dma_single interface in addition to the
existing interface and uses host data bus for DMA transfers. This is
configurable only if INTERNAL_DMAC=0.

>>>> goto no_dma;
>>>> }
>
>>>> + dev_info(host->dev, "Using external DMA controller.\n");
>>>> + }
>>>>
>>>> if (!host->dma_ops)
>>>> goto no_dma;
>>>> @@ -2562,12 +2720,12 @@ static void dw_mci_init_dma(struct dw_mci *host)
>>>> goto no_dma;
>>>> }
>>>>
>>>> - host->use_dma = 1;
>>>> + host->use_dma = trans_mode;
>>>
>>> Also confuse, if trans_mode is assigned host->use_dma, can mode value be directly assigned to host->use_dma?
>>>

Good idea, I will do it. :)

>>> trans_mode = TRAMS_MODE_PIO;
>>> host->use_dma = trans_mode;
>>>

[...]

>>>>
>>>> @@ -2890,7 +3047,7 @@ int dw_mci_probe(struct dw_mci *host)
>>>> * Get the host data width - this assumes that HCON has been set with
>>>> * the correct values.
>>>> */
>>>> - i = (mci_readl(host, HCON) >> 7) & 0x7;
>>>> + i = SDMMC_GET_HDATA_WIDTH(mci_readl(host, HCON));
>>>
>>> This is not related with supporting external dma interface.
>>> Separate this.
>>>

Okay, It will be split into another one.

>>>> if (!i) {
>>>> host->push_data = dw_mci_push_data16;
>>>> host->pull_data = dw_mci_pull_data16;



--
Best Regards
Shawn Lin

2015-08-21 07:38:51

by Jaehoon Chung

[permalink] [raw]
Subject: Re: [RFC PATCH v6 1/9] mmc: dw_mmc: Add external dma interface support

On 08/21/2015 04:27 PM, Shawn Lin wrote:
> On 2015/8/21 14:35, Jaehoon Chung wrote:
>> On 08/21/2015 03:30 PM, Shawn Lin wrote:
>>> On 2015/8/21 14:27, Jaehoon Chung wrote:
>>>> Hi, Shawn.
>>>>
>>>> Is this based on Ulf's repository?
>>>
>>>
>>> no, it's based on "https://github.com/jh80chung/dw-mmc.git tags/dw-mmc-for-ulf-v4.2" :)
>>
>> Oh..I will rebase to Ulf's next branch on this weekend.
>> Then could you rebase this patch? And i added more comments at below.. :)
>>
>
> Okay, I will rebase to Ulf's next.
>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>
> [...]
>
>>>>> index ec6dbcd..7e1d13b 100644
>>>>> --- a/drivers/mmc/host/dw_mmc-pltfm.c
>>>>> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
>>>>> @@ -59,6 +59,8 @@ int dw_mci_pltfm_register(struct platform_device *pdev,
>>>>> host->pdata = pdev->dev.platform_data;
>>>>>
>>>>> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>> + /* Get registers' physical base address */
>>>>> + host->phy_regs = (void *)(regs->start);
>>>>> host->regs = devm_ioremap_resource(&pdev->dev, regs);
>>>>> if (IS_ERR(host->regs))
>>>>> return PTR_ERR(host->regs);
>>>>
>>>> Is this board specific code? If so, separate the patch.
>
> It's might not board specific code.
> dmaengine need dw_mmc's *physical* fifo address for data transfer, so I get controller physical address here in order to calculate physical fifo address.
>
> regs is from dt-bindings, for instance:
> dwmmc0@12200000 {
> compatible = "snps,dw-mshc";
> clocks = <&clock 351>, <&clock 132>;
> clock-names = "biu", "ciu";
> reg = <0x12200000 0x1000>;
> interrupts = <0 75 0>;
> #address-cells = <1>;
> #size-cells = <0>;
> };
>
> so, host->phy_regs will be 0x12200000 .
>
> [...]
>
>>>>> +static void dw_mci_dmac_complete_dma(void *arg)
>>>>> {
>>>>> + struct dw_mci *host = arg;
>>>>> struct mmc_data *data = host->data;
>>>>>
>>>>> dev_vdbg(host->dev, "DMA complete\n");
>>>>>
>>>>> + if (host->use_dma == TRANS_MODE_EDMAC)
>>>>> + if (data && (data->flags & MMC_DATA_READ))
>>>>
>>>> Combine one condition.
>
> okay.
>
> [...]
>
>>>>> + u32 fifo_offset = host->fifo_reg - host->regs;
>>>>> + int ret = 0;
>>>>> +
>>>>> + /* Set external dma config: burst size, burst width */
>>>>> + cfg.dst_addr = (dma_addr_t)(host->phy_regs + fifo_offset);
>>>>
>>>> host->phy_regs is not assigned?
>
> we got it at dw_mci_pltfm_register. See comments above. :)
>
> [...]
>
>>>>> mmc->max_blk_count = mmc->max_req_size / 512;
>>>>> + } else if (host->use_dma == TRANS_MODE_EDMAC) {
>>>>> + mmc->max_segs = 64;
>>>>> + mmc->max_blk_size = 65536;
>>>>> + mmc->max_blk_count = 65535;
>>>>> + mmc->max_req_size =
>>>>> + mmc->max_blk_size * mmc->max_blk_count;
>>>>> + mmc->max_seg_size = mmc->max_req_size;
>>>>
>>>> Fix the indention
>
> Hmm..I check it attentively but can't find the indention . Might it's because you apply it against Ulf's repo?
>
>>>>
>
> [...]
>
>>>>>
>>>>> - /* Alloc memory for sg translation */
>>>>> - host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
>>>>> - &host->sg_dma, GFP_KERNEL);
>>>>> - if (!host->sg_cpu) {
>>>>> - dev_err(host->dev, "%s: could not alloc DMA memory\n",
>>>>> - __func__);
>>>>> + /* Check tansfer mode */
>>>>> + trans_mode = SDMMC_GET_TRANS_MODE(mci_readl(host, HCON));
>>>>> + if (trans_mode == 0) {
>>>>> + trans_mode = TRANS_MODE_IDMAC;
>>>>> + } else if (trans_mode == 1 || trans_mode == 2) {
>>>>> + trans_mode = TRANS_MODE_EDMAC;
>>>>> + } else {
>>>>> + trans_mode = TRANS_MODE_PIO;
>>>>
>>>> what are trans_mode "0, 1, 2"?
>>>> (00 - none) is NO-DMA interface, isn't? is it IDMAC mode?
>>>>
>
> No, I guess the databook's ambiguous description confuse everybody.
>
> I got double comfirm from my ASCI team as well as Synoposys
> 2b'00: NO-DMA interface --> It support IDMAC actually
> 2b'01 & 2b'02: DW_DMA & GENERIC_DMA --> Support 2 types of external dma.
> 2b'02: NON-DW-DMA --> only support PIO

Then Could you add the comment about this?
Use definition instead of "0, 1, 2". Developer don't know meaning that is 0, 1, 2.

Best Regards,
Jaehoon Chung

>
> refer to the description below:
> Parameter Name: DMA_INTERFACE
> Legal Values: 0-3
> Default Value: 0
> Description:
> 0- No DMA Interface
> 1- DesignWare DMA Interface
> 2- Generic DMA Interface
> 3- Non DW DMA Interface
> In DesignWare DMA mode, request/acknowledge protocol meets DW_ahb_dmac
> controller protocol. In this mode, host data bus is also used for DMA transfers.Generic DMA-type interface has simpler request/acknowledge handshake and has dedicated read/write data bus for DMA transfers. Non DW DMAC interface uses dw_dma_single interface in addition to the existing interface and uses host data bus for DMA transfers. This is configurable only if INTERNAL_DMAC=0.
>
>>>>> goto no_dma;
>>>>> }
>>
>>>>> + dev_info(host->dev, "Using external DMA controller.\n");
>>>>> + }
>>>>>
>>>>> if (!host->dma_ops)
>>>>> goto no_dma;
>>>>> @@ -2562,12 +2720,12 @@ static void dw_mci_init_dma(struct dw_mci *host)
>>>>> goto no_dma;
>>>>> }
>>>>>
>>>>> - host->use_dma = 1;
>>>>> + host->use_dma = trans_mode;
>>>>
>>>> Also confuse, if trans_mode is assigned host->use_dma, can mode value be directly assigned to host->use_dma?
>>>>
>
> Good idea, I will do it. :)
>
>>>> trans_mode = TRAMS_MODE_PIO;
>>>> host->use_dma = trans_mode;
>>>>
>
> [...]
>
>>>>>
>>>>> @@ -2890,7 +3047,7 @@ int dw_mci_probe(struct dw_mci *host)
>>>>> * Get the host data width - this assumes that HCON has been set with
>>>>> * the correct values.
>>>>> */
>>>>> - i = (mci_readl(host, HCON) >> 7) & 0x7;
>>>>> + i = SDMMC_GET_HDATA_WIDTH(mci_readl(host, HCON));
>>>>
>>>> This is not related with supporting external dma interface.
>>>> Separate this.
>>>>
>
> Okay, It will be split into another one.
>
>>>>> if (!i) {
>>>>> host->push_data = dw_mci_push_data16;
>>>>> host->pull_data = dw_mci_pull_data16;
>
>
>

2015-08-21 07:48:48

by Shawn Lin

[permalink] [raw]
Subject: Re: [RFC PATCH v6 1/9] mmc: dw_mmc: Add external dma interface support

On 2015/8/21 15:38, Jaehoon Chung wrote:
> On 08/21/2015 04:27 PM, Shawn Lin wrote:
>> On 2015/8/21 14:35, Jaehoon Chung wrote:
>>> On 08/21/2015 03:30 PM, Shawn Lin wrote:
>>>> On 2015/8/21 14:27, Jaehoon Chung wrote:
>>>>> Hi, Shawn.
>>>>>
>>>>> Is this based on Ulf's repository?
>>>>
>>>>
>>>> no, it's based on "https://github.com/jh80chung/dw-mmc.git tags/dw-mmc-for-ulf-v4.2" :)
>>>
>>> Oh..I will rebase to Ulf's next branch on this weekend.
>>> Then could you rebase this patch? And i added more comments at below.. :)
>>>
>>
>> Okay, I will rebase to Ulf's next.
>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>>
>>
>> [...]
>>
>>>>>> index ec6dbcd..7e1d13b 100644
>>>>>> --- a/drivers/mmc/host/dw_mmc-pltfm.c
>>>>>> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
>>>>>> @@ -59,6 +59,8 @@ int dw_mci_pltfm_register(struct platform_device *pdev,
>>>>>> host->pdata = pdev->dev.platform_data;
>>>>>>
>>>>>> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>> + /* Get registers' physical base address */
>>>>>> + host->phy_regs = (void *)(regs->start);
>>>>>> host->regs = devm_ioremap_resource(&pdev->dev, regs);
>>>>>> if (IS_ERR(host->regs))
>>>>>> return PTR_ERR(host->regs);
>>>>>
>>>>> Is this board specific code? If so, separate the patch.
>>
>> It's might not board specific code.
>> dmaengine need dw_mmc's *physical* fifo address for data transfer, so I get controller physical address here in order to calculate physical fifo address.
>>
>> regs is from dt-bindings, for instance:
>> dwmmc0@12200000 {
>> compatible = "snps,dw-mshc";
>> clocks = <&clock 351>, <&clock 132>;
>> clock-names = "biu", "ciu";
>> reg = <0x12200000 0x1000>;
>> interrupts = <0 75 0>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>> };
>>
>> so, host->phy_regs will be 0x12200000 .
>>
>> [...]
>>
>>>>>> +static void dw_mci_dmac_complete_dma(void *arg)
>>>>>> {
>>>>>> + struct dw_mci *host = arg;
>>>>>> struct mmc_data *data = host->data;
>>>>>>
>>>>>> dev_vdbg(host->dev, "DMA complete\n");
>>>>>>
>>>>>> + if (host->use_dma == TRANS_MODE_EDMAC)
>>>>>> + if (data && (data->flags & MMC_DATA_READ))
>>>>>
>>>>> Combine one condition.
>>
>> okay.
>>
>> [...]
>>
>>>>>> + u32 fifo_offset = host->fifo_reg - host->regs;
>>>>>> + int ret = 0;
>>>>>> +
>>>>>> + /* Set external dma config: burst size, burst width */
>>>>>> + cfg.dst_addr = (dma_addr_t)(host->phy_regs + fifo_offset);
>>>>>
>>>>> host->phy_regs is not assigned?
>>
>> we got it at dw_mci_pltfm_register. See comments above. :)
>>
>> [...]
>>
>>>>>> mmc->max_blk_count = mmc->max_req_size / 512;
>>>>>> + } else if (host->use_dma == TRANS_MODE_EDMAC) {
>>>>>> + mmc->max_segs = 64;
>>>>>> + mmc->max_blk_size = 65536;
>>>>>> + mmc->max_blk_count = 65535;
>>>>>> + mmc->max_req_size =
>>>>>> + mmc->max_blk_size * mmc->max_blk_count;
>>>>>> + mmc->max_seg_size = mmc->max_req_size;
>>>>>
>>>>> Fix the indention
>>
>> Hmm..I check it attentively but can't find the indention . Might it's because you apply it against Ulf's repo?
>>
>>>>>
>>
>> [...]
>>
>>>>>>
>>>>>> - /* Alloc memory for sg translation */
>>>>>> - host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
>>>>>> - &host->sg_dma, GFP_KERNEL);
>>>>>> - if (!host->sg_cpu) {
>>>>>> - dev_err(host->dev, "%s: could not alloc DMA memory\n",
>>>>>> - __func__);
>>>>>> + /* Check tansfer mode */
>>>>>> + trans_mode = SDMMC_GET_TRANS_MODE(mci_readl(host, HCON));
>>>>>> + if (trans_mode == 0) {
>>>>>> + trans_mode = TRANS_MODE_IDMAC;
>>>>>> + } else if (trans_mode == 1 || trans_mode == 2) {
>>>>>> + trans_mode = TRANS_MODE_EDMAC;
>>>>>> + } else {
>>>>>> + trans_mode = TRANS_MODE_PIO;
>>>>>
>>>>> what are trans_mode "0, 1, 2"?
>>>>> (00 - none) is NO-DMA interface, isn't? is it IDMAC mode?
>>>>>
>>
>> No, I guess the databook's ambiguous description confuse everybody.
>>
>> I got double comfirm from my ASCI team as well as Synoposys
>> 2b'00: NO-DMA interface --> It support IDMAC actually
>> 2b'01 & 2b'02: DW_DMA & GENERIC_DMA --> Support 2 types of external dma.
>> 2b'02: NON-DW-DMA --> only support PIO
>
> Then Could you add the comment about this?
> Use definition instead of "0, 1, 2". Developer don't know meaning that is 0, 1, 2.
>

Okay. :)))

> Best Regards,
> Jaehoon Chung
>
>>
>> refer to the description below:
>> Parameter Name: DMA_INTERFACE
>> Legal Values: 0-3
>> Default Value: 0
>> Description:
>> 0- No DMA Interface
>> 1- DesignWare DMA Interface
>> 2- Generic DMA Interface
>> 3- Non DW DMA Interface
>> In DesignWare DMA mode, request/acknowledge protocol meets DW_ahb_dmac
>> controller protocol. In this mode, host data bus is also used for DMA transfers.Generic DMA-type interface has simpler request/acknowledge handshake and has dedicated read/write data bus for DMA transfers. Non DW DMAC interface uses dw_dma_single interface in addition to the existing interface and uses host data bus for DMA transfers. This is configurable only if INTERNAL_DMAC=0.
>>
>>>>>> goto no_dma;
>>>>>> }
>>>
>>>>>> + dev_info(host->dev, "Using external DMA controller.\n");
>>>>>> + }
>>>>>>
>>>>>> if (!host->dma_ops)
>>>>>> goto no_dma;
>>>>>> @@ -2562,12 +2720,12 @@ static void dw_mci_init_dma(struct dw_mci *host)
>>>>>> goto no_dma;
>>>>>> }
>>>>>>
>>>>>> - host->use_dma = 1;
>>>>>> + host->use_dma = trans_mode;
>>>>>
>>>>> Also confuse, if trans_mode is assigned host->use_dma, can mode value be directly assigned to host->use_dma?
>>>>>
>>
>> Good idea, I will do it. :)
>>
>>>>> trans_mode = TRAMS_MODE_PIO;
>>>>> host->use_dma = trans_mode;
>>>>>
>>
>> [...]
>>
>>>>>>
>>>>>> @@ -2890,7 +3047,7 @@ int dw_mci_probe(struct dw_mci *host)
>>>>>> * Get the host data width - this assumes that HCON has been set with
>>>>>> * the correct values.
>>>>>> */
>>>>>> - i = (mci_readl(host, HCON) >> 7) & 0x7;
>>>>>> + i = SDMMC_GET_HDATA_WIDTH(mci_readl(host, HCON));
>>>>>
>>>>> This is not related with supporting external dma interface.
>>>>> Separate this.
>>>>>
>>
>> Okay, It will be split into another one.
>>
>>>>>> if (!i) {
>>>>>> host->push_data = dw_mci_push_data16;
>>>>>> host->pull_data = dw_mci_pull_data16;
>>
>>
>>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>


--
Best Regards
Shawn Lin