2018-12-11 09:14:19

by Chunyan Zhang

[permalink] [raw]
Subject: [PATCH V4 0/3] Add support for using external dma in SDHCI

Currently the generic SDHCI code in the Linux kernel supports the SD
standard DMA integrated into the host controller but does not have any
support for external DMA controllers implemented using dmaengine meaning
that custom code is needed for any systems that use a generic DMA
controller with SDHCI which in practice means any SDHCI controller that
doesn't have an integrated DMA controller so we should have this as a
generic feature.

There are already a number of controller specific drivers that have dmaengine
code, and some could use sdhci.c actually, but needed to implement mmc_ops->request()
in their specific driver for sending command with external dma using dmaengine
framework, with this patchset, them will take advantage of the generic support.
TI's omap controller is the case as an example.

Any comments are very appreciated.

Thanks,
Chunyan

Changes from v3 (https://lkml.org/lkml/2018/12/4/74):
(The code only on patch 1/3 was revised since v3)
* Moved the check for command data from sdhci_external_dma_setup() to sdhci_external_dma_prepare_data();
* Cut a copy of prepare_data() for external DMA instead of using sdhci_prepare_data() to
avoid the host use ADMA rather than external DMA; In this way, if failed to setup external DMA, host
can fall back to ADMA/SDMA/PIO which standard sd host controller supports.
* Added dmaengine_terminate_sync() when finished transfers or some error occured;
* Adjusted print message in __sdhci_add_host() for the case of using external DMA.

Changes from v2 (https://lkml.org/lkml/2018/11/12/1936):
* Remove CONFIG_EXTERNAL_DMA prompt and help graph;
* Add checking for cmd->data;
* Select MMC_SDHCI_EXTERNAL_DMA for MMC_SDHCI_OMAP;
* Add checking if there's 'dmas' in device tree before decide using external dma.

Changes from v1 (https://lkml.org/lkml/2018/11/5/110):
(The code on patch 1/3 only was revised)
* Address comments from Arnd:
- Release channel when failed to request it unconditionally;
- Skip warning message if get EPROBE_DEFER;
* Address Andrian's comments:
- Replace extdma with external_dma;
- Add release dma resources in sdhci_cleanup_host() and sdhci_remove_host();
- Release dma resources once dmaengine_submit() failed.
- Put rx/tx_chan in struct sdhci_host, and removed unused structure.
* Fall back to the DMA/PIO which standard SDHCI supports, if sdhci_external_dma_setup()
or sdhci_external_dma_init failed;

Chunyan Zhang (3):
mmc: sdhci: add support for using external DMA devices
mmc: sdhci-omap: Add using external dma
dt-bindings: sdhci-omap: Add example for using external dma

.../devicetree/bindings/mmc/sdhci-omap.txt | 7 +
drivers/mmc/host/Kconfig | 4 +
drivers/mmc/host/sdhci-omap.c | 10 +
drivers/mmc/host/sdhci.c | 250 ++++++++++++++++++++-
drivers/mmc/host/sdhci.h | 8 +
5 files changed, 278 insertions(+), 1 deletion(-)

--
2.7.4



2018-12-11 09:14:27

by Chunyan Zhang

[permalink] [raw]
Subject: [PATCH V4 1/3] mmc: sdhci: add support for using external DMA devices

Some standard SD host controllers can support both external dma
controllers as well as ADMA/SDMA in which the SD host controller
acts as DMA master. TI's omap controller is the case as an example.

Currently the generic SDHCI code supports ADMA/SDMA integrated in
the host controller but does not have any support for external DMA
controllers implemented using dmaengine, meaning that custom code is
needed for any systems that use an external DMA controller with SDHCI.

Signed-off-by: Chunyan Zhang <[email protected]>
---
drivers/mmc/host/Kconfig | 3 +
drivers/mmc/host/sdhci.c | 250 ++++++++++++++++++++++++++++++++++++++++++++++-
drivers/mmc/host/sdhci.h | 8 ++
3 files changed, 260 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 1b58739..3101da6 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -977,3 +977,6 @@ config MMC_SDHCI_OMAP
If you have a controller with this interface, say Y or M here.

If unsure, say N.
+
+config MMC_SDHCI_EXTERNAL_DMA
+ bool
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 99bdae5..3162e60 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -14,6 +14,7 @@
*/

#include <linux/delay.h>
+#include <linux/dmaengine.h>
#include <linux/ktime.h>
#include <linux/highmem.h>
#include <linux/io.h>
@@ -1097,6 +1098,221 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
}
}

+#if IS_ENABLED(CONFIG_MMC_SDHCI_EXTERNAL_DMA)
+static int sdhci_external_dma_init(struct sdhci_host *host)
+{
+ int ret = 0;
+ struct mmc_host *mmc = host->mmc;
+
+ host->tx_chan = dma_request_chan(mmc->parent, "tx");
+ if (IS_ERR(host->tx_chan)) {
+ ret = PTR_ERR(host->tx_chan);
+ if (ret != -EPROBE_DEFER)
+ pr_warn("Failed to request TX DMA channel.\n");
+ host->tx_chan = NULL;
+ return ret;
+ }
+
+ host->rx_chan = dma_request_chan(mmc->parent, "rx");
+ if (IS_ERR(host->rx_chan)) {
+ if (host->tx_chan) {
+ dma_release_channel(host->tx_chan);
+ host->tx_chan = NULL;
+ }
+
+ ret = PTR_ERR(host->rx_chan);
+ if (ret != -EPROBE_DEFER)
+ pr_warn("Failed to request RX DMA channel.\n");
+ host->rx_chan = NULL;
+ }
+
+ return ret;
+}
+
+static inline struct dma_chan *
+sdhci_external_dma_channel(struct sdhci_host *host, struct mmc_data *data)
+{
+ return data->flags & MMC_DATA_WRITE ? host->tx_chan : host->rx_chan;
+}
+
+static int sdhci_external_dma_setup(struct sdhci_host *host,
+ struct mmc_command *cmd)
+{
+ int ret, i;
+ struct dma_async_tx_descriptor *desc;
+ struct mmc_data *data = cmd->data;
+ struct dma_chan *chan;
+ struct dma_slave_config cfg;
+ dma_cookie_t cookie;
+
+ if (!host->mapbase)
+ return -EINVAL;
+
+ cfg.src_addr = host->mapbase + SDHCI_BUFFER;
+ cfg.dst_addr = host->mapbase + SDHCI_BUFFER;
+ cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ cfg.src_maxburst = data->blksz / 4;
+ cfg.dst_maxburst = data->blksz / 4;
+
+ /* Sanity check: all the SG entries must be aligned by block size. */
+ for (i = 0; i < data->sg_len; i++) {
+ if ((data->sg + i)->length % data->blksz)
+ return -EINVAL;
+ }
+
+ chan = sdhci_external_dma_channel(host, data);
+
+ ret = dmaengine_slave_config(chan, &cfg);
+ if (ret)
+ return ret;
+
+ desc = dmaengine_prep_slave_sg(chan, data->sg, data->sg_len,
+ mmc_get_dma_dir(data),
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!desc)
+ return -EINVAL;
+
+ desc->callback = NULL;
+ desc->callback_param = NULL;
+
+ cookie = dmaengine_submit(desc);
+ if (cookie < 0)
+ ret = cookie;
+
+ return ret;
+}
+
+static void sdhci_external_dma_release(struct sdhci_host *host)
+{
+ if (host->tx_chan) {
+ dma_release_channel(host->tx_chan);
+ host->tx_chan = NULL;
+ }
+
+ if (host->rx_chan) {
+ dma_release_channel(host->rx_chan);
+ host->rx_chan = NULL;
+ }
+
+ sdhci_switch_external_dma(host, false);
+}
+
+static int __sdhci_external_dma_prepare_data(struct sdhci_host *host,
+ struct mmc_command *cmd)
+{
+ struct mmc_data *data = cmd->data;
+ int sg_cnt;
+
+ host->data_timeout = 0;
+
+ if (sdhci_data_line_cmd(cmd))
+ sdhci_set_timeout(host, cmd);
+
+ WARN_ON(host->data);
+
+ /* Sanity checks */
+ WARN_ON(data->blksz * data->blocks > 524288);
+ WARN_ON(data->blksz > host->mmc->max_blk_size);
+ WARN_ON(data->blocks > 65535);
+
+ host->flags |= SDHCI_REQ_USE_DMA;
+ host->data = data;
+ host->data_early = 0;
+ host->data->bytes_xfered = 0;
+
+ sg_cnt = sdhci_pre_dma_transfer(host, data, COOKIE_MAPPED);
+ if (sg_cnt <= 0)
+ return -EINVAL;
+
+ sdhci_set_transfer_irqs(host);
+
+ /*
+ * For Version 4.10 onwards, if v4 mode is enabled, 32-bit Block Count
+ * can be supported, in that case 16-bit block count register must be 0.
+ */
+ if (host->version >= SDHCI_SPEC_410 && host->v4_mode &&
+ (host->quirks2 & SDHCI_QUIRK2_USE_32BIT_BLK_CNT)) {
+ if (sdhci_readw(host, SDHCI_BLOCK_COUNT))
+ sdhci_writew(host, 0, SDHCI_BLOCK_COUNT);
+ sdhci_writew(host, data->blocks, SDHCI_32BIT_BLK_CNT);
+ } else {
+ sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
+ }
+
+ return 0;
+}
+
+static void sdhci_external_dma_prepare_data(struct sdhci_host *host,
+ struct mmc_command *cmd)
+{
+ struct mmc_data *data = cmd->data;
+
+ if (!data)
+ return;
+
+ if (sdhci_external_dma_setup(host, cmd) ||
+ __sdhci_external_dma_prepare_data(host, cmd)) {
+ sdhci_external_dma_release(host);
+ pr_err("%s: Cannot use external DMA, switch to the DMA/PIO which standard SDHCI provides.\n",
+ mmc_hostname(host->mmc));
+ sdhci_prepare_data(host, cmd);
+ }
+}
+
+static void sdhci_external_dma_pre_transfer(struct sdhci_host *host,
+ struct mmc_command *cmd)
+{
+ struct dma_chan *chan = sdhci_external_dma_channel(host, cmd->data);
+
+ if (cmd->data && chan)
+ dma_async_issue_pending(chan);
+}
+
+static int sdhci_external_dma_cleanup(struct sdhci_host *host,
+ struct mmc_data *data)
+{
+ struct dma_chan *chan = sdhci_external_dma_channel(host, data);
+ int ret = 0;
+
+ if (chan)
+ ret = dmaengine_terminate_sync(chan);
+
+ return ret;
+}
+#else
+static int sdhci_external_dma_init(struct sdhci_host *host)
+{
+ return -EOPNOTSUPP;
+}
+
+static void sdhci_external_dma_release(struct sdhci_host *host)
+{}
+
+static void sdhci_external_dma_prepare_data(struct sdhci_host *host,
+ struct mmc_command *cmd)
+{
+ /* If MMC_SDHCI_EXTERNAL_DMA not supported, PIO will be used */
+ sdhci_prepare_data(host, cmd);
+}
+
+static void sdhci_external_dma_pre_transfer(struct sdhci_host *host,
+ struct mmc_command *cmd)
+{}
+
+static int sdhci_external_dma_cleanup(struct sdhci_host *host,
+ struct mmc_data *data)
+{
+ return 0;
+}
+#endif
+
+void sdhci_switch_external_dma(struct sdhci_host *host, bool en)
+{
+ host->use_external_dma = en;
+}
+EXPORT_SYMBOL_GPL(sdhci_switch_external_dma);
+
static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
struct mmc_request *mrq)
{
@@ -1273,6 +1489,9 @@ static void sdhci_finish_data(struct sdhci_host *host)
if (!host->cmd || host->cmd == data_cmd)
sdhci_do_reset(host, SDHCI_RESET_CMD);
sdhci_do_reset(host, SDHCI_RESET_DATA);
+
+ if (host->use_external_dma)
+ sdhci_external_dma_cleanup(host, data);
}

/*
@@ -1355,7 +1574,10 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
host->data_cmd = cmd;
}

- sdhci_prepare_data(host, cmd);
+ if (host->use_external_dma)
+ sdhci_external_dma_prepare_data(host, cmd);
+ else
+ sdhci_prepare_data(host, cmd);

sdhci_writel(host, cmd->arg, SDHCI_ARGUMENT);

@@ -1397,6 +1619,9 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
timeout += 10 * HZ;
sdhci_mod_timer(host, cmd->mrq, timeout);

+ if (host->use_external_dma)
+ sdhci_external_dma_pre_transfer(host, cmd);
+
sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
}
EXPORT_SYMBOL_GPL(sdhci_send_command);
@@ -2635,6 +2860,8 @@ static bool sdhci_request_done(struct sdhci_host *host)
dma_unmap_sg(mmc_dev(host->mmc), data->sg,
data->sg_len,
mmc_get_dma_dir(data));
+ if (host->use_external_dma)
+ sdhci_external_dma_cleanup(host, data);
}
data->host_cookie = COOKIE_UNMAPPED;
}
@@ -3742,6 +3969,19 @@ int sdhci_setup_host(struct sdhci_host *host)
}
}

+ if (host->use_external_dma) {
+ ret = sdhci_external_dma_init(host);
+ if (ret == -EPROBE_DEFER)
+ goto unreg;
+
+ /*
+ * Fall back to use the DMA/PIO integrated in standard SDHCI
+ * instead of external DMA devices.
+ */
+ if (ret)
+ sdhci_switch_external_dma(host, false);
+ }
+
/*
* If we use DMA, then it's up to the caller to set the DMA
* mask, but PIO does not need the hw shim so we set a new
@@ -4161,6 +4401,10 @@ void sdhci_cleanup_host(struct sdhci_host *host)
dma_free_coherent(mmc_dev(mmc), host->align_buffer_sz +
host->adma_table_sz, host->align_buffer,
host->align_addr);
+
+ if (host->use_external_dma)
+ sdhci_external_dma_release(host);
+
host->adma_table = NULL;
host->align_buffer = NULL;
}
@@ -4207,6 +4451,7 @@ int __sdhci_add_host(struct sdhci_host *host)

pr_info("%s: SDHCI controller on %s [%s] using %s\n",
mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
+ host->use_external_dma ? "External DMA" :
(host->flags & SDHCI_USE_ADMA) ?
(host->flags & SDHCI_USE_64_BIT_DMA) ? "ADMA 64-bit" : "ADMA" :
(host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO");
@@ -4295,6 +4540,9 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
host->adma_table_sz, host->align_buffer,
host->align_addr);

+ if (host->use_external_dma)
+ sdhci_external_dma_release(host);
+
host->adma_table = NULL;
host->align_buffer = NULL;
}
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index b001cf4..98d3953 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -475,6 +475,7 @@ struct sdhci_host {

int irq; /* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
+ phys_addr_t mapbase; /* physical address base */
char *bounce_buffer; /* For packing SDMA reads/writes */
dma_addr_t bounce_addr;
unsigned int bounce_buffer_size;
@@ -524,6 +525,7 @@ struct sdhci_host {
bool pending_reset; /* Cmd/data reset is pending */
bool irq_wake_enabled; /* IRQ wakeup is enabled */
bool v4_mode; /* Host Version 4 Enable */
+ bool use_external_dma; /* Host selects to use external DMA */

struct mmc_request *mrqs_done[SDHCI_MAX_MRQS]; /* Requests done */
struct mmc_command *cmd; /* Current command */
@@ -552,6 +554,11 @@ struct sdhci_host {
struct timer_list timer; /* Timer for timeouts */
struct timer_list data_timer; /* Timer for data timeouts */

+#if IS_ENABLED(CONFIG_MMC_SDHCI_EXTERNAL_DMA)
+ struct dma_chan *rx_chan;
+ struct dma_chan *tx_chan;
+#endif
+
u32 caps; /* CAPABILITY_0 */
u32 caps1; /* CAPABILITY_1 */
bool read_caps; /* Capability flags have been read */
@@ -785,5 +792,6 @@ void sdhci_start_tuning(struct sdhci_host *host);
void sdhci_end_tuning(struct sdhci_host *host);
void sdhci_reset_tuning(struct sdhci_host *host);
void sdhci_send_tuning(struct sdhci_host *host, u32 opcode);
+void sdhci_switch_external_dma(struct sdhci_host *host, bool en);

#endif /* __SDHCI_HW_H */
--
2.7.4


2018-12-11 09:15:27

by Chunyan Zhang

[permalink] [raw]
Subject: [PATCH V4 3/3] dt-bindings: sdhci-omap: Add example for using external dma

sdhci-omap can support both external dma controller via dmaengine
framework as well as ADMA which standard SD host controller
provides.

Signed-off-by: Chunyan Zhang <[email protected]>
---
Documentation/devicetree/bindings/mmc/sdhci-omap.txt | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
index 393848c..c73fd47 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
@@ -12,6 +12,11 @@ Required properties:
"ddr_1_8v-rev11", "ddr_1_8v" or "ddr_3_3v", "hs200_1_8v-rev11",
"hs200_1_8v",
- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt
+- dmas: List of DMA specifiers with the controller specific format as described
+ in the generic DMA client binding. A tx and rx specifier is required.
+- dma-names: List of DMA request names. These strings correspond 1:1 with the
+ DMA specifiers listed in dmas. The string naming is to be "rx"
+ and "tx" for RX and TX DMA requests, respectively.

Example:
mmc1: mmc@4809c000 {
@@ -20,4 +25,6 @@ Example:
ti,hwmods = "mmc1";
bus-width = <4>;
vmmc-supply = <&vmmc>; /* phandle to regulator node */
+ dmas = <&sdma 61 &sdma 62>;
+ dma-names = "tx", "rx";
};
--
2.7.4


2018-12-11 09:15:29

by Chunyan Zhang

[permalink] [raw]
Subject: [PATCH V4 2/3] mmc: sdhci-omap: Add using external dma

sdhci-omap can support both external dma controller via dmaengine framework
as well as ADMA which standard SD host controller provides.

Signed-off-by: Chunyan Zhang <[email protected]>
---
drivers/mmc/host/Kconfig | 1 +
drivers/mmc/host/sdhci-omap.c | 10 ++++++++++
2 files changed, 11 insertions(+)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 3101da6..7846754 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -969,6 +969,7 @@ config MMC_SDHCI_XENON
config MMC_SDHCI_OMAP
tristate "TI SDHCI Controller Support"
depends on MMC_SDHCI_PLTFM && OF
+ select MMC_SDHCI_EXTERNAL_DMA if DMA_ENGINE
help
This selects the Secure Digital Host Controller Interface (SDHCI)
support present in TI's DRA7 SOCs. The controller supports
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 88347ce..b164fcc 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -896,6 +896,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
const struct of_device_id *match;
struct sdhci_omap_data *data;
const struct soc_device_attribute *soc;
+ struct resource *regs;

match = of_match_device(omap_sdhci_match, dev);
if (!match)
@@ -908,6 +909,10 @@ static int sdhci_omap_probe(struct platform_device *pdev)
}
offset = data->offset;

+ regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!regs)
+ return -ENXIO;
+
host = sdhci_pltfm_init(pdev, &sdhci_omap_pdata,
sizeof(*omap_host));
if (IS_ERR(host)) {
@@ -924,6 +929,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
omap_host->timing = MMC_TIMING_LEGACY;
omap_host->flags = data->flags;
host->ioaddr += offset;
+ host->mapbase = regs->start;

mmc = host->mmc;
sdhci_get_of_property(pdev);
@@ -991,6 +997,10 @@ static int sdhci_omap_probe(struct platform_device *pdev)
host->mmc_host_ops.execute_tuning = sdhci_omap_execute_tuning;
host->mmc_host_ops.enable_sdio_irq = sdhci_omap_enable_sdio_irq;

+ /* Switch to external DMA only if there is the "dmas" property */
+ if (of_find_property(dev->of_node, "dmas", NULL))
+ sdhci_switch_external_dma(host, true);
+
ret = sdhci_setup_host(host);
if (ret)
goto err_put_sync;
--
2.7.4


2018-12-13 12:06:37

by Faiz Abbas

[permalink] [raw]
Subject: Re: [PATCH V4 1/3] mmc: sdhci: add support for using external DMA devices

Hi Chunyan,

On 11/12/18 2:42 PM, Chunyan Zhang wrote:
> Some standard SD host controllers can support both external dma
> controllers as well as ADMA/SDMA in which the SD host controller
> acts as DMA master. TI's omap controller is the case as an example.
>
> Currently the generic SDHCI code supports ADMA/SDMA integrated in
> the host controller but does not have any support for external DMA
> controllers implemented using dmaengine, meaning that custom code is
> needed for any systems that use an external DMA controller with SDHCI.
>
> Signed-off-by: Chunyan Zhang <[email protected]>
> ---
> drivers/mmc/host/Kconfig | 3 +
> drivers/mmc/host/sdhci.c | 250 ++++++++++++++++++++++++++++++++++++++++++++++-
> drivers/mmc/host/sdhci.h | 8 ++
> 3 files changed, 260 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 1b58739..3101da6 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -977,3 +977,6 @@ config MMC_SDHCI_OMAP
> If you have a controller with this interface, say Y or M here.
>
> If unsure, say N.
> +
> +config MMC_SDHCI_EXTERNAL_DMA
> + bool
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 99bdae5..3162e60 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -14,6 +14,7 @@
> */
>
> #include <linux/delay.h>
> +#include <linux/dmaengine.h>
> #include <linux/ktime.h>
> #include <linux/highmem.h>
> #include <linux/io.h>
> @@ -1097,6 +1098,221 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
> }
> }
>
> +#if IS_ENABLED(CONFIG_MMC_SDHCI_EXTERNAL_DMA)
> +static int sdhci_external_dma_init(struct sdhci_host *host)
> +{
> + int ret = 0;
> + struct mmc_host *mmc = host->mmc;
> +
> + host->tx_chan = dma_request_chan(mmc->parent, "tx");
> + if (IS_ERR(host->tx_chan)) {
> + ret = PTR_ERR(host->tx_chan);
> + if (ret != -EPROBE_DEFER)
> + pr_warn("Failed to request TX DMA channel.\n");
> + host->tx_chan = NULL;
> + return ret;
> + }
> +
> + host->rx_chan = dma_request_chan(mmc->parent, "rx");
> + if (IS_ERR(host->rx_chan)) {
> + if (host->tx_chan) {
> + dma_release_channel(host->tx_chan);
> + host->tx_chan = NULL;
> + }
> +
> + ret = PTR_ERR(host->rx_chan);
> + if (ret != -EPROBE_DEFER)
> + pr_warn("Failed to request RX DMA channel.\n");
> + host->rx_chan = NULL;
> + }
> +
> + return ret;
> +}
> +
> +static inline struct dma_chan *
> +sdhci_external_dma_channel(struct sdhci_host *host, struct mmc_data *data)
> +{
> + return data->flags & MMC_DATA_WRITE ? host->tx_chan : host->rx_chan;
> +}
> +
> +static int sdhci_external_dma_setup(struct sdhci_host *host,
> + struct mmc_command *cmd)
> +{
> + int ret, i;
> + struct dma_async_tx_descriptor *desc;
> + struct mmc_data *data = cmd->data;
> + struct dma_chan *chan;
> + struct dma_slave_config cfg;
> + dma_cookie_t cookie;
> +
> + if (!host->mapbase)
> + return -EINVAL;
> +
> + cfg.src_addr = host->mapbase + SDHCI_BUFFER;
> + cfg.dst_addr = host->mapbase + SDHCI_BUFFER;
> + cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + cfg.src_maxburst = data->blksz / 4;
> + cfg.dst_maxburst = data->blksz / 4;
> +
> + /* Sanity check: all the SG entries must be aligned by block size. */
> + for (i = 0; i < data->sg_len; i++) {
> + if ((data->sg + i)->length % data->blksz)
> + return -EINVAL;
> + }
> +
> + chan = sdhci_external_dma_channel(host, data);
> +
> + ret = dmaengine_slave_config(chan, &cfg);
> + if (ret)
> + return ret;
> +
> + desc = dmaengine_prep_slave_sg(chan, data->sg, data->sg_len,
> + mmc_get_dma_dir(data),
> + DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> + if (!desc)
> + return -EINVAL;
> +
> + desc->callback = NULL;
> + desc->callback_param = NULL;
> +
> + cookie = dmaengine_submit(desc);
> + if (cookie < 0)
> + ret = cookie;
> +
> + return ret;
> +}
> +
> +static void sdhci_external_dma_release(struct sdhci_host *host)
> +{
> + if (host->tx_chan) {
> + dma_release_channel(host->tx_chan);
> + host->tx_chan = NULL;
> + }
> +
> + if (host->rx_chan) {
> + dma_release_channel(host->rx_chan);
> + host->rx_chan = NULL;
> + }
> +
> + sdhci_switch_external_dma(host, false);
> +}
> +
> +static int __sdhci_external_dma_prepare_data(struct sdhci_host *host,
> + struct mmc_command *cmd)
> +{
> + struct mmc_data *data = cmd->data;
> + int sg_cnt;
> +
> + host->data_timeout = 0;
> +
> + if (sdhci_data_line_cmd(cmd))
> + sdhci_set_timeout(host, cmd);
> +
> + WARN_ON(host->data);
> +
> + /* Sanity checks */
> + WARN_ON(data->blksz * data->blocks > 524288);
> + WARN_ON(data->blksz > host->mmc->max_blk_size);
> + WARN_ON(data->blocks > 65535);
> +
> + host->flags |= SDHCI_REQ_USE_DMA;
> + host->data = data;
> + host->data_early = 0;
> + host->data->bytes_xfered = 0;
> +
> + sg_cnt = sdhci_pre_dma_transfer(host, data, COOKIE_MAPPED);
> + if (sg_cnt <= 0)
> + return -EINVAL;
> +
> + sdhci_set_transfer_irqs(host);
> +
> + /*
> + * For Version 4.10 onwards, if v4 mode is enabled, 32-bit Block Count
> + * can be supported, in that case 16-bit block count register must be 0.
> + */
> + if (host->version >= SDHCI_SPEC_410 && host->v4_mode &&
> + (host->quirks2 & SDHCI_QUIRK2_USE_32BIT_BLK_CNT)) {
> + if (sdhci_readw(host, SDHCI_BLOCK_COUNT))
> + sdhci_writew(host, 0, SDHCI_BLOCK_COUNT);
> + sdhci_writew(host, data->blocks, SDHCI_32BIT_BLK_CNT);
> + } else {
> + sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
> + }
> +
> + return 0;
> +}
> +
> +static void sdhci_external_dma_prepare_data(struct sdhci_host *host,
> + struct mmc_command *cmd)
> +{
> + struct mmc_data *data = cmd->data;
> +
> + if (!data)
> + return;
> +
> + if (sdhci_external_dma_setup(host, cmd) ||
> + __sdhci_external_dma_prepare_data(host, cmd)) {
> + sdhci_external_dma_release(host);
> + pr_err("%s: Cannot use external DMA, switch to the DMA/PIO which standard SDHCI provides.\n",
> + mmc_hostname(host->mmc));
> + sdhci_prepare_data(host, cmd);
> + }
> +}
> +
> +static void sdhci_external_dma_pre_transfer(struct sdhci_host *host,
> + struct mmc_command *cmd)
> +{
> + struct dma_chan *chan = sdhci_external_dma_channel(host, cmd->data);

Again, cmd->data can be NULL.

Even after fixing above, I get some l3_noc issues.

[ 3.589896] omap_l3_noc 44000000.ocp: L3 application error: target 5
mod:1 (unclearable)
[ 3.598157] omap_l3_noc 44000000.ocp: L3 debug error: target 5 mod:1
(unclearable)
[ 3.630934] softirq: Attempt to kill tasklet from interrupt
[ 3.636790] mmc0: error -84 whilst initialising SD card

Full log: https://pastebin.ubuntu.com/p/wdG88hX5By/

I am in the process of debugging this. Please ping if you need any
further logs.

Thanks,
Faiz



2018-12-27 21:28:40

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH V4 1/3] mmc: sdhci: add support for using external DMA devices

On 11/12/18 11:12 AM, Chunyan Zhang wrote:
> Some standard SD host controllers can support both external dma
> controllers as well as ADMA/SDMA in which the SD host controller
> acts as DMA master. TI's omap controller is the case as an example.
>
> Currently the generic SDHCI code supports ADMA/SDMA integrated in
> the host controller but does not have any support for external DMA
> controllers implemented using dmaengine, meaning that custom code is
> needed for any systems that use an external DMA controller with SDHCI.
>
> Signed-off-by: Chunyan Zhang <[email protected]>

FYI, I am waiting on successful testing before reviewing these patches again.

2018-12-27 21:34:43

by Chunyan Zhang

[permalink] [raw]
Subject: Re: [PATCH V4 1/3] mmc: sdhci: add support for using external DMA devices

On Thu, 27 Dec 2018 at 15:44, Adrian Hunter <[email protected]> wrote:
>
> On 11/12/18 11:12 AM, Chunyan Zhang wrote:
> > Some standard SD host controllers can support both external dma
> > controllers as well as ADMA/SDMA in which the SD host controller
> > acts as DMA master. TI's omap controller is the case as an example.
> >
> > Currently the generic SDHCI code supports ADMA/SDMA integrated in
> > the host controller but does not have any support for external DMA
> > controllers implemented using dmaengine, meaning that custom code is
> > needed for any systems that use an external DMA controller with SDHCI.
> >
> > Signed-off-by: Chunyan Zhang <[email protected]>
>
> FYI, I am waiting on successful testing before reviewing these patches again.

Yeah, that makes sense, thanks Adrian!

I intend to send out another new patchset after the holidays.
After had another look at omap_hsmmc, I knew something need to be
modified (emailed Faiz about this privately), but I was still not sure
if it can fix the problem Faiz found.

Happy new year to all,
Chunyan

2019-01-02 09:19:06

by Faiz Abbas

[permalink] [raw]
Subject: Re: [PATCH V4 1/3] mmc: sdhci: add support for using external DMA devices

Adrian,

On 27/12/18 1:32 PM, Chunyan Zhang wrote:
> On Thu, 27 Dec 2018 at 15:44, Adrian Hunter <[email protected]> wrote:
>>
>> On 11/12/18 11:12 AM, Chunyan Zhang wrote:
>>> Some standard SD host controllers can support both external dma
>>> controllers as well as ADMA/SDMA in which the SD host controller
>>> acts as DMA master. TI's omap controller is the case as an example.
>>>
>>> Currently the generic SDHCI code supports ADMA/SDMA integrated in
>>> the host controller but does not have any support for external DMA
>>> controllers implemented using dmaengine, meaning that custom code is
>>> needed for any systems that use an external DMA controller with SDHCI.
>>>
>>> Signed-off-by: Chunyan Zhang <[email protected]>
>>
>> FYI, I am waiting on successful testing before reviewing these patches again.
>
> Yeah, that makes sense, thanks Adrian!
>
> I intend to send out another new patchset after the holidays.
> After had another look at omap_hsmmc, I knew something need to be
> modified (emailed Faiz about this privately), but I was still not sure
> if it can fix the problem Faiz found.
>

I am currently working on moving am335 and am43 devices from omap_hsmmc
to sdhci-omap driver. External DMA would be an integral part of this.
The plan is to post these patches as a part of my series after testing
on all those devices.

Thanks,
Faiz