Messages in this thread | | | Date | Wed, 27 Jul 2011 12:47:13 +0530 | Subject | Re: [PATCHv2] DMAEngine: Let dmac drivers to set chan_id | From | Jaswinder Singh <> |
| |
On 27 July 2011 09:51, Koul, Vinod <vinod.koul@intel.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@intel.com> wrote: >> > On Tue, Jul 26, 2011 at 7:30 AM, Jaswinder Singh >> > <jaswinder.singh@linaro.org> wrote: >> >> On 26 July 2011 01:38, Williams, Dan J <dan.j.williams@intel.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |