lkml.org 
[lkml]   [2007]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 -mm 5/7] I/OAT: Add support for MSI and MSI-X
    On Thu, 23 Aug 2007 17:15:17 -0700 Shannon Nelson wrote:

    > Add support for MSI and MSI-X interrupt handling, including the ability
    > to choose the desired interrupt method.
    >
    > Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
    > Acked-by: David S. Miller <davem@davemloft.net>
    > ---
    >
    > drivers/dma/ioat_dma.c | 353 ++++++++++++++++++++++++++++++++-------
    > drivers/dma/ioatdma.h | 12 +
    > drivers/dma/ioatdma_registers.h | 6 +
    > 3 files changed, 305 insertions(+), 66 deletions(-)
    >
    > diff --git a/drivers/dma/ioat_dma.c b/drivers/dma/ioat_dma.c
    > index 9012176..0909fee 100644
    > --- a/drivers/dma/ioat_dma.c
    > +++ b/drivers/dma/ioat_dma.c
    > @@ -47,6 +47,71 @@
    > static void ioat_dma_start_null_desc(struct ioat_dma_chan *ioat_chan);
    > static void ioat_dma_memcpy_cleanup(struct ioat_dma_chan *ioat_chan);
    >
    > +#define for_each_bit(bit, addr, size) \
    > + for ((bit) = find_first_bit((addr), (size)); \
    > + (bit) < (size); \
    > + (bit) = find_next_bit((addr), (size), (bit) + 1))

    Any reason that this macro shouldn't be added to
    include/linux/bitops.h instead of here? I'd prefer/expect such
    general-purpose macros to live somewhere else.


    > @@ -276,6 +352,34 @@ static void ioat_dma_free_chan_resources(struct dma_chan *chan)
    > in_use_descs - 1);
    >
    > ioat_chan->last_completion = ioat_chan->completion_addr = 0;
    > + ioat_chan->pending = 0;
    > +}
    > +/**
    > + * ioat_dma_get_next_descriptor - return the next available descriptor from
    > + * the chain

    Unfortunately, function name + short description in kernel-doc must be
    on one line (only). If you want to add more text/description, put it
    after the parameters (like you have done :).

    > + * @ioat_chan: IOAT DMA channel handle

    We normally use one
    *
    line between the parameters and following text.

    > + * Gets the next descriptor from the chain, and must be called with the
    > + * channel's desc_lock held. Allocates more descriptors if the channel
    > + * has run out.
    > + */
    > +static struct ioat_desc_sw *ioat_dma_get_next_descriptor(
    > + struct ioat_dma_chan *ioat_chan)
    > +{
    > + struct ioat_desc_sw *new = NULL;
    > +
    > + if (!list_empty(&ioat_chan->free_desc)) {
    > + new = to_ioat_desc(ioat_chan->free_desc.next);
    > + list_del(&new->node);
    > + } else {
    > + /* try to get another desc */
    > + new = ioat_dma_alloc_descriptor(ioat_chan, GFP_ATOMIC);
    > + /* will this ever happen? */
    > + /* TODO add upper limit on these */
    > + BUG_ON(!new);
    > + }
    > +
    > + prefetch(new->hw);
    > + return new;
    > }
    >
    > static struct dma_async_tx_descriptor *ioat_dma_prep_memcpy(
    > @@ -639,6 +711,162 @@ out:
    > return err;
    > }
    >
    > +static char ioat_interrupt_style[32] = "msix";
    > +module_param_string(ioat_interrupt_style, ioat_interrupt_style,
    > + sizeof(ioat_interrupt_style), 0644);
    > +MODULE_PARM_DESC(ioat_interrupt_style,
    > + "set ioat interrupt style: msix (default), "
    > + "msix-single-vector, msi, intx)");
    > +
    > +/**
    > + * ioat_dma_setup_interrupts - setup interrupt handler, choosing btwn msix,
    > + * msi, and legacy

    Same one-line nit. legacy == intx, I suppose.


    > + * @device: ioat device
    > + */
    > +int ioat_dma_setup_interrupts(struct ioatdma_device *device)
    > +{
    > + struct ioat_dma_chan *ioat_chan;
    > + int err, i, j, msixcnt;
    > + u8 intrctrl = 0;
    > +
    > + if (!strcmp(ioat_interrupt_style, "msix"))
    > + goto msix;
    > + else if (!strcmp(ioat_interrupt_style, "msix-single-vector"))
    > + goto msix_single_vector;
    > + else if (!strcmp(ioat_interrupt_style, "msi"))
    > + goto msi;
    > + else if (!strcmp(ioat_interrupt_style, "intx"))
    > + goto intx;

    The "else"s aren't needed.

    > +
    > +msix:

    ...

    ---
    ~Randy
    *** Remember to use Documentation/SubmitChecklist when testing your code ***
    -
    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: 2007-08-24 19:57    [W:0.030 / U:61.504 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site