* [PATCH 01/14] mmc: mmci: fix qcom dma issue during mmci init with new dma_setup callback
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-08-01 10:08 ` Ulf Hansson
2018-08-01 9:36 ` [PATCH 02/14] mmc: mmci: internalize dma map/unmap into mmci dma functions Ludovic Barre
` (13 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
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 <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 01/14] mmc: mmci: fix qcom dma issue during mmci init with new dma_setup callback
2018-08-01 9:36 ` [PATCH 01/14] mmc: mmci: fix qcom dma issue during mmci init with new dma_setup callback Ludovic Barre
@ 2018-08-01 10:08 ` Ulf Hansson
2018-08-01 10:09 ` Ludovic BARRE
0 siblings, 1 reply; 25+ messages in thread
From: Ulf Hansson @ 2018-08-01 10:08 UTC (permalink / raw)
To: Ludovic Barre
Cc: Rob Herring, Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc
On 1 August 2018 at 11:36, Ludovic Barre <ludovic.Barre@st.com> wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
>
> 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 <ludovic.barre@st.com>
> ---
> 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
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 01/14] mmc: mmci: fix qcom dma issue during mmci init with new dma_setup callback
2018-08-01 10:08 ` Ulf Hansson
@ 2018-08-01 10:09 ` Ludovic BARRE
0 siblings, 0 replies; 25+ messages in thread
From: Ludovic BARRE @ 2018-08-01 10:09 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rob Herring, Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 02/14] mmc: mmci: internalize dma map/unmap into mmci dma functions
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
2018-08-01 9:36 ` [PATCH 01/14] mmc: mmci: fix qcom dma issue during mmci init with new dma_setup callback Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-09-03 12:14 ` Ulf Hansson
2018-08-01 9:36 ` [PATCH 03/14] mmc: mmci: internalize dma_inprogress " Ludovic Barre
` (12 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
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 <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 02/14] mmc: mmci: internalize dma map/unmap into mmci dma functions
2018-08-01 9:36 ` [PATCH 02/14] mmc: mmci: internalize dma map/unmap into mmci dma functions Ludovic Barre
@ 2018-09-03 12:14 ` Ulf Hansson
0 siblings, 0 replies; 25+ messages in thread
From: Ulf Hansson @ 2018-09-03 12:14 UTC (permalink / raw)
To: Ludovic Barre
Cc: Rob Herring, Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc
On 1 August 2018 at 11:36, Ludovic Barre <ludovic.Barre@st.com> wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
>
> 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 <ludovic.barre@st.com>
> ---
> 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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 03/14] mmc: mmci: internalize dma_inprogress into mmci dma functions
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
2018-08-01 9:36 ` [PATCH 01/14] mmc: mmci: fix qcom dma issue during mmci init with new dma_setup callback Ludovic Barre
2018-08-01 9:36 ` [PATCH 02/14] mmc: mmci: internalize dma map/unmap into mmci dma functions Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-09-03 12:15 ` Ulf Hansson
2018-08-01 9:36 ` [PATCH 04/14] mmc: mmci: introduce dma_priv pointer to mmci_host Ludovic Barre
` (11 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
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 <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 03/14] mmc: mmci: internalize dma_inprogress into mmci dma functions
2018-08-01 9:36 ` [PATCH 03/14] mmc: mmci: internalize dma_inprogress " Ludovic Barre
@ 2018-09-03 12:15 ` Ulf Hansson
0 siblings, 0 replies; 25+ messages in thread
From: Ulf Hansson @ 2018-09-03 12:15 UTC (permalink / raw)
To: Ludovic Barre
Cc: Rob Herring, Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc
On 1 August 2018 at 11:36, Ludovic Barre <ludovic.Barre@st.com> wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
>
> 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 <ludovic.barre@st.com>
[...]
> 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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 04/14] mmc: mmci: introduce dma_priv pointer to mmci_host
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (2 preceding siblings ...)
2018-08-01 9:36 ` [PATCH 03/14] mmc: mmci: internalize dma_inprogress " Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-09-03 12:15 ` Ulf Hansson
2018-08-01 9:36 ` [PATCH 05/14] mmc: mmci: move mmci next cookie to mci host Ludovic Barre
` (10 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
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 <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 04/14] mmc: mmci: introduce dma_priv pointer to mmci_host
2018-08-01 9:36 ` [PATCH 04/14] mmc: mmci: introduce dma_priv pointer to mmci_host Ludovic Barre
@ 2018-09-03 12:15 ` Ulf Hansson
0 siblings, 0 replies; 25+ messages in thread
From: Ulf Hansson @ 2018-09-03 12:15 UTC (permalink / raw)
To: Ludovic Barre
Cc: Rob Herring, Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc
On 1 August 2018 at 11:36, Ludovic Barre <ludovic.Barre@st.com> wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
>
> 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.
>
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> ---
> 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.
> +
> +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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 05/14] mmc: mmci: move mmci next cookie to mci host
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (3 preceding siblings ...)
2018-08-01 9:36 ` [PATCH 04/14] mmc: mmci: introduce dma_priv pointer to mmci_host Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-08-01 9:36 ` [PATCH 06/14] mmc: mmci: merge prepare data functions Ludovic Barre
` (9 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
This patch moves next cookie to mmci host structure to
share same cookie management between all variants.
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 06/14] mmc: mmci: merge prepare data functions
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (4 preceding siblings ...)
2018-08-01 9:36 ` [PATCH 05/14] mmc: mmci: move mmci next cookie to mci host Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-09-03 12:15 ` Ulf Hansson
2018-08-01 9:36 ` [PATCH 07/14] mmc: mmci: add prepare/unprepare_data callbacks Ludovic Barre
` (8 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
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 <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 06/14] mmc: mmci: merge prepare data functions
2018-08-01 9:36 ` [PATCH 06/14] mmc: mmci: merge prepare data functions Ludovic Barre
@ 2018-09-03 12:15 ` Ulf Hansson
0 siblings, 0 replies; 25+ messages in thread
From: Ulf Hansson @ 2018-09-03 12:15 UTC (permalink / raw)
To: Ludovic Barre
Cc: Rob Herring, Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc
On 1 August 2018 at 11:36, Ludovic Barre <ludovic.Barre@st.com> wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
>
> 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 <ludovic.barre@st.com>
> ---
> 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.
> + 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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 07/14] mmc: mmci: add prepare/unprepare_data callbacks
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (5 preceding siblings ...)
2018-08-01 9:36 ` [PATCH 06/14] mmc: mmci: merge prepare data functions Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-09-04 9:43 ` Ulf Hansson
2018-08-01 9:36 ` [PATCH 08/14] mmc: mmci: add get_next_data callback Ludovic Barre
` (7 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
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 <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 07/14] mmc: mmci: add prepare/unprepare_data callbacks
2018-08-01 9:36 ` [PATCH 07/14] mmc: mmci: add prepare/unprepare_data callbacks Ludovic Barre
@ 2018-09-04 9:43 ` Ulf Hansson
0 siblings, 0 replies; 25+ messages in thread
From: Ulf Hansson @ 2018-09-04 9:43 UTC (permalink / raw)
To: Ludovic Barre
Cc: Rob Herring, Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc
[...]
>
> 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?
> +
> + 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.
> {
> 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?
>
> +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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 08/14] mmc: mmci: add get_next_data callback
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (6 preceding siblings ...)
2018-08-01 9:36 ` [PATCH 07/14] mmc: mmci: add prepare/unprepare_data callbacks Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-08-01 9:36 ` [PATCH 09/14] mmc: mmci: modify dma_setup callback Ludovic Barre
` (6 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
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 <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 09/14] mmc: mmci: modify dma_setup callback
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (7 preceding siblings ...)
2018-08-01 9:36 ` [PATCH 08/14] mmc: mmci: add get_next_data callback Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-08-01 9:36 ` [PATCH 10/14] mmc: mmci: add dma_release callback Ludovic Barre
` (5 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
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 <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 10/14] mmc: mmci: add dma_release callback
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (8 preceding siblings ...)
2018-08-01 9:36 ` [PATCH 09/14] mmc: mmci: modify dma_setup callback Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-08-01 9:36 ` [PATCH 11/14] mmc: mmci: add dma_start callback Ludovic Barre
` (4 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
This patch adds dma_release callback at mmci_host_ops
to allow to call specific variant.
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 11/14] mmc: mmci: add dma_start callback
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (9 preceding siblings ...)
2018-08-01 9:36 ` [PATCH 10/14] mmc: mmci: add dma_release callback Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-08-01 9:36 ` [PATCH 12/14] mmc: mmci: add dma_finalize callback Ludovic Barre
` (3 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
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 <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 12/14] mmc: mmci: add dma_finalize callback
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (10 preceding siblings ...)
2018-08-01 9:36 ` [PATCH 11/14] mmc: mmci: add dma_start callback Ludovic Barre
@ 2018-08-01 9:36 ` Ludovic Barre
2018-08-01 9:37 ` [PATCH 13/14] mmc: mmci: add dma_error callback Ludovic Barre
` (2 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:36 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
This patch adds dma_finalize callback at mmci_host_ops
to allow to call specific variant.
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 13/14] mmc: mmci: add dma_error callback
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (11 preceding siblings ...)
2018-08-01 9:36 ` [PATCH 12/14] mmc: mmci: add dma_finalize callback Ludovic Barre
@ 2018-08-01 9:37 ` Ludovic Barre
2018-08-01 9:37 ` [PATCH 14/14] mmc: mmci: add validate_data callback Ludovic Barre
2018-09-04 10:00 ` [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ulf Hansson
14 siblings, 0 replies; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:37 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
This patch adds dma_error callback at mmci_host_ops
to allow to call specific variant.
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 14/14] mmc: mmci: add validate_data callback
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (12 preceding siblings ...)
2018-08-01 9:37 ` [PATCH 13/14] mmc: mmci: add dma_error callback Ludovic Barre
@ 2018-08-01 9:37 ` Ludovic Barre
2018-09-04 10:00 ` [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ulf Hansson
14 siblings, 0 replies; 25+ messages in thread
From: Ludovic Barre @ 2018-08-01 9:37 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
Ludovic Barre
From: Ludovic Barre <ludovic.barre@st.com>
This patch adds validate_data callback at mmci_host_ops
to check specific constraints of variant.
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops
2018-08-01 9:36 [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ludovic Barre
` (13 preceding siblings ...)
2018-08-01 9:37 ` [PATCH 14/14] mmc: mmci: add validate_data callback Ludovic Barre
@ 2018-09-04 10:00 ` Ulf Hansson
2018-09-05 9:13 ` Ludovic BARRE
14 siblings, 1 reply; 25+ messages in thread
From: Ulf Hansson @ 2018-09-04 10:00 UTC (permalink / raw)
To: Ludovic Barre
Cc: Rob Herring, Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc
On 1 August 2018 at 11:36, Ludovic Barre <ludovic.Barre@st.com> wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
>
> 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
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops
2018-09-04 10:00 ` [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops Ulf Hansson
@ 2018-09-05 9:13 ` Ludovic BARRE
2018-09-05 10:43 ` Ulf Hansson
0 siblings, 1 reply; 25+ messages in thread
From: Ludovic BARRE @ 2018-09-05 9:13 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rob Herring, Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc
On 09/04/2018 12:00 PM, Ulf Hansson wrote:
> On 1 August 2018 at 11:36, Ludovic Barre <ludovic.Barre@st.com> wrote:
>> From: Ludovic Barre <ludovic.barre@st.com>
>>
>> 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
>>
>> 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
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 00/14] mmc: mmci: prepare dma callbacks with mmci_host_ops
2018-09-05 9:13 ` Ludovic BARRE
@ 2018-09-05 10:43 ` Ulf Hansson
0 siblings, 0 replies; 25+ messages in thread
From: Ulf Hansson @ 2018-09-05 10:43 UTC (permalink / raw)
To: Ludovic BARRE
Cc: Rob Herring, Maxime Coquelin, Alexandre Torgue, Gerald Baeza,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc
On 5 September 2018 at 11:13, Ludovic BARRE <ludovic.barre@st.com> wrote:
>
>
> On 09/04/2018 12:00 PM, Ulf Hansson wrote:
>>
>> On 1 August 2018 at 11:36, Ludovic Barre <ludovic.Barre@st.com> wrote:
>>>
>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>
>>> 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
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
^ permalink raw reply [flat|nested] 25+ messages in thread