[PATCH v2 1/5] dmaengine: add ep93xx DMA support
Rafal Prylowski
prylowski at metasoft.pl
Wed Nov 16 04:00:46 EST 2011
Hello.
> Nice to see someone is trying to get IDE support for the ep93xx into mainline!
> Unfortunately none of my ep93xx hardware supports IDE... :-(
>
This driver is a result of work of several people, which I've seen on linux-ide
mailing list. I only added this dmaengine support. I really would like to
see it in mainline, but I think it's still not ready for inclusion.
>> default:
>> @@ -668,24 +669,28 @@ static void ep93xx_dma_unmap_buffers(str
>> static void ep93xx_dma_tasklet(unsigned long data)
>> {
>> struct ep93xx_dma_chan *edmac = (struct ep93xx_dma_chan *)data;
>> - struct ep93xx_dma_desc *desc, *d;
>> - dma_async_tx_callback callback;
>> - void *callback_param;
>> + struct ep93xx_dma_desc *desc = NULL, *d;
>> + dma_async_tx_callback callback = NULL;
>> + void *callback_param = NULL;
>> LIST_HEAD(list);
>>
>> spin_lock_irq(&edmac->lock);
>> - desc = ep93xx_dma_get_active(edmac);
>> - if (desc->complete) {
>> - edmac->last_completed = desc->txd.cookie;
>> - list_splice_init(&edmac->active, &list);
>> + if (!list_empty(&edmac->active)) {
>> + desc = ep93xx_dma_get_active(edmac);
>> + if (desc->complete) {
>> + edmac->last_completed = desc->txd.cookie;
>> + list_splice_init(&edmac->active, &list);
>> + }
>
> It looks like this might actually catch your BUG_ON issue above.
Yes, I only inserted BUG_ON in ep93xx_dma_get_active() to be sure, that
nowhere else in code happens similar problem.
But now I'm not so sure, that I encountered bug in ep93xx_dma.c, or
maybe I'm misusing dmaengine api (calling dmaengine_termiante_all from
invalid context?). I don't have enough knowledge to judge this.
>
>> }
>> spin_unlock_irq(&edmac->lock);
>>
>> /* Pick up the next descriptor from the queue */
>> ep93xx_dma_advance_work(edmac);
>>
>> - callback = desc->txd.callback;
>> - callback_param = desc->txd.callback_param;
>> + if (desc) {
>> + callback = desc->txd.callback;
>> + callback_param = desc->txd.callback_param;
>> + }
>
> These could be moved up to where 'desc' is getting set. You have already
> verified that the list is not empty and have a valid 'desc' pointer. Set
> the callback pointers there to remove this extra if (desc) test.
>
Following is a patch with suggestions applied (patch addressing problem with
incorrect programming of control register is in another mail sent in reply to
Mika Westerberg).
Regards,
Rafal Prylowski.
Index: linux-2.6/drivers/dma/ep93xx_dma.c
===================================================================
--- linux-2.6.orig/drivers/dma/ep93xx_dma.c
+++ linux-2.6/drivers/dma/ep93xx_dma.c
@@ -669,24 +669,25 @@ static void ep93xx_dma_tasklet(unsigned
{
struct ep93xx_dma_chan *edmac = (struct ep93xx_dma_chan *)data;
struct ep93xx_dma_desc *desc, *d;
- dma_async_tx_callback callback;
- void *callback_param;
+ dma_async_tx_callback callback = NULL;
+ void *callback_param = NULL;
LIST_HEAD(list);
spin_lock_irq(&edmac->lock);
- desc = ep93xx_dma_get_active(edmac);
- if (desc->complete) {
- edmac->last_completed = desc->txd.cookie;
- list_splice_init(&edmac->active, &list);
+ if (!list_empty(&edmac->active)) {
+ desc = ep93xx_dma_get_active(edmac);
+ if (desc->complete) {
+ edmac->last_completed = desc->txd.cookie;
+ list_splice_init(&edmac->active, &list);
+ }
+ callback = desc->txd.callback;
+ callback_param = desc->txd.callback_param;
}
spin_unlock_irq(&edmac->lock);
/* Pick up the next descriptor from the queue */
ep93xx_dma_advance_work(edmac);
- callback = desc->txd.callback;
- callback_param = desc->txd.callback_param;
-
/* Now we can release all the chained descriptors */
list_for_each_entry_safe(desc, d, &list, node) {
/*
More information about the linux-arm-kernel
mailing list