All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
To: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Dan Williams
	<dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org,
	dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org
Subject: Re: [PATCH V2] dma: tegra: register as an OF DMA controller
Date: Fri, 29 Nov 2013 22:08:25 +0100	[thread overview]
Message-ID: <201311292208.26215.arnd@arndb.de> (raw)
In-Reply-To: <5293CF9F.6040305-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

On Monday 25 November 2013, Stephen Warren wrote:
> Well, you suggested not using of_dma_simple_xlate() since it wasn't
> appropriate. I then started to implement an open-coded xlate function,
> but found that it was 99% identical to the same thing in the mmp driver,
> and hence created a common of_dma_slave_xlate() so as not to just
> cut/paste it everywhere. Unfortunately, I only sent that patch to
> dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org and the DMA maintainers, and there's no
> archive of that list:-(

Ok, I see. However, I think the need for nontrivial code to be duplicated
across drivers is not a sign that we are missing a generic xlate function
and another indirection level, but rather that we got the interface
for dma_get_slave_channel() wrong (yes, that would be my fault).

Can you try coming up with a different method to achieve the same
where you use a different helper from the driver specific xlate
function that does not require a callback?

I think dma_get_slave_channel is great if you have one channel per
request line and you can directly look up the channel from the
DT data, but it is not good if you have pick a channel and work
around the race.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2] dma: tegra: register as an OF DMA controller
Date: Fri, 29 Nov 2013 22:08:25 +0100	[thread overview]
Message-ID: <201311292208.26215.arnd@arndb.de> (raw)
In-Reply-To: <5293CF9F.6040305@wwwdotorg.org>

On Monday 25 November 2013, Stephen Warren wrote:
> Well, you suggested not using of_dma_simple_xlate() since it wasn't
> appropriate. I then started to implement an open-coded xlate function,
> but found that it was 99% identical to the same thing in the mmp driver,
> and hence created a common of_dma_slave_xlate() so as not to just
> cut/paste it everywhere. Unfortunately, I only sent that patch to
> dmaengine at vger.kernel.org and the DMA maintainers, and there's no
> archive of that list:-(

Ok, I see. However, I think the need for nontrivial code to be duplicated
across drivers is not a sign that we are missing a generic xlate function
and another indirection level, but rather that we got the interface
for dma_get_slave_channel() wrong (yes, that would be my fault).

Can you try coming up with a different method to achieve the same
where you use a different helper from the driver specific xlate
function that does not require a callback?

I think dma_get_slave_channel is great if you have one channel per
request line and you can directly look up the channel from the
DT data, but it is not good if you have pick a channel and work
around the race.

	Arnd

  parent reply	other threads:[~2013-11-29 21:08 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-25 21:53 [PATCH V2] dma: tegra: register as an OF DMA controller Stephen Warren
2013-11-25 21:53 ` Stephen Warren
     [not found] ` <1385416416-3536-1-git-send-email-swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-11-25 22:09   ` Arnd Bergmann
2013-11-25 22:09     ` Arnd Bergmann
2013-11-25 22:30     ` Stephen Warren
2013-11-25 22:30       ` Stephen Warren
     [not found]       ` <5293CF9F.6040305-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-11-25 23:09         ` Dan Williams
2013-11-25 23:09           ` Dan Williams
     [not found]           ` <CAPcyv4jLh6cyaP2DmnOkc11xeKsB0QtbXQ9VV3BBmHVVt2iHXA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-11-25 23:14             ` Stephen Warren
2013-11-25 23:14               ` Stephen Warren
     [not found]               ` <5293D9EB.9070900-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-11-26  1:13                 ` Dan Williams
2013-11-26  1:13                   ` Dan Williams
2013-11-29 21:08         ` Arnd Bergmann [this message]
2013-11-29 21:08           ` Arnd Bergmann
     [not found]           ` <201311292208.26215.arnd-r2nGTMty4D4@public.gmane.org>
2013-12-03 17:52             ` Stephen Warren
2013-12-03 17:52               ` Stephen Warren
2013-12-04  1:22               ` Arnd Bergmann
2013-12-04  1:22                 ` Arnd Bergmann
     [not found]                 ` <201312040222.03604.arnd-r2nGTMty4D4@public.gmane.org>
2013-12-04 17:09                   ` Stephen Warren
2013-12-04 17:09                     ` Stephen Warren
     [not found]                     ` <529F61D3.3030104-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-12-04 17:18                       ` Lars-Peter Clausen
2013-12-04 17:18                         ` Lars-Peter Clausen
2013-12-06 21:16                   ` Dan Williams
2013-12-06 21:16                     ` Dan Williams
2013-12-06 22:19                     ` Arnd Bergmann
2013-12-06 22:19                       ` Arnd Bergmann
2013-11-29 14:17   ` Thierry Reding
2013-11-29 14:17     ` Thierry Reding
     [not found]     ` <20131129141725.GA22771-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
2013-12-03 17:59       ` Stephen Warren
2013-12-03 17:59         ` Stephen Warren
     [not found]         ` <529E1C1A.5000709-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-12-04  8:29           ` Thierry Reding
2013-12-04  8:29             ` Thierry Reding

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=201311292208.26215.arnd@arndb.de \
    --to=arnd-r2ngtmty4d4@public.gmane.org \
    --cc=dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org \
    --cc=swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.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.