lkml.org 
[lkml]   [2013]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/1] dmaengine: remove obsolete comment reference to dma_data_direction
    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

    --


    \
     
     \ /
      Last update: 2013-12-18 18:41    [W:8.878 / U:0.116 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site