linux-kernel - Re: [PATCH V2] dma: add common of_dma_slave

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] [day] [month] [year] [list]

Message-ID: <CAPcyv4hx5nVth2f4SSDYWBNR+bs-4Rj+Bnua=w97S9OLoq0Gqw@mail.gmail.com>
Date:	Tue, 26 Nov 2013 08:57:19 -0800
From:	Dan Williams <dan.j.williams@...el.com>
To:	Lars-Peter Clausen <lars@...afoo.de>
Cc:	Stephen Warren <swarren@...dotorg.org>,
	Vinod Koul <vinod.koul@...el.com>,
	"dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Stephen Warren <swarren@...dia.com>
Subject: Re: [PATCH V2] dma: add common of_dma_slave_xlate()

On Mon, Nov 25, 2013 at 11:40 PM, Lars-Peter Clausen <lars@...afoo.de> wrote:
> On 11/26/2013 02:25 AM, Stephen Warren wrote:
> [...]
>> +struct dma_chan *of_dma_slave_xlate(struct of_phandle_args *dma_spec,
>> +                                 struct of_dma *ofdma)
>> +{
>> +     struct of_dma_slave_xlate_info *info = ofdma->of_dma_data;
>> +     struct dma_chan *candidate, *chan;
>> +     int ret;
>> +
>> +retry:
>> +     candidate = NULL;
>> +
>> +     /*
>> +      * walk the list of channels registered with the current instance and
>> +      * find one that is currently unused
>> +      */
>> +     list_for_each_entry(chan, &info->device->channels, device_node)
>> +             if (chan->client_count == 0) {
>> +                     candidate = chan;
>> +                     break;
>> +             }
>> +
>> +     if (!candidate)
>> +             return NULL;
>> +
>> +     /*
>> +      * dma_get_slave_channel will return NULL if we lost a race between
>> +      * the lookup and the reservation
>> +      */
>> +     chan = dma_get_slave_channel(candidate);
>> +     if (!chan)
>> +             goto retry;
>
> I think it will be better to implement this functionality in the core. This
> means we can hold the dma_list_mutex and don't have to do this retry loop
> and do not have to peek at the client_count field in non-core code.
> Something like dma_get_free_slave_channel(), which would call
> private_candidate() followed by dma_chan_get().
>
>> +
>> +     if (info->post_alloc) {
>> +             ret = info->post_alloc(chan, dma_spec);
>
> If you need to do something at the end of the function I think it is nicer
> to just wrap this function with your own function instead of adding a callback.
>

Agree with both points above.  And if the common pattern is to limit
the domain to children of a given parent device that can be
implemented generically with a device_for_each_child_loop or similar.
--
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/