No subject

from dma-mapping.h:
	enum dma_data_direction {
		DMA_BIDIRECTIONAL = 0,
		DMA_TO_DEVICE = 1,
		DMA_FROM_DEVICE = 2,
		DMA_NONE = 3,
	};

/me thinks it would all look nicer if s3c2410 dma just replaced
s3c2410_dmasrc to dma_data_direction, and from what I can tell it
would just be a simple search and replace.  :-)

> +
> +	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 dma_release(unsigned ch, struct s3c2410_dma_client *client)

unsigned int

> +{
> +	struct cb_data *data;
> +
> +	list_for_each_entry(data, &dma_list, node)
> +	    if (data->ch == ch)
> +		break;

nit: incorrect indentation.  Use tab characters instead of spaces.
I've got "set list" and "set listchars=tab:\|-,trail:-" in my ~/.vimrc
so I can see the difference between tabs and spaces.

> +	list_del(&data->node);

What happens if the channel isn't found in the list?  Can that
situation happen?

What happens if two drivers call dma_release simultaneously?  It
looks like a mutex is needed to protext the dma_list.

> +
> +	s3c2410_dma_free((enum dma_ch)ch, client);

All the casting in this patch makes me nervous.  When a lot of casting
is required, I wonder if the API needs to be changed.

> +	kfree(data);
> +
> +	return 0;
> +}
> +
> +static int dma_prepare(unsigned ch, struct samsung_dma_prep_info *info)
> +{
> +	struct cb_data *data;
> +
> +	list_for_each_entry(data, &dma_list, node)
> +	    if (data->ch == ch)
> +		break;
> +
> +	if (!data->fp) {
> +		s3c2410_dma_set_buffdone_fn((enum dma_ch)ch, dma_cb);
> +		data->fp = info->fp;
> +		data->fp_param = info->fp_param;
> +	}
> +	s3c2410_dma_enqueue((enum dma_ch)ch, (void *)data, info->buf,
> +			    info->period);
> +
> +	return 0;
> +}
> +
> +static inline int dma_trigger(unsigned ch)
> +{
> +	return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START);
> +}
> +
> +static inline int dma_started(unsigned ch)
> +{
> +	return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED);
> +}
> +
> +static inline int dma_flush(unsigned ch)
> +{
> +	return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH);
> +}
> +
> +static inline int dma_stop(unsigned ch)
> +{
> +	return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP);
> +}
> +
> +static struct samsung_dma_ops s3c_dma_ops = {
> +	.request	= dma_request,
> +	.release	= dma_release,
> +	.prepare	= dma_prepare,
> +	.trigger	= dma_trigger,
> +	.started	= dma_started,
> +	.flush		= dma_flush,
> +	.stop		= dma_stop,
> +};
> +
> +void *samsung_dma_get_ops(void)
> +{
> +	return (void *)&s3c_dma_ops;
> +}
> +EXPORT_SYMBOL(samsung_dma_get_ops);

This is a problem.  This patch adds two implementations of
samsung_dma_get_ops(). New code needs to be multiplatform friendly.
That means that the code nees to handle both dma-ops.c and
s3c-dma-ops.c compiled into the kernel at the same time and select the
correct one at runtime.

g.

> -- 
> 1.7.1
>