On Thursday 18 April 2013, Russell King - ARM Linux wrote:
> On Thu, Apr 18, 2013 at 09:02:38AM +0100, Lee Jones wrote:
> > @@ -321,19 +323,21 @@ static void mmci_dma_setup(struct mmci_host *host)
> > * attempt to use it bidirectionally, however if it is
> > * is specified but cannot be located, DMA will be disabled.
> > */
> > - if (plat->dma_rx_param) {
> > - host->dma_rx_channel = dma_request_channel(mask,
> > - plat->dma_filter,
> > -
> > plat->dma_rx_param);
> > + if (plat->dma_rx_param || np) {
> > + host->dma_rx_channel = dma_request_slave_channel_compat(mask,
> > +
> > plat->dma_filter,
> > +
> > plat->dma_rx_param,
> > + &dev->dev,
> > "rx");
> > /* E.g if no DMA hardware is present */
> > if (!host->dma_rx_channel)
> > dev_err(mmc_dev(host->mmc), "no RX DMA channel\n");
>
> I don't think this is right - I think Arnd has been leading you up the
> garden path saying that this can be simplified. Why?
>
> If you look at what this code does, the DMA channels are optional. If
> they're not provided, then you don't get an error or a warning printk from
> the code. However, after your conversion, if you use DT and avoid giving
> the DMA information (which you have to avoid on the majority of ARM
> platforms) then "np" will be non-NULL, and
> dma_request_slave_channel_compat() will return NULL, causing the error
> and/or warning to be printed.
Right, so I guess we should print the warning only if plat->dma_filter
is non-NULL, or we don't use dma_request_slave_channel_compat, which
does not actually simplify the code here.
Arnd
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 375c109..c97bc92 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -298,20 +298,11 @@ 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)
+static int mmci_dma_plat_setup(struct mmci_host *host)
{
struct mmci_platform_data *plat = host->plat;
- const char *rxname, *txname;
dma_cap_mask_t mask;
- if (!plat || !plat->dma_filter) {
- dev_info(mmc_dev(host->mmc), "no DMA platform data\n");
- return;
- }
-
- /* initialize pre request cookie */
- host->next_data.cookie = 1;
-
/* Try to acquire a generic DMA engine slave channel */
dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
@@ -339,6 +330,25 @@ static void mmci_dma_setup(struct mmci_host *host)
} else {
host->dma_tx_channel = host->dma_rx_channel;
}
+}
+
+static void mmci_dma_setup(struct device *dev, struct mmci_host *host)
+{
+ const char *rxname, *txname;
+
+ host->dma_rx_channel = dma_request_slave_channel(dev, "rx");
+ host->dma_tx_channel = dma_request_slave_channel(dev, "tx");
+
+ if (!host->dma_rx_channel && !host->dma_tx_channel) {
+ if (host->plat && host->plat->dma_filter)
+ mmci_dma_plat_setup(host);
+ else
+ dev_info(mmc_dev(host->mmc), "no DMA platform data\n");
+ return;
+ }
+
+ /* initialize pre request cookie */
+ host->next_data.cookie = 1;
if (host->dma_rx_channel)
rxname = dma_chan_name(host->dma_rx_channel);
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
