Re: [PATCH 3/3] dmaengine: tegra-adma: Add support for Tegra210 ADMA

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday 29 September 2015 13:18:06 Jon Hunter wrote:
> 
> On 28/09/15 16:07, Arnd Bergmann wrote:
> > On Monday 28 September 2015 15:57:46 Jon Hunter wrote:
> 
> [snip]
> 
> >> Actually, what I mentioned about would not work as it is not the DMA
> >> that should assign the requests to the channel.
> >>
> >> I think that I have poorly described the hardware and how it works, so
> >> let me try and explain a bit more.
> >>
> >> From a hardware perspective it looks like the following ...
> >>
> >> memory <-> adma <-> adma-if <-> xbar <-> clients
> >>
> >> where:
> >> - memory is the system memory
> >> - adma is the dma controller
> >> - adma-if is the dma interface to a crossbar
> >> - xbar is the crossbar
> >> - clients are various audio interfaces, such as i2s, etc
> >>
> >> The adma-if is essentially a mux with 10 tx and 10 rx ports. Any of the
> >> 22 adma channels can be mapped to any of the 10 tx or rx ports. The
> >> request signal used by the adma is determined by which port it is
> >> configured to use. However, what makes this even more configurable is
> >> the xbar is also a mux that can route adma-if ports to the various clients.
> >>
> >> So potentially, you could use any adma channel and any port to route
> >> audio to any of the clients. However, the adma-if needs to know which
> >> adma channel is mapped to which port and so it probably makes most sense
> >> for the adma-if binding (not in the mainline yet) so specify a mapping
> >> even though any mapping could be used.
> >>
> >> 	admaif@0x702d0000 {
> >> 		dmas = <&adma 1>, <&adma 1>, <&adma 2>, <&adma 2>,
> >> 		...
> >> 		dma-names = "rx1", "tx1", "rx2", "tx2",
> >> 		...
> >> 	};
> >>
> >> Alternatively, I was thinking that I could set #dma-cells = 0, but what
> >> I should have done is just have something like ...
> >>
> >> 	admaif@0x702d0000 {
> >> 		dmas = <&adma>, <&adma>, <&adma>, <&adma>,
> >> 		...
> >> 		dma-names = "rx1", "tx1", "rx2", "tx2",
> >> 		...
> >> 	};
> >>
> >> In the above case the adma-if driver can just pick whatever channel it
> >> wants for each of the ports.
> >>
> >> Hope this is clearer.
> > 
> > I think what you are saying here is that the ADMA driver does not talk
> > to dma slave devices at all, but only talks to the adma-if, which in
> > turn only talks to the xbar.
> 
> Yes, this is the use-case that this current version of the driver
> supports. However ...
> 
> > That is fine, but the result of that is that there is no point in
> > using the DMA slave binding for this device, or for the driver to
> > register itself as a generic slave driver, which it is not.
> 
> ... the DMA controller itself has a source and destination register and
> from a hardware perspective, the source and/or destination does not need
> to be the ADMA-IF. Potentially, we could support memory to memory
> transfers, but currently we are not using the DMA for this. So I can see
> that with the current usage it may not make sense for it to be
> registered as a generic slave driver, but from a hardware perspective I
> think it still does.

Ok, but imagine a device where the ADMA is actually wired up to a slave
directly. In this case, you need #dma-cells=<1> (or higher) to identify
which slave that is. If you want the binding to be generic enough to
handle that case, it needs to correctly describe the identification of
the request lines.
 
> The current tegra audio drivers use the snd_dmaengine_xxx helpers and so
> it would be nice to do the same here.

That seems to be a separate (non-)problem, as the audio devices would
have their 'dmas' properties pointing to the adma-if or the xbar (depending
on how you define those bindings), and don't need to change either way.

> > We should look at this driver in combination with the adma-if and
> > xbar drivers and then define a binding for the combination of those.
> 
> Yes that makes sense, but I think that I have confused matters here a
> bit and not thought through this entirely. So while we could configure
> an audio interface, such as i2s, to use any adma-if port and hence any
> dma channel with the appropriate hardware request signal, the mapping
> between the adma-if port and request signal is fixed. For example,
> 
> adma-if rx1 port uses adma request signal 1
> adma-if rx2 port uses adma request signal 2
> adma-if rx3 port uses adma request signal 3
> ...
> adma-if rx10 port uses adma request signal 10
> 
> and
> 
> adma-if tx1 port uses adma request signal 1
> adma-if tx2 port uses adma request signal 2
> adma-if tx3 port uses adma request signal 3
> ...
> adma-if tx10 port uses adma request signal 10
> 
> What is connected to these adma-if tx and rx ports who knows but I think
> that is where I was going wrong and made this more complex than it
> should have been. So I think that the adma binding should have
> #dma-cells = 1 and the adma-if binding have the following ...
> 
>  	admaif@0x702d0000 {
>  		dmas = <&adma 1>, <&adma 1>, <&adma 2>, <&adma 2>,
>  		...
>  		dma-names = "rx1", "tx1", "rx2", "tx2",
>  		...
>  	};

Ok, that makes more sense then. A few questions still:

* Would the admaif in turn provide a #dma-cells and have the real slaves
  hooked up to it?
* How do you model the xbar in this scenario?
* How does the adma driver know whether you are using a rx or tx channel
  in the above example, should that perhaps be another cell in dma
  specifier? It seems that the numbers are all overlapping between rx and
  tx (each has 1 through 10).

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux