Patchwork [06/10] MXS-DMA : add more flags for MXS-DMA

login
register
mail settings
Submitter Huang Shijie
Date Jan. 19, 2012, 6:16 a.m.
Message ID <1326953767-24155-7-git-send-email-b32955@freescale.com>
Download mbox | patch
Permalink /patch/136764/
State New
Headers show

Comments

Huang Shijie - Jan. 19, 2012, 6:16 a.m.
[1] Background :
    The GPMI does ECC read page operation with a DMA chain consist of three DMA
    Command Structures. The middle one of the chain is used to enable the BCH,
    and read out the NAND page.

    The WAIT4END(wait for command end) is a comunication signal between
    the GPMI and MXS-DMA.

[2] The current DMA code sets the WAIT4END bit at the last one, such as:

    +-----+               +-----+                      +-----+
    | cmd | ------------> | cmd | ------------------>  | cmd |
    +-----+               +-----+                      +-----+
                                                          ^
                                                          |
                                                          |
                                                     set WAIT4END here

    This chain works fine in the mx23/mx28.

[3] But in the new GPMI version (used in MX50/MX60), the WAIT4END bit should
    be set not only at the last DMA Command Structure,
    but also at the middle one, such as:

    +-----+               +-----+                      +-----+
    | cmd | ------------> | cmd | ------------------>  | cmd |
    +-----+               +-----+                      +-----+
                             ^                            ^
                             |                            |
                             |                            |
                        set WAIT4END here too        set WAIT4END here

   If we do not set WAIT4END, the BCH maybe stall in "ECC reading page" state.
   In the next ECC write page operation, a DMA-timeout occurs.
   This has been catched in the MX6Q board.

In order to fix the bug, we should let the driver to
set the proper DMA flags in the DMA command structrues.

So add the new flags for MXS-DMA.
The driver can use these flags to control the DMA in a more flexible way.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 include/linux/mxs-dma.h |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)
Russell King - ARM Linux - Jan. 19, 2012, 9:02 a.m.
On Thu, Jan 19, 2012 at 02:16:03PM +0800, Huang Shijie wrote:
> [1] Background :
>     The GPMI does ECC read page operation with a DMA chain consist of three DMA
>     Command Structures. The middle one of the chain is used to enable the BCH,
>     and read out the NAND page.
> 
>     The WAIT4END(wait for command end) is a comunication signal between
>     the GPMI and MXS-DMA.
> 
> [2] The current DMA code sets the WAIT4END bit at the last one, such as:
> 
>     +-----+               +-----+                      +-----+
>     | cmd | ------------> | cmd | ------------------>  | cmd |
>     +-----+               +-----+                      +-----+
>                                                           ^
>                                                           |
>                                                           |
>                                                      set WAIT4END here
> 
>     This chain works fine in the mx23/mx28.
> 
> [3] But in the new GPMI version (used in MX50/MX60), the WAIT4END bit should
>     be set not only at the last DMA Command Structure,
>     but also at the middle one, such as:
> 
>     +-----+               +-----+                      +-----+
>     | cmd | ------------> | cmd | ------------------>  | cmd |
>     +-----+               +-----+                      +-----+
>                              ^                            ^
>                              |                            |
>                              |                            |
>                         set WAIT4END here too        set WAIT4END here
> 
>    If we do not set WAIT4END, the BCH maybe stall in "ECC reading page" state.
>    In the next ECC write page operation, a DMA-timeout occurs.
>    This has been catched in the MX6Q board.
> 
> In order to fix the bug, we should let the driver to
> set the proper DMA flags in the DMA command structrues.
> 
> So add the new flags for MXS-DMA.
> The driver can use these flags to control the DMA in a more flexible way.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  include/linux/mxs-dma.h |   26 ++++++++++++++++++++++++++
>  1 files changed, 26 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/mxs-dma.h b/include/linux/mxs-dma.h
> index 203d7c4..3ef73b8 100644
> --- a/include/linux/mxs-dma.h
> +++ b/include/linux/mxs-dma.h
> @@ -11,6 +11,32 @@
>  
>  #include <linux/dmaengine.h>
>  
> +/*
> + * The drivers use these flags for ->device_prep_slave_sg() :
> + *    [1] If there is only one DMA command in the DMA chain, the code should be:
> + *            ......
> + *            ->device_prep_slave_sg(MXS_DMA_F_WAIT4END);
> + *            ......
> + *    [2] If there are two DMA commands in the DMA chain, the code should be
> + *            ......
> + *            ->device_prep_slave_sg(0);
> + *            ......
> + *            ->device_prep_slave_sg(MXS_DMA_F_LASTONE);
> + *            ......
> + *    [3] If there are more than two DMA commands in the DMA chain, the code
> + *        should be:
> + *            ......
> + *            ->device_prep_slave_sg(0);                 // First
> + *            ......
> + *            ->device_prep_slave_sg(MXS_DMA_F_APPEND [| MXS_DMA_F_WAIT4END]);
> + *            ......
> + *            ->device_prep_slave_sg(MXS_DMA_F_LASTONE); // Last
> + */
> +#define MXS_DMA_F_APPEND	(1 << 0)
> +#define MXS_DMA_F_WAIT4END	(1 << 1)
> +
> +#define MXS_DMA_F_LASTONE	(MXS_DMA_F_APPEND | MXS_DMA_F_WAIT4END)

Err, the 'flags' argument to device_prep_slave_sg() is supposed to be
from the set of enum dma_ctrl_flags.  What this means is that your
MXS_DMA_F_APPEND is equivalent to DMA_PREP_INTERRUPT, and
MXS_DMA_F_WAIT4END is equivalent to DMA_CTRL_ACK.

What this does is make your drivers completely dependent on your DMA
engine implementation.  That's not a good idea when devices get
reused in different SoCs.

If you need to supply extra flags which aren't in the dma_ctrl_flags,
at least make sure that they're using different bits.  For bonus points,
also have your driver _check_ the DMA engine it's connected to before
it passes these flags.
Huang Shijie - Jan. 19, 2012, 9:31 a.m.
hi,
> Err, the 'flags' argument to device_prep_slave_sg() is supposed to be
> from the set of enum dma_ctrl_flags.  What this means is that your
> MXS_DMA_F_APPEND is equivalent to DMA_PREP_INTERRUPT, and
> MXS_DMA_F_WAIT4END is equivalent to DMA_CTRL_ACK.
>
thanks a lot.
I will reuse the dma_ctrl_flags in the next version.
> What this does is make your drivers completely dependent on your DMA
> engine implementation.  That's not a good idea when devices get
> reused in different SoCs.
>
Frankly speaking, the APBH-DMA is more coupled with the GPMI  then any 
other modules.
In the MX6Q, the GPMI is the ONLY user of APBH-DMA.
You even can see the NAND_LOCK bit in the DMA command structure which is 
only used by the GPMI
NAND controller,.


To Shawn & Wolfram:
    thanks very much for your comments.

Best Regards
Huang Shijie

> If you need to supply extra flags which aren't in the dma_ctrl_flags,
> at least make sure that they're using different bits.  For bonus points,
> also have your driver_check_  the DMA engine it's connected to before
> it passes these flags.
>

Patch

diff --git a/include/linux/mxs-dma.h b/include/linux/mxs-dma.h
index 203d7c4..3ef73b8 100644
--- a/include/linux/mxs-dma.h
+++ b/include/linux/mxs-dma.h
@@ -11,6 +11,32 @@ 
 
 #include <linux/dmaengine.h>
 
+/*
+ * The drivers use these flags for ->device_prep_slave_sg() :
+ *    [1] If there is only one DMA command in the DMA chain, the code should be:
+ *            ......
+ *            ->device_prep_slave_sg(MXS_DMA_F_WAIT4END);
+ *            ......
+ *    [2] If there are two DMA commands in the DMA chain, the code should be
+ *            ......
+ *            ->device_prep_slave_sg(0);
+ *            ......
+ *            ->device_prep_slave_sg(MXS_DMA_F_LASTONE);
+ *            ......
+ *    [3] If there are more than two DMA commands in the DMA chain, the code
+ *        should be:
+ *            ......
+ *            ->device_prep_slave_sg(0);                 // First
+ *            ......
+ *            ->device_prep_slave_sg(MXS_DMA_F_APPEND [| MXS_DMA_F_WAIT4END]);
+ *            ......
+ *            ->device_prep_slave_sg(MXS_DMA_F_LASTONE); // Last
+ */
+#define MXS_DMA_F_APPEND	(1 << 0)
+#define MXS_DMA_F_WAIT4END	(1 << 1)
+
+#define MXS_DMA_F_LASTONE	(MXS_DMA_F_APPEND | MXS_DMA_F_WAIT4END)
+
 struct mxs_dma_data {
 	int chan_irq;
 };