From: Ludovic BARRE <ludovic.barre@st.com>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>,
Maxime Coquelin <mcoquelin.stm32@gmail.com>,
Alexandre Torgue <alexandre.torgue@st.com>,
Benjamin Gaignard <benjamin.gaignard@linaro.org>,
Gerald Baeza <gerald.baeza@st.com>,
Loic Pallardy <loic.pallardy@st.com>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
DTML <devicetree@vger.kernel.org>,
"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
<linux-stm32@st-md-mailman.stormreply.com>
Subject: Re: [PATCH V2 04/27] mmc: mmci: introduce dma_priv pointer to mmci_host
Date: Tue, 25 Sep 2018 09:19:21 +0200 [thread overview]
Message-ID: <42b6a3dc-df03-9ba6-7863-8791007b3432@st.com> (raw)
In-Reply-To: <CAPDyKFqQNN9X+x=pg9TOArT4-Tx9jNkpM2T+rAxQj-rzHXC0OA@mail.gmail.com>
On 09/24/2018 08:52 PM, Ulf Hansson wrote:
> On 21 September 2018 at 11:45, 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.
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>> ---
>> change v2:
>> -rename specific dma engine structure to mmci_dmae_next/priv
>> -remove dma prefixe of mmci_dmae_priv fields, rename "current"
>> field to "cur" this avoid build issue with "current" defined
>> in include/asm-generic/current.h
>>
>> drivers/mmc/host/mmci.c | 155 ++++++++++++++++++++++++++++++------------------
>> drivers/mmc/host/mmci.h | 18 +-----
>> 2 files changed, 99 insertions(+), 74 deletions(-)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index 2f845f3..6de7c8d 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
>> +struct mmci_dmae_next {
>> + struct dma_async_tx_descriptor *desc;
>> + struct dma_chan *chan;
>> + s32 cookie;
>> +};
>> +
>> +struct mmci_dmae_priv {
>> + struct dma_chan *cur;
>> + struct dma_chan *rx_channel;
>> + struct dma_chan *tx_channel;
>> + struct dma_async_tx_descriptor *desc_current;
>> + struct mmci_dmae_next next_data;
>> + bool in_progress;
>
> I am wondering whether it would make sense to keep the "bool
> in_progress" in the struct mmci_host. I guess it will be used for all
> dma variants anyway!?
Today it's only use in dma_engine config, not used by sdmmc variant.
But it's could moved to mmci_host structure, no problem.
>
>> +};
>> +
>> +#define mmci_dmae_inprogress(dmae) ((dmae)->in_progress)
>> +
>> static int mmci_dma_setup(struct mmci_host *host)
>> {
>> const char *rxname, *txname;
>> + struct mmci_dmae_priv *dmae;
>> +
>> + dmae = devm_kzalloc(mmc_dev(host->mmc), sizeof(*dmae), GFP_KERNEL);
>> + if (!dmae)
>> + return -ENOMEM;
>>
>> - 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");
>> + host->dma_priv = dmae;
>> +
>> + dmae->rx_channel = dma_request_slave_channel(mmc_dev(host->mmc),
>> + "rx");
>> + dmae->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->rx_channel && !dmae->tx_channel)
>> + dmae->tx_channel = dmae->rx_channel;
>>
>> - if (host->dma_rx_channel)
>> - rxname = dma_chan_name(host->dma_rx_channel);
>> + if (dmae->rx_channel)
>> + rxname = dma_chan_name(dmae->rx_channel);
>> else
>> rxname = "none";
>>
>> - if (host->dma_tx_channel)
>> - txname = dma_chan_name(host->dma_tx_channel);
>> + if (dmae->tx_channel)
>> + txname = dma_chan_name(dmae->tx_channel);
>> else
>> txname = "none";
>>
>> @@ -450,15 +476,15 @@ static int 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->tx_channel) {
>> + struct device *dev = dmae->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->rx_channel) {
>> + struct device *dev = dmae->rx_channel->device->dev;
>> unsigned int max_seg_size = dma_get_max_seg_size(dev);
>>
>> if (max_seg_size < host->mmc->max_seg_size)
>> @@ -477,21 +503,24 @@ static int 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 mmci_dmae_priv *dmae = host->dma_priv;
>> +
>> + if (dmae->rx_channel)
>> + dma_release_channel(dmae->rx_channel);
>> + if (dmae->tx_channel)
>> + dma_release_channel(dmae->tx_channel);
>> + dmae->rx_channel = dmae->tx_channel = NULL;
>> }
>>
>> static void mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)
>> {
>> + struct mmci_dmae_priv *dmae = host->dma_priv;
>> struct dma_chan *chan;
>>
>> if (data->flags & MMC_DATA_READ)
>> - chan = host->dma_rx_channel;
>> + chan = dmae->rx_channel;
>> else
>> - chan = host->dma_tx_channel;
>> + chan = dmae->tx_channel;
>>
>> dma_unmap_sg(chan->device->dev, data->sg, data->sg_len,
>> mmc_get_dma_dir(data));
>> @@ -499,14 +528,16 @@ 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 mmci_dmae_priv *dmae = host->dma_priv;
>> +
>> + if (!mmci_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->cur);
>> + dmae->in_progress = false;
>> + dmae->cur = NULL;
>> + dmae->desc_current = NULL;
>> host->data->host_cookie = 0;
>>
>> mmci_dma_unmap(host, host->data);
>> @@ -514,10 +545,11 @@ static void mmci_dma_data_error(struct mmci_host *host)
>>
>> static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data)
>> {
>> + struct mmci_dmae_priv *dmae = host->dma_priv;
>> u32 status;
>> int i;
>>
>> - if (!dma_inprogress(host))
>> + if (!mmci_dmae_inprogress(dmae))
>> return;
>>
>> /* Wait up to 1ms for the DMA to complete */
>> @@ -551,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->in_progress = false;
>> + dmae->cur = NULL;
>> + dmae->desc_current = NULL;
>> }
>>
>> /* prepares DMA channel and DMA descriptor, returns non-zero on failure */
>> @@ -561,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 mmci_dmae_priv *dmae = host->dma_priv;
>> struct variant_data *variant = host->variant;
>> struct dma_slave_config conf = {
>> .src_addr = host->phybase + MMCIFIFO,
>> @@ -579,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->rx_channel;
>> } else {
>> conf.direction = DMA_MEM_TO_DEV;
>> - chan = host->dma_tx_channel;
>> + chan = dmae->tx_channel;
>> }
>>
>> /* If there's no DMA channel, fall back to PIO */
>> @@ -622,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 mmci_dmae_priv *dmae = host->dma_priv;
>> +
>> /* Check if next job is already prepared. */
>> - if (host->dma_current && host->dma_desc_current)
>> + if (dmae->cur && dmae->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->cur,
>> + &dmae->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;
>> - return __mmci_dma_prep_data(host, data, &nd->dma_chan, &nd->dma_desc);
>> + struct mmci_dmae_priv *dmae = host->dma_priv;
>> + struct mmci_dmae_next *nd = &dmae->next_data;
>> +
>> + return __mmci_dma_prep_data(host, data, &nd->chan, &nd->desc);
>> }
>>
>> static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
>> {
>> - int ret;
>> + struct mmci_dmae_priv *dmae = host->dma_priv;
>> struct mmc_data *data = host->data;
>> + int ret;
>
> Looks like "white space". Anyway, please avoid move the declaration of
> ret, unless needed.
Ok
>
>>
>> ret = mmci_dma_prep_data(host, host->data);
>> if (ret)
>> @@ -651,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->in_progress = true;
>> + dmaengine_submit(dmae->desc_current);
>> + dma_async_issue_pending(dmae->cur);
>>
>> if (host->variant->qcom_dml)
>> dml_start_xfer(host, data);
>> @@ -675,22 +713,24 @@ 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 mmci_dmae_priv *dmae = host->dma_priv;
>> + struct mmci_dmae_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));
>> + WARN_ON(!data->host_cookie && (next->desc || next->chan));
>>
>> - host->dma_desc_current = next->dma_desc;
>> - host->dma_current = next->dma_chan;
>> - next->dma_desc = NULL;
>> - next->dma_chan = NULL;
>> + dmae->desc_current = next->desc;
>> + dmae->cur = next->chan;
>> + next->desc = NULL;
>> + next->chan = NULL;
>> }
>>
>> static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
>> {
>> struct mmci_host *host = mmc_priv(mmc);
>> + struct mmci_dmae_priv *dmae = host->dma_priv;
>> struct mmc_data *data = mrq->data;
>> - struct mmci_host_next *nd = &host->next_data;
>> + struct mmci_dmae_next *nd = &dmae->next_data;
>>
>> if (!data)
>> return;
>> @@ -708,6 +748,7 @@ static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
>> int err)
>> {
>> struct mmci_host *host = mmc_priv(mmc);
>> + struct mmci_dmae_priv *dmae = host->dma_priv;
>> struct mmc_data *data = mrq->data;
>>
>> if (!data || !data->host_cookie)
>> @@ -716,24 +757,24 @@ static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
>> mmci_dma_unmap(host, data);
>>
>> if (err) {
>> - struct mmci_host_next *next = &host->next_data;
>> + struct mmci_dmae_next *next = &dmae->next_data;
>> struct dma_chan *chan;
>> if (data->flags & MMC_DATA_READ)
>> - chan = host->dma_rx_channel;
>> + chan = dmae->rx_channel;
>> else
>> - chan = host->dma_tx_channel;
>> + chan = dmae->tx_channel;
>> dmaengine_terminate_all(chan);
>>
>> - if (host->dma_desc_current == next->dma_desc)
>> - host->dma_desc_current = NULL;
>> + if (dmae->desc_current == next->desc)
>> + dmae->desc_current = NULL;
>>
>> - if (host->dma_current == next->dma_chan) {
>> - host->dma_in_progress = false;
>> - host->dma_current = NULL;
>> + if (dmae->cur == next->chan) {
>> + dmae->in_progress = false;
>> + dmae->cur = NULL;
>> }
>>
>> - next->dma_desc = NULL;
>> - next->dma_chan = NULL;
>> + next->desc = NULL;
>> + next->chan = NULL;
>> data->host_cookie = 0;
>> }
>> }
>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
>> index 06299ac..1e9a45b 100644
>> --- a/drivers/mmc/host/mmci.h
>> +++ b/drivers/mmc/host/mmci.h
>> @@ -276,12 +276,6 @@ struct mmci_host_ops {
>> int (*dma_setup)(struct mmci_host *host);
>> };
>>
>> -struct mmci_host_next {
>> - struct dma_async_tx_descriptor *dma_desc;
>> - struct dma_chan *dma_chan;
>> - s32 cookie;
>> -};
>> -
>> struct mmci_host {
>> phys_addr_t phybase;
>> void __iomem *base;
>> @@ -323,16 +317,6 @@ 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;
>> };
>>
>> --
>> 2.7.4
>>
>
> Kind regards
> Uffe
>
next prev parent reply other threads:[~2018-09-25 7:20 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-21 9:45 [PATCH V2 00/27] mmc: mmci: add sdmmc variant for stm32 Ludovic Barre
2018-09-21 9:45 ` [PATCH V2 01/27] mmc: mmci: internalize dma map/unmap into mmci dma functions Ludovic Barre
2018-09-26 23:49 ` Ulf Hansson
2018-09-21 9:45 ` [PATCH V2 02/27] mmc: mmci: internalize dma_inprogress " Ludovic Barre
2018-09-26 23:49 ` Ulf Hansson
2018-09-21 9:45 ` [PATCH V2 03/27] mmc: mmci: convert dma_setup callback to return an int Ludovic Barre
2018-09-24 14:28 ` Ulf Hansson
2018-09-24 16:12 ` Ludovic BARRE
2018-09-26 23:49 ` Ulf Hansson
2018-09-27 8:55 ` Srinivas Kandagatla
2018-09-21 9:45 ` [PATCH V2 04/27] mmc: mmci: introduce dma_priv pointer to mmci_host Ludovic Barre
2018-09-24 18:52 ` Ulf Hansson
2018-09-25 7:19 ` Ludovic BARRE [this message]
2018-09-21 9:45 ` [PATCH V2 05/27] mmc: mmci: move mmci next cookie to mci host Ludovic Barre
2018-09-24 18:46 ` Ulf Hansson
2018-09-25 7:08 ` Ludovic BARRE
2018-09-21 9:46 ` [PATCH V2 06/27] mmc: mmci: merge prepare data functions Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 07/27] mmc: mmci: add prepare/unprepare_data callbacks Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 08/27] mmc: mmci: add get_next_data callback Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 09/27] mmc: mmci: modify dma_setup callback Ludovic Barre
2018-10-01 9:30 ` Ulf Hansson
2018-10-01 9:43 ` Ludovic BARRE
2018-09-21 9:46 ` [PATCH V2 10/27] mmc: mmci: add dma_release callback Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 11/27] mmc: mmci: add dma_start callback Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 12/27] mmc: mmci: add dma_finalize callback Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 13/27] mmc: mmci: add dma_error callback Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 14/27] mmc: mmci: add validate_data callback Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 15/27] mmc: mmci: add set_clk/pwrreg callbacks Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 16/27] mmc: mmci: add datactrl block size variant property Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 17/27] mmc: mmci: expand startbiterr to irqmask and error check Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 18/27] mmc: mmci: add variant properties to define cpsm & cmdresp bits Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 19/27] mmc: mmci: add variant property to define dpsm bit Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 20/27] mmc: mmci: add variant property to define irq pio mask Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 21/27] mmc: mmci: add variant property to write datactrl before command Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 22/27] mmc: mmci: add variant property to not read datacnt Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 23/27] mmc: mmci: add variant property to request a reset Ludovic Barre
2018-10-01 9:30 ` Ulf Hansson
2018-10-01 12:16 ` Ludovic BARRE
2018-10-01 13:43 ` Ulf Hansson
2018-09-21 9:46 ` [PATCH V2 24/27] mmc: mmci: add clock divider for stm32 sdmmc Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 25/27] mmc: mmci: add stm32 sdmmc registers Ludovic Barre
2018-09-21 9:46 ` [PATCH V2 26/27] mmc: mmci: add DT bindings for STM32 sdmmc Ludovic Barre
2018-09-25 15:27 ` Rob Herring
2018-10-01 9:30 ` Ulf Hansson
2018-09-21 9:46 ` [PATCH V2 27/27] mmc: mmci: add stm32 sdmmc variant Ludovic Barre
2018-10-01 9:31 ` Ulf Hansson
2018-10-01 12:56 ` Ludovic BARRE
2018-10-01 13:39 ` Ulf Hansson
2018-10-01 13:53 ` Ludovic BARRE
-- strict thread matches above, loose matches on Subject: below --
2018-09-18 10:55 [PATCH V2 00/27] mmc: mmci: add sdmmc variant for stm32 Ludovic Barre
2018-09-18 10:55 ` [PATCH V2 04/27] mmc: mmci: introduce dma_priv pointer to mmci_host Ludovic Barre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=42b6a3dc-df03-9ba6-7863-8791007b3432@st.com \
--to=ludovic.barre@st.com \
--cc=alexandre.torgue@st.com \
--cc=benjamin.gaignard@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=gerald.baeza@st.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=loic.pallardy@st.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=robh+dt@kernel.org \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®