On 07/05/2018 05:17 PM, Ulf Hansson wrote:
On 12 June 2018 at 15:14, Ludovic Barre <ludovic.Barre@xxxxxx> wrote:
From: Ludovic Barre <ludovic.barre@xxxxxx>
Prepare mmci driver to manage dma interface by new property.
This patch defines and regroups dma operations for mmci drivers.
mmci_dma_XX prototypes are added to call member of mmci_dma_ops
if not null. Based on legacy need, a mmci dma interface has been
defined with:
-mmci_dma_setup
-mmci_dma_release
-mmci_dma_pre_req
-mmci_dma_start
-mmci_dma_finalize
-mmci_dma_post_req
-mmci_dma_error
-mmci_dma_get_next_data
As I suggested for one of the other patches, I would rather turn core
mmci functions into library functions, which can be either invoked
from variant callbacks or assigned directly to them.
In other words, I would leave the functions that you move in this
patch to stay in mmci.c. Although some needs to be re-factored and we
also needs to make some of them available to be called from another
file, hence the functions needs to be shared via mmci.h rather than
being declared static.
In previous exchange mail "STM32MP1 SDMMC driver review"
we are said:
>>> -dma variant à should fit in Qualcomm implementation, reuse (rename)
>>> mmci_qcom_dml.c file and integrate ST dma in.
>>
>> stm32 sdmmc has an internal dma, no need to use dmaengine API;
>> So some modifications in mmci (pre/post request, mmci_dma_xx). perhaps
>> should be done with an ops or not.
>
>Yes.
>
>The Qualcomm variant is also using an internal DMA, hence I thought
>there may be something we could re-use, or at least have some new
>common ops for.
It's not crystal clear for me.
Do you always agree with a dma ops which allow to address different
DMA transfer:
-with dmaengine API
-sdmmc idma, without dmaengine API
-...
Let me take a concrete example on how I would move forward, hopefully
that explains it a bit better. Please see below.
[...]
-/*
- * All the DMA operation mode stuff goes inside this ifdef.
- * This assumes that you have a generic DMA device interface,
- * no custom DMA interfaces are supported.
- */
-#ifdef CONFIG_DMA_ENGINE
-static void mmci_dma_setup(struct mmci_host *host)
-{
- const char *rxname, *txname;
- struct variant_data *variant = host->variant;
-
- 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");
-
- /* initialize pre request cookie */
- host->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 (host->dma_rx_channel)
- rxname = dma_chan_name(host->dma_rx_channel);
- else
- rxname = "none";
-
- if (host->dma_tx_channel)
- txname = dma_chan_name(host->dma_tx_channel);
- else
- txname = "none";
-
- dev_info(mmc_dev(host->mmc), "DMA channels RX %s, TX %s\n",
- rxname, txname);
-
- /*
- * 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;
- 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;
- 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;
- }
Everything above shall be left as generic library function,
mmci_dma_setup() and I would share it via mmci.h and thus change it
from being static.
each interfaces mmci_dma_XXX have very different needs depending
dma_ops (legacy, sdmmc idma)
-
- if (variant->qcom_dml && host->dma_rx_channel && host->dma_tx_channel)
- if (dml_hw_init(host, host->mmc->parent->of_node))
- variant->qcom_dml = false;
This piece of code, should be made specific to the qcom variant and
managed though a "mmci_host_ops" callback. The corresponding code in
that callback would then start by invoking mmci_dma_setup(), before it
continues with the qcom specific operations.
For legacy variants, the corresponding callback would be set directly
to mmci_dma_setup() and called through the callback from mmci.c when
needed. There is no need to have a separate file for DMA for the
legacy variants, I think.
[...]
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
