[PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops

STALE2887d

25 messages, 2 authors, 2018-09-05 · open the first message on its own page

[PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:38:53

From: Ludovic Barre <redacted>

This patch series prepares and adds callbacks for dma transfert at
mmci_host_ops. This series is composed of 3 parts:
-Internalize specific needs of legacy dmaengine.
-Create and setup dma_priv pointer
-Create generic callbacks  which share some features
(like cookie...) and call specific needs

This patch series must be applied on top of 
"mmc: mmci: Add and implement a ->dma_setup() callback for qcom dml"

Ludovic Barre (14):
  mmc: mmci: fix qcom dma issue during mmci init with new dma_setup
    callback
  mmc: mmci: internalize dma map/unmap into mmci dma functions
  mmc: mmci: internalize dma_inprogress into mmci dma functions
  mmc: mmci: introduce dma_priv pointer to mmci_host
  mmc: mmci: move mmci next cookie to mci host
  mmc: mmci: merge prepare data functions
  mmc: mmci: add prepare/unprepare_data callbacks
  mmc: mmci: add get_next_data callback
  mmc: mmci: modify dma_setup callback
  mmc: mmci: add dma_release callback
  mmc: mmci: add dma_start callback
  mmc: mmci: add dma_finalize callback
  mmc: mmci: add dma_error callback
  mmc: mmci: add validate_data callback

 drivers/mmc/host/mmci.c          | 458 ++++++++++++++++++++++++---------------
 drivers/mmc/host/mmci.h          |  45 ++--
 drivers/mmc/host/mmci_qcom_dml.c |  15 +-
 3 files changed, 322 insertions(+), 196 deletions(-)

-- 
2.7.4

[PATCH 03/14] mmc: mmci: internalize dma_inprogress into mmci dma functions

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:37:45

From: Ludovic Barre <redacted>

This patch internalizes the dma_inprogress into mmci dma interfaces.
This allows to simplify and prepare the next dma callbacks
for mmci host ops. __dma_inprogress is called in mmci_dma_data_error
and mmci_dma_finalize.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c | 16 ++++++++++------
 drivers/mmc/host/mmci.h |  4 +---
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index d8fa178..8144a87 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -497,6 +497,9 @@ static void __mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)
 
 static void mmci_dma_data_error(struct mmci_host *host)
 {
+	if (!__dma_inprogress(host))
+		return;
+
 	dev_err(mmc_dev(host->mmc), "error during DMA transfer!\n");
 	dmaengine_terminate_all(host->dma_current);
 	host->dma_in_progress = false;

@@ -512,6 +515,9 @@ static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 	u32 status;
 	int i;
 
+	if (!__dma_inprogress(dmae))
+		return;
+
 	/* Wait up to 1ms for the DMA to complete */
 	for (i = 0; ; i++) {
 		status = readl(host->base + MMCISTATUS);

@@ -903,8 +909,7 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		u32 remain, success;
 
 		/* Terminate the DMA transfer */
-		if (dma_inprogress(host))
-			mmci_dma_data_error(host);
+		mmci_dma_data_error(host);
 
 		/*
 		 * Calculate how far we are into the transfer.  Note that

@@ -942,8 +947,8 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		dev_err(mmc_dev(host->mmc), "stray MCI_DATABLOCKEND interrupt\n");
 
 	if (status & MCI_DATAEND || data->error) {
-		if (dma_inprogress(host))
-			mmci_dma_finalize(host, data);
+		mmci_dma_finalize(host, data);
+
 		mmci_stop_data(host);
 
 		if (!data->error)

@@ -1050,8 +1055,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	if ((!sbc && !cmd->data) || cmd->error) {
 		if (host->data) {
 			/* Terminate the DMA transfer */
-			if (dma_inprogress(host))
-				mmci_dma_data_error(host);
+			mmci_dma_data_error(host);
 
 			mmci_stop_data(host);
 		}

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 696a066..f1ec066 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -332,9 +332,7 @@ struct mmci_host {
 	struct mmci_host_next	next_data;
 	bool			dma_in_progress;
 
-#define dma_inprogress(host)	((host)->dma_in_progress)
-#else
-#define dma_inprogress(host)	(0)
+#define __dma_inprogress(host)	((host)->dma_in_progress)
 #endif
 };

-- 
2.7.4

Re: [PATCH 03/14] mmc: mmci: internalize dma_inprogress into mmci dma functions

From: Ulf Hansson <hidden>
Date: 2018-09-03 12:15:11

On 1 August 2018 at 11:36, Ludovic Barre [off-list ref] wrote:

From: Ludovic Barre <redacted>

This patch internalizes the dma_inprogress into mmci dma interfaces.
This allows to simplify and prepare the next dma callbacks
for mmci host ops. __dma_inprogress is called in mmci_dma_data_error
and mmci_dma_finalize.

Signed-off-by: Ludovic Barre <redacted>

[...]

quoted hunk

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 696a066..f1ec066 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -332,9 +332,7 @@ struct mmci_host {
        struct mmci_host_next   next_data;
        bool                    dma_in_progress;

-#define dma_inprogress(host)   ((host)->dma_in_progress)
-#else
-#define dma_inprogress(host)   (0)
+#define __dma_inprogress(host) ((host)->dma_in_progress)

Please keep the existing function name.

If there are good reasons to change it, please make it a part of a
change where it makes sense.

 #endif
 };

--
2.7.4

Besides the nitpick above, this looks good to me!

Kind regards
Uffe

[PATCH 07/14] mmc: mmci: add prepare/unprepare_data callbacks

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:37:48

From: Ludovic Barre <redacted>

This patch adds prepare/unprepare callbacks to mmci_host_ops.
Like this mmci_pre/post_request can be generic, mmci_prepare_data
and mmci_unprepare_data provide common next_cookie management.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c          | 118 +++++++++++++++++++++++++++------------
 drivers/mmc/host/mmci.h          |  10 ++++
 drivers/mmc/host/mmci_qcom_dml.c |   2 +
 3 files changed, 93 insertions(+), 37 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e4d80f1..345aa2e 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -58,6 +58,7 @@ static struct variant_data variant_arm = {
 	.mmcimask1		= true,
 	.start_err		= MCI_STARTBITERR,
 	.opendrain		= MCI_ROD,
+	.init			= mmci_variant_init,
 };
 
 static struct variant_data variant_arm_extended_fifo = {

@@ -69,6 +70,7 @@ static struct variant_data variant_arm_extended_fifo = {
 	.mmcimask1		= true,
 	.start_err		= MCI_STARTBITERR,
 	.opendrain		= MCI_ROD,
+	.init			= mmci_variant_init,
 };
 
 static struct variant_data variant_arm_extended_fifo_hwfc = {

@@ -81,6 +83,7 @@ static struct variant_data variant_arm_extended_fifo_hwfc = {
 	.mmcimask1		= true,
 	.start_err		= MCI_STARTBITERR,
 	.opendrain		= MCI_ROD,
+	.init			= mmci_variant_init,
 };
 
 static struct variant_data variant_u300 = {

@@ -99,6 +102,7 @@ static struct variant_data variant_u300 = {
 	.mmcimask1		= true,
 	.start_err		= MCI_STARTBITERR,
 	.opendrain		= MCI_OD,
+	.init			= mmci_variant_init,
 };
 
 static struct variant_data variant_nomadik = {

@@ -118,6 +122,7 @@ static struct variant_data variant_nomadik = {
 	.mmcimask1		= true,
 	.start_err		= MCI_STARTBITERR,
 	.opendrain		= MCI_OD,
+	.init			= mmci_variant_init,
 };
 
 static struct variant_data variant_ux500 = {

@@ -143,6 +148,7 @@ static struct variant_data variant_ux500 = {
 	.mmcimask1		= true,
 	.start_err		= MCI_STARTBITERR,
 	.opendrain		= MCI_OD,
+	.init			= mmci_variant_init,
 };
 
 static struct variant_data variant_ux500v2 = {

@@ -170,6 +176,7 @@ static struct variant_data variant_ux500v2 = {
 	.mmcimask1		= true,
 	.start_err		= MCI_STARTBITERR,
 	.opendrain		= MCI_OD,
+	.init			= mmci_variant_init,
 };
 
 static struct variant_data variant_stm32 = {

@@ -187,6 +194,7 @@ static struct variant_data variant_stm32 = {
 	.f_max			= 48000000,
 	.pwrreg_clkgate		= true,
 	.pwrreg_nopower		= true,
+	.init			= mmci_variant_init,
 };
 
 static struct variant_data variant_qcom = {

@@ -357,6 +365,31 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 	mmci_write_clkreg(host, clk);
 }
 
+int mmci_prepare_data(struct mmci_host *host, struct mmc_data *data, bool next)
+{
+	int err;
+
+	if (!host->ops || !host->ops->prepare_data)
+		return 0;
+
+	err = host->ops->prepare_data(host, data, next);
+
+	if (next && !err)
+		data->host_cookie = ++host->next_cookie < 0 ?
+			1 : host->next_cookie;
+
+	return err;
+}
+
+void mmci_unprepare_data(struct mmci_host *host, struct mmc_data *data,
+			 int err)
+{
+	if (host->ops && host->ops->unprepare_data)
+		host->ops->unprepare_data(host, data, err);
+
+	data->host_cookie = 0;
+}
+
 static void
 mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {

@@ -588,9 +621,9 @@ static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 }
 
 /* prepares DMA channel and DMA descriptor, returns non-zero on failure */
-static int __mmci_dma_prep_data(struct mmci_host *host, struct mmc_data *data,
-				struct dma_chan **dma_chan,
-				struct dma_async_tx_descriptor **dma_desc)
+static int __mmci_dmae_prep_data(struct mmci_host *host, struct mmc_data *data,
+				 struct dma_chan **dma_chan,
+				 struct dma_async_tx_descriptor **dma_desc)
 {
 	struct dmaengine_priv *dmae = host->dma_priv;
 	struct variant_data *variant = host->variant;

@@ -651,22 +684,21 @@ static int __mmci_dma_prep_data(struct mmci_host *host, struct mmc_data *data,
 	return -ENOMEM;
 }
 
-static inline int mmci_dma_prepare_data(struct mmci_host *host,
-					struct mmc_data *data,
-					bool next)
+int mmci_dmae_prepare_data(struct mmci_host *host,
+			   struct mmc_data *data, bool next)
 {
 	struct dmaengine_priv *dmae = host->dma_priv;
 	struct dmaengine_next *nd = &dmae->next_data;
 
 	if (next)
-		return __mmci_dma_prep_data(host, data, &nd->dma_chan,
+		return __mmci_dmae_prep_data(host, data, &nd->dma_chan,
 					    &nd->dma_desc);
 	/* Check if next job is already prepared. */
 	if (dmae->dma_current && dmae->dma_desc_current)
 		return 0;
 
 	/* No job were prepared thus do it now. */
-	return __mmci_dma_prep_data(host, data, &dmae->dma_current,
+	return __mmci_dmae_prep_data(host, data, &dmae->dma_current,
 				    &dmae->dma_desc_current);
 }

@@ -676,7 +708,7 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 	struct mmc_data *data = host->data;
 	int ret;
 
-	ret = mmci_dma_prepare_data(host, host->data, false);
+	ret = mmci_prepare_data(host, host->data, false);
 	if (ret)
 		return ret;

@@ -720,33 +752,11 @@ static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
 	next->dma_chan = NULL;
 }
 
-static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
-{
-	struct mmci_host *host = mmc_priv(mmc);
-	struct mmc_data *data = mrq->data;
-
-	if (!data)
-		return;
-
-	BUG_ON(data->host_cookie);
-
-	if (mmci_validate_data(host, data))
-		return;
+void mmci_dmae_unprepare_data(struct mmci_host *host,
+			      struct mmc_data *data, int err)
 
-	if (!mmci_dma_prepare_data(host, data, true))
-		data->host_cookie = ++host->next_cookie < 0 ?
-			1 : host->next_cookie;
-}
-
-static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
-			      int err)
 {
-	struct mmci_host *host = mmc_priv(mmc);
 	struct dmaengine_priv *dmae = host->dma_priv;
-	struct mmc_data *data = mrq->data;
-
-	if (!data || !data->host_cookie)
-		return;
 
 	__mmci_dmae_unmap(host, data);

@@ -769,10 +779,13 @@ static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
 
 		next->dma_desc = NULL;
 		next->dma_chan = NULL;
-		data->host_cookie = 0;
 	}
 }
 
+static struct mmci_host_ops mmci_variant_ops = {
+	.prepare_data = mmci_dmae_prepare_data,
+	.unprepare_data = mmci_dmae_unprepare_data,
+};
 #else
 /* Blank functions if the DMA engine is not available */
 static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)

@@ -802,11 +815,42 @@ static inline int mmci_dma_start_data(struct mmci_host *host, unsigned int datac
 	return -ENOSYS;
 }
 
-#define mmci_pre_request NULL
-#define mmci_post_request NULL
-
+static struct mmci_host_ops mmci_variant_ops = {};
 #endif
 
+void mmci_variant_init(struct mmci_host *host)
+{
+	host->ops = &mmci_variant_ops;
+}
+
+static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	struct mmci_host *host = mmc_priv(mmc);
+	struct mmc_data *data = mrq->data;
+
+	if (!data)
+		return;
+
+	WARN_ON(data->host_cookie);
+
+	if (mmci_validate_data(host, data))
+		return;
+
+	mmci_prepare_data(host, data, true);
+}
+
+static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
+			      int err)
+{
+	struct mmci_host *host = mmc_priv(mmc);
+	struct mmc_data *data = mrq->data;
+
+	if (!data || !data->host_cookie)
+		return;
+
+	mmci_unprepare_data(host, data, err);
+}
+
 static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 {
 	struct variant_data *variant = host->variant;

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index d2ec4fd..fa2702b 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -273,6 +273,10 @@ struct variant_data {
 
 /* mmci variant callbacks */
 struct mmci_host_ops {
+	int (*prepare_data)(struct mmci_host *host, struct mmc_data *data,
+			    bool next);
+	void (*unprepare_data)(struct mmci_host *host, struct mmc_data *data,
+			       int err);
 	int (*dma_setup)(struct mmci_host *host);
 };

@@ -323,3 +327,9 @@ struct mmci_host {
 };
 
 void qcom_variant_init(struct mmci_host *host);
+void mmci_variant_init(struct mmci_host *host);
+
+int mmci_dmae_prepare_data(struct mmci_host *host, struct mmc_data *data,
+			   bool next);
+void mmci_dmae_unprepare_data(struct mmci_host *host,
+			      struct mmc_data *data, int err);

diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index 1bb59cf..d534fa1 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c

@@ -180,6 +180,8 @@ static int qcom_dma_setup(struct mmci_host *host)
 }
 
 static struct mmci_host_ops qcom_variant_ops = {
+	.prepare_data = mmci_dmae_prepare_data,
+	.unprepare_data = mmci_dmae_unprepare_data,
 	.dma_setup = qcom_dma_setup,
 };

-- 
2.7.4

Re: [PATCH 07/14] mmc: mmci: add prepare/unprepare_data callbacks

From: Ulf Hansson <hidden>
Date: 2018-09-04 09:43:11

[...]

quoted hunk

 static struct variant_data variant_qcom = {

@@ -357,6 +365,31 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
        mmci_write_clkreg(host, clk);
 }

+int mmci_prepare_data(struct mmci_host *host, struct mmc_data *data, bool next)

I think we need to agree on naming rules for functions, as I think
this becomes a bit hard to follow when you decide to rename and pick
new names.

First, let's strive towards keeping existing names and in cases when a
subset of a function is re-used and called from the original function,
then name it with the same name, but add "__" or "_" as prefixes.

So for this one it should be:

mmci_dma_prep_data()
_mmci_dma_prep_data()
__mmci_dma_prep_data().

That should also decrease the number lines of code you need to change
and thus also make the review easier. Can you please try this?

+{
+       int err;
+
+       if (!host->ops || !host->ops->prepare_data)
+               return 0;

Is the host->ops optional and so also the ->prepare_data() callback in it?

+
+       err = host->ops->prepare_data(host, data, next);

The job of the callback is to map the dma data. Perhaps a better name
would be ->dma_map_data()?

+
+       if (next && !err)
+               data->host_cookie = ++host->next_cookie < 0 ?
+                       1 : host->next_cookie;
+
+       return err;
+}
+
+void mmci_unprepare_data(struct mmci_host *host, struct mmc_data *data,
+                        int err)
+{
+       if (host->ops && host->ops->unprepare_data)
+               host->ops->unprepare_data(host, data, err);

As stated, please follow existing naming convention. Perhaps
->dma_unmap_data() is a better name?

quoted hunk

+
+       data->host_cookie = 0;
+}
+
 static void
 mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {

@@ -588,9 +621,9 @@ static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 }

 /* prepares DMA channel and DMA descriptor, returns non-zero on failure */
-static int __mmci_dma_prep_data(struct mmci_host *host, struct mmc_data *data,
-                               struct dma_chan **dma_chan,
-                               struct dma_async_tx_descriptor **dma_desc)
+static int __mmci_dmae_prep_data(struct mmci_host *host, struct mmc_data *data,
+                                struct dma_chan **dma_chan,
+                                struct dma_async_tx_descriptor **dma_desc)
 {
        struct dmaengine_priv *dmae = host->dma_priv;
        struct variant_data *variant = host->variant;
@@ -651,22 +684,21 @@ static int __mmci_dma_prep_data(struct mmci_host *host, struct mmc_data *data,
        return -ENOMEM;
 }

-static inline int mmci_dma_prepare_data(struct mmci_host *host,
-                                       struct mmc_data *data,
-                                       bool next)
+int mmci_dmae_prepare_data(struct mmci_host *host,
+                          struct mmc_data *data, bool next)

Here's is yet another rename of a function. As stated, try to stick to
the existing names and use the scheme I described above.

quoted hunk

 {
        struct dmaengine_priv *dmae = host->dma_priv;
        struct dmaengine_next *nd = &dmae->next_data;

        if (next)
-               return __mmci_dma_prep_data(host, data, &nd->dma_chan,
+               return __mmci_dmae_prep_data(host, data, &nd->dma_chan,
                                            &nd->dma_desc);
        /* Check if next job is already prepared. */
        if (dmae->dma_current && dmae->dma_desc_current)
                return 0;

        /* No job were prepared thus do it now. */
-       return __mmci_dma_prep_data(host, data, &dmae->dma_current,
+       return __mmci_dmae_prep_data(host, data, &dmae->dma_current,
                                    &dmae->dma_desc_current);
 }

@@ -676,7 +708,7 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
        struct mmc_data *data = host->data;
        int ret;

-       ret = mmci_dma_prepare_data(host, host->data, false);
+       ret = mmci_prepare_data(host, host->data, false);
        if (ret)
                return ret;
@@ -720,33 +752,11 @@ static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
        next->dma_chan = NULL;
 }

-static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
-{
-       struct mmci_host *host = mmc_priv(mmc);
-       struct mmc_data *data = mrq->data;
-
-       if (!data)
-               return;
-
-       BUG_ON(data->host_cookie);
-
-       if (mmci_validate_data(host, data))
-               return;
+void mmci_dmae_unprepare_data(struct mmci_host *host,
+                             struct mmc_data *data, int err)

-       if (!mmci_dma_prepare_data(host, data, true))
-               data->host_cookie = ++host->next_cookie < 0 ?
-                       1 : host->next_cookie;
-}
-
-static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
-                             int err)
 {
-       struct mmci_host *host = mmc_priv(mmc);
        struct dmaengine_priv *dmae = host->dma_priv;
-       struct mmc_data *data = mrq->data;
-
-       if (!data || !data->host_cookie)
-               return;

        __mmci_dmae_unmap(host, data);
@@ -769,10 +779,13 @@ static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,

                next->dma_desc = NULL;
                next->dma_chan = NULL;
-               data->host_cookie = 0;
        }
 }

+static struct mmci_host_ops mmci_variant_ops = {
+       .prepare_data = mmci_dmae_prepare_data,
+       .unprepare_data = mmci_dmae_unprepare_data,
+};
 #else
 /* Blank functions if the DMA engine is not available */
 static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
@@ -802,11 +815,42 @@ static inline int mmci_dma_start_data(struct mmci_host *host, unsigned int datac
        return -ENOSYS;
 }

-#define mmci_pre_request NULL
-#define mmci_post_request NULL
-
+static struct mmci_host_ops mmci_variant_ops = {};
 #endif

Ahh, now I see why you need the mmci_host_ops to be optional, earlier above.

However, I have been thinking on what granularity we should pick for
the mmci host ops callbacks. Honestly I am not sure, but let me
through out an idea and see what you think about it.

So, having callbacks for dealing with dma_map|unmap() kind of
operations, becomes rather fine-grained and not very flexible.
Instead, what do you think of allowing the variant init function to
dynamically assign the ->pre_req() and the ->post_req() callbacks in
the struct mmc_host_ops. Common mmci functions to manage these
operations can instead be shared via mmci.h and called by the
variants.

The point is, following that scheme may improve flexibility, but
possibly also decrease the number of needed mmci specific host ops
callbacks, don't you think?

quoted hunk

+void mmci_variant_init(struct mmci_host *host)
+{
+       host->ops = &mmci_variant_ops;
+}
+
+static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+       struct mmci_host *host = mmc_priv(mmc);
+       struct mmc_data *data = mrq->data;
+
+       if (!data)
+               return;
+
+       WARN_ON(data->host_cookie);
+
+       if (mmci_validate_data(host, data))
+               return;
+
+       mmci_prepare_data(host, data, true);
+}
+
+static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
+                             int err)
+{
+       struct mmci_host *host = mmc_priv(mmc);
+       struct mmc_data *data = mrq->data;
+
+       if (!data || !data->host_cookie)
+               return;
+
+       mmci_unprepare_data(host, data, err);
+}
+
 static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 {
        struct variant_data *variant = host->variant;

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index d2ec4fd..fa2702b 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -273,6 +273,10 @@ struct variant_data {

 /* mmci variant callbacks */
 struct mmci_host_ops {
+       int (*prepare_data)(struct mmci_host *host, struct mmc_data *data,
+                           bool next);
+       void (*unprepare_data)(struct mmci_host *host, struct mmc_data *data,
+                              int err);
        int (*dma_setup)(struct mmci_host *host);
 };

@@ -323,3 +327,9 @@ struct mmci_host {
 };

 void qcom_variant_init(struct mmci_host *host);
+void mmci_variant_init(struct mmci_host *host);
+
+int mmci_dmae_prepare_data(struct mmci_host *host, struct mmc_data *data,
+                          bool next);
+void mmci_dmae_unprepare_data(struct mmci_host *host,
+                             struct mmc_data *data, int err);

diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index 1bb59cf..d534fa1 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c

@@ -180,6 +180,8 @@ static int qcom_dma_setup(struct mmci_host *host)
 }

 static struct mmci_host_ops qcom_variant_ops = {
+       .prepare_data = mmci_dmae_prepare_data,
+       .unprepare_data = mmci_dmae_unprepare_data,
        .dma_setup = qcom_dma_setup,
 };

--

2.7.4

Kind regards
Uffe

[PATCH 09/14] mmc: mmci: modify dma_setup callback

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:37:49

From: Ludovic Barre <redacted>

This patch creates a generic mmci_dma_setup which calls
dma_setup callback and manages common next_cookie.
This patch is needed for sdmmc variant which has a different
dma settings.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c          | 25 +++++++++++++------------
 drivers/mmc/host/mmci.h          |  1 +
 drivers/mmc/host/mmci_qcom_dml.c |  2 ++
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 0193da6..ae47d08 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -398,6 +398,17 @@ void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
 		host->ops->get_next_data(host, data);
 }
 
+int mmci_dma_setup(struct mmci_host *host)
+{
+	if (!host->ops || !host->ops->dma_setup)
+		return 0;
+
+	/* initialize pre request cookie */
+	host->next_cookie = 1;
+
+	return host->ops->dma_setup(host);
+}
+
 static void
 mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {

@@ -472,7 +483,7 @@ struct dmaengine_priv {
 
 #define __dmae_inprogress(dmae) ((dmae)->dma_in_progress)
 
-static int mmci_dma_setup(struct mmci_host *host)
+int mmci_dmae_setup(struct mmci_host *host)
 {
 	const char *rxname, *txname;
 	struct dmaengine_priv *dmae;

@@ -488,9 +499,6 @@ static int mmci_dma_setup(struct mmci_host *host)
 	dmae->dma_tx_channel = dma_request_slave_channel(mmc_dev(host->mmc),
 							 "tx");
 
-	/* initialize pre request cookie */
-	host->next_cookie = 1;
-
 	/*
 	 * If only an RX channel is specified, the driver will
 	 * attempt to use it bidirectionally, however if it is

@@ -531,9 +539,6 @@ static int mmci_dma_setup(struct mmci_host *host)
 			host->mmc->max_seg_size = max_seg_size;
 	}
 
-	if (host->ops && host->ops->dma_setup)
-		return host->ops->dma_setup(host);
-
 	return 0;
 }

@@ -793,14 +798,10 @@ static struct mmci_host_ops mmci_variant_ops = {
 	.prepare_data = mmci_dmae_prepare_data,
 	.unprepare_data = mmci_dmae_unprepare_data,
 	.get_next_data = mmci_dmae_get_next_data,
+	.dma_setup = mmci_dmae_setup,
 };
 #else
 /* Blank functions if the DMA engine is not available */
-static inline int mmci_dma_setup(struct mmci_host *host)
-{
-	return 0;
-}
-
 static inline void mmci_dma_release(struct mmci_host *host)
 {
 }

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index bb1e4ba..e1b389c 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -335,3 +335,4 @@ int mmci_dmae_prepare_data(struct mmci_host *host, struct mmc_data *data,
 void mmci_dmae_unprepare_data(struct mmci_host *host,
 			      struct mmc_data *data, int err);
 void mmci_dmae_get_next_data(struct mmci_host *host, struct mmc_data *data);
+int mmci_dmae_setup(struct mmci_host *host);

diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index e4c505a..47abbdd 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c

@@ -126,6 +126,8 @@ static int qcom_dma_setup(struct mmci_host *host)
 	int consumer_id, producer_id;
 	struct device_node *np = host->mmc->parent->of_node;
 
+	mmci_dmae_setup(host);
+
 	consumer_id = of_get_dml_pipe_index(np, "tx");
 	producer_id = of_get_dml_pipe_index(np, "rx");

-- 
2.7.4

[PATCH 11/14] mmc: mmci: add dma_start callback

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:37:51

From: Ludovic Barre <redacted>

This patch adds dma_start callback to mmci_host_ops.
Create a generic mmci_dma_start function which regroup
common action between variant.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c          | 63 +++++++++++++++++++++++-----------------
 drivers/mmc/host/mmci.h          |  2 ++
 drivers/mmc/host/mmci_qcom_dml.c |  1 +
 3 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 177e2e8..642ef19 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -415,6 +415,38 @@ void mmci_dma_release(struct mmci_host *host)
 		host->ops->dma_release(host);
 }
 
+int mmci_dma_start(struct mmci_host *host, unsigned int datactrl)
+{
+	struct mmc_data *data = host->data;
+	int ret;
+
+	ret = mmci_prepare_data(host, data, false);
+	if (ret)
+		return ret;
+
+	if (!host->ops || !host->ops->dma_start)
+		return -EINVAL;
+
+	/* Okay, go for it. */
+	dev_vdbg(mmc_dev(host->mmc),
+		 "Submit MMCI DMA job, sglen %d blksz %04x blks %04x flags %08x\n",
+		 data->sg_len, data->blksz, data->blocks, data->flags);
+
+	host->ops->dma_start(host, &datactrl);
+
+	/* Trigger the DMA transfer */
+	mmci_write_datactrlreg(host, datactrl);
+
+	/*
+	 * Let the MMCI say when the data is ended and it's time
+	 * to fire next DMA request. When that happens, MMCI will
+	 * call mmci_data_end()
+	 */
+	writel(readl(host->base + MMCIMASK0) | MCI_DATAENDMASK,
+	       host->base + MMCIMASK0);
+	return 0;
+}
+
 static void
 mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {

@@ -721,20 +753,11 @@ int mmci_dmae_prepare_data(struct mmci_host *host,
 				    &dmae->dma_desc_current);
 }
 
-static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
+int mmci_dmae_start(struct mmci_host *host, unsigned int *datactrl)
 {
 	struct dmaengine_priv *dmae = host->dma_priv;
 	struct mmc_data *data = host->data;
-	int ret;
-
-	ret = mmci_prepare_data(host, host->data, false);
-	if (ret)
-		return ret;
 
-	/* Okay, go for it. */
-	dev_vdbg(mmc_dev(host->mmc),
-		 "Submit MMCI DMA job, sglen %d blksz %04x blks %04x flags %08x\n",
-		 data->sg_len, data->blksz, data->blocks, data->flags);
 	dmae->dma_in_progress = true;
 	dmaengine_submit(dmae->dma_desc_current);
 	dma_async_issue_pending(dmae->dma_current);

@@ -742,18 +765,8 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 	if (host->variant->qcom_dml)
 		dml_start_xfer(host, data);
 
-	datactrl |= MCI_DPSM_DMAENABLE;
-
-	/* Trigger the DMA transfer */
-	mmci_write_datactrlreg(host, datactrl);
+	*datactrl |= MCI_DPSM_DMAENABLE;
 
-	/*
-	 * Let the MMCI say when the data is ended and it's time
-	 * to fire next DMA request. When that happens, MMCI will
-	 * call mmci_data_end()
-	 */
-	writel(readl(host->base + MMCIMASK0) | MCI_DATAENDMASK,
-	       host->base + MMCIMASK0);
 	return 0;
 }

@@ -806,6 +819,7 @@ static struct mmci_host_ops mmci_variant_ops = {
 	.get_next_data = mmci_dmae_get_next_data,
 	.dma_setup = mmci_dmae_setup,
 	.dma_release = mmci_dmae_release,
+	.dma_start = mmci_dmae_start,
 };
 #else
 /* Blank functions if the DMA engine is not available */

@@ -818,11 +832,6 @@ static inline void mmci_dma_data_error(struct mmci_host *host)
 {
 }
 
-static inline int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
-{
-	return -ENOSYS;
-}
-
 static struct mmci_host_ops mmci_variant_ops = {};
 #endif

@@ -925,7 +934,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	 * Attempt to use DMA operation mode, if this
 	 * should fail, fall back to PIO mode
 	 */
-	if (!mmci_dma_start_data(host, datactrl))
+	if (!mmci_dma_start(host, datactrl))
 		return;
 
 	/* IRQ mode, map the SG list for CPU reading/writing */

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index f961f90..3a200a9 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -280,6 +280,7 @@ struct mmci_host_ops {
 	void (*get_next_data)(struct mmci_host *host, struct mmc_data *data);
 	int (*dma_setup)(struct mmci_host *host);
 	void (*dma_release)(struct mmci_host *host);
+	int (*dma_start)(struct mmci_host *host, unsigned int *datactrl);
 };
 
 struct mmci_host {

@@ -338,3 +339,4 @@ void mmci_dmae_unprepare_data(struct mmci_host *host,
 void mmci_dmae_get_next_data(struct mmci_host *host, struct mmc_data *data);
 int mmci_dmae_setup(struct mmci_host *host);
 void mmci_dmae_release(struct mmci_host *host);
+int mmci_dmae_start(struct mmci_host *host, unsigned int *datactrl);

diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index 3c9d32e..e6267ad 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c

@@ -187,6 +187,7 @@ static struct mmci_host_ops qcom_variant_ops = {
 	.get_next_data = mmci_dmae_get_next_data,
 	.dma_setup = qcom_dma_setup,
 	.dma_release = mmci_dmae_release,
+	.dma_start = mmci_dmae_start,
 };
 
 void qcom_variant_init(struct mmci_host *host)

-- 
2.7.4

[PATCH 12/14] mmc: mmci: add dma_finalize callback

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:37:52

From: Ludovic Barre <redacted>

This patch adds dma_finalize callback at mmci_host_ops
to allow to call specific variant.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c          | 14 ++++++++------
 drivers/mmc/host/mmci.h          |  2 ++
 drivers/mmc/host/mmci_qcom_dml.c |  1 +
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 642ef19..b124a73 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -447,6 +447,12 @@ int mmci_dma_start(struct mmci_host *host, unsigned int datactrl)
 	return 0;
 }
 
+void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
+{
+	if (host->ops && host->ops->dma_finalize)
+		host->ops->dma_finalize(host, data);
+}
+
 static void
 mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {

@@ -626,7 +632,7 @@ static void mmci_dma_data_error(struct mmci_host *host)
 	__mmci_dmae_unmap(host, host->data);
 }
 
-static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
+void mmci_dmae_finalize(struct mmci_host *host, struct mmc_data *data)
 {
 	struct dmaengine_priv *dmae = host->dma_priv;
 	u32 status;

@@ -820,14 +826,10 @@ static struct mmci_host_ops mmci_variant_ops = {
 	.dma_setup = mmci_dmae_setup,
 	.dma_release = mmci_dmae_release,
 	.dma_start = mmci_dmae_start,
+	.dma_finalize = mmci_dmae_finalize,
 };
 #else
 /* Blank functions if the DMA engine is not available */
-static inline void mmci_dma_finalize(struct mmci_host *host,
-				     struct mmc_data *data)
-{
-}
-
 static inline void mmci_dma_data_error(struct mmci_host *host)
 {
 }

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 3a200a9..3f482d5 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -281,6 +281,7 @@ struct mmci_host_ops {
 	int (*dma_setup)(struct mmci_host *host);
 	void (*dma_release)(struct mmci_host *host);
 	int (*dma_start)(struct mmci_host *host, unsigned int *datactrl);
+	void (*dma_finalize)(struct mmci_host *host, struct mmc_data *data);
 };
 
 struct mmci_host {

@@ -340,3 +341,4 @@ void mmci_dmae_get_next_data(struct mmci_host *host, struct mmc_data *data);
 int mmci_dmae_setup(struct mmci_host *host);
 void mmci_dmae_release(struct mmci_host *host);
 int mmci_dmae_start(struct mmci_host *host, unsigned int *datactrl);
+void mmci_dmae_finalize(struct mmci_host *host, struct mmc_data *data);

diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index e6267ad..ba7e311 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c

@@ -188,6 +188,7 @@ static struct mmci_host_ops qcom_variant_ops = {
 	.dma_setup = qcom_dma_setup,
 	.dma_release = mmci_dmae_release,
 	.dma_start = mmci_dmae_start,
+	.dma_finalize = mmci_dmae_finalize,
 };
 
 void qcom_variant_init(struct mmci_host *host)

-- 
2.7.4

[PATCH 04/14] mmc: mmci: introduce dma_priv pointer to mmci_host

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:37:55

From: Ludovic Barre <redacted>

This patch introduces dma_priv pointer to define specific
needs for each dma engine. This patch is needed to prepare
sdmmc variant with internal dma which not use dmaengine API.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c          | 165 +++++++++++++++++++++++++--------------
 drivers/mmc/host/mmci.h          |  20 +----
 drivers/mmc/host/mmci_qcom_dml.c |   6 +-
 3 files changed, 112 insertions(+), 79 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 8144a87..bdc48c3 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -415,31 +415,57 @@ static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
  * no custom DMA interfaces are supported.
  */
 #ifdef CONFIG_DMA_ENGINE
-static void mmci_dma_setup(struct mmci_host *host)
+struct dmaengine_next {
+	struct dma_async_tx_descriptor *dma_desc;
+	struct dma_chan	*dma_chan;
+	s32 cookie;
+};
+
+struct dmaengine_priv {
+	struct dma_chan	*dma_current;
+	struct dma_chan	*dma_rx_channel;
+	struct dma_chan	*dma_tx_channel;
+	struct dma_async_tx_descriptor	*dma_desc_current;
+	struct dmaengine_next next_data;
+	bool dma_in_progress;
+};
+
+#define __dmae_inprogress(dmae) ((dmae)->dma_in_progress)
+
+static int mmci_dma_setup(struct mmci_host *host)
 {
 	const char *rxname, *txname;
+	struct dmaengine_priv *dmae;
 
-	host->dma_rx_channel = dma_request_slave_channel(mmc_dev(host->mmc), "rx");
-	host->dma_tx_channel = dma_request_slave_channel(mmc_dev(host->mmc), "tx");
+	dmae = devm_kzalloc(mmc_dev(host->mmc), sizeof(*dmae), GFP_KERNEL);
+	if (!dmae)
+		return -ENOMEM;
+
+	host->dma_priv = dmae;
+
+	dmae->dma_rx_channel = dma_request_slave_channel(mmc_dev(host->mmc),
+							 "rx");
+	dmae->dma_tx_channel = dma_request_slave_channel(mmc_dev(host->mmc),
+							 "tx");
 
 	/* initialize pre request cookie */
-	host->next_data.cookie = 1;
+	dmae->next_data.cookie = 1;
 
 	/*
 	 * If only an RX channel is specified, the driver will
 	 * attempt to use it bidirectionally, however if it is
 	 * is specified but cannot be located, DMA will be disabled.
 	 */
-	if (host->dma_rx_channel && !host->dma_tx_channel)
-		host->dma_tx_channel = host->dma_rx_channel;
+	if (dmae->dma_rx_channel && !dmae->dma_tx_channel)
+		dmae->dma_tx_channel = dmae->dma_rx_channel;
 
-	if (host->dma_rx_channel)
-		rxname = dma_chan_name(host->dma_rx_channel);
+	if (dmae->dma_rx_channel)
+		rxname = dma_chan_name(dmae->dma_rx_channel);
 	else
 		rxname = "none";
 
-	if (host->dma_tx_channel)
-		txname = dma_chan_name(host->dma_tx_channel);
+	if (dmae->dma_tx_channel)
+		txname = dma_chan_name(dmae->dma_tx_channel);
 	else
 		txname = "none";

@@ -450,15 +476,15 @@ static void mmci_dma_setup(struct mmci_host *host)
 	 * Limit the maximum segment size in any SG entry according to
 	 * the parameters of the DMA engine device.
 	 */
-	if (host->dma_tx_channel) {
-		struct device *dev = host->dma_tx_channel->device->dev;
+	if (dmae->dma_tx_channel) {
+		struct device *dev = dmae->dma_tx_channel->device->dev;
 		unsigned int max_seg_size = dma_get_max_seg_size(dev);
 
 		if (max_seg_size < host->mmc->max_seg_size)
 			host->mmc->max_seg_size = max_seg_size;
 	}
-	if (host->dma_rx_channel) {
-		struct device *dev = host->dma_rx_channel->device->dev;
+	if (dmae->dma_rx_channel) {
+		struct device *dev = dmae->dma_rx_channel->device->dev;
 		unsigned int max_seg_size = dma_get_max_seg_size(dev);
 
 		if (max_seg_size < host->mmc->max_seg_size)

@@ -466,7 +492,9 @@ static void mmci_dma_setup(struct mmci_host *host)
 	}
 
 	if (host->ops && host->ops->dma_setup)
-		host->ops->dma_setup(host);
+		return host->ops->dma_setup(host);
+
+	return 0;
 }
 
 /*

@@ -475,21 +503,24 @@ static void mmci_dma_setup(struct mmci_host *host)
  */
 static inline void mmci_dma_release(struct mmci_host *host)
 {
-	if (host->dma_rx_channel)
-		dma_release_channel(host->dma_rx_channel);
-	if (host->dma_tx_channel)
-		dma_release_channel(host->dma_tx_channel);
-	host->dma_rx_channel = host->dma_tx_channel = NULL;
+	struct dmaengine_priv *dmae = host->dma_priv;
+
+	if (dmae->dma_rx_channel)
+		dma_release_channel(dmae->dma_rx_channel);
+	if (dmae->dma_tx_channel)
+		dma_release_channel(dmae->dma_tx_channel);
+	dmae->dma_rx_channel = dmae->dma_tx_channel = NULL;
 }
 
-static void __mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)
+static void __mmci_dmae_unmap(struct mmci_host *host, struct mmc_data *data)
 {
+	struct dmaengine_priv *dmae = host->dma_priv;
 	struct dma_chan *chan;
 
 	if (data->flags & MMC_DATA_READ)
-		chan = host->dma_rx_channel;
+		chan = dmae->dma_rx_channel;
 	else
-		chan = host->dma_tx_channel;
+		chan = dmae->dma_tx_channel;
 
 	dma_unmap_sg(chan->device->dev, data->sg, data->sg_len,
 		     mmc_get_dma_dir(data));

@@ -497,25 +528,28 @@ static void __mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)
 
 static void mmci_dma_data_error(struct mmci_host *host)
 {
-	if (!__dma_inprogress(host))
+	struct dmaengine_priv *dmae = host->dma_priv;
+
+	if (!__dmae_inprogress(dmae))
 		return;
 
 	dev_err(mmc_dev(host->mmc), "error during DMA transfer!\n");
-	dmaengine_terminate_all(host->dma_current);
-	host->dma_in_progress = false;
-	host->dma_current = NULL;
-	host->dma_desc_current = NULL;
+	dmaengine_terminate_all(dmae->dma_current);
+	dmae->dma_in_progress = false;
+	dmae->dma_current = NULL;
+	dmae->dma_desc_current = NULL;
 	host->data->host_cookie = 0;
 
-	__mmci_dma_unmap(host, host->data);
+	__mmci_dmae_unmap(host, host->data);
 }
 
 static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 {
+	struct dmaengine_priv *dmae = host->dma_priv;
 	u32 status;
 	int i;
 
-	if (!__dma_inprogress(dmae))
+	if (!__dmae_inprogress(dmae))
 		return;
 
 	/* Wait up to 1ms for the DMA to complete */

@@ -537,7 +571,7 @@ static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 		if (!data->error)
 			data->error = -EIO;
 	} else if (!data->host_cookie) {
-		__mmci_dma_unmap(host, data);
+		__mmci_dmae_unmap(host, data);
 	}
 
 	/*

@@ -549,9 +583,9 @@ static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 		mmci_dma_release(host);
 	}
 
-	host->dma_in_progress = false;
-	host->dma_current = NULL;
-	host->dma_desc_current = NULL;
+	dmae->dma_in_progress = false;
+	dmae->dma_current = NULL;
+	dmae->dma_desc_current = NULL;
 }
 
 /* prepares DMA channel and DMA descriptor, returns non-zero on failure */

@@ -559,6 +593,7 @@ static int __mmci_dma_prep_data(struct mmci_host *host, struct mmc_data *data,
 				struct dma_chan **dma_chan,
 				struct dma_async_tx_descriptor **dma_desc)
 {
+	struct dmaengine_priv *dmae = host->dma_priv;
 	struct variant_data *variant = host->variant;
 	struct dma_slave_config conf = {
 		.src_addr = host->phybase + MMCIFIFO,

@@ -577,10 +612,10 @@ static int __mmci_dma_prep_data(struct mmci_host *host, struct mmc_data *data,
 
 	if (data->flags & MMC_DATA_READ) {
 		conf.direction = DMA_DEV_TO_MEM;
-		chan = host->dma_rx_channel;
+		chan = dmae->dma_rx_channel;
 	} else {
 		conf.direction = DMA_MEM_TO_DEV;
-		chan = host->dma_tx_channel;
+		chan = dmae->dma_tx_channel;
 	}
 
 	/* If there's no DMA channel, fall back to PIO */

@@ -620,26 +655,31 @@ static int __mmci_dma_prep_data(struct mmci_host *host, struct mmc_data *data,
 static inline int mmci_dma_prep_data(struct mmci_host *host,
 				     struct mmc_data *data)
 {
+	struct dmaengine_priv *dmae = host->dma_priv;
+
 	/* Check if next job is already prepared. */
-	if (host->dma_current && host->dma_desc_current)
+	if (dmae->dma_current && dmae->dma_desc_current)
 		return 0;
 
 	/* No job were prepared thus do it now. */
-	return __mmci_dma_prep_data(host, data, &host->dma_current,
-				    &host->dma_desc_current);
+	return __mmci_dma_prep_data(host, data, &dmae->dma_current,
+				    &dmae->dma_desc_current);
 }
 
 static inline int mmci_dma_prep_next(struct mmci_host *host,
 				     struct mmc_data *data)
 {
-	struct mmci_host_next *nd = &host->next_data;
+	struct dmaengine_priv *dmae = host->dma_priv;
+	struct dmaengine_next *nd = &dmae->next_data;
+
 	return __mmci_dma_prep_data(host, data, &nd->dma_chan, &nd->dma_desc);
 }
 
 static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 {
-	int ret;
+	struct dmaengine_priv *dmae = host->dma_priv;
 	struct mmc_data *data = host->data;
+	int ret;
 
 	ret = mmci_dma_prep_data(host, host->data);
 	if (ret)

@@ -649,9 +689,9 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 	dev_vdbg(mmc_dev(host->mmc),
 		 "Submit MMCI DMA job, sglen %d blksz %04x blks %04x flags %08x\n",
 		 data->sg_len, data->blksz, data->blocks, data->flags);
-	host->dma_in_progress = true;
-	dmaengine_submit(host->dma_desc_current);
-	dma_async_issue_pending(host->dma_current);
+	dmae->dma_in_progress = true;
+	dmaengine_submit(dmae->dma_desc_current);
+	dma_async_issue_pending(dmae->dma_current);
 
 	if (host->variant->qcom_dml)
 		dml_start_xfer(host, data);

@@ -673,13 +713,14 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 
 static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
 {
-	struct mmci_host_next *next = &host->next_data;
+	struct dmaengine_priv *dmae = host->dma_priv;
+	struct dmaengine_next *next = &dmae->next_data;
 
 	WARN_ON(data->host_cookie && data->host_cookie != next->cookie);
 	WARN_ON(!data->host_cookie && (next->dma_desc || next->dma_chan));
 
-	host->dma_desc_current = next->dma_desc;
-	host->dma_current = next->dma_chan;
+	dmae->dma_desc_current = next->dma_desc;
+	dmae->dma_current = next->dma_chan;
 	next->dma_desc = NULL;
 	next->dma_chan = NULL;
 }

@@ -687,8 +728,9 @@ static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
 static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct mmci_host *host = mmc_priv(mmc);
+	struct dmaengine_priv *dmae = host->dma_priv;
 	struct mmc_data *data = mrq->data;
-	struct mmci_host_next *nd = &host->next_data;
+	struct dmaengine_next *nd = &dmae->next_data;
 
 	if (!data)
 		return;

@@ -706,28 +748,29 @@ static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
 			      int err)
 {
 	struct mmci_host *host = mmc_priv(mmc);
+	struct dmaengine_priv *dmae = host->dma_priv;
 	struct mmc_data *data = mrq->data;
 
 	if (!data || !data->host_cookie)
 		return;
 
-	__mmci_dma_unmap(host, data);
+	__mmci_dmae_unmap(host, data);
 
 	if (err) {
-		struct mmci_host_next *next = &host->next_data;
+		struct dmaengine_next *next = &dmae->next_data;
 		struct dma_chan *chan;
 		if (data->flags & MMC_DATA_READ)
-			chan = host->dma_rx_channel;
+			chan = dmae->dma_rx_channel;
 		else
-			chan = host->dma_tx_channel;
+			chan = dmae->dma_tx_channel;
 		dmaengine_terminate_all(chan);
 
-		if (host->dma_desc_current == next->dma_desc)
-			host->dma_desc_current = NULL;
+		if (dmae->dma_desc_current == next->dma_desc)
+			dmae->dma_desc_current = NULL;
 
-		if (host->dma_current == next->dma_chan) {
-			host->dma_in_progress = false;
-			host->dma_current = NULL;
+		if (dmae->dma_current == next->dma_chan) {
+			dmae->dma_in_progress = false;
+			dmae->dma_current = NULL;
 		}
 
 		next->dma_desc = NULL;

@@ -741,8 +784,10 @@ static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
 static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
 {
 }
-static inline void mmci_dma_setup(struct mmci_host *host)
+
+static inline int mmci_dma_setup(struct mmci_host *host)
 {
+	return 0;
 }
 
 static inline void mmci_dma_release(struct mmci_host *host)

@@ -1796,7 +1841,9 @@ static int mmci_probe(struct amba_device *dev,
 		 amba_rev(dev), (unsigned long long)dev->res.start,
 		 dev->irq[0], dev->irq[1]);
 
-	mmci_dma_setup(host);
+	ret = mmci_dma_setup(host);
+	if (ret)
+		goto clk_disable;
 
 	pm_runtime_set_autosuspend_delay(&dev->dev, 50);
 	pm_runtime_use_autosuspend(&dev->dev);

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index f1ec066..260a1de 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -273,13 +273,7 @@ struct variant_data {
 
 /* mmci variant callbacks */
 struct mmci_host_ops {
-	void (*dma_setup)(struct mmci_host *host);
-};
-
-struct mmci_host_next {
-	struct dma_async_tx_descriptor	*dma_desc;
-	struct dma_chan			*dma_chan;
-	s32				cookie;
+	int (*dma_setup)(struct mmci_host *host);
 };
 
 struct mmci_host {

@@ -323,17 +317,7 @@ struct mmci_host {
 	unsigned int		size;
 	int (*get_rx_fifocnt)(struct mmci_host *h, u32 status, int remain);
 
-#ifdef CONFIG_DMA_ENGINE
-	/* DMA stuff */
-	struct dma_chan		*dma_current;
-	struct dma_chan		*dma_rx_channel;
-	struct dma_chan		*dma_tx_channel;
-	struct dma_async_tx_descriptor	*dma_desc_current;
-	struct mmci_host_next	next_data;
-	bool			dma_in_progress;
-
-#define __dma_inprogress(host)	((host)->dma_in_progress)
-#endif
+	void			*dma_priv;
 };
 
 void qcom_variant_init(struct mmci_host *host);

diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index be3fab5..1bb59cf 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c

@@ -119,7 +119,7 @@ static int of_get_dml_pipe_index(struct device_node *np, const char *name)
 }
 
 /* Initialize the dml hardware connected to SD Card controller */
-static void qcom_dma_setup(struct mmci_host *host)
+static int qcom_dma_setup(struct mmci_host *host)
 {
 	u32 config;
 	void __iomem *base;

@@ -131,7 +131,7 @@ static void qcom_dma_setup(struct mmci_host *host)
 
 	if (producer_id < 0 || consumer_id < 0) {
 		host->variant->qcom_dml = false;
-		return;
+		return -EINVAL;
 	}
 
 	base = host->base + DML_OFFSET;

@@ -175,6 +175,8 @@ static void qcom_dma_setup(struct mmci_host *host)
 
 	/* Make sure dml initialization is finished */
 	mb();
+
+	return 0;
 }
 
 static struct mmci_host_ops qcom_variant_ops = {

-- 
2.7.4

Re: [PATCH 04/14] mmc: mmci: introduce dma_priv pointer to mmci_host

From: Ulf Hansson <hidden>
Date: 2018-09-03 12:15:31

On 1 August 2018 at 11:36, Ludovic Barre [off-list ref] wrote:

From: Ludovic Barre <redacted>

This patch introduces dma_priv pointer to define specific
needs for each dma engine. This patch is needed to prepare
sdmmc variant with internal dma which not use dmaengine API.

Overall this looks good, however a couple a few things below, mostly nitpicks.

quoted hunk

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c          | 165 +++++++++++++++++++++++++--------------
 drivers/mmc/host/mmci.h          |  20 +----
 drivers/mmc/host/mmci_qcom_dml.c |   6 +-
 3 files changed, 112 insertions(+), 79 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 8144a87..bdc48c3 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -415,31 +415,57 @@ static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
  * no custom DMA interfaces are supported.
  */
 #ifdef CONFIG_DMA_ENGINE
-static void mmci_dma_setup(struct mmci_host *host)
+struct dmaengine_next {

I would rather rename this struct to something along the lines of
"mmci_dma_next", that should follow how most of the data structures
are named in mmci.

+       struct dma_async_tx_descriptor *dma_desc;
+       struct dma_chan *dma_chan;

For these two, I think you should remove the "dma_" prefix from their
names. At least to me, it's of obvious they are about dma if they are
part of a struct used (and named) used solely for that purpose.

+       s32 cookie;
+};
+
+struct dmaengine_priv {
+       struct dma_chan *dma_current;
+       struct dma_chan *dma_rx_channel;
+       struct dma_chan *dma_tx_channel;
+       struct dma_async_tx_descriptor  *dma_desc_current;
+       struct dmaengine_next next_data;
+       bool dma_in_progress;

For similar reasons as above, I suggest to rename the struct to
"mmci_dma_priv" and to drop the "dma_" prefix from the variable names.

+};
+
+#define __dmae_inprogress(dmae) ((dmae)->dma_in_progress)

How about naming this to mmci_dma_inprogress() instead?

BTW, in general it looks like you are a bit fond of using "__" as
function name prefix for internally called functions. Please try to
avoid that, but rather try to pick names that explains what the
functions do.

quoted hunk

+
+static int mmci_dma_setup(struct mmci_host *host)
 {
        const char *rxname, *txname;
+       struct dmaengine_priv *dmae;

-       host->dma_rx_channel = dma_request_slave_channel(mmc_dev(host->mmc), "rx");
-       host->dma_tx_channel = dma_request_slave_channel(mmc_dev(host->mmc), "tx");
+       dmae = devm_kzalloc(mmc_dev(host->mmc), sizeof(*dmae), GFP_KERNEL);
+       if (!dmae)
+               return -ENOMEM;
+
+       host->dma_priv = dmae;
+
+       dmae->dma_rx_channel = dma_request_slave_channel(mmc_dev(host->mmc),
+                                                        "rx");
+       dmae->dma_tx_channel = dma_request_slave_channel(mmc_dev(host->mmc),
+                                                        "tx");

        /* initialize pre request cookie */
-       host->next_data.cookie = 1;
+       dmae->next_data.cookie = 1;

        /*
         * If only an RX channel is specified, the driver will
         * attempt to use it bidirectionally, however if it is
         * is specified but cannot be located, DMA will be disabled.
         */
-       if (host->dma_rx_channel && !host->dma_tx_channel)
-               host->dma_tx_channel = host->dma_rx_channel;
+       if (dmae->dma_rx_channel && !dmae->dma_tx_channel)
+               dmae->dma_tx_channel = dmae->dma_rx_channel;

-       if (host->dma_rx_channel)
-               rxname = dma_chan_name(host->dma_rx_channel);
+       if (dmae->dma_rx_channel)
+               rxname = dma_chan_name(dmae->dma_rx_channel);
        else
                rxname = "none";

-       if (host->dma_tx_channel)
-               txname = dma_chan_name(host->dma_tx_channel);
+       if (dmae->dma_tx_channel)
+               txname = dma_chan_name(dmae->dma_tx_channel);
        else
                txname = "none";

@@ -450,15 +476,15 @@ static void mmci_dma_setup(struct mmci_host *host)
         * Limit the maximum segment size in any SG entry according to
         * the parameters of the DMA engine device.
         */
-       if (host->dma_tx_channel) {
-               struct device *dev = host->dma_tx_channel->device->dev;
+       if (dmae->dma_tx_channel) {
+               struct device *dev = dmae->dma_tx_channel->device->dev;
                unsigned int max_seg_size = dma_get_max_seg_size(dev);

                if (max_seg_size < host->mmc->max_seg_size)
                        host->mmc->max_seg_size = max_seg_size;
        }
-       if (host->dma_rx_channel) {
-               struct device *dev = host->dma_rx_channel->device->dev;
+       if (dmae->dma_rx_channel) {
+               struct device *dev = dmae->dma_rx_channel->device->dev;
                unsigned int max_seg_size = dma_get_max_seg_size(dev);

                if (max_seg_size < host->mmc->max_seg_size)
@@ -466,7 +492,9 @@ static void mmci_dma_setup(struct mmci_host *host)
        }

        if (host->ops && host->ops->dma_setup)
-               host->ops->dma_setup(host);
+               return host->ops->dma_setup(host);

I agree that converting the ->dma_setup() callback to return an int makes sense.

However, please make that a separate change and while doing that,
don't forget to implement the error path, as that is missing here.

+
+       return 0;
 }

[...]

Kind regards
Uffe

[PATCH 10/14] mmc: mmci: add dma_release callback

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:37:56

From: Ludovic Barre <redacted>

This patch adds dma_release callback at mmci_host_ops
to allow to call specific variant.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c          | 13 ++++++++-----
 drivers/mmc/host/mmci.h          |  2 ++
 drivers/mmc/host/mmci_qcom_dml.c |  1 +
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index ae47d08..177e2e8 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -409,6 +409,12 @@ int mmci_dma_setup(struct mmci_host *host)
 	return host->ops->dma_setup(host);
 }
 
+void mmci_dma_release(struct mmci_host *host)
+{
+	if (host->ops && host->ops->dma_release)
+		host->ops->dma_release(host);
+}
+
 static void
 mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {

@@ -546,7 +552,7 @@ int mmci_dmae_setup(struct mmci_host *host)
  * This is used in or so inline it
  * so it can be discarded.
  */
-static inline void mmci_dma_release(struct mmci_host *host)
+void mmci_dmae_release(struct mmci_host *host)
 {
 	struct dmaengine_priv *dmae = host->dma_priv;

@@ -799,13 +805,10 @@ static struct mmci_host_ops mmci_variant_ops = {
 	.unprepare_data = mmci_dmae_unprepare_data,
 	.get_next_data = mmci_dmae_get_next_data,
 	.dma_setup = mmci_dmae_setup,
+	.dma_release = mmci_dmae_release,
 };
 #else
 /* Blank functions if the DMA engine is not available */
-static inline void mmci_dma_release(struct mmci_host *host)
-{
-}
-
 static inline void mmci_dma_finalize(struct mmci_host *host,
 				     struct mmc_data *data)
 {

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index e1b389c..f961f90 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -279,6 +279,7 @@ struct mmci_host_ops {
 			       int err);
 	void (*get_next_data)(struct mmci_host *host, struct mmc_data *data);
 	int (*dma_setup)(struct mmci_host *host);
+	void (*dma_release)(struct mmci_host *host);
 };
 
 struct mmci_host {

@@ -336,3 +337,4 @@ void mmci_dmae_unprepare_data(struct mmci_host *host,
 			      struct mmc_data *data, int err);
 void mmci_dmae_get_next_data(struct mmci_host *host, struct mmc_data *data);
 int mmci_dmae_setup(struct mmci_host *host);
+void mmci_dmae_release(struct mmci_host *host);

diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index 47abbdd..3c9d32e 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c

@@ -186,6 +186,7 @@ static struct mmci_host_ops qcom_variant_ops = {
 	.unprepare_data = mmci_dmae_unprepare_data,
 	.get_next_data = mmci_dmae_get_next_data,
 	.dma_setup = qcom_dma_setup,
+	.dma_release = mmci_dmae_release,
 };
 
 void qcom_variant_init(struct mmci_host *host)

-- 
2.7.4

[PATCH 14/14] mmc: mmci: add validate_data callback

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:37:59

From: Ludovic Barre <redacted>

This patch adds validate_data callback at mmci_host_ops
to check specific constraints of variant.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c | 39 +++++++++++++++++++++------------------
 drivers/mmc/host/mmci.h |  1 +
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index d5ca93e..7ba2f61 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -234,24 +234,6 @@ static int mmci_card_busy(struct mmc_host *mmc)
 	return busy;
 }
 
-/*
- * Validate mmc prerequisites
- */
-static int mmci_validate_data(struct mmci_host *host,
-			      struct mmc_data *data)
-{
-	if (!data)
-		return 0;
-
-	if (!is_power_of_2(data->blksz)) {
-		dev_err(mmc_dev(host->mmc),
-			"unsupported block size (%d bytes)\n", data->blksz);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static void mmci_reg_delay(struct mmci_host *host)
 {
 	/*

@@ -365,6 +347,27 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 	mmci_write_clkreg(host, clk);
 }
 
+/*
+ * Validate mmc prerequisites
+ */
+static int mmci_validate_data(struct mmci_host *host,
+			      struct mmc_data *data)
+{
+	if (!data)
+		return 0;
+
+	if (!is_power_of_2(data->blksz)) {
+		dev_err(mmc_dev(host->mmc),
+			"unsupported block size (%d bytes)\n", data->blksz);
+		return -EINVAL;
+	}
+
+	if (host->ops && host->ops->validate_data)
+		return host->ops->validate_data(host, data);
+
+	return 0;
+}
+
 int mmci_prepare_data(struct mmci_host *host, struct mmc_data *data, bool next)
 {
 	int err;

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 0a4811d..10b71bf 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -273,6 +273,7 @@ struct variant_data {
 
 /* mmci variant callbacks */
 struct mmci_host_ops {
+	int (*validate_data)(struct mmci_host *host, struct mmc_data *data);
 	int (*prepare_data)(struct mmci_host *host, struct mmc_data *data,
 			    bool next);
 	void (*unprepare_data)(struct mmci_host *host, struct mmc_data *data,

-- 
2.7.4

[PATCH 13/14] mmc: mmci: add dma_error callback

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:38:01

From: Ludovic Barre <redacted>

This patch adds dma_error callback at mmci_host_ops
to allow to call specific variant.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c          | 20 +++++++++++---------
 drivers/mmc/host/mmci.h          |  2 ++
 drivers/mmc/host/mmci_qcom_dml.c |  1 +
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index b124a73..d5ca93e 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -453,6 +453,12 @@ void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 		host->ops->dma_finalize(host, data);
 }
 
+void mmci_dma_error(struct mmci_host *host)
+{
+	if (host->ops && host->ops->dma_error)
+		host->ops->dma_error(host);
+}
+
 static void
 mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {

@@ -615,7 +621,7 @@ static void __mmci_dmae_unmap(struct mmci_host *host, struct mmc_data *data)
 		     mmc_get_dma_dir(data));
 }
 
-static void mmci_dma_data_error(struct mmci_host *host)
+void mmci_dmae_error(struct mmci_host *host)
 {
 	struct dmaengine_priv *dmae = host->dma_priv;

@@ -656,7 +662,7 @@ void mmci_dmae_finalize(struct mmci_host *host, struct mmc_data *data)
 	 * contiguous buffers.  On TX, we'll get a FIFO underrun error.
 	 */
 	if (status & MCI_RXDATAAVLBLMASK) {
-		mmci_dma_data_error(host);
+		mmci_dma_error(host);
 		if (!data->error)
 			data->error = -EIO;
 	} else if (!data->host_cookie) {

@@ -827,13 +833,9 @@ static struct mmci_host_ops mmci_variant_ops = {
 	.dma_release = mmci_dmae_release,
 	.dma_start = mmci_dmae_start,
 	.dma_finalize = mmci_dmae_finalize,
+	.dma_error = mmci_dmae_error,
 };
 #else
-/* Blank functions if the DMA engine is not available */
-static inline void mmci_dma_data_error(struct mmci_host *host)
-{
-}
-
 static struct mmci_host_ops mmci_variant_ops = {};
 #endif

@@ -1011,7 +1013,7 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		u32 remain, success;
 
 		/* Terminate the DMA transfer */
-		mmci_dma_data_error(host);
+		mmci_dma_error(host);
 
 		/*
 		 * Calculate how far we are into the transfer.  Note that

@@ -1157,7 +1159,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	if ((!sbc && !cmd->data) || cmd->error) {
 		if (host->data) {
 			/* Terminate the DMA transfer */
-			mmci_dma_data_error(host);
+			mmci_dma_error(host);
 
 			mmci_stop_data(host);
 		}

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 3f482d5..0a4811d 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -282,6 +282,7 @@ struct mmci_host_ops {
 	void (*dma_release)(struct mmci_host *host);
 	int (*dma_start)(struct mmci_host *host, unsigned int *datactrl);
 	void (*dma_finalize)(struct mmci_host *host, struct mmc_data *data);
+	void (*dma_error)(struct mmci_host *host);
 };
 
 struct mmci_host {

@@ -342,3 +343,4 @@ int mmci_dmae_setup(struct mmci_host *host);
 void mmci_dmae_release(struct mmci_host *host);
 int mmci_dmae_start(struct mmci_host *host, unsigned int *datactrl);
 void mmci_dmae_finalize(struct mmci_host *host, struct mmc_data *data);
+void mmci_dmae_error(struct mmci_host *host);

diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index ba7e311..80701b4 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c

@@ -189,6 +189,7 @@ static struct mmci_host_ops qcom_variant_ops = {
 	.dma_release = mmci_dmae_release,
 	.dma_start = mmci_dmae_start,
 	.dma_finalize = mmci_dmae_finalize,
+	.dma_error = mmci_dmae_error,
 };
 
 void qcom_variant_init(struct mmci_host *host)

-- 
2.7.4

[PATCH 02/14] mmc: mmci: internalize dma map/unmap into mmci dma functions

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:38:15

From: Ludovic Barre <redacted>

This patch internalizes the management of dma map/unmap into
mmci dma interfaces. This allows to simplify and prepare the next dma
callbacks for mmci host ops.
mmci_dma_unmap was called in mmci_data_irq & mmci_cmd_irq functions
and can be integrated in mmci_dma_data_error.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c | 44 +++++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 1841d250..d8fa178 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -482,17 +482,7 @@ static inline void mmci_dma_release(struct mmci_host *host)
 	host->dma_rx_channel = host->dma_tx_channel = NULL;
 }
 
-static void mmci_dma_data_error(struct mmci_host *host)
-{
-	dev_err(mmc_dev(host->mmc), "error during DMA transfer!\n");
-	dmaengine_terminate_all(host->dma_current);
-	host->dma_in_progress = false;
-	host->dma_current = NULL;
-	host->dma_desc_current = NULL;
-	host->data->host_cookie = 0;
-}
-
-static void mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)
+static void __mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)
 {
 	struct dma_chan *chan;

@@ -505,6 +495,18 @@ static void mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)
 		     mmc_get_dma_dir(data));
 }
 
+static void mmci_dma_data_error(struct mmci_host *host)
+{
+	dev_err(mmc_dev(host->mmc), "error during DMA transfer!\n");
+	dmaengine_terminate_all(host->dma_current);
+	host->dma_in_progress = false;
+	host->dma_current = NULL;
+	host->dma_desc_current = NULL;
+	host->data->host_cookie = 0;
+
+	__mmci_dma_unmap(host, host->data);
+}
+
 static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 {
 	u32 status;

@@ -528,11 +530,10 @@ static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
 		mmci_dma_data_error(host);
 		if (!data->error)
 			data->error = -EIO;
+	} else if (!data->host_cookie) {
+		__mmci_dma_unmap(host, data);
 	}
 
-	if (!data->host_cookie)
-		mmci_dma_unmap(host, data);
-
 	/*
 	 * Use of DMA with scatter-gather is impossible.
 	 * Give up with DMA and switch back to PIO mode.

@@ -704,7 +705,7 @@ static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
 	if (!data || !data->host_cookie)
 		return;
 
-	mmci_dma_unmap(host, data);
+	__mmci_dma_unmap(host, data);
 
 	if (err) {
 		struct mmci_host_next *next = &host->next_data;

@@ -742,10 +743,6 @@ static inline void mmci_dma_release(struct mmci_host *host)
 {
 }
 
-static inline void mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)
-{
-}
-
 static inline void mmci_dma_finalize(struct mmci_host *host,
 				     struct mmc_data *data)
 {

@@ -906,10 +903,8 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		u32 remain, success;
 
 		/* Terminate the DMA transfer */
-		if (dma_inprogress(host)) {
+		if (dma_inprogress(host))
 			mmci_dma_data_error(host);
-			mmci_dma_unmap(host, data);
-		}
 
 		/*
 		 * Calculate how far we are into the transfer.  Note that

@@ -1055,10 +1050,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	if ((!sbc && !cmd->data) || cmd->error) {
 		if (host->data) {
 			/* Terminate the DMA transfer */
-			if (dma_inprogress(host)) {
+			if (dma_inprogress(host))
 				mmci_dma_data_error(host);
-				mmci_dma_unmap(host, host->data);
-			}
+
 			mmci_stop_data(host);
 		}
 		mmci_request_end(host, host->mrq);

-- 
2.7.4

Re: [PATCH 02/14] mmc: mmci: internalize dma map/unmap into mmci dma functions

From: Ulf Hansson <hidden>
Date: 2018-09-03 12:14:59

On 1 August 2018 at 11:36, Ludovic Barre [off-list ref] wrote:

quoted hunk

From: Ludovic Barre <redacted>

This patch internalizes the management of dma map/unmap into
mmci dma interfaces. This allows to simplify and prepare the next dma
callbacks for mmci host ops.
mmci_dma_unmap was called in mmci_data_irq & mmci_cmd_irq functions
and can be integrated in mmci_dma_data_error.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c | 44 +++++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 1841d250..d8fa178 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -482,17 +482,7 @@ static inline void mmci_dma_release(struct mmci_host *host)
        host->dma_rx_channel = host->dma_tx_channel = NULL;
 }

-static void mmci_dma_data_error(struct mmci_host *host)
-{
-       dev_err(mmc_dev(host->mmc), "error during DMA transfer!\n");
-       dmaengine_terminate_all(host->dma_current);
-       host->dma_in_progress = false;
-       host->dma_current = NULL;
-       host->dma_desc_current = NULL;
-       host->data->host_cookie = 0;
-}
-
-static void mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)
+static void __mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)

The renaming of the function seems irrelevant to this change. Can you
please keep the existing name?

[...]

Besides the minor thing above, this looks good to me!

Kind regards
Uffe

[PATCH 08/14] mmc: mmci: add get_next_data callback

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:38:18

From: Ludovic Barre <redacted>

This patch adds get_next_data callback to mmci_host_ops.
Generic mmci_get_next_data factorizes next_cookie check and
the host ops call.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c          | 16 ++++++++++------
 drivers/mmc/host/mmci.h          |  2 ++
 drivers/mmc/host/mmci_qcom_dml.c |  1 +
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 345aa2e..0193da6 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -390,6 +390,14 @@ void mmci_unprepare_data(struct mmci_host *host, struct mmc_data *data,
 	data->host_cookie = 0;
 }
 
+void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
+{
+	WARN_ON(data->host_cookie && data->host_cookie != host->next_cookie);
+
+	if (host->ops && host->ops->get_next_data)
+		host->ops->get_next_data(host, data);
+}
+
 static void
 mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 {

@@ -738,12 +746,11 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 	return 0;
 }
 
-static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
+void mmci_dmae_get_next_data(struct mmci_host *host, struct mmc_data *data)
 {
 	struct dmaengine_priv *dmae = host->dma_priv;
 	struct dmaengine_next *next = &dmae->next_data;
 
-	WARN_ON(data->host_cookie && data->host_cookie != host->next_cookie);
 	WARN_ON(!data->host_cookie && (next->dma_desc || next->dma_chan));
 
 	dmae->dma_desc_current = next->dma_desc;

@@ -785,13 +792,10 @@ void mmci_dmae_unprepare_data(struct mmci_host *host,
 static struct mmci_host_ops mmci_variant_ops = {
 	.prepare_data = mmci_dmae_prepare_data,
 	.unprepare_data = mmci_dmae_unprepare_data,
+	.get_next_data = mmci_dmae_get_next_data,
 };
 #else
 /* Blank functions if the DMA engine is not available */
-static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
-{
-}
-
 static inline int mmci_dma_setup(struct mmci_host *host)
 {
 	return 0;

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index fa2702b..bb1e4ba 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -277,6 +277,7 @@ struct mmci_host_ops {
 			    bool next);
 	void (*unprepare_data)(struct mmci_host *host, struct mmc_data *data,
 			       int err);
+	void (*get_next_data)(struct mmci_host *host, struct mmc_data *data);
 	int (*dma_setup)(struct mmci_host *host);
 };

@@ -333,3 +334,4 @@ int mmci_dmae_prepare_data(struct mmci_host *host, struct mmc_data *data,
 			   bool next);
 void mmci_dmae_unprepare_data(struct mmci_host *host,
 			      struct mmc_data *data, int err);
+void mmci_dmae_get_next_data(struct mmci_host *host, struct mmc_data *data);

diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c
index d534fa1..e4c505a 100644
--- a/drivers/mmc/host/mmci_qcom_dml.c
+++ b/drivers/mmc/host/mmci_qcom_dml.c

@@ -182,6 +182,7 @@ static int qcom_dma_setup(struct mmci_host *host)
 static struct mmci_host_ops qcom_variant_ops = {
 	.prepare_data = mmci_dmae_prepare_data,
 	.unprepare_data = mmci_dmae_unprepare_data,
+	.get_next_data = mmci_dmae_get_next_data,
 	.dma_setup = qcom_dma_setup,
 };

-- 
2.7.4

[PATCH 06/14] mmc: mmci: merge prepare data functions

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:38:32

From: Ludovic Barre <redacted>

This patch merges the prepare data functions.
This allows to define a single access to prepare data service.
This prepares integration for mmci host ops.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 5646c2e6..e4d80f1 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -651,11 +651,16 @@ static int __mmci_dma_prep_data(struct mmci_host *host, struct mmc_data *data,
 	return -ENOMEM;
 }
 
-static inline int mmci_dma_prep_data(struct mmci_host *host,
-				     struct mmc_data *data)
+static inline int mmci_dma_prepare_data(struct mmci_host *host,
+					struct mmc_data *data,
+					bool next)
 {
 	struct dmaengine_priv *dmae = host->dma_priv;
+	struct dmaengine_next *nd = &dmae->next_data;
 
+	if (next)
+		return __mmci_dma_prep_data(host, data, &nd->dma_chan,
+					    &nd->dma_desc);
 	/* Check if next job is already prepared. */
 	if (dmae->dma_current && dmae->dma_desc_current)
 		return 0;

@@ -665,22 +670,13 @@ static inline int mmci_dma_prep_data(struct mmci_host *host,
 				    &dmae->dma_desc_current);
 }
 
-static inline int mmci_dma_prep_next(struct mmci_host *host,
-				     struct mmc_data *data)
-{
-	struct dmaengine_priv *dmae = host->dma_priv;
-	struct dmaengine_next *nd = &dmae->next_data;
-
-	return __mmci_dma_prep_data(host, data, &nd->dma_chan, &nd->dma_desc);
-}
-
 static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 {
 	struct dmaengine_priv *dmae = host->dma_priv;
 	struct mmc_data *data = host->data;
 	int ret;
 
-	ret = mmci_dma_prep_data(host, host->data);
+	ret = mmci_dma_prepare_data(host, host->data, false);
 	if (ret)
 		return ret;

@@ -737,7 +733,7 @@ static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	if (mmci_validate_data(host, data))
 		return;
 
-	if (!mmci_dma_prep_next(host, data))
+	if (!mmci_dma_prepare_data(host, data, true))
 		data->host_cookie = ++host->next_cookie < 0 ?
 			1 : host->next_cookie;
 }

-- 
2.7.4

Re: [PATCH 06/14] mmc: mmci: merge prepare data functions

From: Ulf Hansson <hidden>
Date: 2018-09-03 12:15:43

On 1 August 2018 at 11:36, Ludovic Barre [off-list ref] wrote:

quoted hunk

From: Ludovic Barre <redacted>

This patch merges the prepare data functions.
This allows to define a single access to prepare data service.
This prepares integration for mmci host ops.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 5646c2e6..e4d80f1 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -651,11 +651,16 @@ static int __mmci_dma_prep_data(struct mmci_host *host, struct mmc_data *data,
        return -ENOMEM;
 }

-static inline int mmci_dma_prep_data(struct mmci_host *host,
-                                    struct mmc_data *data)
+static inline int mmci_dma_prepare_data(struct mmci_host *host,

Nitpick: I don't see the reason to why you need to rename this
function here, please keep it as is.

quoted hunk

+                                       struct mmc_data *data,
+                                       bool next)
 {
        struct dmaengine_priv *dmae = host->dma_priv;
+       struct dmaengine_next *nd = &dmae->next_data;

+       if (next)
+               return __mmci_dma_prep_data(host, data, &nd->dma_chan,
+                                           &nd->dma_desc);
        /* Check if next job is already prepared. */
        if (dmae->dma_current && dmae->dma_desc_current)
                return 0;

@@ -665,22 +670,13 @@ static inline int mmci_dma_prep_data(struct mmci_host *host,
                                    &dmae->dma_desc_current);
 }

-static inline int mmci_dma_prep_next(struct mmci_host *host,
-                                    struct mmc_data *data)
-{
-       struct dmaengine_priv *dmae = host->dma_priv;
-       struct dmaengine_next *nd = &dmae->next_data;
-
-       return __mmci_dma_prep_data(host, data, &nd->dma_chan, &nd->dma_desc);
-}
-
 static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 {
        struct dmaengine_priv *dmae = host->dma_priv;
        struct mmc_data *data = host->data;
        int ret;

-       ret = mmci_dma_prep_data(host, host->data);
+       ret = mmci_dma_prepare_data(host, host->data, false);
        if (ret)
                return ret;
@@ -737,7 +733,7 @@ static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
        if (mmci_validate_data(host, data))
                return;

-       if (!mmci_dma_prep_next(host, data))
+       if (!mmci_dma_prepare_data(host, data, true))
                data->host_cookie = ++host->next_cookie < 0 ?
                        1 : host->next_cookie;
 }
--

2.7.4

Kind regards
Uffe

[PATCH 01/14] mmc: mmci: fix qcom dma issue during mmci init with new dma_setup callback

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:38:41

From: Ludovic Barre <redacted>

This patch fixes qcom dma issue during mmci init.
Like init callback of qcom variant is not set, the qcom dma
is not correctly initialized and fail while dma transfer
("buggy DMA detected. Taking evasive action").

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c | 1 +
 drivers/mmc/host/mmci.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 71e9336..1841d250 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -208,6 +208,7 @@ static struct variant_data variant_qcom = {
 	.mmcimask1		= true,
 	.start_err		= MCI_STARTBITERR,
 	.opendrain		= MCI_ROD,
+	.init			= qcom_variant_init,
 };
 
 /* Busy detection for the ST Micro variant */

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 517591d..696a066 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -338,3 +338,4 @@ struct mmci_host {
 #endif
 };
 
+void qcom_variant_init(struct mmci_host *host);

-- 
2.7.4

Re: [PATCH 01/14] mmc: mmci: fix qcom dma issue during mmci init with new dma_setup callback

From: Ulf Hansson <hidden>
Date: 2018-08-01 10:08:42

On 1 August 2018 at 11:36, Ludovic Barre [off-list ref] wrote:

quoted hunk

From: Ludovic Barre <redacted>

This patch fixes qcom dma issue during mmci init.
Like init callback of qcom variant is not set, the qcom dma
is not correctly initialized and fail while dma transfer
("buggy DMA detected. Taking evasive action").

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c | 1 +
 drivers/mmc/host/mmci.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 71e9336..1841d250 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -208,6 +208,7 @@ static struct variant_data variant_qcom = {
        .mmcimask1              = true,
        .start_err              = MCI_STARTBITERR,
        .opendrain              = MCI_ROD,
+       .init                   = qcom_variant_init,
 };

 /* Busy detection for the ST Micro variant */

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 517591d..696a066 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -338,3 +338,4 @@ struct mmci_host {
 #endif
 };

+void qcom_variant_init(struct mmci_host *host);

This isn't needed.

--
2.7.4

Anyway, we can just drop this patch from your series as I amended the
patch causing the problem.

I will continue to review the rest.

Kind regards
Uffe

Re: [PATCH 01/14] mmc: mmci: fix qcom dma issue during mmci init with new dma_setup callback

From: Ludovic BARRE <hidden>
Date: 2018-08-01 10:10:21


On 08/01/2018 12:08 PM, Ulf Hansson wrote:

Anyway, we can just drop this patch from your series as I amended the
patch causing the problem.

I will continue to review the rest.

yes, I sent to early. (sorry)

BR
Ludo

[PATCH 05/14] mmc: mmci: move mmci next cookie to mci host

From: Ludovic Barre <hidden>
Date: 2018-08-01 09:38:46

From: Ludovic Barre <redacted>

This patch moves next cookie to mmci host structure to
share same cookie management between all variants.

Signed-off-by: Ludovic Barre <redacted>
---
 drivers/mmc/host/mmci.c | 10 ++++------
 drivers/mmc/host/mmci.h |  2 ++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index bdc48c3..5646c2e6 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c

@@ -418,7 +418,6 @@ static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
 struct dmaengine_next {
 	struct dma_async_tx_descriptor *dma_desc;
 	struct dma_chan	*dma_chan;
-	s32 cookie;
 };
 
 struct dmaengine_priv {

@@ -449,7 +448,7 @@ static int mmci_dma_setup(struct mmci_host *host)
 							 "tx");
 
 	/* initialize pre request cookie */
-	dmae->next_data.cookie = 1;
+	host->next_cookie = 1;
 
 	/*
 	 * If only an RX channel is specified, the driver will

@@ -716,7 +715,7 @@ static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
 	struct dmaengine_priv *dmae = host->dma_priv;
 	struct dmaengine_next *next = &dmae->next_data;
 
-	WARN_ON(data->host_cookie && data->host_cookie != next->cookie);
+	WARN_ON(data->host_cookie && data->host_cookie != host->next_cookie);
 	WARN_ON(!data->host_cookie && (next->dma_desc || next->dma_chan));
 
 	dmae->dma_desc_current = next->dma_desc;

@@ -728,9 +727,7 @@ static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
 static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct mmci_host *host = mmc_priv(mmc);
-	struct dmaengine_priv *dmae = host->dma_priv;
 	struct mmc_data *data = mrq->data;
-	struct dmaengine_next *nd = &dmae->next_data;
 
 	if (!data)
 		return;

@@ -741,7 +738,8 @@ static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
 		return;
 
 	if (!mmci_dma_prep_next(host, data))
-		data->host_cookie = ++nd->cookie < 0 ? 1 : nd->cookie;
+		data->host_cookie = ++host->next_cookie < 0 ?
+			1 : host->next_cookie;
 }
 
 static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,

diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 260a1de..d2ec4fd 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h

@@ -318,6 +318,8 @@ struct mmci_host {
 	int (*get_rx_fifocnt)(struct mmci_host *h, u32 status, int remain);
 
 	void			*dma_priv;
+
+	s32			next_cookie;
 };
 
 void qcom_variant_init(struct mmci_host *host);

-- 
2.7.4

Re: [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops

From: Ulf Hansson <hidden>
Date: 2018-09-04 10:00:40

On 1 August 2018 at 11:36, Ludovic Barre [off-list ref] wrote:

From: Ludovic Barre <redacted>

This patch series prepares and adds callbacks for dma transfert at
mmci_host_ops. This series is composed of 3 parts:
-Internalize specific needs of legacy dmaengine.
-Create and setup dma_priv pointer
-Create generic callbacks  which share some features
(like cookie...) and call specific needs

I have now reviewed part of this series and provided you with some
comments, but will stop at this point.

Overall, the comments are about renaming and picking better function
names. Those comments should be easy to address in a new version.

However, the other more important point is the number of variant
callbacks you are adding. It's of course a balance to pick the right
level, to get both flexibility but also to avoid open coding. In the
end we don't want to get too many callbacks, but then it's better to
share common mmci code for variants, through mmci.h.

Finally, I would like to see a patch on top adding the support for the
new ST variant, so I can see how the callbacks and changes really are
being used. Can you please add that?

This patch series must be applied on top of
"mmc: mmci: Add and implement a ->dma_setup() callback for qcom dml"

Ludovic Barre (14):
  mmc: mmci: fix qcom dma issue during mmci init with new dma_setup
    callback
  mmc: mmci: internalize dma map/unmap into mmci dma functions
  mmc: mmci: internalize dma_inprogress into mmci dma functions
  mmc: mmci: introduce dma_priv pointer to mmci_host
  mmc: mmci: move mmci next cookie to mci host
  mmc: mmci: merge prepare data functions
  mmc: mmci: add prepare/unprepare_data callbacks
  mmc: mmci: add get_next_data callback
  mmc: mmci: modify dma_setup callback
  mmc: mmci: add dma_release callback
  mmc: mmci: add dma_start callback
  mmc: mmci: add dma_finalize callback
  mmc: mmci: add dma_error callback
  mmc: mmci: add validate_data callback

 drivers/mmc/host/mmci.c          | 458 ++++++++++++++++++++++++---------------
 drivers/mmc/host/mmci.h          |  45 ++--
 drivers/mmc/host/mmci_qcom_dml.c |  15 +-
 3 files changed, 322 insertions(+), 196 deletions(-)

--
2.7.4

Kind regards
Uffe

Re: [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops

From: Ludovic BARRE <hidden>
Date: 2018-09-05 09:14:24


On 09/04/2018 12:00 PM, Ulf Hansson wrote:

On 1 August 2018 at 11:36, Ludovic Barre [off-list ref] wrote:

quoted

From: Ludovic Barre <redacted>

This patch series prepares and adds callbacks for dma transfert at
mmci_host_ops. This series is composed of 3 parts:
-Internalize specific needs of legacy dmaengine.
-Create and setup dma_priv pointer
-Create generic callbacks  which share some features
(like cookie...) and call specific needs

I have now reviewed part of this series and provided you with some
comments, but will stop at this point.

thanks for your review

Overall, the comments are about renaming and picking better function
names. Those comments should be easy to address in a new version.

yes, there is no problem for the naming, I will change following your
recommendations.

However, the other more important point is the number of variant
callbacks you are adding. It's of course a balance to pick the right
level, to get both flexibility but also to avoid open coding. In the
end we don't want to get too many callbacks, but then it's better to
share common mmci code for variants, through mmci.h.

Finally, I would like to see a patch on top adding the support for the
new ST variant, so I can see how the callbacks and changes really are
being used. Can you please add that?

yes, I prepare a patch with sdmmc variant to show how callbacks are used.

About comment on patch 07/14:
 > So, having callbacks for dealing with dma_map|unmap() kind of
 > operations, becomes rather fine-grained and not very flexible.
 > Instead, what do you think of allowing the variant init function to
 > dynamically assign the ->pre_req() and the ->post_req() callbacks in
 > the struct mmc_host_ops. Common mmci functions to manage these
 > operations can instead be shared via mmci.h and called by the
 > variants.

I think we have the same goal or idea, regroup the common needs to avoid
too much specific drift.

I will try to describe the functions which are commons and the link to 
specific mmci_host_ops callbacks.

(I use function names of this series, but it's just for this example)

commons function used by mmci core:
-mmci_pre_request:
  check data and cookie, call common mmci_validate_data
  and mmci_prepare_data.

-mmci_post_request:
  check data and cookie then call common mmci_unprepare_data

-mmci_validate_data:
  check the common constraint of variants, call specific
  validate_data if defined.

-mmci_prepare_data:
  setup common next cookie, call specific prepare data if defined

-mmci_unprepare_data:
  clear the common cookie, call specific unprepare data if defined

-mmci_get_next_data:
  check cookie, call specific get_next_data if defined

-mmci_dma_setup:
  initialize common next_cookie, call specific dma_setup if defined

-mmci_dma_release
  just call dma_release if defined

-mmci_dma_start
  call common prepare data if not yet done (by next)
  call specific dma_start
  write common registers to start transfer and setup mmci mask

-mmci_dma_finalize:
  just call dma_finalize if defined

-mmci_dma_error
  just call dma_error if defined

mmci_host_ops specific:
struct mmci_host_ops
validate_data: 	could be use to check specific constraint of variant.
  		sdmmc has constraints on base & size for each element
		excepted the last element which has no constraint on
		size.
prepare_data: 	specific needs to prepare current or next data request.
		mmci: dma_map_sg on channel, use dmaengine api
		"dmaengine_prep_slave_sg" to queue a transfer
		sdmmc: dma_map_sg on sdmmc device and prepare the link
		list of internal dma
		
unprepare_data: specific needs to unprepare the data of request
		mmci: dma_unmap_sg on channel, use dmaengine api to
		terminate transfert.
		sdmmc: just unmap on sdmmc device.

get_next_data: 	manage specific needs to move on next data
		mmci: get next dmaengine descriptor and channel
		sdmmc: today, nothing

dma_setup: 	setup specific need if you use a Direct Memory Access
		mmci: use dmaengine api to request slave channel.
		sdmmc: alloc memory for link list, and define specific
		mmc->max_segs and mmc->max_seg_size.

dma_release: 	release specific resource if you use a Direct Memory
		access.
		mmci: use dmaengine api to release channel
		sdmmc: nothing

dma_start: 	set specific needs to start dma request
		mmci: use dmaengine api to submit and pending a dma
		transfert.
		sdmmc: set specific sdmmc registers to start an internal
		dma transfert

dma_finalize: 	set specific needs to finalize a request
		mmci: specific check on fifo status and
		channel/descriptor management.
		sdmmc: just clear a specific register of sdmmc

dma_error: 	specific error management.
		mmci: dmaengine api to terminate a transfer
		sdmmc: nothing

BR
Ludo

quoted

This patch series must be applied on top of
"mmc: mmci: Add and implement a ->dma_setup() callback for qcom dml"

Ludovic Barre (14):
   mmc: mmci: fix qcom dma issue during mmci init with new dma_setup
     callback
   mmc: mmci: internalize dma map/unmap into mmci dma functions
   mmc: mmci: internalize dma_inprogress into mmci dma functions
   mmc: mmci: introduce dma_priv pointer to mmci_host
   mmc: mmci: move mmci next cookie to mci host
   mmc: mmci: merge prepare data functions
   mmc: mmci: add prepare/unprepare_data callbacks
   mmc: mmci: add get_next_data callback
   mmc: mmci: modify dma_setup callback
   mmc: mmci: add dma_release callback
   mmc: mmci: add dma_start callback
   mmc: mmci: add dma_finalize callback
   mmc: mmci: add dma_error callback
   mmc: mmci: add validate_data callback

  drivers/mmc/host/mmci.c          | 458 ++++++++++++++++++++++++---------------
  drivers/mmc/host/mmci.h          |  45 ++--
  drivers/mmc/host/mmci_qcom_dml.c |  15 +-
  3 files changed, 322 insertions(+), 196 deletions(-)

--
2.7.4

Kind regards
Uffe

Re: [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops

From: Ulf Hansson <hidden>
Date: 2018-09-05 10:44:01

On 5 September 2018 at 11:13, Ludovic BARRE [off-list ref] wrote:


On 09/04/2018 12:00 PM, Ulf Hansson wrote:

quoted

On 1 August 2018 at 11:36, Ludovic Barre [off-list ref] wrote:

quoted

From: Ludovic Barre <redacted>

This patch series prepares and adds callbacks for dma transfert at
mmci_host_ops. This series is composed of 3 parts:
-Internalize specific needs of legacy dmaengine.
-Create and setup dma_priv pointer
-Create generic callbacks  which share some features
(like cookie...) and call specific needs


I have now reviewed part of this series and provided you with some
comments, but will stop at this point.


thanks for your review

quoted

Overall, the comments are about renaming and picking better function
names. Those comments should be easy to address in a new version.


yes, there is no problem for the naming, I will change following your
recommendations.

quoted

However, the other more important point is the number of variant
callbacks you are adding. It's of course a balance to pick the right
level, to get both flexibility but also to avoid open coding. In the
end we don't want to get too many callbacks, but then it's better to
share common mmci code for variants, through mmci.h.

Finally, I would like to see a patch on top adding the support for the
new ST variant, so I can see how the callbacks and changes really are
being used. Can you please add that?

yes, I prepare a patch with sdmmc variant to show how callbacks are used.

About comment on patch 07/14:

quoted

So, having callbacks for dealing with dma_map|unmap() kind of
operations, becomes rather fine-grained and not very flexible.
Instead, what do you think of allowing the variant init function to
dynamically assign the ->pre_req() and the ->post_req() callbacks in
the struct mmc_host_ops. Common mmci functions to manage these
operations can instead be shared via mmci.h and called by the
variants.

I think we have the same goal or idea, regroup the common needs to avoid
too much specific drift.

I will try to describe the functions which are commons and the link to
specific mmci_host_ops callbacks.

(I use function names of this series, but it's just for this example)

commons function used by mmci core:
-mmci_pre_request:
 check data and cookie, call common mmci_validate_data
 and mmci_prepare_data.

-mmci_post_request:
 check data and cookie then call common mmci_unprepare_data

-mmci_validate_data:
 check the common constraint of variants, call specific
 validate_data if defined.

-mmci_prepare_data:
 setup common next cookie, call specific prepare data if defined

-mmci_unprepare_data:
 clear the common cookie, call specific unprepare data if defined

-mmci_get_next_data:
 check cookie, call specific get_next_data if defined

-mmci_dma_setup:
 initialize common next_cookie, call specific dma_setup if defined

-mmci_dma_release
 just call dma_release if defined

-mmci_dma_start
 call common prepare data if not yet done (by next)
 call specific dma_start
 write common registers to start transfer and setup mmci mask

-mmci_dma_finalize:
 just call dma_finalize if defined

-mmci_dma_error
 just call dma_error if defined

mmci_host_ops specific:
struct mmci_host_ops
validate_data:  could be use to check specific constraint of variant.
                sdmmc has constraints on base & size for each element
                excepted the last element which has no constraint on
                size.
prepare_data:   specific needs to prepare current or next data request.
                mmci: dma_map_sg on channel, use dmaengine api
                "dmaengine_prep_slave_sg" to queue a transfer
                sdmmc: dma_map_sg on sdmmc device and prepare the link
                list of internal dma

unprepare_data: specific needs to unprepare the data of request
                mmci: dma_unmap_sg on channel, use dmaengine api to
                terminate transfert.
                sdmmc: just unmap on sdmmc device.

get_next_data:  manage specific needs to move on next data
                mmci: get next dmaengine descriptor and channel
                sdmmc: today, nothing

dma_setup:      setup specific need if you use a Direct Memory Access
                mmci: use dmaengine api to request slave channel.
                sdmmc: alloc memory for link list, and define specific
                mmc->max_segs and mmc->max_seg_size.

dma_release:    release specific resource if you use a Direct Memory
                access.
                mmci: use dmaengine api to release channel
                sdmmc: nothing

dma_start:      set specific needs to start dma request
                mmci: use dmaengine api to submit and pending a dma
                transfert.
                sdmmc: set specific sdmmc registers to start an internal
                dma transfert

dma_finalize:   set specific needs to finalize a request
                mmci: specific check on fifo status and
                channel/descriptor management.
                sdmmc: just clear a specific register of sdmmc

dma_error:      specific error management.
                mmci: dmaengine api to terminate a transfer
                sdmmc: nothing

Ludo, thanks for the detailed explanation and summary!

Following this, it looks like it makes sense to keep the callbacks as
on the level you have suggested. At least, I don't want to delay you
from getting this upstream, by just giving some vague ideas of how to
change the number of callbacks.

So, I am fine if you stick to the existing approach! Then, if we later
on realizes that it makes sense to share more common code through
mmci.h, to get rid of some callback, we can always do that on top.

[...]

Kind regards
Uffe

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help