Shimoda-san
On Tue, 31 Jan 2012, Shimoda, Yoshihiro wrote:
Hi Guennadi-san,
2012/01/27 17:48, Guennadi Liakhovetski wrote:
Hi Shimoda-san
On Fri, 27 Jan 2012, Shimoda, Yoshihiro wrote:
[ snip ]
- switch from using a tasklet to threaded IRQ, which allowed to
I tested this driver on 2 environment, but it could not work correctly.
- renesas_usbhs driver with shdma driver on the mackerel board
- renesas_usbhs driver with experimental SUDMAC driver
In this case, should we modify the renesas_usbhs driver?
Yes, you do. Please, see the respective version of the shdma patch. It doesn't request channel IRQs any more directly, instead, it uses a dma-simple wrapper function. Also operations change a bit.
Thank you for your comment.
I investigaed the issue. I found out the renesas_usbhs driver may call tx_submit() in the callback() of the dma-simple finally.
Sorry, in my first reply to this your email I misread the renesas_usbhs for an issue with your new SUDMAC driver. Since this is not the case and my patch seems to be causing a regression, I'll look at it.
Thanks Guennadi
In this case, the value of power_up in the simple_tx_submit is 0, the start_xfer() is not called. And then, even if the ld_queue is not empty, the DMA doesn't start.
So, if I add codes like the following in the dma-simple.c, the renesas_usbhs driver can work correctly without the driver changes. If you think the code is good, would you merge it to your code? ======= chan_irqt() ======= simple_chan_ld_cleanup(schan, false);
spin_lock_irq(&schan->chan_lock);
/* Next desc */
simple_chan_xfer_ld_queue(schan);
spin_unlock_irq(&schan->chan_lock);
return IRQ_HANDLED;
==============
Best regards, Yoshihiro Shimoda
Thanks Guennadi
For reference, if I changed the threaded IRQ to tasklet, the 2 environment worked correctly: ============== diff --git a/drivers/dma/dma-simple.c b/drivers/dma/dma-simple.c index 49d8f7d..65a5170 100644 --- a/drivers/dma/dma-simple.c +++ b/drivers/dma/dma-simple.c @@ -715,16 +715,18 @@ static irqreturn_t chan_irq(int irq, void *dev)
spin_lock(&schan->chan_lock);
- ret = ops->chan_irq(schan, irq) ? IRQ_WAKE_THREAD : IRQ_NONE;
ret = ops->chan_irq(schan, irq) ? IRQ_HANDLED : IRQ_NONE;
if (ret == IRQ_HANDLED)
tasklet_schedule(&schan->tasklet);spin_unlock(&schan->chan_lock);
return ret;
}
-static irqreturn_t chan_irqt(int irq, void *dev) +static void chan_irqt(unsigned long dev) {
- struct dma_simple_chan *schan = dev;
- struct dma_simple_chan *schan = (struct dma_simple_chan *)dev; const struct dma_simple_ops *ops = to_simple_dev(schan->dma_chan.device)->ops; struct dma_simple_desc *sdesc;
@@ -744,15 +746,13 @@ static irqreturn_t chan_irqt(int irq, void *dev) spin_unlock_irq(&schan->chan_lock);
simple_chan_ld_cleanup(schan, false);
- return IRQ_HANDLED;
}
int dma_simple_request_irq(struct dma_simple_chan *schan, int irq, unsigned long flags, const char *name) {
- int ret = request_threaded_irq(irq, chan_irq, chan_irqt,
flags, name, schan);
int ret = request_irq(irq, chan_irq, flags, name, schan);
tasklet_init(&schan->tasklet, chan_irqt, (unsigned long)schan);
schan->irq = ret < 0 ? ret : irq;
diff --git a/include/linux/dma-simple.h b/include/linux/dma-simple.h index 5336674..beb1489 100644 --- a/include/linux/dma-simple.h +++ b/include/linux/dma-simple.h @@ -68,6 +68,7 @@ struct dma_simple_chan { int id; /* Raw id of this channel */ int irq; /* Channel IRQ */ enum dma_simple_pm_state pm_state;
- struct tasklet_struct tasklet;
};
/**
Best regards, Yoshihiro Shimoda
Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
-- Yoshihiro Shimoda yoshihiro.shimoda.uh@renesas.com EC No.
--- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/