Messages in this thread | | | Date | Tue, 26 Nov 2013 08:57:19 -0800 | Subject | Re: [PATCH V2] dma: add common of_dma_slave_xlate() | From | Dan Williams <> |
| |
On Mon, Nov 25, 2013 at 11:40 PM, Lars-Peter Clausen <lars@metafoo.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.
| |