LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Olof Johansson <olof@lixom.net>
To: Dan Williams <dan.j.williams@intel.com>
Cc: "Nelson, Shannon" <shannon.nelson@intel.com>,
	linux-kernel@vger.kernel.org, pasemi-linux@ozlabs.org,
	linuxppc-dev@ozlabs.org, hskinnemoen@atmel.com
Subject: Re: [PATCH] pasemi_dma: Driver for PA Semi PWRficient on-chip DMAengine
Date: Thu, 13 Mar 2008 14:54:31 -0500	[thread overview]
Message-ID: <20080313195431.GA1449@lixom.net> (raw)
In-Reply-To: <1205255051.26723.19.camel@dwillia2-linux.ch.intel.com>

On Tue, Mar 11, 2008 at 10:04:11AM -0700, Dan Williams wrote:

> I notice that the driver does not handle callbacks in its descriptor
> cleanup path.  This could be ok if your intent is only to support the
> net_dma style polled operations, but this will not work for the
> raid-offload async_tx case.  I think the solution is for async_tx to
> ignore channels without the DMA_INTERRUPT capability.

Good point, and correct - I have mostly been testing this with the
NET_DMA offload. async_tx doesn't make use of just memcpy, does it?

Also, how is DMA_INTERRUPT supposed to work? I see there's a separate
"prep_dma_interrupt" function, but that doesn't make sense to me. Don't
you want the interrupt associated with a specific transaction instead
of added as a separate (empty) transaction?

Once I add the descriptor to the ring, I can't change it to set the
interrupt request bit on it. I suppose I could just add a dummy descriptor
to the ring, but that doesn't seem quite right either.

> > +static void pasemi_dma_clean(struct pasemi_dma_chan *chan)
> > +{
> > +       int old, new, i;
> > +       unsigned long flags;
> > +       struct pasemi_dma_desc *desc;
> > +       spin_lock_irqsave(&chan->desc_lock, flags);
> 
> Is spin_lock_bh() insufficient here?
> 
> ...that's all that jumps out at the moment.

Can't do that if it's called both from the polling as well as the IRQ
context, it'd need to hold off irqs. I.e. once I add the DMA_INTERRUPT
support it will be needed.


-Olof


  reply	other threads:[~2008-03-13 19:48 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-06 23:39 [PATCH] pasemi_dma: Driver for PA Semi PWRficient on-chip DMA engine Olof Johansson
2008-03-07  0:31 ` Stephen Rothwell
2008-03-07  1:35   ` Olof Johansson
2008-03-11  7:06 ` Andrew Morton
2008-03-11 14:25   ` Olof Johansson
2008-03-11 17:53     ` Andrew Morton
2008-03-11 18:15       ` Dan Williams
2008-03-11 18:29         ` Kumar Gala
2008-03-11 20:37           ` Dan Williams
2008-03-11 17:04 ` [PATCH] pasemi_dma: Driver for PA Semi PWRficient on-chip DMAengine Dan Williams
2008-03-13 19:54   ` Olof Johansson [this message]
2008-03-13 22:29     ` Dan Williams
2008-03-13 23:14       ` Olof Johansson
2008-03-14  0:06         ` Dan Williams
2008-03-16 21:30 ` [PATCH v2] pasemi_dma: Driver for PA Semi PWRficient on-chip DMA engine Olof Johansson
2008-03-17 18:46   ` Dan Williams
2008-03-18  0:27     ` Olof Johansson
2008-03-17 20:34   ` [PATCH v2] pasemi_dma: Driver for PA Semi PWRficient on-chip DMAengine Nelson, Shannon
2008-03-18  0:21     ` Olof Johansson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080313195431.GA1449@lixom.net \
    --to=olof@lixom.net \
    --cc=dan.j.williams@intel.com \
    --cc=hskinnemoen@atmel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=pasemi-linux@ozlabs.org \
    --cc=shannon.nelson@intel.com \
    --subject='Re: [PATCH] pasemi_dma: Driver for PA Semi PWRficient on-chip DMAengine' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).