On Wed, Aug 10, 2011 at 1:24 PM, Jassi Brar <jassisinghb...@gmail.com> wrote:
> On Wed, Aug 10, 2011 at 4:10 PM, Russell King - ARM Linux
>>
>> I discussed this with Linus on the bus back from Cambridge in the evening,
>> and it appears that the story you gave me was inaccurate - Linus had not
>> agreed to your proposal and saw more or less the same problems with it
>> which I've been on at you about via your other email alias/lkml.  So that's
>> essentially invalidated everything we discussed there as part of my thinking
>> was "if Linus is happy with it, then...".
>
> IIRC, Linus W mainly opined to involve device pointer during channel 
> selection.
> Other than that I thought there was only ambiguity about implementation 
> details.

Bah now we have two "Linus oracles" in this world instead
of just one... haha :-)

> Linus W, was there anything you said wouldn't work with the scheme ?
> Please tell now on the record.

It would *work* but the current proposal is *not elegant* IMO.

Let me put it like this:

Yes, there is a problem with how all platforms have to pass in a
filter function and some opaque cookie for every driver to look up
its DMA channel.

You seem to want to address this problem, which is good.

I think your scheme passing in a device ID and instance tuple
e.g. REQ(MMC,2) would *work*, but I don't think it is a proper
solution and I would never ACK it, as I said.

The lookup of a device DMA channel should follow the
design pattern set by regulators and clocks. Which means
you use struct device * or alternatively a string (if the
instance is not available) to look it up, such that:

struct dma_slave_map {
   char name[16];
   struct device *dev;
   char dev_name[16];
   struct devive *dma_dev;
   char dma_dev_name[16];
};

struct dma_slave_map[] = {
  {
     .name = "MMC-RX",
     .dev_name = "mmc.0",
     .dma_dev_name = "pl08x.0",
  },
  {
     .name = "MMC-TX",
     .dev_name = "mmc.0",
     .dma_dev_name = "pl08x.0",
  },
  {
     .name = "SSP-RX",
     .dev_name = "pl022.0",
     .dma_dev_name = "pl08x.1",
  },
  ...
};

The dmaengine core should take this mapping, and
the device driver would only have to:

struct dma_chan *rxc;
struct dma_chan *txc;
struct device *dev;

rxc = dma_request_slave_channel(dev, "MMC-RX");
txc = dma_request_slave_channel(dev, "MMC-TX");

I also recognized that you needed a priority scheme and
a few other bits for the above, so struct dma_slave_map
may need a few more members, but the above would
sure solve all usecases we have today. mem2mem could
use the old request function and doesn't need anything
like this.

Pros: well established design pattern, channels tied to
devices, intuitive for anyone who used clock or
regulator frameworks.

If the majority is happy with this scheme I can even
try implementing it.

Yours,
Linus Walleij

------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Reply via email to