On Tue, 2012-06-26 at 20:27 +0000, Arnd Bergmann wrote:

On Tuesday 26 June 2012, Vinod Koul wrote:

quoted

On Tue, 2012-06-26 at 14:59 +0000, Arnd Bergmann wrote:

quoted

On Tuesday 26 June 2012, Vinod Koul wrote:

quoted

Today, we just ask for a channel with specific mask. Further filtering
is done in filter function as we request a channel, not a specific one.
In most slave cases, we need a specific channel from a specific
controller, and that is where DT can play a role. In addition to DMA
resources for dma and client driver, I would expect DT to provide the
channel mapping information, which is anyway known only by platform.

Can you describe what you mean by "channel mapping information"?
Is that not what we pass into the filter function?

Today many dmaengine drivers have a filter function which is exported
and then used by clients to filter out the channel. That is not a right
way to do, so any future plan which is based on filter is not correct.

I agree that exporting a filter function from the dmaengine driver is
very wrong and should not be done because it requires that the device
driver knows which engine is used and that is contrary to the idea
of having an abstraction layer.

We were talking about using a filter function because that would be
easy to do without changing the core dmaengine code. However, in the
proposal, there would actually just be a single filter function that
gets used by all drivers that have DT bindings, and it can be
completely encapsulated in the of_dma_request_channel() function
so it does not have to be a global symbol.

I kind of like this idea.

If we instead modify the dmaengine code itself to know about DT
rather than wrapping around it, we would not need this filter
function, but we should still have a probe() function that is
called by dmaengine code to interpret the data that is specific
to one dmaengine driver.

I was hoping we can have dmaengine binding, that way dmaengine core code
knows about what to do when some client requests a channel.

quoted

IMHO dmaengine driver should *not* know anything about mapping. By
mapping I refer to platform information which tells me which client can
use which channel from which dmac.

Agreed too. That information shoudd be part of the slave device-node
in DT, as I have argued in this thread already. The slave device driver
does not need to care about the format or the contents of it,
but we need some code to interpret the contents. From all I can tell,
the structure of this data cannot be completely generic because of
all the special cases, so the code to interpret it would live in the
probe() function I mentioned about that the dmaengine driver provides.

rather than slave driver, why dont we keep this binding within
dmaengine. That would make slave and clients completely independent.

quoted

quoted

I think encoding a description for a dma request in a single number is
the last thing we want to do here. We've tried that with IRQ and GPIO
numbers and it got us into a huge mess that will need a long time to
get out of.

No i wasn't thinking of a number. The mapping shouldn't be a global
number at all, though that is a very easy but not very scalable
solution.
We need to take care of 1:1 mapping of client and channels as well as
many:1  cases as well. A single global number cannot represent that
properly.

My idea is platform gives this information to dmaengine. Clients and
dmaengine driver do not worry about it. That also paves way for arch
independent clients and drivers.

IMO the platform should have no part in this. I absolutely want to
get rid of any platform-specific hardcoded tables in the kernel for
stuff that can easily be run-time detected from the device tree.
There are cases where hard-coding in the kernel is easier, but I don't
think this is one of them.

Again, you got me wrong. We don't want any hardcoded table is kernel.
The information in table should come from whatever way that platform can
give me. For your case it should be DT.
We can have the map of which client can use which channel as DT binding
of dmaengine core. So dmaengine can easily arbitrate about channel
requests. Again this mapping information is not some even linux
independent

quoted

quoted

Some platforms actually use IORESOURCE_DMA, which was useful to describe
ISA DMA channels, for encoding some form of channel or request number,
but this causes all sorts of problems. These are almost exclusively
used by those platforms that don't have a dmaengine driver yet, so I'd
hope that we can remove this as we convert those platforms over to
dmaengine and device tree.

The representation in device tree as we have it now is a combination of
a pointer to the dmaengine and a description of the request line in it,
typically a single small integer number local to the dmaengine. We should
not try to make that a global integer number again that just serves the
purpose of looking up the dmaengine and local number again.

IMHO no device driver should be bothered with any artificial resource
information, but instead I want all the DT parsing to happen in the
dmaengine code (or some wrapper around it) where it gets used. The only
thing that a device driver needs to know is that it wants to use a
channel based on what is described in the device tree.

Sure, but I would expect the clients and dmacs to find information about
their devices from DT?
dmaengine should get only mapping information used for allocating
channel to client.

Let's take a look at a concrete example. The
arch/arm/mach-ux500/board-mop500-sdi.c file defines dma channel
configuration for the mmc devices on the ux500 platform that looks
like

static struct stedma40_chan_cfg mop500_sdi2_dma_cfg_tx = {
        .mode = STEDMA40_MODE_LOGICAL,
        .dir = STEDMA40_MEM_TO_PERIPH,
        .src_dev_type = STEDMA40_DEV_SRC_MEMORY,
        .dst_dev_type = DB8500_DMA_DEV28_SD_MM2_TX,
        .src_info.data_width = STEDMA40_WORD_WIDTH,
        .dst_info.data_width = STEDMA40_WORD_WIDTH,
};

I want to move this information to the device tree in a way that the
device driver does not have to care about it. With the proposed
binding, this would mean we get an mmci device node with a property
containing something like

	dma-requests = <&dma40  /* pointer to dma engine */
			0x01    /* logical, mem to dev */
			28      /* DEV28_SD_MM2_TX */
			32>,	/* 32 bit width */
		       <&dma40 0x02 32 32>; /* dev to mem channel */ 

The fact that this dmaengine driver requires two cells (request number
and data width) should only be known to the code that deals with that
one driver and that should interpret those two cells, while the first
two cells (pointer to dma-engine and direction) can be handled
by the common dmaengine layer.

In order to do that, we need some code in the dmaengine driver that
gets the property data as its argument.

Yes but not thru the slave drivers. 


-- 
~Vinod