[PATCH 1/1] dmaengine: remove obsolete comment reference to dma_data_direction

[PATCH 1/1] dmaengine: remove obsolete comment reference to dma_data_direction

[Alexander Popov]

enum dma_transfer_direction is currently used
in struct dma_slave_config, so update the comment

Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
 include/linux/dmaengine.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 41cf0c3..bd6b882 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -305,9 +305,8 @@ enum dma_slave_buswidth {
 /**
  * struct dma_slave_config - dma slave channel runtime config
  * @direction: whether the data shall go in or out on this slave
- * channel, right now. DMA_TO_DEVICE and DMA_FROM_DEVICE are
- * legal values, DMA_BIDIRECTIONAL is not acceptable since we
- * need to differentiate source and target addresses.
+ * channel, right now. DMA_MEM_TO_DEV and DMA_DEV_TO_MEM are
+ * legal values.
  * @src_addr: this is the physical address where DMA slave data
  * should be read (RX), if the source is memory this argument is
  * ignored.
-- 
1.8.4.2

Re: [PATCH 1/1] dmaengine: remove obsolete comment reference to dma_data_direction

[Gerhard Sittig]

On Wed, Dec 11, 2013 at 10:07 +0400, Alexander Popov wrote:
> 
> enum dma_transfer_direction is currently used
> in struct dma_slave_config, so update the comment
> 
> Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
> ---
>  include/linux/dmaengine.h | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 41cf0c3..bd6b882 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -305,9 +305,8 @@ enum dma_slave_buswidth {
>  /**
>   * struct dma_slave_config - dma slave channel runtime config
>   * @direction: whether the data shall go in or out on this slave
> - * channel, right now. DMA_TO_DEVICE and DMA_FROM_DEVICE are
> - * legal values, DMA_BIDIRECTIONAL is not acceptable since we
> - * need to differentiate source and target addresses.
> + * channel, right now. DMA_MEM_TO_DEV and DMA_DEV_TO_MEM are
> + * legal values.
>   * @src_addr: this is the physical address where DMA slave data
>   * should be read (RX), if the source is memory this argument is
>   * ignored.

While I agree with the change to the comment text, I'd put the
description of the patch differently.

You are not removing an obsolete reference, but you are fixing an
erroneous comment.  It's not that dma_data_direction would have
become obsolete, instead it's that dma_slave_config was
documented wrongly.  The previous comment used the wrong data
type for one of its members and thus discussed inappropriate
values out of the type's domain.  Maybe "fix incorrect kerneldoc
for struct dma_slave_config" or something similar could be a
better title.  Especially for those who read the log later and
neither have the patch nor its description at hand.

And you might improve the description's body.  Something like
"the 'direction' member of 'struct dma_slave_config' is of data
type 'enum dma_transfer_direction', so update the 'struct
dma_slave_config' kerneldoc to refer to appropriate values" or
similar.


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

Re: [PATCH 1/1] dmaengine: remove obsolete comment reference to dma_data_direction

[Vinod Koul]

On Sat, Dec 14, 2013 at 02:20:27PM +0100, Gerhard Sittig wrote:
> On Wed, Dec 11, 2013 at 10:07 +0400, Alexander Popov wrote:
> > 
> > enum dma_transfer_direction is currently used
> > in struct dma_slave_config, so update the comment
> > 
> > Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
> > ---
> >  include/linux/dmaengine.h | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index 41cf0c3..bd6b882 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -305,9 +305,8 @@ enum dma_slave_buswidth {
> >  /**
> >   * struct dma_slave_config - dma slave channel runtime config
> >   * @direction: whether the data shall go in or out on this slave
> > - * channel, right now. DMA_TO_DEVICE and DMA_FROM_DEVICE are
> > - * legal values, DMA_BIDIRECTIONAL is not acceptable since we
> > - * need to differentiate source and target addresses.
> > + * channel, right now. DMA_MEM_TO_DEV and DMA_DEV_TO_MEM are
> > + * legal values.
> >   * @src_addr: this is the physical address where DMA slave data
> >   * should be read (RX), if the source is memory this argument is
> >   * ignored.
> 
> While I agree with the change to the comment text, I'd put the
> description of the patch differently.
> 
> You are not removing an obsolete reference, but you are fixing an
> erroneous comment.  It's not that dma_data_direction would have
> become obsolete, instead it's that dma_slave_config was
> documented wrongly.  The previous comment used the wrong data
> type for one of its members and thus discussed inappropriate
> values out of the type's domain.  Maybe "fix incorrect kerneldoc
> for struct dma_slave_config" or something similar could be a
> better title.  Especially for those who read the log later and
> neither have the patch nor its description at hand.
comment was apt when it was created, it wanst updated at the time of conversion!

--
~Vinod
> 
> And you might improve the description's body.  Something like
> "the 'direction' member of 'struct dma_slave_config' is of data
> type 'enum dma_transfer_direction', so update the 'struct
> dma_slave_config' kerneldoc to refer to appropriate values" or
> similar.
> 
> 
> virtually yours
> Gerhard Sittig
> -- 
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--