Re: [PATCH/RFC] dmaengine: add a slave parameter to __dma_request_channel()

[Guennadi Liakhovetski]

On Fri, 9 Mar 2012, Vinod Koul wrote:

> On Fri, 2012-03-09 at 10:24 +0100, Guennadi Liakhovetski wrote:
> > On Fri, 9 Mar 2012, Vinod Koul wrote:
> > 
> > > On Thu, 2012-03-08 at 14:18 +0100, Guennadi Liakhovetski wrote:
> > > > > Assuming I didn't miss...
> > > > > 
> > > > > The case B can be handled without sweat by platforms channel mapping
> > > > > information.
> > > > > 
> > > > > Case A where we don't find that devices exist in map, thus being treated
> > > > > as generic DMA channels and can be handled easily in sequence. So when
> > > > > someone in Q request a channel it would get first channel in Ps
> > > > > 
> > > > > This way we handle both of them in a transparent manner to both clients
> > > > > and controllers. 
> > > > > 
> > > > > Perhaps we can also add capability to know that if channel is to be
> > > > > searched in map or not - would be anyway required for non slave cases.
> > > > 
> > > > Right, but I don't understand then what this gives us. You propose some 
> > > > channel maps, that will not be used for your "case A." Which means, for 
> > > > "case A" nothing changes. So, the reason for this whole thread hasn't been 
> > > > addressed: how to pass channel configuration to the DMA controller driver.
> > > For "Case A" there should be no filtering or any issues even now. You
> > > have controller requesting a channel and as long as they get a channel
> > > for respective pool, it should work.
> > > 
> > > Or is there anything else which is required in this case?
> > 
> > Yes, there is. You keep complaining about my use of .priv pointer, which I 
> > use in the filter to pass channel configuration to the DMA controller 
> > driver. That is the original reason for this whole discussion. If you're 
> > fine with that, I'll just carry that .priv use over in the "simple" DMA 
> > engine driver and stay happy forever:-)
> Channel configuration != filtering & channel allocation
> 
> The discussion is focused on getting request_channel fixed so that we
> take care of oddities in respective platforms for channel maps which
> exist.
> What the heck does it have to on how you configure you channel??
> 
> Once you have channel allocated correctly which can work for you,

It can be made to work as long as there's only one DMAC group with 
configurable channels and all other DMACs are dedicated to specific 
peripherals, yes. I don't know whether there are already now or are 
approaching any platforms with multiple reconfigurable groups.

> you
> need to configure you channel. Since this is slave case you to tell what
> your client FIFO depth, burst size, width etc parameters. There is
> dma_slave_config for that purpose ONLY. Is there anything else required
> for _configuring_ if yes, we can add those. But then again that doesn't
> have anything to do with how you get a channel.

As Russell mentioned, struct dma_slave_config isn't suitable for channel 
configuration. Currently we use struct sh_dmae_slave_config to configure 
channels. As you can see, apart from the client's address, it also 
contains two fields with register values, that have to be used with this 
client. We could use dmaengine_slave_config() and embed struct 
dma_slave_config in our private type, but out of 7 fields of that struct 
effectively only one would be used, which makes this approach both clumsy 
and inefficient.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/