Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On Mon, 2012-10-15 at 08:59 +0530, Viresh Kumar wrote:
> On 12 October 2012 20:28, Andy Shevchenko
> wrote:
> >> + if (last_dw) {
> >> + if ((last_bus_id == param) && (last_dw == dw))
> >> + return false;
> >> + }
> > Just came to my mind.
> > dw can't be NULL, can't it?
> > Then
> > if (last_dw) {
> > ...
> > }
> > is unneeded.
>
> Fixup for this:
>
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index d72c26f..764c159 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -1196,11 +1196,8 @@ bool dw_dma_generic_filter(struct dma_chan
> *chan, void *param)
> * failure. If dw and param are same, i.e. trying on same dw with
> * different channel, return false.
> */
> - if (last_dw) {
> - if ((last_bus_id == param) && (last_dw == dw))
> - return false;
> - }
> -
> + if ((last_dw == dw) && (last_bus_id == param))
> + return false;
> /*
> * Return true:
> * - If dw_dma's platform data is not filled with slave info, then all
Good.
So, have my
Reviewed-by: Andy Shevchenko
for this patch.
--
Andy Shevchenko
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On Mon, 2012-10-15 at 08:59 +0530, Viresh Kumar wrote:
On 12 October 2012 20:28, Andy Shevchenko
andriy.shevche...@linux.intel.com wrote:
+ if (last_dw) {
+ if ((last_bus_id == param) (last_dw == dw))
+ return false;
+ }
Just came to my mind.
dw can't be NULL, can't it?
Then
if (last_dw) {
...
}
is unneeded.
Fixup for this:
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index d72c26f..764c159 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1196,11 +1196,8 @@ bool dw_dma_generic_filter(struct dma_chan
*chan, void *param)
* failure. If dw and param are same, i.e. trying on same dw with
* different channel, return false.
*/
- if (last_dw) {
- if ((last_bus_id == param) (last_dw == dw))
- return false;
- }
-
+ if ((last_dw == dw) (last_bus_id == param))
+ return false;
/*
* Return true:
* - If dw_dma's platform data is not filled with slave info, then all
Good.
So, have my
Reviewed-by: Andy Shevchenko andriy.shevche...@linux.intel.com
for this patch.
--
Andy Shevchenko andriy.shevche...@linux.intel.com
Intel Finland Oy
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 12 October 2012 20:28, Andy Shevchenko
wrote:
>> + if (last_dw) {
>> + if ((last_bus_id == param) && (last_dw == dw))
>> + return false;
>> + }
> Just came to my mind.
> dw can't be NULL, can't it?
> Then
> if (last_dw) {
> ...
> }
> is unneeded.
Fixup for this:
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index d72c26f..764c159 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1196,11 +1196,8 @@ bool dw_dma_generic_filter(struct dma_chan
*chan, void *param)
* failure. If dw and param are same, i.e. trying on same dw with
* different channel, return false.
*/
- if (last_dw) {
- if ((last_bus_id == param) && (last_dw == dw))
- return false;
- }
-
+ if ((last_dw == dw) && (last_bus_id == param))
+ return false;
/*
* Return true:
* - If dw_dma's platform data is not filled with slave info, then all
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 12 October 2012 20:28, Andy Shevchenko
andriy.shevche...@linux.intel.com wrote:
+ if (last_dw) {
+ if ((last_bus_id == param) (last_dw == dw))
+ return false;
+ }
Just came to my mind.
dw can't be NULL, can't it?
Then
if (last_dw) {
...
}
is unneeded.
Fixup for this:
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index d72c26f..764c159 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1196,11 +1196,8 @@ bool dw_dma_generic_filter(struct dma_chan
*chan, void *param)
* failure. If dw and param are same, i.e. trying on same dw with
* different channel, return false.
*/
- if (last_dw) {
- if ((last_bus_id == param) (last_dw == dw))
- return false;
- }
-
+ if ((last_dw == dw) (last_bus_id == param))
+ return false;
/*
* Return true:
* - If dw_dma's platform data is not filled with slave info, then all
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 12 October 2012 21:50, Jean-Christophe PLAGNIOL-VILLARD wrote: >> >> - pass platform data via DT, non-DT way still takes precedence if both >> >> are used. >> > why keep it all platform are DT >> >> I would love to remove that, but not sure if somebody want's the non-DT >> way too. >> >> I didn't wanted to get into fixing user SoCs of this driver. > no drop it at the mailine if do not need it But what's the harm in keeping it until we are sure, all users of it have moved into DT? It is not adding any overhead for DT case. And this is what most of the drivers today are doing, because not every platform is DT compatible. This driver is used in multiple architectures, not only ARM. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 20:25 Fri 12 Oct , Viresh Kumar wrote: > On 12 October 2012 20:20, Jean-Christophe PLAGNIOL-VILLARD > wrote: > > On 20:01 Fri 12 Oct , Viresh Kumar wrote: > >> dw_dmac driver already supports device tree but it used to have its > >> platform > >> data passed the non-DT way. > >> > >> This patch does following changes: > >> - pass platform data via DT, non-DT way still takes precedence if both are > >> used. > > why keep it all platform are DT > > I would love to remove that, but not sure if somebody want's the non-DT > way too. > > I didn't wanted to get into fixing user SoCs of this driver. no drop it at the mailine if do not need it Best Regads, J. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 12 October 2012 21:23, Andy Shevchenko wrote: > 1. This is an equivalent of > if (last_dw && (last_bus_id == ... ) && (last_dw == dw)) > return false; > 2. In case dw is always non-NULL the last_dw == dw is false if last_dw is > NULL. > > Where am I wrong? Nowhere, I am drunk ;) -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On Fri, Oct 12, 2012 at 6:18 PM, Viresh Kumar wrote:
> On 12 October 2012 20:28, Andy Shevchenko
> wrote:
>> On Fri, 2012-10-12 at 20:01 +0530, Viresh Kumar wrote:
>
>>> + if (last_dw) {
>>> + if ((last_bus_id == param) && (last_dw == dw))
>>> + return false;
>>> + }
1. This is an equivalent of
if (last_dw && (last_bus_id == ... ) && (last_dw == dw))
return false;
2. In case dw is always non-NULL the last_dw == dw is false if last_dw is NULL.
Where am I wrong?
> You are already drunk.
Not yet, but tired.
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 12 October 2012 20:28, Andy Shevchenko
wrote:
> On Fri, 2012-10-12 at 20:01 +0530, Viresh Kumar wrote:
>> + if (last_dw) {
>> + if ((last_bus_id == param) && (last_dw == dw))
>> + return false;
>> + }
> Just came to my mind.
> dw can't be NULL, can't it?
> Then
> if (last_dw) {
> ...
> }
> is unneeded.
dw can't be but last_dw can be, which we are making NULL when
we find a channel :)
You are already drunk.
--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On Fri, 2012-10-12 at 20:01 +0530, Viresh Kumar wrote:
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> @@ -1179,6 +1179,53 @@ static void dwc_free_chan_resources(struct dma_chan
> *chan)
> dev_vdbg(chan2dev(chan), "%s: done\n", __func__);
> }
>
> +bool dw_dma_generic_filter(struct dma_chan *chan, void *param)
> +{
> + struct dw_dma *dw = to_dw_dma(chan->device);
> + static struct dw_dma *last_dw;
> + static char *last_bus_id;
> + int i = -1;
> +
> + /*
> + * dmaengine framework calls this routine for all channels of all dma
> + * controller, until true is returned. If 'param' bus_id is not
> + * registered with a dma controller (dw), then there is no need of
> + * running below function for all channels of dw.
> + *
> + * This block of code does this by saving the parameters of last
> + * failure. If dw and param are same, i.e. trying on same dw with
> + * different channel, return false.
> + */
> + if (last_dw) {
> + if ((last_bus_id == param) && (last_dw == dw))
> + return false;
> + }
Just came to my mind.
dw can't be NULL, can't it?
Then
if (last_dw) {
...
}
is unneeded.
Please, check twice my thought because it's a Friday evening.
> @@ -1462,6 +1509,91 @@ static void dw_dma_off(struct dw_dma *dw)
> dw->chan[i].initialized = false;
> }
>
> +#ifdef CONFIG_OF
> +static struct dw_dma_platform_data *
> +__devinit dw_dma_parse_dt(struct platform_device *pdev)
> +{
> + struct device_node *sn, *cn, *np = pdev->dev.of_node;
> + struct dw_dma_platform_data *pdata;
> + struct dw_dma_slave *sd;
> + u32 val, arr[4];
Let me weekend to think about naming. I really can't offer anything else
right now.
--
Andy Shevchenko
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 12 October 2012 20:20, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 20:01 Fri 12 Oct , Viresh Kumar wrote: >> dw_dmac driver already supports device tree but it used to have its platform >> data passed the non-DT way. >> >> This patch does following changes: >> - pass platform data via DT, non-DT way still takes precedence if both are >> used. > why keep it all platform are DT I would love to remove that, but not sure if somebody want's the non-DT way too. I didn't wanted to get into fixing user SoCs of this driver. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 20:01 Fri 12 Oct , Viresh Kumar wrote: > dw_dmac driver already supports device tree but it used to have its platform > data passed the non-DT way. > > This patch does following changes: > - pass platform data via DT, non-DT way still takes precedence if both are > used. why keep it all platform are DT Best Regards, J. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 20:01 Fri 12 Oct , Viresh Kumar wrote: dw_dmac driver already supports device tree but it used to have its platform data passed the non-DT way. This patch does following changes: - pass platform data via DT, non-DT way still takes precedence if both are used. why keep it all platform are DT Best Regards, J. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 12 October 2012 20:20, Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com wrote: On 20:01 Fri 12 Oct , Viresh Kumar wrote: dw_dmac driver already supports device tree but it used to have its platform data passed the non-DT way. This patch does following changes: - pass platform data via DT, non-DT way still takes precedence if both are used. why keep it all platform are DT I would love to remove that, but not sure if somebody want's the non-DT way too. I didn't wanted to get into fixing user SoCs of this driver. -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On Fri, 2012-10-12 at 20:01 +0530, Viresh Kumar wrote:
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
@@ -1179,6 +1179,53 @@ static void dwc_free_chan_resources(struct dma_chan
*chan)
dev_vdbg(chan2dev(chan), %s: done\n, __func__);
}
+bool dw_dma_generic_filter(struct dma_chan *chan, void *param)
+{
+ struct dw_dma *dw = to_dw_dma(chan-device);
+ static struct dw_dma *last_dw;
+ static char *last_bus_id;
+ int i = -1;
+
+ /*
+ * dmaengine framework calls this routine for all channels of all dma
+ * controller, until true is returned. If 'param' bus_id is not
+ * registered with a dma controller (dw), then there is no need of
+ * running below function for all channels of dw.
+ *
+ * This block of code does this by saving the parameters of last
+ * failure. If dw and param are same, i.e. trying on same dw with
+ * different channel, return false.
+ */
+ if (last_dw) {
+ if ((last_bus_id == param) (last_dw == dw))
+ return false;
+ }
Just came to my mind.
dw can't be NULL, can't it?
Then
if (last_dw) {
...
}
is unneeded.
Please, check twice my thought because it's a Friday evening.
@@ -1462,6 +1509,91 @@ static void dw_dma_off(struct dw_dma *dw)
dw-chan[i].initialized = false;
}
+#ifdef CONFIG_OF
+static struct dw_dma_platform_data *
+__devinit dw_dma_parse_dt(struct platform_device *pdev)
+{
+ struct device_node *sn, *cn, *np = pdev-dev.of_node;
+ struct dw_dma_platform_data *pdata;
+ struct dw_dma_slave *sd;
+ u32 val, arr[4];
Let me weekend to think about naming. I really can't offer anything else
right now.
--
Andy Shevchenko andriy.shevche...@linux.intel.com
Intel Finland Oy
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 12 October 2012 20:28, Andy Shevchenko
andriy.shevche...@linux.intel.com wrote:
On Fri, 2012-10-12 at 20:01 +0530, Viresh Kumar wrote:
+ if (last_dw) {
+ if ((last_bus_id == param) (last_dw == dw))
+ return false;
+ }
Just came to my mind.
dw can't be NULL, can't it?
Then
if (last_dw) {
...
}
is unneeded.
dw can't be but last_dw can be, which we are making NULL when
we find a channel :)
You are already drunk.
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On Fri, Oct 12, 2012 at 6:18 PM, Viresh Kumar viresh.ku...@linaro.org wrote:
On 12 October 2012 20:28, Andy Shevchenko
andriy.shevche...@linux.intel.com wrote:
On Fri, 2012-10-12 at 20:01 +0530, Viresh Kumar wrote:
+ if (last_dw) {
+ if ((last_bus_id == param) (last_dw == dw))
+ return false;
+ }
1. This is an equivalent of
if (last_dw (last_bus_id == ... ) (last_dw == dw))
return false;
2. In case dw is always non-NULL the last_dw == dw is false if last_dw is NULL.
Where am I wrong?
You are already drunk.
Not yet, but tired.
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 12 October 2012 21:23, Andy Shevchenko andy.shevche...@gmail.com wrote: 1. This is an equivalent of if (last_dw (last_bus_id == ... ) (last_dw == dw)) return false; 2. In case dw is always non-NULL the last_dw == dw is false if last_dw is NULL. Where am I wrong? Nowhere, I am drunk ;) -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 20:25 Fri 12 Oct , Viresh Kumar wrote: On 12 October 2012 20:20, Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com wrote: On 20:01 Fri 12 Oct , Viresh Kumar wrote: dw_dmac driver already supports device tree but it used to have its platform data passed the non-DT way. This patch does following changes: - pass platform data via DT, non-DT way still takes precedence if both are used. why keep it all platform are DT I would love to remove that, but not sure if somebody want's the non-DT way too. I didn't wanted to get into fixing user SoCs of this driver. no drop it at the mailine if do not need it Best Regads, J. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 2/3] dmaengine: dw_dmac: Enhance device tree support
On 12 October 2012 21:50, Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com wrote: - pass platform data via DT, non-DT way still takes precedence if both are used. why keep it all platform are DT I would love to remove that, but not sure if somebody want's the non-DT way too. I didn't wanted to get into fixing user SoCs of this driver. no drop it at the mailine if do not need it But what's the harm in keeping it until we are sure, all users of it have moved into DT? It is not adding any overhead for DT case. And this is what most of the drivers today are doing, because not every platform is DT compatible. This driver is used in multiple architectures, not only ARM. -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
