diff mbox

[03/12] dma: mxs-dma: move to generic device tree binding

Message ID 1361978748-25281-4-git-send-email-shawn.guo@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Shawn Guo Feb. 27, 2013, 3:25 p.m. UTC 
Update mxs-dma driver to adopt generic DMA device tree binding.  It
calls of_dma_controller_register() with mxs specific of_dma_xlate to
get the generic DMA device tree helper support.  Then DMA clients only
need to call dma_request_slave_channel() for requesting a DMA channel
from dmaengine.

The existing way of requesting channel, clients directly call
dma_request_channel(), still work there, and will be removed after
all mxs-dma clients get converted to generic DMA device tree helper.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/dma/mxs-dma.c |   77 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 4 deletions(-)

Comments

Arnd Bergmann Feb. 27, 2013, 8:50 p.m. UTC | #1 
On Wednesday 27 February 2013, Shawn Guo wrote:
> @@ -139,6 +140,9 @@ struct mxs_dma_engine {
>  	struct dma_device		dma_device;
>  	struct device_dma_parameters	dma_parms;
>  	struct mxs_dma_chan		mxs_chans[MXS_DMA_CHANNELS];
> +	struct platform_device		*pdev;
> +	struct of_dma_filter_info	filter_info;
> +	unsigned int			nr_channels;
>  };

I don't think using of_dma_filter_info makes sense when we are not
using the generic xlate function:

> @@ -665,8 +667,59 @@ err_out:
>  	return ret;
>  }
>  
> +struct mxs_dma_filter_param {
> +	struct device_node *of_node;
> +	unsigned int chan_id;
> +};
> +
> +struct dma_chan *mxs_dma_xlate(struct of_phandle_args *dma_spec,
> +			       struct of_dma *ofdma)
> +{
> +	struct mxs_dma_engine *mxs_dma = ofdma->of_dma_data;
> +	struct of_dma_filter_info *info = &mxs_dma->filter_info;
> +	struct mxs_dma_filter_param param;
> +	int count = dma_spec->args_count;
> +
> +	if (!info || !info->filter_fn)
> +		return NULL;
> +
> +	if (count != 1)
> +		return NULL;
> +
> +	param.of_node = ofdma->of_node;
> +	param.chan_id = dma_spec->args[0];
> +
> +	if (param.chan_id >= mxs_dma->nr_channels)
> +		return NULL;
> +
> +	return dma_request_channel(info->dma_cap, info->filter_fn, &param);
> +}
> +
> +static bool mxs_dma_filter_fn(struct dma_chan *chan, void *fn_param)

You already know the value of info->filter_fn, it's always &mxs_dma_filter_fn,
so if you reorder these two functions, you can pass it directly into
dma_request_channel.

> +	struct mxs_dma_filter_param *param = fn_param;
> +	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
> +	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
> +	int chan_irq;
> +
> +	if (mxs_dma->dma_device.dev->of_node != param->of_node)
> +		return false;
> +
> +	if (chan->chan_id != param->chan_id)
> +		return false;

It would be nice if we could just get a pointer to the right channel
structure from the xlate() function, rather than having to walk the
list of channels and trying each one. Your method is what everyone
else is doing as well, so we can stick with it for now, but at some
point I'd like to open up this discussion again.

	Arnd
Shawn Guo Feb. 28, 2013, 7:24 a.m. UTC | #2 
On Wed, Feb 27, 2013 at 08:50:14PM +0000, Arnd Bergmann wrote:
> On Wednesday 27 February 2013, Shawn Guo wrote:
> > @@ -139,6 +140,9 @@ struct mxs_dma_engine {
> >  	struct dma_device		dma_device;
> >  	struct device_dma_parameters	dma_parms;
> >  	struct mxs_dma_chan		mxs_chans[MXS_DMA_CHANNELS];
> > +	struct platform_device		*pdev;
> > +	struct of_dma_filter_info	filter_info;
> > +	unsigned int			nr_channels;
> >  };
> 
> I don't think using of_dma_filter_info makes sense when we are not
> using the generic xlate function:
...
> > +static bool mxs_dma_filter_fn(struct dma_chan *chan, void *fn_param)
> 
> You already know the value of info->filter_fn, it's always &mxs_dma_filter_fn,
> so if you reorder these two functions, you can pass it directly into
> dma_request_channel.

Yeah, you're right.  I was blindly following the generic xlate example.
Thanks for spotting this.  Will fix it.

Shawn
diff mbox

Patch

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index bb86f7f..4edf8e9 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -27,6 +27,7 @@ 
 #include <linux/stmp_device.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_dma.h>
 
 #include <asm/irq.h>
 
@@ -139,6 +140,9 @@  struct mxs_dma_engine {
 	struct dma_device		dma_device;
 	struct device_dma_parameters	dma_parms;
 	struct mxs_dma_chan		mxs_chans[MXS_DMA_CHANNELS];
+	struct platform_device		*pdev;
+	struct of_dma_filter_info	filter_info;
+	unsigned int			nr_channels;
 };
 
 struct mxs_dma_type {
@@ -350,10 +354,8 @@  static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
 	int ret;
 
-	if (!data)
-		return -EINVAL;
-
-	mxs_chan->chan_irq = data->chan_irq;
+	if (data)
+		mxs_chan->chan_irq = data->chan_irq;
 
 	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
 				CCW_BLOCK_SIZE, &mxs_chan->ccw_phys,
@@ -665,8 +667,59 @@  err_out:
 	return ret;
 }
 
+struct mxs_dma_filter_param {
+	struct device_node *of_node;
+	unsigned int chan_id;
+};
+
+struct dma_chan *mxs_dma_xlate(struct of_phandle_args *dma_spec,
+			       struct of_dma *ofdma)
+{
+	struct mxs_dma_engine *mxs_dma = ofdma->of_dma_data;
+	struct of_dma_filter_info *info = &mxs_dma->filter_info;
+	struct mxs_dma_filter_param param;
+	int count = dma_spec->args_count;
+
+	if (!info || !info->filter_fn)
+		return NULL;
+
+	if (count != 1)
+		return NULL;
+
+	param.of_node = ofdma->of_node;
+	param.chan_id = dma_spec->args[0];
+
+	if (param.chan_id >= mxs_dma->nr_channels)
+		return NULL;
+
+	return dma_request_channel(info->dma_cap, info->filter_fn, &param);
+}
+
+static bool mxs_dma_filter_fn(struct dma_chan *chan, void *fn_param)
+{
+	struct mxs_dma_filter_param *param = fn_param;
+	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
+	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+	int chan_irq;
+
+	if (mxs_dma->dma_device.dev->of_node != param->of_node)
+		return false;
+
+	if (chan->chan_id != param->chan_id)
+		return false;
+
+	chan_irq = platform_get_irq(mxs_dma->pdev, param->chan_id);
+	if (chan_irq < 0)
+		return false;
+
+	mxs_chan->chan_irq = chan_irq;
+
+	return true;
+}
+
 static int __init mxs_dma_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	const struct platform_device_id *id_entry;
 	const struct of_device_id *of_id;
 	const struct mxs_dma_type *dma_type;
@@ -678,6 +731,12 @@  static int __init mxs_dma_probe(struct platform_device *pdev)
 	if (!mxs_dma)
 		return -ENOMEM;
 
+	ret = of_property_read_u32(np, "dma-channels", &mxs_dma->nr_channels);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to read dma-channels\n");
+		return ret;
+	}
+
 	of_id = of_match_device(mxs_dma_dt_ids, &pdev->dev);
 	if (of_id)
 		id_entry = of_id->data;
@@ -723,6 +782,7 @@  static int __init mxs_dma_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	mxs_dma->pdev = pdev;
 	mxs_dma->dma_device.dev = &pdev->dev;
 
 	/* mxs_dma gets 65535 bytes maximum sg size */
@@ -743,6 +803,15 @@  static int __init mxs_dma_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	mxs_dma->filter_info.dma_cap = mxs_dma->dma_device.cap_mask;
+	mxs_dma->filter_info.filter_fn = mxs_dma_filter_fn;
+	ret = of_dma_controller_register(np, mxs_dma_xlate, mxs_dma);
+	if (ret) {
+		dev_err(mxs_dma->dma_device.dev,
+			"failed to register controller\n");
+		dma_async_device_unregister(&mxs_dma->dma_device);
+	}
+
 	dev_info(mxs_dma->dma_device.dev, "initialized\n");
 
 	return 0;