[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Richard Zhao]

pass of_phandle_args dma_spec to dma_request_channel in of_dma_simple_xlate,
so the filter function could access of_node in of_phandle_args.

It also remove restriction of #dma-cells has to be one.

Signed-off-by: Richard Zhao <rizhao@nvidia.com>
---
 drivers/dma/edma.c     |  7 +++++--
 drivers/dma/of-dma.c   | 10 ++++------
 drivers/dma/omap-dma.c |  6 ++++--
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 4f6d87b..54582fd 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/of_dma.h>
 
 #include <linux/platform_data/edma.h>
 
@@ -604,9 +605,11 @@ static struct platform_driver edma_driver = {
 
 bool edma_filter_fn(struct dma_chan *chan, void *param)
 {
-	if (chan->device->dev->driver == &edma_driver.driver) {
+	struct of_phandle_args *dma_spec = param;
+	if (chan->device->dev->driver == &edma_driver.driver &&
+			dma_spec->np == chan->device->dev->of_node) {
 		struct edma_chan *echan = to_edma_chan(chan);
-		unsigned ch_req = *(unsigned *)param;
+		unsigned ch_req = dma_spec->args[0];
 		return ch_req == echan->ch_num;
 	}
 	return false;
diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 75334bd..e1c4d3b 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -192,11 +192,9 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
  * @dma_spec:	pointer to DMA specifier as found in the device tree
  * @of_dma:	pointer to DMA controller data
  *
- * A simple translation function for devices that use a 32-bit value for the
+ * A simple translation function for devices that use dma_spec for the
  * filter_param when calling the DMA engine dma_request_channel() function.
- * Note that this translation function requires that #dma-cells is equal to 1
- * and the argument of the dma specifier is the 32-bit filter_param. Returns
- * pointer to appropriate dma channel on success or NULL on error.
+ * Returns pointer to appropriate dma channel on success or NULL on error.
  */
 struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 						struct of_dma *ofdma)
@@ -207,10 +205,10 @@ struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 	if (!info || !info->filter_fn)
 		return NULL;
 
-	if (count != 1)
+	if (count < 1)
 		return NULL;
 
 	return dma_request_channel(info->dma_cap, info->filter_fn,
-			&dma_spec->args[0]);
+			dma_spec);
 }
 EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index ec3fc4f..32547ec 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -693,9 +693,11 @@ static struct platform_driver omap_dma_driver = {
 
 bool omap_dma_filter_fn(struct dma_chan *chan, void *param)
 {
-	if (chan->device->dev->driver == &omap_dma_driver.driver) {
+	struct of_phandle_args *dma_spec = param;
+	if (chan->device->dev->driver == &omap_dma_driver.driver &&
+			dma_spec->np == chan->device->dev->of_node) {
 		struct omap_chan *c = to_omap_dma_chan(chan);
-		unsigned req = *(unsigned *)param;
+		unsigned req = dma_spec->args[0];
 
 		return req == c->dma_sig;
 	}
-- 
1.8.1.5

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Santosh Shilimkar]

On Thursday 01 August 2013 10:00 PM, Richard Zhao wrote:
> pass of_phandle_args dma_spec to dma_request_channel in of_dma_simple_xlate,
> so the filter function could access of_node in of_phandle_args.
>
Am just curious the reasoning behind doing so. Can you please expand
above bit more with why you need to change it.
 
> It also remove restriction of #dma-cells has to be one.
> 
> Signed-off-by: Richard Zhao <rizhao@nvidia.com>
> ---
>  drivers/dma/edma.c     |  7 +++++--
>  drivers/dma/of-dma.c   | 10 ++++------
>  drivers/dma/omap-dma.c |  6 ++++--
>  3 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 4f6d87b..54582fd 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -23,6 +23,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <linux/of_dma.h>
>  
>  #include <linux/platform_data/edma.h>
>  
> @@ -604,9 +605,11 @@ static struct platform_driver edma_driver = {
>  
>  bool edma_filter_fn(struct dma_chan *chan, void *param)
>  {
> -	if (chan->device->dev->driver == &edma_driver.driver) {
> +	struct of_phandle_args *dma_spec = param;
> +	if (chan->device->dev->driver == &edma_driver.driver &&
> +			dma_spec->np == chan->device->dev->of_node) {
>  		struct edma_chan *echan = to_edma_chan(chan);
> -		unsigned ch_req = *(unsigned *)param;
> +		unsigned ch_req = dma_spec->args[0];
>  		return ch_req == echan->ch_num;
>  	}
>  	return false;
> diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
> index 75334bd..e1c4d3b 100644
> --- a/drivers/dma/of-dma.c
> +++ b/drivers/dma/of-dma.c
> @@ -192,11 +192,9 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
>   * @dma_spec:	pointer to DMA specifier as found in the device tree
>   * @of_dma:	pointer to DMA controller data
>   *
> - * A simple translation function for devices that use a 32-bit value for the
> + * A simple translation function for devices that use dma_spec for the
>   * filter_param when calling the DMA engine dma_request_channel() function.
> - * Note that this translation function requires that #dma-cells is equal to 1
> - * and the argument of the dma specifier is the 32-bit filter_param. Returns
> - * pointer to appropriate dma channel on success or NULL on error.
> + * Returns pointer to appropriate dma channel on success or NULL on error.
>   */
>  struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
>  						struct of_dma *ofdma)
> @@ -207,10 +205,10 @@ struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
>  	if (!info || !info->filter_fn)
>  		return NULL;
>  
> -	if (count != 1)
> +	if (count < 1)
>  		return NULL;
>  
>  	return dma_request_channel(info->dma_cap, info->filter_fn,
> -			&dma_spec->args[0]);
> +			dma_spec);
>  }
>  EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
> index ec3fc4f..32547ec 100644
> --- a/drivers/dma/omap-dma.c
> +++ b/drivers/dma/omap-dma.c
> @@ -693,9 +693,11 @@ static struct platform_driver omap_dma_driver = {
>  
>  bool omap_dma_filter_fn(struct dma_chan *chan, void *param)
>  {
> -	if (chan->device->dev->driver == &omap_dma_driver.driver) {
> +	struct of_phandle_args *dma_spec = param;
> +	if (chan->device->dev->driver == &omap_dma_driver.driver &&
> +			dma_spec->np == chan->device->dev->of_node) {
>  		struct omap_chan *c = to_omap_dma_chan(chan);
> -		unsigned req = *(unsigned *)param;
> +		unsigned req = dma_spec->args[0];
>  
>  		return req == c->dma_sig;
>  	}
> 

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Stephen Warren]

On 08/02/2013 06:06 AM, Santosh Shilimkar wrote:
> On Thursday 01 August 2013 10:00 PM, Richard Zhao wrote:
>> pass of_phandle_args dma_spec to dma_request_channel in of_dma_simple_xlate,
>> so the filter function could access of_node in of_phandle_args.
>>
> Am just curious the reasoning behind doing so. Can you please expand
> above bit more with why you need to change it.

I believe that this patch is attempting to solve an issue I pointed out
with the following patch, which enhances the Tegra DMA controller driver
to support the standard DMA DT bindings:

https://lkml.org/lkml/2013/7/24/7
[PATCH 2/9] dma: tegra20-apbdma: move to generic device tree bindings

The issue is in particular that patch included:

> +static bool tegra_dma_filter_fn(struct dma_chan *dc, void *param)
> +{
> +	if (dc->device->dev->driver == &tegra_dmac_driver.driver) {
> +		struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
> +		unsigned req = *(unsigned *)param;
> +
> +		tdc->slave_id = req;
> +
> +		return true;
> +	}
> +	return false;
> +}

Which is checking that the provider of the DMA channel is the correct
DMA controller. The DMA core should be able to work this out, since at
least under DT, the DT property specifies both the DMA controller's
phandle and the DMA specifier, so that DMA core should be able to
validate that it only attempts to match DMA channels for the specified
DMA controller, without each DMA controller driver having to implement a
custom filter function to do that.

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Richard Zhao]

On Fri, Aug 02, 2013 at 10:00:00AM +0800, Richard Zhao wrote:
> pass of_phandle_args dma_spec to dma_request_channel in of_dma_simple_xlate,
> so the filter function could access of_node in of_phandle_args.
> 
> It also remove restriction of #dma-cells has to be one.
> 
> Signed-off-by: Richard Zhao <rizhao@nvidia.com>
> ---
>  drivers/dma/edma.c     |  7 +++++--
>  drivers/dma/of-dma.c   | 10 ++++------
>  drivers/dma/omap-dma.c |  6 ++++--
>  3 files changed, 13 insertions(+), 10 deletions(-)
> 

Hi Vinod,

Can you please pick up this change?

Hi Stephen,

Can you please give a ack or reviewed-by etc?

Thanks
Richard

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Stephen Warren]

On 08/21/2013 11:19 PM, Richard Zhao wrote:
> On Fri, Aug 02, 2013 at 10:00:00AM +0800, Richard Zhao wrote:
>> pass of_phandle_args dma_spec to dma_request_channel in of_dma_simple_xlate,
>> so the filter function could access of_node in of_phandle_args.
>>
>> It also remove restriction of #dma-cells has to be one.
>>
>> Signed-off-by: Richard Zhao <rizhao@nvidia.com>
>> ---
>>  drivers/dma/edma.c     |  7 +++++--
>>  drivers/dma/of-dma.c   | 10 ++++------
>>  drivers/dma/omap-dma.c |  6 ++++--
>>  3 files changed, 13 insertions(+), 10 deletions(-)
>>
> 
> Hi Vinod,
> 
> Can you please pick up this change?
> 
> Hi Stephen,
> 
> Can you please give a ack or reviewed-by etc?

Hmm. Looking at the patch, I'm not sure it's right.

This patch simply passes all the specfier args to the filter function,
and the code to check the equality of the of_node to the filter args is
still duplicated in each DMA driver. Instead, the DMA core should be
implementing the equality check, and only even calling the
driver-specific filter function for devices where the client's phandle
matches the DMA providing device's of_node handle.

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Richard Zhao]

On Fri, Aug 23, 2013 at 04:18:27AM +0800, Stephen Warren wrote:
> On 08/21/2013 11:19 PM, Richard Zhao wrote:
> > On Fri, Aug 02, 2013 at 10:00:00AM +0800, Richard Zhao wrote:
> >> pass of_phandle_args dma_spec to dma_request_channel in of_dma_simple_xlate,
> >> so the filter function could access of_node in of_phandle_args.
> >>
> >> It also remove restriction of #dma-cells has to be one.
> >>
> >> Signed-off-by: Richard Zhao <rizhao@nvidia.com>
> >> ---
> >>  drivers/dma/edma.c     |  7 +++++--
> >>  drivers/dma/of-dma.c   | 10 ++++------
> >>  drivers/dma/omap-dma.c |  6 ++++--
> >>  3 files changed, 13 insertions(+), 10 deletions(-)
> >>
> > 
> > Hi Vinod,
> > 
> > Can you please pick up this change?
> > 
> > Hi Stephen,
> > 
> > Can you please give a ack or reviewed-by etc?
> 
> Hmm. Looking at the patch, I'm not sure it's right.
> 
> This patch simply passes all the specfier args to the filter function,
> and the code to check the equality of the of_node to the filter args is
> still duplicated in each DMA driver. Instead, the DMA core should be
> implementing the equality check, and only even calling the
> driver-specific filter function for devices where the client's phandle
> matches the DMA providing device's of_node handle.

Filter function is called in dmaengine core code, independent of dt.
And the reason why the driver has to write its own filter function is
it has to store slave id there in its own way.

Thanks
Richard

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Stephen Warren]

On 08/22/2013 07:29 PM, Richard Zhao wrote:
> On Fri, Aug 23, 2013 at 04:18:27AM +0800, Stephen Warren wrote:
>> On 08/21/2013 11:19 PM, Richard Zhao wrote:
>>> On Fri, Aug 02, 2013 at 10:00:00AM +0800, Richard Zhao wrote:
>>>> pass of_phandle_args dma_spec to dma_request_channel in of_dma_simple_xlate,
>>>> so the filter function could access of_node in of_phandle_args.
>>>>
>>>> It also remove restriction of #dma-cells has to be one.
>>>>
>>>> Signed-off-by: Richard Zhao <rizhao@nvidia.com>
>>>> ---
>>>>  drivers/dma/edma.c     |  7 +++++--
>>>>  drivers/dma/of-dma.c   | 10 ++++------
>>>>  drivers/dma/omap-dma.c |  6 ++++--
>>>>  3 files changed, 13 insertions(+), 10 deletions(-)
>>>>
>>>
>>> Hi Vinod,
>>>
>>> Can you please pick up this change?
>>>
>>> Hi Stephen,
>>>
>>> Can you please give a ack or reviewed-by etc?
>>
>> Hmm. Looking at the patch, I'm not sure it's right.
>>
>> This patch simply passes all the specfier args to the filter function,
>> and the code to check the equality of the of_node to the filter args is
>> still duplicated in each DMA driver. Instead, the DMA core should be
>> implementing the equality check, and only even calling the
>> driver-specific filter function for devices where the client's phandle
>> matches the DMA providing device's of_node handle.
> 
> Filter function is called in dmaengine core code, independent of dt.

The core code can still check if a dmaengine's driver was instantiated
from DT and take additional actions in that case.

> And the reason why the driver has to write its own filter function is
> it has to store slave id there in its own way.

I'm not saying don't call the driver's filter function, but rather that
the dmaengine core should perform the common checks before doing so.

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Laurent Pinchart]

On Friday 23 August 2013 09:57:43 Stephen Warren wrote:
> On 08/22/2013 07:29 PM, Richard Zhao wrote:
> > On Fri, Aug 23, 2013 at 04:18:27AM +0800, Stephen Warren wrote:
> >> On 08/21/2013 11:19 PM, Richard Zhao wrote:
> >>> On Fri, Aug 02, 2013 at 10:00:00AM +0800, Richard Zhao wrote:
> >>>> pass of_phandle_args dma_spec to dma_request_channel in
> >>>> of_dma_simple_xlate, so the filter function could access of_node in
> >>>> of_phandle_args.
> >>>> 
> >>>> It also remove restriction of #dma-cells has to be one.
> >>>> 
> >>>> Signed-off-by: Richard Zhao <rizhao@nvidia.com>
> >>>> ---
> >>>> 
> >>>>  drivers/dma/edma.c     |  7 +++++--
> >>>>  drivers/dma/of-dma.c   | 10 ++++------
> >>>>  drivers/dma/omap-dma.c |  6 ++++--
> >>>>  3 files changed, 13 insertions(+), 10 deletions(-)
> >>> 
> >>> Hi Vinod,
> >>> 
> >>> Can you please pick up this change?
> >>> 
> >>> Hi Stephen,
> >>> 
> >>> Can you please give a ack or reviewed-by etc?
> >> 
> >> Hmm. Looking at the patch, I'm not sure it's right.
> >> 
> >> This patch simply passes all the specfier args to the filter function,
> >> and the code to check the equality of the of_node to the filter args is
> >> still duplicated in each DMA driver. Instead, the DMA core should be
> >> implementing the equality check, and only even calling the
> >> driver-specific filter function for devices where the client's phandle
> >> matches the DMA providing device's of_node handle.
> > 
> > Filter function is called in dmaengine core code, independent of dt.
> 
> The core code can still check if a dmaengine's driver was instantiated
> from DT and take additional actions in that case.
> 
> > And the reason why the driver has to write its own filter function is
> > it has to store slave id there in its own way.
> 
> I'm not saying don't call the driver's filter function, but rather that
> the dmaengine core should perform the common checks before doing so.

And it looks to me like the common case could even get rid of the driver's 
filter function:

https://lkml.org/lkml/2013/5/15/270
https://lkml.org/lkml/2013/3/25/250

-- 
Regards,

Laurent Pinchart

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Richard Zhao]

On Mon, Aug 26, 2013 at 02:17:43PM +0200, Laurent Pinchart wrote:
> On Friday 23 August 2013 09:57:43 Stephen Warren wrote:
> > On 08/22/2013 07:29 PM, Richard Zhao wrote:
> > > On Fri, Aug 23, 2013 at 04:18:27AM +0800, Stephen Warren wrote:
> > >> On 08/21/2013 11:19 PM, Richard Zhao wrote:
> > >>> On Fri, Aug 02, 2013 at 10:00:00AM +0800, Richard Zhao wrote:
> > >>>> pass of_phandle_args dma_spec to dma_request_channel in
> > >>>> of_dma_simple_xlate, so the filter function could access of_node in
> > >>>> of_phandle_args.
> > >>>> 
> > >>>> It also remove restriction of #dma-cells has to be one.
> > >>>> 
> > >>>> Signed-off-by: Richard Zhao <rizhao@nvidia.com>
> > >>>> ---
> > >>>> 
> > >>>>  drivers/dma/edma.c     |  7 +++++--
> > >>>>  drivers/dma/of-dma.c   | 10 ++++------
> > >>>>  drivers/dma/omap-dma.c |  6 ++++--
> > >>>>  3 files changed, 13 insertions(+), 10 deletions(-)
> > >>> 
> > >>> Hi Vinod,
> > >>> 
> > >>> Can you please pick up this change?
> > >>> 
> > >>> Hi Stephen,
> > >>> 
> > >>> Can you please give a ack or reviewed-by etc?
> > >> 
> > >> Hmm. Looking at the patch, I'm not sure it's right.
> > >> 
> > >> This patch simply passes all the specfier args to the filter function,
> > >> and the code to check the equality of the of_node to the filter args is
> > >> still duplicated in each DMA driver. Instead, the DMA core should be
> > >> implementing the equality check, and only even calling the
> > >> driver-specific filter function for devices where the client's phandle
> > >> matches the DMA providing device's of_node handle.
> > > 
> > > Filter function is called in dmaengine core code, independent of dt.
> > 
> > The core code can still check if a dmaengine's driver was instantiated
> > from DT and take additional actions in that case.
> > 
> > > And the reason why the driver has to write its own filter function is
> > > it has to store slave id there in its own way.
> > 
> > I'm not saying don't call the driver's filter function, but rather that
> > the dmaengine core should perform the common checks before doing so.
> 
> And it looks to me like the common case could even get rid of the driver's 
> filter function:
> 
> https://lkml.org/lkml/2013/5/15/270
> https://lkml.org/lkml/2013/3/25/250
For general case, the slave id is not staticly bind to a specific channel.

Thanks
Richard

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Laurent Pinchart]

Hi Richard,

(Dropping Dan Williams from the CC list as his e-mail address doesn't seem to 
be valid anymore)

On Monday 26 August 2013 20:55:57 Richard Zhao wrote:
> On Mon, Aug 26, 2013 at 02:17:43PM +0200, Laurent Pinchart wrote:
> > On Friday 23 August 2013 09:57:43 Stephen Warren wrote:
> > > On 08/22/2013 07:29 PM, Richard Zhao wrote:
> > > > On Fri, Aug 23, 2013 at 04:18:27AM +0800, Stephen Warren wrote:
> > > >> On 08/21/2013 11:19 PM, Richard Zhao wrote:
> > > >>> On Fri, Aug 02, 2013 at 10:00:00AM +0800, Richard Zhao wrote:
> > > >>>> pass of_phandle_args dma_spec to dma_request_channel in
> > > >>>> of_dma_simple_xlate, so the filter function could access of_node in
> > > >>>> of_phandle_args.
> > > >>>> 
> > > >>>> It also remove restriction of #dma-cells has to be one.
> > > >>>> 
> > > >>>> Signed-off-by: Richard Zhao <rizhao@nvidia.com>
> > > >>>> ---
> > > >>>> 
> > > >>>>  drivers/dma/edma.c     |  7 +++++--
> > > >>>>  drivers/dma/of-dma.c   | 10 ++++------
> > > >>>>  drivers/dma/omap-dma.c |  6 ++++--
> > > >>>>  3 files changed, 13 insertions(+), 10 deletions(-)
> > > >>> 
> > > >>> Hi Vinod,
> > > >>> 
> > > >>> Can you please pick up this change?
> > > >>> 
> > > >>> Hi Stephen,
> > > >>> 
> > > >>> Can you please give a ack or reviewed-by etc?
> > > >> 
> > > >> Hmm. Looking at the patch, I'm not sure it's right.
> > > >> 
> > > >> This patch simply passes all the specfier args to the filter
> > > >> function, and the code to check the equality of the of_node to the
> > > >> filter args is still duplicated in each DMA driver. Instead, the DMA
> > > >> core should be implementing the equality check, and only even calling
> > > >> the driver-specific filter function for devices where the client's
> > > >> phandle matches the DMA providing device's of_node handle.
> > > > 
> > > > Filter function is called in dmaengine core code, independent of dt.
> > > 
> > > The core code can still check if a dmaengine's driver was instantiated
> > > from DT and take additional actions in that case.
> > > 
> > > > And the reason why the driver has to write its own filter function is
> > > > it has to store slave id there in its own way.
> > > 
> > > I'm not saying don't call the driver's filter function, but rather that
> > > the dmaengine core should perform the common checks before doing so.
> > 
> > And it looks to me like the common case could even get rid of the driver's
> > filter function:
> > 
> > https://lkml.org/lkml/2013/5/15/270
> > https://lkml.org/lkml/2013/3/25/250
> 
> For general case, the slave id is not staticly bind to a specific channel.

Certainly not in all cases, but I think it's common enough to deserve a 
specific helper function.

-- 
Regards,

Laurent Pinchart

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Vinod Koul]

On Mon, Aug 26, 2013 at 03:18:00PM +0200, Laurent Pinchart wrote:
> Hi Richard,
> 
> (Dropping Dan Williams from the CC list as his e-mail address doesn't seem to 
> be valid anymore)
> 
> > > > >> Hmm. Looking at the patch, I'm not sure it's right.
> > > > >> 
> > > > >> This patch simply passes all the specfier args to the filter
> > > > >> function, and the code to check the equality of the of_node to the
> > > > >> filter args is still duplicated in each DMA driver. Instead, the DMA
> > > > >> core should be implementing the equality check, and only even calling
> > > > >> the driver-specific filter function for devices where the client's
> > > > >> phandle matches the DMA providing device's of_node handle.
> > > > > 
> > > > > Filter function is called in dmaengine core code, independent of dt.
> > > > 
> > > > The core code can still check if a dmaengine's driver was instantiated
> > > > from DT and take additional actions in that case.
> > > > 
> > > > > And the reason why the driver has to write its own filter function is
> > > > > it has to store slave id there in its own way.
> > > > 
> > > > I'm not saying don't call the driver's filter function, but rather that
> > > > the dmaengine core should perform the common checks before doing so.
> > > 
> > > And it looks to me like the common case could even get rid of the driver's
> > > filter function:
> > > 
> > > https://lkml.org/lkml/2013/5/15/270
> > > https://lkml.org/lkml/2013/3/25/250
> > 
> > For general case, the slave id is not staticly bind to a specific channel.
> 
> Certainly not in all cases, but I think it's common enough to deserve a 
> specific helper function.
Why does DT need the fliter function in the first place. The DT enabled drivers
should not even have a filter function...

The dmaengine core calls the optional filter function. This needs to be
implemented in driver in order for driver to check if the channel is what it
needs or not.

And the selection should be done for the cases where you dont have programmable
mux in dmac. For programmable ones passing slave_id in dma_slave_config should
be fine.

~Vinod

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Arnd Bergmann]

On Monday 26 August 2013 19:40:57 Vinod Koul wrote:
> Why does DT need the fliter function in the first place. The DT enabled drivers
> should not even have a filter function...
> 
> The dmaengine core calls the optional filter function. This needs to be
> implemented in driver in order for driver to check if the channel is what it
> needs or not.

You only just merged the dma_get_slave_channel() patch, which allows having
no filter function. Up to Linux-3.11, the filter was always needed but
could be 'static' and only called by the xlate function. Now the xlate
function can pick a channel itself.

> And the selection should be done for the cases where you dont have programmable
> mux in dmac. For programmable ones passing slave_id in dma_slave_config should
> be fine.

I think passing a slave_id from the slave driver is never correct with DT,
since the ID is a property of the system rather than the slave device, so
the driver has no access to it. Drivers have to always take the settings
from DT and ignore what dma_slave_config() sets later.

	Arnd

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Vinod Koul]

On Mon, Aug 26, 2013 at 08:15:17PM +0200, Arnd Bergmann wrote:
> On Monday 26 August 2013 19:40:57 Vinod Koul wrote:
> > And the selection should be done for the cases where you dont have programmable
> > mux in dmac. For programmable ones passing slave_id in dma_slave_config should
> > be fine.
> 
> I think passing a slave_id from the slave driver is never correct with DT,
> since the ID is a property of the system rather than the slave device, so
> the driver has no access to it. Drivers have to always take the settings
> from DT and ignore what dma_slave_config() sets later.
Shouldnt slave id be property of the client driver. That way we can pass it to
the driver using dma_slave_config(). That should make it right..

~Vinod

-- 

[PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node

[Richard Zhao]

On Fri, Aug 23, 2013 at 09:57:43AM -0600, Stephen Warren wrote:
> On 08/22/2013 07:29 PM, Richard Zhao wrote:
> > On Fri, Aug 23, 2013 at 04:18:27AM +0800, Stephen Warren wrote:
> >> On 08/21/2013 11:19 PM, Richard Zhao wrote:
> >>> On Fri, Aug 02, 2013 at 10:00:00AM +0800, Richard Zhao wrote:
> >>>> pass of_phandle_args dma_spec to dma_request_channel in of_dma_simple_xlate,
> >>>> so the filter function could access of_node in of_phandle_args.
> >>>>
> >>>> It also remove restriction of #dma-cells has to be one.
> >>>>
> >>>> Signed-off-by: Richard Zhao <rizhao@nvidia.com>
> >>>> ---
> >>>>  drivers/dma/edma.c     |  7 +++++--
> >>>>  drivers/dma/of-dma.c   | 10 ++++------
> >>>>  drivers/dma/omap-dma.c |  6 ++++--
> >>>>  3 files changed, 13 insertions(+), 10 deletions(-)
> >>>>
> >>>
> >>> Hi Vinod,
> >>>
> >>> Can you please pick up this change?
> >>>
> >>> Hi Stephen,
> >>>
> >>> Can you please give a ack or reviewed-by etc?
> >>
> >> Hmm. Looking at the patch, I'm not sure it's right.
> >>
> >> This patch simply passes all the specfier args to the filter function,
> >> and the code to check the equality of the of_node to the filter args is
> >> still duplicated in each DMA driver. Instead, the DMA core should be
> >> implementing the equality check, and only even calling the
> >> driver-specific filter function for devices where the client's phandle
> >> matches the DMA providing device's of_node handle.
> > 
> > Filter function is called in dmaengine core code, independent of dt.
> 
> The core code can still check if a dmaengine's driver was instantiated
> from DT and take additional actions in that case.

>From perspective of core code, the filter function is not attached to
any dma driver. So it doesn't care whether dt dma driver is instantiated.
I'm not sure I've got your point. Can you please write pieces of code if
possible?
The goal of this patch is to use least changes to add check of dt node.

Thanks
Richard
> 
> > And the reason why the driver has to write its own filter function is
> > it has to store slave id there in its own way.
> 
> I'm not saying don't call the driver's filter function, but rather that
> the dmaengine core should perform the common checks before doing so.
>