All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: linux-arm-kernel@lists.infradead.org
Cc: Viresh Kumar <viresh.kumar@linaro.org>,
	vinod.koul@intel.com, devicetree-discuss@lists.ozlabs.org,
	spear-devel@list.st.com, linux-kernel@vger.kernel.org,
	andriy.shevchenko@linux.intel.com
Subject: Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support
Date: Mon, 10 Dec 2012 22:08:51 +0000	[thread overview]
Message-ID: <201212102208.52347.arnd@arndb.de> (raw)
In-Reply-To: <169edae67b9a051ca7448e5b62c3cd2c5c1db470.1350360935.git.viresh.kumar@linaro.org>

On Tuesday 16 October 2012, 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.
> - create generic filter routine
> - Earlier slave information was made available by slave specific filter routines
>   in chan->private field. Now, this information would be passed from within dmac
>   DT node. Slave drivers would now be required to pass bus_id (a string) as
>   parameter to this generic filter(), which would be compared against the slave
>   data passed from DT, by the generic filter routine.
> - Update binding document
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

I'm deeply sorry for the very late complaint on this, but I only now
noticed this patch as I was seeing build breakage in linux-next
because of it.

The build bug is not the problem however, but the abuse of the
API is. Andy, are you sure you understood what this does when
you gave you Reviewed-by comment?

Viresh, there are multiple problems with your approach unfortunately:

* It does not follow the binding from
  Documentation/devicetree/bindings/dma/dma.txt

* It requires slave drivers to know that they are using the dw_dmac
  driver and pass a pointer to dw_generic_filter, which is not
  generic at all

* It requires the dmac node to have information about all slaves

There are also some minor issues, such as the naming of DT
properties, but the above need to be resolved first.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support
Date: Mon, 10 Dec 2012 22:08:51 +0000	[thread overview]
Message-ID: <201212102208.52347.arnd@arndb.de> (raw)
In-Reply-To: <169edae67b9a051ca7448e5b62c3cd2c5c1db470.1350360935.git.viresh.kumar@linaro.org>

On Tuesday 16 October 2012, 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.
> - create generic filter routine
> - Earlier slave information was made available by slave specific filter routines
>   in chan->private field. Now, this information would be passed from within dmac
>   DT node. Slave drivers would now be required to pass bus_id (a string) as
>   parameter to this generic filter(), which would be compared against the slave
>   data passed from DT, by the generic filter routine.
> - Update binding document
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

I'm deeply sorry for the very late complaint on this, but I only now
noticed this patch as I was seeing build breakage in linux-next
because of it.

The build bug is not the problem however, but the abuse of the
API is. Andy, are you sure you understood what this does when
you gave you Reviewed-by comment?

Viresh, there are multiple problems with your approach unfortunately:

* It does not follow the binding from
  Documentation/devicetree/bindings/dma/dma.txt

* It requires slave drivers to know that they are using the dw_dmac
  driver and pass a pointer to dw_generic_filter, which is not
  generic at all

* It requires the dmac node to have information about all slaves

There are also some minor issues, such as the naming of DT
properties, but the above need to be resolved first.

	Arnd

  parent reply	other threads:[~2012-12-10 22:09 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-16  4:19 [PATCH V3 1/3] dmaengine: dw_dmac: Update documentation style comments for dw_dma_platform_data Viresh Kumar
2012-10-16  4:19 ` Viresh Kumar
2012-10-16  4:19 ` [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support Viresh Kumar
2012-10-16  4:19   ` Viresh Kumar
2012-10-26  8:55   ` Viresh Kumar
2012-10-26  8:55     ` Viresh Kumar
2012-10-26  8:48     ` Vinod Koul
2012-10-26  8:48       ` Vinod Koul
2012-10-26  9:05       ` Viresh Kumar
2012-10-26  9:05         ` Viresh Kumar
2012-10-26  9:10         ` Vinod Koul
2012-10-26  9:10           ` Vinod Koul
2012-12-10 22:08   ` Arnd Bergmann [this message]
2012-12-10 22:08     ` Arnd Bergmann
2012-12-10 22:21     ` Arnd Bergmann
2012-12-10 22:21       ` Arnd Bergmann
2012-12-12  3:00     ` Viresh Kumar
2012-12-12  3:00       ` Viresh Kumar
2012-12-12  8:40       ` Andy Shevchenko
2012-12-12  8:40         ` Andy Shevchenko
2012-12-12  8:43         ` Viresh Kumar
2012-12-12  8:43           ` Viresh Kumar
2012-12-12  8:43           ` Viresh Kumar
2013-01-08 23:09       ` Arnd Bergmann
2013-01-08 23:09         ` Arnd Bergmann
2012-12-12  8:42     ` Andy Shevchenko
2012-12-12  8:42       ` Andy Shevchenko
2012-10-16  4:19 ` [PATCH V3 3/3] ARM: SPEAr13xx: Pass DW DMAC platform data from DT Viresh Kumar
2012-10-16  4:19   ` Viresh Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201212102208.52347.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=spear-devel@list.st.com \
    --cc=vinod.koul@intel.com \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.