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: <CAJe_ZhfaPfobs5P+q+t-kRSK4fAvm-v_brYgqmco8fwOOBeT8A@mail.gmail.com>
Date:	Wed, 27 Jul 2011 12:47:13 +0530
From:	Jaswinder Singh <jaswinder.singh@...aro.org>
To:	"Koul, Vinod" <vinod.koul@...el.com>
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 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 ?

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