linux-kernel - Re: [PATCHv2] DMAEngine: Let dmac drivers to set chan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1311740499.1536.56.camel@vkoul-udesk3>
Date:	Wed, 27 Jul 2011 09:51:39 +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 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. IMO sysfs representation needs to be with
dmaengine only. How do we guarantee uniqueness b/w two controllers?

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


-- 
~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/