On Tuesday 07 April 2009, Rajashekhara, Sudhakar wrote: > Dave, > > > > > This patch is needlessly intrusive and complex. [...] > > I agree with you. > > > > > - Define one new channel macros, along the lines of > > > > #define EDMA_CHANNEL(ctlr,chn) (((ctlr)<<10)|(chan))
Maybe if you make that "16" the compiler will notice the obvious optimizations and eliminate some shifts in favor of halfword load instructions. :) > > Those values would be passed in IORESOURCE_DMA resources, > > returned by edma_alloc_channel(), and accepted by all the > > current interfaces accepting an EDMA channel. Of course > > EDMA_CHANNEL_ANY would still work, but it could look at > > more controllers than just the first one. > > I started off with something similar initially and I came till > the point where I had to add an extra argument to edma_alloc_slot(). > At this point, I thought of adding the EDMA CC instance as an > argument to all the interfaces to maintain uniformity in the code. I was thinking of a different level of uniformity: the IORESOURCE_DMA identifiers, as well as the fairly common model throughout the kernel that DMA channels only need a single-word cookie for identification. > [...] > > > > > - Slot allocation would need some change, because it would > > need to be coupled to a controller (and thus its channels). > > This should be the only driver-visible change. > > Meaning, this should be the only change that is visible to the > drivers using EDMA. Right? Well, that and the fallout from defining those utilities to pull the controller and "real" channel numbers from a "logical" channel/slot ID. Which will be minor - Dave > [...] > > > > > Those three calls for logical EDMA channel IDs should work > > for slot IDs too: one to build IDs from a controller and > > slot number, and two extracting those numbers from the ID. > > Difference being that "raw" channel numbers are max 63, > > while "raw' slot numbers can be bigger (I've seen 511). > > > > - Rather than converting static state to arrays, first > > collect all that state into a "struct edma" or somesuch, > > and have an array of those. Use platform_device.id to > > say which instance is being configured, and configure > > each instance normally in probe(). > > > > I'll work on these comments and post the patches again. During > this, in case of any doubts, I'll get back to you. > > Thanks, Sudhakar > > _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
