On 06/14/2012 06:48 AM, Arnd Bergmann wrote: > On Wednesday 13 June 2012, Jon Hunter wrote: > >>> As I said previously, I think just encoding the direction but not >>> the client specific ID (meaning we would have to disambiguate >>> the more complex cases that Stephen mentioned using a dma-names >>> property) would be the best because it keeps the common case simple, >>> but I could live with other things going in there too. >> >> Ok, so you are saying that there are some DMA controllers where there is >> no channel/request ID and only a direction is needed? So an even simpler >> case than what I had imagined. > > No, that was not the case I was thinking about. Ok, good because that would be the most simple dma controller I have heard about ;-) >> So in that case, I don't see why the first cell after the phandle could >> not be an index which could be either a direction or request-id and then >> the next cell after that be a secondary match variable. >> >> So simple case with just a index (either req-id or direction) ... >> >> dmas = <&dmac0 index> >> >> More complex case ... >> >> dmas = <&dmac0 index match> >> >> For example, for OMAP index = req-id and match = direction ... >> >> dmas = <&dmac0 req-id direction> >> >> Or am I way off the page? > > The intention was instead to remove the need for the /index/ in those > cases, because having a client-specific index in here makes it inconsistent > with other similar bindings (reg, interrupts, gpios, ...) that people > are familiar with. They use an implicit index by counting the > fields in the respective properties. So maybe "index" was not the right term to use here. What I meant was that this is really the req/chan-id associated with this device. It is not an arbitrary index that in turn gets resolved into the req-id. So in other words, just like gpio where you have the gpio number, here you have the dma req-id. > The existing method we have for avoiding index numbers is to use > named fields, like > > dmas = <&dmac0 matchA>, <dmac1 matchB>, <dmac2 matchC>; > dma-names = "rx", "rx", "tx"; > > This is similar to how we use named interrupt and mmio resources, but > it requires that we always request the dma lines by name, which is > slightly more complex than we might want it to be. Ok, but how do you get the req-id from the above binding? Doesn't it need to be stored there somewhere even for the most simplest case? Or are you saying that in this case you are just returning a name and the dma driver resolves that into a req-id? > Because the vast majority of cases just use a single channel, or one > channel per direction, my idea was to encode the direction in the > dmas property, which lets us request a dma channel by direction from > the driver side, without explicitly encoding the name. Yes, thats fine and so the direction is really the match criteria in this case. > This would let us handle the following cases very easily: > > 1. one read-write channel > > dmas = <&dmac 0x3 match>; Where 0x3 is the req-id? Just to confirm ;-) Why not have match after the phandle to be consistent with the simple example using name fields above? > 2. a choice of two read-write channels: > > dmas = <&dmacA 0x3 matchA>, <&dmacB 0x3 matchB>; > > 3. one read-channel, one write channel: > > dmas = <&dmac 0x1 match-read>, <&dmac 0x2 match-write>; > > 4. a choice of two read channels and one write channel: > > dmas = <&dmacA 0x1 match-readA>, <&dmacA 0x2 match-write> > <&dmacB match-readB>; > > And only the cases where we have more multiple channels that differ > in more aspects would require named properties: > > 5. two different channels > > dmas = <&dmac 0x3 match-rwdata>, <&dmac 0x1 match-status>; > dma-names = "rwdata", "status"; > > 6. same as 5, but with a choice of channels: > > dmas = <&dmacA 0x3 match-rwdataA>, <&dmacA 0x1 match-status>; > <dmacB 0x3 match-rwdataB>; > dma-names = "rwdata", "status", "rwdata"; > > > With a definition like that, we can implement a very simple device > driver interface for the common cases, and a slightly more complex > one for the more complex cases: > > 1. chan = of_dma_request_channel(dev->of_node, 0); > 2. chan = of_dma_request_channel(dev->of_node, 0); > 3. rxchan = of_dma_request_channel(dev->of_node, DMA_MEM_TO_DEV); > txchan = of_dma_request_channel(dev->of_node, DMA_DEV_TO_MEM); > 4. rxchan = of_dma_request_channel(dev->of_node, DMA_MEM_TO_DEV); > txchan = of_dma_request_channel(dev->of_node, DMA_DEV_TO_MEM); > 5. chan = of_dma_request_named_channel(dev->of_node, "rwdata", 0); > auxchan = of_dma_request_named_channel(dev->of_node, "status", DMA_DEV_TO_MEM); > 6. chan = of_dma_request_named_channel(dev->of_node, "rwdata", 0); > auxchan = of_dma_request_named_channel(dev->of_node, "status", DMA_DEV_TO_MEM); Yes, that looks good to me. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
