[V4,06/14] ARM: SAMSUNG: Add common DMA operations
diff mbox

Message ID 1311557312-26107-7-git-send-email-boojin.kim@samsung.com
State New, archived
Headers show

Commit Message

boojin.kim July 25, 2011, 1:28 a.m. UTC
This patch adds common DMA operations which are used for Samsung DMA
drivers. Currently there are two types of DMA driver for Samsung SoCs.
The one is S3C-DMA for S3C SoCs and the other is PL330-DMA for S5P SoCs.
This patch provides funcion pointers for common DMA operations to DMA
client driver like SPI and Audio. It makes DMA client drivers support
multi-platform.
In addition, this common DMA operations implement the shared actions
that are needed for DMA client driver. For example shared actions are
filter() function for dma_request_channel() and parameter passing for
device_prep_slave_sg().

Signed-off-by: Boojin Kim <boojin.kim@samsung.com>
---
 arch/arm/mach-s3c2410/include/mach/dma.h       |    8 ++-
 arch/arm/mach-s3c64xx/include/mach/dma.h       |    4 +
 arch/arm/plat-samsung/Makefile                 |    6 +-
 arch/arm/plat-samsung/dma-ops.c                |  133 ++++++++++++++++++++++++
 arch/arm/plat-samsung/include/plat/dma-ops.h   |   64 +++++++++++
 arch/arm/plat-samsung/include/plat/dma-pl330.h |    4 +
 arch/arm/plat-samsung/include/plat/dma.h       |    1 +
 arch/arm/plat-samsung/s3c-dma-ops.c            |  133 ++++++++++++++++++++++++
 8 files changed, 350 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/plat-samsung/dma-ops.c
 create mode 100644 arch/arm/plat-samsung/include/plat/dma-ops.h
 create mode 100644 arch/arm/plat-samsung/s3c-dma-ops.c

Comments

Russell King - ARM Linux July 25, 2011, 9:36 a.m. UTC | #1
On Mon, Jul 25, 2011 at 10:28:24AM +0900, Boojin Kim wrote:
> +static unsigned samsung_dmadev_request(enum dma_ch dma_ch,
> +				struct samsung_dma_info *info)
> +{
> +	struct dma_chan *chan;
> +	dma_cap_mask_t mask;
> +	struct dma_slave_config slave_config;
> +
> +	dma_cap_zero(mask);
> +	dma_cap_set(info->cap, mask);
> +
> +	chan = dma_request_channel(mask, pl330_filter, (void *)dma_ch);
> +
> +	if (info->direction == DMA_FROM_DEVICE) {
> +		memset(&slave_config, 0, sizeof(struct dma_slave_config));
> +		slave_config.direction = info->direction;
> +		slave_config.src_addr = info->fifo;
> +		slave_config.src_addr_width = info->width;

This should really set slave_config.src_maxburst to something sensible too,
even if that's just '1'.

> +		dmaengine_slave_config(chan, &slave_config);
> +	} else if (info->direction == DMA_TO_DEVICE) {
> +		memset(&slave_config, 0, sizeof(struct dma_slave_config));
> +		slave_config.direction = info->direction;
> +		slave_config.dst_addr = info->fifo;
> +		slave_config.dst_addr_width = info->width;

Ditto for dst_maxburst.

> +		dmaengine_slave_config(chan, &slave_config);
> +	}
> +
> +	return (unsigned)chan;

I hope these interfaces yet cleaned away soon so that these casts can be
killed off.

> +struct samsung_dma_prep_info {
> +	enum dma_transaction_type cap;
> +	enum dma_data_direction direction;
> +	unsigned buf;

dma_addr_t ?

> +	unsigned long period;
> +	unsigned long len;
> +	void (*fp)(void *data);
> +	void *fp_param;
> +};
> +
> +struct samsung_dma_info {
> +	enum dma_transaction_type cap;
> +	enum dma_data_direction direction;
> +	enum dma_slave_buswidth width;
> +	unsigned fifo;

dma_addr_t ?

> +	struct s3c2410_dma_client *client;
> +};
> +
> +struct samsung_dma_ops {
> +	bool init;

Unused?
boojin.kim July 25, 2011, 9:57 a.m. UTC | #2
Russell King - ARM Linux Wrote:

> Sent: Monday, July 25, 2011 6:36 PM
> To: Boojin Kim
> Cc: linux-arm-kernel@lists.infradead.org;
linux-samsung-soc@vger.kernel.org;
> Kukjin Kim; Vinod Koul; Jassi Brar; Grant Likely; Mark Brown; Dan Williams
> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations
>
> On Mon, Jul 25, 2011 at 10:28:24AM +0900, Boojin Kim wrote:
> > +static unsigned samsung_dmadev_request(enum dma_ch dma_ch,
> > +				struct samsung_dma_info *info)
> > +{
> > +	struct dma_chan *chan;
> > +	dma_cap_mask_t mask;
> > +	struct dma_slave_config slave_config;
> > +
> > +	dma_cap_zero(mask);
> > +	dma_cap_set(info->cap, mask);
> > +
> > +	chan = dma_request_channel(mask, pl330_filter, (void *)dma_ch);
> > +
> > +	if (info->direction == DMA_FROM_DEVICE) {
> > +		memset(&slave_config, 0, sizeof(struct dma_slave_config));
> > +		slave_config.direction = info->direction;
> > +		slave_config.src_addr = info->fifo;
> > +		slave_config.src_addr_width = info->width;
>
> This should really set slave_config.src_maxburst to something sensible
too,
> even if that's just '1'.
I will address your comment.

>
> > +		dmaengine_slave_config(chan, &slave_config);
> > +	} else if (info->direction == DMA_TO_DEVICE) {
> > +		memset(&slave_config, 0, sizeof(struct dma_slave_config));
> > +		slave_config.direction = info->direction;
> > +		slave_config.dst_addr = info->fifo;
> > +		slave_config.dst_addr_width = info->width;
>
> Ditto for dst_maxburst.
>
> > +		dmaengine_slave_config(chan, &slave_config);
> > +	}
> > +
> > +	return (unsigned)chan;
>
> I hope these interfaces yet cleaned away soon so that these casts can be
> killed off.
>
> > +struct samsung_dma_prep_info {
> > +	enum dma_transaction_type cap;
> > +	enum dma_data_direction direction;
> > +	unsigned buf;
>
> dma_addr_t ?
>
> > +	unsigned long period;
> > +	unsigned long len;
> > +	void (*fp)(void *data);
> > +	void *fp_param;
> > +};
> > +
> > +struct samsung_dma_info {
> > +	enum dma_transaction_type cap;
> > +	enum dma_data_direction direction;
> > +	enum dma_slave_buswidth width;
> > +	unsigned fifo;
>
> dma_addr_t ?
>
> > +	struct s3c2410_dma_client *client;
> > +};
> > +
> > +struct samsung_dma_ops {
> > +	bool init;
>
> Unused?
Yes, It's my mistake. I will address your comment.
Jassi Brar July 25, 2011, 11:51 a.m. UTC | #3
On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim <boojin.kim@samsung.com> wrote:

> +
> +static bool pl330_filter(struct dma_chan *chan, void *param)
> +{
> +       struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan->private;
> +       unsigned dma_ch = (unsigned)param;
> +
> +       if (peri->peri_id != dma_ch)
> +               return false;
> +
> +       return true;
> +}
This is what I meant... if we keep chan_id for paltform assigned IDs,
these filter functions could simply become

static bool pl330_filter(struct dma_chan *chan, void *param)
{
        return chan->chan_id == param
}

And ideally in the long run, we could just drop the filter callback
and add expected channel ID to the request_channel call.



> +
> +static inline int s3c_dma_trigger(unsigned ch)
> +{
> +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START);
> +}
> +
> +static inline int s3c_dma_started(unsigned ch)
> +{
> +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED);
> +}
> +
> +static inline int s3c_dma_flush(unsigned ch)
> +{
> +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH);
> +}
> +
> +static inline int s3c_dma_stop(unsigned ch)
> +{
> +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP);
> +}
> +
> +static struct samsung_dma_ops s3c_dma_ops = {
> +       .request        = s3c_dma_request,
> +       .release        = s3c_dma_release,
> +       .prepare        = s3c_dma_prepare,
> +       .trigger        = s3c_dma_trigger,
> +       .started        = s3c_dma_started,
> +       .flush          = s3c_dma_flush,
> +       .stop           = s3c_dma_stop,

These last 4 should be gnereallized into one callback with OP argument.
Russell King - ARM Linux July 26, 2011, 8:02 a.m. UTC | #4
On Mon, Jul 25, 2011 at 05:21:44PM +0530, Jassi Brar wrote:
> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim <boojin.kim@samsung.com> wrote:
> 
> > +
> > +static bool pl330_filter(struct dma_chan *chan, void *param)
> > +{
> > +       struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan->private;
> > +       unsigned dma_ch = (unsigned)param;
> > +
> > +       if (peri->peri_id != dma_ch)
> > +               return false;
> > +
> > +       return true;
> > +}
> This is what I meant... if we keep chan_id for paltform assigned IDs,
> these filter functions could simply become
> 
> static bool pl330_filter(struct dma_chan *chan, void *param)
> {
>         return chan->chan_id == param
> }
> 
> And ideally in the long run, we could just drop the filter callback
> and add expected channel ID to the request_channel call.

So what if you have a PL080 and PL330 drivers registered with the DMA
engine code, and you need to match a specific PL330 channel?
boojin.kim July 26, 2011, 9:35 a.m. UTC | #5
Jassi Brar Wrote:
> Sent: Monday, July 25, 2011 8:52 PM
> To: Boojin Kim
> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant
> Likely; Mark Brown
> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations
>
> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim <boojin.kim@samsung.com>
> wrote:
>
> > +
> > +static bool pl330_filter(struct dma_chan *chan, void *param)
> > +{
> > +       struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan-
> >private;
> > +       unsigned dma_ch = (unsigned)param;
> > +
> > +       if (peri->peri_id != dma_ch)
> > +               return false;
> > +
> > +       return true;
> > +}
> This is what I meant... if we keep chan_id for paltform assigned IDs,
> these filter functions could simply become
>
> static bool pl330_filter(struct dma_chan *chan, void *param)
> {
>         return chan->chan_id == param
> }
>
> And ideally in the long run, we could just drop the filter callback
> and add expected channel ID to the request_channel call.
The chan_id is set by dmaengine. So, We don't use it to hold the user specific 
Id.

>
>
>
> > +
> > +static inline int s3c_dma_trigger(unsigned ch)
> > +{
> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START);
> > +}
> > +
> > +static inline int s3c_dma_started(unsigned ch)
> > +{
> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED);
> > +}
> > +
> > +static inline int s3c_dma_flush(unsigned ch)
> > +{
> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH);
> > +}
> > +
> > +static inline int s3c_dma_stop(unsigned ch)
> > +{
> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP);
> > +}
> > +
> > +static struct samsung_dma_ops s3c_dma_ops = {
> > +       .request        = s3c_dma_request,
> > +       .release        = s3c_dma_release,
> > +       .prepare        = s3c_dma_prepare,
> > +       .trigger        = s3c_dma_trigger,
> > +       .started        = s3c_dma_started,
> > +       .flush          = s3c_dma_flush,
> > +       .stop           = s3c_dma_stop,
>
> These last 4 should be gnereallized into one callback with OP argument.
I don't have any idea about it. Can you explain it in more detail?
Jassi Brar July 26, 2011, 5:43 p.m. UTC | #6
On Tue, Jul 26, 2011 at 1:32 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jul 25, 2011 at 05:21:44PM +0530, Jassi Brar wrote:
>> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim <boojin.kim@samsung.com> wrote:
>>
>> > +
>> > +static bool pl330_filter(struct dma_chan *chan, void *param)
>> > +{
>> > +       struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan->private;
>> > +       unsigned dma_ch = (unsigned)param;
>> > +
>> > +       if (peri->peri_id != dma_ch)
>> > +               return false;
>> > +
>> > +       return true;
>> > +}
>> This is what I meant... if we keep chan_id for paltform assigned IDs,
>> these filter functions could simply become
>>
>> static bool pl330_filter(struct dma_chan *chan, void *param)
>> {
>>         return chan->chan_id == param
>> }
>>
>> And ideally in the long run, we could just drop the filter callback
>> and add expected channel ID to the request_channel call.
>
> So what if you have a PL080 and PL330 drivers registered with the DMA
> engine code, and you need to match a specific PL330 channel?
>

Before we start, let me be clear that I don't say it's all possible
today as such.
We will need changes to the API and modifications to the damc/client
drivers (which are already misusing the API as it turns out).

I hope we agree, clients (esp 'generic' ones of the future)
should not ask for channels from a particular dmac.
They should simply tell DMA API, what capabilities they expect
from the allocated channel - of whatever index on whichever dmac.
And the platform should have specified capabilities of a channel
while introducing it to the DMA API.   Yes, imho, DMA API should
only work with a pool of channels and not dmacs - it shouldn't
matter from which dmac a channel comes.

So when a client asks for a channel, the dmaengine would return
the first free channel that has just as much the requested capability.

That is, the generic filter function would look equivalent of :-

      return is_free(chan) && (cap_requested == cap_of(chan) & cap_requested)


Now it all boils down to defining the set of _practically_ possible capabilities
and a method to exchange them between the API, Client and DMAC drivers.

The above is the overall approach.

The following is my idea of implementing it as of now, I am open to suggestions.

Assuming :-
***********
There are no more than 256 types of DMA'able devices
 (MMC, USB, SPI etc) --  [8bits]

A type of device never has more than 16 instances in a platform
 (MMC-0, MMC-1, MMC-2 etc) --  [4bits]

Mem->Mem, Mem->Dev, Dev->Mem, Dev->Dev capability in [4bits]

Max_Burst_Len for any channel is less than 64KB, in power of two [4bits]
Support programmable Max_Burst_Len(tunable in geometric steps of 2)  [1bit]

Max_RW_width/fifo_width is less than 128, in power of two -- [3bits]
Support programmable Max_RW_Width(tunable in geometric steps of 2)  [1bit]

Finally, No platform has more than 128 channels with identicial
capabilities [7bits]


For a channel, when specific values of these fields are appended together,
we get a unqiue 32-bits value called 'chan_id'

Though we can use 64-bits to store the capabilities if we think number
or size of these fields need to be increased.

Thanks,
Jassi
Russell King - ARM Linux July 26, 2011, 6:14 p.m. UTC | #7
On Tue, Jul 26, 2011 at 11:13:35PM +0530, Jassi Brar wrote:
> Before we start, let me be clear that I don't say it's all possible
> today as such.

It _is_ possible today.

1. Arrange for individual DMA engine drivers to provide a filter
   function - eg, pl08x_filter_id() for pl08x channels.

2. Have the filter function check chan->device->dev->driver == its
   own struct driver as the very first thing, and reject on failure.

3. Have its own matching algorithm using whatever data is appropriate
   for the DMA engine driver.

4. Platforms (or SoC code) provide the match function plus match data
   via platform data to each driver.

That is essentially (except for (2)) how we deal with differing DMA
engine drivers with AMBA peripherals, and it seems to work remarkably
well.

> I hope we agree, clients (esp 'generic' ones of the future)
> should not ask for channels from a particular dmac.
> They should simply tell DMA API, what capabilities they expect
> from the allocated channel - of whatever index on whichever dmac.
> And the platform should have specified capabilities of a channel
> while introducing it to the DMA API.   Yes, imho, DMA API should
> only work with a pool of channels and not dmacs - it shouldn't
> matter from which dmac a channel comes.

You're quite clearly confused.  "DMA API" has not very much to do with
the subject we're discussing.  The "DMA API" is the API for mapping and
unmapping buffers for DMA.  It has not much to do with the DMA engine
API.  So I'm not going to bother trying to decode what you said above
until you start using the right terminology.  I'm a stickler for that,
get used to it.

No point talking about how round bananas are when you're actually
discussing orange fruit.

> So when a client asks for a channel, the dmaengine would return
> the first free channel that has just as much the requested capability.

No.  For slave DMA, peripheral drivers normally need a specific
channel on the DMA controller.  Peripheral drivers normally do not
know which channel they need, or even which DMA controller they
require.  That information is specific to the platform and SoC.

So, returning the "first free channel that matches the capability"
is senseless - insane - for slave DMA.

> That is, the generic filter function would look equivalent of :-
> 
>       return is_free(chan) && (cap_requested == cap_of(chan) & cap_requested)
> 
> 
> Now it all boils down to defining the set of _practically_ possible capabilities
> and a method to exchange them between the API, Client and DMAC drivers.

What are these capabilities?

> The above is the overall approach.
> 
> The following is my idea of implementing it as of now, I am open to suggestions.
> 
> Assuming :-
> ***********
> There are no more than 256 types of DMA'able devices
>  (MMC, USB, SPI etc) --  [8bits]
> 
> A type of device never has more than 16 instances in a platform
>  (MMC-0, MMC-1, MMC-2 etc) --  [4bits]
> 
> Mem->Mem, Mem->Dev, Dev->Mem, Dev->Dev capability in [4bits]

No.  You're being far too specific to your own DMA controller.  Many
DMA controllers (eg, PL08x) have a number of physical channels, all of
which can be programmed for M2M, M2P, P2M and P2P.  There is nothing
in the design which restricts what a channel can do.

So to put that as the basis for a generic match algorithm is absolutely
senseless.

> Max_Burst_Len for any channel is less than 64KB, in power of two [4bits]
> Support programmable Max_Burst_Len(tunable in geometric steps of 2)  [1bit]
> 
> Max_RW_width/fifo_width is less than 128, in power of two -- [3bits]
> Support programmable Max_RW_Width(tunable in geometric steps of 2)  [1bit]
> 
> Finally, No platform has more than 128 channels with identicial
> capabilities [7bits]

There is absolutely no way that this will work in reality - are you on
a different planet?  As has already been described to you, MMCI can
use one DMA channel, and programs it according to the DMA direction,
and has in the past adjusted the burst size according to what's going
on with the driver.

With your model, it would have to claim N channels from the DMA
engine for all possible different configurations.

That's utterly unworkable and unrealistic.

So no, I don't think your idea of 'capabilities' is any solution.  It
just creates yet more problems - more than it solves.
Jassi Brar July 26, 2011, 7:54 p.m. UTC | #8
On Tue, Jul 26, 2011 at 11:44 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

>
> 1. Arrange for individual DMA engine drivers to provide a filter
>   function - eg, pl08x_filter_id() for pl08x channels.
>
> 2. Have the filter function check chan->device->dev->driver == its
>   own struct driver as the very first thing, and reject on failure.
>
> 3. Have its own matching algorithm using whatever data is appropriate
>   for the DMA engine driver.
IMHO it should be possible to have client drivers not use platform specific
details while choosing a channel. Otherwise, they are not really 'generic'
'platform-independent' client drivers.


>> I hope we agree, clients (esp 'generic' ones of the future)
>> should not ask for channels from a particular dmac.
>> They should simply tell DMA API, what capabilities they expect
>> from the allocated channel - of whatever index on whichever dmac.
>> And the platform should have specified capabilities of a channel
>> while introducing it to the DMA API.   Yes, imho, DMA API should
>> only work with a pool of channels and not dmacs - it shouldn't
>> matter from which dmac a channel comes.
>
> You're quite clearly confused.  "DMA API" has not very much to do with
> the subject we're discussing.  The "DMA API" is the API for mapping and
> unmapping buffers for DMA.  It has not much to do with the DMA engine
> API.  So I'm not going to bother trying to decode what you said above
> until you start using the right terminology.  I'm a stickler for that,
> get used to it.
>
> No point talking about how round bananas are when you're actually
> discussing orange fruit.
Sorry, my carelessness.
I should have written DMAENGINE API, instead of DMA API.

If you want I can repost with s/DMA API/DMAENGINE API/

>
>> So when a client asks for a channel, the dmaengine would return
>> the first free channel that has just as much the requested capability.
>
> No.  For slave DMA, peripheral drivers normally need a specific
> channel on the DMA controller.
Of course they do.
And It is the responsibility of platform to encode such information as a
'capability' of the channel.  Capability to reach a particular peripheral, say.

>  Peripheral drivers normally do not
> know which channel they need, or even which DMA controller they
> require.  That information is specific to the platform and SoC.
For that very reason I proposed, say, if MMC client driver is probed
for 2nd instance
of MMC controller, it should ask for the following capabilities
    (MMC << TYPE_SHFT) | (2 << INST_SHFT)    //I don't show other
fields for simplicity

It is the job of _platform_ to set TYPE and INSTANCE fields to
'MMC' and 2 resp _only_ for the channel(s) that can reach its MMC_2 controller.

That way, when the client 'generically' asks for DMA_MMC_2, it will be given
only one of those channels that can talk to second instance of MMC controller.


>> That is, the generic filter function would look equivalent of :-
>>
>>       return is_free(chan) && (cap_requested == cap_of(chan) & cap_requested)
>>
>>
>> Now it all boils down to defining the set of _practically_ possible capabilities
>> and a method to exchange them between the API, Client and DMAC drivers.
>
> What are these capabilities?
I had mentioned the list below under 'Assuming'.  We may add more.

>
>> The above is the overall approach.
>>
>> The following is my idea of implementing it as of now, I am open to suggestions.
>>
>> Assuming :-
>> ***********
>> There are no more than 256 types of DMA'able devices
>>  (MMC, USB, SPI etc) --  [8bits]
>>
>> A type of device never has more than 16 instances in a platform
>>  (MMC-0, MMC-1, MMC-2 etc) --  [4bits]
>>
>> Mem->Mem, Mem->Dev, Dev->Mem, Dev->Dev capability in [4bits]
>
> No.  You're being far too specific to your own DMA controller.  Many
> DMA controllers (eg, PL08x) have a number of physical channels, all of
> which can be programmed for M2M, M2P, P2M and P2P.  There is nothing
> in the design which restricts what a channel can do.
I am not being specific, at least not on this point. Btw, I have worked on pl080
as well and I have tried to be not specific to not only pl330 but also pl080.

If you notice I have assigned 4bits to the field. Each of the 4 bits can be set
independently.
So, a capable pl08x channel can set all 4 bits to indicate that it can
do all 4 types
 - M2M, M2P, P2M and P2P.


>> Max_Burst_Len for any channel is less than 64KB, in power of two [4bits]
>> Support programmable Max_Burst_Len(tunable in geometric steps of 2)  [1bit]
>>
>> Max_RW_width/fifo_width is less than 128, in power of two -- [3bits]
>> Support programmable Max_RW_Width(tunable in geometric steps of 2)  [1bit]
>>
>> Finally, No platform has more than 128 channels with identicial
>> capabilities [7bits]
>
> There is absolutely no way that this will work in reality - are you on
> a different planet?  As has already been described to you, MMCI can
> use one DMA channel, and programs it according to the DMA direction,
> and has in the past adjusted the burst size according to what's going
> on with the driver.
>
> With your model, it would have to claim N channels from the DMA
> engine for all possible different configurations.
I don't think so.
What I proposed is only a list of capabilities and method to _select_ a suitable
channel in a platform independent way.

For example, if MMCI driver wants to be able to change directions, it should
specify Mem->Dev and Dev->Mem capability while requesting a channel.
And use slave_config call to switch directions. (and yes we have to modify
slave_config call and maybe some other parts too)

For being able to change burst size, it could set some 'ignore' bit +
default value
to the 'fifo_width' field so that DMAENGINE ignores checking the fifo_width
capability and upon slave_config either set requested burst_size if supported
or return error if the requested burst size is not supported.
That is implementation detail.

Sincerely, I would to know other capabilities that are more flexible than I have
assumed here or missed altogether.

I have not yet written any code. I just wanted to share my belief that
it is possible
to have client drivers truly independent of the platform they run on.
And by not using platform_data of DMA channels.

Probably you don't, but if you do want, I can send rudimentary implementation
of what I propose, in a few days.

Thanks
Jassi Brar July 27, 2011, 1:33 a.m. UTC | #9
On Tue, Jul 26, 2011 at 3:05 PM, Boojin Kim <boojin.kim@samsung.com> wrote:
> Jassi Brar Wrote:
>> Sent: Monday, July 25, 2011 8:52 PM
>> To: Boojin Kim
>> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-
>> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant
>> Likely; Mark Brown
>> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations
>>
>> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim <boojin.kim@samsung.com>
>> wrote:
>>
>> > +
>> > +static bool pl330_filter(struct dma_chan *chan, void *param)
>> > +{
>> > +       struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan-
>> >private;
>> > +       unsigned dma_ch = (unsigned)param;
>> > +
>> > +       if (peri->peri_id != dma_ch)
>> > +               return false;
>> > +
>> > +       return true;
>> > +}
>> This is what I meant... if we keep chan_id for paltform assigned IDs,
>> these filter functions could simply become
>>
>> static bool pl330_filter(struct dma_chan *chan, void *param)
>> {
>>         return chan->chan_id == param
>> }
>>
>> And ideally in the long run, we could just drop the filter callback
>> and add expected channel ID to the request_channel call.
> The chan_id is set by dmaengine. So, We don't use it to hold the user specific
> Id.
Not for long.
See  https://lkml.org/lkml/2011/7/26/245


>> > +
>> > +static inline int s3c_dma_trigger(unsigned ch)
>> > +{
>> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START);
>> > +}
>> > +
>> > +static inline int s3c_dma_started(unsigned ch)
>> > +{
>> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED);
>> > +}
>> > +
>> > +static inline int s3c_dma_flush(unsigned ch)
>> > +{
>> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH);
>> > +}
>> > +
>> > +static inline int s3c_dma_stop(unsigned ch)
>> > +{
>> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP);
>> > +}
>> > +
>> > +static struct samsung_dma_ops s3c_dma_ops = {
>> > +       .request        = s3c_dma_request,
>> > +       .release        = s3c_dma_release,
>> > +       .prepare        = s3c_dma_prepare,
>> > +       .trigger        = s3c_dma_trigger,
>> > +       .started        = s3c_dma_started,
>> > +       .flush          = s3c_dma_flush,
>> > +       .stop           = s3c_dma_stop,
>>
>> These last 4 should be gnereallized into one callback with OP argument.
> I don't have any idea about it. Can you explain it in more detail?

static int s3c_dma_control(unsigned ch, enum s3c2410_chan_op/*or similar*/ op)
 {
        return s3c2410_dma_ctrl(ch, op);
 }

static struct samsung_dma_ops s3c_dma_ops = {
        .request        = s3c_dma_request,
        .release        = s3c_dma_release,
        .prepare        = s3c_dma_prepare,
        .control        = s3c_dma_control,
boojin.kim July 27, 2011, 5:17 a.m. UTC | #10
Jassi Brar wrote:
> Sent: Wednesday, July 27, 2011 10:34 AM
> To: Boojin Kim
> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant
> Likely; Mark Brown
> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations
>
> On Tue, Jul 26, 2011 at 3:05 PM, Boojin Kim <boojin.kim@samsung.com>
> wrote:
> > Jassi Brar Wrote:
> >> Sent: Monday, July 25, 2011 8:52 PM
> >> To: Boojin Kim
> >> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-
> >> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant
> >> Likely; Mark Brown
> >> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA
> operations
> >>
> >> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim
> <boojin.kim@samsung.com>
> >> wrote:
> >>
> >> > +
> >> > +static bool pl330_filter(struct dma_chan *chan, void *param)
> >> > +{
> >> > +       struct dma_pl330_peri *peri = (struct dma_pl330_peri
> *)chan-
> >> >private;
> >> > +       unsigned dma_ch = (unsigned)param;
> >> > +
> >> > +       if (peri->peri_id != dma_ch)
> >> > +               return false;
> >> > +
> >> > +       return true;
> >> > +}
> >> This is what I meant... if we keep chan_id for paltform assigned
> IDs,
> >> these filter functions could simply become
> >>
> >> static bool pl330_filter(struct dma_chan *chan, void *param)
> >> {
> >>         return chan->chan_id == param
> >> }
> >>
> >> And ideally in the long run, we could just drop the filter callback
> >> and add expected channel ID to the request_channel call.
> > The chan_id is set by dmaengine. So, We don't use it to hold the
> user specific
> > Id.
> Not for long.
> See  https://lkml.org/lkml/2011/7/26/245
>
>
> >> > +
> >> > +static inline int s3c_dma_trigger(unsigned ch)
> >> > +{
> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START);
> >> > +}
> >> > +
> >> > +static inline int s3c_dma_started(unsigned ch)
> >> > +{
> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED);
> >> > +}
> >> > +
> >> > +static inline int s3c_dma_flush(unsigned ch)
> >> > +{
> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH);
> >> > +}
> >> > +
> >> > +static inline int s3c_dma_stop(unsigned ch)
> >> > +{
> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP);
> >> > +}
> >> > +
> >> > +static struct samsung_dma_ops s3c_dma_ops = {
> >> > +       .request        = s3c_dma_request,
> >> > +       .release        = s3c_dma_release,
> >> > +       .prepare        = s3c_dma_prepare,
> >> > +       .trigger        = s3c_dma_trigger,
> >> > +       .started        = s3c_dma_started,
> >> > +       .flush          = s3c_dma_flush,
> >> > +       .stop           = s3c_dma_stop,
> >>
> >> These last 4 should be gnereallized into one callback with OP
> argument.
> > I don't have any idea about it. Can you explain it in more detail?
>
> static int s3c_dma_control(unsigned ch, enum s3c2410_chan_op/*or
> similar*/ op)
>  {
>         return s3c2410_dma_ctrl(ch, op);
>  }
>
> static struct samsung_dma_ops s3c_dma_ops = {
>         .request        = s3c_dma_request,
>         .release        = s3c_dma_release,
>         .prepare        = s3c_dma_prepare,
>         .control        = s3c_dma_control,
'Struct samsung_dma_ops' is used for both 'S3C-DMA-OPS' for 's3c dma' and 
'DMA-OPS' for 'dmaengine'.
If I modify "Struct samsung_dma_ops" as your guide, It gives un-expectable 
effect to DMA-OPS.
Actually, We don't want the client with 'DMA-OPS' uses 'enum s3c2410_chan_op' 
operation anymore.
Jassi Brar July 27, 2011, 7:57 a.m. UTC | #11
On Wed, Jul 27, 2011 at 10:47 AM, Boojin Kim <boojin.kim@samsung.com> wrote:
> Jassi Brar wrote:
>> Sent: Wednesday, July 27, 2011 10:34 AM
>> To: Boojin Kim
>> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-
>> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant
>> Likely; Mark Brown
>> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations
>>
>> On Tue, Jul 26, 2011 at 3:05 PM, Boojin Kim <boojin.kim@samsung.com>
>> wrote:
>> > Jassi Brar Wrote:
>> >> Sent: Monday, July 25, 2011 8:52 PM
>> >> To: Boojin Kim
>> >> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-
>> >> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant
>> >> Likely; Mark Brown
>> >> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA
>> operations
>> >>
>> >> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim
>> <boojin.kim@samsung.com>
>> >> wrote:
>> >>
>> >> > +
>> >> > +static bool pl330_filter(struct dma_chan *chan, void *param)
>> >> > +{
>> >> > +       struct dma_pl330_peri *peri = (struct dma_pl330_peri
>> *)chan-
>> >> >private;
>> >> > +       unsigned dma_ch = (unsigned)param;
>> >> > +
>> >> > +       if (peri->peri_id != dma_ch)
>> >> > +               return false;
>> >> > +
>> >> > +       return true;
>> >> > +}
>> >> This is what I meant... if we keep chan_id for paltform assigned
>> IDs,
>> >> these filter functions could simply become
>> >>
>> >> static bool pl330_filter(struct dma_chan *chan, void *param)
>> >> {
>> >>         return chan->chan_id == param
>> >> }
>> >>
>> >> And ideally in the long run, we could just drop the filter callback
>> >> and add expected channel ID to the request_channel call.
>> > The chan_id is set by dmaengine. So, We don't use it to hold the
>> user specific
>> > Id.
>> Not for long.
>> See  https://lkml.org/lkml/2011/7/26/245
>>
>>
>> >> > +
>> >> > +static inline int s3c_dma_trigger(unsigned ch)
>> >> > +{
>> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START);
>> >> > +}
>> >> > +
>> >> > +static inline int s3c_dma_started(unsigned ch)
>> >> > +{
>> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED);
>> >> > +}
>> >> > +
>> >> > +static inline int s3c_dma_flush(unsigned ch)
>> >> > +{
>> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH);
>> >> > +}
>> >> > +
>> >> > +static inline int s3c_dma_stop(unsigned ch)
>> >> > +{
>> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP);
>> >> > +}
>> >> > +
>> >> > +static struct samsung_dma_ops s3c_dma_ops = {
>> >> > +       .request        = s3c_dma_request,
>> >> > +       .release        = s3c_dma_release,
>> >> > +       .prepare        = s3c_dma_prepare,
>> >> > +       .trigger        = s3c_dma_trigger,
>> >> > +       .started        = s3c_dma_started,
>> >> > +       .flush          = s3c_dma_flush,
>> >> > +       .stop           = s3c_dma_stop,
>> >>
>> >> These last 4 should be gnereallized into one callback with OP
>> argument.
>> > I don't have any idea about it. Can you explain it in more detail?
>>
>> static int s3c_dma_control(unsigned ch, enum s3c2410_chan_op/*or
>> similar*/ op)
>>  {
>>         return s3c2410_dma_ctrl(ch, op);
>>  }
>>
>> static struct samsung_dma_ops s3c_dma_ops = {
>>         .request        = s3c_dma_request,
>>         .release        = s3c_dma_release,
>>         .prepare        = s3c_dma_prepare,
>>         .control        = s3c_dma_control,
> 'Struct samsung_dma_ops' is used for both 'S3C-DMA-OPS' for 's3c dma' and
> 'DMA-OPS' for 'dmaengine'.
> If I modify "Struct samsung_dma_ops" as your guide, It gives un-expectable
> effect to DMA-OPS.
> Actually, We don't want the client with 'DMA-OPS' uses 'enum s3c2410_chan_op'
> operation anymore.

" enum s3c2410_chan_op /* or similar */ "  <<<<  note 'or similar'

Defining a callback for each value of the argument doesn't make sense.
boojin.kim July 28, 2011, 12:38 a.m. UTC | #12
Jassi Brar wrote:
> Sent: Wednesday, July 27, 2011 4:58 PM
> To: Boojin Kim
> Cc: Kukjin Kim; Vinod Koul; Mark Brown; Grant Likely; linux-samsung-
> soc@vger.kernel.org; Dan Williams; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations
>
> On Wed, Jul 27, 2011 at 10:47 AM, Boojin Kim <boojin.kim@samsung.com>
> wrote:
> > Jassi Brar wrote:
> >> Sent: Wednesday, July 27, 2011 10:34 AM
> >> To: Boojin Kim
> >> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-
> >> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant
> >> Likely; Mark Brown
> >> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA
> operations
> >>
> >> On Tue, Jul 26, 2011 at 3:05 PM, Boojin Kim
> <boojin.kim@samsung.com>
> >> wrote:
> >> > Jassi Brar Wrote:
> >> >> Sent: Monday, July 25, 2011 8:52 PM
> >> >> To: Boojin Kim
> >> >> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-
> >> >> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant
> >> >> Likely; Mark Brown
> >> >> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA
> >> operations
> >> >>
> >> >> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim
> >> <boojin.kim@samsung.com>
> >> >> wrote:
> >> >>
> >> >> > +
> >> >> > +static bool pl330_filter(struct dma_chan *chan, void *param)
> >> >> > +{
> >> >> > +       struct dma_pl330_peri *peri = (struct dma_pl330_peri
> >> *)chan-
> >> >> >private;
> >> >> > +       unsigned dma_ch = (unsigned)param;
> >> >> > +
> >> >> > +       if (peri->peri_id != dma_ch)
> >> >> > +               return false;
> >> >> > +
> >> >> > +       return true;
> >> >> > +}
> >> >> This is what I meant... if we keep chan_id for paltform assigned
> >> IDs,
> >> >> these filter functions could simply become
> >> >>
> >> >> static bool pl330_filter(struct dma_chan *chan, void *param)
> >> >> {
> >> >>         return chan->chan_id == param
> >> >> }
> >> >>
> >> >> And ideally in the long run, we could just drop the filter
> callback
> >> >> and add expected channel ID to the request_channel call.
> >> > The chan_id is set by dmaengine. So, We don't use it to hold the
> >> user specific
> >> > Id.
> >> Not for long.
> >> See  https://lkml.org/lkml/2011/7/26/245
> >>
> >>
> >> >> > +
> >> >> > +static inline int s3c_dma_trigger(unsigned ch)
> >> >> > +{
> >> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START);
> >> >> > +}
> >> >> > +
> >> >> > +static inline int s3c_dma_started(unsigned ch)
> >> >> > +{
> >> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED);
> >> >> > +}
> >> >> > +
> >> >> > +static inline int s3c_dma_flush(unsigned ch)
> >> >> > +{
> >> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH);
> >> >> > +}
> >> >> > +
> >> >> > +static inline int s3c_dma_stop(unsigned ch)
> >> >> > +{
> >> >> > +       return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP);
> >> >> > +}
> >> >> > +
> >> >> > +static struct samsung_dma_ops s3c_dma_ops = {
> >> >> > +       .request        = s3c_dma_request,
> >> >> > +       .release        = s3c_dma_release,
> >> >> > +       .prepare        = s3c_dma_prepare,
> >> >> > +       .trigger        = s3c_dma_trigger,
> >> >> > +       .started        = s3c_dma_started,
> >> >> > +       .flush          = s3c_dma_flush,
> >> >> > +       .stop           = s3c_dma_stop,
> >> >>
> >> >> These last 4 should be gnereallized into one callback with OP
> >> argument.
> >> > I don't have any idea about it. Can you explain it in more detail?
> >>
> >> static int s3c_dma_control(unsigned ch, enum s3c2410_chan_op/*or
> >> similar*/ op)
> >>  {
> >>         return s3c2410_dma_ctrl(ch, op);
> >>  }
> >>
> >> static struct samsung_dma_ops s3c_dma_ops = {
> >>         .request        = s3c_dma_request,
> >>         .release        = s3c_dma_release,
> >>         .prepare        = s3c_dma_prepare,
> >>         .control        = s3c_dma_control,
> > 'Struct samsung_dma_ops' is used for both 'S3C-DMA-OPS' for 's3c
> dma' and
> > 'DMA-OPS' for 'dmaengine'.
> > If I modify "Struct samsung_dma_ops" as your guide, It gives un-
> expectable
> > effect to DMA-OPS.
> > Actually, We don't want the client with 'DMA-OPS' uses 'enum
> s3c2410_chan_op'
> > operation anymore.
>
> " enum s3c2410_chan_op /* or similar */ "  <<<<  note 'or similar'
>
> Defining a callback for each value of the argument doesn't make sense.
Yes, The command may not be 'enum s3c2410_chan_op'. But it's very similar with 
'enum s3c2410_chan_op'.
So, It brings same result that I sad.
Actually, I quietly agree with your comment. If the 'struct samsung_dma_ops' 
is only used for 's3c-dma-ops', I would address your comment. But, 'struct 
samsung_dma_ops' is used for all Samsung dma clients. I aim that 'struct 
samsung_dma_ops' provide the DMA clients with 'dmaengine' friendly DMA 
interface without any other command.

Patch
diff mbox

diff --git a/arch/arm/mach-s3c2410/include/mach/dma.h b/arch/arm/mach-s3c2410/include/mach/dma.h
index b2b2a5b..e61a91f 100644
--- a/arch/arm/mach-s3c2410/include/mach/dma.h
+++ b/arch/arm/mach-s3c2410/include/mach/dma.h
@@ -13,7 +13,6 @@ 
 #ifndef __ASM_ARCH_DMA_H
 #define __ASM_ARCH_DMA_H __FILE__
 
-#include <plat/dma.h>
 #include <linux/sysdev.h>
 
 #define MAX_DMA_TRANSFER_SIZE   0x100000 /* Data Unit is half word  */
@@ -51,6 +50,13 @@  enum dma_ch {
 	DMACH_MAX,		/* the end entry */
 };
 
+static inline bool samsung_dma_is_dmadev(void)
+{
+	return false;
+}
+
+#include <plat/dma.h>
+
 #define DMACH_LOW_LEVEL	(1<<28)	/* use this to specifiy hardware ch no */
 
 /* we have 4 dma channels */
diff --git a/arch/arm/mach-s3c64xx/include/mach/dma.h b/arch/arm/mach-s3c64xx/include/mach/dma.h
index 0a5d926..49c3a53 100644
--- a/arch/arm/mach-s3c64xx/include/mach/dma.h
+++ b/arch/arm/mach-s3c64xx/include/mach/dma.h
@@ -63,6 +63,10 @@  static __inline__ bool s3c_dma_has_circular(void)
 	return true;
 }
 
+static inline bool samsung_dma_is_dmadev(void)
+{
+	return false;
+}
 #define S3C2410_DMAF_CIRCULAR		(1 << 0)
 
 #include <plat/dma.h>
diff --git a/arch/arm/plat-samsung/Makefile b/arch/arm/plat-samsung/Makefile
index 53eb15b..9ecf2aa 100644
--- a/arch/arm/plat-samsung/Makefile
+++ b/arch/arm/plat-samsung/Makefile
@@ -62,9 +62,11 @@  obj-$(CONFIG_SAMSUNG_DEV_PWM)	+= dev-pwm.o
 
 # DMA support
 
-obj-$(CONFIG_S3C_DMA)		+= dma.o
+obj-$(CONFIG_S3C_DMA)		+= dma.o s3c-dma-ops.o
 
-obj-$(CONFIG_S3C_PL330_DMA)	+= s3c-pl330.o
+obj-$(CONFIG_DMADEV_PL330)	+= dma-ops.o
+
+obj-$(CONFIG_S3C_PL330_DMA)	+= s3c-pl330.o s3c-dma-ops.o
 
 # PM support
 
diff --git a/arch/arm/plat-samsung/dma-ops.c b/arch/arm/plat-samsung/dma-ops.c
new file mode 100644
index 0000000..9053433
--- /dev/null
+++ b/arch/arm/plat-samsung/dma-ops.c
@@ -0,0 +1,133 @@ 
+/* linux/arch/arm/plat-samsung/dma-ops.c
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Samsung DMA Operations
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/amba/pl330.h>
+
+#include <mach/dma.h>
+
+static bool pl330_filter(struct dma_chan *chan, void *param)
+{
+	struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan->private;
+	unsigned dma_ch = (unsigned)param;
+
+	if (peri->peri_id != dma_ch)
+		return false;
+
+	return true;
+}
+
+static unsigned samsung_dmadev_request(enum dma_ch dma_ch,
+				struct samsung_dma_info *info)
+{
+	struct dma_chan *chan;
+	dma_cap_mask_t mask;
+	struct dma_slave_config slave_config;
+
+	dma_cap_zero(mask);
+	dma_cap_set(info->cap, mask);
+
+	chan = dma_request_channel(mask, pl330_filter, (void *)dma_ch);
+
+	if (info->direction == DMA_FROM_DEVICE) {
+		memset(&slave_config, 0, sizeof(struct dma_slave_config));
+		slave_config.direction = info->direction;
+		slave_config.src_addr = info->fifo;
+		slave_config.src_addr_width = info->width;
+		dmaengine_slave_config(chan, &slave_config);
+	} else if (info->direction == DMA_TO_DEVICE) {
+		memset(&slave_config, 0, sizeof(struct dma_slave_config));
+		slave_config.direction = info->direction;
+		slave_config.dst_addr = info->fifo;
+		slave_config.dst_addr_width = info->width;
+		dmaengine_slave_config(chan, &slave_config);
+	}
+
+	return (unsigned)chan;
+}
+
+static int samsung_dmadev_release(unsigned ch,
+			struct s3c2410_dma_client *client)
+{
+	dma_release_channel((struct dma_chan *)ch);
+
+	return 0;
+}
+
+static int samsung_dmadev_prepare(unsigned ch,
+			struct samsung_dma_prep_info *info)
+{
+	struct scatterlist sg;
+	struct dma_chan *chan = (struct dma_chan *)ch;
+	struct dma_async_tx_descriptor *desc;
+
+	switch (info->cap) {
+	case DMA_SLAVE:
+		sg_init_table(&sg, 1);
+		sg_dma_len(&sg) = info->len;
+		sg_set_page(&sg, pfn_to_page(PFN_DOWN(info->buf)),
+			    info->len, offset_in_page(info->buf));
+		sg_dma_address(&sg) = info->buf;
+
+		desc = chan->device->device_prep_slave_sg(chan,
+			&sg, 1, info->direction, DMA_PREP_INTERRUPT);
+		break;
+	case DMA_CYCLIC:
+		desc = chan->device->device_prep_dma_cyclic(chan,
+			info->buf, info->len, info->period, info->direction);
+		break;
+	default:
+		dev_err(&chan->dev->device, "unsupported format\n");
+		return -EFAULT;
+	}
+
+	if (!desc) {
+		dev_err(&chan->dev->device, "cannot prepare cyclic dma\n");
+		return -EFAULT;
+	}
+
+	desc->callback = info->fp;
+	desc->callback_param = info->fp_param;
+
+	dmaengine_submit((struct dma_async_tx_descriptor *)desc);
+
+	return 0;
+}
+
+static inline int samsung_dmadev_trigger(unsigned ch)
+{
+	dma_async_issue_pending((struct dma_chan *)ch);
+
+	return 0;
+}
+
+static inline int samsung_dmadev_flush(unsigned ch)
+{
+	return dmaengine_terminate_all((struct dma_chan *)ch);
+}
+
+struct samsung_dma_ops dmadev_ops = {
+	.request	= samsung_dmadev_request,
+	.release	= samsung_dmadev_release,
+	.prepare	= samsung_dmadev_prepare,
+	.trigger	= samsung_dmadev_trigger,
+	.started	= NULL,
+	.flush		= samsung_dmadev_flush,
+	.stop		= samsung_dmadev_flush,
+};
+
+void *samsung_dmadev_get_ops(void)
+{
+	return &dmadev_ops;
+}
+EXPORT_SYMBOL(samsung_dmadev_get_ops);
diff --git a/arch/arm/plat-samsung/include/plat/dma-ops.h b/arch/arm/plat-samsung/include/plat/dma-ops.h
new file mode 100644
index 0000000..e5a68c0
--- /dev/null
+++ b/arch/arm/plat-samsung/include/plat/dma-ops.h
@@ -0,0 +1,64 @@ 
+/* arch/arm/plat-samsung/include/plat/dma-ops.h
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Samsung DMA support
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __SAMSUNG_DMA_OPS_H_
+#define __SAMSUNG_DMA_OPS_H_ __FILE__
+
+#include <linux/dmaengine.h>
+
+struct samsung_dma_prep_info {
+	enum dma_transaction_type cap;
+	enum dma_data_direction direction;
+	unsigned buf;
+	unsigned long period;
+	unsigned long len;
+	void (*fp)(void *data);
+	void *fp_param;
+};
+
+struct samsung_dma_info {
+	enum dma_transaction_type cap;
+	enum dma_data_direction direction;
+	enum dma_slave_buswidth width;
+	unsigned fifo;
+	struct s3c2410_dma_client *client;
+};
+
+struct samsung_dma_ops {
+	bool init;
+	unsigned (*request)(enum dma_ch ch, struct samsung_dma_info *info);
+	int (*release)(unsigned ch, struct s3c2410_dma_client *client);
+	int (*prepare)(unsigned ch, struct samsung_dma_prep_info *info);
+	int (*trigger)(unsigned ch);
+	int (*started)(unsigned ch);
+	int (*flush)(unsigned ch);
+	int (*stop)(unsigned ch);
+};
+
+extern void *samsung_dmadev_get_ops(void);
+extern void *s3c_dma_get_ops(void);
+
+static inline void *__samsung_dma_get_ops(void)
+{
+	if (samsung_dma_is_dmadev())
+		return samsung_dmadev_get_ops();
+	else
+		return s3c_dma_get_ops();
+}
+
+/*
+ * samsung_dma_get_ops
+ * get the set of samsung dma operations
+ */
+#define samsung_dma_get_ops() __samsung_dma_get_ops()
+
+#endif /* __SAMSUNG_DMA_OPS_H_ */
diff --git a/arch/arm/plat-samsung/include/plat/dma-pl330.h b/arch/arm/plat-samsung/include/plat/dma-pl330.h
index c402719..687489e 100644
--- a/arch/arm/plat-samsung/include/plat/dma-pl330.h
+++ b/arch/arm/plat-samsung/include/plat/dma-pl330.h
@@ -100,6 +100,10 @@  static inline bool s3c_dma_has_circular(void)
 	return true;
 }
 
+static inline bool samsung_dma_is_dmadev(void)
+{
+	return true;
+}
 #include <plat/dma.h>
 
 #endif	/* __DMA_PL330_H_ */
diff --git a/arch/arm/plat-samsung/include/plat/dma.h b/arch/arm/plat-samsung/include/plat/dma.h
index 2e8f8c6..e87c6be 100644
--- a/arch/arm/plat-samsung/include/plat/dma.h
+++ b/arch/arm/plat-samsung/include/plat/dma.h
@@ -125,3 +125,4 @@  extern int s3c2410_dma_set_opfn(unsigned int, s3c2410_dma_opfn_t rtn);
 extern int s3c2410_dma_set_buffdone_fn(unsigned int, s3c2410_dma_cbfn_t rtn);
 
 
+#include <plat/dma-ops.h>
diff --git a/arch/arm/plat-samsung/s3c-dma-ops.c b/arch/arm/plat-samsung/s3c-dma-ops.c
new file mode 100644
index 0000000..33ab324
--- /dev/null
+++ b/arch/arm/plat-samsung/s3c-dma-ops.c
@@ -0,0 +1,133 @@ 
+/* linux/arch/arm/plat-samsung/s3c-dma-ops.c
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Samsung S3C-DMA Operations
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include <mach/dma.h>
+
+struct cb_data {
+	void (*fp) (void *);
+	void *fp_param;
+	unsigned ch;
+	struct list_head node;
+};
+
+static LIST_HEAD(dma_list);
+
+static void s3c_dma_cb(struct s3c2410_dma_chan *channel, void *param,
+		       int size, enum s3c2410_dma_buffresult res)
+{
+	struct cb_data *data = param;
+
+	data->fp(data->fp_param);
+}
+
+static unsigned s3c_dma_request(enum dma_ch dma_ch,
+				 struct samsung_dma_info *info)
+{
+	struct cb_data *data;
+
+	if (s3c2410_dma_request(dma_ch, info->client, NULL) < 0) {
+		s3c2410_dma_free(dma_ch, info->client);
+		return 0;
+	}
+
+	data = kzalloc(sizeof(struct cb_data), GFP_KERNEL);
+	data->ch = dma_ch;
+	list_add_tail(&data->node, &dma_list);
+
+	if (info->direction == DMA_FROM_DEVICE)
+		s3c2410_dma_devconfig(dma_ch, S3C2410_DMASRC_HW, info->fifo);
+	else
+		s3c2410_dma_devconfig(dma_ch, S3C2410_DMASRC_MEM, info->fifo);
+
+	if (info->cap == DMA_CYCLIC)
+		s3c2410_dma_setflags(dma_ch, S3C2410_DMAF_CIRCULAR);
+
+	s3c2410_dma_config(dma_ch, info->width);
+
+	return (unsigned)dma_ch;
+}
+
+static int s3c_dma_release(unsigned ch, struct s3c2410_dma_client *client)
+{
+	struct cb_data *data;
+
+	list_for_each_entry(data, &dma_list, node)
+		if (data->ch == ch)
+			break;
+	list_del(&data->node);
+
+	s3c2410_dma_free(ch, client);
+	kfree(data);
+
+	return 0;
+}
+
+static int s3c_dma_prepare(unsigned ch, struct samsung_dma_prep_info *info)
+{
+	struct cb_data *data;
+	int len = (info->cap == DMA_CYCLIC) ? info->period : info->len;
+
+	list_for_each_entry(data, &dma_list, node)
+		if (data->ch == ch)
+			break;
+
+	if (!data->fp) {
+		s3c2410_dma_set_buffdone_fn(ch, s3c_dma_cb);
+		data->fp = info->fp;
+		data->fp_param = info->fp_param;
+	}
+
+	s3c2410_dma_enqueue(ch, (void *)data, info->buf, len);
+
+	return 0;
+}
+
+static inline int s3c_dma_trigger(unsigned ch)
+{
+	return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START);
+}
+
+static inline int s3c_dma_started(unsigned ch)
+{
+	return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED);
+}
+
+static inline int s3c_dma_flush(unsigned ch)
+{
+	return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH);
+}
+
+static inline int s3c_dma_stop(unsigned ch)
+{
+	return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP);
+}
+
+static struct samsung_dma_ops s3c_dma_ops = {
+	.request	= s3c_dma_request,
+	.release	= s3c_dma_release,
+	.prepare	= s3c_dma_prepare,
+	.trigger	= s3c_dma_trigger,
+	.started	= s3c_dma_started,
+	.flush		= s3c_dma_flush,
+	.stop		= s3c_dma_stop,
+};
+
+void *s3c_dma_get_ops(void)
+{
+	return &s3c_dma_ops;
+}
+EXPORT_SYMBOL(s3c_dma_get_ops);