RE: [PATCHv10 2/2] dma: Add Freescale eDMA engine driver support

From: Jingchang Lu
Date: Mon Jan 20 2014 - 09:26:52 EST



________________________________________
From: Vinod Koul <vinod.koul@xxxxxxxxx>
Sent: Monday, January 20, 2014 6:20 PM
To: Lu Jingchang-B35083
Cc: dan.j.williams@xxxxxxxxx; arnd@xxxxxxxx; shawn.guo@xxxxxxxxxx; pawel.moll@xxxxxxx; mark.rutland@xxxxxxx; swarren@xxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; Wang Huan-B18965
Subject: Re: [PATCHv10 2/2] dma: Add Freescale eDMA engine driver support

On Mon, Jan 20, 2014 at 11:05:03AM +0000, Jingchang Lu wrote:
>> > > > > + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
>> > > > > + struct dma_slave_config *cfg = (void *)arg;
>> > > > > + unsigned long flags;
>> > > > > + LIST_HEAD(head);
>> > > > > +
>> > > > > + switch (cmd) {
>> > > > > + case DMA_TERMINATE_ALL:
>> > > > > + spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
>> > > > > + fsl_edma_disable_request(fsl_chan);
>> > > > > + fsl_chan->edesc = NULL;
>> > > > > + vchan_get_all_descriptors(&fsl_chan->vchan, &head);
>> > > > > + spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
>> > > > > + vchan_dma_desc_free_list(&fsl_chan->vchan, &head);
>> > > > > + return 0;
> >> > > well what happens to the current ongoing transactions, i don't see
> >> those
> >> > > getting
> >> > > terminated?
> >> > The fsl_edma_disable_request(fsl_chan) would end the channel's ongoing
> >> transaction, then
> >> > the eDMA would not response to device dma request, and the
> >> vchan_dma_desc_free_list()
> >> > will release all associate memory. Thanks.
> >> Can you explain a bit more how terminate will happen, given taht you are
> >> using
> >> same thing for pause?
>> It works just like an interrupt controller on irq enable and disable. It has a register called
>> set/clear enable request register(SERQ/CERQ) to enable or disable the DMA request for a given
>> channel. It won't transfer data any more with the enable request register cleared. So for the
>> pause and terminate the pause is the same. And there is no other way to stop the channels.
>well then it is not pause! If you jave no way to stop the channel, you cant
>claim to support pause and resume!

>>Also, for terminate this will be problematic. Assuming you are doing transfers
>and terminate is invoked. Then you will disable irq. The transfers may get stuck
>and periphral clock may go away after transfer so current transaction never gets
>completed, how do we recovery from this?
The clear enable request would isolate the dma request from the eDMA engine,
and the eDMA engine could not receive any dma request from the device any more,
so the transmission is stopped, all the channel parameters will not be changed any more, and the
eDMA engine will not do any transmission for that channel until the request is enabled
by set enable request. And the channel parameters could be changed when the enable request
cleared.
We have tested the eDMA drive with the SAI audio driver which uses the cyclic dma tranfer, and it
work well.
Thanks.

>Alstly, have you looked at edma driver already existing, any similarties in teh
>controller with that?
Yes, I have notice the TI's edma driver, but they are not the same. So we implement this driver.

Best Regards,
Jingchang--
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/