Date:	Wed, 27 Jul 2011 14:32:26 +0530
From:	"Koul, Vinod" <vinod.koul@...el.com>
To:	Jaswinder Singh <jaswinder.singh@...aro.org>
Cc:	"Williams, Dan J" <dan.j.williams@...el.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	linux-kernel@...r.kernel.org, rmk+kernel@....linux.org.uk,
	linus.walleij@...ricsson.com, per.friden@...ricsson.com,
	wei.zhang@...escale.com, ebony.zhu@...escale.com,
	iws@...o.caltech.edu, s.hauer@...gutronix.de,
	maciej.sosnowski@...el.com, saeed@...vell.com,
	shawn.guo@...escale.com, yur@...raft.com, agust@...x.de,
	iwamatsu.nobuhiro@...esas.com, per.forlin@...ricsson.com,
	jonas.aberg@...ricsson.com, anemo@....ocn.ne.jp
Subject: Re: [PATCHv2] DMAEngine: Let dmac drivers to set chan_id

On Wed, 2011-07-27 at 12:47 +0530, Jaswinder Singh wrote:
> On 27 July 2011 09:51, Koul, Vinod <vinod.koul@...el.com> wrote:
> > On Tue, 2011-07-26 at 23:42 +0530, Jaswinder Singh wrote:
> >> On 26 July 2011 20:59, Williams, Dan J <dan.j.williams@...el.com> wrote:
> >> > On Tue, Jul 26, 2011 at 7:30 AM, Jaswinder Singh
> >> > <jaswinder.singh@...aro.org> wrote:
> >> >> On 26 July 2011 01:38, Williams, Dan J <dan.j.williams@...el.com> wrote:
> >> >>> Correct, it is meant that chan_id is only a sysfs property.  Any
> >> >>> driver usage that is assuming chan_id is anything more than a
> >> >>> guaranteed unique number within a given dma_device's list of channels
> >> >>> is probably inferring too much.
> >> >>
> >> >> So you mean dmac/client drivers are wrong if they make use of chan_id.
> >> >> They shouldn't count upon it's value - which is set by DMA API for a completely
> >> >> independent purpose, i.e, creating contiguous sysfs entries.
> >> >
> >> > They can count on it being unique, and maybe the fact that it is in
> >> > the same order as dma_device.channels.
> >> The latter implies the former. And it is already the dmac driver that
> >> decides the
> >> rank of a channel in the list.
> >>
> >> >
> >> >>
> >> >> Since "chan_id is only a sysfs property" and the fact that it is used
> >> >> only _once_
> >> >> by the DMA API
> >> >>
> >> >> In drivers/dma/dmaengine.c
> >> >>
> >> >>      chan->chan_id = chancnt++;
> >> >>      dev_set_name(&chan->dev->device, "dma%dchan%d",
> >> >>                             device->dev_id, chan->chan_id);
> >> >>
> >> >>
> >> >> Can't we do away with chan_id altogether ? by having
> >> >>
> >> >>      dev_set_name(&chan->dev->device, "dma%dchan%d",
> >> >>                             device->dev_id, chancnt++);
> >> >>
> >> >> I mean why make every instance of dma_chan bigger by 4bytes ?
> >> >>
> >> >> So why shouldn't we remove chan_id completely from the DMA API ?
> >> >
> >> > Good point...  Let's remove chan_id from the core and push it into the
> >> > drivers that need it.
> >> >
> >> If you agree, I would preserve the chan_id in 'struct dma_chan' but remove
> >> any assignment to it in dmaengine.c  and let the dmac drivers use it freely.
> >> That would:-
> >> a) Let dmac drivers decide what numbers they want to show up in sysfs.
> >> b) chan_id is easily reachable by client drivers, so it is better this way.
> >> c) It would mean lesser and simpler changes to extant users of it.
> > But this can cause conflict between two controllers who think they are
> > assigning unique numbers.
> Could you please clarify, which two controllers ?
You can have two different DMACs in same system. At least I have two
from current intel_mid_dma which are used. Both give their channel id
starting from 0, 1....
Further as we integrate video, audio, spi, emmc dmacs possibility of
having multiple dmacs will increase in a system
> 
> > IMO sysfs representation needs to be with
> > dmaengine only. How do we guarantee uniqueness b/w two controllers?
> Again, how is chan_id currently unique between two controllers ?
> 
> Btw, do you not want to keep chan_id in dma_chan or do you not want to change
> anything at all ?
> 
> >
> > Also I am not sure about the approach: The whole point is to make filter
> > function select based on some channel number "x", but you should filter
> > channels based on controller you want and capability of a channel. If
> > caps is not enough to filter we should add more flags but asking that I
> > need channel 'y' doesn't sound right to me.
> > This is all coming from that fact that some drivers assume channel 'a'
> > is for this type of transfer and channel 'b' for something else, for a
> > dma controller that really doesn't matter, as all channels have similar
> > capabilities and can do similar things so you should
> > _get_channel_based_on_caps rather than on some numbering.
> >
> > Lastly, why do you need a channel reserved for some type of transfer, is
> > it for assigning h/w interface for dma transfer, if so that can be
> > achieved in different ways as well
> 
> Please look at this patch from POV of utility of chan_id, which isn't much
> currently as explainined.
Sorry I didn't get you. 
As I understand you are trying to simplify the filter function by
assigning unique ids to all channels, but whole point itself is not
quite right, as I said selection should be based on capability and not
channel number "x". 
Can you please help me understand why channel number "x" is important
and strictly needed by some client?

-- 
~Vinod

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists