Re: [PATCH] mmc: mmci: Allow MMCI to request channels with information acquired from DT

[Arnd Bergmann]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/