[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Fabio Estevam]

Initialize the DMA controller so that the following errors are fixed:

U-Boot 2012.04-rc1-00001-g037cbfd (Apr 02 2012 - 10:57:48)

Freescale i.MX28 family at 454 MHz
DRAM:  128 MiB
MMC:   MXS MMC: 0
MMC0: DMA transfer failed
MMC0: DMA transfer failed
MMC0: DMA transfer failed
MMC0: DMA transfer failed
MMC init failed

In order to not break the DMA NAND support, remove mxs_dma_init from the
NAND driver now that it is called from the core dma file.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
--
Changes since v1:
- Call mxs_dma_init() from a common location and remove it
from the mxs_nand driver

 drivers/dma/apbh_dma.c      |    2 ++
 drivers/mtd/nand/mxs_nand.c |    3 ---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/apbh_dma.c b/drivers/dma/apbh_dma.c
index c086629..68c4be2 100644
--- a/drivers/dma/apbh_dma.c
+++ b/drivers/dma/apbh_dma.c
@@ -346,6 +346,8 @@ struct mxs_dma_desc *mxs_dma_desc_alloc(void)
 	struct mxs_dma_desc *pdesc;
 	uint32_t size;
 
+	/* Initialize the DMA controller. */
+	mxs_dma_init();
 	size = roundup(sizeof(struct mxs_dma_desc), MXS_DMA_ALIGNMENT);
 	pdesc = memalign(MXS_DMA_ALIGNMENT, size);
 
diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index 4b1297a..95eac35 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -1072,9 +1072,6 @@ int mxs_nand_init(struct mxs_nand_info *info)
 			goto err2;
 	}
 
-	/* Init the DMA controller. */
-	mxs_dma_init();
-
 	/* Reset the GPMI block. */
 	mx28_reset_block(&gpmi_regs->hw_gpmi_ctrl0_reg);
 
-- 
1.7.1

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Marek Vasut]

Dear Fabio Estevam,

> Initialize the DMA controller so that the following errors are fixed:
> 
> U-Boot 2012.04-rc1-00001-g037cbfd (Apr 02 2012 - 10:57:48)
> 
> Freescale i.MX28 family at 454 MHz
> DRAM:  128 MiB
> MMC:   MXS MMC: 0
> MMC0: DMA transfer failed
> MMC0: DMA transfer failed
> MMC0: DMA transfer failed
> MMC0: DMA transfer failed
> MMC init failed
> 
> In order to not break the DMA NAND support, remove mxs_dma_init from the
> NAND driver now that it is called from the core dma file.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> --
> Changes since v1:
> - Call mxs_dma_init() from a common location and remove it
> from the mxs_nand driver
> 
>  drivers/dma/apbh_dma.c      |    2 ++
>  drivers/mtd/nand/mxs_nand.c |    3 ---
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma/apbh_dma.c b/drivers/dma/apbh_dma.c
> index c086629..68c4be2 100644
> --- a/drivers/dma/apbh_dma.c
> +++ b/drivers/dma/apbh_dma.c
> @@ -346,6 +346,8 @@ struct mxs_dma_desc *mxs_dma_desc_alloc(void)
>  	struct mxs_dma_desc *pdesc;
>  	uint32_t size;
> 
> +	/* Initialize the DMA controller. */
> +	mxs_dma_init();

This mxs_dma_desc_alloc() is called in a loop in the MXS NAND driver, this is 
even worse ;-)

Possibly some common mx28 init code would be better?

NAK (sorry, I know it hurts)

Best regards,
Marek Vasut

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Fabio Estevam]

On Mon, Apr 2, 2012 at 3:39 PM, Marek Vasut <marex@denx.de> wrote:

> This mxs_dma_desc_alloc() is called in a loop in the MXS NAND driver, this is
> even worse ;-)
>
> Possibly some common mx28 init code would be better?

If I put it in mx28.c like that:

+++ b/arch/arm/cpu/arm926ejs/mx28/mx28.c
@@ -172,6 +172,8 @@ int arch_cpu_init(void)
         */
        mxs_gpio_init();

+       mxs_dma_init();
+
        return 0;
 }
 #endif

,MMC still fails.

Still looking for a good place for calling mxs_dma_init();

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Marek Vasut]

Dear Fabio Estevam,

> On Mon, Apr 2, 2012 at 3:39 PM, Marek Vasut <marex@denx.de> wrote:
> > This mxs_dma_desc_alloc() is called in a loop in the MXS NAND driver,
> > this is even worse ;-)
> > 
> > Possibly some common mx28 init code would be better?
> 
> If I put it in mx28.c like that:
> 
> +++ b/arch/arm/cpu/arm926ejs/mx28/mx28.c
> @@ -172,6 +172,8 @@ int arch_cpu_init(void)
>          */
>         mxs_gpio_init();
> 
> +       mxs_dma_init();
> +
>         return 0;
>  }
>  #endif
> 
> ,MMC still fails.

Because that's called before relocation ;-)

> 
> Still looking for a good place for calling mxs_dma_init();

Best regards,
Marek Vasut

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Fabio Estevam]

On Mon, Apr 2, 2012 at 4:03 PM, Marek Vasut <marex@denx.de> wrote:

> Because that's called before relocation ;-)

What about this?

--- a/drivers/dma/apbh_dma.c
+++ b/drivers/dma/apbh_dma.c
@@ -35,6 +35,7 @@

 static struct mxs_dma_chan mxs_dma_channels[MXS_MAX_DMA_CHANNELS];

+static int first;
 /*
  * Test is the DMA channel is valid channel
  */
@@ -346,6 +347,10 @@ struct mxs_dma_desc *mxs_dma_desc_alloc(void)
        struct mxs_dma_desc *pdesc;
        uint32_t size;

+       if (!first)
+               mxs_dma_init();
+       first++;
+
        size = roundup(sizeof(struct mxs_dma_desc), MXS_DMA_ALIGNMENT);
        pdesc = memalign(MXS_DMA_ALIGNMENT, size);

Too ugly? Or any better place for calling  mxs_dma_init()?

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Marek Vasut]

Dear Fabio Estevam,

> On Mon, Apr 2, 2012 at 4:03 PM, Marek Vasut <marex@denx.de> wrote:
> > Because that's called before relocation ;-)
> 
> What about this?
> 
> --- a/drivers/dma/apbh_dma.c
> +++ b/drivers/dma/apbh_dma.c
> @@ -35,6 +35,7 @@
> 
>  static struct mxs_dma_chan mxs_dma_channels[MXS_MAX_DMA_CHANNELS];
> 
> +static int first;
>  /*
>   * Test is the DMA channel is valid channel
>   */
> @@ -346,6 +347,10 @@ struct mxs_dma_desc *mxs_dma_desc_alloc(void)
>         struct mxs_dma_desc *pdesc;
>         uint32_t size;
> 
> +       if (!first)
> +               mxs_dma_init();
> +       first++;
> +
>         size = roundup(sizeof(struct mxs_dma_desc), MXS_DMA_ALIGNMENT);
>         pdesc = memalign(MXS_DMA_ALIGNMENT, size);
> 
> Too ugly? Or any better place for calling  mxs_dma_init()?

I'd say arch_misc_init(), if it was before nand_init() though. Maybe we need 
arch_early_init_r() ?

Best regards,
Marek Vasut

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Anatolij Gustschin]

Hi Fabio,

On Mon, 2 Apr 2012 16:18:18 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> On Mon, Apr 2, 2012 at 4:03 PM, Marek Vasut <marex@denx.de> wrote:
> 
> > Because that's called before relocation ;-)
> 
> What about this?
> 
> --- a/drivers/dma/apbh_dma.c
> +++ b/drivers/dma/apbh_dma.c
> @@ -35,6 +35,7 @@
> 
>  static struct mxs_dma_chan mxs_dma_channels[MXS_MAX_DMA_CHANNELS];
> 
> +static int first;
>  /*
>   * Test is the DMA channel is valid channel
>   */
> @@ -346,6 +347,10 @@ struct mxs_dma_desc *mxs_dma_desc_alloc(void)
>         struct mxs_dma_desc *pdesc;
>         uint32_t size;
> 
> +       if (!first)
> +               mxs_dma_init();
> +       first++;
> +
>         size = roundup(sizeof(struct mxs_dma_desc), MXS_DMA_ALIGNMENT);
>         pdesc = memalign(MXS_DMA_ALIGNMENT, size);
> 
> Too ugly? Or any better place for calling  mxs_dma_init()?

Won't calling mxs_dma_init() from arch_misc_init() work?

Thanks,
Anatolij

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Marek Vasut]

Dear Anatolij Gustschin,

> Hi Fabio,
> 
> On Mon, 2 Apr 2012 16:18:18 -0300
> 
> Fabio Estevam <festevam@gmail.com> wrote:
> > On Mon, Apr 2, 2012 at 4:03 PM, Marek Vasut <marex@denx.de> wrote:
> > > Because that's called before relocation ;-)
> > 
> > What about this?
> > 
> > --- a/drivers/dma/apbh_dma.c
> > +++ b/drivers/dma/apbh_dma.c
> > @@ -35,6 +35,7 @@
> > 
> >  static struct mxs_dma_chan mxs_dma_channels[MXS_MAX_DMA_CHANNELS];
> > 
> > +static int first;
> > 
> >  /*
> >  
> >   * Test is the DMA channel is valid channel
> >   */
> > 
> > @@ -346,6 +347,10 @@ struct mxs_dma_desc *mxs_dma_desc_alloc(void)
> > 
> >         struct mxs_dma_desc *pdesc;
> >         uint32_t size;
> > 
> > +       if (!first)
> > +               mxs_dma_init();
> > +       first++;
> > +
> > 
> >         size = roundup(sizeof(struct mxs_dma_desc), MXS_DMA_ALIGNMENT);
> >         pdesc = memalign(MXS_DMA_ALIGNMENT, size);
> > 
> > Too ugly? Or any better place for calling  mxs_dma_init()?
> 
> Won't calling mxs_dma_init() from arch_misc_init() work?

No, because that's called after NAND is inited. And NAND needs DMA for it's init 
:(

> 
> Thanks,
> Anatolij

Best regards,
Marek Vasut

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Fabio Estevam]

On Mon, Apr 2, 2012 at 4:38 PM, Marek Vasut <marex@denx.de> wrote:

>> Won't calling mxs_dma_init() from arch_misc_init() work?
>
> No, because that's called after NAND is inited. And NAND needs DMA for it's init
> :(

Correct.

Should we go with my ugly hack then?

Or what do you mean when you proposed arch_early_init_r ?

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Marek Vasut]

Dear Fabio Estevam,

> On Mon, Apr 2, 2012 at 4:38 PM, Marek Vasut <marex@denx.de> wrote:
> >> Won't calling mxs_dma_init() from arch_misc_init() work?
> > 
> > No, because that's called after NAND is inited. And NAND needs DMA for
> > it's init
> > 
> > :(
> 
> Correct.
> 
> Should we go with my ugly hack then?
> 
> Or what do you mean when you proposed arch_early_init_r ?

Yes another hook function that'd be called early in board_init_r()

btw. Fabio, I really appreciate your work on this, sorry to keep tormenting you 
so much.

Best regards,
Marek Vasut

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Fabio Estevam]

On Mon, Apr 2, 2012 at 4:54 PM, Marek Vasut <marex@denx.de> wrote:

> Yes another hook function that'd be called early in board_init_r()

Ok, please see the patch below and let me know if you like it.

If you do,  then I can probably split it in 2 parts:
one that introduces arch_early_init_r() and another one that fixes the
mmc problem on mx28.


diff --git a/arch/arm/cpu/arm926ejs/mx28/mx28.c
b/arch/arm/cpu/arm926ejs/mx28/mx28.c
index cf6d4e9..6c148a8 100644
--- a/arch/arm/cpu/arm926ejs/mx28/mx28.c
+++ b/arch/arm/cpu/arm926ejs/mx28/mx28.c
@@ -34,6 +34,7 @@
 #include <asm/arch/iomux.h>
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/sys_proto.h>
+#include <asm/arch/dma.h>

 DECLARE_GLOBAL_DATA_PTR;

@@ -297,3 +298,14 @@ U_BOOT_CMD(
 	"display clocks",
 	""
 );
+
+#ifdef CONFIG_ARCH_EARLY_INIT_R
+int arch_early_init_r(void)
+{
+#ifdef	CONFIG_APBH_DMA
+	/* Initialize the DMA controller. */
+	mxs_dma_init();
+#endif	
+	return 0;
+}
+#endif
diff --git a/arch/arm/include/asm/u-boot-arm.h
b/arch/arm/include/asm/u-boot-arm.h
index 4ca75f9..9f3cae5 100644
--- a/arch/arm/include/asm/u-boot-arm.h
+++ b/arch/arm/include/asm/u-boot-arm.h
@@ -52,6 +52,7 @@ void	cpu_init_cp15(void);
 /* cpu/.../arch/cpu.c */
 int	arch_cpu_init(void);
 int	arch_misc_init(void);
+int	arch_early_init_r(void);

 /* board/.../... */
 int	board_init(void);
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 5270c11..025d6ca 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -500,6 +500,10 @@ void board_init_r(gd_t *id, ulong dest_addr)
 	malloc_start = dest_addr - TOTAL_MALLOC_LEN;
 	mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);

+#ifdef CONFIG_ARCH_EARLY_INIT_R
+	arch_early_init_r();
+#endif
+
 #if !defined(CONFIG_SYS_NO_FLASH)
 	puts("Flash: ");

diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index 4b1297a..95eac35 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -1072,9 +1072,6 @@ int mxs_nand_init(struct mxs_nand_info *info)
 			goto err2;
 	}

-	/* Init the DMA controller. */
-	mxs_dma_init();
-
 	/* Reset the GPMI block. */
 	mx28_reset_block(&gpmi_regs->hw_gpmi_ctrl0_reg);

diff --git a/include/configs/m28evk.h b/include/configs/m28evk.h
index 8b83180..b0e6d3b 100644
--- a/include/configs/m28evk.h
+++ b/include/configs/m28evk.h
@@ -42,6 +42,7 @@
 #define	CONFIG_BOARD_EARLY_INIT_F
 #define	CONFIG_ARCH_CPU_INIT
 #define	CONFIG_ARCH_MISC_INIT
+#define	CONFIG_ARCH_EARLY_INIT_R

 /*
  * SPL
diff --git a/include/configs/mx28evk.h b/include/configs/mx28evk.h
index 705fdab..350e833 100644
--- a/include/configs/mx28evk.h
+++ b/include/configs/mx28evk.h
@@ -36,6 +36,7 @@
 #define CONFIG_BOARD_EARLY_INIT_F
 #define CONFIG_ARCH_CPU_INIT
 #define CONFIG_ARCH_MISC_INIT
+#define CONFIG_ARCH_EARLY_INIT_R

 /*
  * SPL
-- 
1.7.1

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Marek Vasut]

Dear Fabio Estevam,

> On Mon, Apr 2, 2012 at 4:54 PM, Marek Vasut <marex@denx.de> wrote:
> > Yes another hook function that'd be called early in board_init_r()
> 
> Ok, please see the patch below and let me know if you like it.
> 
> If you do,  then I can probably split it in 2 parts:
> one that introduces arch_early_init_r() and another one that fixes the
> mmc problem on mx28.

Well ... CCing Albert to express his concerns about this.

> 
> diff --git a/arch/arm/cpu/arm926ejs/mx28/mx28.c
> b/arch/arm/cpu/arm926ejs/mx28/mx28.c
> index cf6d4e9..6c148a8 100644
> --- a/arch/arm/cpu/arm926ejs/mx28/mx28.c
> +++ b/arch/arm/cpu/arm926ejs/mx28/mx28.c
> @@ -34,6 +34,7 @@
>  #include <asm/arch/iomux.h>
>  #include <asm/arch/imx-regs.h>
>  #include <asm/arch/sys_proto.h>
> +#include <asm/arch/dma.h>
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> @@ -297,3 +298,14 @@ U_BOOT_CMD(
>  	"display clocks",
>  	""
>  );
> +
> +#ifdef CONFIG_ARCH_EARLY_INIT_R
> +int arch_early_init_r(void)
> +{
> +#ifdef	CONFIG_APBH_DMA
> +	/* Initialize the DMA controller. */
> +	mxs_dma_init();
> +#endif
> +	return 0;
> +}
> +#endif
> diff --git a/arch/arm/include/asm/u-boot-arm.h
> b/arch/arm/include/asm/u-boot-arm.h
> index 4ca75f9..9f3cae5 100644
> --- a/arch/arm/include/asm/u-boot-arm.h
> +++ b/arch/arm/include/asm/u-boot-arm.h
> @@ -52,6 +52,7 @@ void	cpu_init_cp15(void);
>  /* cpu/.../arch/cpu.c */
>  int	arch_cpu_init(void);
>  int	arch_misc_init(void);
> +int	arch_early_init_r(void);
> 
>  /* board/.../... */
>  int	board_init(void);
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 5270c11..025d6ca 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -500,6 +500,10 @@ void board_init_r(gd_t *id, ulong dest_addr)
>  	malloc_start = dest_addr - TOTAL_MALLOC_LEN;
>  	mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
> 
> +#ifdef CONFIG_ARCH_EARLY_INIT_R
> +	arch_early_init_r();
> +#endif
> +
>  #if !defined(CONFIG_SYS_NO_FLASH)
>  	puts("Flash: ");
> 
> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> index 4b1297a..95eac35 100644
> --- a/drivers/mtd/nand/mxs_nand.c
> +++ b/drivers/mtd/nand/mxs_nand.c
> @@ -1072,9 +1072,6 @@ int mxs_nand_init(struct mxs_nand_info *info)
>  			goto err2;
>  	}
> 
> -	/* Init the DMA controller. */
> -	mxs_dma_init();
> -
>  	/* Reset the GPMI block. */
>  	mx28_reset_block(&gpmi_regs->hw_gpmi_ctrl0_reg);
> 
> diff --git a/include/configs/m28evk.h b/include/configs/m28evk.h
> index 8b83180..b0e6d3b 100644
> --- a/include/configs/m28evk.h
> +++ b/include/configs/m28evk.h
> @@ -42,6 +42,7 @@
>  #define	CONFIG_BOARD_EARLY_INIT_F
>  #define	CONFIG_ARCH_CPU_INIT
>  #define	CONFIG_ARCH_MISC_INIT
> +#define	CONFIG_ARCH_EARLY_INIT_R
> 
>  /*
>   * SPL
> diff --git a/include/configs/mx28evk.h b/include/configs/mx28evk.h
> index 705fdab..350e833 100644
> --- a/include/configs/mx28evk.h
> +++ b/include/configs/mx28evk.h
> @@ -36,6 +36,7 @@
>  #define CONFIG_BOARD_EARLY_INIT_F
>  #define CONFIG_ARCH_CPU_INIT
>  #define CONFIG_ARCH_MISC_INIT
> +#define CONFIG_ARCH_EARLY_INIT_R
> 
>  /*
>   * SPL

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Fabio Estevam]

On Mon, Apr 2, 2012 at 5:52 PM, Marek Vasut <marex@denx.de> wrote:

> Well ... CCing Albert to express his concerns about this.

Ok, great.

Just to let you know that I also tested NAND on my board (will add
mx28evk support for NAND later).

[U-Boot] [PATCH v2] mmc: mxsmmc: Fix operation with DMA

[Marek Vasut]

Dear Fabio Estevam,

> On Mon, Apr 2, 2012 at 5:52 PM, Marek Vasut <marex@denx.de> wrote:
> > Well ... CCing Albert to express his concerns about this.
> 
> Ok, great.
> 
> Just to let you know that I also tested NAND on my board (will add
> mx28evk support for NAND later).

Good job, Fabio! :)

Best regards,
Marek Vasut