Hi Marc.

On Thu, Jun 28, 2007 at 01:49:13PM -0600, Marc St-Jean ([EMAIL PROTECTED]) 
wrote:
> +static int
> +sec_init_queues(void)
> +{
> +     int i;
> +     struct workq *wq;
> +     struct compq *cq;
> +
> +     /*
> +      * Allocate uncached space for hw_ptr values.
> +      * NOTE: status ptr value is not currently used.
> +      */
> +     status_ptr = dma_alloc_coherent(NULL, sizeof(int), &status_dma_addr,
> +                                     GFP_KERNEL);
> +     DBG_SEC("Allocated status ptr memory at 0x%p (0x%08x)\n",
> +                     status_ptr, status_dma_addr);
> +     if (!status_ptr)
> +             return -ENOMEM;
> +
> +     for (i = 0; i < HW_NR_COMP_QUEUES; i++) {
> +             void *base; /* slowpath virtual address of base */
> +             dma_addr_t base_dma_addr; /* DMA bus address of base */
> +
> +             base = dma_alloc_coherent(NULL, SEC_COMP_Q_SIZE,
> +                             &base_dma_addr, GFP_KERNEL);
> +             DBG_SEC("Allocated CQ%d at 0x%p (0x%08x)\n",
> +                     i, base, base_dma_addr);
> +             if (!base)
> +                     return -ENOMEM;

This leaks allocations.

> +             cq = &sec_comp_queues[i];
> +
> +             cq->compq_lock = SPIN_LOCK_UNLOCKED;
> +             cq->cq_regs = &sec2_regs->cq[i];
> +             cq->base = base;
> +             cq->base_dma_addr = base_dma_addr;
> +             cq->out = 0;
> +
> +             cq->cq_regs->ofst_ptr = (unsigned int *)status_dma_addr;
> +             cq->cq_regs->base = (unsigned char *)cq->base_dma_addr;
> +             cq->cq_regs->size = SEC_COMP_Q_SIZE;
> +             cq->cq_regs->in = 0;
> +             cq->cq_regs->out = 0;
> +     }
> +
> +     for (i = 0; i < HW_NR_WORK_QUEUES; i++) {
> +             void *base; /* slowpath virtual address of base */
> +             dma_addr_t base_dma_addr; /* DMA bus address of base */
> +
> +             base = dma_alloc_coherent(NULL, SEC_WORK_Q_SIZE,
> +                                     &base_dma_addr, GFP_KERNEL);
> +             DBG_SEC("Allocated WQ%d at 0x%p (0x%08x)\n",
> +                     i, base, base_dma_addr);
> +             if (!base)
> +                     return -ENOMEM;

This too.

> +             wq = &sec_work_queues[i];
> +
> +             init_waitqueue_head(&wq->space_wait);
> +
> +             wq->workq_lock = SPIN_LOCK_UNLOCKED;
> +             wq->wq_regs = &sec2_regs->wq[i];
> +             wq->base = base;
> +             wq->base_dma_addr = base_dma_addr;
> +             wq->in = 0;
> +             wq->low_water = SEC_WORK_Q_SIZE >> 1; /* wake when half full */
> +
> +             wq->wq_regs->ofst_ptr = (unsigned int *)status_dma_addr;
> +             wq->wq_regs->base = (unsigned char *)wq->base_dma_addr;
> +             wq->wq_regs->size = SEC_WORK_Q_SIZE;
> +             wq->wq_regs->in = 0;
> +             wq->wq_regs->out = 0;
> +     }
> +     
> +     debug_dump_sec_regs();
> +
> +     return 0;
> +}
> +
> +static int __init
> +msp_secv2_init(void)

Shouldn't this and other places be marked as __devinit?

...

> +static irqreturn_t
> +msp_secv2_interrupt(int irq, void *dev_id)
> +{
> +     /*
> +      * TODO: This clears all interrupts, and assumes
> +      * that the cause was a completion queue update.
> +      */
> +     unsigned int status;
> +
> +     status = sec2_regs->sis;
> +     sec2_regs->sis = /* ~status */ 0;
> +
> +     DBG_SEC("interrupt irq %d status was %x\n", irq, status);
> +
> +     poll_completion();
> +
> +     return IRQ_HANDLED;
> +}

Irqs can not be shared?

...

> +static int
> +poll_completion(void)
> +{
> +     struct compq *cq;
> +     int flags;
> +     int work_ct = 0;
> +     
> +     /*
> +      * Check IPSEC engine register to see if at least one
> +      * completion element is in completion queue.
> +      */
> +     cq = sec_comp_queues;
> +     spin_lock_irqsave(&cq->compq_lock, flags);

This lock seems not to protect against desc_do_work() for example, but
there are register/mmio access under both - what is a locking rules
there?

-- 
        Evgeniy Polyakov
-
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to