Re: [PATCH] dmaengine: add CSR SiRFprimaII DMAC driver

[Barry Song]

>>> +/* Interrupt handler */
>>> +static irqreturn_t sirfsoc_dma_irq(int irq, void *data)
>>> +{
>>> + Â Â struct sirfsoc_dma *sdma = data;
>>> + Â Â struct sirfsoc_dma_chan *schan;
>>> + Â Â u32 is;
>>> + Â Â int ch;
>>> +
>>> + Â Â is = readl_relaxed(sdma->regs + SIRFSOC_DMA_CH_INT);
>>> + Â Â while ((ch = fls(is) - 1) >= 0) {
>>> + Â Â Â Â Â Â is &= ~(1 << ch);
>>> + Â Â Â Â Â Â writel_relaxed(1 << ch, sdma->regs + SIRFSOC_DMA_CH_INT);
>>> + Â Â Â Â Â Â schan = &sdma->channels[ch];
>>> +
>>> + Â Â Â Â Â Â spin_lock(&schan->lock);
>>> +
>>> + Â Â Â Â Â Â /* Execute queued descriptors */
>>> + Â Â Â Â Â Â list_splice_tail_init(&schan->active, &schan->completed);
>>> + Â Â Â Â Â Â if (!list_empty(&schan->queued))
>>> + Â Â Â Â Â Â Â Â Â Â sirfsoc_dma_execute(schan);
>>> +
>>> + Â Â Â Â Â Â spin_unlock(&schan->lock);
>>> + Â Â }
>> Here you know which channel has triggered interrupt and you may pass
>> this info to your tasklet and avoid scanning again there
>
> ok. let me see.

we really know what channels have been trigger in irq. but we lose a
good way to transfer that to tasklet actually. if we place a flag in
schan data.
1. there can be more 1 channels triggers, then tasklet still need a for(...)
2. we actually need a lock between irq and tasklet. otherwise, new irq
coming in tasklet might change the flag.

so i think current way should be better.

>
>>
>>> +
>>> + Â Â /* Schedule tasklet */
>>> + Â Â tasklet_schedule(&sdma->tasklet);
>>> +
>>> + Â Â return IRQ_HANDLED;
>>> +}
>>> +
>>> +/* process completed descriptors */
>>> +static void sirfsoc_dma_process_completed(struct sirfsoc_dma *sdma)
>>> +{
>>> + Â Â dma_cookie_t last_cookie = 0;
>>> + Â Â struct sirfsoc_dma_chan *schan;
>>> + Â Â struct sirfsoc_dma_desc *mdesc;
>>> + Â Â struct dma_async_tx_descriptor *desc;
>>> + Â Â unsigned long flags;
>>> + Â Â LIST_HEAD(list);
>>> + Â Â int i;
>>> +
>>> + Â Â for (i = 0; i < sdma->dma.chancnt; i++) {
>>> + Â Â Â Â Â Â schan = &sdma->channels[i];
>>> +
>>> + Â Â Â Â Â Â /* Get all completed descriptors */
>>> + Â Â Â Â Â Â spin_lock_irqsave(&schan->lock, flags);
>> this will block interrupts, i dont see a reason why this should be used
>> here??
>
> ok. no irq is accessing completed list.

sorry. after reading more carefully, we actually need this lock since
irq will move finished active node to completed list. we need to keep
the completed safe.

i have fixed other issues and used jassi's v1 patch (generic xfer) and
will send v2.

Thanks
barry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/