Messages in this thread | | | Subject | Re: [PATCHv2] DMAEngine: Let dmac drivers to set chan_id | From | "Koul, Vinod" <> | Date | Wed, 27 Jul 2011 14:32:26 +0530 |
| |
On Wed, 2011-07-27 at 12:47 +0530, Jaswinder Singh wrote: > 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 ? You can have two different DMACs in same system. At least I have two from current intel_mid_dma which are used. Both give their channel id starting from 0, 1.... Further as we integrate video, audio, spi, emmc dmacs possibility of having multiple dmacs will increase in a system > > > 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. Sorry I didn't get you. As I understand you are trying to simplify the filter function by assigning unique ids to all channels, but whole point itself is not quite right, as I said selection should be based on capability and not channel number "x". Can you please help me understand why channel number "x" is important and strictly needed by some client?
-- ~Vinod
| |