[2/3] dmaengine: add wrapper functions for dmaengine
diff mbox series

Message ID 1281956870-12463-3-git-send-email-s.hauer@pengutronix.de
State New, archived
Headers show
Series
  • [1/3] dmaengine: add possibility for cyclic transfers
Related show

Commit Message

Sascha Hauer Aug. 16, 2010, 11:07 a.m. UTC
Currently dmaengine users have to explicitely dereference function
pointers in struct dma_device. For the convenience of drivers and
to be more flexible when changing the dmaengine later add static
inline wrapper functions for the dma commands.

This patch is not complete yet. If there's consensus on this patch
I'll provide an updated patch with the missing functions.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/linux/dmaengine.h |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

Comments

Sascha Hauer Aug. 23, 2010, 7:17 a.m. UTC | #1
On Mon, Aug 16, 2010 at 01:07:49PM +0200, Sascha Hauer wrote:
> Currently dmaengine users have to explicitely dereference function
> pointers in struct dma_device. For the convenience of drivers and
> to be more flexible when changing the dmaengine later add static
> inline wrapper functions for the dma commands.
> 
> This patch is not complete yet. If there's consensus on this patch
> I'll provide an updated patch with the missing functions.

Dan,

Any comment on this one?

Sascha

> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  include/linux/dmaengine.h |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 0df7864..635c60b 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -491,6 +491,47 @@ struct dma_device {
>  	void (*device_issue_pending)(struct dma_chan *chan);
>  };
>  
> +static inline int dmaengine_device_control(struct dma_chan *chan,
> +					   enum dma_ctrl_cmd cmd,
> +					   unsigned long arg)
> +{
> +	return chan->device->device_control(chan, cmd, arg);
> +}
> +
> +static inline int dmaengine_slave_config(struct dma_chan *chan,
> +					  struct dma_slave_config *config)
> +{
> +	return dmaengine_device_control(chan, DMA_SLAVE_CONFIG,
> +			(unsigned long)config);
> +}
> +
> +static inline int dmaengine_terminate_all(struct dma_chan *chan)
> +{
> +	return dmaengine_device_control(chan, DMA_TERMINATE_ALL, 0);
> +}
> +
> +static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_sg(
> +		struct dma_chan *chan, struct scatterlist *sgl,
> +		unsigned int sg_len, enum dma_data_direction direction,
> +		unsigned long flags)
> +{
> +	return chan->device->device_prep_slave_sg(chan, sgl, sg_len, direction,
> +			flags);
> +}
> +
> +static inline struct dma_async_tx_descriptor *dmaengine_prep_cyclic(
> +		struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> +		size_t period_len, enum dma_data_direction direction)
> +{
> +	return chan->device->device_prep_dma_cyclic(chan, buf_addr, buf_len,
> +			period_len, direction);
> +}
> +
> +static inline int dmaengine_tx_submit(struct dma_async_tx_descriptor *desc)
> +{
> +	return desc->tx_submit(desc);
> +}
> +
>  static inline bool dmaengine_check_align(u8 align, size_t off1, size_t off2, size_t len)
>  {
>  	size_t mask;
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Sascha Hauer Sept. 20, 2010, 1:02 p.m. UTC | #2
Hi Dan,

Any comment?

Sascha

On Mon, Aug 16, 2010 at 01:07:49PM +0200, Sascha Hauer wrote:
> Currently dmaengine users have to explicitely dereference function
> pointers in struct dma_device. For the convenience of drivers and
> to be more flexible when changing the dmaengine later add static
> inline wrapper functions for the dma commands.
> 
> This patch is not complete yet. If there's consensus on this patch
> I'll provide an updated patch with the missing functions.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  include/linux/dmaengine.h |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 0df7864..635c60b 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -491,6 +491,47 @@ struct dma_device {
>  	void (*device_issue_pending)(struct dma_chan *chan);
>  };
>  
> +static inline int dmaengine_device_control(struct dma_chan *chan,
> +					   enum dma_ctrl_cmd cmd,
> +					   unsigned long arg)
> +{
> +	return chan->device->device_control(chan, cmd, arg);
> +}
> +
> +static inline int dmaengine_slave_config(struct dma_chan *chan,
> +					  struct dma_slave_config *config)
> +{
> +	return dmaengine_device_control(chan, DMA_SLAVE_CONFIG,
> +			(unsigned long)config);
> +}
> +
> +static inline int dmaengine_terminate_all(struct dma_chan *chan)
> +{
> +	return dmaengine_device_control(chan, DMA_TERMINATE_ALL, 0);
> +}
> +
> +static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_sg(
> +		struct dma_chan *chan, struct scatterlist *sgl,
> +		unsigned int sg_len, enum dma_data_direction direction,
> +		unsigned long flags)
> +{
> +	return chan->device->device_prep_slave_sg(chan, sgl, sg_len, direction,
> +			flags);
> +}
> +
> +static inline struct dma_async_tx_descriptor *dmaengine_prep_cyclic(
> +		struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> +		size_t period_len, enum dma_data_direction direction)
> +{
> +	return chan->device->device_prep_dma_cyclic(chan, buf_addr, buf_len,
> +			period_len, direction);
> +}
> +
> +static inline int dmaengine_tx_submit(struct dma_async_tx_descriptor *desc)
> +{
> +	return desc->tx_submit(desc);
> +}
> +
>  static inline bool dmaengine_check_align(u8 align, size_t off1, size_t off2, size_t len)
>  {
>  	size_t mask;
> -- 
> 1.7.1
> 
>
Dan Williams Sept. 23, 2010, 7:53 p.m. UTC | #3
On Mon, Aug 16, 2010 at 4:07 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> Currently dmaengine users have to explicitely dereference function
> pointers in struct dma_device. For the convenience of drivers and
> to be more flexible when changing the dmaengine later add static
> inline wrapper functions for the dma commands.
>
> This patch is not complete yet. If there's consensus on this patch
> I'll provide an updated patch with the missing functions.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  include/linux/dmaengine.h |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 0df7864..635c60b 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -491,6 +491,47 @@ struct dma_device {
>        void (*device_issue_pending)(struct dma_chan *chan);
>  };
>
> +static inline int dmaengine_device_control(struct dma_chan *chan,
> +                                          enum dma_ctrl_cmd cmd,
> +                                          unsigned long arg)
> +{
> +       return chan->device->device_control(chan, cmd, arg);
> +}
> +
> +static inline int dmaengine_slave_config(struct dma_chan *chan,
> +                                         struct dma_slave_config *config)
> +{
> +       return dmaengine_device_control(chan, DMA_SLAVE_CONFIG,
> +                       (unsigned long)config);
> +}
> +
> +static inline int dmaengine_terminate_all(struct dma_chan *chan)
> +{
> +       return dmaengine_device_control(chan, DMA_TERMINATE_ALL, 0);
> +}
> +
> +static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_sg(
> +               struct dma_chan *chan, struct scatterlist *sgl,
> +               unsigned int sg_len, enum dma_data_direction direction,
> +               unsigned long flags)
> +{
> +       return chan->device->device_prep_slave_sg(chan, sgl, sg_len, direction,
> +                       flags);
> +}
> +
> +static inline struct dma_async_tx_descriptor *dmaengine_prep_cyclic(
> +               struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> +               size_t period_len, enum dma_data_direction direction)
> +{
> +       return chan->device->device_prep_dma_cyclic(chan, buf_addr, buf_len,
> +                       period_len, direction);
> +}
> +

No strong disagreements on the above, the type safety of
dmaengine_slave_config() is nice.

> +static inline int dmaengine_tx_submit(struct dma_async_tx_descriptor *desc)
> +{
> +       return desc->tx_submit(desc);
> +}

This one can drop the tx.

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Sascha Hauer Sept. 24, 2010, 7:25 a.m. UTC | #4
Hi Dan,

On Thu, Sep 23, 2010 at 12:53:58PM -0700, Dan Williams wrote:
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index 0df7864..635c60b 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -491,6 +491,47 @@ struct dma_device {
> >        void (*device_issue_pending)(struct dma_chan *chan);
> >  };
> >
> > +static inline int dmaengine_device_control(struct dma_chan *chan,
> > +                                          enum dma_ctrl_cmd cmd,
> > +                                          unsigned long arg)
> > +{
> > +       return chan->device->device_control(chan, cmd, arg);
> > +}
> > +
> > +static inline int dmaengine_slave_config(struct dma_chan *chan,
> > +                                         struct dma_slave_config *config)
> > +{
> > +       return dmaengine_device_control(chan, DMA_SLAVE_CONFIG,
> > +                       (unsigned long)config);
> > +}
> > +
> > +static inline int dmaengine_terminate_all(struct dma_chan *chan)
> > +{
> > +       return dmaengine_device_control(chan, DMA_TERMINATE_ALL, 0);
> > +}
> > +
> > +static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_sg(
> > +               struct dma_chan *chan, struct scatterlist *sgl,
> > +               unsigned int sg_len, enum dma_data_direction direction,
> > +               unsigned long flags)
> > +{
> > +       return chan->device->device_prep_slave_sg(chan, sgl, sg_len, direction,
> > +                       flags);
> > +}
> > +
> > +static inline struct dma_async_tx_descriptor *dmaengine_prep_cyclic(
> > +               struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> > +               size_t period_len, enum dma_data_direction direction)
> > +{
> > +       return chan->device->device_prep_dma_cyclic(chan, buf_addr, buf_len,
> > +                       period_len, direction);
> > +}
> > +
> 
> No strong disagreements on the above, the type safety of
> dmaengine_slave_config() is nice.

So you have only small disagreements? ;) I can drop the dmaengine_prep_*
functions and only keep the device_control functions if like it better.

> 
> > +static inline int dmaengine_tx_submit(struct dma_async_tx_descriptor *desc)
> > +{
> > +       return desc->tx_submit(desc);
> > +}
> 
> This one can drop the tx.

You mean the function should be named dmaengine_submit?

Sascha
Dan Williams Sept. 24, 2010, 3:45 p.m. UTC | #5
On Fri, Sep 24, 2010 at 12:25 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> Hi Dan,
>
> On Thu, Sep 23, 2010 at 12:53:58PM -0700, Dan Williams wrote:
>> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>> > index 0df7864..635c60b 100644
>> > --- a/include/linux/dmaengine.h
>> > +++ b/include/linux/dmaengine.h
>> > @@ -491,6 +491,47 @@ struct dma_device {
>> >        void (*device_issue_pending)(struct dma_chan *chan);
>> >  };
>> >
>> > +static inline int dmaengine_device_control(struct dma_chan *chan,
>> > +                                          enum dma_ctrl_cmd cmd,
>> > +                                          unsigned long arg)
>> > +{
>> > +       return chan->device->device_control(chan, cmd, arg);
>> > +}
>> > +
>> > +static inline int dmaengine_slave_config(struct dma_chan *chan,
>> > +                                         struct dma_slave_config *config)
>> > +{
>> > +       return dmaengine_device_control(chan, DMA_SLAVE_CONFIG,
>> > +                       (unsigned long)config);
>> > +}
>> > +
>> > +static inline int dmaengine_terminate_all(struct dma_chan *chan)
>> > +{
>> > +       return dmaengine_device_control(chan, DMA_TERMINATE_ALL, 0);
>> > +}
>> > +
>> > +static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_sg(
>> > +               struct dma_chan *chan, struct scatterlist *sgl,
>> > +               unsigned int sg_len, enum dma_data_direction direction,
>> > +               unsigned long flags)
>> > +{
>> > +       return chan->device->device_prep_slave_sg(chan, sgl, sg_len, direction,
>> > +                       flags);
>> > +}
>> > +
>> > +static inline struct dma_async_tx_descriptor *dmaengine_prep_cyclic(
>> > +               struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
>> > +               size_t period_len, enum dma_data_direction direction)
>> > +{
>> > +       return chan->device->device_prep_dma_cyclic(chan, buf_addr, buf_len,
>> > +                       period_len, direction);
>> > +}
>> > +
>>
>> No strong disagreements on the above, the type safety of
>> dmaengine_slave_config() is nice.
>
> So you have only small disagreements? ;) I can drop the dmaengine_prep_*
> functions and only keep the device_control functions if like it better.
>

The prep versions may just be code churn, unless there is another
advantage for having a helper... debug/tracing perhaps?

>>
>> > +static inline int dmaengine_tx_submit(struct dma_async_tx_descriptor *desc)
>> > +{
>> > +       return desc->tx_submit(desc);
>> > +}
>>
>> This one can drop the tx.
>
> You mean the function should be named dmaengine_submit?

Yes, the "tx" is often mistaken, understandably, for "transmit".
Linus proposed renaming ->tx_submit() to just ->submit().  This helper
would be a softer way to introduce that rename.

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Patch
diff mbox series

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 0df7864..635c60b 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -491,6 +491,47 @@  struct dma_device {
 	void (*device_issue_pending)(struct dma_chan *chan);
 };
 
+static inline int dmaengine_device_control(struct dma_chan *chan,
+					   enum dma_ctrl_cmd cmd,
+					   unsigned long arg)
+{
+	return chan->device->device_control(chan, cmd, arg);
+}
+
+static inline int dmaengine_slave_config(struct dma_chan *chan,
+					  struct dma_slave_config *config)
+{
+	return dmaengine_device_control(chan, DMA_SLAVE_CONFIG,
+			(unsigned long)config);
+}
+
+static inline int dmaengine_terminate_all(struct dma_chan *chan)
+{
+	return dmaengine_device_control(chan, DMA_TERMINATE_ALL, 0);
+}
+
+static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_sg(
+		struct dma_chan *chan, struct scatterlist *sgl,
+		unsigned int sg_len, enum dma_data_direction direction,
+		unsigned long flags)
+{
+	return chan->device->device_prep_slave_sg(chan, sgl, sg_len, direction,
+			flags);
+}
+
+static inline struct dma_async_tx_descriptor *dmaengine_prep_cyclic(
+		struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
+		size_t period_len, enum dma_data_direction direction)
+{
+	return chan->device->device_prep_dma_cyclic(chan, buf_addr, buf_len,
+			period_len, direction);
+}
+
+static inline int dmaengine_tx_submit(struct dma_async_tx_descriptor *desc)
+{
+	return desc->tx_submit(desc);
+}
+
 static inline bool dmaengine_check_align(u8 align, size_t off1, size_t off2, size_t len)
 {
 	size_t mask;