lkml.org 
[lkml]   [2009]   [Apr]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH] DMA: TXx9 Soc DMA Controller driver (v2)
    From
    On Sat, 18 Apr 2009 13:05:15 -0700, Dan Williams <dan.j.williams@intel.com> wrote:
    > Not quite "ackable" yet...

    Thank you for review!

    > > +#ifdef CONFIG_MACH_TX49XX
    > > +#define TXX9_DMA_MAY_HAVE_64BIT_REGS
    > > +#define TXX9_DMA_HAVE_CCR_LE
    > > +#define TXX9_DMA_HAVE_SMPCHN
    > > +#define TXX9_DMA_HAVE_IRQ_PER_CHAN
    > > +#endif
    > > +
    > > +#ifdef TXX9_DMA_HAVE_SMPCHN
    > > +#define TXX9_DMA_USE_SIMPLE_CHAIN
    > > +#endif
    > > +
    >
    > There seems to be a lot of ifdef magic in the code based on these
    > defines. Can we move this magic and some of the pure definitions to
    > drivers/dma/txx9dmac.h? (See the "#ifdefs are ugly" section of
    > Documentation/SubmittingPatches)

    OK, I will try to clean them up. But since I don't want to export
    internal implementation details, some of the magics will be left in
    txx9dmac.c, perhaps.

    > > +static struct dma_async_tx_descriptor *
    > > +txx9dmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
    > > +               size_t len, unsigned long flags)
    > [..]
    > > +               if (!first) {
    > > +                       first = desc;
    > > +               } else {
    > > +                       desc_write_CHAR(dc, prev, desc->txd.phys);
    > > +                       dma_sync_single_for_device(chan2parent(&dc->chan),
    > > +                                       prev->txd.phys, ddev->descsize,
    > > +                                       DMA_TO_DEVICE);
    > > +                       list_add_tail(&desc->desc_node,
    > > +                                       &first->txd.tx_list);
    > > +               }
    >
    > Is there a reason to keep f'irst' off of the tx_list? It seems like
    > you could simplify this logic and get rid of the scary looking
    > list_splice followed by list_add in txx9dmac_desc_put. It also seems
    > odd that the descriptors on tx_list are not reachable from the
    > dc->queue list after a submit... but maybe I am missing a subtle
    > detail?

    Well, I'm not sure what do you mean...

    The completion callback handler of the first descriptor should be
    called _after_ the completion of the _last_ child of the descriptor.
    Also I use desc_node for both dc->queue, dc->active_list and
    txd.tx_list. So if I putted all children to dc->queue or
    dc->active_list, txx9dmac_descriptor_complete() (or its caller) will
    be more complex.

    Or do you mean adding another list_head to maintain txd.tx_list? Or
    something another at all?

    ---
    Atsushi Nemoto
    --
    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/

    \
     
     \ /
      Last update: 2009-04-19 20:37    [W:0.050 / U:125.156 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site