Re: [PATCH] dma: add new DMA control commands

2012-10-18 Thread Huang Shijie
On Thu, Oct 18, 2012 at 5:29 AM, Jassi Brar  wrote:
> On 18 October 2012 12:15, Huang Shijie  wrote:
>> 于 2012年10月18日 14:18, Vinod Koul 写道:
>>
>>> Why cant you do start (prepare clock etc) when you submit the descriptor
>>> to dmaengine. Can be done in tx_submit callback.
>>> Similarly remove the clock when dma transaction gets completed.
>>
>> I ever thought this method too.
>>
>> But it will become low efficient in the following case:
>>
>>   Assuming the gpmi-nand driver has to read out 1024 pages in one _SINGLE_
>> read operation.
>> The gpmi-nand will submit the descriptor to dmaengine per page. So with your
>> method,
>> the system will repeat the enable/disable dma clock 1024 time. At every
>> enable/disable dma clock,
>> the system has to enable the clock chain and it's parents ...
>>
>> But with this patch, we only need to enable/disable dma clock one time, just
>> at we select the nand chip.
>>
> If the clock is the dmac's property (not channels'), the toggling
> seems too aggressive.
> You could try using runtime_suspend/resume for clock
> disabling/enabling. How about employing autosuspend with a few ms
> delay?

Yes, employing the autosuspend is workable method too.o
But it's a little more complicated then this patch. You have to create
a thread or workqueue to
do the job.

What's more, I think other DMA engine may also meets the same issue as
the mxs-dma, such as the imx-sdma.
It's somehow a common issue to disable the clocks when no one use the
DMA engine.
Do you also suggest employing the autosuspend? Why not use a simple
way to handle this issue?
When the dma device want to use a DMA engine, it just needs to issue a
DMA_START command.

thanks
Huang Shijie
Huang Shijie
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] dma: add new DMA control commands

2012-10-18 Thread Huang Shijie
On Thu, Oct 18, 2012 at 6:51 AM, Marek Vasut  wrote:
> Dear Huang Shijie,
>
>> 于 2012年10月18日 16:49, Marek Vasut 写道:
>> > Dear Huang Shijie,
>> >
>> >> 于 2012年10月18日 16:16, Marek Vasut 写道:
>> >>> So we can't stream data from the chip? About time to adjust the MTD
>> >>> framework to allow that. Maybe implement a command queue?
>> >>
>> >> IMHO, it's not possible. Because the READ-PAGE(00h-30h) command needs to
>> >> check the busy status
>> >> which means we have to stop in the middle, so we can not chain the all
>> >> the read-pages DMA commands.
>> >
>> > Can the DMA not branch?
>>
>> it's too complicated to the MTD layer, as well as the gpmi driver.
>
> Can you please elaborate ?
could you read the nand spec about how to read a page out with the
READ-PAGE(00-30) command,
and tell me how to make the DMA branch?
I really have no idea about how to make the DMA branch.

thanks
Huang Shijie
.


> Best regards,
> Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] dma: add new DMA control commands

2012-10-18 Thread Huang Shijie

于 2012年10月18日 16:49, Marek Vasut 写道:

Dear Huang Shijie,


于 2012年10月18日 16:16, Marek Vasut 写道:

So we can't stream data from the chip? About time to adjust the MTD
framework to allow that. Maybe implement a command queue?

IMHO, it's not possible. Because the READ-PAGE(00h-30h) command needs to
check the busy status
which means we have to stop in the middle, so we can not chain the all
the read-pages DMA commands.

Can the DMA not branch?

it's too complicated to the MTD layer, as well as the gpmi driver.

Best Regards
Huang Shijie


--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] dma: add new DMA control commands

2012-10-18 Thread Huang Shijie

于 2012年10月18日 16:52, Russell King - ARM Linux 写道:

On Thu, Oct 18, 2012 at 02:45:41PM +0800, Huang Shijie wrote:

于 2012年10月18日 14:18, Vinod Koul 写道:

Why cant you do start (prepare clock etc) when you submit the descriptor
to dmaengine. Can be done in tx_submit callback.
Similarly remove the clock when dma transaction gets completed.

I ever thought this method too.

But it will become low efficient in the following case:

   Assuming the gpmi-nand driver has to read out 1024 pages in one
_SINGLE_ read operation.
The gpmi-nand will submit the descriptor to dmaengine per page. So with
your method,
the system will repeat the enable/disable dma clock 1024 time. At every
enable/disable dma clock,
the system has to enable the clock chain and it's parents ...

And what if you stop using clk_prepare_enable(), and prepare the clock
when the channel is requested and only use clk_enable() in the tx_submit

yes. it's a little better.
There is nearly no difference between the clk_prepare_enable() and 
clk_enable() in actually.

the clk_gate2_ops does not have any @->prepare.

thanks
Huang Shijie


method?




--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] dma: add new DMA control commands

2012-10-18 Thread Huang Shijie

于 2012年10月18日 16:16, Marek Vasut 写道:

So we can't stream data from the chip? About time to adjust the MTD framework to
allow that. Maybe implement a command queue?

IMHO, it's not possible. Because the READ-PAGE(00h-30h) command needs to 
check the busy status
which means we have to stop in the middle, so we can not chain the all 
the read-pages DMA commands.


thanks
Huang Shijie


--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] dma: add new DMA control commands

2012-10-18 Thread Huang Shijie

于 2012年10月18日 16:16, Marek Vasut 写道:

So we can't stream data from the chip? About time to adjust the MTD framework to
allow that. Maybe implement a command queue?



to Artem & David:
   is this possible to stream the data out with a command queue?

thanks
Huang Shijie

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] dma: add new DMA control commands

2012-10-18 Thread Huang Shijie

于 2012年10月18日 15:14, Marek Vasut 写道:

Dear Huang Shijie,

Why such massive CC ?


于 2012年10月18日 14:18, Vinod Koul 写道:

Why cant you do start (prepare clock etc) when you submit the descriptor
to dmaengine. Can be done in tx_submit callback.
Similarly remove the clock when dma transaction gets completed.

I ever thought this method too.

But it will become low efficient in the following case:

Assuming the gpmi-nand driver has to read out 1024 pages in one
_SINGLE_ read operation.
The gpmi-nand will submit the descriptor to dmaengine per page.

It will? Then GPMI NAND is flat our broken ... again.

yes.

Please read the NAND chip spec about the comand READ PAGE(00h-30h) and 
the code

nand_do_read_ops(). The nand chip limits us only read one page out one time.
So the driver will submit the descriptor to dmaengine per page.




So with
your method,
the system will repeat the enable/disable dma clock 1024 time.

Yes, it is the driver that's wrong.

not the driver.

At every
enable/disable dma clock,
the system has to enable the clock chain and it's parents ...

But with this patch, we only need to enable/disable dma clock one time,
just at we select the nand chip.

You are fixing a driver problem at a framework level, wrong.

So, check how the MXS SPI driver handles descriptor chaining. Indeed, the
Sigmatel screwed the DMA stuff good. But if you analyze the SPI driver, you'll
notice a few important points that might come handy when you fix the GPMI NAND
driver properly.

The direction to take here is:
1) Implement DMA chaining into the GPMI NAND driver
How can i implement the DMA chain if the nand chip READ-PAGE command 
gives us the one page limit?


thanks
Huang Shijie



2) You might need to do one PIO transfer to reconfigure the IP registers between
each segment of the DMA chain (just as MXS SPI does it)
3) You might run out of DMA descriptors when doing too long chains -- so you
might need to fix that part of the mxs DMA driver.



--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] dma: add new DMA control commands

2012-10-17 Thread Huang Shijie

于 2012年10月18日 14:18, Vinod Koul 写道:

Why cant you do start (prepare clock etc) when you submit the descriptor
to dmaengine. Can be done in tx_submit callback.
Similarly remove the clock when dma transaction gets completed.

I ever thought this method too.

But it will become low efficient in the following case:

  Assuming the gpmi-nand driver has to read out 1024 pages in one 
_SINGLE_ read operation.
The gpmi-nand will submit the descriptor to dmaengine per page. So with 
your method,
the system will repeat the enable/disable dma clock 1024 time. At every 
enable/disable dma clock,

the system has to enable the clock chain and it's parents ...

But with this patch, we only need to enable/disable dma clock one time, 
just at we select the nand chip.


thanks
Huang Shijie




--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] dma: add new DMA control commands

2012-10-17 Thread Huang Shijie
From: Huang Shijie 

[1] Why add these new DMA control commands?
In mx6q, the gpmi-nand driver is the only user of the APBH-DMA. The dma 
clock
is enabled when we have successfully requested a DMA channel. So even when
the gpmi-nand driver does not work, the dma clock(apbh-dma) still runs
in high speed (198MHz). To save some power, it is better to disable the dma
clock when the dma device, such as gpmi-nand, is not working anymore.
When the dma device becomes work again, enable the dma clock again.

[2] add new DMA control commands: DMA_START/DMA_END
DMA_START: do some preprations to start the DMA engine, such as enable the
   necessary clocks.
DMA_END: do some works to end the DMA engine, such as disable the
   necessary clocks.

[3] This patch does not change any logic in i2c-mxs driver and mxs-pcm driver.
But for gpmi-nand driver, we will enable the the clock only when we select
the nand chip, and want to do some real jobs with the nand chip.
For mxs-mmc driver, disable the dma clock in the suspend; and enable the
dma clock in the resume.

Signed-off-by: Huang Shijie 
---
 drivers/dma/mxs-dma.c  |   17 +
 drivers/i2c/busses/i2c-mxs.c   |   10 +-
 drivers/mmc/host/mxs-mmc.c |   19 ---
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |   10 ++
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   10 ++
 include/linux/dmaengine.h  |6 ++
 sound/soc/mxs/mxs-pcm.c|   12 
 7 files changed, 76 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 9f02e79..89286f4 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -384,6 +384,8 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan 
*chan)
/* the descriptor is ready */
async_tx_ack(&mxs_chan->desc);
 
+   clk_disable_unprepare(mxs_dma->clk);
+
return 0;
 
 err_clk:
@@ -399,6 +401,14 @@ static void mxs_dma_free_chan_resources(struct dma_chan 
*chan)
 {
struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+   int ret;
+
+   ret = clk_prepare_enable(mxs_dma->clk);
+   if (ret) {
+   dev_err(mxs_dma->dma_device.dev,
+   "failed in enabling the dma clock\n");
+   return;
+   }
 
mxs_dma_disable_chan(mxs_chan);
 
@@ -597,9 +607,13 @@ static int mxs_dma_control(struct dma_chan *chan, enum 
dma_ctrl_cmd cmd,
unsigned long arg)
 {
struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
+   struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
int ret = 0;
 
switch (cmd) {
+   case DMA_START:
+   ret = clk_prepare_enable(mxs_dma->clk);
+   break;
case DMA_TERMINATE_ALL:
mxs_dma_reset_chan(mxs_chan);
mxs_dma_disable_chan(mxs_chan);
@@ -610,6 +624,9 @@ static int mxs_dma_control(struct dma_chan *chan, enum 
dma_ctrl_cmd cmd,
case DMA_RESUME:
mxs_dma_resume_chan(mxs_chan);
break;
+   case DMA_END:
+   clk_disable_unprepare(mxs_dma->clk);
+   break;
default:
ret = -ENOSYS;
}
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 1f58197..da1e881 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -643,6 +643,12 @@ static int __devinit mxs_i2c_probe(struct platform_device 
*pdev)
dev_err(dev, "Failed to request dma\n");
return -ENODEV;
}
+   err = dmaengine_device_control(i2c->dmach, DMA_START, 0);
+   if (err) {
+   dma_release_channel(i2c->dmach);
+   dev_err(dev, "Failed to start dma\n");
+   return err;
+   }
}
 
platform_set_drvdata(pdev, i2c);
@@ -680,8 +686,10 @@ static int __devexit mxs_i2c_remove(struct platform_device 
*pdev)
if (ret)
return -EBUSY;
 
-   if (i2c->dmach)
+   if (i2c->dmach) {
+   dmaengine_device_control(i2c->dmach, DMA_END, 0);
dma_release_channel(i2c->dmach);
+   }
 
writel(MXS_I2C_CTRL0_SFTRST, i2c->regs + MXS_I2C_CTRL0_SET);
 
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 80d1e6d..aa91830 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -676,6 +676,9 @@ static int mxs_mmc_probe(struct platform_device *pdev)
"%s: failed to request dma\n", __func__);
goto out_clk_put;
}
+   ret = dmaengine_device_control(ssp->dmach, DMA_START, 

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

2012-04-27 Thread Huang Shijie
On Fri, Apr 27, 2012 at 11:13 AM, Lothar Waßmann  
wrote:
> Hi,
>
> Richard Zhao writes:
>> On Fri, Apr 27, 2012 at 11:18:31AM +0200, Lothar Waßmann wrote:
>> > Hi,
>> >
>> > Richard Zhao writes:
>> > > device_prep_dma_cyclic may be call in audio trigger function which is
>> > > atomic context, so we make it atomic too.
>> > >
>> > >  - change channel0 lock to spinlock.
>> > >  - Use polling to wait for channel0 finish running.
>> > >
>> > > Signed-off-by: Lothar Waßmann 
>> > >
>> > Actually I didn't sign off the patch that I posted, because I wanted
>> > to wait for more comments first.
>> I send it out with slight modifications because the series highly
>> depend on it. Will you take it over or let me put it in next version?
>> Both are ok to me.
>>
> I think you should keep it as part of your sound patches and I will
> test your final version on our hardware.
>
I hope we can get a conclusion that the prep_slave_sg() can be called
in atomic context or not.
My patch "add DMA support to UART" heavily depends on it.

Huang Shijie

>
> Lothar Waßmann
> --
> ___
>
> Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
> Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
> Geschäftsführer: Matthias Kaussen
> Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
>
> www.karo-electronics.de | i...@karo-electronics.de
> ___
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html